linux-kernel - Re: [PATCH] x86/cpu: clear SVM feature if disabled by BIOS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZQ3DuFZNFXNWqlbz@google.com>
Date:   Fri, 22 Sep 2023 09:41:28 -0700
From:   Sean Christopherson <seanjc@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org, bp@...en8.de
Subject: Re: [PATCH] x86/cpu: clear SVM feature if disabled by BIOS

On Thu, Sep 21, 2023, Paolo Bonzini wrote:
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index f283eb47f6ac..7b91efb72ea6 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -531,8 +531,6 @@ static bool __kvm_is_svm_supported(void)
>  	int cpu = smp_processor_id();
>  	struct cpuinfo_x86 *c = &cpu_data(cpu);
>  
> -	u64 vm_cr;
> -
>  	if (c->x86_vendor != X86_VENDOR_AMD &&
>  	    c->x86_vendor != X86_VENDOR_HYGON) {
>  		pr_err("CPU %d isn't AMD or Hygon\n", cpu);
> @@ -549,12 +547,6 @@ static bool __kvm_is_svm_supported(void)
>  		return false;
>  	}

Hidden in here is

	if (!cpu_has(c, X86_FEATURE_SVM)) {
		pr_err("SVM not supported by CPU %d\n", cpu);
		return false;
	}

which will be technically wrong and potentially misleading when SVM is disabled
by BIOS, but supported by the CPU.  We should do the same thing that VMX does
and manually query CPUID to check "is SVM supported", and then rely on cpu_has()
for the "is SVM supported _and_ enabled".  E.g.

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index f283eb47f6ac..9bcd8aad28d7 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -539,22 +539,21 @@ static bool __kvm_is_svm_supported(void)
                return false;
        }
 
-       if (!cpu_has(c, X86_FEATURE_SVM)) {
+       if (!(cpuid_ecx(0x80000001) & feature_bit(SVM))) {
                pr_err("SVM not supported by CPU %d\n", cpu);
                return false;
        }
 
+       if (!cpu_has(c, X86_FEATURE_SVM)) {
+               pr_err("SVM disabled (by BIOS) in MSR_VM_CR on CPU %d\n", cpu);
+               return false;
+       }
+
        if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
                pr_info("KVM is unsupported when running as an SEV guest\n");
                return false;
        }
 
-       rdmsrl(MSR_VM_CR, vm_cr);
-       if (vm_cr & (1 << SVM_VM_CR_SVM_DISABLE)) {
-               pr_err("SVM disabled (by BIOS) in MSR_VM_CR on CPU %d\n", cpu);
-               return false;
-       }
-
        return true;
 }