[PATCH 0/5] KVM: EFER.LMSLE cleanup

[PATCH 0/5] KVM: EFER.LMSLE cleanup

[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."

My preference would be to simply scrub all references to LMSLE from the
Linux kernel, but I don't want to break any guests that rely in it (on
hardware that supports it).

So, here's a series to clean things up.

I have not been successful in getting new macros into cpufeatures.h in
the past, but I'm going to try again, because I am a glutton for
punishment.

Jim Mattson (5):
  x86/cpufeatures: Introduce X86_FEATURE_NO_LMSLE
  KVM: svm: Disallow EFER.LMSLE on hardware that doesn't support it
  KVM: x86: Report host's X86_FEATURE_NO_LMSLE in
    KVM_GET_SUPPORTED_CPUID
  KVM: x86: Enforce X86_FEATURE_NO_LMSLE in guest cpuid
  KVM: svm: Set X86_FEATURE_NO_LMSLE when !nested

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

-- 
2.37.3.968.ga6b4b080e4-goog

[PATCH 1/5] x86/cpufeatures: Introduce X86_FEATURE_NO_LMSLE

[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 deciding whether or not EFER.LMSLE should be a
reserved bit in a KVM guest.

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

[PATCH 2/5] KVM: svm: Disallow EFER.LMSLE on hardware that doesn't support it

[Jim Mattson]

KVM has never properly virtualized EFER.LMSLE. When the "nested"
module parameter is true, it allows an SVM guest to set EFER.LMSLE,
and it passes the bit through in the VMCB, but the KVM emulator
doesn't perform the required data segment limit checks in 64-bit mode.

With Zen3, AMD has dropped support for EFER.LMSLE. Hence, if a Zen3
guest sets EFER.LMSLE, the next VMRUN will fail with "invalid VMCB."

When the host reports X86_FEATURE_NO_LMSLE, treat EFER.LMSLE as a
reserved bit in the guest. Now, if a guest tries to set EFER.LMSLE on
a host without support for EFER.LMSLE, the WRMSR will raise a #GP.

At the moment, the #GP may come as a surprise, but it's an improvement
over the failed VMRUN. The #GP will be vindicated anon.

Fixes: eec4b140c924 ("KVM: SVM: Allow EFER.LMSLE to be set with nested svm")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/svm/svm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index f3813dbacb9f..7c4fd594166c 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5012,7 +5012,9 @@ 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);
+		if (!boot_cpu_has(X86_FEATURE_NO_LMSLE))
+			kvm_enable_efer_bits(EFER_LMSLE);
 	}
 
 	/*
-- 
2.37.3.968.ga6b4b080e4-goog

[PATCH 3/5] KVM: x86: Report host's X86_FEATURE_NO_LMSLE in KVM_GET_SUPPORTED_CPUID

[Jim Mattson]

EFER.LMSLE is not supported in a KVM guest if it is not supported on
the underlying hardware. Inform the guest by exposing the host's value
of X86_FEATURE_NO_LMSLE in KVM_GET_SUPPORTED_CPUID.

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

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 75dcf7a72605..b4975467d686 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -670,7 +670,7 @@ void kvm_set_cpu_caps(void)
 		kvm_cpu_cap_set(X86_FEATURE_GBPAGES);
 
 	kvm_cpu_cap_mask(CPUID_8000_0008_EBX,
-		F(CLZERO) | F(XSAVEERPTR) |
+		F(CLZERO) | F(XSAVEERPTR) | F(NO_LMSLE) |
 		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)
-- 
2.37.3.968.ga6b4b080e4-goog

[PATCH 4/5] KVM: x86: Enforce X86_FEATURE_NO_LMSLE in guest cpuid

[Jim Mattson]

When the guest CPUID reports that EFER.LMSLE is not supported, treat the
bit as reserved.

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

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 43a6a7efc6ec..26c4ca73e389 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1686,6 +1686,9 @@ static bool __kvm_valid_efer(struct kvm_vcpu *vcpu, u64 efer)
 	if (efer & EFER_NX && !guest_cpuid_has(vcpu, X86_FEATURE_NX))
 		return false;
 
+	if (efer & EFER_LMSLE && guest_cpuid_has(vcpu, X86_FEATURE_NO_LMSLE))
+		return false;
+
 	return true;
 
 }
-- 
2.37.3.968.ga6b4b080e4-goog

[PATCH 5/5] KVM: svm: Set X86_FEATURE_NO_LMSLE when !nested

[Jim Mattson]

KVM has never allowed a guest to set EFER.LMSLE when the "nested"
module parameter was false. In the past, there was no way for a guest
to know whether or not this was a legal EFER bit. Now, we can let the
guest know this bit is illegal by reporting X86_FEATURE_NO_LMSLE in
KVM_GET_SUPPORTED_CPUID.

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

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 7c4fd594166c..942602d503ad 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4945,6 +4945,8 @@ 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);
+	if (!nested)
+		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

Re: [PATCH 2/5] KVM: svm: Disallow EFER.LMSLE on hardware that doesn't support it

[Sean Christopherson]

On Thu, Sep 15, 2022, Jim Mattson wrote:
> KVM has never properly virtualized EFER.LMSLE. When the "nested"
> module parameter is true, it allows an SVM guest to set EFER.LMSLE,
> and it passes the bit through in the VMCB, but the KVM emulator
> doesn't perform the required data segment limit checks in 64-bit mode.
> 
> With Zen3, AMD has dropped support for EFER.LMSLE. Hence, if a Zen3
> guest sets EFER.LMSLE, the next VMRUN will fail with "invalid VMCB."
> 
> When the host reports X86_FEATURE_NO_LMSLE, treat EFER.LMSLE as a
> reserved bit in the guest. Now, if a guest tries to set EFER.LMSLE on
> a host without support for EFER.LMSLE, the WRMSR will raise a #GP.
> 
> At the moment, the #GP may come as a surprise, but it's an improvement
> over the failed VMRUN. The #GP will be vindicated anon.
> 
> Fixes: eec4b140c924 ("KVM: SVM: Allow EFER.LMSLE to be set with nested svm")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/svm/svm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index f3813dbacb9f..7c4fd594166c 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -5012,7 +5012,9 @@ 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);
> +		if (!boot_cpu_has(X86_FEATURE_NO_LMSLE))
> +			kvm_enable_efer_bits(EFER_LMSLE);

Since KVM doesn't correctly virtualize EFER.LMSLE, I wonder if we can get away with
dropping support entirely.  I.e. delete the reference to EFER_LMSLE and unconditionally
set F(NO_LMSLE) in KVM_GET_SUPPORTED_CPUID.

Re: [PATCH 2/5] KVM: svm: Disallow EFER.LMSLE on hardware that doesn't support it

[Jim Mattson]

On Fri, Sep 16, 2022 at 1:14 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Sep 15, 2022, Jim Mattson wrote:
> > KVM has never properly virtualized EFER.LMSLE. When the "nested"
> > module parameter is true, it allows an SVM guest to set EFER.LMSLE,
> > and it passes the bit through in the VMCB, but the KVM emulator
> > doesn't perform the required data segment limit checks in 64-bit mode.
> >
> > With Zen3, AMD has dropped support for EFER.LMSLE. Hence, if a Zen3
> > guest sets EFER.LMSLE, the next VMRUN will fail with "invalid VMCB."
> >
> > When the host reports X86_FEATURE_NO_LMSLE, treat EFER.LMSLE as a
> > reserved bit in the guest. Now, if a guest tries to set EFER.LMSLE on
> > a host without support for EFER.LMSLE, the WRMSR will raise a #GP.
> >
> > At the moment, the #GP may come as a surprise, but it's an improvement
> > over the failed VMRUN. The #GP will be vindicated anon.
> >
> > Fixes: eec4b140c924 ("KVM: SVM: Allow EFER.LMSLE to be set with nested svm")
> > Signed-off-by: Jim Mattson <jmattson@google.com>
> > ---
> >  arch/x86/kvm/svm/svm.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index f3813dbacb9f..7c4fd594166c 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -5012,7 +5012,9 @@ 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);
> > +             if (!boot_cpu_has(X86_FEATURE_NO_LMSLE))
> > +                     kvm_enable_efer_bits(EFER_LMSLE);
>
> Since KVM doesn't correctly virtualize EFER.LMSLE, I wonder if we can get away with
> dropping support entirely.  I.e. delete the reference to EFER_LMSLE and unconditionally
> set F(NO_LMSLE) in KVM_GET_SUPPORTED_CPUID.

It's possible that SLES11 Xen 4.0 sets the bit, but never actually
uses truncated segments in 64-bit mode. In any case, according to the
original commit, it won't boot if setting EFER.LMSLE is not allowed.

Re: [PATCH 2/5] KVM: svm: Disallow EFER.LMSLE on hardware that doesn't support it

[Borislav Petkov]

On Fri, Sep 16, 2022 at 02:00:26PM -0700, Jim Mattson wrote:
> It's possible that SLES11 Xen 4.0 sets the bit, but never actually
> uses truncated segments in 64-bit mode. In any case, according to the
> original commit, it won't boot if setting EFER.LMSLE is not allowed.

How is SLE11 at all relevant to the upstream kernel?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 2/5] KVM: svm: Disallow EFER.LMSLE on hardware that doesn't support it

[Sean Christopherson]

On Sat, Sep 17, 2022, Borislav Petkov wrote:
> On Fri, Sep 16, 2022 at 02:00:26PM -0700, Jim Mattson wrote:
> > It's possible that SLES11 Xen 4.0 sets the bit, but never actually
> > uses truncated segments in 64-bit mode. In any case, according to the
> > original commit, it won't boot if setting EFER.LMSLE is not allowed.
> 
> How is SLE11 at all relevant to the upstream kernel?

Yeah, I'm inclined to revert the original commit and advertise NO_LSMLE unconditionally.
I don't like the idea of knowingly ignoring the fact that KVM doesn't correctly
virtualize LMSLE.

Xen itself already does exactly this:

  commit 23ccf530431561268b0190f0f1b740b618771b7b
  Author: Andrew Cooper <andrew.cooper3@citrix.com>
  Date:   Fri Apr 2 14:10:25 2021 +0100

    x86/cpuid: Advertise no-lmsl unilaterally to hvm guests
    
    While part of the original AMD64 spec, Long Mode Segment Limit was a feature
    not picked up by Intel, and therefore didn't see much adoption in software.
    AMD have finally dropped the feature from hardware, and allocated a CPUID bit
    to indicate its absence.
    
    Xen has never supported the feature for guests, even when running on capable
    hardware, so advertise the feature's absence unilaterally.
    
    There is nothing specifically wrong with exposing this bit to PV guests, but
    the PV ABI doesn't include a working concept of MSR_EFER in the first place,
    so exposing it to PV guests would be out-of-place.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
    Reviewed-by: Jan Beulich <jbeulich@suse.com>

because as noted in commit f2c6084442 ("x86/SVM: correct boot time cpu_data[] handling"),
Xen broke detection of LMSLE support shortly after it was added in 2010, presumably
before any official release.  "Support" was added for HVM guests in by commit 

  727bc17d20 ("svm: support EFER.LMSLE for guests")

and then broken a few weeks later by commit
 
  566ddbe833 ("x86: Fail CPU bringup cleanly if it cannot initialise HVM.")

Note that Xen did a "safe" WRMSR+RDMSR to detect LMSLE, so either someone added
extra out-of-tree code that caused Xen to fail to boot, or "necessary ... to boot
with nested svm" only meant being able to expose SVM to L2.

Either way, KVM appears to be carrying a half-baked "fix" for a buggy guest that's
long since gone.  So like we did in commit 8805875aa473 ("Revert "KVM: nVMX: Do not
expose MPX VMX controls when guest MPX disabled""), I think we should just revert
the "fix".

Re: [PATCH 2/5] KVM: svm: Disallow EFER.LMSLE on hardware that doesn't support it

[Borislav Petkov]

On Fri, Sep 16, 2022 at 10:33:29PM +0000, Sean Christopherson wrote:
> ...
> Either way, KVM appears to be carrying a half-baked "fix" for a buggy guest that's
> long since gone.  So like we did in commit 8805875aa473 ("Revert "KVM: nVMX: Do not
> expose MPX VMX controls when guest MPX disabled""), I think we should just revert
> the "fix".

If, as message 0/5 says, setting this bit so that SLE11 Xen 4.0 boots as
a nested hypervisor is the use case, then sure, unconditional NO_LSMLE
and we all should go on with our lives.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 2/5] KVM: svm: Disallow EFER.LMSLE on hardware that doesn't support it

[Jim Mattson]

On Sun, Sep 18, 2022 at 12:04 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Fri, Sep 16, 2022 at 10:33:29PM +0000, Sean Christopherson wrote:
> > ...
> > Either way, KVM appears to be carrying a half-baked "fix" for a buggy guest that's
> > long since gone.  So like we did in commit 8805875aa473 ("Revert "KVM: nVMX: Do not
> > expose MPX VMX controls when guest MPX disabled""), I think we should just revert
> > the "fix".
>
> If, as message 0/5 says, setting this bit so that SLE11 Xen 4.0 boots as
> a nested hypervisor is the use case, then sure, unconditional NO_LSMLE
> and we all should go on with our lives.

Fantastic! That's what I'll do in V2.