Re: [PATCH Part2 RFC v4 24/40] KVM: SVM: Add KVM_SEV_SNP_LAUNCH_UPDATE command

From: Brijesh Singh
Date: Mon Jul 19 2021 - 18:06:53 EST

On 7/19/21 3:51 PM, Sean Christopherson wrote:

Hmm, and there's no indication on success that the previous entry was assigned?
Adding a tracepoint in rmpupdate() to allow tracking transitions is probably a
good idea, otherwise debugging RMP violations and/or unexpected #VC is going to
be painful.

Absolutely agree. It's in my TODO list for v5. I have been using my private debug patches with all those trace debug and will try to pull some of those in v5.

And/or if the kernel/KVM behavior is to never reassign directly and reading an RMP
entry isn't prohibitively expensive, then we could add a sanity check that the RMP
is unassigned and reject rmpupdate() if the page is already assigned. Probably
not worth it if the overhead is noticeable, but it could be nice to have if things
go sideways.

In later patches you see that during the page-state change, I do try to read RMP entry to detect some of these condition and warn user about them. The GHCB specification lets the hypervisor choose how it wants to handle the case in guest wanting to add the previously validated page.

To be clear, it's not just an optimization. Pages that haven't yet been touched
may be already owned by a different VM (or even this VM). I.e. "reverting" those
pages would actually result in a form of corruption. It's somewhat of a moot point
because assigning a single page to multiple guests is going to be fatal anyways,
but potentially making a bug worse by introducing even more noise/confusion is not

As you said, if a process is assigning the same page to multiple VMs then its fatal but I agree that we should do the right thing from the kernel ioctl handling. I will just clear the RMP entry for the pages which we touched.