1. Patchew
  2. Xen
  3. [PATCH 0/6] x86: Expose consistent topology to guests
  4. View patch

[PATCH 2/6] tools/xc: Add xc_cpu_policy to the public xenctrl.h header

Alejandro Vallejo posted 6 patches 10 months, 1 week ago
There is a newer version of this series
[PATCH 2/6] tools/xc: Add xc_cpu_policy to the public xenctrl.h header
Posted by Alejandro Vallejo 10 months, 1 week ago 
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(-)

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)
-- 
2.34.1
Re: [PATCH 2/6] tools/xc: Add xc_cpu_policy to the public xenctrl.h header
Posted by Roger Pau Monné 7 months, 4 weeks ago 
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.
Re: [PATCH 2/6] tools/xc: Add xc_cpu_policy to the public xenctrl.h header
Posted by Alejandro Vallejo 7 months, 3 weeks ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.