The AMD_* bits have to be set from the vendor-independent
feature and bug flags, because KVM_GET_SUPPORTED_CPUID does not care
about the vendor and they should be set on Intel processors as well.
On top of this, SSBD, STIBP and AMD_SSB_NO bit were not set, and
VIRT_SSBD does not have to be added manually because it is a
cpufeature that comes directly from the host's CPUID bit.
Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/x86/kvm/cpuid.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 22c2720cd948..145ec050d45d 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -730,15 +730,20 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
entry->eax = g_phys_as | (virt_as << 8);
entry->edx = 0;
/*
- * IBRS, IBPB and VIRT_SSBD aren't necessarily present in
- * hardware cpuid
+ * 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.
*/
- if (boot_cpu_has(X86_FEATURE_AMD_IBPB))
+ if (boot_cpu_has(X86_FEATURE_IBPB))
entry->ebx |= F(AMD_IBPB);
- if (boot_cpu_has(X86_FEATURE_AMD_IBRS))
+ if (boot_cpu_has(X86_FEATURE_IBRS))
entry->ebx |= F(AMD_IBRS);
- if (boot_cpu_has(X86_FEATURE_VIRT_SSBD))
- entry->ebx |= F(VIRT_SSBD);
+ if (boot_cpu_has(X86_FEATURE_STIBP))
+ entry->ebx |= F(AMD_STIBP);
+ if (boot_cpu_has(X86_FEATURE_SSBD))
+ entry->ebx |= F(AMD_SSBD);
+ if (!boot_cpu_has_bug(X86_BUG_SPEC_STORE_BYPASS))
+ entry->ebx |= F(AMD_SSB_NO);
entry->ebx &= kvm_cpuid_8000_0008_ebx_x86_features;
cpuid_mask(&entry->ebx, CPUID_8000_0008_EBX);
/*
--
1.8.3.1
On Thu, Aug 15, 2019 at 12:41 AM Paolo Bonzini <[email protected]> wrote:
>
> The AMD_* bits have to be set from the vendor-independent
> feature and bug flags, because KVM_GET_SUPPORTED_CPUID does not care
> about the vendor and they should be set on Intel processors as well.
> On top of this, SSBD, STIBP and AMD_SSB_NO bit were not set, and
> VIRT_SSBD does not have to be added manually because it is a
> cpufeature that comes directly from the host's CPUID bit.
>
> Signed-off-by: Paolo Bonzini <[email protected]>
On AMD systems, aren't AMD_SSBD, AMD_STIBP, and AMD_SSB_NO set by
inheritance from the host:
/* cpuid 0x80000008.ebx */
const u32 kvm_cpuid_8000_0008_ebx_x86_features =
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);
I am curious why the cross-vendor settings go only one way. For
example, you set AMD_STIBP on Intel processors that have STIBP, but
you do not set INTEL_STIBP on AMD processors that have STIBP?
Similarly, you set AMD_SSB_NO for Intel processors that are immune to
SSB, but you do not set IA32_ARCH_CAPABILITIES.SSB_NO for AMD
processors that are immune to SSB?
Perhaps there is another patch coming for reporting Intel bits on AMD?
On 16/08/19 23:45, Jim Mattson wrote:
> On Thu, Aug 15, 2019 at 12:41 AM Paolo Bonzini <[email protected]> wrote:
>>
>> The AMD_* bits have to be set from the vendor-independent
>> feature and bug flags, because KVM_GET_SUPPORTED_CPUID does not care
>> about the vendor and they should be set on Intel processors as well.
>> On top of this, SSBD, STIBP and AMD_SSB_NO bit were not set, and
>> VIRT_SSBD does not have to be added manually because it is a
>> cpufeature that comes directly from the host's CPUID bit.
>>
>> Signed-off-by: Paolo Bonzini <[email protected]>
>
> On AMD systems, aren't AMD_SSBD, AMD_STIBP, and AMD_SSB_NO set by
> inheritance from the host:
>
> /* cpuid 0x80000008.ebx */
> const u32 kvm_cpuid_8000_0008_ebx_x86_features =
> 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);
>
> I am curious why the cross-vendor settings go only one way. For
> example, you set AMD_STIBP on Intel processors that have STIBP, but
> you do not set INTEL_STIBP on AMD processors that have STIBP?
> Similarly, you set AMD_SSB_NO for Intel processors that are immune to
> SSB, but you do not set IA32_ARCH_CAPABILITIES.SSB_NO for AMD
> processors that are immune to SSB?
>
> Perhaps there is another patch coming for reporting Intel bits on AMD?
I wasn't going to work on it but yes, they should be. This patch just
fixed what was half-implemented.
Paolo
On Mon, Aug 19, 2019 at 8:18 AM Paolo Bonzini <[email protected]> wrote:
>
> On 16/08/19 23:45, Jim Mattson wrote:
> > On Thu, Aug 15, 2019 at 12:41 AM Paolo Bonzini <[email protected]> wrote:
> >>
> >> The AMD_* bits have to be set from the vendor-independent
> >> feature and bug flags, because KVM_GET_SUPPORTED_CPUID does not care
> >> about the vendor and they should be set on Intel processors as well.
> >> On top of this, SSBD, STIBP and AMD_SSB_NO bit were not set, and
> >> VIRT_SSBD does not have to be added manually because it is a
> >> cpufeature that comes directly from the host's CPUID bit.
> >>
> >> Signed-off-by: Paolo Bonzini <[email protected]>
> >
> > On AMD systems, aren't AMD_SSBD, AMD_STIBP, and AMD_SSB_NO set by
> > inheritance from the host:
> >
> > /* cpuid 0x80000008.ebx */
> > const u32 kvm_cpuid_8000_0008_ebx_x86_features =
> > 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);
> >
> > I am curious why the cross-vendor settings go only one way. For
> > example, you set AMD_STIBP on Intel processors that have STIBP, but
> > you do not set INTEL_STIBP on AMD processors that have STIBP?
> > Similarly, you set AMD_SSB_NO for Intel processors that are immune to
> > SSB, but you do not set IA32_ARCH_CAPABILITIES.SSB_NO for AMD
> > processors that are immune to SSB?
> >
> > Perhaps there is another patch coming for reporting Intel bits on AMD?
>
> I wasn't going to work on it but yes, they should be. This patch just
> fixed what was half-implemented.
I'm not sure that the original intent was to enumerate the AMD
features on Intel hosts, but it seems reasonable to do so.
Should we also populate the AMD cache topology leaf (0x8000001d) on
Intel hosts? And so on? :-)
Reviewed-by: Jim Mattson <[email protected]>
On 19/08/19 20:30, Jim Mattson wrote:
>>> Perhaps there is another patch coming for reporting Intel bits on AMD?
>> I wasn't going to work on it but yes, they should be. This patch just
>> fixed what was half-implemented.
> I'm not sure that the original intent was to enumerate the AMD
> features on Intel hosts, but it seems reasonable to do so.
>
> Should we also populate the AMD cache topology leaf (0x8000001d) on
> Intel hosts? And so on? :-)
>
> Reviewed-by: Jim Mattson <[email protected]>
Thanks. Note that I plan to send v2 tomorrow, and I've also done the
part that reports Intel bits unconditionally.
Paolo