| Message ID | 20220830225210.2381310-1-jmattson@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] KVM: x86: Insert "AMD" in KVM_X86_FEATURE_PSFD | expand |
On Tue, Aug 30, 2022, Jim Mattson wrote: > Intel and AMD have separate CPUID bits for each SPEC_CTRL bit. In the > case of every bit other than PFSD, the Intel CPUID bit has no vendor > name qualifier, but the AMD CPUID bit does. For consistency, rename > KVM_X86_FEATURE_PSFD to KVM_X86_FEATURE_AMD_PSFD. > > No functional change intended. > > Signed-off-by: Jim Mattson <jmattson@google.com> > Cc: Babu Moger <Babu.Moger@amd.com> > --- > v1 -> v2: Dropped patch 2/3. > > arch/x86/kvm/cpuid.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 75dcf7a72605..07be45c5bb93 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -62,7 +62,7 @@ u32 xstate_required_size(u64 xstate_bv, bool compacted) > * This one is tied to SSB in the user API, and not > * visible in /proc/cpuinfo. > */ > -#define KVM_X86_FEATURE_PSFD (13*32+28) /* Predictive Store Forwarding Disable */ > +#define KVM_X86_FEATURE_AMD_PSFD (13*32+28) /* Predictive Store Forwarding Disable */ This is asinine. If KVM is forced to carry the feature bit then IMO we have every right to the "real" name. If we can't convince others that this belongs in cpufeatures.h, then I vote to rename this to X86_FEATURE_AMD_PSFD so that we don't have to special case this thing. > #define F feature_bit > #define SF(name) (boot_cpu_has(X86_FEATURE_##name) ? F(name) : 0) > @@ -673,7 +673,7 @@ void kvm_set_cpu_caps(void) > F(CLZERO) | F(XSAVEERPTR) | > F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) | > F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON) | > - __feature_bit(KVM_X86_FEATURE_PSFD) > + __feature_bit(KVM_X86_FEATURE_AMD_PSFD) > ); > > /* > -- > 2.37.2.672.g94769d06f0-goog >
On Tue, Aug 30, 2022 at 4:27 PM Sean Christopherson <seanjc@google.com> wrote: > > On Tue, Aug 30, 2022, Jim Mattson wrote: > > Intel and AMD have separate CPUID bits for each SPEC_CTRL bit. In the > > case of every bit other than PFSD, the Intel CPUID bit has no vendor > > name qualifier, but the AMD CPUID bit does. For consistency, rename > > KVM_X86_FEATURE_PSFD to KVM_X86_FEATURE_AMD_PSFD. > > > > No functional change intended. > > > > Signed-off-by: Jim Mattson <jmattson@google.com> > > Cc: Babu Moger <Babu.Moger@amd.com> > > --- > > v1 -> v2: Dropped patch 2/3. > > > > arch/x86/kvm/cpuid.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > > index 75dcf7a72605..07be45c5bb93 100644 > > --- a/arch/x86/kvm/cpuid.c > > +++ b/arch/x86/kvm/cpuid.c > > @@ -62,7 +62,7 @@ u32 xstate_required_size(u64 xstate_bv, bool compacted) > > * This one is tied to SSB in the user API, and not > > * visible in /proc/cpuinfo. > > */ > > -#define KVM_X86_FEATURE_PSFD (13*32+28) /* Predictive Store Forwarding Disable */ > > +#define KVM_X86_FEATURE_AMD_PSFD (13*32+28) /* Predictive Store Forwarding Disable */ > > This is asinine. If KVM is forced to carry the feature bit then IMO we have every > right to the "real" name. If we can't convince others that this belongs in > cpufeatures.h, then I vote to rename this to X86_FEATURE_AMD_PSFD so that we don't > have to special case this thing. You won't get any argument from me! If Borislav objects to seeing the feature in /proc/cpuinfo, can't we just begin the cpufeatures.h descriptive comment with ""?
On 8/31/22 01:32, Jim Mattson wrote: >> This is asinine. If KVM is forced to carry the feature bit then IMO we have every >> right to the "real" name. If we can't convince others that this belongs in >> cpufeatures.h, then I vote to rename this to X86_FEATURE_AMD_PSFD so that we don't >> have to special case this thing. > You won't get any argument from me! > > If Borislav objects to seeing the feature in /proc/cpuinfo, can't we > just begin the cpufeatures.h descriptive comment with ""? Yes, since this is just part of word 13 it should just use "". It won't help for the Intel one which uses CPUID[EAX=7,ECX=2] though. Paolo
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 75dcf7a72605..07be45c5bb93 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -62,7 +62,7 @@ u32 xstate_required_size(u64 xstate_bv, bool compacted) * This one is tied to SSB in the user API, and not * visible in /proc/cpuinfo. */ -#define KVM_X86_FEATURE_PSFD (13*32+28) /* Predictive Store Forwarding Disable */ +#define KVM_X86_FEATURE_AMD_PSFD (13*32+28) /* Predictive Store Forwarding Disable */ #define F feature_bit #define SF(name) (boot_cpu_has(X86_FEATURE_##name) ? F(name) : 0) @@ -673,7 +673,7 @@ void kvm_set_cpu_caps(void) F(CLZERO) | F(XSAVEERPTR) | F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) | F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON) | - __feature_bit(KVM_X86_FEATURE_PSFD) + __feature_bit(KVM_X86_FEATURE_AMD_PSFD) ); /*
Intel and AMD have separate CPUID bits for each SPEC_CTRL bit. In the case of every bit other than PFSD, the Intel CPUID bit has no vendor name qualifier, but the AMD CPUID bit does. For consistency, rename KVM_X86_FEATURE_PSFD to KVM_X86_FEATURE_AMD_PSFD. No functional change intended. Signed-off-by: Jim Mattson <jmattson@google.com> Cc: Babu Moger <Babu.Moger@amd.com> --- v1 -> v2: Dropped patch 2/3. arch/x86/kvm/cpuid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)