linux-kernel - Re: [PATCH v2 07/18] x86/reboot: Disable virtualization during reboot iff callback is registered

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZA9t04+rtW22AMQq@google.com>
Date:   Mon, 13 Mar 2023 11:40: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 07/18] x86/reboot: Disable virtualization during reboot
 iff callback is registered

On Mon, Mar 13, 2023, Huang, Kai wrote:
> On Fri, 2023-03-10 at 13:42 -0800, Sean Christopherson wrote:
> > Attempt to disable virtualization during an emergency reboot if and only
> > if there is a registered virt callback, i.e. iff a hypervisor (KVM) is
> > active.  If there's no active hypervisor, then the CPU can't be operating
> > with VMX or SVM enabled (barring an egregious bug).
> 
> IIUC, this patch is the final one that you want to achieve how the "disable
> virtualization" callback should work in the non-KVM core kernel (the rest
> patches are related to moving VMXOFF code to KVM as the core-kernel now just
> calls the callback, etc). ï¿½
> 
> There are middle step patches (2-7) to eventually help to get to this point. 
> But to be honest, personally, I am not sure whether those patches are necessary,
> i.e. to me they actually cost more time to review since I have to think whether
> such intermediate status is reasonable or not.  I am wondering whether we can
> just merge those patches together as single one, so it's easy to see what is the
> final goal to achieve?

I agree that the fine granularity makes it difficult to see the final form, but
from a bisection perspective I really, really want each change to be isolated as
much as possible.  This code is extremely difficult, if not practically impossible,
to exhaustively test due to multiple points of entry from "this should never happen!"
types of flows.  If any of these changes breaks someones deployment, I want to
make it as easy as possible for that someone to determine exactly what broke.