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().
