On Wed, Sep 14, 2022 at 5:32 PM Marc Orr <marcorr@xxxxxxxxxx> wrote: > > On Wed, Sep 14, 2022 at 5:15 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > On Wed, Sep 14, 2022, Marc Orr wrote: > > > On Wed, Sep 14, 2022 at 9:05 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > > > > > On Thu, Sep 08, 2022, Michael Roth wrote: > > > > > On Fri, Oct 15, 2021 at 05:16:28PM +0000, Sean Christopherson wrote: > > > > > So in the context of this interim solution, we're trying to look for a > > > > > solution that's simple enough that it can be used reliably, without > > > > > introducing too much additional complexity into KVM. There is one > > > > > approach that seems to fit that bill, that Brijesh attempted in an > > > > > earlier version of this series (I'm not sure what exactly was the > > > > > catalyst to changing the approach, as I wasn't really in the loop at > > > > > the time, but AIUI there weren't any showstoppers there, but please > > > > > correct me if I'm missing anything): > > > > > > > > > > - if the host is writing to a page that it thinks is supposed to be > > > > > shared, and the guest switches it to private, we get an RMP fault > > > > > (actually, we will get a !PRESENT fault, since as of v5 we now > > > > > remove the mapping from the directmap as part of conversion) > > > > > - in the host #PF handler, if we see that the page is marked private > > > > > in the RMP table, simply switch it back to shared > > > > > - if this was a bug on the part of the host, then the guest will see > > > > > > > > As discussed off-list, attempting to fix up RMP violations in the host #PF handler > > > > is not a viable approach. There was also extensive discussion on-list a while back: > > > > > > > > https://lore.kernel.org/all/8a244d34-2b10-4cf8-894a-1bf12b59cf92@xxxxxxxxxxxxxxxx > > > > > > I mentioned this during Mike's talk at the micro-conference: For pages > > > mapped in by the kernel can we disallow them to be converted to > > > private? > > > > In theory, yes. Do we want to do something like this? No. kmap() does something > > vaguely similar for 32-bit PAE/PSE kernels, but that's a lot of complexity and > > overhead to take on. And this issue goes far beyond a kmap(); when the kernel gup()s > > a page, the kernel expects the pfn to be available, no exceptions (pun intended). > > > > > Note, userspace accesses are already handled by UPM. > > > > I'm confused by the UPM comment. Isn't the gist of this thread about the ability > > to merge SNP _without_ UPM? Or am I out in left field? > > I think that was the overall gist: yes. But it's not what I was trying > to comment on :-). > > HOWEVER, thinking about this more: I was confused when I wrote out my > last reply. I had thought that the issue that Michael brought up > applied even with UPM. That is, I was thinking it was still possibly > for a guest to maliciously convert a page to private mapped in by the > kernel and assumed to be shared. > > But I now realize that is not what will actually happen. To be > concrete, let's assume the GHCB page. What will happen is: > - KVM has GHCB page mapped in. GHCB is always assumed to be shared. So > far so good. > - Malicious guest converts GHCB page to private (e.g., via Page State > Change request) > - Guest exits to KVM > - KVM exits to userspace VMM > - Userspace VM allocates page in private FD. > > Now, what happens here depends on how UPM works. If we allow double > allocation then our host kernel is safe. However, now we have the > "double allocation problem". > > If on the other hand, we deallocate the page in the shared FD, the > host kernel can segfault. And now we actually do have essentially the > same problem Michael was describing that we have without UPM. Because > we'll end up in fault.c in the kernel context and likely panic the > host. Thinking about this even more... Even if we deallocate in the userspace VMM's shared FD, the kernel has its own page tables -- right? So maybe we are actually 100% OK under UPM then regardless of the userspace VMM's policy around managing the private and shared FDs.