Re: [PATCH v7 7/7] KVM: SVM: Add Page modification logging support

[Sean Christopherson <seanjc@google.com>]

On Mon, May 18, 2026, Yosry Ahmed wrote:
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 4ef9bc6a553f3..dd30aef9fc497 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -882,6 +882,12 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm)
> >  	vmcb02->control.msrpm_base_pa = vmcb01->control.msrpm_base_pa;
> >  	vmcb_mark_dirty(vmcb02, VMCB_PERM_MAP);
> >  
> > +	/* Clear PML fields to avoid stale data in vmcb02. */
> > +	if (pml) {
> > +		vmcb02->control.pml_addr = 0;
> > +		vmcb02->control.pml_index = -1;
> > +	}
> 
> I think the comment here is misleading.

+1000

> vmcb02 is allocated with
> __GFP_ZERO, and IIUC pml_index should not matter when pml_addr is zero.
> This is strictly for hardening as far as I can tell, especially looking
> at commit c3bb9a20834f ("KVM: nVMX: Disable PML in hardware when running
> L2"), which introduced something similar for VMX.
> 
> So maybe something like:
> 	/*
> 	 * PML is never enabled in hardware for L2. Make sure that an
> 	 * unexpected PML write would trigger a PML_FULL VM-Exit.
> 	 */

Hmm, I was going to say we should drop the code entirely, but I do think it makes
sense to set PML_INDEX to -1 to ensure a VM-Exit occurs.

Actually, even better idea, at least for nVMX.  Rather than write '0' for the
address, which is what I really dislike (bad past-Sean!), we should write -1ull
so that unexpectedly enabling PML in vmcs02 results in VM-Fail, i.e. don't even
rely on the WARN for safety.

If SVM has a consistency check on the address, then do the same, otherwise I
guess just go with setting pml_index to -1 and WARNing on PML_FULL?

> Also, the above commit also hanlded a PML_FULL VM-Exit as unexpected,
> maybe we want to do that here as well? Or is that too paranoid?

Not too paranoid.

> Annoyingly, the unexpected exit reason handling is in
> svm_invoke_exit_handler(), but the guest mode check is in
> svm_handle_exit(), so if we do that we may need to move some code
> around.

Why's that?  Oh, if we want to reuse the unexpected exit code.  What about this?

We don't even have to document the same thing in two different places, because
the reasoning for nested_svm_exit_special() can and is different.  E.g. even if
we decided to utilize PML while running L2 (which would be possible, we'd "just"
have to translate the GPAs), the exits would still need to be routed to KVM.
Ditto for if we did actually enable PML in vmcb02 on behalf of L1: we'd need to
remove the check in nested_svm_exit_special(), but not the check in
svm_invoke_exit_handler() (though the latter's comment would need to be updated).

diff --git arch/x86/kvm/svm/nested.c arch/x86/kvm/svm/nested.c
index 4ef9bc6a553f..401a0ac44018 100644
--- arch/x86/kvm/svm/nested.c
+++ arch/x86/kvm/svm/nested.c
@@ -1782,6 +1782,13 @@ int nested_svm_exit_special(struct vcpu_svm *svm)
                if (nested_svm_is_l2_tlb_flush_hcall(vcpu))
                        return NESTED_EXIT_HOST;
                break;
+       case SVM_EXIT_PML_FULL:
+               /*
+                * All PML full exits are handled by KVM.  KVM emulates PML in
+                * software for L1, but never enables PML in hardware on behalf
+                * of L1.
+                */
+               return NESTED_EXIT_HOST;
        default:
                break;
        }
diff --git arch/x86/kvm/svm/svm.c arch/x86/kvm/svm/svm.c
index e74fcde6155e..6bcb191d725b 100644
--- arch/x86/kvm/svm/svm.c
+++ arch/x86/kvm/svm/svm.c
@@ -3635,6 +3635,13 @@ int svm_invoke_exit_handler(struct kvm_vcpu *vcpu, u64 __exit_code)
            (u64)exit_code != __exit_code)
                goto unexpected_vmexit;
 
+       /*
+        * PML is never enabled when running L2, bail immediately if a PML full
+        * exit occurs as something is horribly wrong.
+        */
+       if (unlikely(is_guest_mode(vcpu) && exit_code == SVM_EXIT_PML_FULL))
+               goto unexpected_vmexit;
+
 #ifdef CONFIG_MITIGATION_RETPOLINE
        if (exit_code == SVM_EXIT_MSR)
                return msr_interception(vcpu);