| Message ID | 20240318181332.3817631-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/features: More AMD features | expand |
On 18.03.2024 19:13, Andrew Cooper wrote: > I'm not sure about FSRSC as a name, but it definitely beats AMD's longhand > name of FAST_REP_SCASB. With FSRS already used, I guess that's the closest we can get to keep names for similar features similar. > --- a/tools/misc/xen-cpuid.c > +++ b/tools/misc/xen-cpuid.c > @@ -91,6 +91,7 @@ static const char *const str_e1c[32] = > [24] = "perfctr-nb", /* [25] */ > [26] = "dbx", [27] = "perftsc", > [28] = "pcx-l2i", [29] = "monitorx", > + [30] = "dbext2", > }; AMD calls this AddrMaskExt (PM) or AdMskExtn (PPR). I can see where your different name choice is coming from, yet I still wonder whether we should try to stay closer where possible. Jan
On 19.03.2024 08:33, Jan Beulich wrote: > On 18.03.2024 19:13, Andrew Cooper wrote: >> I'm not sure about FSRSC as a name, but it definitely beats AMD's longhand >> name of FAST_REP_SCASB. > > With FSRS already used, I guess that's the closest we can get to keep > names for similar features similar. > >> --- a/tools/misc/xen-cpuid.c >> +++ b/tools/misc/xen-cpuid.c >> @@ -91,6 +91,7 @@ static const char *const str_e1c[32] = >> [24] = "perfctr-nb", /* [25] */ >> [26] = "dbx", [27] = "perftsc", >> [28] = "pcx-l2i", [29] = "monitorx", >> + [30] = "dbext2", >> }; > > AMD calls this AddrMaskExt (PM) or AdMskExtn (PPR). I can see where your > different name choice is coming from, yet I still wonder whether we should > try to stay closer where possible. Having located the corresponding doc, Reviewed-by: Jan Beulich <jbeulich@suse.com> with a slight preference to adjusted names for this one feature. Jan
On 19/03/2024 1:11 pm, Jan Beulich wrote: > On 19.03.2024 08:33, Jan Beulich wrote: >> On 18.03.2024 19:13, Andrew Cooper wrote: >>> I'm not sure about FSRSC as a name, but it definitely beats AMD's longhand >>> name of FAST_REP_SCASB. >> With FSRS already used, I guess that's the closest we can get to keep >> names for similar features similar. >> >>> --- a/tools/misc/xen-cpuid.c >>> +++ b/tools/misc/xen-cpuid.c >>> @@ -91,6 +91,7 @@ static const char *const str_e1c[32] = >>> [24] = "perfctr-nb", /* [25] */ >>> [26] = "dbx", [27] = "perftsc", >>> [28] = "pcx-l2i", [29] = "monitorx", >>> + [30] = "dbext2", >>> }; >> AMD calls this AddrMaskExt (PM) or AdMskExtn (PPR). I can see where your >> different name choice is coming from, yet I still wonder whether we should >> try to stay closer where possible. > Having located the corresponding doc, > Reviewed-by: Jan Beulich <jbeulich@suse.com> > with a slight preference to adjusted names for this one feature. Neither are great. How about "addr-msk-ext" ? ~Andrew
On 18/03/2024 6:13 pm, Andrew Cooper wrote: > All of these are informational and require no further logic changes in Xen to > support. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > > I'm not sure about FSRSC as a name, but it definitely beats AMD's longhand > name of FAST_REP_SCASB. > --- > tools/misc/xen-cpuid.c | 5 +++++ > xen/include/public/arch-x86/cpufeatureset.h | 8 ++++++++ > 2 files changed, 13 insertions(+) > > diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c > index 51efbff579e6..b562ee839d38 100644 > --- a/tools/misc/xen-cpuid.c > +++ b/tools/misc/xen-cpuid.c > @@ -91,6 +91,7 @@ static const char *const str_e1c[32] = > [24] = "perfctr-nb", /* [25] */ > [26] = "dbx", [27] = "perftsc", > [28] = "pcx-l2i", [29] = "monitorx", > + [30] = "dbext2", > }; > > static const char *const str_7b0[32] = > @@ -199,11 +200,15 @@ static const char *const str_7a1[32] = > > static const char *const str_e21a[32] = > { > + [ 0] = "no-nest-bp", [ 1] = "fs-gs-ns", > [ 2] = "lfence+", > [ 6] = "nscb", > [ 8] = "auto-ibrs", > + [10] = "amd-fsrs", [11] = "amd-fsrc", > > /* 16 */ [17] = "cpuid-user-dis", > + [18] = "epsf", [19] = "fsrsc", It occurs to me that I need this hunk too. diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py index 25d329ce486f..bf3f9ec01e6e 100755 --- a/xen/tools/gen-cpuid.py +++ b/xen/tools/gen-cpuid.py @@ -329,6 +329,10 @@ def crunch_numbers(state): # In principle the TSXLDTRK insns could also be considered independent. RTM: [TSXLDTRK], + # Enhanced Predictive Store-Forwarding is a informational note on top + # of PSF. + PSFD: [EPSF], + # The ARCH_CAPS CPUID bit enumerates the availability of the whole register. ARCH_CAPS: list(range(RDCL_NO, RDCL_NO + 64)), To cause EPSF to disappear properly when levelling. ~Andrew
On 19.03.2024 18:14, Andrew Cooper wrote: > On 19/03/2024 1:11 pm, Jan Beulich wrote: >> On 19.03.2024 08:33, Jan Beulich wrote: >>> On 18.03.2024 19:13, Andrew Cooper wrote: >>>> I'm not sure about FSRSC as a name, but it definitely beats AMD's longhand >>>> name of FAST_REP_SCASB. >>> With FSRS already used, I guess that's the closest we can get to keep >>> names for similar features similar. >>> >>>> --- a/tools/misc/xen-cpuid.c >>>> +++ b/tools/misc/xen-cpuid.c >>>> @@ -91,6 +91,7 @@ static const char *const str_e1c[32] = >>>> [24] = "perfctr-nb", /* [25] */ >>>> [26] = "dbx", [27] = "perftsc", >>>> [28] = "pcx-l2i", [29] = "monitorx", >>>> + [30] = "dbext2", >>>> }; >>> AMD calls this AddrMaskExt (PM) or AdMskExtn (PPR). I can see where your >>> different name choice is coming from, yet I still wonder whether we should >>> try to stay closer where possible. >> Having located the corresponding doc, >> Reviewed-by: Jan Beulich <jbeulich@suse.com> >> with a slight preference to adjusted names for this one feature. > > Neither are great. How about "addr-msk-ext" ? I'd be okay with that. Jan
On 19.03.2024 18:40, Andrew Cooper wrote: > It occurs to me that I need this hunk too. > > diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py > index 25d329ce486f..bf3f9ec01e6e 100755 > --- a/xen/tools/gen-cpuid.py > +++ b/xen/tools/gen-cpuid.py > @@ -329,6 +329,10 @@ def crunch_numbers(state): > # In principle the TSXLDTRK insns could also be considered > independent. > RTM: [TSXLDTRK], > > + # Enhanced Predictive Store-Forwarding is a informational note > on top > + # of PSF. > + PSFD: [EPSF], > + > # The ARCH_CAPS CPUID bit enumerates the availability of the > whole register. > ARCH_CAPS: list(range(RDCL_NO, RDCL_NO + 64)), > > > To cause EPSF to disappear properly when levelling. What exactly is wrong with exposing EPSF when PSFD is not there? (The PPR I'm looking at has no mention of what exactly the bit means, and hence whether e.g. it indicates PSFD can be avoided in certain use cases.) When leveling across a pool, EPSF may need hiding, yes, but that would need to be a result of admin activity, not by introducing a fake dependency. Just consider a pool with PSFD supported everywhere, but not EPSF: The admin would then still need to take action to make sure EPSF is uniformly invisible to guests. Jan
On 20/03/2024 8:12 am, Jan Beulich wrote: > On 19.03.2024 18:40, Andrew Cooper wrote: >> It occurs to me that I need this hunk too. >> >> diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py >> index 25d329ce486f..bf3f9ec01e6e 100755 >> --- a/xen/tools/gen-cpuid.py >> +++ b/xen/tools/gen-cpuid.py >> @@ -329,6 +329,10 @@ def crunch_numbers(state): >> # In principle the TSXLDTRK insns could also be considered >> independent. >> RTM: [TSXLDTRK], >> >> + # Enhanced Predictive Store-Forwarding is a informational note >> on top >> + # of PSF. >> + PSFD: [EPSF], >> + >> # The ARCH_CAPS CPUID bit enumerates the availability of the >> whole register. >> ARCH_CAPS: list(range(RDCL_NO, RDCL_NO + 64)), >> >> >> To cause EPSF to disappear properly when levelling. > What exactly is wrong with exposing EPSF when PSFD is not there? https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/white-papers/security-analysis-of-amd-predictive-store-forwarding.pdf Final sentence before the conclusion: "Software can determine the presence of EPSF through CPUID Fn8000_0021 EAX[18]. All processors that support EPSF will also support PSFD." i.e. you'll never see anything about PSF without having the controls to turn it off. ~Andrew
On 20.03.2024 12:45, Andrew Cooper wrote: > On 20/03/2024 8:12 am, Jan Beulich wrote: >> On 19.03.2024 18:40, Andrew Cooper wrote: >>> It occurs to me that I need this hunk too. >>> >>> diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py >>> index 25d329ce486f..bf3f9ec01e6e 100755 >>> --- a/xen/tools/gen-cpuid.py >>> +++ b/xen/tools/gen-cpuid.py >>> @@ -329,6 +329,10 @@ def crunch_numbers(state): >>> # In principle the TSXLDTRK insns could also be considered >>> independent. >>> RTM: [TSXLDTRK], >>> >>> + # Enhanced Predictive Store-Forwarding is a informational note >>> on top >>> + # of PSF. >>> + PSFD: [EPSF], >>> + >>> # The ARCH_CAPS CPUID bit enumerates the availability of the >>> whole register. >>> ARCH_CAPS: list(range(RDCL_NO, RDCL_NO + 64)), >>> >>> >>> To cause EPSF to disappear properly when levelling. >> What exactly is wrong with exposing EPSF when PSFD is not there? > > https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/white-papers/security-analysis-of-amd-predictive-store-forwarding.pdf > > Final sentence before the conclusion: > > "Software can determine the presence of EPSF through CPUID Fn8000_0021 > EAX[18]. All processors that support EPSF will also support PSFD." > > i.e. you'll never see anything about PSF without having the controls to > turn it off. Ah, I only had an old copy of that. My R-b holds then with that addition. Jan
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c index 51efbff579e6..b562ee839d38 100644 --- a/tools/misc/xen-cpuid.c +++ b/tools/misc/xen-cpuid.c @@ -91,6 +91,7 @@ static const char *const str_e1c[32] = [24] = "perfctr-nb", /* [25] */ [26] = "dbx", [27] = "perftsc", [28] = "pcx-l2i", [29] = "monitorx", + [30] = "dbext2", }; static const char *const str_7b0[32] = @@ -199,11 +200,15 @@ static const char *const str_7a1[32] = static const char *const str_e21a[32] = { + [ 0] = "no-nest-bp", [ 1] = "fs-gs-ns", [ 2] = "lfence+", [ 6] = "nscb", [ 8] = "auto-ibrs", + [10] = "amd-fsrs", [11] = "amd-fsrc", /* 16 */ [17] = "cpuid-user-dis", + [18] = "epsf", [19] = "fsrsc", + [20] = "amd-prefetchi", /* 26 */ [27] = "sbpb", [28] = "ibpb-brtype", [29] = "srso-no", diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h index eb9f552948be..11287aaabe43 100644 --- a/xen/include/public/arch-x86/cpufeatureset.h +++ b/xen/include/public/arch-x86/cpufeatureset.h @@ -168,6 +168,7 @@ XEN_CPUFEATURE(TBM, 3*32+21) /*A trailing bit manipulations */ XEN_CPUFEATURE(TOPOEXT, 3*32+22) /* topology extensions CPUID leafs */ XEN_CPUFEATURE(DBEXT, 3*32+26) /*A data breakpoint extension */ XEN_CPUFEATURE(MONITORX, 3*32+29) /* MONITOR extension (MONITORX/MWAITX) */ +XEN_CPUFEATURE(DBEXT2, 3*32+30) /*A Address Mask Extentions */ /* Intel-defined CPU features, CPUID level 0x0000000D:1.eax, word 4 */ XEN_CPUFEATURE(XSAVEOPT, 4*32+ 0) /*A XSAVEOPT instruction */ @@ -290,10 +291,17 @@ XEN_CPUFEATURE(WRMSRNS, 10*32+19) /*S WRMSR Non-Serialising */ XEN_CPUFEATURE(AVX_IFMA, 10*32+23) /*A AVX-IFMA Instructions */ /* AMD-defined CPU features, CPUID level 0x80000021.eax, word 11 */ +XEN_CPUFEATURE(NO_NEST_BP, 11*32+ 0) /*A No Nested Data Breakpoints */ +XEN_CPUFEATURE(FS_GS_NS, 11*32+ 1) /*S FS/GS base MSRs non-serialising */ XEN_CPUFEATURE(LFENCE_DISPATCH, 11*32+ 2) /*A LFENCE always serializing */ XEN_CPUFEATURE(NSCB, 11*32+ 6) /*A Null Selector Clears Base (and limit too) */ XEN_CPUFEATURE(AUTO_IBRS, 11*32+ 8) /*S Automatic IBRS */ +XEN_CPUFEATURE(AMD_FSRS, 11*32+10) /*A Fast Short REP STOSB */ +XEN_CPUFEATURE(AMD_FSRC, 11*32+11) /*A Fast Short REP CMPSB */ XEN_CPUFEATURE(CPUID_USER_DIS, 11*32+17) /* CPUID disable for CPL > 0 software */ +XEN_CPUFEATURE(EPSF, 11*32+18) /*A Enhanced Predictive Store Forwarding */ +XEN_CPUFEATURE(FSRSC, 11*32+19) /*A Fast Short REP SCASB */ +XEN_CPUFEATURE(AMD_PREFETCHI, 11*32+20) /*A PREFETCHIT{0,1} Instructions */ XEN_CPUFEATURE(SBPB, 11*32+27) /*A Selective Branch Predictor Barrier */ XEN_CPUFEATURE(IBPB_BRTYPE, 11*32+28) /*A IBPB flushes Branch Type predictions too */ XEN_CPUFEATURE(SRSO_NO, 11*32+29) /*A Hardware not vulenrable to Speculative Return Stack Overflow */
All of these are informational and require no further logic changes in Xen to support. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> I'm not sure about FSRSC as a name, but it definitely beats AMD's longhand name of FAST_REP_SCASB. --- tools/misc/xen-cpuid.c | 5 +++++ xen/include/public/arch-x86/cpufeatureset.h | 8 ++++++++ 2 files changed, 13 insertions(+) base-commit: 62018f08708a5ff6ef8fc8ff2aaaac46e5a60430