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

[Christian Lindig <christian.lindig@xxxxxxxxx>]

> On 10 Mar 2023, at 20:37, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Christian Lindig <christian.lindig@xxxxxxxxxx>
> CC: David Scott <dave@xxxxxxxxxx>
> CC: Edwin Török <edwin.torok@xxxxxxxxx>
> CC: Rob Hoes <Rob.Hoes@xxxxxxxxxx>
> 
> 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@xxxxxxxxx>

> 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
>