| Message ID | 20240229104304.2478614-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/cpu-policy: Fix x2APIC visibility for PV guests | expand |
On Thu, Feb 29, 2024 at 10:43:04AM +0000, Andrew Cooper wrote: > Right now, the host x2APIC setting filters into the PV max and default > policies, yet PV guests cannot set MSR_APIC_BASE.EXTD or access any of the > x2APIC MSR range. Therefore they absolutely shouldn't see the x2APIC bit. > > Linux has workarounds for the collateral damage caused by this leakage; it > unconditionally filters out the x2APIC CPUID bit, and EXTD when reading > MSR_APIC_BASE. > > Hide the x2APIC bit in the PV default policy, but for compatibility, tolerate > incoming VMs which already saw the bit. This is logic from before the > default/max split in Xen 4.14 which wasn't correctly adjusted at the time. > > Update the annotation from !A to !S which slightly better describes that it > doesn't really exist in PV guests. HVM guests, for which x2APIC can be > emulated completely, already has it unconditionally set in the max policy. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Wei Liu <wl@xen.org> > > This wants backporting as far as people can tollerate, but it's really not > obvious which commit in 4.14 should be referenced in a Fixes: tag. Oh, so we didn't use to expose x2APIC in Xen < 4.14 for PV at all? I think this need mentioning in the commit message, as it's not clear whether x2APIC has always been advertised to guests. If it's indeed only Xen 4.14 that started exposing the flag, it's IMO less dangerous to stop exposing it. My main concern would be OSes having grow some dependency on it, and us no longer exposing it causing collateral damage (which would be an OS bug anyway). > --- > xen/arch/x86/cpu-policy.c | 19 +++++++++++++++++-- > xen/include/public/arch-x86/cpufeatureset.h | 2 +- > 2 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c > index 10079c26ae24..a0205672428d 100644 > --- a/xen/arch/x86/cpu-policy.c > +++ b/xen/arch/x86/cpu-policy.c > @@ -534,6 +534,14 @@ static void __init calculate_pv_max_policy(void) > *p = host_cpu_policy; > x86_cpu_policy_to_featureset(p, fs); > > + /* > + * Xen at the time of writing (Feb 2024, 4.19 dev cycle) used to leak the > + * host x2APIC capability into PV guests, but never supported the guest > + * trying to turn x2APIC mode on. Tolerate an incoming VM which saw the > + * x2APIC CPUID bit. > + */ > + __set_bit(X86_FEATURE_X2APIC, fs); > + > for ( i = 0; i < ARRAY_SIZE(fs); ++i ) > fs[i] &= pv_max_featuremask[i]; > > @@ -566,6 +574,14 @@ static void __init calculate_pv_def_policy(void) > *p = pv_max_cpu_policy; > x86_cpu_policy_to_featureset(p, fs); > > + /* > + * PV guests have never been able to use x2APIC mode, but at the time of > + * writing (Feb 2024, 4.19 dev cycle), the host value used to leak into > + * guests. Hide it by default so new guests don't get mislead into > + * thinking that they can use x2APIC. > + */ > + __clear_bit(X86_FEATURE_X2APIC, fs); IIRC if you use the 'S' tag it won't be added to the default PV policy already, so there should be nothing to clear? pv_def_featuremask shouldn't contain the bit in the first place. Thanks, Roger.
On 29.02.2024 11:43, Andrew Cooper wrote: > Right now, the host x2APIC setting filters into the PV max and default > policies, yet PV guests cannot set MSR_APIC_BASE.EXTD or access any of the > x2APIC MSR range. Therefore they absolutely shouldn't see the x2APIC bit. > > Linux has workarounds for the collateral damage caused by this leakage; it > unconditionally filters out the x2APIC CPUID bit, and EXTD when reading > MSR_APIC_BASE. > > Hide the x2APIC bit in the PV default policy, but for compatibility, tolerate > incoming VMs which already saw the bit. This is logic from before the > default/max split in Xen 4.14 which wasn't correctly adjusted at the time. What about guest_cpuid()'s handling of leaf 0xb then? The %edx value will change once a guest is rebooted, aiui. The comment in recalculate_cpuid_policy() that you update refers to that. > Update the annotation from !A to !S which slightly better describes that it > doesn't really exist in PV guests. HVM guests, for which x2APIC can be > emulated completely, already has it unconditionally set in the max policy. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Wei Liu <wl@xen.org> > > This wants backporting as far as people can tollerate, but it's really not > obvious which commit in 4.14 should be referenced in a Fixes: tag. Why 4.14? In 4.7.0 I see ... > @@ -830,11 +846,10 @@ void recalculate_cpuid_policy(struct domain *d) > } > > /* > - * Allow the toolstack to set HTT, X2APIC and CMP_LEGACY. These bits > + * Allow the toolstack to set HTT and CMP_LEGACY. These bits > * affect how to interpret topology information in other cpuid leaves. > */ > __set_bit(X86_FEATURE_HTT, max_fs); > - __set_bit(X86_FEATURE_X2APIC, max_fs); > __set_bit(X86_FEATURE_CMP_LEGACY, max_fs); > > /* ... these adjustments, just still in calculate_pv_featureset(). I haven't gone further backwards to check if/when this exposure has really appeared. I wouldn't be surprised if it's been like that for all the time since we gained x2APIC support in the hypervisor. Jan
On 29/02/2024 11:56 am, Roger Pau Monné wrote: > On Thu, Feb 29, 2024 at 10:43:04AM +0000, Andrew Cooper wrote: >> Right now, the host x2APIC setting filters into the PV max and default >> policies, yet PV guests cannot set MSR_APIC_BASE.EXTD or access any of the >> x2APIC MSR range. Therefore they absolutely shouldn't see the x2APIC bit. >> >> Linux has workarounds for the collateral damage caused by this leakage; it >> unconditionally filters out the x2APIC CPUID bit, and EXTD when reading >> MSR_APIC_BASE. >> >> Hide the x2APIC bit in the PV default policy, but for compatibility, tolerate >> incoming VMs which already saw the bit. This is logic from before the >> default/max split in Xen 4.14 which wasn't correctly adjusted at the time. >> >> Update the annotation from !A to !S which slightly better describes that it >> doesn't really exist in PV guests. HVM guests, for which x2APIC can be >> emulated completely, already has it unconditionally set in the max policy. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> CC: Wei Liu <wl@xen.org> >> >> This wants backporting as far as people can tollerate, but it's really not >> obvious which commit in 4.14 should be referenced in a Fixes: tag. > Oh, so we didn't use to expose x2APIC in Xen < 4.14 for PV at all? We did conditionally expose x2APIC to PV guests prior to that. What we didn't have in Xen < 4.14 was a split between max and default policies. Everything before 4.14 is far harder to reason about. > > I think this need mentioning in the commit message, as it's not clear > whether x2APIC has always been advertised to guests. > > If it's indeed only Xen 4.14 that started exposing the flag, it's IMO > less dangerous to stop exposing it. My main concern would be OSes > having grow some dependency on it, and us no longer exposing it > causing collateral damage (which would be an OS bug anyway). As I said, Linux explicitly self-hides the cap, because at one point it tried turning x2APIC on and got unhappy at getting a #GP back. Juergen may remember better. IIRC it was fallout from making WRMSR not always-silently-safe.AFAICT NetBSD doesn't explode because it's x2APIC-enablement logic is inside #ifndef XENPV. >> --- >> xen/arch/x86/cpu-policy.c | 19 +++++++++++++++++-- >> xen/include/public/arch-x86/cpufeatureset.h | 2 +- >> 2 files changed, 18 insertions(+), 3 deletions(-) >> >> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c >> index 10079c26ae24..a0205672428d 100644 >> --- a/xen/arch/x86/cpu-policy.c >> +++ b/xen/arch/x86/cpu-policy.c >> @@ -534,6 +534,14 @@ static void __init calculate_pv_max_policy(void) >> *p = host_cpu_policy; >> x86_cpu_policy_to_featureset(p, fs); >> >> + /* >> + * Xen at the time of writing (Feb 2024, 4.19 dev cycle) used to leak the >> + * host x2APIC capability into PV guests, but never supported the guest >> + * trying to turn x2APIC mode on. Tolerate an incoming VM which saw the >> + * x2APIC CPUID bit. >> + */ >> + __set_bit(X86_FEATURE_X2APIC, fs); >> + >> for ( i = 0; i < ARRAY_SIZE(fs); ++i ) >> fs[i] &= pv_max_featuremask[i]; >> >> @@ -566,6 +574,14 @@ static void __init calculate_pv_def_policy(void) >> *p = pv_max_cpu_policy; >> x86_cpu_policy_to_featureset(p, fs); >> >> + /* >> + * PV guests have never been able to use x2APIC mode, but at the time of >> + * writing (Feb 2024, 4.19 dev cycle), the host value used to leak into >> + * guests. Hide it by default so new guests don't get mislead into >> + * thinking that they can use x2APIC. >> + */ >> + __clear_bit(X86_FEATURE_X2APIC, fs); > IIRC if you use the 'S' tag it won't be added to the default PV policy > already, so there should be nothing to clear? pv_def_featuremask > shouldn't contain the bit in the first place. Bah. That means there's a bug visible in context. Serves me right for last minute clean-up... I need to set the bit after applying pv_max_featuremask[]. ~Andrew
On 29/02/2024 12:47 pm, Jan Beulich wrote: > On 29.02.2024 11:43, Andrew Cooper wrote: >> Right now, the host x2APIC setting filters into the PV max and default >> policies, yet PV guests cannot set MSR_APIC_BASE.EXTD or access any of the >> x2APIC MSR range. Therefore they absolutely shouldn't see the x2APIC bit. >> >> Linux has workarounds for the collateral damage caused by this leakage; it >> unconditionally filters out the x2APIC CPUID bit, and EXTD when reading >> MSR_APIC_BASE. >> >> Hide the x2APIC bit in the PV default policy, but for compatibility, tolerate >> incoming VMs which already saw the bit. This is logic from before the >> default/max split in Xen 4.14 which wasn't correctly adjusted at the time. > What about guest_cpuid()'s handling of leaf 0xb then? The %edx value > will change once a guest is rebooted, aiui. The comment in > recalculate_cpuid_policy() that you update refers to that. That comment is going in the next patch irrespective. But yes - this will change leaf 0xb from being host-conditionally-visible to always hidden. PV guests don't have any coherent idea of topology. Linux (with the topo fixes) now explicitly ignores everything it can see and just fakes up a flat non-SMT topology in a single package. For PV dom0, where it sees the host MADT, the values in leaf 0xb are bogus as they don't come from the same enumeration space. >> Update the annotation from !A to !S which slightly better describes that it >> doesn't really exist in PV guests. HVM guests, for which x2APIC can be >> emulated completely, already has it unconditionally set in the max policy. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> CC: Wei Liu <wl@xen.org> >> >> This wants backporting as far as people can tollerate, but it's really not >> obvious which commit in 4.14 should be referenced in a Fixes: tag. > Why 4.14? In 4.7.0 I see ... > >> @@ -830,11 +846,10 @@ void recalculate_cpuid_policy(struct domain *d) >> } >> >> /* >> - * Allow the toolstack to set HTT, X2APIC and CMP_LEGACY. These bits >> + * Allow the toolstack to set HTT and CMP_LEGACY. These bits >> * affect how to interpret topology information in other cpuid leaves. >> */ >> __set_bit(X86_FEATURE_HTT, max_fs); >> - __set_bit(X86_FEATURE_X2APIC, max_fs); >> __set_bit(X86_FEATURE_CMP_LEGACY, max_fs); >> >> /* > ... these adjustments, just still in calculate_pv_featureset(). I > haven't gone further backwards to check if/when this exposure has > really appeared. I wouldn't be surprised if it's been like that > for all the time since we gained x2APIC support in the hypervisor. 4.14 was when we got the proper default vs max split. Before then, this block of logic was an opencoded "max(ish) for tookstacks which know about it" kind of thing. ~Andrew
On 29.02.2024 14:23, Andrew Cooper wrote: > On 29/02/2024 12:47 pm, Jan Beulich wrote: >> On 29.02.2024 11:43, Andrew Cooper wrote: >>> Right now, the host x2APIC setting filters into the PV max and default >>> policies, yet PV guests cannot set MSR_APIC_BASE.EXTD or access any of the >>> x2APIC MSR range. Therefore they absolutely shouldn't see the x2APIC bit. >>> >>> Linux has workarounds for the collateral damage caused by this leakage; it >>> unconditionally filters out the x2APIC CPUID bit, and EXTD when reading >>> MSR_APIC_BASE. >>> >>> Hide the x2APIC bit in the PV default policy, but for compatibility, tolerate >>> incoming VMs which already saw the bit. This is logic from before the >>> default/max split in Xen 4.14 which wasn't correctly adjusted at the time. >> What about guest_cpuid()'s handling of leaf 0xb then? The %edx value >> will change once a guest is rebooted, aiui. The comment in >> recalculate_cpuid_policy() that you update refers to that. > > That comment is going in the next patch irrespective. > > But yes - this will change leaf 0xb from being > host-conditionally-visible to always hidden. Imo this wants saying explicitly, including why that's okay to do, especially since ... > PV guests don't have any coherent idea of topology. Linux (with the > topo fixes) now explicitly ignores everything it can see and just fakes > up a flat non-SMT topology in a single package. ... you validly use "now" here. Plus Linux isn't the only PV guest we need to care about. What's wrong (more wrong than the present putting of vCPU ID * 2 there) with retaining the population of that leaf (by dropping the x2apic dependency there)? >>> This wants backporting as far as people can tollerate, but it's really not >>> obvious which commit in 4.14 should be referenced in a Fixes: tag. >> Why 4.14? In 4.7.0 I see ... >> >>> @@ -830,11 +846,10 @@ void recalculate_cpuid_policy(struct domain *d) >>> } >>> >>> /* >>> - * Allow the toolstack to set HTT, X2APIC and CMP_LEGACY. These bits >>> + * Allow the toolstack to set HTT and CMP_LEGACY. These bits >>> * affect how to interpret topology information in other cpuid leaves. >>> */ >>> __set_bit(X86_FEATURE_HTT, max_fs); >>> - __set_bit(X86_FEATURE_X2APIC, max_fs); >>> __set_bit(X86_FEATURE_CMP_LEGACY, max_fs); >>> >>> /* >> ... these adjustments, just still in calculate_pv_featureset(). I >> haven't gone further backwards to check if/when this exposure has >> really appeared. I wouldn't be surprised if it's been like that >> for all the time since we gained x2APIC support in the hypervisor. > > 4.14 was when we got the proper default vs max split. Before then, this > block of logic was an opencoded "max(ish) for tookstacks which know > about it" kind of thing. Except it was also affecting what guests get to see, afaict. Jan
On 29/02/2024 1:29 pm, Jan Beulich wrote: > On 29.02.2024 14:23, Andrew Cooper wrote: >> On 29/02/2024 12:47 pm, Jan Beulich wrote: >>> On 29.02.2024 11:43, Andrew Cooper wrote: >>>> Right now, the host x2APIC setting filters into the PV max and default >>>> policies, yet PV guests cannot set MSR_APIC_BASE.EXTD or access any of the >>>> x2APIC MSR range. Therefore they absolutely shouldn't see the x2APIC bit. >>>> >>>> Linux has workarounds for the collateral damage caused by this leakage; it >>>> unconditionally filters out the x2APIC CPUID bit, and EXTD when reading >>>> MSR_APIC_BASE. >>>> >>>> Hide the x2APIC bit in the PV default policy, but for compatibility, tolerate >>>> incoming VMs which already saw the bit. This is logic from before the >>>> default/max split in Xen 4.14 which wasn't correctly adjusted at the time. >>> What about guest_cpuid()'s handling of leaf 0xb then? The %edx value >>> will change once a guest is rebooted, aiui. The comment in >>> recalculate_cpuid_policy() that you update refers to that. >> That comment is going in the next patch irrespective. >> >> But yes - this will change leaf 0xb from being >> host-conditionally-visible to always hidden. > Imo this wants saying explicitly, Yeah - already started that for v2. > including why that's okay to do, > especially since ... > >> PV guests don't have any coherent idea of topology. Linux (with the >> topo fixes) now explicitly ignores everything it can see and just fakes >> up a flat non-SMT topology in a single package. > ... you validly use "now" here. Plus Linux isn't the only PV guest we > need to care about. As I said on the other thread, NetBSD works in the spirit in which PV guests were intended and completely ignores x2APIC in XENPV builds. > > What's wrong (more wrong than the present putting of vCPU ID * 2 there) > with retaining the population of that leaf (by dropping the x2apic > dependency there)? Without an MADT it's meaningless. For PV dom0, it's actively wrong because there is an MADT and it's in the wrong space. libxc has never written anything coherent in here, because it's never had any coherent idea about topology. Alejandro is working on that, and I believe one of the prep series has been posted. There's a lot more to go. Even today, we end up overlaying the host's APIC_ID space layout over the blind vCPU_ID * 2, which makes the result still nonsense. Various versions of Xen have tried playing with this, without understanding properly what they're doing, and XenServer still has a revert of a Xen 3.4 patch in the patch, as it broken migration of guests at the time... There is going to be a future (hopefully soon) where HVM guests get to see something which conforms to the architectural specs, and is sane. But doing the same for PV guests is more complicated, because of the conflicting requirements between PV guests not really having an APIC, but APIC being the x86 architectural expression of topology. >>>> This wants backporting as far as people can tollerate, but it's really not >>>> obvious which commit in 4.14 should be referenced in a Fixes: tag. >>> Why 4.14? In 4.7.0 I see ... >>> >>>> @@ -830,11 +846,10 @@ void recalculate_cpuid_policy(struct domain *d) >>>> } >>>> >>>> /* >>>> - * Allow the toolstack to set HTT, X2APIC and CMP_LEGACY. These bits >>>> + * Allow the toolstack to set HTT and CMP_LEGACY. These bits >>>> * affect how to interpret topology information in other cpuid leaves. >>>> */ >>>> __set_bit(X86_FEATURE_HTT, max_fs); >>>> - __set_bit(X86_FEATURE_X2APIC, max_fs); >>>> __set_bit(X86_FEATURE_CMP_LEGACY, max_fs); >>>> >>>> /* >>> ... these adjustments, just still in calculate_pv_featureset(). I >>> haven't gone further backwards to check if/when this exposure has >>> really appeared. I wouldn't be surprised if it's been like that >>> for all the time since we gained x2APIC support in the hypervisor. >> 4.14 was when we got the proper default vs max split. Before then, this >> block of logic was an opencoded "max(ish) for tookstacks which know >> about it" kind of thing. > Except it was also affecting what guests get to see, afaict. No - this hunk explicitly doesn't. What this hunk says is "don't override the toolstack's choice based on what the host can see". Because even today, Xen is still blindly zeroing toolstack settings it doesn't like, because my series fixing this is still sat on the mailing list from years and years ago over and argument over whether a function lives in libx86 or elsewhere... ~Andrew
On 29/02/2024 12:47 pm, Jan Beulich wrote: >> @@ -830,11 +846,10 @@ void recalculate_cpuid_policy(struct domain *d) >> } >> >> /* >> - * Allow the toolstack to set HTT, X2APIC and CMP_LEGACY. These bits >> + * Allow the toolstack to set HTT and CMP_LEGACY. These bits >> * affect how to interpret topology information in other cpuid leaves. >> */ >> __set_bit(X86_FEATURE_HTT, max_fs); >> - __set_bit(X86_FEATURE_X2APIC, max_fs); >> __set_bit(X86_FEATURE_CMP_LEGACY, max_fs); >> >> /* > ... these adjustments, just still in calculate_pv_featureset(). I > haven't gone further backwards to check if/when this exposure has > really appeared. I wouldn't be surprised if it's been like that > for all the time since we gained x2APIC support in the hypervisor. 4.7 has: cpufeatureset.h:140:XEN_CPUFEATURE(X2APIC, 1*32+21) /*!A Extended xAPIC */ so will expose it to PV guests. 4.6 has this gem I'd forgotten, in pv_cpuid(): if ( !cpu_has_apic ) __clear_bit(X86_FEATURE_X2APIC % 32, &c); but I don't see any (sensible) logic to hide x2APIC from the view of a PV guest. So PV guests really have see x2APIC from the host forever (and irrespective of Xen's x2APIC support). ~Andrew
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c index 10079c26ae24..a0205672428d 100644 --- a/xen/arch/x86/cpu-policy.c +++ b/xen/arch/x86/cpu-policy.c @@ -534,6 +534,14 @@ static void __init calculate_pv_max_policy(void) *p = host_cpu_policy; x86_cpu_policy_to_featureset(p, fs); + /* + * Xen at the time of writing (Feb 2024, 4.19 dev cycle) used to leak the + * host x2APIC capability into PV guests, but never supported the guest + * trying to turn x2APIC mode on. Tolerate an incoming VM which saw the + * x2APIC CPUID bit. + */ + __set_bit(X86_FEATURE_X2APIC, fs); + for ( i = 0; i < ARRAY_SIZE(fs); ++i ) fs[i] &= pv_max_featuremask[i]; @@ -566,6 +574,14 @@ static void __init calculate_pv_def_policy(void) *p = pv_max_cpu_policy; x86_cpu_policy_to_featureset(p, fs); + /* + * PV guests have never been able to use x2APIC mode, but at the time of + * writing (Feb 2024, 4.19 dev cycle), the host value used to leak into + * guests. Hide it by default so new guests don't get mislead into + * thinking that they can use x2APIC. + */ + __clear_bit(X86_FEATURE_X2APIC, fs); + for ( i = 0; i < ARRAY_SIZE(fs); ++i ) fs[i] &= pv_def_featuremask[i]; @@ -830,11 +846,10 @@ void recalculate_cpuid_policy(struct domain *d) } /* - * Allow the toolstack to set HTT, X2APIC and CMP_LEGACY. These bits + * Allow the toolstack to set HTT and CMP_LEGACY. These bits * affect how to interpret topology information in other cpuid leaves. */ __set_bit(X86_FEATURE_HTT, max_fs); - __set_bit(X86_FEATURE_X2APIC, max_fs); __set_bit(X86_FEATURE_CMP_LEGACY, max_fs); /* diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h index be5c1b748e27..b230d3a6907d 100644 --- a/xen/include/public/arch-x86/cpufeatureset.h +++ b/xen/include/public/arch-x86/cpufeatureset.h @@ -123,7 +123,7 @@ XEN_CPUFEATURE(PCID, 1*32+17) /*H Process Context ID */ XEN_CPUFEATURE(DCA, 1*32+18) /* Direct Cache Access */ XEN_CPUFEATURE(SSE4_1, 1*32+19) /*A Streaming SIMD Extensions 4.1 */ XEN_CPUFEATURE(SSE4_2, 1*32+20) /*A Streaming SIMD Extensions 4.2 */ -XEN_CPUFEATURE(X2APIC, 1*32+21) /*!A Extended xAPIC */ +XEN_CPUFEATURE(X2APIC, 1*32+21) /*!S Extended xAPIC */ XEN_CPUFEATURE(MOVBE, 1*32+22) /*A movbe instruction */ XEN_CPUFEATURE(POPCNT, 1*32+23) /*A POPCNT instruction */ XEN_CPUFEATURE(TSC_DEADLINE, 1*32+24) /*S TSC Deadline Timer */
Right now, the host x2APIC setting filters into the PV max and default policies, yet PV guests cannot set MSR_APIC_BASE.EXTD or access any of the x2APIC MSR range. Therefore they absolutely shouldn't see the x2APIC bit. Linux has workarounds for the collateral damage caused by this leakage; it unconditionally filters out the x2APIC CPUID bit, and EXTD when reading MSR_APIC_BASE. Hide the x2APIC bit in the PV default policy, but for compatibility, tolerate incoming VMs which already saw the bit. This is logic from before the default/max split in Xen 4.14 which wasn't correctly adjusted at the time. Update the annotation from !A to !S which slightly better describes that it doesn't really exist in PV guests. HVM guests, for which x2APIC can be emulated completely, already has it unconditionally set in the max policy. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> This wants backporting as far as people can tollerate, but it's really not obvious which commit in 4.14 should be referenced in a Fixes: tag. --- xen/arch/x86/cpu-policy.c | 19 +++++++++++++++++-- xen/include/public/arch-x86/cpufeatureset.h | 2 +- 2 files changed, 18 insertions(+), 3 deletions(-)