On 23/03/2021 09:58, Roger Pau Monne wrote: > This logic is pulled out from xc_cpuid_apply_policy and placed into a > separate helper. > > No functional change intended. > > Signed-off-by: Roger Pau Monné <roger....@citrix.com> > --- > tools/include/xenctrl.h | 4 + > tools/libs/guest/xg_cpuid_x86.c | 181 +++++++++++++++++--------------- > 2 files changed, 102 insertions(+), 83 deletions(-) > > diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h > index 6f7158156fa..9f94e61523e 100644 > --- a/tools/include/xenctrl.h > +++ b/tools/include/xenctrl.h > @@ -2631,6 +2631,10 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch, > int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy, > bool hvm); > > +/* Setup the policy topology. */ > +int xc_cpu_policy_topology(xc_interface *xch, xc_cpu_policy_t policy, > + bool hvm);
I'm not sure how I feel about this. It's repeating the mistake we've currently got with topology handling. One part of it needs to be part of "compatible". We need to run the below logic, *in this form* as part of magic-ing a policy out of thin air for the incoming VM with no data. However, for any non-broken logic, the caller needs to specify the topology which wishes to be expressed. Do we want SMT at all? Do we want 1, 2, 4 or other threads/core. How many cores per socket? Its very common these days for non-power-of-2 numbers. Our default case ought to be to match the host topology. Do we want to support 3 threads/core? Sure - its weird to think about, but its semantically equivalent to using non-power-of-2 numbers at other levels, and would certainly be useful to express for testing purposes. What about Intel's leaf 0x1f with the SMT > Core > Module > Tile > Die topology layout? The answers to these questions also need to fix Xen so that APIC_ID isn't vcpu_id * 2 (which is horribly broken on non-Intel or Intel Knight* hardware). It also needs to change how the MADT is written for guests, and how the IO-APIC IDs are assigned (matters for the AMD topology algorithms). There are further implications. Should we prohibit creating a 4-vcpu VM with cores/socket=128? A regular kernel will demand an IOMMU for this configuration as we end up with APIC IDs above 255. OTOH, there are also virtualisation schemes now to support 32k vcpus without an IOMMU, which KVM and HyperV now speak. Fixing our topology problems is a monumental can of worms. While we should keep it in mind, we should try not to conflate it with "make libxl/libxc's CPUID logic more sane, and include MSRs", which is large enough task on its own. What I suspect we want in the short term is xc_cpu_policy_legacy_adjust() or equivalent, which is very clear that it is a transitional API only, which for now can be used everywhere where xc_cpuid_apply_policy() is used. As we pull various logical areas out of this, we'll adjust the callers appropriately, and eventually delete this function. ~Andrew
