linux-kernel - Re: [PATCH v2 05/18] x86/reboot: KVM: Disable SVM during reboot via virt/KVM reboot callback

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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZA9avcHRPoIqZP/n@google.com>
Date:   Mon, 13 Mar 2023 10:18:50 -0700
From:   Sean Christopherson <seanjc@...gle.com>
To:     Kai Huang <kai.huang@...el.com>
Cc:     "tglx@...utronix.de" <tglx@...utronix.de>,
        "x86@...nel.org" <x86@...nel.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Chao Gao <chao.gao@...el.com>,
        "andrew.cooper3@...rix.com" <andrew.cooper3@...rix.com>
Subject: Re: [PATCH v2 05/18] x86/reboot: KVM: Disable SVM during reboot via
 virt/KVM reboot callback

On Mon, Mar 13, 2023, Huang, Kai wrote:
> On Fri, 2023-03-10 at 13:42 -0800, Sean Christopherson wrote:
> > Use the virt callback to disable SVM (and set GIF=1) during an emergency
> > instead of blindly attempting to disable SVM.ï¿½ Like the VMX case, if KVM
> > (or an out-of-tree hypervisor) isn't loaded/active, SVM can't be in use.
> > 
> > Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> 
> [...]
> 
> > -#if IS_ENABLED(CONFIG_KVM_INTEL)
> > +#if IS_ENABLED(CONFIG_KVM_INTEL) || IS_ENABLED(CONFIG_KVM_AMD)
> > ï¿½/* RCU-protected callback to disable virtualization prior to reboot. */
> > ï¿½static cpu_emergency_virt_cb __rcu *cpu_emergency_virt_callback;
> > ï¿½
> > @@ -821,7 +821,7 @@ int crashing_cpu = -1;
> > ï¿½ */
> > ï¿½void cpu_emergency_disable_virtualization(void)
> > ï¿½{
> > -#if IS_ENABLED(CONFIG_KVM_INTEL)
> > +#if IS_ENABLED(CONFIG_KVM_INTEL) || IS_ENABLED(CONFIG_KVM_AMD)
> > ï¿½	cpu_emergency_virt_cb *callback;
> > ï¿½
> > ï¿½	rcu_read_lock();
> > @@ -830,8 +830,6 @@ void cpu_emergency_disable_virtualization(void)
> > ï¿½		callback();
> > ï¿½	rcu_read_unlock();
> > ï¿½#endif
> > -	/* KVM_AMD doesn't yet utilize the common callback. */
> > -	cpu_emergency_svm_disable();
> > ï¿½}
> 
> Shouldn't the callback be always present since you want to consider 'out-of-
> tree' hypervisor case?

No?  The kernel doesn't provide any guarantees for out-of-tree code.  I don't have
a super strong preference, though I do like the effective documentation the checks
provide.  Buy more importantly, my understanding is that the x86 maintainers want
to limit the exposure for these types of interfaces, e.g. `git grep IS_ENABLED\(CONFIG_KVM`
for a variety of hooks that are enabled iff KVM is enabled in the kernel config.