[PATCH 0/3] x86: Minor improvements to xen-cpuid

[PATCH 0/3] x86: Minor improvements to xen-cpuid

[Andrew Cooper]

All to make it easier to do CPUID development work (which is fairness is
mostly me).

Andrew Cooper (3):
  tools/xen-cpuid: Support dash as a bitmap delimiter
  tools/xen-cpuid: Rework the handling of dynamic featuresets
  x86/sysctl: Retrofit XEN_SYSCTL_cpu_featureset_{pv,hvm}_max

 tools/misc/xen-cpuid.c          | 60 +++++++++++++++------------------
 tools/ocaml/libs/xc/xenctrl.ml  |  8 ++++-
 tools/ocaml/libs/xc/xenctrl.mli |  8 ++++-
 xen/arch/x86/sysctl.c           |  4 ++-
 xen/include/public/sysctl.h     |  2 ++
 5 files changed, 46 insertions(+), 36 deletions(-)

-- 
2.30.2

[PATCH 1/3] tools/xen-cpuid: Support dash as a bitmap delimiter

[Andrew Cooper]

Xapi chose to render its featureset bitmaps with dashes rather than colongs as
a separator.  Have xen-cpuid support both forms, for convenience.

Fix a trivial style issue in the vacinity while here.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/misc/xen-cpuid.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 868054ab96a6..361102d8cfb1 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -607,9 +607,10 @@ int main(int argc, char **argv)
                 if ( i == nr_features )
                     break;
 
-                if ( *ptr == ':' )
+                if ( *ptr == ':' || *ptr == '-' )
                 {
-                    ptr++; continue;
+                    ptr++;
+                    continue;
                 }
                 break;
             }
-- 
2.30.2

[PATCH 2/3] tools/xen-cpuid: Rework the handling of dynamic featuresets

[Andrew Cooper]

struct fsinfo is the vestigial remnant of an older internal design which
didn't survive very long.

Simplify things by inlining get_featureset() and having a single memory
allocation that gets reused.  This in turn changes featuresets[] to be a
simple list of names, so rename it to fs_names[].

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/misc/xen-cpuid.c | 53 ++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 31 deletions(-)

diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 361102d8cfb1..227df7352e2b 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -246,16 +246,11 @@ static const struct {
 
 #define COL_ALIGN "18"
 
-static struct fsinfo {
-    const char *name;
-    uint32_t len;
-    uint32_t *fs;
-} featuresets[] =
-{
-    [XEN_SYSCTL_cpu_featureset_host] = { "Host", 0, NULL },
-    [XEN_SYSCTL_cpu_featureset_raw]  = { "Raw",  0, NULL },
-    [XEN_SYSCTL_cpu_featureset_pv]   = { "PV",   0, NULL },
-    [XEN_SYSCTL_cpu_featureset_hvm]  = { "HVM",  0, NULL },
+static const char *const fs_names[] = {
+    [XEN_SYSCTL_cpu_featureset_host] = "Host",
+    [XEN_SYSCTL_cpu_featureset_raw]  = "Raw",
+    [XEN_SYSCTL_cpu_featureset_pv]   = "PV",
+    [XEN_SYSCTL_cpu_featureset_hvm]  = "HVM",
 };
 
 static void dump_leaf(uint32_t leaf, const char *const *strs)
@@ -302,22 +297,10 @@ static void decode_featureset(const uint32_t *features,
     }
 }
 
-static int get_featureset(xc_interface *xch, unsigned int idx)
-{
-    struct fsinfo *f = &featuresets[idx];
-
-    f->len = nr_features;
-    f->fs = calloc(nr_features, sizeof(*f->fs));
-
-    if ( !f->fs )
-        err(1, "calloc(, featureset)");
-
-    return xc_get_cpu_featureset(xch, idx, &f->len, f->fs);
-}
-
 static void dump_info(xc_interface *xch, bool detail)
 {
     unsigned int i;
+    uint32_t *fs;
 
     printf("nr_features: %u\n", nr_features);
 
@@ -348,26 +331,34 @@ static void dump_info(xc_interface *xch, bool detail)
                       nr_features, "HVM Hap Default", detail);
 
     printf("\nDynamic sets:\n");
-    for ( i = 0; i < ARRAY_SIZE(featuresets); ++i )
+
+    fs = malloc(sizeof(*fs) * nr_features);
+    if ( !fs )
+        err(1, "malloc(featureset)");
+
+    for ( i = 0; i < ARRAY_SIZE(fs_names); ++i )
     {
-        if ( get_featureset(xch, i) )
+        uint32_t len = nr_features;
+        int ret;
+
+        memset(fs, 0, sizeof(*fs) * nr_features);
+
+        ret = xc_get_cpu_featureset(xch, i, &len, fs);
+        if ( ret )
         {
             if ( errno == EOPNOTSUPP )
             {
-                printf("%s featureset not supported by Xen\n",
-                       featuresets[i].name);
+                printf("%s featureset not supported by Xen\n", fs_names[i]);
                 continue;
             }
 
             err(1, "xc_get_featureset()");
         }
 
-        decode_featureset(featuresets[i].fs, featuresets[i].len,
-                          featuresets[i].name, detail);
+        decode_featureset(fs, len, fs_names[i], detail);
     }
 
-    for ( i = 0; i < ARRAY_SIZE(featuresets); ++i )
-        free(featuresets[i].fs);
+    free(fs);
 }
 
 static void print_policy(const char *name,
-- 
2.30.2

[PATCH 3/3] x86/sysctl: Retrofit XEN_SYSCTL_cpu_featureset_{pv,hvm}_max

[Andrew Cooper]

Featuresets are supposed to be disappearing when the CPU policy infrastructure
is complete, but that has taken longer than expected, and isn't going to be
complete imminently either.

In the meantime, Xen does have proper default/max featuresets, and xen-cpuid
can even get them via the XEN_SYSCTL_cpu_policy_* interface, but only knows
now to render them nicely via the featureset interface.

Differences between default and max are a frequent source of errors,
frequently too in secret leading up to an embargo, so extend the featureset
sysctl to allow xen-cpuid to render them all nicely.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Christian Lindig <christian.lindig@citrix.com>
CC: David Scott <dave@recoil.org>
CC: Edwin Török <edwin.torok@cloud.com>
CC: Rob Hoes <Rob.Hoes@citrix.com>

I actually this SYSCTL extention in the XenServer patchqueue for reasons that
started with the TSX fiasco; I have no idea why its taken until now to think
it would be a good idea to wire into xen-cpuid too...
---
 tools/misc/xen-cpuid.c          | 10 ++++++----
 tools/ocaml/libs/xc/xenctrl.ml  |  8 +++++++-
 tools/ocaml/libs/xc/xenctrl.mli |  8 +++++++-
 xen/arch/x86/sysctl.c           |  4 +++-
 xen/include/public/sysctl.h     |  2 ++
 5 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 227df7352e2b..37a7eaa8edfc 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -247,10 +247,12 @@ static const struct {
 #define COL_ALIGN "18"
 
 static const char *const fs_names[] = {
-    [XEN_SYSCTL_cpu_featureset_host] = "Host",
-    [XEN_SYSCTL_cpu_featureset_raw]  = "Raw",
-    [XEN_SYSCTL_cpu_featureset_pv]   = "PV",
-    [XEN_SYSCTL_cpu_featureset_hvm]  = "HVM",
+    [XEN_SYSCTL_cpu_featureset_raw]     = "Raw",
+    [XEN_SYSCTL_cpu_featureset_host]    = "Host",
+    [XEN_SYSCTL_cpu_featureset_pv]      = "PV Default",
+    [XEN_SYSCTL_cpu_featureset_hvm]     = "HVM Default",
+    [XEN_SYSCTL_cpu_featureset_pv_max]  = "PV Max",
+    [XEN_SYSCTL_cpu_featureset_hvm_max] = "HVM Max",
 };
 
 static void dump_leaf(uint32_t leaf, const char *const *strs)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index a59dee093897..e4096bf92c1d 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -370,7 +370,13 @@ external version_changeset: handle -> string = "stub_xc_version_changeset"
 external version_capabilities: handle -> string =
   "stub_xc_version_capabilities"
 
-type featureset_index = Featureset_raw | Featureset_host | Featureset_pv | Featureset_hvm
+type featureset_index =
+  | Featureset_raw
+  | Featureset_host
+  | Featureset_pv
+  | Featureset_hvm
+  | Featureset_pv_max
+  | Featureset_hvm_max
 external get_cpu_featureset : handle -> featureset_index -> int64 array = "stub_xc_get_cpu_featureset"
 
 external watchdog : handle -> int -> int32 -> int
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 3154e90f4f98..ef2254537430 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -297,7 +297,13 @@ external version_changeset : handle -> string = "stub_xc_version_changeset"
 external version_capabilities : handle -> string
   = "stub_xc_version_capabilities"
 
-type featureset_index = Featureset_raw | Featureset_host | Featureset_pv | Featureset_hvm
+type featureset_index =
+  | Featureset_raw
+  | Featureset_host
+  | Featureset_pv
+  | Featureset_hvm
+  | Featureset_pv_max
+  | Featureset_hvm_max
 external get_cpu_featureset : handle -> featureset_index -> int64 array = "stub_xc_get_cpu_featureset"
 
 external pages_to_kib : int64 -> int64 = "stub_pages_to_kib"
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index f42a3b843ba7..6600eb43471b 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -323,14 +323,16 @@ long arch_do_sysctl(
 
     case XEN_SYSCTL_get_cpu_featureset:
     {
-        static const struct cpuid_policy *const policy_table[4] = {
+        static const struct cpuid_policy *const policy_table[6] = {
             [XEN_SYSCTL_cpu_featureset_raw]  = &raw_cpuid_policy,
             [XEN_SYSCTL_cpu_featureset_host] = &host_cpuid_policy,
 #ifdef CONFIG_PV
             [XEN_SYSCTL_cpu_featureset_pv]   = &pv_def_cpuid_policy,
+            [XEN_SYSCTL_cpu_featureset_pv_max] = &pv_max_cpuid_policy,
 #endif
 #ifdef CONFIG_HVM
             [XEN_SYSCTL_cpu_featureset_hvm]  = &hvm_def_cpuid_policy,
+            [XEN_SYSCTL_cpu_featureset_hvm_max] = &hvm_max_cpuid_policy,
 #endif
         };
         const struct cpuid_policy *p = NULL;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 001a4de27375..e8dded9fb94a 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -796,6 +796,8 @@ struct xen_sysctl_cpu_featureset {
 #define XEN_SYSCTL_cpu_featureset_host     1
 #define XEN_SYSCTL_cpu_featureset_pv       2
 #define XEN_SYSCTL_cpu_featureset_hvm      3
+#define XEN_SYSCTL_cpu_featureset_pv_max   4
+#define XEN_SYSCTL_cpu_featureset_hvm_max  5
     uint32_t index;       /* IN: Which featureset to query? */
     uint32_t nr_features; /* IN/OUT: Number of entries in/written to
                            * 'features', or the maximum number of features if
-- 
2.30.2

Re: [PATCH 3/3] x86/sysctl: Retrofit XEN_SYSCTL_cpu_featureset_{pv,hvm}_max

[Christian Lindig]

> On 10 Mar 2023, at 20:37, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote:
> 
> Featuresets are supposed to be disappearing when the CPU policy infrastructure
> is complete, but that has taken longer than expected, and isn't going to be
> complete imminently either.
> 
> In the meantime, Xen does have proper default/max featuresets, and xen-cpuid
> can even get them via the XEN_SYSCTL_cpu_policy_* interface, but only knows
> now to render them nicely via the featureset interface.
> 
> Differences between default and max are a frequent source of errors,
> frequently too in secret leading up to an embargo, so extend the featureset
> sysctl to allow xen-cpuid to render them all nicely.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Christian Lindig <christian.lindig@citrix.com>
> CC: David Scott <dave@recoil.org>
> CC: Edwin Török <edwin.torok@cloud.com>
> CC: Rob Hoes <Rob.Hoes@citrix.com>
> 
> I actually this SYSCTL extention in the XenServer patchqueue for reasons that
> started with the TSX fiasco; I have no idea why its taken until now to think
> it would be a good idea to wire into xen-cpuid too...
> ---
> tools/misc/xen-cpuid.c          | 10 ++++++----
> tools/ocaml/libs/xc/xenctrl.ml  |  8 +++++++-
> tools/ocaml/libs/xc/xenctrl.mli |  8 +++++++-
> xen/arch/x86/sysctl.c           |  4 +++-
> xen/include/public/sysctl.h     |  2 ++
> 5 files changed, 25 insertions(+), 7 deletions(-)
> 


Acked-by: Christian Lindig <christian.lindig@cloud.com>

> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
> index 227df7352e2b..37a7eaa8edfc 100644
> --- a/tools/misc/xen-cpuid.c
> +++ b/tools/misc/xen-cpuid.c
> @@ -247,10 +247,12 @@ static const struct {
> #define COL_ALIGN "18"
> 
> static const char *const fs_names[] = {
> -    [XEN_SYSCTL_cpu_featureset_host] = "Host",
> -    [XEN_SYSCTL_cpu_featureset_raw]  = "Raw",
> -    [XEN_SYSCTL_cpu_featureset_pv]   = "PV",
> -    [XEN_SYSCTL_cpu_featureset_hvm]  = "HVM",
> +    [XEN_SYSCTL_cpu_featureset_raw]     = "Raw",
> +    [XEN_SYSCTL_cpu_featureset_host]    = "Host",
> +    [XEN_SYSCTL_cpu_featureset_pv]      = "PV Default",
> +    [XEN_SYSCTL_cpu_featureset_hvm]     = "HVM Default",
> +    [XEN_SYSCTL_cpu_featureset_pv_max]  = "PV Max",
> +    [XEN_SYSCTL_cpu_featureset_hvm_max] = "HVM Max",
> };
> 
> static void dump_leaf(uint32_t leaf, const char *const *strs)
> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> index a59dee093897..e4096bf92c1d 100644
> --- a/tools/ocaml/libs/xc/xenctrl.ml
> +++ b/tools/ocaml/libs/xc/xenctrl.ml
> @@ -370,7 +370,13 @@ external version_changeset: handle -> string = "stub_xc_version_changeset"
> external version_capabilities: handle -> string =
>   "stub_xc_version_capabilities"
> 
> -type featureset_index = Featureset_raw | Featureset_host | Featureset_pv | Featureset_hvm
> +type featureset_index =
> +  | Featureset_raw
> +  | Featureset_host
> +  | Featureset_pv
> +  | Featureset_hvm
> +  | Featureset_pv_max
> +  | Featureset_hvm_max
> external get_cpu_featureset : handle -> featureset_index -> int64 array = "stub_xc_get_cpu_featureset"
> 
> external watchdog : handle -> int -> int32 -> int
> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
> index 3154e90f4f98..ef2254537430 100644
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -297,7 +297,13 @@ external version_changeset : handle -> string = "stub_xc_version_changeset"
> external version_capabilities : handle -> string
>   = "stub_xc_version_capabilities"
> 
> -type featureset_index = Featureset_raw | Featureset_host | Featureset_pv | Featureset_hvm
> +type featureset_index =
> +  | Featureset_raw
> +  | Featureset_host
> +  | Featureset_pv
> +  | Featureset_hvm
> +  | Featureset_pv_max
> +  | Featureset_hvm_max
> external get_cpu_featureset : handle -> featureset_index -> int64 array = "stub_xc_get_cpu_featureset"
> 
> external pages_to_kib : int64 -> int64 = "stub_pages_to_kib"
> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> index f42a3b843ba7..6600eb43471b 100644
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -323,14 +323,16 @@ long arch_do_sysctl(
> 
>     case XEN_SYSCTL_get_cpu_featureset:
>     {
> -        static const struct cpuid_policy *const policy_table[4] = {
> +        static const struct cpuid_policy *const policy_table[6] = {
>             [XEN_SYSCTL_cpu_featureset_raw]  = &raw_cpuid_policy,
>             [XEN_SYSCTL_cpu_featureset_host] = &host_cpuid_policy,
> #ifdef CONFIG_PV
>             [XEN_SYSCTL_cpu_featureset_pv]   = &pv_def_cpuid_policy,
> +            [XEN_SYSCTL_cpu_featureset_pv_max] = &pv_max_cpuid_policy,
> #endif
> #ifdef CONFIG_HVM
>             [XEN_SYSCTL_cpu_featureset_hvm]  = &hvm_def_cpuid_policy,
> +            [XEN_SYSCTL_cpu_featureset_hvm_max] = &hvm_max_cpuid_policy,
> #endif
>         };
>         const struct cpuid_policy *p = NULL;
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 001a4de27375..e8dded9fb94a 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -796,6 +796,8 @@ struct xen_sysctl_cpu_featureset {
> #define XEN_SYSCTL_cpu_featureset_host     1
> #define XEN_SYSCTL_cpu_featureset_pv       2
> #define XEN_SYSCTL_cpu_featureset_hvm      3
> +#define XEN_SYSCTL_cpu_featureset_pv_max   4
> +#define XEN_SYSCTL_cpu_featureset_hvm_max  5
>     uint32_t index;       /* IN: Which featureset to query? */
>     uint32_t nr_features; /* IN/OUT: Number of entries in/written to
>                            * 'features', or the maximum number of features if
> -- 
> 2.30.2
> 

Re: [PATCH 0/3] x86: Minor improvements to xen-cpuid

[Jan Beulich]

On 10.03.2023 21:37, Andrew Cooper wrote:
> All to make it easier to do CPUID development work (which is fairness is
> mostly me).
> 
> Andrew Cooper (3):
>   tools/xen-cpuid: Support dash as a bitmap delimiter
>   tools/xen-cpuid: Rework the handling of dynamic featuresets
>   x86/sysctl: Retrofit XEN_SYSCTL_cpu_featureset_{pv,hvm}_max

Acked-by: Jan Beulich <jbeulich@suse.com>