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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 17/06/2020 11:39, Jan Beulich wrote:
> On 16.06.2020 18:26, Andrew Cooper wrote:
>> On 16/06/2020 11:00, Jan Beulich wrote:
>>> On 15.06.2020 16:15, Andrew Cooper wrote:
>>>> --- 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?
>> Answer the same as previous.
>>
>> Although I do see now that this should be simplified to:
>>
>>     p->basic.rdrand = test_bit(X86_FEATURE_RDRAND, host_featureset);
>>
>> which I've done.
> Right. It makes even more noticeable though that this may mean a
> new feature suddenly appearing after the guest was migrated. But
> aiui this still is the default behavior for all features anyway.

That is how migration always worked, until my migration v3 work in this
release.

I'm still surprised that it did, but both Linux and Windows were fine
with FMS changing on migrate (Linux because it never checked again,
while Windows would notice and install a new CPU HAL driver.)

>>>> --- 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.
>> I've updated the command line doc, and patch subject.
> Thanks - with the adjustments
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

~Andrew