kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] KVM: EFER.LMSLE cleanup
@ 2022-09-20 20:59 Jim Mattson
  2022-09-20 20:59 ` [PATCH v2 1/3] Revert "KVM: SVM: Allow EFER.LMSLE to be set with nested svm" Jim Mattson
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Jim Mattson @ 2022-09-20 20:59 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin, Sean Christopherson, Paolo Bonzini,
	linux-kernel, kvm
  Cc: Jim Mattson

KVM has never properly virtualized EFER.LMSLE. However, when the
"nested" module parameter is set, KVM lets the guest set EFER.LMSLE.
Ostensibly, this is so that SLES11 Xen 4.0 will boot as a nested
hypervisor.

KVM passes EFER.LMSLE to the hardware through the VMCB, so
the setting works most of the time, but the KVM instruction emulator
completely ignores the bit, so incorrect guest behavior is almost
certainly assured.

With Zen3, AMD has abandoned EFER.LMSLE. KVM still allows it, though, as
long as "nested" is set. However, since the hardware doesn't support it,
the next VMRUN after the emulated WRMSR will fail with "invalid VMCB."

To clean things up, revert the hack that allowed a KVM guest to set
EFER.LMSLE, and enumerate CPUID.80000008H:EDX.EferLmsleUnsupported[bit
20] in KVM_GET_SUPPORTED_CPUID on SVM hosts.

Jim Mattson (3):
  Revert "KVM: SVM: Allow EFER.LMSLE to be set with nested svm"
  x86/cpufeatures: Introduce X86_FEATURE_NO_LMSLE
  KVM: SVM: Unconditionally enumerate EferLmsleUnsupported

 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/include/asm/msr-index.h   | 2 --
 arch/x86/kvm/svm/svm.c             | 3 ++-
 3 files changed, 3 insertions(+), 3 deletions(-)

v1 -> v2: Make no attempt to preserve existing behavior [Sean, Borislav]

-- 
2.37.3.968.ga6b4b080e4-goog


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

* [PATCH v2 1/3] Revert "KVM: SVM: Allow EFER.LMSLE to be set with nested svm"
  2022-09-20 20:59 [PATCH v2 0/3] KVM: EFER.LMSLE cleanup Jim Mattson
@ 2022-09-20 20:59 ` Jim Mattson
  2022-09-20 20:59 ` [PATCH v2 2/3] x86/cpufeatures: Introduce X86_FEATURE_NO_LMSLE Jim Mattson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Jim Mattson @ 2022-09-20 20:59 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin, Sean Christopherson, Paolo Bonzini,
	linux-kernel, kvm
  Cc: Jim Mattson

Revert the hack that allowed a guest to set EFER.LMSLE, since no
attempt was ever made to properly virtualize the feature.

Now that AMD has deprecated the feature, the ROI for properly
virtualizing it is vanishingly small.

This reverts commit eec4b140c924b4c650e9a89e01d223266490e325.

Fixes: eec4b140c924 ("KVM: SVM: Allow EFER.LMSLE to be set with nested svm")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/include/asm/msr-index.h | 2 --
 arch/x86/kvm/svm/svm.c           | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 6674bdb096f3..0a0426f284a3 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -28,7 +28,6 @@
 #define _EFER_LMA		10 /* Long mode active (read-only) */
 #define _EFER_NX		11 /* No execute enable */
 #define _EFER_SVME		12 /* Enable virtualization */
-#define _EFER_LMSLE		13 /* Long Mode Segment Limit Enable */
 #define _EFER_FFXSR		14 /* Enable Fast FXSAVE/FXRSTOR */
 
 #define EFER_SCE		(1<<_EFER_SCE)
@@ -36,7 +35,6 @@
 #define EFER_LMA		(1<<_EFER_LMA)
 #define EFER_NX			(1<<_EFER_NX)
 #define EFER_SVME		(1<<_EFER_SVME)
-#define EFER_LMSLE		(1<<_EFER_LMSLE)
 #define EFER_FFXSR		(1<<_EFER_FFXSR)
 
 /* Intel MSRs. Some also available on other CPUs */
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index f3813dbacb9f..3af360fe21e6 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5012,7 +5012,7 @@ static __init int svm_hardware_setup(void)
 
 	if (nested) {
 		printk(KERN_INFO "kvm: Nested Virtualization enabled\n");
-		kvm_enable_efer_bits(EFER_SVME | EFER_LMSLE);
+		kvm_enable_efer_bits(EFER_SVME);
 	}
 
 	/*
-- 
2.37.3.968.ga6b4b080e4-goog


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

* [PATCH v2 2/3] x86/cpufeatures: Introduce X86_FEATURE_NO_LMSLE
  2022-09-20 20:59 [PATCH v2 0/3] KVM: EFER.LMSLE cleanup Jim Mattson
  2022-09-20 20:59 ` [PATCH v2 1/3] Revert "KVM: SVM: Allow EFER.LMSLE to be set with nested svm" Jim Mattson
@ 2022-09-20 20:59 ` Jim Mattson
  2022-09-21 16:07   ` Borislav Petkov
  2022-09-20 20:59 ` [PATCH v2 3/3] KVM: SVM: Unconditionally enumerate EferLmsleUnsupported Jim Mattson
  2022-09-20 21:17 ` [PATCH v2 0/3] KVM: EFER.LMSLE cleanup Borislav Petkov
  3 siblings, 1 reply; 17+ messages in thread
From: Jim Mattson @ 2022-09-20 20:59 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin, Sean Christopherson, Paolo Bonzini,
	linux-kernel, kvm
  Cc: Jim Mattson

When AMD introduced "Long Mode Segment Limit Enable" (a.k.a. "VMware
mode"), the feature was not enumerated by a CPUID bit. Now that VMware
has abandoned binary translation, AMD has deprecated EFER.LMSLE.

The absence of the feature *is* now enumerated by a CPUID bit (a la
Intel's X86_FEATURE_ZERO_FCS_DCS and X86_FEATURE_FDP_EXCPTN_ONLY).

This defeature bit is already present in feature word 13, but it was
previously anonymous. Name it X86_FEATURE_NO_LMSLE, so that KVM can
reference it when constructing a supported guest CPUID table.

Since this bit indicates the absence of a feature, don't enumerate it
in /proc/cpuinfo.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index ef4775c6db01..0f5a3285d8d8 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -319,6 +319,7 @@
 #define X86_FEATURE_AMD_IBRS		(13*32+14) /* "" Indirect Branch Restricted Speculation */
 #define X86_FEATURE_AMD_STIBP		(13*32+15) /* "" Single Thread Indirect Branch Predictors */
 #define X86_FEATURE_AMD_STIBP_ALWAYS_ON	(13*32+17) /* "" Single Thread Indirect Branch Predictors always-on preferred */
+#define X86_FEATURE_NO_LMSLE		(13*32+20) /* "" EFER_LMSLE is unsupported */
 #define X86_FEATURE_AMD_PPIN		(13*32+23) /* Protected Processor Inventory Number */
 #define X86_FEATURE_AMD_SSBD		(13*32+24) /* "" Speculative Store Bypass Disable */
 #define X86_FEATURE_VIRT_SSBD		(13*32+25) /* Virtualized Speculative Store Bypass Disable */
-- 
2.37.3.968.ga6b4b080e4-goog


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

* [PATCH v2 3/3] KVM: SVM: Unconditionally enumerate EferLmsleUnsupported
  2022-09-20 20:59 [PATCH v2 0/3] KVM: EFER.LMSLE cleanup Jim Mattson
  2022-09-20 20:59 ` [PATCH v2 1/3] Revert "KVM: SVM: Allow EFER.LMSLE to be set with nested svm" Jim Mattson
  2022-09-20 20:59 ` [PATCH v2 2/3] x86/cpufeatures: Introduce X86_FEATURE_NO_LMSLE Jim Mattson
@ 2022-09-20 20:59 ` Jim Mattson
  2022-10-07 22:41   ` Sean Christopherson
  2022-09-20 21:17 ` [PATCH v2 0/3] KVM: EFER.LMSLE cleanup Borislav Petkov
  3 siblings, 1 reply; 17+ messages in thread
From: Jim Mattson @ 2022-09-20 20:59 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin, Sean Christopherson, Paolo Bonzini,
	linux-kernel, kvm
  Cc: Jim Mattson

CPUID.80000008H:EDX.EferLmsleUnsupported[bit 20] indicates that
IA32_EFER.LMSLE[bit 13] is unsupported and must be zero.

KVM doesn't support "Long Mode Segment Limit Enable," even if the
underlying physical processor does, so set that bit in the guest CPUID
table returned by KVM_GET_SUPPORTED_CPUID.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/svm/svm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 3af360fe21e6..0bf6ac51f097 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4945,6 +4945,7 @@ static __init void svm_set_cpu_caps(void)
 	if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) ||
 	    boot_cpu_has(X86_FEATURE_AMD_SSBD))
 		kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD);
+	kvm_cpu_cap_set(X86_FEATURE_NO_LMSLE);
 
 	/* AMD PMU PERFCTR_CORE CPUID */
 	if (enable_pmu && boot_cpu_has(X86_FEATURE_PERFCTR_CORE))
-- 
2.37.3.968.ga6b4b080e4-goog


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

* Re: [PATCH v2 0/3] KVM: EFER.LMSLE cleanup
  2022-09-20 20:59 [PATCH v2 0/3] KVM: EFER.LMSLE cleanup Jim Mattson
                   ` (2 preceding siblings ...)
  2022-09-20 20:59 ` [PATCH v2 3/3] KVM: SVM: Unconditionally enumerate EferLmsleUnsupported Jim Mattson
@ 2022-09-20 21:17 ` Borislav Petkov
  2022-09-20 21:36   ` Sean Christopherson
  2022-09-20 21:36   ` Jim Mattson
  3 siblings, 2 replies; 17+ messages in thread
From: Borislav Petkov @ 2022-09-20 21:17 UTC (permalink / raw)
  To: Jim Mattson
  Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, x86, H . Peter Anvin,
	Sean Christopherson, Paolo Bonzini, linux-kernel, kvm

On Tue, Sep 20, 2022 at 01:59:19PM -0700, Jim Mattson wrote:
> Jim Mattson (3):
>   Revert "KVM: SVM: Allow EFER.LMSLE to be set with nested svm"
>   x86/cpufeatures: Introduce X86_FEATURE_NO_LMSLE
>   KVM: SVM: Unconditionally enumerate EferLmsleUnsupported

Why do you need those two if you revert the hack? After the revert,
anything that tries to set LMSLE should get a #GP anyway, no?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH v2 0/3] KVM: EFER.LMSLE cleanup
  2022-09-20 21:17 ` [PATCH v2 0/3] KVM: EFER.LMSLE cleanup Borislav Petkov
@ 2022-09-20 21:36   ` Sean Christopherson
  2022-09-20 21:36   ` Jim Mattson
  1 sibling, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2022-09-20 21:36 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Jim Mattson, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	H . Peter Anvin, Paolo Bonzini, linux-kernel, kvm

On Tue, Sep 20, 2022, Borislav Petkov wrote:
> On Tue, Sep 20, 2022 at 01:59:19PM -0700, Jim Mattson wrote:
> > Jim Mattson (3):
> >   Revert "KVM: SVM: Allow EFER.LMSLE to be set with nested svm"
> >   x86/cpufeatures: Introduce X86_FEATURE_NO_LMSLE
> >   KVM: SVM: Unconditionally enumerate EferLmsleUnsupported
> 
> Why do you need those two if you revert the hack? After the revert,
> anything that tries to set LMSLE should get a #GP anyway, no?

Yes, but ideally KVM would explicitly tell the guest "you don't have LMSLE".
Probably a moot point, but at the same time I don't see a reason not to be
explicit.

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

* Re: [PATCH v2 0/3] KVM: EFER.LMSLE cleanup
  2022-09-20 21:17 ` [PATCH v2 0/3] KVM: EFER.LMSLE cleanup Borislav Petkov
  2022-09-20 21:36   ` Sean Christopherson
@ 2022-09-20 21:36   ` Jim Mattson
  2022-09-21  9:28     ` Borislav Petkov
  1 sibling, 1 reply; 17+ messages in thread
From: Jim Mattson @ 2022-09-20 21:36 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, x86, H . Peter Anvin,
	Sean Christopherson, Paolo Bonzini, linux-kernel, kvm

On Tue, Sep 20, 2022 at 2:17 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Tue, Sep 20, 2022 at 01:59:19PM -0700, Jim Mattson wrote:
> > Jim Mattson (3):
> >   Revert "KVM: SVM: Allow EFER.LMSLE to be set with nested svm"
> >   x86/cpufeatures: Introduce X86_FEATURE_NO_LMSLE
> >   KVM: SVM: Unconditionally enumerate EferLmsleUnsupported
>
> Why do you need those two if you revert the hack? After the revert,
> anything that tries to set LMSLE should get a #GP anyway, no?

Reporting that CPUID bit gives us the right to raise #GP. AMD CPUs
(going way back) that don't report EferLmsleUnsupported do not raise
#GP.

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

* Re: [PATCH v2 0/3] KVM: EFER.LMSLE cleanup
  2022-09-20 21:36   ` Jim Mattson
@ 2022-09-21  9:28     ` Borislav Petkov
  2022-09-21 13:45       ` Jim Mattson
  0 siblings, 1 reply; 17+ messages in thread
From: Borislav Petkov @ 2022-09-21  9:28 UTC (permalink / raw)
  To: Jim Mattson, Sean Christopherson
  Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, x86, H . Peter Anvin,
	Paolo Bonzini, linux-kernel, kvm

On Tue, Sep 20, 2022 at 09:36:18PM +0000, Sean Christopherson wrote:
> Yes, but ideally KVM would explicitly tell the guest "you don't have LMSLE".
> Probably a moot point, but at the same time I don't see a reason not to be
> explicit.

Yes but...

On Tue, Sep 20, 2022 at 02:36:34PM -0700, Jim Mattson wrote:
> Reporting that CPUID bit gives us the right to raise #GP. AMD CPUs
> (going way back) that don't report EferLmsleUnsupported do not raise
> #GP.

... what does "gives us the right" mean exactly?

I'm pretty sure I'm missing something about how KVM works but wouldn't
it raise a guest #GP when the guest tries to set an unsupported EFER
bit? I.e., why do you need to explicitly do

	kvm_cpu_cap_set(X86_FEATURE_NO_LMSLE);

and not handle this like any other EFER reserved bit?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH v2 0/3] KVM: EFER.LMSLE cleanup
  2022-09-21  9:28     ` Borislav Petkov
@ 2022-09-21 13:45       ` Jim Mattson
  2022-09-21 13:54         ` Borislav Petkov
  0 siblings, 1 reply; 17+ messages in thread
From: Jim Mattson @ 2022-09-21 13:45 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Sean Christopherson, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	x86, H . Peter Anvin, Paolo Bonzini, linux-kernel, kvm

On Wed, Sep 21, 2022 at 2:28 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Tue, Sep 20, 2022 at 09:36:18PM +0000, Sean Christopherson wrote:
> > Yes, but ideally KVM would explicitly tell the guest "you don't have LMSLE".
> > Probably a moot point, but at the same time I don't see a reason not to be
> > explicit.
>
> Yes but...
>
> On Tue, Sep 20, 2022 at 02:36:34PM -0700, Jim Mattson wrote:
> > Reporting that CPUID bit gives us the right to raise #GP. AMD CPUs
> > (going way back) that don't report EferLmsleUnsupported do not raise
> > #GP.
>
> ... what does "gives us the right" mean exactly?
>
> I'm pretty sure I'm missing something about how KVM works but wouldn't
> it raise a guest #GP when the guest tries to set an unsupported EFER
> bit? I.e., why do you need to explicitly do
>
>         kvm_cpu_cap_set(X86_FEATURE_NO_LMSLE);
>
> and not handle this like any other EFER reserved bit?

EFER.LMLSE is not a reserved bit on AMD64 CPUs, unless
CPUID.80000008:EBX[20] is set (or you're running very, very old
hardware).

We really shouldn't just decide on a whim to treat EFER.LMSLE as
reserved under KVM. The guest CPUID information represents our
detailed contract with the guest software. By setting
CPUID.80000008:EBX[20], we are telling the guest that if it tries to
set EFER.LMSLE, we will raise a #GP.

If we don't set that bit in the guest CPUID information and we raise
#GP on an attempt to set EFER.LMSLE, the virtual hardware is
defective. We could document this behavior as an erratum, but since a
mechanism exists to declare that the guest can expect EFER.LMSLE to
#GP, doesn't it make sense to use it?

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

* Re: [PATCH v2 0/3] KVM: EFER.LMSLE cleanup
  2022-09-21 13:45       ` Jim Mattson
@ 2022-09-21 13:54         ` Borislav Petkov
  2022-09-21 15:11           ` Jim Mattson
  0 siblings, 1 reply; 17+ messages in thread
From: Borislav Petkov @ 2022-09-21 13:54 UTC (permalink / raw)
  To: Jim Mattson
  Cc: Sean Christopherson, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	x86, H . Peter Anvin, Paolo Bonzini, linux-kernel, kvm

On Wed, Sep 21, 2022 at 06:45:24AM -0700, Jim Mattson wrote:
> EFER.LMLSE is not a reserved bit on AMD64 CPUs, unless
> CPUID.80000008:EBX[20] is set (or you're running very, very old
> hardware).
> 
> We really shouldn't just decide on a whim to treat EFER.LMSLE as
> reserved under KVM. The guest CPUID information represents our
> detailed contract with the guest software. By setting
> CPUID.80000008:EBX[20], we are telling the guest that if it tries to
> set EFER.LMSLE, we will raise a #GP.

I understand all that. What I'm asking is, what happens in KVM *after*
your patch 1/3 is applied when a guest tries to set EFER.LMSLE? Does it
#GP or does it allow the WRMSR to succeed? I.e., does KVM check when
reserved bits in that MSR are being set?

By looking at it, there's kvm_enable_efer_bits() so it looks like KVM
does control which bits are allowed to set and which not...?

> If we don't set that bit in the guest CPUID information and we raise
> #GP on an attempt to set EFER.LMSLE, the virtual hardware is
> defective.

See, this is what I don't get - why is it defective? After the revert,
that bit to KVM is reserved.

> We could document this behavior as an erratum, but since a
> mechanism exists to declare that the guest can expect EFER.LMSLE to
> #GP, doesn't it make sense to use it?

I don't mind all that and the X86_FEATURE bit and so on - I'm just
trying to ask you guys: what is KVM's behavior when the guest tries to
set a reserved EFER bit.

Maybe I'm not expressing myself precisely enough...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH v2 0/3] KVM: EFER.LMSLE cleanup
  2022-09-21 13:54         ` Borislav Petkov
@ 2022-09-21 15:11           ` Jim Mattson
  2022-09-21 16:06             ` Borislav Petkov
  0 siblings, 1 reply; 17+ messages in thread
From: Jim Mattson @ 2022-09-21 15:11 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Sean Christopherson, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	x86, H . Peter Anvin, Paolo Bonzini, linux-kernel, kvm

On Wed, Sep 21, 2022 at 6:54 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Sep 21, 2022 at 06:45:24AM -0700, Jim Mattson wrote:
> > EFER.LMLSE is not a reserved bit on AMD64 CPUs, unless
> > CPUID.80000008:EBX[20] is set (or you're running very, very old
> > hardware).
> >
> > We really shouldn't just decide on a whim to treat EFER.LMSLE as
> > reserved under KVM. The guest CPUID information represents our
> > detailed contract with the guest software. By setting
> > CPUID.80000008:EBX[20], we are telling the guest that if it tries to
> > set EFER.LMSLE, we will raise a #GP.
>
> I understand all that. What I'm asking is, what happens in KVM *after*
> your patch 1/3 is applied when a guest tries to set EFER.LMSLE? Does it
> #GP or does it allow the WRMSR to succeed? I.e., does KVM check when
> reserved bits in that MSR are being set?
>
> By looking at it, there's kvm_enable_efer_bits() so it looks like KVM
> does control which bits are allowed to set and which not...?

Yes, after the revert, KVM will treat the bit as reserved, and it will
synthesize a #GP, *in violation of the architectural specification.*
As I said, we could document this behavior as a KVM erratum.

> > If we don't set that bit in the guest CPUID information and we raise
> > #GP on an attempt to set EFER.LMSLE, the virtual hardware is
> > defective.
>
> See, this is what I don't get - why is it defective? After the revert,
> that bit to KVM is reserved.

KVM can't just decide willy nilly to reserve arbitrary bits. If it is
in violation of AMD's architectural specification, the virtual CPU is
defective.

> > We could document this behavior as an erratum, but since a
> > mechanism exists to declare that the guest can expect EFER.LMSLE to
> > #GP, doesn't it make sense to use it?
>
> I don't mind all that and the X86_FEATURE bit and so on - I'm just
> trying to ask you guys: what is KVM's behavior when the guest tries to
> set a reserved EFER bit.
>
> Maybe I'm not expressing myself precisely enough...

I feel the same way. :-(

The two patches after the revert are to amend the contract with the
guest (as expressed by the guest CPUID table) so that the KVM virtual
CPU can raise a #GP on EFER.LMSLE and still conform to the
architectural specification.

From the APM, volume 2, 4.12.2 Data Limit Checks in 64-bit Mode:

> Data segment limit checking in 64-bit mode is not supported by all processor implementations and has been deprecated. If CPUID Fn8000_0008_EBX[EferLmlseUnsupported](bit 20) = 1, 64-bit mode segment limit checking is not supported and attempting to enable this feature by setting EFER.LMSLE =1 will result in a #GP exception.

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

* Re: [PATCH v2 0/3] KVM: EFER.LMSLE cleanup
  2022-09-21 15:11           ` Jim Mattson
@ 2022-09-21 16:06             ` Borislav Petkov
  2022-09-21 16:23               ` Jim Mattson
  0 siblings, 1 reply; 17+ messages in thread
From: Borislav Petkov @ 2022-09-21 16:06 UTC (permalink / raw)
  To: Jim Mattson
  Cc: Sean Christopherson, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	x86, H . Peter Anvin, Paolo Bonzini, linux-kernel, kvm

On Wed, Sep 21, 2022 at 08:11:29AM -0700, Jim Mattson wrote:
> Yes, after the revert, KVM will treat the bit as reserved, and it will
> synthesize a #GP, *in violation of the architectural specification.*

Architectural, schmarchitectural... Intel hasn't implemented it so meh.

> KVM can't just decide willy nilly to reserve arbitrary bits. If it is
> in violation of AMD's architectural specification, the virtual CPU is
> defective.

Grrr, after your revert that this bit was *only* reserved and nothing
else to KVM. Like every other reserved bit in EFER. Yeah, yeah, AMD
specified it as architectural but Intel didn't implement it so there's
this thing on paper and there's reality...

Anyway, enough about this - we're on the same page.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH v2 2/3] x86/cpufeatures: Introduce X86_FEATURE_NO_LMSLE
  2022-09-20 20:59 ` [PATCH v2 2/3] x86/cpufeatures: Introduce X86_FEATURE_NO_LMSLE Jim Mattson
@ 2022-09-21 16:07   ` Borislav Petkov
  0 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2022-09-21 16:07 UTC (permalink / raw)
  To: Jim Mattson
  Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, x86, H . Peter Anvin,
	Sean Christopherson, Paolo Bonzini, linux-kernel, kvm

On Tue, Sep 20, 2022 at 01:59:21PM -0700, Jim Mattson wrote:
> When AMD introduced "Long Mode Segment Limit Enable" (a.k.a. "VMware
> mode"), the feature was not enumerated by a CPUID bit. Now that VMware
> has abandoned binary translation, AMD has deprecated EFER.LMSLE.
> 
> The absence of the feature *is* now enumerated by a CPUID bit (a la
> Intel's X86_FEATURE_ZERO_FCS_DCS and X86_FEATURE_FDP_EXCPTN_ONLY).
> 
> This defeature bit is already present in feature word 13, but it was
> previously anonymous. Name it X86_FEATURE_NO_LMSLE, so that KVM can
> reference it when constructing a supported guest CPUID table.
> 
> Since this bit indicates the absence of a feature, don't enumerate it
> in /proc/cpuinfo.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/include/asm/cpufeatures.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index ef4775c6db01..0f5a3285d8d8 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -319,6 +319,7 @@
>  #define X86_FEATURE_AMD_IBRS		(13*32+14) /* "" Indirect Branch Restricted Speculation */
>  #define X86_FEATURE_AMD_STIBP		(13*32+15) /* "" Single Thread Indirect Branch Predictors */
>  #define X86_FEATURE_AMD_STIBP_ALWAYS_ON	(13*32+17) /* "" Single Thread Indirect Branch Predictors always-on preferred */
> +#define X86_FEATURE_NO_LMSLE		(13*32+20) /* "" EFER_LMSLE is unsupported */
>  #define X86_FEATURE_AMD_PPIN		(13*32+23) /* Protected Processor Inventory Number */
>  #define X86_FEATURE_AMD_SSBD		(13*32+24) /* "" Speculative Store Bypass Disable */
>  #define X86_FEATURE_VIRT_SSBD		(13*32+25) /* Virtualized Speculative Store Bypass Disable */
> -- 

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH v2 0/3] KVM: EFER.LMSLE cleanup
  2022-09-21 16:06             ` Borislav Petkov
@ 2022-09-21 16:23               ` Jim Mattson
  2022-09-21 17:11                 ` Borislav Petkov
  0 siblings, 1 reply; 17+ messages in thread
From: Jim Mattson @ 2022-09-21 16:23 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Sean Christopherson, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	x86, H . Peter Anvin, Paolo Bonzini, linux-kernel, kvm

On Wed, Sep 21, 2022 at 9:06 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Sep 21, 2022 at 08:11:29AM -0700, Jim Mattson wrote:
> > Yes, after the revert, KVM will treat the bit as reserved, and it will
> > synthesize a #GP, *in violation of the architectural specification.*
>
> Architectural, schmarchitectural... Intel hasn't implemented it so meh.
>
> > KVM can't just decide willy nilly to reserve arbitrary bits. If it is
> > in violation of AMD's architectural specification, the virtual CPU is
> > defective.
>
> Grrr, after your revert that this bit was *only* reserved and nothing
> else to KVM. Like every other reserved bit in EFER. Yeah, yeah, AMD
> specified it as architectural but Intel didn't implement it so there's
> this thing on paper and there's reality...

AMD defined the 64-bit x86 extensions while Intel was distracted with
their VLIW science fair project. In this space, Intel produces AMD64
compatible CPUs. The definitive specification comes from AMD (which is
sad, because AMD's documentation is abysmal).

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

* Re: [PATCH v2 0/3] KVM: EFER.LMSLE cleanup
  2022-09-21 16:23               ` Jim Mattson
@ 2022-09-21 17:11                 ` Borislav Petkov
  2022-09-21 17:45                   ` Jim Mattson
  0 siblings, 1 reply; 17+ messages in thread
From: Borislav Petkov @ 2022-09-21 17:11 UTC (permalink / raw)
  To: Jim Mattson
  Cc: Sean Christopherson, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	x86, H . Peter Anvin, Paolo Bonzini, linux-kernel, kvm

On Wed, Sep 21, 2022 at 09:23:40AM -0700, Jim Mattson wrote:
> AMD defined the 64-bit x86 extensions while Intel was distracted with
> their VLIW science fair project. In this space, Intel produces AMD64
> compatible CPUs.

Almost-compatible. And maybe, just maybe, because Intel were probably
and practically forced to implement AMD64 but then thought, oh well,
we'll do some things differently.

> The definitive specification comes from AMD (which is sad, because
> AMD's documentation is abysmal).

Just don't tell me the SDM is better...

But you and I are really talking past each other: there's nothing
definitive about a spec if, while implementing it, the other vendor is
doing some subtle, but very software visible things differently.

I.e., the theory vs reality point I'm trying to get across.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH v2 0/3] KVM: EFER.LMSLE cleanup
  2022-09-21 17:11                 ` Borislav Petkov
@ 2022-09-21 17:45                   ` Jim Mattson
  0 siblings, 0 replies; 17+ messages in thread
From: Jim Mattson @ 2022-09-21 17:45 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Sean Christopherson, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	x86, H . Peter Anvin, Paolo Bonzini, linux-kernel, kvm

On Wed, Sep 21, 2022 at 10:11 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Sep 21, 2022 at 09:23:40AM -0700, Jim Mattson wrote:
> > AMD defined the 64-bit x86 extensions while Intel was distracted with
> > their VLIW science fair project. In this space, Intel produces AMD64
> > compatible CPUs.
>
> Almost-compatible. And maybe, just maybe, because Intel were probably
> and practically forced to implement AMD64 but then thought, oh well,
> we'll do some things differently.
>
> > The definitive specification comes from AMD (which is sad, because
> > AMD's documentation is abysmal).
>
> Just don't tell me the SDM is better...
>
> But you and I are really talking past each other: there's nothing
> definitive about a spec if, while implementing it, the other vendor is
> doing some subtle, but very software visible things differently.
>
> I.e., the theory vs reality point I'm trying to get across.

I get it. In reality, all of the reverse polarity CPUID feature bits
are essentially useless.

The only software that actually uses LMSLE is defunct. That software
predated the definition of CPUID.80000008:EBX.EferLmlseUnsupported and
is no longer being updated, so it isn't going to check the feature
bit. It's just going to fail with an unexpected #GP.

Maybe you think I'm being overly pedantic, but the code to do the
right thing is trivial, so why not do it?

This way, if anyone files a bug against KVM because an old VMware
hypervisor dies with an unexpected #GP, I can point to the spec and
say that it's user error.

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

* Re: [PATCH v2 3/3] KVM: SVM: Unconditionally enumerate EferLmsleUnsupported
  2022-09-20 20:59 ` [PATCH v2 3/3] KVM: SVM: Unconditionally enumerate EferLmsleUnsupported Jim Mattson
@ 2022-10-07 22:41   ` Sean Christopherson
  0 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2022-10-07 22:41 UTC (permalink / raw)
  To: Jim Mattson
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin, Paolo Bonzini, linux-kernel, kvm

On Tue, Sep 20, 2022, Jim Mattson wrote:
> CPUID.80000008H:EDX.EferLmsleUnsupported[bit 20] indicates that
> IA32_EFER.LMSLE[bit 13] is unsupported and must be zero.
> 
> KVM doesn't support "Long Mode Segment Limit Enable," even if the
> underlying physical processor does, so set that bit in the guest CPUID
> table returned by KVM_GET_SUPPORTED_CPUID.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/svm/svm.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 3af360fe21e6..0bf6ac51f097 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4945,6 +4945,7 @@ static __init void svm_set_cpu_caps(void)
>  	if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) ||
>  	    boot_cpu_has(X86_FEATURE_AMD_SSBD))
>  		kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD);
> +	kvm_cpu_cap_set(X86_FEATURE_NO_LMSLE);

This can go in common x86 code, e.g. if someone wants to run an AMD VM on Intel
hardware.

Side topic, in the context of this series, the below diff highlights how silly it
is for PSFD to be banished from cpufeatures.h.  While we have Boris's attention
(and ACK!), can you tack on a patch to move drop KVM_X86_FEATURE_PSFD and move the
bit to cpufeatures.h where it belongs?

--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -696,6 +696,7 @@ void kvm_set_cpu_caps(void)
                F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON) |
                __feature_bit(KVM_X86_FEATURE_PSFD)
        );
+       kvm_cpu_cap_set(X86_FEATURE_NO_LMSLE);
 
        /*
         * AMD has separate bits for each SPEC_CTRL bit.

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

end of thread, other threads:[~2022-10-07 22:41 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-20 20:59 [PATCH v2 0/3] KVM: EFER.LMSLE cleanup Jim Mattson
2022-09-20 20:59 ` [PATCH v2 1/3] Revert "KVM: SVM: Allow EFER.LMSLE to be set with nested svm" Jim Mattson
2022-09-20 20:59 ` [PATCH v2 2/3] x86/cpufeatures: Introduce X86_FEATURE_NO_LMSLE Jim Mattson
2022-09-21 16:07   ` Borislav Petkov
2022-09-20 20:59 ` [PATCH v2 3/3] KVM: SVM: Unconditionally enumerate EferLmsleUnsupported Jim Mattson
2022-10-07 22:41   ` Sean Christopherson
2022-09-20 21:17 ` [PATCH v2 0/3] KVM: EFER.LMSLE cleanup Borislav Petkov
2022-09-20 21:36   ` Sean Christopherson
2022-09-20 21:36   ` Jim Mattson
2022-09-21  9:28     ` Borislav Petkov
2022-09-21 13:45       ` Jim Mattson
2022-09-21 13:54         ` Borislav Petkov
2022-09-21 15:11           ` Jim Mattson
2022-09-21 16:06             ` Borislav Petkov
2022-09-21 16:23               ` Jim Mattson
2022-09-21 17:11                 ` Borislav Petkov
2022-09-21 17:45                   ` Jim Mattson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).