>>> On 27.08.18 at 18:54, wrote:
> Edit the initialization code for AMD's SSBD via the LS_CFG MSR and
> enable it to pass the status to the initial spec-ctrl print_details at
> boot.
>
> Signed-off-by: Brian Woods
> ---
I think I had indicated so before: A brief revision log is expected here,
in particular to aid people having looked at prior versions of a patch.
Whether you also put such a log in the cover letter is up to you, but
there it's not normally going to be patch specific.
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -604,7 +604,7 @@ static void init_amd(struct cpuinfo_x86 *c)
>* If the user has explicitly chosen to disable Memory Disambiguation
>* to mitigiate Speculative Store Bypass, poke the appropriate MSR.
>*/
> - if (opt_ssbd) {
> + if (!ssbd_amd_ls_cfg_mask) {
> int bit = -1;
>
> switch (c->x86) {
> @@ -613,8 +613,14 @@ static void init_amd(struct cpuinfo_x86 *c)
> case 0x17: bit = 10; break;
> }
>
> - if (bit >= 0 && !rdmsr_safe(MSR_AMD64_LS_CFG, value)) {
> - value |= 1ull << bit;
> + if (bit >= 0)
> + ssbd_amd_ls_cfg_mask = 1ull << bit;
> + }
> +
> + if (ssbd_amd_ls_cfg_mask && !rdmsr_safe(MSR_AMD64_LS_CFG, value)) {
> + ssbd_amd_ls_cfg_av = true;
"av" is standing for "avail" I guess? If so, I'd prefer if you spelled it out.
Also two of the three comments given on v2 still apply here. Please
address _all_ comments either verbally or by code changes before
sending a new version.
> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -58,6 +58,9 @@ paddr_t __read_mostly l1tf_addr_mask, __read_mostly
> l1tf_safe_maddr;
> static bool __initdata cpu_has_bug_l1tf;
> static unsigned int __initdata l1d_maxphysaddr;
>
> +bool ssbd_amd_ls_cfg_av;
__read_mostly (afaict)
Also the variable seems pointless - this ...
> +uint64_t __read_mostly ssbd_amd_ls_cfg_mask;
... variable being (non-)zero should suffice as indication.
Otherwise the definition of this variable in this file looks
wrong, as there's no reference to it in here.
> @@ -281,11 +284,12 @@ static void __init print_details(enum ind_thunk thunk,
> uint64_t caps)
> printk("Speculative mitigation facilities:\n");
>
> /* Hardware features which pertain to speculative mitigations. */
> -printk(" Hardware features:%s%s%s%s%s%s%s%s%s%s\n",
> +printk(" Hardware features:%s%s%s%s%s%s%s%s%s%s%s\n",
> (_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
> (_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP" : "",
> (_7d0 & cpufeat_mask(X86_FEATURE_L1D_FLUSH)) ? " L1D_FLUSH" : "",
> (_7d0 & cpufeat_mask(X86_FEATURE_SSBD)) ? " SSBD" : "",
> + ssbd_amd_ls_cfg_av ? " LS_CFG_SSBD" : "",
> (e8b & cpufeat_mask(X86_FEATURE_IBPB)) ? " IBPB" : "",
> (caps & ARCH_CAPABILITIES_IBRS_ALL) ? " IBRS_ALL" : "",
> (caps & ARCH_CAPABILITIES_RDCL_NO) ? " RDCL_NO" : "",
> @@ -305,7 +309,7 @@ static void __init print_details(enum ind_thunk thunk,
> uint64_t caps)
> "\n");
>
> /* Settings for Xen's protection, irrespective of guests. */
> -printk(" Xen settings: BTI-Thunk %s, SPEC_CTRL: %s%s, Other:%s%s\n",
> +printk(" Xen settings: BTI-Thunk %s, SPEC_CTRL: %s%s%s, Other:%s%s\n",
> thunk == THUNK_NONE ? "N/A" :
> thunk == THUNK_RETPOLINE ? "RETPOLINE" :
> thunk == THUNK_LFENCE? "LFENCE" :
> @@ -314,6 +318,8 @@ static void __init print_details(enum ind_thunk thunk,
> uint64_t caps)
> (default_xen_spec_ctrl & SPEC_CTRL_IBRS) ? "IBRS+" : "IBRS-",
> !boot_cpu_has(X86_FEATURE_SSBD) ? "" :
> (default_xen_spec_ctrl & SPEC_CTRL_SSBD) ? " SSBD+" : " SSBD-",
> + !ssbd_amd_ls_cfg_av ? "" :
> + opt_ssbd ? " LS_CFG_SSBD+" : "
> LS_CFG_SSBD-",
> opt_ibpb ? " IBPB" : "",
> opt_l1d_flush ? " L1D_FLUSH" : "");
>
> diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
> index 8f8aad40bb..1b9101a988 100644
> --- a/xen/include/asm-x86/spec_ctrl.h
> +++ b/xen/include/asm-x86/spec_ctrl.h
> @@ -50,6 +50,9 @@ extern int8_t opt_pv_l1tf;
> */
> extern paddr_t l1tf_addr_mask, l1tf_safe_maddr;
>
> +extern bool ssbd_amd_ls_cfg_av;
> +extern uint64_t ssbd_amd_ls_cfg_mask;
> +
> static inline void init_shadow_spec_ctrl_state(void)
> {
> struct cpu_info *info = get_cpu_info();
> --
> 2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
