Re: [PATCH 9/9] x86/spec-ctrl: Hide RDRAND by default on IvyBridge

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.06.2020 16:15, Andrew Cooper wrote:
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -512,11 +512,21 @@ The Speculation Control hardware features `srbds-ctrl`, 
> `md-clear`, `ibrsb`,
>  `stibp`, `ibpb`, `l1d-flush` and `ssbd` are used by default if available and
>  applicable.  They can all be ignored.
>  
> -`rdrand` and `rdseed` can be ignored, as a mitigation to XSA-320 /
> -CVE-2020-0543.  The RDRAND feature is disabled by default on certain AMD
> -systems, due to possible malfunctions after ACPI S3 suspend/resume.  `rdrand`
> -may be used in its positive form to override Xen's default behaviour on these
> -systems, and make the feature fully usable.
> +`rdrand` and `rdseed` have multiple interactions.
> +
> +*   For Special Register Buffer Data Sampling (SRBDS, XSA-320, 
> CVE-2020-0543),
> +    RDRAND and RDSEED can be ignored.
> +
> +    Due to the absence microcode to address SRBDS on IvyBridge hardware, the

Nit: "... absence of microcode ..."

> +    RDRAND feature is hidden by default for guests, unless `rdrand` is used 
> in
> +    its positive form.  Irrespective of the default setting here, VMs can use
> +    RDRAND if explicitly enabled in guest config file, and VMs already using
> +    RDRAND can migrate in.

I'm somewhat confused by the use of "default setting" here, when at the same
time you talk about our default behavior for guests. Aiui the two "default"
mean different things, so I'd suggest dropping that latter "default".

This raises a question though: Is disabling RDRAND just for guests good
enough? I.e. what about Xen's own uses of RDRAND? There may not be any
problematic ones right now, but wouldn't there be a latent issue no-one is
going to notice?

> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -503,6 +503,9 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t 
> domid, bool restore,
>       */
>      if ( restore )
>      {
> +        if ( test_bit(X86_FEATURE_RDRAND, host_featureset) && 
> !p->basic.rdrand )
> +            p->basic.rdrand = true;

Same question as before: Why do you derive from the host feature set rather
than the domain type's maximum one?

> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -340,6 +340,25 @@ static void __init calculate_host_policy(void)
>      }
>  }
>  
> +static void __init guest_common_default_feature_adjustments(uint32_t *fs)
> +{
> +    /*
> +     * IvyBridge client parts suffer from leakage of RDRAND data due to SRBDS
> +     * (XSA-320 / CVE-2020-0543), and won't be receiving microcode to
> +     * compensate.
> +     *
> +     * Mitigate by hiding RDRAND from guests by default, unless explicitly
> +     * overridden on the Xen command line (cpuid=rdrand).  Irrespective of 
> the
> +     * default setting, guests can use RDRAND if explicitly enabled
> +     * (cpuid="host,rdrand=1") in the VM's config file, and VMs which were
> +     * previously using RDRAND can migrate in.
> +     */
> +    if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
> +         boot_cpu_data.x86 == 6 && boot_cpu_data.x86_model == 0x3a &&

This is the first time (description plus patch so far) that the issue
gets mentioned to be for and the workaround restricted to client parts
only. If so, I think at least the doc should say so too.

Jan