| Message ID | 20200615141532.1927-10-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | XSA-320 follow for IvyBridge | expand |
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
On 16/06/2020 11:00, Jan Beulich wrote: > 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 ..." Fixed. > >> + 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". Ok, done. > > 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? I was sorely tempted to delete all of Xen's use of RDRAND, seeing as its not even safe to the AMD issue. What we don't have is a "no-xen" concept for CPUID features, so I can't stop Xen using it without hiding it from all guests, which in turn would render the point of this series useless, and reintroduce the migration problems we're trying to code around. I was planning to leave the Xen uses as-are for now. > >> --- 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. > >> --- 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. ~Andrew
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. >>> --- 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@suse.com> Jan
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@suse.com> Thanks. ~Andrew
diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index fde749c669..c8ebfaf813 100644 --- 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 + 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. + +* The RDRAND feature is disabled by default on AMD Fam15/16 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. ### cpuid_mask_cpu > `= fam_0f_rev_[cdefg] | fam_10_rev_[bc] | fam_11_rev_b` diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c index 5649913e69..877a5601f3 100644 --- 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; + if ( di.hvm ) { p->feat.mpx = test_bit(X86_FEATURE_MPX, host_featureset); diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c index f2fc0aa895..6a4a787b68 100644 --- 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 && + cpu_has_rdrand && !is_forced_cpu_cap(X86_FEATURE_RDRAND) ) + __clear_bit(X86_FEATURE_RDRAND, fs); +} + static void __init guest_common_feature_adjustments(uint32_t *fs) { /* Unconditionally claim to be able to set the hypervisor bit. */ @@ -403,6 +422,7 @@ static void __init calculate_pv_def_policy(void) pv_featureset[i] &= pv_def_featuremask[i]; guest_common_feature_adjustments(pv_featureset); + guest_common_default_feature_adjustments(pv_featureset); sanitise_featureset(pv_featureset); cpuid_featureset_to_policy(pv_featureset, p); @@ -485,6 +505,7 @@ static void __init calculate_hvm_def_policy(void) hvm_featureset[i] &= hvm_featuremask[i]; guest_common_feature_adjustments(hvm_featureset); + guest_common_default_feature_adjustments(hvm_featureset); sanitise_featureset(hvm_featureset); cpuid_featureset_to_policy(hvm_featureset, p); diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h index af1b8a96a6..fe7492a225 100644 --- a/xen/include/public/arch-x86/cpufeatureset.h +++ b/xen/include/public/arch-x86/cpufeatureset.h @@ -149,7 +149,7 @@ XEN_CPUFEATURE(XSAVE, 1*32+26) /*A XSAVE/XRSTOR/XSETBV/XGETBV */ XEN_CPUFEATURE(OSXSAVE, 1*32+27) /*! OSXSAVE */ XEN_CPUFEATURE(AVX, 1*32+28) /*A Advanced Vector Extensions */ XEN_CPUFEATURE(F16C, 1*32+29) /*A Half-precision convert instruction */ -XEN_CPUFEATURE(RDRAND, 1*32+30) /*A Digital Random Number Generator */ +XEN_CPUFEATURE(RDRAND, 1*32+30) /*!A Digital Random Number Generator */ XEN_CPUFEATURE(HYPERVISOR, 1*32+31) /*!A Running under some hypervisor */ /* AMD-defined CPU features, CPUID level 0x80000001.edx, word 2 */
To combat the absence of mitigating microcode, arrange to hide RDRAND by default on IvyBridge hardware. Adjust the default feature derivation to hide RDRAND on IvyBridge client parts, unless `cpuid=rdrand` is explicitly provided. Adjust the restore path in xc_cpuid_apply_policy() to not hide RDRAND from VMs which migrated from pre-4.14. In all cases, individual guests can continue using RDRAND if explicitly enabled in their config files. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Ian Jackson <Ian.Jackson@citrix.com> CC: Wei Liu <wl@xen.org> CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wl@xen.org> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Paul Durrant <paul@xen.org> --- docs/misc/xen-command-line.pandoc | 20 +++++++++++++++----- tools/libxc/xc_cpuid_x86.c | 3 +++ xen/arch/x86/cpuid.c | 21 +++++++++++++++++++++ xen/include/public/arch-x86/cpufeatureset.h | 2 +- 4 files changed, 40 insertions(+), 6 deletions(-)