linux-kernel - Re: [PATCH v2] KVM: SVM: use vmsave/vmload for saving/restoring additional host state

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: <X9qcsq2kW1kkoVWI@google.com>
Date:   Wed, 16 Dec 2020 15:48:02 -0800
From:   Sean Christopherson <seanjc@...gle.com>
To:     Michael Roth <michael.roth@....com>
Cc:     kvm@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, "H . Peter Anvin" <hpa@...or.com>,
        linux-kernel@...r.kernel.org,
        Tom Lendacky <thomas.lendacky@....com>,
        Andy Lutomirski <luto@...nel.org>
Subject: Re: [PATCH v2] KVM: SVM: use vmsave/vmload for saving/restoring
 additional host state

On Tue, Dec 15, 2020, Michael Roth wrote:
> Hi Sean,
> 
> Sorry to reply out-of-thread, our mail server is having issues with
> certain email addresses at the moment so I only see your message via
> the archives atm. But regarding:
> 
> >>> I think we can defer this until we're actually planning on running
> >>> the guest,
> >>> i.e. put this in svm_prepare_guest_switch().
> >>
> >> It looks like the SEV-ES patches might land before this one, and those
> >> introduce similar handling of VMSAVE in svm_vcpu_load(), so I think it
> >> might also create some churn there if we take this approach and want
> >> to keep the SEV-ES and non-SEV-ES handling similar.
> >
> >Hmm, I'll make sure to pay attention to that when I review the SEV-ES
> >patches,
> >which I was hoping to get to today, but that's looking unlikely at this
> >point.
> 
> It looks like SEV-ES patches are queued now. Those patches have
> undergone a lot of internal testing so I'm really hesitant to introduce
> any significant change to those at this stage as a prereq for my little
> patch. So for v3 I'm a little unsure how best to approach this.
> 
> The main options are:
> 
> a) go ahead and move the vmsave handling for non-sev-es case into
>    prepare_guest_switch() as you suggested, but leave the sev-es where
>    they are. then we can refactor those as a follow-up patch that can be
>    tested/reviewed as a separate series after we've had some time to
>    re-test, though that would probably just complicate the code in the
>    meantime...
> 
> b) stick with the current approach for now, and consider a follow-up series
>    to refactor both sev-es and non-sev-es as a whole that we can test
>    separately.
> 
> c) refactor SEV-ES handling as part of this series. it's only a small change
>    to the SEV-ES code but it re-orders enough things around that I'm
>    concerned it might invalidate some of the internal testing we've done.
>    whereas a follow-up refactoring such as the above options can be rolled
>    into our internal testing so we can let our test teams re-verify
> 
> Obviously I prefer b) but I'm biased on the matter and fine with whatever
> you and others think is best. I just wanted to point out my concerns with
> the various options.

Definitely (c).  This has already missed 5.11 (unless Paolo plans on shooting
from the hip), which means SEV-ES will get to enjoy a full (LTS) kernel release
before these optimizations take effect.

And, the series can be structured so that the optimization (VMSAVE during
.prepare_guest_switch()) is done in a separate patch.  That way, if it does
break SEV-ES (or legacy VMs), the optimized variant can be easily bisected and
fixed or reverted as needed.  E.g. first convert legacy VMs to use VMSAVE+VMLOAD,
possibly consolidating code along the way, then convert all VM types to do
VMSAVE during .prepare_guest_switch().