| Message ID | 20240109153834.4192-3-alejandro.vallejo@cloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: Expose consistent topology to guests | expand |
On Tue, Jan 09, 2024 at 03:38:30PM +0000, Alejandro Vallejo wrote: > Move struct xc_cpu_policy data structure out of xg_private.h and into > the public xenguest.h so it can be used by libxl. I will let Andrew comment on this one, IIRC the initial idea was to not leak cpu_policy into libxl, and to instead have it as an opaque object that libxl can interact with using helpers. I haven't looked at followup patches - I assume this is done to manipulate the cpuid data more easily from libxl, and ultimately drop xc_xend_cpuid? > Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> > --- > tools/include/xenguest.h | 8 +++++++- > tools/libs/guest/xg_private.h | 10 ---------- > xen/include/xen/lib/x86/cpu-policy.h | 5 +++++ > 3 files changed, 12 insertions(+), 11 deletions(-) > > diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h > index e01f494b77..4e9078fdee 100644 > --- a/tools/include/xenguest.h > +++ b/tools/include/xenguest.h > @@ -784,7 +784,13 @@ xen_pfn_t *xc_map_m2p(xc_interface *xch, > unsigned long *mfn0); > > #if defined(__i386__) || defined(__x86_64__) > -typedef struct xc_cpu_policy xc_cpu_policy_t; > +#include <xen/lib/x86/cpu-policy.h> > + > +typedef struct xc_cpu_policy { > + struct cpu_policy policy; > + xen_cpuid_leaf_t leaves[CPUID_MAX_SERIALISED_LEAVES]; > + xen_msr_entry_t msrs[MSR_MAX_SERIALISED_ENTRIES]; > +} xc_cpu_policy_t; > > /* Create and free a xc_cpu_policy object. */ > xc_cpu_policy_t *xc_cpu_policy_init(void); > diff --git a/tools/libs/guest/xg_private.h b/tools/libs/guest/xg_private.h > index d73947094f..d1940f1ea4 100644 > --- a/tools/libs/guest/xg_private.h > +++ b/tools/libs/guest/xg_private.h > @@ -170,14 +170,4 @@ int pin_table(xc_interface *xch, unsigned int type, unsigned long mfn, > #define M2P_SIZE(_m) ROUNDUP(((_m) * sizeof(xen_pfn_t)), M2P_SHIFT) > #define M2P_CHUNKS(_m) (M2P_SIZE((_m)) >> M2P_SHIFT) > > -#if defined(__x86_64__) || defined(__i386__) > -#include <xen/lib/x86/cpu-policy.h> > - > -struct xc_cpu_policy { > - struct cpu_policy policy; > - xen_cpuid_leaf_t leaves[CPUID_MAX_SERIALISED_LEAVES]; > - xen_msr_entry_t msrs[MSR_MAX_SERIALISED_ENTRIES]; > -}; > -#endif /* x86 */ > - > #endif /* XG_PRIVATE_H */ > diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h > index 14724cedff..65f6335b32 100644 > --- a/xen/include/xen/lib/x86/cpu-policy.h > +++ b/xen/include/xen/lib/x86/cpu-policy.h > @@ -85,6 +85,11 @@ unsigned int x86_cpuid_lookup_vendor(uint32_t ebx, uint32_t ecx, uint32_t edx); > */ > const char *x86_cpuid_vendor_to_str(unsigned int vendor); > > +#ifndef __XEN__ > +/* Needed for MAX() */ > +#include <xen-tools/common-macros.h> > +#endif /* __XEN__ */ I think for this header it is up to the user to provide the required context, iow: it should be tools/include/xenguest.h to include xen-tools/common-macros.h ahead of cpu-policy.h. Otherwise we should likely do the same for #ifdef __XEN__ branch and include whatever header that defines MAX(). Thanks, Roger.
On 19/03/2024 17:55, Roger Pau Monné wrote: > On Tue, Jan 09, 2024 at 03:38:30PM +0000, Alejandro Vallejo wrote: >> Move struct xc_cpu_policy data structure out of xg_private.h and into >> the public xenguest.h so it can be used by libxl. > > I will let Andrew comment on this one, IIRC the initial idea was to > not leak cpu_policy into libxl, and to instead have it as an opaque > object that libxl can interact with using helpers. I wrote this a while ago, and can't quite remember why I left this here. Looking closely I think it's leftover from something else I eventually turned into its own series[1]. I don't _think_ it's needed for this series, so I'll just drop it in v2. If everything blows up I'll tell you why I needed it :) [1] https://lore.kernel.org/xen-devel/20240207173957.19811-1-alejandro.vallejo@cloud.com/ > > I haven't looked at followup patches - I assume this is done to > manipulate the cpuid data more easily from libxl, and ultimately drop > xc_xend_cpuid? Yes, we can't drop that altogether because xl exposes an interface allowing fine-grained manipulation of CPUID/MSR data, but it becomes minimal. The general idea is to is to stop moving separate buffers around in tandem and use a single data structure instead whenever possible. Cheers, Alejandro
diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h index e01f494b77..4e9078fdee 100644 --- a/tools/include/xenguest.h +++ b/tools/include/xenguest.h @@ -784,7 +784,13 @@ xen_pfn_t *xc_map_m2p(xc_interface *xch, unsigned long *mfn0); #if defined(__i386__) || defined(__x86_64__) -typedef struct xc_cpu_policy xc_cpu_policy_t; +#include <xen/lib/x86/cpu-policy.h> + +typedef struct xc_cpu_policy { + struct cpu_policy policy; + xen_cpuid_leaf_t leaves[CPUID_MAX_SERIALISED_LEAVES]; + xen_msr_entry_t msrs[MSR_MAX_SERIALISED_ENTRIES]; +} xc_cpu_policy_t; /* Create and free a xc_cpu_policy object. */ xc_cpu_policy_t *xc_cpu_policy_init(void); diff --git a/tools/libs/guest/xg_private.h b/tools/libs/guest/xg_private.h index d73947094f..d1940f1ea4 100644 --- a/tools/libs/guest/xg_private.h +++ b/tools/libs/guest/xg_private.h @@ -170,14 +170,4 @@ int pin_table(xc_interface *xch, unsigned int type, unsigned long mfn, #define M2P_SIZE(_m) ROUNDUP(((_m) * sizeof(xen_pfn_t)), M2P_SHIFT) #define M2P_CHUNKS(_m) (M2P_SIZE((_m)) >> M2P_SHIFT) -#if defined(__x86_64__) || defined(__i386__) -#include <xen/lib/x86/cpu-policy.h> - -struct xc_cpu_policy { - struct cpu_policy policy; - xen_cpuid_leaf_t leaves[CPUID_MAX_SERIALISED_LEAVES]; - xen_msr_entry_t msrs[MSR_MAX_SERIALISED_ENTRIES]; -}; -#endif /* x86 */ - #endif /* XG_PRIVATE_H */ diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h index 14724cedff..65f6335b32 100644 --- a/xen/include/xen/lib/x86/cpu-policy.h +++ b/xen/include/xen/lib/x86/cpu-policy.h @@ -85,6 +85,11 @@ unsigned int x86_cpuid_lookup_vendor(uint32_t ebx, uint32_t ecx, uint32_t edx); */ const char *x86_cpuid_vendor_to_str(unsigned int vendor); +#ifndef __XEN__ +/* Needed for MAX() */ +#include <xen-tools/common-macros.h> +#endif /* __XEN__ */ + #define CPUID_GUEST_NR_BASIC (0xdu + 1) #define CPUID_GUEST_NR_CACHE (5u + 1) #define CPUID_GUEST_NR_FEAT (2u + 1)
Move struct xc_cpu_policy data structure out of xg_private.h and into the public xenguest.h so it can be used by libxl. Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> --- tools/include/xenguest.h | 8 +++++++- tools/libs/guest/xg_private.h | 10 ---------- xen/include/xen/lib/x86/cpu-policy.h | 5 +++++ 3 files changed, 12 insertions(+), 11 deletions(-)