diff mbox series

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

Message ID 20230310160238.1321765-3-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series x86: Misc enumeration improvements | expand

Commit Message

Andrew Cooper March 10, 2023, 4:02 p.m. UTC
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(-)

Comments

Jan Beulich March 13, 2023, 9:21 a.m. UTC | #1
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
Jan Beulich March 13, 2023, 9:24 a.m. UTC | #2
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
Andrew Cooper March 13, 2023, 11:12 a.m. UTC | #3
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
diff mbox series

Patch

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