[PATCH v2 3/4] x86/cpuid: Introduce dom0-cpuid command line option

Andrew Cooper posted 4 patches 4 years, 1 month ago
[PATCH v2 3/4] x86/cpuid: Introduce dom0-cpuid command line option
Posted by Andrew Cooper 4 years, 1 month ago
Specifically, this lets the user opt in to non-default for dom0.

Collect all dom0 settings together in dom0_{en,dis}able_feat[], and apply it
to dom0's policy when other tweaks are being made.

As recalculate_cpuid_policy() is an expensive action, and dom0-cpuid= is
likely to only be used by the x86 maintainers for development purposes, forgo
the recalculation in the general case.

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>

v2:
 * Rework almost from scratch, on top of broken-out changes.
---
 docs/misc/xen-command-line.pandoc | 17 +++++++++++++++++
 xen/arch/x86/cpuid.c              | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index f7797ea233f9..383a854dec60 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -801,6 +801,23 @@ Controls for how dom0 is constructed on x86 systems.
 
     If using this option is necessary to fix an issue, please report a bug.
 
+### dom0-cpuid
+    = List of comma separated booleans
+
+    Applicability: x86
+
+This option allows for fine tuning of the facilities dom0 will use, after
+accounting for hardware capabilities and Xen settings as enumerated via CPUID.
+
+Options are accepted in positive and negative form, to enable or disable
+specific features, but specify both forms of the same option is undefined.
+All selections via this mechanism are subject to normal CPU Policy safety and
+dependency logic.
+
+This option is intended for developers to opt dom0 into non-default features,
+and is not intended for use in production circumstances.  If using this option
+is necessary to fix an issue, please report a bug.
+
 ### dom0-iommu
     = List of [ passthrough=<bool>, strict=<bool>, map-inclusive=<bool>,
                 map-reserved=<bool>, none ]
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index e11f5a3c9a6b..83a80ba6de70 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -116,6 +116,23 @@ static int __init parse_xen_cpuid(const char *s)
 }
 custom_param("cpuid", parse_xen_cpuid);
 
+static bool __initdata dom0_cpuid_cmdline;
+static uint32_t __initdata dom0_enable_feat[FSCAPINTS];
+static uint32_t __initdata dom0_disable_feat[FSCAPINTS];
+
+static void __init _parse_dom0_cpuid(unsigned int feat, bool val)
+{
+    __set_bit(feat, val ? dom0_enable_feat : dom0_disable_feat);
+}
+
+static int __init parse_dom0_cpuid(const char *s)
+{
+    dom0_cpuid_cmdline = true;
+
+    return parse_cpuid(s, _parse_dom0_cpuid);
+}
+custom_param("dom0-cpuid", parse_dom0_cpuid);
+
 #define EMPTY_LEAF ((struct cpuid_leaf){})
 static void zero_leaves(struct cpuid_leaf *l,
                         unsigned int first, unsigned int last)
@@ -768,6 +785,25 @@ void __init init_dom0_cpuid_policy(struct domain *d)
      */
     if ( cpu_has_arch_caps )
         p->feat.arch_caps = true;
+
+    /* Apply dom0-cpuid= command line settings, if provided. */
+    if ( dom0_cpuid_cmdline )
+    {
+        uint32_t fs[FSCAPINTS];
+        unsigned int i;
+
+        cpuid_policy_to_featureset(p, fs);
+
+        for ( i = 0; i < ARRAY_SIZE(fs); ++i )
+        {
+            fs[i] |= dom0_enable_feat[i];
+            fs[i] &= ~dom0_disable_feat[i];
+        }
+
+        cpuid_featureset_to_policy(fs, p);
+
+        recalculate_cpuid_policy(d);
+    }
 }
 
 void guest_cpuid(const struct vcpu *v, uint32_t leaf,
-- 
2.11.0


Re: [PATCH v2 3/4] x86/cpuid: Introduce dom0-cpuid command line option
Posted by Jan Beulich 4 years, 1 month ago
On 15.12.2021 23:21, Andrew Cooper wrote:
> Specifically, this lets the user opt in to non-default for dom0.
> 
> Collect all dom0 settings together in dom0_{en,dis}able_feat[], and apply it
> to dom0's policy when other tweaks are being made.
> 
> As recalculate_cpuid_policy() is an expensive action, and dom0-cpuid= is
> likely to only be used by the x86 maintainers for development purposes, forgo
> the recalculation in the general case.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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


Re: [PATCH v2 3/4] x86/cpuid: Introduce dom0-cpuid command line option
Posted by Andrew Cooper 4 years, 1 month ago
On 15/12/2021 22:21, Andrew Cooper wrote:
> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
> index e11f5a3c9a6b..83a80ba6de70 100644
> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -116,6 +116,23 @@ static int __init parse_xen_cpuid(const char *s)
>  }
>  custom_param("cpuid", parse_xen_cpuid);
>  
> +static bool __initdata dom0_cpuid_cmdline;
> +static uint32_t __initdata dom0_enable_feat[FSCAPINTS];
> +static uint32_t __initdata dom0_disable_feat[FSCAPINTS];
> +
> +static void __init _parse_dom0_cpuid(unsigned int feat, bool val)
> +{
> +    __set_bit(feat, val ? dom0_enable_feat : dom0_disable_feat);

Based on Jan's observation in v1, I've folded this delta in:

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 83a80ba6de70..39baeae9a6cd 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -122,7 +122,8 @@ static uint32_t __initdata dom0_disable_feat[FSCAPINTS];
 
 static void __init _parse_dom0_cpuid(unsigned int feat, bool val)
 {
-    __set_bit(feat, val ? dom0_enable_feat : dom0_disable_feat);
+    __set_bit  (feat, val ? dom0_enable_feat  : dom0_disable_feat);
+    __clear_bit(feat, val ? dom0_disable_feat : dom0_enable_feat );
 }
 
 static int __init parse_dom0_cpuid(const char *s)

~Andrew

Re: [PATCH v2 3/4] x86/cpuid: Introduce dom0-cpuid command line option
Posted by Jan Beulich 4 years, 1 month ago
On 16.12.2021 12:56, Andrew Cooper wrote:
> On 15/12/2021 22:21, Andrew Cooper wrote:
>> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
>> index e11f5a3c9a6b..83a80ba6de70 100644
>> --- a/xen/arch/x86/cpuid.c
>> +++ b/xen/arch/x86/cpuid.c
>> @@ -116,6 +116,23 @@ static int __init parse_xen_cpuid(const char *s)
>>  }
>>  custom_param("cpuid", parse_xen_cpuid);
>>  
>> +static bool __initdata dom0_cpuid_cmdline;
>> +static uint32_t __initdata dom0_enable_feat[FSCAPINTS];
>> +static uint32_t __initdata dom0_disable_feat[FSCAPINTS];
>> +
>> +static void __init _parse_dom0_cpuid(unsigned int feat, bool val)
>> +{
>> +    __set_bit(feat, val ? dom0_enable_feat : dom0_disable_feat);
> 
> Based on Jan's observation in v1, I've folded this delta in:
> 
> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
> index 83a80ba6de70..39baeae9a6cd 100644
> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -122,7 +122,8 @@ static uint32_t __initdata dom0_disable_feat[FSCAPINTS];
>  
>  static void __init _parse_dom0_cpuid(unsigned int feat, bool val)
>  {
> -    __set_bit(feat, val ? dom0_enable_feat : dom0_disable_feat);
> +    __set_bit  (feat, val ? dom0_enable_feat  : dom0_disable_feat);
> +    __clear_bit(feat, val ? dom0_disable_feat : dom0_enable_feat );
>  }

FAOD my R-b applies with this included; I had meant to reply here
but then replied to the original patch.

Jan