[Xen-devel] [PATCH] x86/hvm: always expose x2APIC feature in max HVM cpuid policy

[Xen-devel] [PATCH] x86/hvm: always expose x2APIC feature in max HVM cpuid policy

[Roger Pau Monne]

On hardware without x2APIC support Xen emulated local APIC will
provide such mode, and hence the feature should be set in the maximum
HVM cpuid policy.

Not exposing it in the maximum policy results in HVM domains not
getting such feature exposed unless it's also supported by the
underlying hardware.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
This is a regression, but I'm not able to identify the commit that
introduced the bogus behavior.
---
 xen/arch/x86/cpuid.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 7055509ed6..b1ed33d524 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -396,10 +396,11 @@ static void __init calculate_hvm_max_policy(void)
         hvm_featureset[i] &= hvm_featuremask[i];
 
     /*
-     * Xen can provide an APIC emulation to HVM guests even if the host's APIC
-     * isn't enabled.
+     * Xen can provide an (x2)APIC emulation to HVM guests even if the host's
+     * (x2)APIC isn't enabled.
      */
     __set_bit(X86_FEATURE_APIC, hvm_featureset);
+    __set_bit(X86_FEATURE_X2APIC, hvm_featureset);
 
     /*
      * On AMD, PV guests are entirely unable to use SYSENTER as Xen runs in
-- 
2.24.1


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

Re: [Xen-devel] [PATCH] x86/hvm: always expose x2APIC feature in max HVM cpuid policy

[Andrew Cooper]

On 24/12/2019 10:18, Roger Pau Monne wrote:
> On hardware without x2APIC support Xen emulated local APIC will
> provide such mode, and hence the feature should be set in the maximum
> HVM cpuid policy.
>
> Not exposing it in the maximum policy results in HVM domains not
> getting such feature exposed unless it's also supported by the
> underlying hardware.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

x2apic has never been exposed via this mechanism, due to its effects on
topology calculations.

It has however always been down to the toolstack to opt in, and Xen
permits this via recalculate_cpuid_policy(), on the expectation that the
toolstack knew what it was doing WRT topology (all evidence actually to
the contrary).

If we're seeing a recent change in behaviour, then it will be from libxc.

~Andrew

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

Re: [Xen-devel] [PATCH] x86/hvm: always expose x2APIC feature in max HVM cpuid policy

[Roger Pau Monné]

On Tue, Dec 24, 2019 at 12:23:12PM +0000, Andrew Cooper wrote:
> On 24/12/2019 10:18, Roger Pau Monne wrote:
> > On hardware without x2APIC support Xen emulated local APIC will
> > provide such mode, and hence the feature should be set in the maximum
> > HVM cpuid policy.
> >
> > Not exposing it in the maximum policy results in HVM domains not
> > getting such feature exposed unless it's also supported by the
> > underlying hardware.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> x2apic has never been exposed via this mechanism, due to its effects on
> topology calculations.

Well, it's exposed in hvm_max_cpuid_policy if it's present in the
hardware. IMO it's kind of weird that the presence of the x2APIC feature
on the max policy depends on the underlying hardware, when it's always
supported by the emulated vlapic.

I think x2APIC must either always be part of the max policy, or never,
or else it's very easy to cause regressions because it's not so easy
to find a box without x2APIC.

> It has however always been down to the toolstack to opt in, and Xen
> permits this via recalculate_cpuid_policy(), on the expectation that the
> toolstack knew what it was doing WRT topology (all evidence actually to
> the contrary).

Hm, I can try to force the setting in libxc.

> If we're seeing a recent change in behaviour, then it will be from libxc.

IIRC x2APIC was always exposed to HVM guests regardless of the
underlying hardware support.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH] x86/hvm: always expose x2APIC feature in max HVM cpuid policy

[Andrew Cooper]

On 24/12/2019 12:42, Roger Pau Monné wrote:
> On Tue, Dec 24, 2019 at 12:23:12PM +0000, Andrew Cooper wrote:
>> On 24/12/2019 10:18, Roger Pau Monne wrote:
>>> On hardware without x2APIC support Xen emulated local APIC will
>>> provide such mode, and hence the feature should be set in the maximum
>>> HVM cpuid policy.
>>>
>>> Not exposing it in the maximum policy results in HVM domains not
>>> getting such feature exposed unless it's also supported by the
>>> underlying hardware.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> x2apic has never been exposed via this mechanism, due to its effects on
>> topology calculations.
> Well, it's exposed in hvm_max_cpuid_policy if it's present in the
> hardware. IMO it's kind of weird that the presence of the x2APIC feature
> on the max policy depends on the underlying hardware, when it's always
> supported by the emulated vlapic.
>
> I think x2APIC must either always be part of the max policy, or never,
> or else it's very easy to cause regressions because it's not so easy
> to find a box without x2APIC.

Hmm - this does highlight the inconsistency in the existing logic.  I'm
not overly surprised - this is a remnant of the old model which hasn't
been rewritten yet.

>
>> It has however always been down to the toolstack to opt in, and Xen
>> permits this via recalculate_cpuid_policy(), on the expectation that the
>> toolstack knew what it was doing WRT topology (all evidence actually to
>> the contrary).
> Hm, I can try to force the setting in libxc.
>
>> If we're seeing a recent change in behaviour, then it will be from libxc.
> IIRC x2APIC was always exposed to HVM guests regardless of the
> underlying hardware support.

I have a suspicion that this may have been broken by c/s 3e0c8272f in
2015...

~Andrew

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

Re: [Xen-devel] [PATCH] x86/hvm: always expose x2APIC feature in max HVM cpuid policy

[Roger Pau Monné]

On Tue, Dec 24, 2019 at 04:00:27PM +0000, Andrew Cooper wrote:
> On 24/12/2019 12:42, Roger Pau Monné wrote:
> > On Tue, Dec 24, 2019 at 12:23:12PM +0000, Andrew Cooper wrote:
> >> On 24/12/2019 10:18, Roger Pau Monne wrote:
> >>> On hardware without x2APIC support Xen emulated local APIC will
> >>> provide such mode, and hence the feature should be set in the maximum
> >>> HVM cpuid policy.
> >>>
> >>> Not exposing it in the maximum policy results in HVM domains not
> >>> getting such feature exposed unless it's also supported by the
> >>> underlying hardware.
> >>>
> >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >> x2apic has never been exposed via this mechanism, due to its effects on
> >> topology calculations.
> > Well, it's exposed in hvm_max_cpuid_policy if it's present in the
> > hardware. IMO it's kind of weird that the presence of the x2APIC feature
> > on the max policy depends on the underlying hardware, when it's always
> > supported by the emulated vlapic.
> >
> > I think x2APIC must either always be part of the max policy, or never,
> > or else it's very easy to cause regressions because it's not so easy
> > to find a box without x2APIC.
> 
> Hmm - this does highlight the inconsistency in the existing logic.  I'm
> not overly surprised - this is a remnant of the old model which hasn't
> been rewritten yet.

I could do something like:

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 519d6d8bd0..a7adc41854 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -641,6 +641,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
         p->extd.itsc = true;
         p->basic.vmx = true;
         p->extd.svm = true;
+        p->basic.x2apic = true;
     }
 
     rc = x86_cpuid_copy_to_buffer(p, leaves, &nr_leaves);

But it seems kind of bogus to me that such feature is not part of the
hvm_max_cpuid_policy, at the end of day the toolstack has no knowledge
of whether the hypervisor provides a x2APIC interface or not (apart
from us hardcoding it in the tools like the above patch).

> >
> >> It has however always been down to the toolstack to opt in, and Xen
> >> permits this via recalculate_cpuid_policy(), on the expectation that the
> >> toolstack knew what it was doing WRT topology (all evidence actually to
> >> the contrary).
> > Hm, I can try to force the setting in libxc.
> >
> >> If we're seeing a recent change in behaviour, then it will be from libxc.
> > IIRC x2APIC was always exposed to HVM guests regardless of the
> > underlying hardware support.
> 
> I have a suspicion that this may have been broken by c/s 3e0c8272f in
> 2015...

I could swear Xen was exposing the x2APIC CPUID feature on the box I'm
currently testing during the 4.13 development cycle, or else I did
hack it myself for development purposes and completely forgot to send
a patch afterwards.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH] x86/hvm: always expose x2APIC feature in max HVM cpuid policy

[Roger Pau Monné]

On Tue, Dec 24, 2019 at 07:17:52PM +0100, Roger Pau Monné wrote:
> On Tue, Dec 24, 2019 at 04:00:27PM +0000, Andrew Cooper wrote:
> > On 24/12/2019 12:42, Roger Pau Monné wrote:
> > > On Tue, Dec 24, 2019 at 12:23:12PM +0000, Andrew Cooper wrote:
> > >> On 24/12/2019 10:18, Roger Pau Monne wrote:
> > >>> On hardware without x2APIC support Xen emulated local APIC will
> > >>> provide such mode, and hence the feature should be set in the maximum
> > >>> HVM cpuid policy.
> > >>>
> > >>> Not exposing it in the maximum policy results in HVM domains not
> > >>> getting such feature exposed unless it's also supported by the
> > >>> underlying hardware.
> > >>>
> > >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > >> x2apic has never been exposed via this mechanism, due to its effects on
> > >> topology calculations.
> > > Well, it's exposed in hvm_max_cpuid_policy if it's present in the
> > > hardware. IMO it's kind of weird that the presence of the x2APIC feature
> > > on the max policy depends on the underlying hardware, when it's always
> > > supported by the emulated vlapic.
> > >
> > > I think x2APIC must either always be part of the max policy, or never,
> > > or else it's very easy to cause regressions because it's not so easy
> > > to find a box without x2APIC.
> > 
> > Hmm - this does highlight the inconsistency in the existing logic.  I'm
> > not overly surprised - this is a remnant of the old model which hasn't
> > been rewritten yet.
> 
> I could do something like:
> 
> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
> index 519d6d8bd0..a7adc41854 100644
> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -641,6 +641,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
>          p->extd.itsc = true;
>          p->basic.vmx = true;
>          p->extd.svm = true;
> +        p->basic.x2apic = true;
>      }
>  
>      rc = x86_cpuid_copy_to_buffer(p, leaves, &nr_leaves);
> 
> But it seems kind of bogus to me that such feature is not part of the
> hvm_max_cpuid_policy, at the end of day the toolstack has no knowledge
> of whether the hypervisor provides a x2APIC interface or not (apart
> from us hardcoding it in the tools like the above patch).

Ping?

I don't think we reached a conclusion as to whether x2APIC should be
forced from the toolstack side or part of the HVM max policy.

Thanks, Roger.

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