This enables a set of follow-up simplifications in the toolstack.
No functional change.
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 | 6 ++++--
3 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h
index e01f494b772a..4e9078fdee4d 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 d73947094f2e..d1940f1ea482 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 d5e447e9dc06..0cd454dfc0b6 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -85,6 +85,8 @@ 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);
+#define XENLIB_MAX(x, y) ((x) < (y) ? (y) : (x))
+
#define CPUID_GUEST_NR_BASIC (0xdu + 1)
#define CPUID_GUEST_NR_CACHE (5u + 1)
#define CPUID_GUEST_NR_FEAT (2u + 1)
@@ -92,8 +94,8 @@ const char *x86_cpuid_vendor_to_str(unsigned int vendor);
#define CPUID_GUEST_NR_XSTATE (62u + 1)
#define CPUID_GUEST_NR_EXTD_INTEL (0x8u + 1)
#define CPUID_GUEST_NR_EXTD_AMD (0x21u + 1)
-#define CPUID_GUEST_NR_EXTD MAX(CPUID_GUEST_NR_EXTD_INTEL, \
- CPUID_GUEST_NR_EXTD_AMD)
+#define CPUID_GUEST_NR_EXTD XENLIB_MAX(CPUID_GUEST_NR_EXTD_INTEL, \
+ CPUID_GUEST_NR_EXTD_AMD)
/*
* Maximum number of leaves a struct cpu_policy turns into when serialised for
--
2.34.1
On Wed, Feb 07, 2024 at 05:39:55PM +0000, Alejandro Vallejo wrote: > diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h > index e01f494b772a..4e9078fdee4d 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> I don't think it's a good idea to expose "cpu-policy.h" to "xenguest.h", and it's going to break the build of every tools outside of xen repository that are using xenguest.h. xenguest.h is installed on a system, but cpu-policy.h isn't. > +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; Would using an accessor be an option to access `leaves` and `msrs` for "xen-cpuid" tool? That would avoid the need to expose the "cpu-policy.h" headers. With accessors, we might not need to expose xc_cpu_policy_serialise() to the world anymore, and it could be internal to libxenguest, probably, I haven't check. Thanks, -- Anthony PERARD
On 30/04/2024 15:13, Anthony PERARD wrote: > On Wed, Feb 07, 2024 at 05:39:55PM +0000, Alejandro Vallejo wrote: >> diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h >> index e01f494b772a..4e9078fdee4d 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> > > I don't think it's a good idea to expose "cpu-policy.h" to "xenguest.h", > and it's going to break the build of every tools outside of xen > repository that are using xenguest.h. xenguest.h is installed on a > system, but cpu-policy.h isn't. > >> +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; > > Would using an accessor be an option to access `leaves` and `msrs` for > "xen-cpuid" tool? That would avoid the need to expose the "cpu-policy.h" > headers. > > With accessors, we might not need to expose xc_cpu_policy_serialise() to > the world anymore, and it could be internal to libxenguest, probably, I > haven't check. > > Thanks, > That would work out for xen-cpuid. I'm on the fence in the general case because I think it'd be advantageous to let clients (i.e: xl) manipulate directly the deserialized form of the policy. That would allow flipping features on or off a heck of lot easier. We could create per-feature accessors, but then we'll end up with _a lot_ of them. I guess we'll get there when we get there. The xen-cpuid case definitely doesn't warrant it. I'll give this a go. Cheers, Alejandro
© 2016 - 2024 Red Hat, Inc.