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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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@xxxxxxxxxx>
> >> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel