Re: [Xen-devel] [PATCH 5/9] x86/amd: Probe for legacy SSBD interfaces on boot

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.12.18 at 17:18, <andrew.cooper3@xxxxxxxxxx> wrote:
> +static void __init noinline amd_probe_legacy_ssbd(void)
> +{
> +     uint64_t new;
> +
> +     /*
> +      * Search for mechanisms of controlling Memory Disambiguation.
> +      *
> +      * If the CPU reports that it is fixed, there is nothing to do.  If we
> +      * have an architectural MSR_SPEC_CTRL.SSBD control, leave everything
> +      * to the common code.
> +      */
> +     if (cpu_has_amd_ssb_no || cpu_has_amd_ssbd)
> +             return;
> +
> +     /* Use MSR_VIRT_SPEC_CTRL if our hypervisor offers it. */
> +     if (cpu_has_virt_sc_ssbd) {
> +             setup_force_cpu_cap(X86_FEATURE_LEGACY_SSBD);

The only issue I have is with this double meaning of the new
synthetic feature here and ...

> +             return;
> +     }
> +
> +     /* Probe for LS_CFG settings. */
> +     switch (boot_cpu_data.x86) {
> +     default: return; /* No known LS_CFG settings. */
> +     case 0x15: ls_cfg_ssbd_mask = 1ull << 54; break;
> +     case 0x16: ls_cfg_ssbd_mask = 1ull << 33; break;
> +     case 0x17: ls_cfg_ssbd_mask = 1ull << 10; break;
> +     }
> +
> +     /*
> +      * MSR_AMD64_LS_CFG isn't architectural, and may not be virtualised
> +      * fully.  Check that we can actually flip the bit before concluding
> +      * that LS_CFG is available for use.
> +      */
> +     if (rdmsr_safe(MSR_AMD64_LS_CFG, ls_cfg_base) ||
> +         wrmsr_safe(MSR_AMD64_LS_CFG, ls_cfg_base ^ ls_cfg_ssbd_mask))
> +             return;
> +
> +     rdmsrl(MSR_AMD64_LS_CFG, new);
> +     if (new != (ls_cfg_base ^ ls_cfg_ssbd_mask))
> +             return;
> +
> +     /*
> +      * Leave ls_cfg_base with the bit clear.  This is Xen's overall
> +      * default, and it simplifies the context switch logic.
> +      */
> +     ls_cfg_base &= ~ls_cfg_ssbd_mask;
> +     if ((new != ls_cfg_base) && wrmsr_safe(MSR_AMD64_LS_CFG, ls_cfg_base))
> +             return;
> +
> +     /* LS_CFG appears to work fully.  Lets choose to use it. */
> +     setup_force_cpu_cap(X86_FEATURE_LEGACY_SSBD);

... here. Granted you explicitly say so ...

> --- a/xen/include/asm-x86/cpufeatures.h
> +++ b/xen/include/asm-x86/cpufeatures.h
> @@ -25,6 +25,7 @@ XEN_CPUFEATURE(XEN_SMAP,        (FSCAPINTS+0)*32+11) /* 
> SMAP gets used by Xen it
>  XEN_CPUFEATURE(LFENCE_DISPATCH, (FSCAPINTS+0)*32+12) /* lfence set as 
> Dispatch Serialising */
>  XEN_CPUFEATURE(IND_THUNK_LFENCE,(FSCAPINTS+0)*32+13) /* Use IND_THUNK_LFENCE 
> */
>  XEN_CPUFEATURE(IND_THUNK_JMP,   (FSCAPINTS+0)*32+14) /* Use IND_THUNK_JMP */
> +XEN_CPUFEATURE(LEGACY_SSBD,     (FSCAPINTS+0)*32+15) /* LS_CFG or 
> VIRT_SPEC_CTRL available for SSBD */

... here, but I still will need to see how this gets used before
giving my ack here. Additionally I can see "legacy" as a suitable
name for the LS_CFG approach, but does this also fit the
VIRT_SPEC_CTRL one?

One other question: There's now redundant code in init_amd()
handling opt_ssbd. Would it not fit here to remove/replace that?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel