KVM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] KVM: x86: fixes for AMD speculation bug CPUID leaf
@ 2019-08-15  7:41 Paolo Bonzini
  2019-08-15  7:41 ` [PATCH 1/2] KVM: x86: fix reporting of " Paolo Bonzini
  2019-08-15  7:41 ` [PATCH 2/2] KVM: x86: always expose VIRT_SSBD to guests Paolo Bonzini
  0 siblings, 2 replies; 9+ messages in thread
From: Paolo Bonzini @ 2019-08-15  7:41 UTC (permalink / raw)
  To: linux-kernel, kvm

Patch 1 fixes the reporting of bugs and mitigations via the
0x8000_0008 CPUID leaf on Intel processors.

Patch 2 fixes the reporting of VIRT_SPEC availability on
AMD processors.

Paolo

Paolo Bonzini (2):
  KVM: x86: fix reporting of AMD speculation bug CPUID leaf
  KVM: x86: always expose VIRT_SSBD to guests

 arch/x86/kvm/cpuid.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] KVM: x86: fix reporting of AMD speculation bug CPUID leaf
  2019-08-15  7:41 [PATCH 0/2] KVM: x86: fixes for AMD speculation bug CPUID leaf Paolo Bonzini
@ 2019-08-15  7:41 ` " Paolo Bonzini
  2019-08-16 21:45   ` Jim Mattson
  2019-08-15  7:41 ` [PATCH 2/2] KVM: x86: always expose VIRT_SSBD to guests Paolo Bonzini
  1 sibling, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2019-08-15  7:41 UTC (permalink / raw)
  To: linux-kernel, kvm

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 <pbonzini@redhat.com>
---
 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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/2] KVM: x86: always expose VIRT_SSBD to guests
  2019-08-15  7:41 [PATCH 0/2] KVM: x86: fixes for AMD speculation bug CPUID leaf Paolo Bonzini
  2019-08-15  7:41 ` [PATCH 1/2] KVM: x86: fix reporting of " Paolo Bonzini
@ 2019-08-15  7:41 ` Paolo Bonzini
  2019-08-15 17:14   ` Eduardo Habkost
  1 sibling, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2019-08-15  7:41 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Konrad Rzeszutek Wilk

Even though it is preferrable to use SPEC_CTRL (represented by
X86_FEATURE_AMD_SSBD) instead of VIRT_SPEC, VIRT_SPEC is always
supported anyway because otherwise it would be impossible to
migrate from old to new CPUs.  Make this apparent in the
result of KVM_GET_SUPPORTED_CPUID as well.

Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reported-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/cpuid.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 145ec050d45d..5865bc73bbb5 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -747,11 +747,13 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
 		entry->ebx &= kvm_cpuid_8000_0008_ebx_x86_features;
 		cpuid_mask(&entry->ebx, CPUID_8000_0008_EBX);
 		/*
-		 * The preference is to use SPEC CTRL MSR instead of the
-		 * VIRT_SPEC MSR.
+		 * VIRT_SPEC is only implemented for AMD processors,
+		 * but the host could set AMD_SSBD if it wanted even
+		 * for Intel processors.
 		 */
-		if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) &&
-		    !boot_cpu_has(X86_FEATURE_AMD_SSBD))
+		if ((boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) ||
+		     boot_cpu_has(X86_FEATURE_AMD_SSBD)) &&
+		    boot_cpu_has(X86_FEATURE_SVM))
 			entry->ebx |= F(VIRT_SSBD);
 		break;
 	}
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] KVM: x86: always expose VIRT_SSBD to guests
  2019-08-15  7:41 ` [PATCH 2/2] KVM: x86: always expose VIRT_SSBD to guests Paolo Bonzini
@ 2019-08-15 17:14   ` Eduardo Habkost
  2019-08-19 15:41     ` Paolo Bonzini
  0 siblings, 1 reply; 9+ messages in thread
From: Eduardo Habkost @ 2019-08-15 17:14 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, Konrad Rzeszutek Wilk

On Thu, Aug 15, 2019 at 09:41:23AM +0200, Paolo Bonzini wrote:
> Even though it is preferrable to use SPEC_CTRL (represented by
> X86_FEATURE_AMD_SSBD) instead of VIRT_SPEC, VIRT_SPEC is always
> supported anyway because otherwise it would be impossible to
> migrate from old to new CPUs.  Make this apparent in the
> result of KVM_GET_SUPPORTED_CPUID as well.
> 
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Reported-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/cpuid.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 145ec050d45d..5865bc73bbb5 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -747,11 +747,13 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
>  		entry->ebx &= kvm_cpuid_8000_0008_ebx_x86_features;
>  		cpuid_mask(&entry->ebx, CPUID_8000_0008_EBX);
>  		/*
> -		 * The preference is to use SPEC CTRL MSR instead of the
> -		 * VIRT_SPEC MSR.
> +		 * VIRT_SPEC is only implemented for AMD processors,
> +		 * but the host could set AMD_SSBD if it wanted even
> +		 * for Intel processors.
>  		 */
> -		if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) &&
> -		    !boot_cpu_has(X86_FEATURE_AMD_SSBD))
> +		if ((boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) ||
> +		     boot_cpu_has(X86_FEATURE_AMD_SSBD)) &&
> +		    boot_cpu_has(X86_FEATURE_SVM))

Would it be desirable to move this code to
svm_set_supported_cpuid(), or is there a reason for keeping this
in cpuid.c?


>  			entry->ebx |= F(VIRT_SSBD);
>  		break;
>  	}
> -- 
> 1.8.3.1
> 

-- 
Eduardo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] KVM: x86: fix reporting of AMD speculation bug CPUID leaf
  2019-08-15  7:41 ` [PATCH 1/2] KVM: x86: fix reporting of " Paolo Bonzini
@ 2019-08-16 21:45   ` Jim Mattson
  2019-08-19 15:18     ` Paolo Bonzini
  0 siblings, 1 reply; 9+ messages in thread
From: Jim Mattson @ 2019-08-16 21:45 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: LKML, kvm list

On Thu, Aug 15, 2019 at 12:41 AM Paolo Bonzini <pbonzini@redhat.com> 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 <pbonzini@redhat.com>

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?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] KVM: x86: fix reporting of AMD speculation bug CPUID leaf
  2019-08-16 21:45   ` Jim Mattson
@ 2019-08-19 15:18     ` Paolo Bonzini
  2019-08-19 18:30       ` Jim Mattson
  0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2019-08-19 15:18 UTC (permalink / raw)
  To: Jim Mattson; +Cc: LKML, kvm list

On 16/08/19 23:45, Jim Mattson wrote:
> On Thu, Aug 15, 2019 at 12:41 AM Paolo Bonzini <pbonzini@redhat.com> 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 <pbonzini@redhat.com>
> 
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] KVM: x86: always expose VIRT_SSBD to guests
  2019-08-15 17:14   ` Eduardo Habkost
@ 2019-08-19 15:41     ` Paolo Bonzini
  0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2019-08-19 15:41 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: linux-kernel, kvm, Konrad Rzeszutek Wilk

On 15/08/19 19:14, Eduardo Habkost wrote:
> On Thu, Aug 15, 2019 at 09:41:23AM +0200, Paolo Bonzini wrote:
>> Even though it is preferrable to use SPEC_CTRL (represented by
>> X86_FEATURE_AMD_SSBD) instead of VIRT_SPEC, VIRT_SPEC is always
>> supported anyway because otherwise it would be impossible to
>> migrate from old to new CPUs.  Make this apparent in the
>> result of KVM_GET_SUPPORTED_CPUID as well.
>>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Reported-by: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  arch/x86/kvm/cpuid.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>> index 145ec050d45d..5865bc73bbb5 100644
>> --- a/arch/x86/kvm/cpuid.c
>> +++ b/arch/x86/kvm/cpuid.c
>> @@ -747,11 +747,13 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
>>  		entry->ebx &= kvm_cpuid_8000_0008_ebx_x86_features;
>>  		cpuid_mask(&entry->ebx, CPUID_8000_0008_EBX);
>>  		/*
>> -		 * The preference is to use SPEC CTRL MSR instead of the
>> -		 * VIRT_SPEC MSR.
>> +		 * VIRT_SPEC is only implemented for AMD processors,
>> +		 * but the host could set AMD_SSBD if it wanted even
>> +		 * for Intel processors.
>>  		 */
>> -		if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) &&
>> -		    !boot_cpu_has(X86_FEATURE_AMD_SSBD))
>> +		if ((boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) ||
>> +		     boot_cpu_has(X86_FEATURE_AMD_SSBD)) &&
>> +		    boot_cpu_has(X86_FEATURE_SVM))
> 
> Would it be desirable to move this code to
> svm_set_supported_cpuid(), or is there a reason for keeping this
> in cpuid.c?

Yes, of course.  Forgot about it.

Paolo


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] KVM: x86: fix reporting of AMD speculation bug CPUID leaf
  2019-08-19 15:18     ` Paolo Bonzini
@ 2019-08-19 18:30       ` Jim Mattson
  2019-08-19 18:33         ` Paolo Bonzini
  0 siblings, 1 reply; 9+ messages in thread
From: Jim Mattson @ 2019-08-19 18:30 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: LKML, kvm list

On Mon, Aug 19, 2019 at 8:18 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 16/08/19 23:45, Jim Mattson wrote:
> > On Thu, Aug 15, 2019 at 12:41 AM Paolo Bonzini <pbonzini@redhat.com> 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 <pbonzini@redhat.com>
> >
> > 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 <jmattson@google.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] KVM: x86: fix reporting of AMD speculation bug CPUID leaf
  2019-08-19 18:30       ` Jim Mattson
@ 2019-08-19 18:33         ` Paolo Bonzini
  0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2019-08-19 18:33 UTC (permalink / raw)
  To: Jim Mattson; +Cc: LKML, kvm list

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 <jmattson@google.com>

Thanks.  Note that I plan to send v2 tomorrow, and I've also done the
part that reports Intel bits unconditionally.

Paolo

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-15  7:41 [PATCH 0/2] KVM: x86: fixes for AMD speculation bug CPUID leaf Paolo Bonzini
2019-08-15  7:41 ` [PATCH 1/2] KVM: x86: fix reporting of " Paolo Bonzini
2019-08-16 21:45   ` Jim Mattson
2019-08-19 15:18     ` Paolo Bonzini
2019-08-19 18:30       ` Jim Mattson
2019-08-19 18:33         ` Paolo Bonzini
2019-08-15  7:41 ` [PATCH 2/2] KVM: x86: always expose VIRT_SSBD to guests Paolo Bonzini
2019-08-15 17:14   ` Eduardo Habkost
2019-08-19 15:41     ` Paolo Bonzini

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org kvm@archiver.kernel.org
	public-inbox-index kvm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox