| Message ID | 20230509164336.12523-2-alejandro.vallejo@cloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add CpuidUserDis support | expand |
On 09.05.2023 18:43, Alejandro Vallejo wrote: > --- a/xen/include/public/arch-x86/cpufeatureset.h > +++ b/xen/include/public/arch-x86/cpufeatureset.h > @@ -287,6 +287,7 @@ XEN_CPUFEATURE(AVX_IFMA, 10*32+23) /*A AVX-IFMA Instructions */ > /* AMD-defined CPU features, CPUID level 0x80000021.eax, word 11 */ > 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(CPUID_USER_DIS, 11*32+17) /* CPUID disable for non-privileged software */ While I can accept your argument towards staying close to AMD's doc with the name, I'd really like to ask that the comment then be disambiguated: "non-privileged" is more likely mean CPL=3 than all of CPL>0. Since "not fully privileged" is getting a little long, maybe indeed say "CPL > 0 software"? I would then offer you my R-b, if only I could find proof of the HWCR bit being bit 35. The PM mentions it only by name, and the PPRs I've checked all have it marked reserved. Jan
On Thu, May 11, 2023 at 11:41:13AM +0200, Jan Beulich wrote: > On 09.05.2023 18:43, Alejandro Vallejo wrote: > > --- a/xen/include/public/arch-x86/cpufeatureset.h > > +++ b/xen/include/public/arch-x86/cpufeatureset.h > > @@ -287,6 +287,7 @@ XEN_CPUFEATURE(AVX_IFMA, 10*32+23) /*A AVX-IFMA Instructions */ > > /* AMD-defined CPU features, CPUID level 0x80000021.eax, word 11 */ > > 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(CPUID_USER_DIS, 11*32+17) /* CPUID disable for non-privileged software */ > > While I can accept your argument towards staying close to AMD's doc > with the name, I'd really like to ask that the comment then be > disambiguated: "non-privileged" is more likely mean CPL=3 than all > of CPL>0. Since "not fully privileged" is getting a little long, > maybe indeed say "CPL > 0 software"? [...] Fair point. That was copied from AMD's PM, but there's no good reason to keep it that way. I'll modify it as you pointed out. > I would then offer you my R-b, > if only I could find proof of the HWCR bit being bit 35. The PM > mentions it only by name, and the PPRs I've checked all have it > marked reserved. > > Jan It is in the Vol2 of the PM. Section 3.2.10 on the HWCR. I'm looking at revision 4.06, from January 2023. --- CpuidUserDis. Bit 35. Setting this bit to 1 causes #GP(0) when the CPUID instruction is executed by non-privileged software (CPL > 0) outside SMM. Support for the CPUID User Disable feature is indicated by CPUID Fn80000021_EAX[CpuidUserDis]=1. --- Alejandro
On 11.05.2023 12:38, Alejandro Vallejo wrote: > On Thu, May 11, 2023 at 11:41:13AM +0200, Jan Beulich wrote: >> On 09.05.2023 18:43, Alejandro Vallejo wrote: >>> --- a/xen/include/public/arch-x86/cpufeatureset.h >>> +++ b/xen/include/public/arch-x86/cpufeatureset.h >>> @@ -287,6 +287,7 @@ XEN_CPUFEATURE(AVX_IFMA, 10*32+23) /*A AVX-IFMA Instructions */ >>> /* AMD-defined CPU features, CPUID level 0x80000021.eax, word 11 */ >>> 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(CPUID_USER_DIS, 11*32+17) /* CPUID disable for non-privileged software */ >> >> While I can accept your argument towards staying close to AMD's doc >> with the name, I'd really like to ask that the comment then be >> disambiguated: "non-privileged" is more likely mean CPL=3 than all >> of CPL>0. Since "not fully privileged" is getting a little long, >> maybe indeed say "CPL > 0 software"? [...] > > Fair point. That was copied from AMD's PM, but there's no good reason to > keep it that way. I'll modify it as you pointed out. > >> I would then offer you my R-b, >> if only I could find proof of the HWCR bit being bit 35. The PM >> mentions it only by name, and the PPRs I've checked all have it >> marked reserved. > > It is in the Vol2 of the PM. Section 3.2.10 on the HWCR. I'm looking at > revision 4.06, from January 2023. Oh, my fault then: It didn't even occur to me to check Vol 2, as normally it's the other way around: Only the PPRs can be sufficiently relied upon to be at least halfway complete. With the comment adjustment (which I'd also be okay to do while committing) then Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c index 5f0bf93810..4d2fab5414 100644 --- a/tools/libs/light/libxl_cpuid.c +++ b/tools/libs/light/libxl_cpuid.c @@ -317,6 +317,7 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str) {"lfence+", 0x80000021, NA, CPUID_REG_EAX, 2, 1}, {"nscb", 0x80000021, NA, CPUID_REG_EAX, 6, 1}, + {"cpuid-user-dis", 0x80000021, NA, CPUID_REG_EAX, 17, 1}, {"maxhvleaf", 0x40000000, NA, CPUID_REG_EAX, 0, 8}, diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c index d7efc59d31..8ec143ebc8 100644 --- a/tools/misc/xen-cpuid.c +++ b/tools/misc/xen-cpuid.c @@ -199,6 +199,8 @@ static const char *const str_e21a[32] = { [ 2] = "lfence+", [ 6] = "nscb", + + /* 16 */ [17] = "cpuid-user-dis", }; static const char *const str_7b1[32] = diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h index fa771ed0b5..082fb2e0d9 100644 --- a/xen/arch/x86/include/asm/msr-index.h +++ b/xen/arch/x86/include/asm/msr-index.h @@ -337,6 +337,7 @@ #define MSR_K8_HWCR 0xc0010015 #define K8_HWCR_TSC_FREQ_SEL (1ULL << 24) +#define K8_HWCR_CPUID_USER_DIS (1ULL << 35) #define MSR_K7_FID_VID_CTL 0xc0010041 #define MSR_K7_FID_VID_STATUS 0xc0010042 diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h index 12e3dc80c6..623dcb1bce 100644 --- a/xen/include/public/arch-x86/cpufeatureset.h +++ b/xen/include/public/arch-x86/cpufeatureset.h @@ -287,6 +287,7 @@ XEN_CPUFEATURE(AVX_IFMA, 10*32+23) /*A AVX-IFMA Instructions */ /* AMD-defined CPU features, CPUID level 0x80000021.eax, word 11 */ 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(CPUID_USER_DIS, 11*32+17) /* CPUID disable for non-privileged software */ /* Intel-defined CPU features, CPUID level 0x00000007:1.ebx, word 12 */ XEN_CPUFEATURE(INTEL_PPIN, 12*32+ 0) /* Protected Processor Inventory Number */
AMD reports support for CpuidUserDis in CPUID and provides the toggle in HWCR. This patch adds the positions of both of those bits to both xen and tools. No functional change. Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> --- 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, 5 insertions(+)