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

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

Jan 9, 2024, 7:38 AM

Post #1 of 3 (24 views)

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 [ In reply to ]

roger.pau at citrix

Mar 19, 2024, 10:55 AM

Post #2 of 3 (16 views)

Permalink

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 [ In reply to ]

alejandro.vallejo at cloud

Mar 25, 2024, 11:19 AM

Post #3 of 3 (8 views)

Permalink

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