[PATCH 0/2] x86: Misc enumeration improvements

[PATCH 0/2] x86: Misc enumeration improvements

[Andrew Cooper]

Patch 1 noticed because xen-cpuid currently says:

  [13] 0x00000007:2.edx   intel-psfd <1> <2> <4> mcdt-no

when dumping the static known-feature bitmap.  In some copious free time, we
should have a unit test that catches this.

Patch 2 when double checking that the enumeration hole was in fact correct.

We should offer DDP-CTRL to guests, but I'm not sure when it is appearing in
hardware, and it's not massively urgent - the DDP behaviour is mostly sane
(having been designed in a post-spectre world) and this is a just-in-case
control.

Andrew Cooper (2):
  x86/spec-ctrl: Add BHI controls to userspace components
  x86/spec-ctrl: Enumerations for DDP controls

 tools/libs/light/libxl_cpuid.c              | 4 ++++
 tools/misc/xen-cpuid.c                      | 6 +++---
 xen/arch/x86/include/asm/msr-index.h        | 1 +
 xen/include/public/arch-x86/cpufeatureset.h | 1 +
 4 files changed, 9 insertions(+), 3 deletions(-)

-- 
2.30.2

[PATCH 1/2] x86/spec-ctrl: Add BHI controls to userspace components

[Andrew Cooper]

This was an oversight when adding the Xen parts.

Fixes: cea9ae062295 ("x86/spec-ctrl: Enumeration for new Intel BHI controls")
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>
---
 tools/libs/light/libxl_cpuid.c | 3 +++
 tools/misc/xen-cpuid.c         | 6 +++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c
index 06b1227a2fd3..1d4e8a6b0067 100644
--- a/tools/libs/light/libxl_cpuid.c
+++ b/tools/libs/light/libxl_cpuid.c
@@ -240,6 +240,9 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str)
         {"cet-sss",      0x00000007,  1, CPUID_REG_EDX, 18,  1},
 
         {"intel-psfd",   0x00000007,  2, CPUID_REG_EDX,  0,  1},
+        {"ipred-ctrl",   0x00000007,  2, CPUID_REG_EDX,  1,  1},
+        {"rrsba-ctrl",   0x00000007,  2, CPUID_REG_EDX,  2,  1},
+        {"bhi-ctrl",     0x00000007,  2, CPUID_REG_EDX,  4,  1},
         {"mcdt-no",      0x00000007,  2, CPUID_REG_EDX,  5,  1},
 
         {"lahfsahf",     0x80000001, NA, CPUID_REG_ECX,  0,  1},
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 3ab820571d8d..4f4261f4aa95 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -215,9 +215,9 @@ static const char *const str_7d1[32] =
 
 static const char *const str_7d2[32] =
 {
-    [ 0] = "intel-psfd",
-
-    /* 4 */                 [ 5] = "mcdt-no",
+    [ 0] = "intel-psfd",    [ 1] = "ipred-ctrl",
+    [ 2] = "rrsba-ctrl",
+    [ 4] = "bhi-ctrl",      [ 5] = "mcdt-no",
 };
 
 static const struct {
-- 
2.30.2

[PATCH 2/2] x86/spec-ctrl: Enumerations for DDP controls

[Andrew Cooper]

https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/data-dependent-prefetcher.html

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>
---
 tools/libs/light/libxl_cpuid.c              | 1 +
 tools/misc/xen-cpuid.c                      | 2 +-
 xen/arch/x86/include/asm/msr-index.h        | 1 +
 xen/include/public/arch-x86/cpufeatureset.h | 1 +
 4 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c
index 1d4e8a6b0067..968635981086 100644
--- a/tools/libs/light/libxl_cpuid.c
+++ b/tools/libs/light/libxl_cpuid.c
@@ -242,6 +242,7 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str)
         {"intel-psfd",   0x00000007,  2, CPUID_REG_EDX,  0,  1},
         {"ipred-ctrl",   0x00000007,  2, CPUID_REG_EDX,  1,  1},
         {"rrsba-ctrl",   0x00000007,  2, CPUID_REG_EDX,  2,  1},
+        {"ddp-ctrl",     0x00000007,  2, CPUID_REG_EDX,  3,  1},
         {"bhi-ctrl",     0x00000007,  2, CPUID_REG_EDX,  4,  1},
         {"mcdt-no",      0x00000007,  2, CPUID_REG_EDX,  5,  1},
 
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 4f4261f4aa95..868054ab96a6 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -216,7 +216,7 @@ static const char *const str_7d1[32] =
 static const char *const str_7d2[32] =
 {
     [ 0] = "intel-psfd",    [ 1] = "ipred-ctrl",
-    [ 2] = "rrsba-ctrl",
+    [ 2] = "rrsba-ctrl",    [ 3] = "ddp-ctrl",
     [ 4] = "bhi-ctrl",      [ 5] = "mcdt-no",
 };
 
diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 7615d8087f46..4e90b361eb08 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -41,6 +41,7 @@
 #define  SPEC_CTRL_RRSBA_DIS_U              (_AC(1, ULL) <<  5)
 #define  SPEC_CTRL_RRSBA_DIS_S              (_AC(1, ULL) <<  6)
 #define  SPEC_CTRL_PSFD                     (_AC(1, ULL) <<  7)
+#define  SPEC_CTRL_DDPU_D                   (_AC(1, ULL) <<  8)
 #define  SPEC_CTRL_BHI_DIS_S                (_AC(1, ULL) << 10)
 
 #define MSR_PRED_CMD                        0x00000049
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 336744b47119..4976a27690af 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -295,6 +295,7 @@ XEN_CPUFEATURE(INTEL_PSFD,         13*32+ 0) /*A  MSR_SPEC_CTRL.PSFD */
 XEN_CPUFEATURE(IPRED_CTRL,         13*32+ 1) /*   MSR_SPEC_CTRL.IPRED_DIS_* */
 XEN_CPUFEATURE(RRSBA_CTRL,         13*32+ 2) /*   MSR_SPEC_CTRL.RRSBA_DIS_* */
 XEN_CPUFEATURE(BHI_CTRL,           13*32+ 4) /*   MSR_SPEC_CTRL.BHI_DIS_S */
+XEN_CPUFEATURE(DDP_CTRL,           13*32+ 3) /*   MSR_SPEC_CTRL.DDPU_U */
 XEN_CPUFEATURE(MCDT_NO,            13*32+ 5) /*A  MCDT_NO */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:1.ecx, word 14 */
-- 
2.30.2

Re: [PATCH 1/2] x86/spec-ctrl: Add BHI controls to userspace components

[Jan Beulich]

On 10.03.2023 17:02, Andrew Cooper wrote:
> This was an oversight when adding the Xen parts.
> 
> Fixes: cea9ae062295 ("x86/spec-ctrl: Enumeration for new Intel BHI controls")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH 2/2] x86/spec-ctrl: Enumerations for DDP controls

[Jan Beulich]

On 10.03.2023 17:02, Andrew Cooper wrote:
> https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/data-dependent-prefetcher.html
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

In the cover letter you mention that we should enable use of this by guests.
Perhaps also mention this here?

> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -41,6 +41,7 @@
>  #define  SPEC_CTRL_RRSBA_DIS_U              (_AC(1, ULL) <<  5)
>  #define  SPEC_CTRL_RRSBA_DIS_S              (_AC(1, ULL) <<  6)
>  #define  SPEC_CTRL_PSFD                     (_AC(1, ULL) <<  7)
> +#define  SPEC_CTRL_DDPU_D                   (_AC(1, ULL) <<  8)

The doc calls this DDPD_U - typo, or do you happen to know that the doc has
it the wrong way round (and is going to be fixed)?

> --- a/xen/include/public/arch-x86/cpufeatureset.h
> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> @@ -295,6 +295,7 @@ XEN_CPUFEATURE(INTEL_PSFD,         13*32+ 0) /*A  MSR_SPEC_CTRL.PSFD */
>  XEN_CPUFEATURE(IPRED_CTRL,         13*32+ 1) /*   MSR_SPEC_CTRL.IPRED_DIS_* */
>  XEN_CPUFEATURE(RRSBA_CTRL,         13*32+ 2) /*   MSR_SPEC_CTRL.RRSBA_DIS_* */
>  XEN_CPUFEATURE(BHI_CTRL,           13*32+ 4) /*   MSR_SPEC_CTRL.BHI_DIS_S */
> +XEN_CPUFEATURE(DDP_CTRL,           13*32+ 3) /*   MSR_SPEC_CTRL.DDPU_U */
>  XEN_CPUFEATURE(MCDT_NO,            13*32+ 5) /*A  MCDT_NO */

Looks like an off-by-1 in where you add the new line.

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

Jan

Re: [PATCH 2/2] x86/spec-ctrl: Enumerations for DDP controls

[Jan Beulich]

On 13.03.2023 10:21, Jan Beulich wrote:
> On 10.03.2023 17:02, Andrew Cooper wrote:
>> --- a/xen/arch/x86/include/asm/msr-index.h
>> +++ b/xen/arch/x86/include/asm/msr-index.h
>> @@ -41,6 +41,7 @@
>>  #define  SPEC_CTRL_RRSBA_DIS_U              (_AC(1, ULL) <<  5)
>>  #define  SPEC_CTRL_RRSBA_DIS_S              (_AC(1, ULL) <<  6)
>>  #define  SPEC_CTRL_PSFD                     (_AC(1, ULL) <<  7)
>> +#define  SPEC_CTRL_DDPU_D                   (_AC(1, ULL) <<  8)
> 
> The doc calls this DDPD_U - typo, or do you happen to know that the doc has
> it the wrong way round (and is going to be fixed)?

Actually ...

>> --- a/xen/include/public/arch-x86/cpufeatureset.h
>> +++ b/xen/include/public/arch-x86/cpufeatureset.h
>> @@ -295,6 +295,7 @@ XEN_CPUFEATURE(INTEL_PSFD,         13*32+ 0) /*A  MSR_SPEC_CTRL.PSFD */
>>  XEN_CPUFEATURE(IPRED_CTRL,         13*32+ 1) /*   MSR_SPEC_CTRL.IPRED_DIS_* */
>>  XEN_CPUFEATURE(RRSBA_CTRL,         13*32+ 2) /*   MSR_SPEC_CTRL.RRSBA_DIS_* */
>>  XEN_CPUFEATURE(BHI_CTRL,           13*32+ 4) /*   MSR_SPEC_CTRL.BHI_DIS_S */
>> +XEN_CPUFEATURE(DDP_CTRL,           13*32+ 3) /*   MSR_SPEC_CTRL.DDPU_U */

... here you have even two 'U' in the comment, when one of them wants to
be 'D'.

Jan

Re: [PATCH 2/2] x86/spec-ctrl: Enumerations for DDP controls

[Andrew Cooper]

On 13/03/2023 9:24 am, Jan Beulich wrote:
> On 13.03.2023 10:21, Jan Beulich wrote:
>> On 10.03.2023 17:02, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/include/asm/msr-index.h
>>> +++ b/xen/arch/x86/include/asm/msr-index.h
>>> @@ -41,6 +41,7 @@
>>>  #define  SPEC_CTRL_RRSBA_DIS_U              (_AC(1, ULL) <<  5)
>>>  #define  SPEC_CTRL_RRSBA_DIS_S              (_AC(1, ULL) <<  6)
>>>  #define  SPEC_CTRL_PSFD                     (_AC(1, ULL) <<  7)
>>> +#define  SPEC_CTRL_DDPU_D                   (_AC(1, ULL) <<  8)
>> The doc calls this DDPD_U - typo, or do you happen to know that the doc has
>> it the wrong way round (and is going to be fixed)?
> Actually ...
>
>>> --- a/xen/include/public/arch-x86/cpufeatureset.h
>>> +++ b/xen/include/public/arch-x86/cpufeatureset.h
>>> @@ -295,6 +295,7 @@ XEN_CPUFEATURE(INTEL_PSFD,         13*32+ 0) /*A  MSR_SPEC_CTRL.PSFD */
>>>  XEN_CPUFEATURE(IPRED_CTRL,         13*32+ 1) /*   MSR_SPEC_CTRL.IPRED_DIS_* */
>>>  XEN_CPUFEATURE(RRSBA_CTRL,         13*32+ 2) /*   MSR_SPEC_CTRL.RRSBA_DIS_* */
>>>  XEN_CPUFEATURE(BHI_CTRL,           13*32+ 4) /*   MSR_SPEC_CTRL.BHI_DIS_S */
>>> +XEN_CPUFEATURE(DDP_CTRL,           13*32+ 3) /*   MSR_SPEC_CTRL.DDPU_U */
> ... here you have even two 'U' in the comment, when one of them wants to
> be 'D'.

Yeah, I've messed this up.  It's supposed to be consistent with the
public document (which I ought to have checked was consistent with the
non-public reference that I was using...)

I'll fix up and repost.

~Andrew

[PATCH v2 2/2] x86/spec-ctrl: Enumerations for DDP controls

[Andrew Cooper]

https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/data-dependent-prefetcher.html

We should offer DDP-CTRL to guests, but I'm not sure when it is appearing in
hardware, and it's not massively urgent - the DDP behaviour is mostly sane
(having been designed in a post-spectre world) and this is a just-in-case
control.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

v2:
 * Use DDP_DIS_U for consistency with the other bit names.
 * Fix ordering/typos, expand commit message.
---
 tools/libs/light/libxl_cpuid.c              | 1 +
 tools/misc/xen-cpuid.c                      | 2 +-
 xen/arch/x86/include/asm/msr-index.h        | 1 +
 xen/include/public/arch-x86/cpufeatureset.h | 1 +
 4 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c
index 1d4e8a6b0067..968635981086 100644
--- a/tools/libs/light/libxl_cpuid.c
+++ b/tools/libs/light/libxl_cpuid.c
@@ -242,6 +242,7 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str)
         {"intel-psfd",   0x00000007,  2, CPUID_REG_EDX,  0,  1},
         {"ipred-ctrl",   0x00000007,  2, CPUID_REG_EDX,  1,  1},
         {"rrsba-ctrl",   0x00000007,  2, CPUID_REG_EDX,  2,  1},
+        {"ddp-ctrl",     0x00000007,  2, CPUID_REG_EDX,  3,  1},
         {"bhi-ctrl",     0x00000007,  2, CPUID_REG_EDX,  4,  1},
         {"mcdt-no",      0x00000007,  2, CPUID_REG_EDX,  5,  1},
 
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 4f4261f4aa95..868054ab96a6 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -216,7 +216,7 @@ static const char *const str_7d1[32] =
 static const char *const str_7d2[32] =
 {
     [ 0] = "intel-psfd",    [ 1] = "ipred-ctrl",
-    [ 2] = "rrsba-ctrl",
+    [ 2] = "rrsba-ctrl",    [ 3] = "ddp-ctrl",
     [ 4] = "bhi-ctrl",      [ 5] = "mcdt-no",
 };
 
diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 7615d8087f46..fa771ed0b50f 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -41,6 +41,7 @@
 #define  SPEC_CTRL_RRSBA_DIS_U              (_AC(1, ULL) <<  5)
 #define  SPEC_CTRL_RRSBA_DIS_S              (_AC(1, ULL) <<  6)
 #define  SPEC_CTRL_PSFD                     (_AC(1, ULL) <<  7)
+#define  SPEC_CTRL_DDP_DIS_U                (_AC(1, ULL) <<  8)
 #define  SPEC_CTRL_BHI_DIS_S                (_AC(1, ULL) << 10)
 
 #define MSR_PRED_CMD                        0x00000049
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 336744b47119..08885190876a 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -294,6 +294,7 @@ XEN_CPUFEATURE(INTEL_PPIN,         12*32+ 0) /*   Protected Processor Inventory
 XEN_CPUFEATURE(INTEL_PSFD,         13*32+ 0) /*A  MSR_SPEC_CTRL.PSFD */
 XEN_CPUFEATURE(IPRED_CTRL,         13*32+ 1) /*   MSR_SPEC_CTRL.IPRED_DIS_* */
 XEN_CPUFEATURE(RRSBA_CTRL,         13*32+ 2) /*   MSR_SPEC_CTRL.RRSBA_DIS_* */
+XEN_CPUFEATURE(DDP_CTRL,           13*32+ 3) /*   MSR_SPEC_CTRL.DDP_DIS_U */
 XEN_CPUFEATURE(BHI_CTRL,           13*32+ 4) /*   MSR_SPEC_CTRL.BHI_DIS_S */
 XEN_CPUFEATURE(MCDT_NO,            13*32+ 5) /*A  MCDT_NO */
 
-- 
2.30.2