* [PATCH v2 1/2] KVM: x86: Insert "AMD" in KVM_X86_FEATURE_PSFD @ 2022-08-30 22:52 Jim Mattson 2022-08-30 22:52 ` [PATCH v2 2/2] KVM: x86: Expose Predictive Store Forwarding Disable on Intel parts Jim Mattson 2022-08-30 23:27 ` [PATCH v2 1/2] KVM: x86: Insert "AMD" in KVM_X86_FEATURE_PSFD Sean Christopherson 0 siblings, 2 replies; 8+ messages in thread From: Jim Mattson @ 2022-08-30 22:52 UTC (permalink / raw) To: kvm, pbonzini; +Cc: Jim Mattson, Babu Moger 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 */ #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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] KVM: x86: Expose Predictive Store Forwarding Disable on Intel parts 2022-08-30 22:52 [PATCH v2 1/2] KVM: x86: Insert "AMD" in KVM_X86_FEATURE_PSFD Jim Mattson @ 2022-08-30 22:52 ` Jim Mattson 2022-08-30 23:38 ` Sean Christopherson 2022-08-30 23:27 ` [PATCH v2 1/2] KVM: x86: Insert "AMD" in KVM_X86_FEATURE_PSFD Sean Christopherson 1 sibling, 1 reply; 8+ messages in thread From: Jim Mattson @ 2022-08-30 22:52 UTC (permalink / raw) To: kvm, pbonzini; +Cc: Jim Mattson Intel enumerates support for PSFD in CPUID.(EAX=7,ECX=2):EDX.PSFD[bit 0]. Report support for this feature in KVM if it is available on the host. Presumably, this feature bit, like its AMD counterpart, is not welcome in cpufeatures.h, so add a local definition of this feature in KVM. Signed-off-by: Jim Mattson <jmattson@google.com> --- arch/x86/kvm/cpuid.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 07be45c5bb93..b5af9e451bef 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -62,6 +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 0 /* Predictive Store Forwarding Disable */ #define KVM_X86_FEATURE_AMD_PSFD (13*32+28) /* Predictive Store Forwarding Disable */ #define F feature_bit @@ -677,9 +678,9 @@ void kvm_set_cpu_caps(void) ); /* - * AMD has separate bits for each SPEC_CTRL bit. - * arch/x86/kernel/cpu/bugs.c is kind enough to - * record that in cpufeatures so use them. + * AMD has separate bits for each SPEC_CTRL bit. Except for + * PSFD, arch/x86/kernel/cpu/bugs.c is kind enough to record + * that in cpufeatures so use them. */ if (boot_cpu_has(X86_FEATURE_IBPB)) kvm_cpu_cap_set(X86_FEATURE_AMD_IBPB); @@ -880,13 +881,13 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) break; /* function 7 has additional index. */ case 7: - entry->eax = min(entry->eax, 1u); + /* KVM only supports leaf 7 indices 0 through 2. */ + max_idx = entry->eax = min(entry->eax, 2u); cpuid_entry_override(entry, CPUID_7_0_EBX); cpuid_entry_override(entry, CPUID_7_ECX); cpuid_entry_override(entry, CPUID_7_EDX); - /* KVM only supports 0x7.0 and 0x7.1, capped above via min(). */ - if (entry->eax == 1) { + if (max_idx >= 1) { entry = do_host_cpuid(array, function, 1); if (!entry) goto out; @@ -896,6 +897,16 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) entry->ecx = 0; entry->edx = 0; } + if (max_idx >= 2) { + entry = do_host_cpuid(array, function, 2); + if (!entry) + goto out; + + entry->eax = 0; + entry->ebx = 0; + entry->ecx = 0; + entry->edx &= BIT(KVM_X86_FEATURE_PSFD); + } break; case 9: break; -- 2.37.2.672.g94769d06f0-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] KVM: x86: Expose Predictive Store Forwarding Disable on Intel parts 2022-08-30 22:52 ` [PATCH v2 2/2] KVM: x86: Expose Predictive Store Forwarding Disable on Intel parts Jim Mattson @ 2022-08-30 23:38 ` Sean Christopherson 2022-08-30 23:46 ` Jim Mattson 0 siblings, 1 reply; 8+ messages in thread From: Sean Christopherson @ 2022-08-30 23:38 UTC (permalink / raw) To: Jim Mattson; +Cc: kvm, pbonzini On Tue, Aug 30, 2022, Jim Mattson wrote: > Intel enumerates support for PSFD in CPUID.(EAX=7,ECX=2):EDX.PSFD[bit > 0]. Report support for this feature in KVM if it is available on the > host. > > Presumably, this feature bit, like its AMD counterpart, is not welcome > in cpufeatures.h, so add a local definition of this feature in KVM. > > Signed-off-by: Jim Mattson <jmattson@google.com> > --- > arch/x86/kvm/cpuid.c | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 07be45c5bb93..b5af9e451bef 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -62,6 +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 0 /* Predictive Store Forwarding Disable */ I believe we can use "enum kvm_only_cpuid_leafs" to handle this. E.g. enum kvm_only_cpuid_leafs { CPUID_12_EAX = NCAPINTS, CPUID_7_2_EDX, NR_KVM_CPU_CAPS, NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS, }; then the intended use of KVM_X86_FEATURE_* #define KVM_X86_FEATURE_PSFD KVM_X86_FEATURE(CPUID_7_2_EDX, 0) I _think_ we can then define an arbitrary word for X86_FEATURE_PSFD, e.g. #define X86_FEATURE_PSFD (NKVMCAPINTS*32+0) and then wire up the translation: static __always_inline u32 __feature_translate(int x86_feature) { if (x86_feature == X86_FEATURE_SGX1) return KVM_X86_FEATURE_SGX1; else if (x86_feature == X86_FEATURE_SGX2) return KVM_X86_FEATURE_SGX2; else if (x86_feature == X86_FEATURE_PSFD) return KVM_X86_FEATURE_PSFD; return x86_feature; } I believe/hope that allows us to use at least cpuid_entry_override(). Open coding masking of specific registers was a mess that I don't want to repeat. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] KVM: x86: Expose Predictive Store Forwarding Disable on Intel parts 2022-08-30 23:38 ` Sean Christopherson @ 2022-08-30 23:46 ` Jim Mattson 2022-10-19 14:42 ` Sean Christopherson 0 siblings, 1 reply; 8+ messages in thread From: Jim Mattson @ 2022-08-30 23:46 UTC (permalink / raw) To: Sean Christopherson; +Cc: kvm, pbonzini On Tue, Aug 30, 2022 at 4:39 PM Sean Christopherson <seanjc@google.com> wrote: > > On Tue, Aug 30, 2022, Jim Mattson wrote: > > Intel enumerates support for PSFD in CPUID.(EAX=7,ECX=2):EDX.PSFD[bit > > 0]. Report support for this feature in KVM if it is available on the > > host. > > > > Presumably, this feature bit, like its AMD counterpart, is not welcome > > in cpufeatures.h, so add a local definition of this feature in KVM. > > > > Signed-off-by: Jim Mattson <jmattson@google.com> > > --- > > arch/x86/kvm/cpuid.c | 23 +++++++++++++++++------ > > 1 file changed, 17 insertions(+), 6 deletions(-) > > > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > > index 07be45c5bb93..b5af9e451bef 100644 > > --- a/arch/x86/kvm/cpuid.c > > +++ b/arch/x86/kvm/cpuid.c > > @@ -62,6 +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 0 /* Predictive Store Forwarding Disable */ > > I believe we can use "enum kvm_only_cpuid_leafs" to handle this. E.g. > > enum kvm_only_cpuid_leafs { > CPUID_12_EAX = NCAPINTS, > CPUID_7_2_EDX, > NR_KVM_CPU_CAPS, > > NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS, > }; > > then the intended use of KVM_X86_FEATURE_* > > #define KVM_X86_FEATURE_PSFD KVM_X86_FEATURE(CPUID_7_2_EDX, 0) > > I _think_ we can then define an arbitrary word for X86_FEATURE_PSFD, e.g. > > #define X86_FEATURE_PSFD (NKVMCAPINTS*32+0) We may run afoul of reverse_cpuid_check(), depending on usage. > and then wire up the translation: > > static __always_inline u32 __feature_translate(int x86_feature) > { > if (x86_feature == X86_FEATURE_SGX1) > return KVM_X86_FEATURE_SGX1; > else if (x86_feature == X86_FEATURE_SGX2) > return KVM_X86_FEATURE_SGX2; > else if (x86_feature == X86_FEATURE_PSFD) > return KVM_X86_FEATURE_PSFD; > > return x86_feature; > } > > I believe/hope that allows us to use at least cpuid_entry_override(). Open coding > masking of specific registers was a mess that I don't want to repeat. Maybe we can plead for a bit in word 11? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] KVM: x86: Expose Predictive Store Forwarding Disable on Intel parts 2022-08-30 23:46 ` Jim Mattson @ 2022-10-19 14:42 ` Sean Christopherson 0 siblings, 0 replies; 8+ messages in thread From: Sean Christopherson @ 2022-10-19 14:42 UTC (permalink / raw) To: Jim Mattson; +Cc: kvm, pbonzini On Tue, Aug 30, 2022, Jim Mattson wrote: > On Tue, Aug 30, 2022 at 4:39 PM Sean Christopherson <seanjc@google.com> wrote: > > > > On Tue, Aug 30, 2022, Jim Mattson wrote: > > > Intel enumerates support for PSFD in CPUID.(EAX=7,ECX=2):EDX.PSFD[bit > > > 0]. Report support for this feature in KVM if it is available on the > > > host. > > > > > > Presumably, this feature bit, like its AMD counterpart, is not welcome > > > in cpufeatures.h, so add a local definition of this feature in KVM. > > > > > > Signed-off-by: Jim Mattson <jmattson@google.com> > > > --- > > > arch/x86/kvm/cpuid.c | 23 +++++++++++++++++------ > > > 1 file changed, 17 insertions(+), 6 deletions(-) > > > > > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > > > index 07be45c5bb93..b5af9e451bef 100644 > > > --- a/arch/x86/kvm/cpuid.c > > > +++ b/arch/x86/kvm/cpuid.c > > > @@ -62,6 +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 0 /* Predictive Store Forwarding Disable */ > > > > I believe we can use "enum kvm_only_cpuid_leafs" to handle this. E.g. > > > > enum kvm_only_cpuid_leafs { > > CPUID_12_EAX = NCAPINTS, > > CPUID_7_2_EDX, > > NR_KVM_CPU_CAPS, > > > > NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS, > > }; > > > > then the intended use of KVM_X86_FEATURE_* > > > > #define KVM_X86_FEATURE_PSFD KVM_X86_FEATURE(CPUID_7_2_EDX, 0) > > > > I _think_ we can then define an arbitrary word for X86_FEATURE_PSFD, e.g. > > > > #define X86_FEATURE_PSFD (NKVMCAPINTS*32+0) > > We may run afoul of reverse_cpuid_check(), depending on usage. Oh, yeah, an entry in reverse_cpuid[] would also be needed. For posterity since PSFD doesn't need a KVM-only entry[*]... [*] https://lore.kernel.org/all/Y0CrER%2FyiHheWiZw@google.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] KVM: x86: Insert "AMD" in KVM_X86_FEATURE_PSFD 2022-08-30 22:52 [PATCH v2 1/2] KVM: x86: Insert "AMD" in KVM_X86_FEATURE_PSFD Jim Mattson 2022-08-30 22:52 ` [PATCH v2 2/2] KVM: x86: Expose Predictive Store Forwarding Disable on Intel parts Jim Mattson @ 2022-08-30 23:27 ` Sean Christopherson 2022-08-30 23:32 ` Jim Mattson 1 sibling, 1 reply; 8+ messages in thread From: Sean Christopherson @ 2022-08-30 23:27 UTC (permalink / raw) To: Jim Mattson; +Cc: kvm, pbonzini, Babu Moger 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] KVM: x86: Insert "AMD" in KVM_X86_FEATURE_PSFD 2022-08-30 23:27 ` [PATCH v2 1/2] KVM: x86: Insert "AMD" in KVM_X86_FEATURE_PSFD Sean Christopherson @ 2022-08-30 23:32 ` Jim Mattson 2022-10-22 9:12 ` Paolo Bonzini 0 siblings, 1 reply; 8+ messages in thread From: Jim Mattson @ 2022-08-30 23:32 UTC (permalink / raw) To: Sean Christopherson; +Cc: kvm, pbonzini, Babu Moger 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 ""? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] KVM: x86: Insert "AMD" in KVM_X86_FEATURE_PSFD 2022-08-30 23:32 ` Jim Mattson @ 2022-10-22 9:12 ` Paolo Bonzini 0 siblings, 0 replies; 8+ messages in thread From: Paolo Bonzini @ 2022-10-22 9:12 UTC (permalink / raw) To: Jim Mattson, Sean Christopherson; +Cc: kvm, Babu Moger 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-10-22 10:23 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-30 22:52 [PATCH v2 1/2] KVM: x86: Insert "AMD" in KVM_X86_FEATURE_PSFD Jim Mattson 2022-08-30 22:52 ` [PATCH v2 2/2] KVM: x86: Expose Predictive Store Forwarding Disable on Intel parts Jim Mattson 2022-08-30 23:38 ` Sean Christopherson 2022-08-30 23:46 ` Jim Mattson 2022-10-19 14:42 ` Sean Christopherson 2022-08-30 23:27 ` [PATCH v2 1/2] KVM: x86: Insert "AMD" in KVM_X86_FEATURE_PSFD Sean Christopherson 2022-08-30 23:32 ` Jim Mattson 2022-10-22 9:12 ` Paolo Bonzini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.