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

["Nikunj A. Dadhania" <nikunj@amd.com>]

On 1/12/2026 3:54 PM, Huang, Kai wrote:
> On Mon, 2026-01-05 at 06:36 +0000, Nikunj A Dadhania wrote:
>> Currently, dirty logging relies on write protecting guest memory and
>> marking dirty GFNs during subsequent write faults. This method works but
>> incurs overhead due to additional write faults for each dirty GFN.
>>
>> Implement support for the Page Modification Logging (PML) feature, a
>> hardware-assisted method for efficient dirty logging. PML automatically
>> logs dirty GPA[51:12] to a 4K buffer when the CPU sets NPT D-bits. Two new
>> VMCB fields are utilized: PML_ADDR and PML_INDEX. The PML_INDEX is
>> initialized to 511 (8 bytes per GPA entry), and the CPU decreases the
>> PML_INDEX after logging each GPA. When the PML buffer is full, a
>> VMEXIT(PML_FULL) with exit code 0x407 is generated.
>>
>> Disable PML for nested guests.
>>
>> PML is enabled by default when supported and can be disabled via the 'pml'
>> module parameter.
> 
> Nit:
> 
> If a new version is needed, use imperative mode:
> 
>   Add a new module parameter to enable/disable PML, and enable it by 
>   default when supported.

Ack

> 
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> 
> It's a bit weird for me to review, but I did anyway and it seems fine to
> me, so:

Thank you for taking the time to review the patches and for the detailed feedback
throughout this series. Your insights have been very helpful.

> 
> Acked-by: Kai Huang <kai.huang@intel.com>
> 
> One minor thing below ...
> 
> [...]
> 
>> @@ -748,12 +748,19 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
>>  						V_NMI_BLOCKING_MASK);
>>  	}
>>  
>> -	/* Copied from vmcb01.  msrpm_base can be overwritten later.  */
>> +	/* Copied from vmcb01. msrpm_base/nested_ctl can be overwritten later. */
>>  	vmcb02->control.nested_ctl = vmcb01->control.nested_ctl;
>>  	vmcb02->control.iopm_base_pa = vmcb01->control.iopm_base_pa;
>>  	vmcb02->control.msrpm_base_pa = vmcb01->control.msrpm_base_pa;
>>  	vmcb_mark_dirty(vmcb02, VMCB_PERM_MAP);
>>  
>> +	/* Disable PML for nested guest as the A/D update is emulated by MMU */
> 
> This comment isn't accurate to me.  I think the key reason is, for L2 if
> PML enabled the recorded GPA will be L2's GPA, but not L1's.
> 
> Please update the comment if a new version is needed?

How about the below:

+	/*
+	 * Disable PML for nested guests. When L2 runs with PML enabled, the
+	 * CPU logs L2 GPAs rather than L1 GPAs, breaking dirty page tracking
+	 * for the L0 hypervisor.
+	 */

Regards
Nikunj