Re: [PATCH] KVM: nSVM: Enter guest mode before initializing nested NPT MMU

[Yosry Ahmed]

On Thu, Jan 30, 2025 at 08:13:10AM -0800, Sean Christopherson wrote:
> On Thu, Jan 30, 2025, Yosry Ahmed wrote:
> > On Wed, Jan 29, 2025 at 05:08:25PM -0800, Sean Christopherson wrote:
> > > When preparing vmcb02 for nested VMRUN (or state restore), "enter" guest
> > > mode prior to initializing the MMU for nested NPT so that guest_mode is
> > > set in the MMU's role.  KVM's model is that all L2 MMUs are tagged with
> > > guest_mode, as the behavior of hypervisor MMUs tends to be significantly
> > > different than kernel MMUs.
> > > 
> > > Practically speaking, the bug is relatively benign, as KVM only directly
> > > queries role.guest_mode in kvm_mmu_free_guest_mode_roots(), which SVM
> > > doesn't use, and in paths that are optimizations (mmu_page_zap_pte() and
> > > shadow_mmu_try_split_huge_pages()).
> > 
> > Just curious, what about kvm_mmu_page_ad_need_write_protect()?
> 
> Doh, I missed that usage.
> 
> > Is it also bengin?
> 
> Yes.  Better to be lucky than good :-)
> 
> That path forces KVM to use write-protection instead of dirty-bit based Page
> Modification Logging (PML) when L2 is active, because the GPAs captured by the
> CPU would be L2 GPAs, not L1 GPAs, and there's no guarantee that the L2=>L1
> translation would be valid when KVM processes the PML buffer.  To ensure the
> correct page gets marked dirty, KVM uses it's standard write-protect scheme when
> running L2, even if KVM is using PML to dirty log L1 accesses.
> 
> Lucky for me, PML isn't supported on any AMD CPUs.

Well, that worked out nicely, and probably explains why the bug was not
noticed. Thanks for explaining it!