[PATCH 0/5] Share CPUID stuff between hypervisor and toolstack

[PATCH 0/5] Share CPUID stuff between hypervisor and toolstack

[Wei Liu]

The hypervisor has some nice objects and definitions that toolstack can use,
too. Make that happen.

The anticipation is in the future MSR objects and definitions should be shared,
too.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

Wei Liu (5):
  x86: move definition of struct cpuid_leaf to cpuid.h
  x86: split out cpuid objects and helpers
  tools: link arch-shared directory
  libxc: introduce xc_cpuid_x86.h
  XXX DO NOT APPLY: compilation test

 .gitignore                              |   1 +
 tools/include/Makefile                  |   5 +-
 tools/libxc/xc_cpuid_x86.c              |  23 +++-
 tools/libxc/xc_cpuid_x86.h              |  16 +++
 tools/tests/x86_emulator/x86-emulate.h  |   6 +
 xen/arch/x86/x86_emulate/x86_emulate.h  |   6 +-
 xen/include/asm-x86/arch-shared/cpuid.h | 213 ++++++++++++++++++++++++++++++++
 xen/include/asm-x86/cpuid.h             | 206 +-----------------------------
 8 files changed, 261 insertions(+), 215 deletions(-)
 create mode 100644 tools/libxc/xc_cpuid_x86.h
 create mode 100644 xen/include/asm-x86/arch-shared/cpuid.h

-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 1/5] x86: move definition of struct cpuid_leaf to cpuid.h

[Wei Liu]

This is a step towards consolidating relevant data structures and
defines to one location.

It then requires defining cpuid_leaf in user space harness headers to
make them continue to compile.

No functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/tests/x86_emulator/x86-emulate.h | 6 ++++++
 xen/arch/x86/x86_emulate/x86_emulate.h | 6 +-----
 xen/include/asm-x86/cpuid.h            | 4 ++++
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/tests/x86_emulator/x86-emulate.h b/tools/tests/x86_emulator/x86-emulate.h
index c5e85de3a2..8ae31a2f9a 100644
--- a/tools/tests/x86_emulator/x86-emulate.h
+++ b/tools/tests/x86_emulator/x86-emulate.h
@@ -79,6 +79,12 @@ WRAP(puts);
 
 #undef WRAP
 
+#ifndef cpuid_leaf
+struct cpuid_leaf {
+    uint32_t a, b, c, d;
+};
+#endif
+
 #include "x86_emulate/x86_emulate.h"
 
 static inline uint64_t xgetbv(uint32_t xcr)
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index c22e7745ad..cc2cf54141 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -172,11 +172,7 @@ enum x86_emulate_fpu_type {
     X86EMUL_FPU_none
 };
 
-struct cpuid_leaf
-{
-    uint32_t a, b, c, d;
-};
-
+struct cpuid_leaf;
 struct x86_emulate_state;
 
 /*
diff --git a/xen/include/asm-x86/cpuid.h b/xen/include/asm-x86/cpuid.h
index 4cce2686cb..ca664d50e2 100644
--- a/xen/include/asm-x86/cpuid.h
+++ b/xen/include/asm-x86/cpuid.h
@@ -66,6 +66,10 @@ extern struct cpuidmasks cpuidmask_defaults;
 #define CPUID_GUEST_NR_EXTD_AMD   (0x1cu + 1)
 #define CPUID_GUEST_NR_EXTD       MAX(CPUID_GUEST_NR_EXTD_INTEL, \
                                       CPUID_GUEST_NR_EXTD_AMD)
+struct cpuid_leaf
+{
+    uint32_t a, b, c, d;
+};
 
 struct cpuid_policy
 {
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] x86: move definition of struct cpuid_leaf to cpuid.h

[Roger Pau Monné]

On Thu, May 24, 2018 at 05:05:18PM +0100, Wei Liu wrote:
> This is a step towards consolidating relevant data structures and
> defines to one location.
> 
> It then requires defining cpuid_leaf in user space harness headers to
> make them continue to compile.
> 
> No functional change.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  tools/tests/x86_emulator/x86-emulate.h | 6 ++++++
>  xen/arch/x86/x86_emulate/x86_emulate.h | 6 +-----
>  xen/include/asm-x86/cpuid.h            | 4 ++++
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/tests/x86_emulator/x86-emulate.h b/tools/tests/x86_emulator/x86-emulate.h
> index c5e85de3a2..8ae31a2f9a 100644
> --- a/tools/tests/x86_emulator/x86-emulate.h
> +++ b/tools/tests/x86_emulator/x86-emulate.h
> @@ -79,6 +79,12 @@ WRAP(puts);
>  
>  #undef WRAP
>  
> +#ifndef cpuid_leaf
> +struct cpuid_leaf {
> +    uint32_t a, b, c, d;
> +};
> +#endif

Do you really need the ifndef? AFAICT this header will always be
included from the test harness, which has no definition of cpuid_leaf.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] x86: move definition of struct cpuid_leaf to cpuid.h

[Wei Liu]

On Fri, May 25, 2018 at 11:31:17AM +0200, Roger Pau Monné wrote:
> On Thu, May 24, 2018 at 05:05:18PM +0100, Wei Liu wrote:
> > This is a step towards consolidating relevant data structures and
> > defines to one location.
> > 
> > It then requires defining cpuid_leaf in user space harness headers to
> > make them continue to compile.
> > 
> > No functional change.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> >  tools/tests/x86_emulator/x86-emulate.h | 6 ++++++
> >  xen/arch/x86/x86_emulate/x86_emulate.h | 6 +-----
> >  xen/include/asm-x86/cpuid.h            | 4 ++++
> >  3 files changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/tests/x86_emulator/x86-emulate.h b/tools/tests/x86_emulator/x86-emulate.h
> > index c5e85de3a2..8ae31a2f9a 100644
> > --- a/tools/tests/x86_emulator/x86-emulate.h
> > +++ b/tools/tests/x86_emulator/x86-emulate.h
> > @@ -79,6 +79,12 @@ WRAP(puts);
> >  
> >  #undef WRAP
> >  
> > +#ifndef cpuid_leaf
> > +struct cpuid_leaf {
> > +    uint32_t a, b, c, d;
> > +};
> > +#endif
> 
> Do you really need the ifndef? AFAICT this header will always be
> included from the test harness, which has no definition of cpuid_leaf.

It is not strictly required.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] x86: move definition of struct cpuid_leaf to cpuid.h

[Jan Beulich]

>>> On 24.05.18 at 18:05, <wei.liu2@citrix.com> wrote:
> This is a step towards consolidating relevant data structures and
> defines to one location.

Sort of contrary to what the patch does - it converts one instance of the
structure to two of them.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] x86: move definition of struct cpuid_leaf to cpuid.h

[Wei Liu]

On Fri, May 25, 2018 at 05:50:12AM -0600, Jan Beulich wrote:
> >>> On 24.05.18 at 18:05, <wei.liu2@citrix.com> wrote:
> > This is a step towards consolidating relevant data structures and
> > defines to one location.
> 
> Sort of contrary to what the patch does - it converts one instance of the
> structure to two of them.

But isn't this test harness an exception to how things are normally
done? I don't think x86 insn emulator is as standalone as other
components.

I never understood why cpuid_leaf was put into x86_emulate.h in the
first place. I tried to convert it to use cpuid.h but that opened an
even bigger can of worms.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] x86: move definition of struct cpuid_leaf to cpuid.h

[Jan Beulich]

>>> On 25.05.18 at 14:03, <wei.liu2@citrix.com> wrote:
> On Fri, May 25, 2018 at 05:50:12AM -0600, Jan Beulich wrote:
>> >>> On 24.05.18 at 18:05, <wei.liu2@citrix.com> wrote:
>> > This is a step towards consolidating relevant data structures and
>> > defines to one location.
>> 
>> Sort of contrary to what the patch does - it converts one instance of the
>> structure to two of them.
> 
> But isn't this test harness an exception to how things are normally
> done? I don't think x86 insn emulator is as standalone as other
> components.

FAOD: I don't really mind the movement. I do think, though, that the
description doesn't match what the patch does.

> I never understood why cpuid_leaf was put into x86_emulate.h in the
> first place. I tried to convert it to use cpuid.h but that opened an
> even bigger can of worms.

I guess that's the reason you're looking for.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] x86: move definition of struct cpuid_leaf to cpuid.h

[Andrew Cooper]

On 25/05/18 14:05, Jan Beulich wrote:
>>>> On 25.05.18 at 14:03, <wei.liu2@citrix.com> wrote:
>> On Fri, May 25, 2018 at 05:50:12AM -0600, Jan Beulich wrote:
>>>>>> On 24.05.18 at 18:05, <wei.liu2@citrix.com> wrote:
>>>> This is a step towards consolidating relevant data structures and
>>>> defines to one location.
>>> Sort of contrary to what the patch does - it converts one instance of the
>>> structure to two of them.
>> But isn't this test harness an exception to how things are normally
>> done? I don't think x86 insn emulator is as standalone as other
>> components.
> FAOD: I don't really mind the movement. I do think, though, that the
> description doesn't match what the patch does.
>
>> I never understood why cpuid_leaf was put into x86_emulate.h in the
>> first place. I tried to convert it to use cpuid.h but that opened an
>> even bigger can of worms.
> I guess that's the reason you're looking for.

Yes - cpuid_leaf is used in the cpuid() hook, which is why it is
necessary here.  That said, by splitting it (and other bits) out into a
shared header, this disentangles itself nicely.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 2/5] x86: split out cpuid objects and helpers

[Wei Liu]

They are moved to a new header which is going to be consumed by both
the hypervisor and toolstack.

Create a new directory for this kind of headers in anticipation of
more will come.

No functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

Any suggestion on the directory name?
---
 xen/include/asm-x86/arch-shared/cpuid.h | 213 ++++++++++++++++++++++++++++++++
 xen/include/asm-x86/cpuid.h             | 210 +------------------------------
 2 files changed, 215 insertions(+), 208 deletions(-)
 create mode 100644 xen/include/asm-x86/arch-shared/cpuid.h

diff --git a/xen/include/asm-x86/arch-shared/cpuid.h b/xen/include/asm-x86/arch-shared/cpuid.h
new file mode 100644
index 0000000000..5c049e11a6
--- /dev/null
+++ b/xen/include/asm-x86/arch-shared/cpuid.h
@@ -0,0 +1,213 @@
+/* Common data structures and functions consumed by hypervisor and toolstack */
+#ifndef __X86_CPUID_SHARED_H__
+#define __X86_CPUID_SHARED_H__
+
+#define FEATURESET_1d     0 /* 0x00000001.edx      */
+#define FEATURESET_1c     1 /* 0x00000001.ecx      */
+#define FEATURESET_e1d    2 /* 0x80000001.edx      */
+#define FEATURESET_e1c    3 /* 0x80000001.ecx      */
+#define FEATURESET_Da1    4 /* 0x0000000d:1.eax    */
+#define FEATURESET_7b0    5 /* 0x00000007:0.ebx    */
+#define FEATURESET_7c0    6 /* 0x00000007:0.ecx    */
+#define FEATURESET_e7d    7 /* 0x80000007.edx      */
+#define FEATURESET_e8b    8 /* 0x80000008.ebx      */
+#define FEATURESET_7d0    9 /* 0x00000007:0.edx    */
+
+
+#define CPUID_GUEST_NR_BASIC      (0xdu + 1)
+#define CPUID_GUEST_NR_FEAT       (0u + 1)
+#define CPUID_GUEST_NR_CACHE      (5u + 1)
+#define CPUID_GUEST_NR_XSTATE     (62u + 1)
+#define CPUID_GUEST_NR_EXTD_INTEL (0x8u + 1)
+#define CPUID_GUEST_NR_EXTD_AMD   (0x1cu + 1)
+#define CPUID_GUEST_NR_EXTD       MAX(CPUID_GUEST_NR_EXTD_INTEL, \
+                                      CPUID_GUEST_NR_EXTD_AMD)
+struct cpuid_leaf
+{
+    uint32_t a, b, c, d;
+};
+
+struct cpuid_policy
+{
+#define DECL_BITFIELD(word) _DECL_BITFIELD(FEATURESET_ ## word)
+#define _DECL_BITFIELD(x)   __DECL_BITFIELD(x)
+#define __DECL_BITFIELD(x)  CPUID_BITFIELD_ ## x
+
+    /* Basic leaves: 0x000000xx */
+    union {
+        struct cpuid_leaf raw[CPUID_GUEST_NR_BASIC];
+        struct {
+            /* Leaf 0x0 - Max and vendor. */
+            uint32_t max_leaf, vendor_ebx, vendor_ecx, vendor_edx;
+
+            /* Leaf 0x1 - Family/model/stepping and features. */
+            uint32_t raw_fms;
+            uint8_t :8,       /* Brand ID. */
+                clflush_size, /* Number of 8-byte blocks per cache line. */
+                lppp,         /* Logical processors per package. */
+                apic_id;      /* Initial APIC ID. */
+            union {
+                uint32_t _1c;
+                struct { DECL_BITFIELD(1c); };
+            };
+            union {
+                uint32_t _1d;
+                struct { DECL_BITFIELD(1d); };
+            };
+
+            /* Leaf 0x2 - TLB/Cache/Prefetch. */
+            uint8_t l2_nr_queries; /* Documented as fixed to 1. */
+            uint8_t l2_desc[15];
+
+            uint64_t :64, :64; /* Leaf 0x3 - PSN. */
+            uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
+            uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
+            uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
+            uint64_t :64, :64; /* Leaf 0x7 - Structured Features. */
+            uint64_t :64, :64; /* Leaf 0x8 - rsvd */
+            uint64_t :64, :64; /* Leaf 0x9 - DCA */
+
+            /* Leaf 0xa - Intel PMU. */
+            uint8_t pmu_version;
+        };
+    } basic;
+
+    /* Structured cache leaf: 0x00000004[xx] */
+    union {
+        struct cpuid_leaf raw[CPUID_GUEST_NR_CACHE];
+        struct cpuid_cache_leaf {
+            uint32_t type:5,
+                :27, :32, :32, :32;
+        } subleaf[CPUID_GUEST_NR_CACHE];
+    } cache;
+
+    /* Structured feature leaf: 0x00000007[xx] */
+    union {
+        struct cpuid_leaf raw[CPUID_GUEST_NR_FEAT];
+        struct {
+            /* Subleaf 0. */
+            uint32_t max_subleaf;
+            union {
+                uint32_t _7b0;
+                struct { DECL_BITFIELD(7b0); };
+            };
+            union {
+                uint32_t _7c0;
+                struct { DECL_BITFIELD(7c0); };
+            };
+            union {
+                uint32_t _7d0;
+                struct { DECL_BITFIELD(7d0); };
+            };
+        };
+    } feat;
+
+    /* Xstate feature leaf: 0x0000000D[xx] */
+    union {
+        struct cpuid_leaf raw[CPUID_GUEST_NR_XSTATE];
+
+        struct {
+            /* Subleaf 0. */
+            uint32_t xcr0_low, /* b */:32, max_size, xcr0_high;
+
+            /* Subleaf 1. */
+            union {
+                uint32_t Da1;
+                struct { DECL_BITFIELD(Da1); };
+            };
+            uint32_t /* b */:32, xss_low, xss_high;
+        };
+
+        /* Per-component common state.  Valid for i >= 2. */
+        struct {
+            uint32_t size, offset;
+            bool xss:1, align:1;
+            uint32_t _res_d;
+        } comp[CPUID_GUEST_NR_XSTATE];
+    } xstate;
+
+    /* Extended leaves: 0x800000xx */
+    union {
+        struct cpuid_leaf raw[CPUID_GUEST_NR_EXTD];
+        struct {
+            /* Leaf 0x80000000 - Max and vendor. */
+            uint32_t max_leaf, vendor_ebx, vendor_ecx, vendor_edx;
+
+            /* Leaf 0x80000001 - Family/model/stepping and features. */
+            uint32_t raw_fms, /* b */:32;
+            union {
+                uint32_t e1c;
+                struct { DECL_BITFIELD(e1c); };
+            };
+            union {
+                uint32_t e1d;
+                struct { DECL_BITFIELD(e1d); };
+            };
+
+            uint64_t :64, :64; /* Brand string. */
+            uint64_t :64, :64; /* Brand string. */
+            uint64_t :64, :64; /* Brand string. */
+            uint64_t :64, :64; /* L1 cache/TLB. */
+            uint64_t :64, :64; /* L2/3 cache/TLB. */
+
+            /* Leaf 0x80000007 - Advanced Power Management. */
+            uint32_t /* a */:32, /* b */:32, /* c */:32;
+            union {
+                uint32_t e7d;
+                struct { DECL_BITFIELD(e7d); };
+            };
+
+            /* Leaf 0x80000008 - Misc addr/feature info. */
+            uint8_t maxphysaddr, maxlinaddr, :8, :8;
+            union {
+                uint32_t e8b;
+                struct { DECL_BITFIELD(e8b); };
+            };
+            uint32_t /* c */:32, /* d */:32;
+        };
+    } extd;
+
+#undef __DECL_BITFIELD
+#undef _DECL_BITFIELD
+#undef DECL_BITFIELD
+
+    /* Toolstack selected Hypervisor max_leaf (if non-zero). */
+    uint8_t hv_limit, hv2_limit;
+
+    /* Value calculated from raw data above. */
+    uint8_t x86_vendor;
+};
+
+/* Fill in a featureset bitmap from a CPUID policy. */
+static inline void cpuid_policy_to_featureset(
+    const struct cpuid_policy *p, uint32_t fs[FSCAPINTS])
+{
+    fs[FEATURESET_1d]  = p->basic._1d;
+    fs[FEATURESET_1c]  = p->basic._1c;
+    fs[FEATURESET_e1d] = p->extd.e1d;
+    fs[FEATURESET_e1c] = p->extd.e1c;
+    fs[FEATURESET_Da1] = p->xstate.Da1;
+    fs[FEATURESET_7b0] = p->feat._7b0;
+    fs[FEATURESET_7c0] = p->feat._7c0;
+    fs[FEATURESET_e7d] = p->extd.e7d;
+    fs[FEATURESET_e8b] = p->extd.e8b;
+    fs[FEATURESET_7d0] = p->feat._7d0;
+}
+
+/* Fill in a CPUID policy from a featureset bitmap. */
+static inline void cpuid_featureset_to_policy(
+    const uint32_t fs[FSCAPINTS], struct cpuid_policy *p)
+{
+    p->basic._1d  = fs[FEATURESET_1d];
+    p->basic._1c  = fs[FEATURESET_1c];
+    p->extd.e1d   = fs[FEATURESET_e1d];
+    p->extd.e1c   = fs[FEATURESET_e1c];
+    p->xstate.Da1 = fs[FEATURESET_Da1];
+    p->feat._7b0  = fs[FEATURESET_7b0];
+    p->feat._7c0  = fs[FEATURESET_7c0];
+    p->extd.e7d   = fs[FEATURESET_e7d];
+    p->extd.e8b   = fs[FEATURESET_e8b];
+    p->feat._7d0  = fs[FEATURESET_7d0];
+}
+
+#endif /* __X86_CPUID_SHARED_H__ */
diff --git a/xen/include/asm-x86/cpuid.h b/xen/include/asm-x86/cpuid.h
index ca664d50e2..970c5a2161 100644
--- a/xen/include/asm-x86/cpuid.h
+++ b/xen/include/asm-x86/cpuid.h
@@ -4,17 +4,6 @@
 #include <asm/cpufeatureset.h>
 #include <asm/percpu.h>
 
-#define FEATURESET_1d     0 /* 0x00000001.edx      */
-#define FEATURESET_1c     1 /* 0x00000001.ecx      */
-#define FEATURESET_e1d    2 /* 0x80000001.edx      */
-#define FEATURESET_e1c    3 /* 0x80000001.ecx      */
-#define FEATURESET_Da1    4 /* 0x0000000d:1.eax    */
-#define FEATURESET_7b0    5 /* 0x00000007:0.ebx    */
-#define FEATURESET_7c0    6 /* 0x00000007:0.ecx    */
-#define FEATURESET_e7d    7 /* 0x80000007.edx      */
-#define FEATURESET_e8b    8 /* 0x80000008.ebx      */
-#define FEATURESET_7d0    9 /* 0x00000007:0.edx    */
-
 #ifndef __ASSEMBLY__
 #include <xen/types.h>
 #include <xen/kernel.h>
@@ -55,205 +44,10 @@ struct cpuidmasks
 /* Per CPU shadows of masking MSR values, for lazy context switching. */
 DECLARE_PER_CPU(struct cpuidmasks, cpuidmasks);
 
+#include "arch-shared/cpuid.h"
+
 /* Default masking MSR values, calculated at boot. */
 extern struct cpuidmasks cpuidmask_defaults;
-
-#define CPUID_GUEST_NR_BASIC      (0xdu + 1)
-#define CPUID_GUEST_NR_FEAT       (0u + 1)
-#define CPUID_GUEST_NR_CACHE      (5u + 1)
-#define CPUID_GUEST_NR_XSTATE     (62u + 1)
-#define CPUID_GUEST_NR_EXTD_INTEL (0x8u + 1)
-#define CPUID_GUEST_NR_EXTD_AMD   (0x1cu + 1)
-#define CPUID_GUEST_NR_EXTD       MAX(CPUID_GUEST_NR_EXTD_INTEL, \
-                                      CPUID_GUEST_NR_EXTD_AMD)
-struct cpuid_leaf
-{
-    uint32_t a, b, c, d;
-};
-
-struct cpuid_policy
-{
-#define DECL_BITFIELD(word) _DECL_BITFIELD(FEATURESET_ ## word)
-#define _DECL_BITFIELD(x)   __DECL_BITFIELD(x)
-#define __DECL_BITFIELD(x)  CPUID_BITFIELD_ ## x
-
-    /* Basic leaves: 0x000000xx */
-    union {
-        struct cpuid_leaf raw[CPUID_GUEST_NR_BASIC];
-        struct {
-            /* Leaf 0x0 - Max and vendor. */
-            uint32_t max_leaf, vendor_ebx, vendor_ecx, vendor_edx;
-
-            /* Leaf 0x1 - Family/model/stepping and features. */
-            uint32_t raw_fms;
-            uint8_t :8,       /* Brand ID. */
-                clflush_size, /* Number of 8-byte blocks per cache line. */
-                lppp,         /* Logical processors per package. */
-                apic_id;      /* Initial APIC ID. */
-            union {
-                uint32_t _1c;
-                struct { DECL_BITFIELD(1c); };
-            };
-            union {
-                uint32_t _1d;
-                struct { DECL_BITFIELD(1d); };
-            };
-
-            /* Leaf 0x2 - TLB/Cache/Prefetch. */
-            uint8_t l2_nr_queries; /* Documented as fixed to 1. */
-            uint8_t l2_desc[15];
-
-            uint64_t :64, :64; /* Leaf 0x3 - PSN. */
-            uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
-            uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
-            uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
-            uint64_t :64, :64; /* Leaf 0x7 - Structured Features. */
-            uint64_t :64, :64; /* Leaf 0x8 - rsvd */
-            uint64_t :64, :64; /* Leaf 0x9 - DCA */
-
-            /* Leaf 0xa - Intel PMU. */
-            uint8_t pmu_version;
-        };
-    } basic;
-
-    /* Structured cache leaf: 0x00000004[xx] */
-    union {
-        struct cpuid_leaf raw[CPUID_GUEST_NR_CACHE];
-        struct cpuid_cache_leaf {
-            uint32_t type:5,
-                :27, :32, :32, :32;
-        } subleaf[CPUID_GUEST_NR_CACHE];
-    } cache;
-
-    /* Structured feature leaf: 0x00000007[xx] */
-    union {
-        struct cpuid_leaf raw[CPUID_GUEST_NR_FEAT];
-        struct {
-            /* Subleaf 0. */
-            uint32_t max_subleaf;
-            union {
-                uint32_t _7b0;
-                struct { DECL_BITFIELD(7b0); };
-            };
-            union {
-                uint32_t _7c0;
-                struct { DECL_BITFIELD(7c0); };
-            };
-            union {
-                uint32_t _7d0;
-                struct { DECL_BITFIELD(7d0); };
-            };
-        };
-    } feat;
-
-    /* Xstate feature leaf: 0x0000000D[xx] */
-    union {
-        struct cpuid_leaf raw[CPUID_GUEST_NR_XSTATE];
-
-        struct {
-            /* Subleaf 0. */
-            uint32_t xcr0_low, /* b */:32, max_size, xcr0_high;
-
-            /* Subleaf 1. */
-            union {
-                uint32_t Da1;
-                struct { DECL_BITFIELD(Da1); };
-            };
-            uint32_t /* b */:32, xss_low, xss_high;
-        };
-
-        /* Per-component common state.  Valid for i >= 2. */
-        struct {
-            uint32_t size, offset;
-            bool xss:1, align:1;
-            uint32_t _res_d;
-        } comp[CPUID_GUEST_NR_XSTATE];
-    } xstate;
-
-    /* Extended leaves: 0x800000xx */
-    union {
-        struct cpuid_leaf raw[CPUID_GUEST_NR_EXTD];
-        struct {
-            /* Leaf 0x80000000 - Max and vendor. */
-            uint32_t max_leaf, vendor_ebx, vendor_ecx, vendor_edx;
-
-            /* Leaf 0x80000001 - Family/model/stepping and features. */
-            uint32_t raw_fms, /* b */:32;
-            union {
-                uint32_t e1c;
-                struct { DECL_BITFIELD(e1c); };
-            };
-            union {
-                uint32_t e1d;
-                struct { DECL_BITFIELD(e1d); };
-            };
-
-            uint64_t :64, :64; /* Brand string. */
-            uint64_t :64, :64; /* Brand string. */
-            uint64_t :64, :64; /* Brand string. */
-            uint64_t :64, :64; /* L1 cache/TLB. */
-            uint64_t :64, :64; /* L2/3 cache/TLB. */
-
-            /* Leaf 0x80000007 - Advanced Power Management. */
-            uint32_t /* a */:32, /* b */:32, /* c */:32;
-            union {
-                uint32_t e7d;
-                struct { DECL_BITFIELD(e7d); };
-            };
-
-            /* Leaf 0x80000008 - Misc addr/feature info. */
-            uint8_t maxphysaddr, maxlinaddr, :8, :8;
-            union {
-                uint32_t e8b;
-                struct { DECL_BITFIELD(e8b); };
-            };
-            uint32_t /* c */:32, /* d */:32;
-        };
-    } extd;
-
-#undef __DECL_BITFIELD
-#undef _DECL_BITFIELD
-#undef DECL_BITFIELD
-
-    /* Toolstack selected Hypervisor max_leaf (if non-zero). */
-    uint8_t hv_limit, hv2_limit;
-
-    /* Value calculated from raw data above. */
-    uint8_t x86_vendor;
-};
-
-/* Fill in a featureset bitmap from a CPUID policy. */
-static inline void cpuid_policy_to_featureset(
-    const struct cpuid_policy *p, uint32_t fs[FSCAPINTS])
-{
-    fs[FEATURESET_1d]  = p->basic._1d;
-    fs[FEATURESET_1c]  = p->basic._1c;
-    fs[FEATURESET_e1d] = p->extd.e1d;
-    fs[FEATURESET_e1c] = p->extd.e1c;
-    fs[FEATURESET_Da1] = p->xstate.Da1;
-    fs[FEATURESET_7b0] = p->feat._7b0;
-    fs[FEATURESET_7c0] = p->feat._7c0;
-    fs[FEATURESET_e7d] = p->extd.e7d;
-    fs[FEATURESET_e8b] = p->extd.e8b;
-    fs[FEATURESET_7d0] = p->feat._7d0;
-}
-
-/* Fill in a CPUID policy from a featureset bitmap. */
-static inline void cpuid_featureset_to_policy(
-    const uint32_t fs[FSCAPINTS], struct cpuid_policy *p)
-{
-    p->basic._1d  = fs[FEATURESET_1d];
-    p->basic._1c  = fs[FEATURESET_1c];
-    p->extd.e1d   = fs[FEATURESET_e1d];
-    p->extd.e1c   = fs[FEATURESET_e1c];
-    p->xstate.Da1 = fs[FEATURESET_Da1];
-    p->feat._7b0  = fs[FEATURESET_7b0];
-    p->feat._7c0  = fs[FEATURESET_7c0];
-    p->extd.e7d   = fs[FEATURESET_e7d];
-    p->extd.e8b   = fs[FEATURESET_e8b];
-    p->feat._7d0  = fs[FEATURESET_7d0];
-}
-
 extern struct cpuid_policy raw_cpuid_policy, host_cpuid_policy,
     pv_max_cpuid_policy, hvm_max_cpuid_policy;
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/5] x86: split out cpuid objects and helpers

[Roger Pau Monné]

On Thu, May 24, 2018 at 05:05:19PM +0100, Wei Liu wrote:
> They are moved to a new header which is going to be consumed by both
> the hypervisor and toolstack.
> 
> Create a new directory for this kind of headers in anticipation of
                                                                  ^that?
> more will come.
> 
> No functional change.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Any suggestion on the directory name?
> ---
>  xen/include/asm-x86/arch-shared/cpuid.h | 213 ++++++++++++++++++++++++++++++++
>  xen/include/asm-x86/cpuid.h             | 210 +------------------------------

I would have placed those inside of:

xen/include/public/arch-x86/cpuid.h

Protected with a #if defined(__XEN__) || defined(__XEN_TOOLS__)?

That's how structures are generally shared between the toolstack and
the hypervisor. But then that would also require you to prefix those
structures with 'xen_', which will add more churn to the patch...

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/5] x86: split out cpuid objects and helpers

[Wei Liu]

On Fri, May 25, 2018 at 11:41:04AM +0200, Roger Pau Monné wrote:
> On Thu, May 24, 2018 at 05:05:19PM +0100, Wei Liu wrote:
> > They are moved to a new header which is going to be consumed by both
> > the hypervisor and toolstack.
> > 
> > Create a new directory for this kind of headers in anticipation of
>                                                                   ^that?
> > more will come.
> > 
> > No functional change.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > 
> > Any suggestion on the directory name?
> > ---
> >  xen/include/asm-x86/arch-shared/cpuid.h | 213 ++++++++++++++++++++++++++++++++
> >  xen/include/asm-x86/cpuid.h             | 210 +------------------------------
> 
> I would have placed those inside of:
> 
> xen/include/public/arch-x86/cpuid.h
> 
> Protected with a #if defined(__XEN__) || defined(__XEN_TOOLS__)?
> 

I envision this is going to grow into libcpuid or something, just like
libelf.

Also this isn't some sort of interface between HV and toolstack -- the
interface works on serialised data. Putting the internal representation
in public header would be wrong.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/5] x86: split out cpuid objects and helpers

[Andrew Cooper]

On 25/05/18 10:49, Wei Liu wrote:
> On Fri, May 25, 2018 at 11:41:04AM +0200, Roger Pau Monné wrote:
>> On Thu, May 24, 2018 at 05:05:19PM +0100, Wei Liu wrote:
>>> They are moved to a new header which is going to be consumed by both
>>> the hypervisor and toolstack.
>>>
>>> Create a new directory for this kind of headers in anticipation of
>>                                                                   ^that?
>>> more will come.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>>> ---
>>> Cc: Jan Beulich <jbeulich@suse.com>
>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>>>
>>> Any suggestion on the directory name?
>>> ---
>>>  xen/include/asm-x86/arch-shared/cpuid.h | 213 ++++++++++++++++++++++++++++++++
>>>  xen/include/asm-x86/cpuid.h             | 210 +------------------------------
>> I would have placed those inside of:
>>
>> xen/include/public/arch-x86/cpuid.h
>>
>> Protected with a #if defined(__XEN__) || defined(__XEN_TOOLS__)?
>>
> I envision this is going to grow into libcpuid or something, just like
> libelf.

I was actually considering making a libx86 or equivalent.  At the
minimum, we'll need a similar split for the MSR code.

Also, making struct {cpuid,msr}_policy available to the emulation code
will drop the code volume massively, and has been on my TODO list for ages.

> Also this isn't some sort of interface between HV and toolstack -- the
> interface works on serialised data. Putting the internal representation
> in public header would be wrong.

+1

We happen to want to share some code between Xen and libxc, but it
doesn't want to be in any kind of public interface.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/5] x86: split out cpuid objects and helpers

[Roger Pau Monné]

On Fri, May 25, 2018 at 11:01:18AM +0100, Andrew Cooper wrote:
> On 25/05/18 10:49, Wei Liu wrote:
> > On Fri, May 25, 2018 at 11:41:04AM +0200, Roger Pau Monné wrote:
> >> On Thu, May 24, 2018 at 05:05:19PM +0100, Wei Liu wrote:
> >>> They are moved to a new header which is going to be consumed by both
> >>> the hypervisor and toolstack.
> >>>
> >>> Create a new directory for this kind of headers in anticipation of
> >>                                                                   ^that?
> >>> more will come.
> >>>
> >>> No functional change.
> >>>
> >>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >>> ---
> >>> Cc: Jan Beulich <jbeulich@suse.com>
> >>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> >>>
> >>> Any suggestion on the directory name?
> >>> ---
> >>>  xen/include/asm-x86/arch-shared/cpuid.h | 213 ++++++++++++++++++++++++++++++++
> >>>  xen/include/asm-x86/cpuid.h             | 210 +------------------------------
> >> I would have placed those inside of:
> >>
> >> xen/include/public/arch-x86/cpuid.h
> >>
> >> Protected with a #if defined(__XEN__) || defined(__XEN_TOOLS__)?
> >>
> > I envision this is going to grow into libcpuid or something, just like
> > libelf.
> 
> I was actually considering making a libx86 or equivalent.  At the
> minimum, we'll need a similar split for the MSR code.
> 
> Also, making struct {cpuid,msr}_policy available to the emulation code
> will drop the code volume massively, and has been on my TODO list for ages.
> 
> > Also this isn't some sort of interface between HV and toolstack -- the
> > interface works on serialised data. Putting the internal representation
> > in public header would be wrong.
> 
> +1
> 
> We happen to want to share some code between Xen and libxc, but it
> doesn't want to be in any kind of public interface.

Maybe xen/common/libx86?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/5] x86: split out cpuid objects and helpers

[Jan Beulich]

>>> On 24.05.18 at 18:05, <wei.liu2@citrix.com> wrote:
> They are moved to a new header which is going to be consumed by both
> the hypervisor and toolstack.

By the end of this series it is not clear what use all of the moved stuff is going
to be to the tool stack. Can we move things as they are indeed gaining a 2nd
use?

> Create a new directory for this kind of headers in anticipation of
> more will come.

I prefer Roger's suggestion to follow more the libelf style of sharing bits.

>  xen/include/asm-x86/arch-shared/cpuid.h | 213 ++++++++++++++++++++++++++++++++
>  xen/include/asm-x86/cpuid.h             | 210 +------------------------------

And if this was the way to go, I further dislike something like arch-*/
underneath asm-*/. How about xen/include/shared/x86/cpuid.h?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 3/5] tools: link arch-shared directory

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 .gitignore             | 1 +
 tools/include/Makefile | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 7004349d5a..808f4f5497 100644
--- a/.gitignore
+++ b/.gitignore
@@ -198,6 +198,7 @@ tools/hotplug/Linux/xendomains
 tools/hotplug/NetBSD/rc.d/xencommons
 tools/hotplug/NetBSD/rc.d/xendriverdomain
 tools/include/acpi
+tools/include/arch-shared
 tools/include/xen/*
 tools/include/xen-xsm/*
 tools/include/xen-foreign/*.(c|h|size)
diff --git a/tools/include/Makefile b/tools/include/Makefile
index 666510530e..2c0a7f6e3c 100644
--- a/tools/include/Makefile
+++ b/tools/include/Makefile
@@ -21,6 +21,9 @@ xen/.dir:
 	ln -sf $(addprefix $(XEN_ROOT)/xen/include/xen/,libelf.h elfstructs.h) xen/libelf/
 	ln -s ../xen-foreign xen/foreign
 	ln -sf $(XEN_ROOT)/xen/include/acpi acpi
+ifeq ($(CONFIG_X86),y)
+	ln -sf $(XEN_ROOT)/xen/include/asm-x86/arch-shared arch-shared
+endif
 	touch $@
 
 # Not xen/xsm as that clashes with link to
@@ -65,7 +68,7 @@ uninstall:
 
 .PHONY: clean
 clean:
-	rm -rf xen xen-xsm acpi
+	rm -rf xen xen-xsm acpi arch-shared
 	$(MAKE) -C xen-foreign clean
 
 .PHONY: dist
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 4/5] libxc: introduce xc_cpuid_x86.h

[Wei Liu]

Collect cpuid related things into a header file. Provide the necessary
macros to make it work.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxc/xc_cpuid_x86.c |  6 +-----
 tools/libxc/xc_cpuid_x86.h | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 5 deletions(-)
 create mode 100644 tools/libxc/xc_cpuid_x86.h

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 9fa2f7c360..89ded718bc 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -27,11 +27,7 @@
 #include <xen/hvm/params.h>
 #include <xen-tools/libs.h>
 
-enum {
-#define XEN_CPUFEATURE(name, value) X86_FEATURE_##name = value,
-#include <xen/arch-x86/cpufeatureset.h>
-};
-#include "_xc_cpuid_autogen.h"
+#include "xc_cpuid_x86.h"
 
 #define bitmaskof(idx)      (1u << ((idx) & 31))
 #define featureword_of(idx) ((idx) >> 5)
diff --git a/tools/libxc/xc_cpuid_x86.h b/tools/libxc/xc_cpuid_x86.h
new file mode 100644
index 0000000000..1ac12f0e30
--- /dev/null
+++ b/tools/libxc/xc_cpuid_x86.h
@@ -0,0 +1,16 @@
+#ifndef XC_X86_CPUID_H
+#define XC_X86_CPUID_H
+
+enum {
+#define XEN_CPUFEATURE(name, value) X86_FEATURE_##name = value,
+#include <xen/arch-x86/cpufeatureset.h>
+};
+#include "_xc_cpuid_autogen.h"
+
+#define MAX(x,y) ((x) > (y) ? (x) : (y))
+
+#define FSCAPINTS FEATURESET_NR_ENTRIES
+
+#include <arch-shared/cpuid.h>
+
+#endif
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 5/5] XXX DO NOT APPLY: compilation test

[Wei Liu]

---
 tools/libxc/xc_cpuid_x86.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 89ded718bc..0af04a4e5a 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -869,3 +869,20 @@ int xc_cpuid_set(
     free_cpuid_domain_info(&info);
     return rc;
 }
+
+static void __attribute__((__unused__)) test_func(void)
+{
+    struct cpuid_leaf leaf;
+    struct cpuid_policy policy;
+    struct cpuid_cache_leaf cache_leaf;
+    uint32_t fs[FSCAPINTS] = { 0, };
+
+    memset(&leaf, 0, sizeof(leaf));
+    memset(&policy, 0, sizeof(policy));
+    memset(&cache_leaf, 0, sizeof(cache_leaf));
+
+    cpuid_policy_to_featureset(&policy, fs);
+    cpuid_featureset_to_policy(fs, &policy);
+
+    return;
+}
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel