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

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

Sashiko reported a couple of issues [1]. Let me address them here:

On 4/7/2026 12:02 PM, Nikunj A Dadhania wrote:
> @@ -1206,6 +1209,16 @@ static void init_vmcb(struct kvm_vcpu *vcpu, bool init_event)
>  	if (vcpu->kvm->arch.bus_lock_detection_enabled)
>  		svm_set_intercept(svm, INTERCEPT_BUSLOCK);
>  
> +	if (pml) {
> +		/*
> +		 * Populate the page address and index here, PML is enabled
> +		 * when dirty logging is enabled on the memslot through
> +		 * svm_update_cpu_dirty_logging()
> +		 */
> +		control->pml_addr = (u64)__sme_set(page_to_phys(vcpu->arch.pml_page));
> +		control->pml_index = PML_HEAD_INDEX;
> +	}
> +

> If the guest receives an INIT IPI and init_vmcb() is called to reset the
> vCPU, does unconditionally setting pml_index to PML_HEAD_INDEX discard any
> un-flushed dirty GPAs logged by the hardware?

There are two scenarios where init_vmcb() is called:

1) During vCPU creation time, where we need to set pml_index to PML_HEAD_INDEX
2) During vCPU reset, when init_event=true

  Before vCPU reset:
    vcpu_enter_guest()
      └─> kvm_x86_call(vcpu_run) [VMRUN]
      └─> [guest executes, PML accumulates dirty pages]
      └─> VMEXIT
      └─> svm_handle_exit() --> PML buffer flushed here
      └─> return to vcpu_run()

  vCPU Reset:
    vcpu_enter_guest()
      ├─> kvm_check_request(KVM_REQ_EVENT)
      ├─> kvm_apic_accept_events()
      │     └─> kvm_vcpu_reset(..., true)
      │           └─> init_vmcb(..., true)
      │                 └─> control->pml_index = PML_HEAD_INDEX -- PML buffer was already flushed
      └─> kvm_x86_call(): Next VMRUN

> Could this result in the hypervisor losing track of dirty memory during live
> migration, leading to memory corruption on the destination host, since
> svm_flush_pml_buffer() isn't called before resetting the index?

AFAIU, no. The PML buffer is always flushed opportunistically at every VM exit.

> +static void svm_flush_pml_buffer(struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +	struct vmcb_control_area *control = &svm->vmcb->control;
> +
> +	/* Do nothing if PML buffer is empty */
> +	if (control->pml_index == PML_HEAD_INDEX)
> +		return;
> +
> +	kvm_flush_pml_buffer(vcpu, control->pml_index);
> +
> +	/* Reset the PML index */
> +	control->pml_index = PML_HEAD_INDEX;
> +}
> +

> Since the AMD SVM architecture aggressively caches VMCB control fields across
> VMRUN instructions using VMCB clean bits, will the hardware recognize this
> updated pml_index?
>
> If the VMCB isn't explicitly marked as dirty here, for example by calling
> vmcb_mark_dirty(svm->vmcb, VMCB_NPT), could the CPU continue using its
> cached index? 
>
> That cached index would be less than 0 after a PML_FULL exit, so would the
> CPU immediately trigger another PML_FULL exit on the next logged write and
> cause an infinite VM-Exit loop?

I got clarification from the design team: No clean bits were defined for the
PML_ADDR and PML_INDEX fields, so on a VMRUN, microcode always loads them from
the VMCB if the feature is enabled.

Regards,
Nikunj

1. https://sashiko.dev/#/patchset/20260407063245.2755579-1-nikunj%40amd.com