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: <20201215185541.nxm2upy76u7z2ko6@amd.com>
Date:   Tue, 15 Dec 2020 12:55:41 -0600
From:   Michael Roth <michael.roth@....com>
To:     Sean Christopherson <seanjc@...gle.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

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.

-Mike