Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Date:   Fri, 22 Sep 2023 09:41:28 -0700
In-Reply-To: <20230921114940.957141-1-pbonzini@redhat.com>
Mime-Version: 1.0
References: <20230921114940.957141-1-pbonzini@redhat.com>
Message-ID: <ZQ3DuFZNFXNWqlbz@google.com>
Subject: Re: [PATCH] x86/cpu: clear SVM feature if disabled by BIOS
From:   Sean Christopherson <seanjc@google.com>
To:     Paolo Bonzini <pbonzini@redhat.com>
Cc:     linux-kernel@vger.kernel.org, kvm@vger.kernel.org, bp@alien8.de
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

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;
 }