[RFC PATCH 0/5] KVM: nSVM: Emulate Page Modification Logging

[RFC PATCH 0/5] KVM: nSVM: Emulate Page Modification Logging

[Nikunj A Dadhania]

Emulate Page Modification Logging for nested SVM guests, allowing an L1
hypervisor to use PML to track dirty pages in its L2 guests, e.g. for
nested live migration.  Hardware PML is never enabled for L2; KVM instead
logs L2 GPAs into L1's buffer during the nested NPT walk and synthesizes
PML Full exits when it fills, following the existing nVMX emulation.

Based on kvm-x86/next (dea4f6380f4e).

Tested on AMD hardware with PML:

 - A 16G 2-vCPU L2 guest running stress-ng was ping-pong migrated 100
   times within L1, with liveness and memory writes checked after each
   iteration.

 - nested_dirty_log_test and dirty_log_test pass with npt=1 pml=1,
   npt=1 pml=0 and npt=0, with no kernel warnings.

Nikunj A Dadhania (5):
  KVM: x86/mmu: Log dirty GPAs only for the nested GPA walk
  KVM: nSVM: Add nested PML emulation
  KVM: nSVM: Advertise PML to L1
  KVM: selftests: nVMX: Add nested PML coverage to nested_dirty_log_test
  KVM: selftests: nSVM: Add nested PML coverage to nested_dirty_log_test

 arch/x86/include/asm/kvm_host.h               |   1 +
 arch/x86/kvm/cpuid.c                          |   6 +-
 arch/x86/kvm/mmu/paging_tmpl.h                |   6 +-
 arch/x86/kvm/reverse_cpuid.h                  |   5 +
 arch/x86/kvm/svm/nested.c                     |  78 ++++++-
 arch/x86/kvm/svm/svm.c                        |   3 +
 arch/x86/kvm/svm/svm.h                        |   8 +
 .../selftests/kvm/include/x86/processor.h     |   1 +
 tools/testing/selftests/kvm/include/x86/svm.h |   6 +-
 .../selftests/kvm/include/x86/svm_util.h      |   4 +
 tools/testing/selftests/kvm/include/x86/vmx.h |   5 +
 tools/testing/selftests/kvm/lib/x86/svm.c     |   3 +
 tools/testing/selftests/kvm/lib/x86/vmx.c     |  18 ++
 .../selftests/kvm/x86/nested_dirty_log_test.c | 191 +++++++++++++++++-
 14 files changed, 329 insertions(+), 6 deletions(-)


base-commit: dea4f6380f4ec96e8f3b0aaf2343764bd0d25ef4
-- 
2.48.1

[RFC PATCH 1/5] KVM: x86/mmu: Log dirty GPAs only for the nested GPA walk

[Nikunj A Dadhania]

Gate PML logging on the walk being the nested GPA walk, as only that walk
yields an L2 GPA.

No functional change intended, as the EPT template only backs the nested
GPA walk.  The upcoming nested SVM PML support runs this code for PTTYPE
64, which other walks reach too.

Assisted-by: LLM
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 arch/x86/kvm/mmu/paging_tmpl.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index 8e350095508c..043d0ec02b27 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -235,7 +235,9 @@ static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
 				!(pte & PT_GUEST_DIRTY_MASK)) {
 			trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
 #if PTTYPE == PTTYPE_EPT
-			if (kvm_nested_call(write_log_dirty)(vcpu, addr))
+			/* Only the nested GPA walk yields an L2 GPA to log. */
+			if (w == &vcpu->arch.ngpa_walk &&
+			    kvm_nested_call(write_log_dirty)(vcpu, addr))
 				return -EINVAL;
 #endif
 			pte |= PT_GUEST_DIRTY_MASK;
-- 
2.48.1

Re: [RFC PATCH 1/5] KVM: x86/mmu: Log dirty GPAs only for the nested GPA walk

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: x86/mmu: L2 GFN type confusion and missed PML logs for L2 page tables
--

commit 39018b0fb43ef6b337e5d8b61b880163828654c8
Author: Nikunj A Dadhania <nikunj@amd.com>

KVM: x86/mmu: Log dirty GPAs only for the nested GPA walk

This patch gates PML logging on the walk being the nested GPA walk, assuming
only that walk yields an L2 GPA. No functional change is intended as the EPT
template only backs the nested GPA walk.

> diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
> index 8e350095508c5..043d0ec02b272 100644
> --- a/arch/x86/kvm/mmu/paging_tmpl.h
> +++ b/arch/x86/kvm/mmu/paging_tmpl.h
> @@ -235,7 +235,9 @@ static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
>  				!(pte & PT_GUEST_DIRTY_MASK)) {
>  			trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
>  #if PTTYPE == PTTYPE_EPT
> -			if (kvm_nested_call(write_log_dirty)(vcpu, addr))
> +			/* Only the nested GPA walk yields an L2 GPA to log. */
> +			if (w == &vcpu->arch.ngpa_walk &&
> +			    kvm_nested_call(write_log_dirty)(vcpu, addr))

[Severity: High]
Does this change miss PML logs for L2 page table modifications that occur
during an L2 gva_walk?

When KVM emulates setting A/D bits in an L2 page table (during an L2
gva_walk), it is writing to an L2 GPA. Hardware logs page table updates
to the PML buffer, so it seems KVM must do the same.

By ignoring gva_walk, does this fail to log L2 page table modifications
to L1's PML buffer, breaking L1's dirty page tracking?

Additionally, looking further down in update_accessed_dirty_bits():

arch/x86/kvm/mmu/paging_tmpl.h:update_accessed_dirty_bits() {
        ...
        ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
        if (ret)
            return ret;

        kvm_vcpu_mark_page_dirty(vcpu, table_gfn);
        ...
}

During a nested guest page table walk (for example, when
w == &vcpu->arch.gva_walk for L2), walk_addr_generic() populates
walker->table_gfn with the untranslated L2 GFN of the guest page table.

When update_accessed_dirty_bits() sets A/D bits in the L2 page table,
it passes this untranslated L2 GFN to kvm_vcpu_mark_page_dirty().

Since KVM's memslots use L1 physical addresses, does looking up an L2 GFN
here cause type confusion that could corrupt L1's dirty bitmap and fail to
dirty-track the actual L1 backing page (real_gpa)?

>  				return -EINVAL;
>  #endif
>  			pte |= PT_GUEST_DIRTY_MASK;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015103933.2088307-1-nikunj@amd.com?part=1

[RFC PATCH 2/5] KVM: nSVM: Add nested PML emulation

[Nikunj A Dadhania]

Emulate Page Modification Logging (PML) for nested SVM guests, allowing
L1 hypervisors to use PML to track dirty pages in their L2 guests.
KVM emulates PML in software by write-protecting L2 SPTEs and logging
dirty GPAs to L1's PML buffer via the write_log_dirty hook.

PML_FULL exits are prioritized over NPF exits to ensure L1 processes
the PML buffer before continuing L2 execution.

Clear any pending PML Full on nested VMRUN.  The flag is derived from L1's
log index, and a fault that KVM discards instead of injecting can leave it
set after L1 has drained its buffer.  No event is lost: if the buffer is
genuinely full, the next dirty write raises PML Full again.

Assisted-by: LLM
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 arch/x86/kvm/mmu/paging_tmpl.h |  2 +-
 arch/x86/kvm/svm/nested.c      | 75 +++++++++++++++++++++++++++++++++-
 arch/x86/kvm/svm/svm.h         |  8 ++++
 3 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index 043d0ec02b27..c9ec05620e44 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -234,7 +234,7 @@ static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
 		if (level == walker->level && write_fault &&
 				!(pte & PT_GUEST_DIRTY_MASK)) {
 			trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
-#if PTTYPE == PTTYPE_EPT
+#if (PTTYPE == PTTYPE_EPT) || (PTTYPE == 64)
 			/* Only the nested GPA walk yields an L2 GPA to log. */
 			if (w == &vcpu->arch.ngpa_walk &&
 			    kvm_nested_call(write_log_dirty)(vcpu, addr))
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index af0ef678433d..8251b38d1479 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -17,6 +17,7 @@
 #include <linux/kvm_types.h>
 #include <linux/kvm_host.h>
 #include <linux/kernel.h>
+#include <linux/mm.h>
 
 #include <asm/msr-index.h>
 #include <asm/debugreg.h>
@@ -63,7 +64,14 @@ static void nested_svm_inject_npf_exit(struct kvm_vcpu *vcpu,
 	if (WARN_ON_ONCE(hweight64(fault_stage) != 1))
 		fault_stage = PFERR_GUEST_FINAL_MASK;
 
-	vmcb->control.exit_code = SVM_EXIT_NPF;
+	/*
+	 * Only the exit code differs for a PML Full; hardware leaves the
+	 * EXITINFO fields as the nested page fault that filled the buffer
+	 * would set them.
+	 */
+	vmcb->control.exit_code = svm->nested.pml_full ? SVM_EXIT_PML_FULL : SVM_EXIT_NPF;
+	svm->nested.pml_full = false;
+
 	vmcb->control.exit_info_1 = fault_stage |
 				    (fault->error_code & ~PFERR_GUEST_FAULT_STAGE_MASK);
 	vmcb->control.exit_info_2 = fault->address;
@@ -416,6 +424,16 @@ static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
 	       !kvm_vcpu_is_legal_gpa(vcpu, control->nested_cr3)))
 		return false;
 
+	if (control->misc_ctl & SVM_MISC_ENABLE_PML) {
+		if (CC(!(control->misc_ctl & SVM_MISC_ENABLE_NP)))
+			return false;
+		if (CC(!IS_ALIGNED(control->pml_addr, PAGE_SIZE)))
+			return false;
+		if (CC(!nested_svm_check_bitmap_pa(vcpu, control->pml_addr,
+						   PAGE_SIZE)))
+			return false;
+	}
+
 	if (CC(!nested_svm_check_bitmap_pa(vcpu, control->msrpm_base_pa,
 					   MSRPM_SIZE)))
 		return false;
@@ -556,6 +574,8 @@ void __nested_copy_vmcb_control_to_cache(struct kvm_vcpu *vcpu,
 	to->nested_cr3          = from->nested_cr3;
 	to->pause_filter_count  = from->pause_filter_count;
 	to->pause_filter_thresh = from->pause_filter_thresh;
+	to->pml_addr            = from->pml_addr;
+	to->pml_index           = from->pml_index;
 
 	/* Copy asid here because nested_vmcb_check_controls() will check it */
 	to->asid           = from->asid;
@@ -909,6 +929,13 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm)
 	if (pml)
 		vmcb02->control.pml_index = -1;
 
+	/*
+	 * Clear any pending PML Full, as L1's log index has just been
+	 * reloaded from vmcb12.  A fault that KVM discards instead of
+	 * injecting can leave the flag set after L1 drained its buffer.
+	 */
+	svm->nested.pml_full = false;
+
 	/*
 	 * Stash vmcb02's counter if the guest hasn't moved past the guilty
 	 * instruction; otherwise, reset the counter to '0'.
@@ -1311,6 +1338,8 @@ static int nested_svm_vmexit_update_vmcb12(struct kvm_vcpu *vcpu)
 	vmcb12->control.event_inj	  = 0;
 	vmcb12->control.event_inj_err	  = 0;
 	vmcb12->control.int_ctl           = svm->nested.ctl.int_ctl;
+	vmcb12->control.pml_addr          = svm->nested.ctl.pml_addr;
+	vmcb12->control.pml_index         = svm->nested.ctl.pml_index;
 
 	trace_kvm_nested_vmexit_inject(vmcb12->control.exit_code,
 				       vmcb12->control.exit_info_1,
@@ -1886,6 +1915,8 @@ static void nested_copy_vmcb_cache_to_control(struct vmcb_control_area *dst,
 	dst->misc_ctl2		  = from->misc_ctl2;
 	dst->pause_filter_count   = from->pause_filter_count;
 	dst->pause_filter_thresh  = from->pause_filter_thresh;
+	dst->pml_addr             = from->pml_addr;
+	dst->pml_index            = from->pml_index;
 	/* 'clean' and 'hv_enlightenments' are not changed by KVM */
 }
 
@@ -2169,6 +2200,47 @@ static gpa_t svm_translate_nested_gpa(struct kvm_vcpu *vcpu, gpa_t gpa,
 	return w->gva_to_gpa(vcpu, w, gpa, access, exception);
 }
 
+static int nested_svm_write_pml_buffer(struct kvm_vcpu *vcpu, gpa_t gpa)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+	u16 index;
+	gpa_t dst;
+
+	if (WARN_ON_ONCE(!is_guest_mode(vcpu)) || !nested_pml_enabled(svm))
+		return 0;
+
+	/* The flag can be set here if the fault that raised it was discarded. */
+	if (svm->nested.pml_full)
+		return 1;
+
+	index = svm->nested.ctl.pml_index;
+	if (index >= PML_LOG_NR_ENTRIES) {
+		svm->nested.pml_full = true;
+		return 1;
+	}
+
+	gpa &= ~0xFFFULL;
+	dst = svm->nested.ctl.pml_addr + sizeof(u64) * index;
+
+	/*
+	 * Drop the entry if the write fails.  VMRUN only verifies that L1's
+	 * buffer address is a legal GPA, not that it is backed, so a failure
+	 * here means L1 has pointed PML at memory it does not have, and only
+	 * L1's own dirty tracking suffers.
+	 */
+	if (kvm_write_guest_page(vcpu->kvm, gpa_to_gfn(dst), &gpa,
+				 offset_in_page(dst), sizeof(gpa)))
+		return 0;
+
+	/*
+	 * Decrement PML_INDEX after logging the GPA (buffer fills backwards
+	 * from index 511 to 0)
+	 */
+	svm->nested.ctl.pml_index = index - 1;
+
+	return 0;
+}
+
 struct kvm_x86_nested_ops svm_nested_ops __initdata = {
 	.leave_nested = svm_leave_nested,
 	.translate_nested_gpa = svm_translate_nested_gpa,
@@ -2179,4 +2251,5 @@ struct kvm_x86_nested_ops svm_nested_ops __initdata = {
 	.get_state = svm_get_nested_state,
 	.set_state = svm_set_nested_state,
 	.hv_inject_synthetic_vmexit_post_tlb_flush = svm_hv_inject_synthetic_vmexit_post_tlb_flush,
+	.write_log_dirty = nested_svm_write_pml_buffer,
 };
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 6d0242966dc8..6a0359eb0c7c 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -196,6 +196,8 @@ struct vmcb_ctrl_area_cached {
 	u64 exit_info_2;
 	u32 exit_int_info;
 	u32 exit_int_info_err;
+	u64 pml_addr;
+	u16 pml_index;
 	u64 misc_ctl;
 	u32 event_inj;
 	u32 event_inj_err;
@@ -235,6 +237,7 @@ struct svm_nested_state {
 	struct vmcb_save_area_cached save;
 
 	bool initialized;
+	bool pml_full;
 
 	/*
 	 * Indicates whether MSR bitmap for L2 needs to be rebuilt due to
@@ -657,6 +660,11 @@ static inline bool nested_npt_enabled(struct vcpu_svm *svm)
 	return svm->nested.ctl.misc_ctl & SVM_MISC_ENABLE_NP;
 }
 
+static inline bool nested_pml_enabled(struct vcpu_svm *svm)
+{
+	return svm->nested.ctl.misc_ctl & SVM_MISC_ENABLE_PML;
+}
+
 static inline bool l2_has_separate_pat(struct kvm_vcpu *vcpu)
 {
 	/*
-- 
2.48.1

Re: [RFC PATCH 2/5] KVM: nSVM: Add nested PML emulation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: nSVM: spurious PML_FULL VM-Exit injection on discarded faults
--

commit ac3a6ec9423b67a67c9dfb9dd5ebd698f451f130
Author: Nikunj A Dadhania <nikunj@amd.com>

KVM: nSVM: Add nested PML emulation

This commit emulates Page Modification Logging (PML) for nested SVM guests,
allowing L1 hypervisors to track dirty pages in their L2 guests. KVM emulates
PML in software by write-protecting L2 SPTEs and logging dirty GPAs to L1's
PML buffer.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index af0ef678433d7..8251b38d14790 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c

[ ... ]

> @@ -63,7 +64,14 @@ static void nested_svm_inject_npf_exit(struct kvm_vcpu *vcpu,
>  	if (WARN_ON_ONCE(hweight64(fault_stage) != 1))
>  		fault_stage = PFERR_GUEST_FINAL_MASK;
>  
> -	vmcb->control.exit_code = SVM_EXIT_NPF;
> +	/*
> +	 * Only the exit code differs for a PML Full; hardware leaves the
> +	 * EXITINFO fields as the nested page fault that filled the buffer
> +	 * would set them.
> +	 */
> +	vmcb->control.exit_code = svm->nested.pml_full ? SVM_EXIT_PML_FULL : SVM_EXIT_NPF;
> +	svm->nested.pml_full = false;
> +
>  	vmcb->control.exit_info_1 = fault_stage |
>  				    (fault->error_code & ~PFERR_GUEST_FAULT_STAGE_MASK);
>  	vmcb->control.exit_info_2 = fault->address;

[Severity: High]
If L2 triggers a legitimate but unrelated nested page fault (such as a read
fault) after a previous discarded fault left pml_full set, does this blindly
consume the lingering pml_full flag? 

It looks like this would inject an SVM_EXIT_PML_FULL exit into L1 carrying
the error code of a read fault. Since hardware PML only logs writes, could
this architecturally impossible state crash or confuse the L1 hypervisor?

[ ... ]

> @@ -2169,6 +2200,47 @@ static gpa_t svm_translate_nested_gpa(struct kvm_vcpu *vcpu, gpa_t gpa,
>  	return w->gva_to_gpa(vcpu, w, gpa, access, exception);
>  }
>  
> +static int nested_svm_write_pml_buffer(struct kvm_vcpu *vcpu, gpa_t gpa)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +	u16 index;
> +	gpa_t dst;
> +
> +	if (WARN_ON_ONCE(!is_guest_mode(vcpu)) || !nested_pml_enabled(svm))
> +		return 0;
> +
> +	/* The flag can be set here if the fault that raised it was discarded. */
> +	if (svm->nested.pml_full)
> +		return 1;
> +
> +	index = svm->nested.ctl.pml_index;
> +	if (index >= PML_LOG_NR_ENTRIES) {
> +		svm->nested.pml_full = true;
> +		return 1;
> +	}

[Severity: High]
If KVM resolves or discards the page fault internally without injecting it
into L1, this leaves the pml_full flag set while L2 execution resumes.

The comment acknowledges that the flag can be left set by discarded faults.
However, is it safe to leave it set if the next fault isn't guaranteed to be
a write?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015103933.2088307-1-nikunj@amd.com?part=2

[RFC PATCH 3/5] KVM: nSVM: Advertise PML to L1

[Nikunj A Dadhania]

Enumerate PML to L1 via CPUID 0x8000000A ECX[4] so that an L1 hypervisor
can use PML to track dirty pages in its L2 guests, and clear the capability
when the host lacks PML in hardware.

Honor the capability on VMRUN by stripping PML from L1's VMCB when the
feature is not exposed to the guest.

Assisted-by: LLM
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 arch/x86/include/asm/kvm_host.h | 1 +
 arch/x86/kvm/cpuid.c            | 6 +++++-
 arch/x86/kvm/reverse_cpuid.h    | 5 +++++
 arch/x86/kvm/svm/nested.c       | 3 +++
 arch/x86/kvm/svm/svm.c          | 3 +++
 5 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index e2eef7057bd3..aaddac3c20e3 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -711,6 +711,7 @@ enum kvm_only_cpuid_leafs {
 	CPUID_7_1_ECX,
 	CPUID_1E_1_EAX,
 	CPUID_24_1_ECX,
+	CPUID_8000_000A_ECX,
 	NR_KVM_CPU_CAPS,
 
 	NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index adafd6fa2c54..bc81e906082f 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1195,6 +1195,10 @@ void kvm_initialize_cpu_caps(void)
 		kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD);
 
 	/* All SVM features required additional vendor module enabling. */
+	kvm_cpu_cap_init(CPUID_8000_000A_ECX,
+		SCATTERED_F(PML),
+	);
+
 	kvm_cpu_cap_init(CPUID_8000_000A_EDX,
 		VENDOR_F(NPT),
 		VENDOR_F(VMCBCLEAN),
@@ -1848,7 +1852,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 		entry->eax = 1; /* SVM revision 1 */
 		entry->ebx = 8; /* Lets support 8 ASIDs in case we add proper
 				   ASID emulation to nested SVM */
-		entry->ecx = 0; /* Reserved */
+		cpuid_entry_override(entry, CPUID_8000_000A_ECX);
 		cpuid_entry_override(entry, CPUID_8000_000A_EDX);
 		break;
 	case 0x80000019:
diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index 89345af0f7ad..2ec5aa715777 100644
--- a/arch/x86/kvm/reverse_cpuid.h
+++ b/arch/x86/kvm/reverse_cpuid.h
@@ -71,6 +71,9 @@
 /* CPUID level 0x80000007 (EDX). */
 #define KVM_X86_FEATURE_CONSTANT_TSC	KVM_X86_FEATURE(CPUID_8000_0007_EDX, 8)
 
+/* CPUID level 0x8000000A (ECX) */
+#define KVM_X86_FEATURE_PML		KVM_X86_FEATURE(CPUID_8000_000A_ECX, 4)
+
 /* CPUID level 0x80000022 (EAX) */
 #define KVM_X86_FEATURE_PERFMON_V2	KVM_X86_FEATURE(CPUID_8000_0022_EAX, 0)
 
@@ -111,6 +114,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
 	[CPUID_7_1_ECX]       = {         7, 1, CPUID_ECX},
 	[CPUID_1E_1_EAX]      = {      0x1e, 1, CPUID_EAX},
 	[CPUID_24_1_ECX]      = {      0x24, 1, CPUID_ECX},
+	[CPUID_8000_000A_ECX] = {0x8000000a, 0, CPUID_ECX},
 };
 
 /*
@@ -153,6 +157,7 @@ static __always_inline u32 __feature_translate(int x86_feature)
 	KVM_X86_TRANSLATE_FEATURE(TSA_SQ_NO);
 	KVM_X86_TRANSLATE_FEATURE(TSA_L1_NO);
 	KVM_X86_TRANSLATE_FEATURE(MSR_IMM);
+	KVM_X86_TRANSLATE_FEATURE(PML);
 	default:
 		return x86_feature;
 	}
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 8251b38d1479..dd3226e83b70 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -552,6 +552,9 @@ void __nested_copy_vmcb_control_to_cache(struct kvm_vcpu *vcpu,
 	if (!gmet_enabled || !guest_cpu_cap_has(vcpu, X86_FEATURE_GMET))
 		to->misc_ctl &= ~SVM_MISC_ENABLE_GMET;
 
+	if (!pml || !guest_cpu_cap_has(vcpu, X86_FEATURE_PML))
+		to->misc_ctl &= ~SVM_MISC_ENABLE_PML;
+
 	if (!guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV))
 		to->misc_ctl2 &= ~SVM_MISC2_ENABLE_V_LBR;
 
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 136069dfcd44..a8609f860415 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5751,6 +5751,9 @@ static __init void svm_set_cpu_caps(void)
 	if (!lbrv)
 		kvm_cpu_cap_clear(X86_FEATURE_BUS_LOCK_DETECT);
 
+	if (!pml)
+		kvm_cpu_cap_clear(X86_FEATURE_PML);
+
 	kvm_setup_xss_caps();
 	kvm_finalize_cpu_caps();
 }
-- 
2.48.1

[RFC PATCH 4/5] KVM: selftests: nVMX: Add nested PML coverage to nested_dirty_log_test

[Nikunj A Dadhania]

Add coverage for nested PML, i.e. for KVM logging L2's dirty GPAs into L1's
PML buffer on L1's behalf.  Verify each page is logged exactly once, no
matter how many times L2 writes to it, and that L1 takes a PML_FULL exit
when the buffer fills, can drain it, and can resume L2 without losing
entries.

Seed PML_INDEX past the end of the buffer so that L2's first write takes a
PML_FULL exit before anything is logged.  This keeps the number of exits
deterministic, and leaves the buffer contents identical to a run that never
fills, so the logging checks apply unchanged.

Assisted-by: LLM
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 tools/testing/selftests/kvm/include/x86/vmx.h |   5 +
 tools/testing/selftests/kvm/lib/x86/vmx.c     |  18 +++
 .../selftests/kvm/x86/nested_dirty_log_test.c | 149 +++++++++++++++++-
 3 files changed, 171 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
index 8fd97434eee5..b52b2770ba0a 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -471,6 +471,10 @@ struct vmx_pages {
 	void *stack;
 
 	u64 eptp_gpa;
+
+	/* PML buffer */
+	void *pml;
+	u64 pml_gpa;
 };
 
 union vmx_basic {
@@ -515,6 +519,7 @@ static inline bool kvm_cpu_has_vmx_virtualize_apic_accesses(void)
 	return kvm_cpu_has_secondary_exec_control(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES);
 }
 
+bool kvm_cpu_has_vmx_pml(void);
 void vm_enable_ept(struct kvm_vm *vm);
 void prepare_virtualize_apic_accesses(struct vmx_pages *vmx, struct kvm_vm *vm);
 
diff --git a/tools/testing/selftests/kvm/lib/x86/vmx.c b/tools/testing/selftests/kvm/lib/x86/vmx.c
index 5a06746ebea4..8ccdae85d761 100644
--- a/tools/testing/selftests/kvm/lib/x86/vmx.c
+++ b/tools/testing/selftests/kvm/lib/x86/vmx.c
@@ -118,6 +118,9 @@ vcpu_alloc_vmx(struct kvm_vm *vm, gva_t *p_vmx_gva)
 
 	vmx->stack = (void *)vm_alloc_stack(vm, 1);
 
+	vmx->pml = (void *)vm_alloc_page(vm);
+	vmx->pml_gpa = addr_gva2gpa(vm, (uintptr_t)vmx->pml);
+
 	if (vm->stage2_mmu.pgd_created)
 		vmx->eptp_gpa = vm->stage2_mmu.pgd;
 
@@ -437,6 +440,21 @@ bool kvm_cpu_has_secondary_exec_control(u32 ctrl)
 	return ctrl_msr & ctrl;
 }
 
+bool kvm_cpu_has_vmx_pml(void)
+{
+	u64 ctrl;
+
+	if (!kvm_cpu_has(X86_FEATURE_VMX))
+		return false;
+
+	ctrl = kvm_get_feature_msr(MSR_IA32_VMX_TRUE_PROCBASED_CTLS) >> 32;
+	if (!(ctrl & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS))
+		return false;
+
+	ctrl = kvm_get_feature_msr(MSR_IA32_VMX_PROCBASED_CTLS2) >> 32;
+	return ctrl & SECONDARY_EXEC_ENABLE_PML;
+}
+
 void prepare_virtualize_apic_accesses(struct vmx_pages *vmx, struct kvm_vm *vm)
 {
 	vmx->apic_access = (void *)vm_alloc_page(vm);
diff --git a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
index 7e5f9ae1cb00..b1008724f46d 100644
--- a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
@@ -192,6 +192,149 @@ static void test_handle_ucall_sync(struct kvm_vm *vm, u64 arg,
 	}
 }
 
+#define PML_LOG_NR_ENTRIES	512
+#define PML_HEAD_INDEX		(PML_LOG_NR_ENTRIES - 1)
+
+#define PML_FULL_MAX_EXITS	64
+
+static void l2_pml_guest_code(void)
+{
+	gva_t page0 = TEST_ALIAS_GPA(0);
+	gva_t page1 = TEST_ALIAS_GPA(1);
+
+	/*
+	 * PML logs a GPA when the stage-2 Dirty bit goes from clear to set, so
+	 * the duplicate write to page0 must not produce a second entry.
+	 */
+	WRITE_ONCE(*(u64 *)page0, 1);
+	WRITE_ONCE(*(u64 *)page0, 1);
+	WRITE_ONCE(*(u64 *)page1, 1);
+
+	vmcall();
+}
+
+static void l1_pml_vmx_code(struct vmx_pages *vmx)
+{
+	int nr_pml_full = 0;
+
+	GUEST_ASSERT(vmx->vmcs_gpa);
+	prepare_for_vmx_operation(vmx);
+	load_vmcs(vmx);
+
+	/* PML requires EPT. */
+	GUEST_ASSERT(vmx->eptp_gpa);
+
+	prepare_vmcs(vmx, l2_pml_guest_code);
+
+	vmwrite(PML_ADDRESS, vmx->pml_gpa);
+	vmwrite(GUEST_PML_INDEX, PML_LOG_NR_ENTRIES);
+	vmwrite(SECONDARY_VM_EXEC_CONTROL,
+		vmread(SECONDARY_VM_EXEC_CONTROL) | SECONDARY_EXEC_ENABLE_PML);
+
+	vmlaunch();
+
+	for (;;) {
+		u64 reason = vmread(VM_EXIT_REASON);
+
+		if (reason == EXIT_REASON_VMCALL)
+			break;
+
+		GUEST_ASSERT_EQ(reason, EXIT_REASON_PML_FULL);
+		GUEST_ASSERT(++nr_pml_full < PML_FULL_MAX_EXITS);
+
+		vmwrite(GUEST_PML_INDEX, PML_HEAD_INDEX);
+		vmresume();
+	}
+
+	GUEST_SYNC_ARGS(vmread(GUEST_PML_INDEX), nr_pml_full, 0, 0, 0);
+	GUEST_DONE();
+}
+
+static void check_pml_buffer(struct kvm_vm *vm, u64 pml_gpa, u64 pml_index)
+{
+	u64 *pml_buf = addr_gpa2hva(vm, pml_gpa);
+	int nr_page0 = 0, nr_page1 = 0;
+	int i;
+
+	/*
+	 * L2 also dirties its stack, page tables, etc., so the buffer holds
+	 * more than just the test pages.  Search it rather than assuming the
+	 * test pages land at a fixed offset.  PML fills the buffer backwards,
+	 * starting at PML_HEAD_INDEX.
+	 */
+	TEST_ASSERT(pml_index < PML_HEAD_INDEX,
+		    "PML buffer is empty, PML_INDEX = %lu", pml_index);
+
+	for (i = PML_HEAD_INDEX; i > pml_index; i--) {
+		if (pml_buf[i] == TEST_ALIAS_GPA(0))
+			nr_page0++;
+		else if (pml_buf[i] == TEST_ALIAS_GPA(1))
+			nr_page1++;
+	}
+
+	TEST_ASSERT(nr_page0 == 1, "GPA 0x%lx logged %d times, expected 1",
+		    (u64)TEST_ALIAS_GPA(0), nr_page0);
+	TEST_ASSERT(nr_page1 == 1, "GPA 0x%lx logged %d times, expected 1",
+		    (u64)TEST_ALIAS_GPA(1), nr_page1);
+}
+
+static void test_nested_pml(void)
+{
+	u64 pml_gpa, pml_index = PML_HEAD_INDEX, nr_pml_full = 0;
+	struct kvm_vcpu *vcpu;
+	gva_t nested_gva = 0;
+	struct kvm_vm *vm;
+	struct ucall uc;
+	bool done = false;
+
+	pr_info("Nested TDP: enabled, PML\n");
+
+	vm = vm_create_with_one_vcpu(&vcpu, l1_pml_vmx_code);
+	vm_enable_tdp(vm);
+
+	pml_gpa = vcpu_alloc_vmx(vm, &nested_gva)->pml_gpa;
+
+	vcpu_args_set(vcpu, 1, nested_gva);
+
+	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, TEST_MEM_BASE,
+				    TEST_MEM_SLOT_INDEX, TEST_MEM_PAGES,
+				    KVM_MEM_LOG_DIRTY_PAGES);
+
+	virt_map(vm, TEST_MEM_BASE, TEST_MEM_BASE, TEST_MEM_PAGES);
+
+	/* Leave the stage-2 Dirty bits clear, or L2's writes won't be logged. */
+	tdp_identity_map_default_memslots(vm);
+	tdp_map(vm, TEST_ALIAS_GPA(0), TEST_GPA(0), PAGE_SIZE);
+	tdp_map(vm, TEST_ALIAS_GPA(1), TEST_GPA(1), PAGE_SIZE);
+
+	while (!done) {
+		vcpu_run(vcpu);
+		TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
+
+		switch (get_ucall(vcpu, &uc)) {
+		case UCALL_ABORT:
+			REPORT_GUEST_ASSERT(uc);
+			/* NOT REACHED */
+		case UCALL_SYNC:
+			pml_index = uc.args[1];
+			nr_pml_full = uc.args[2];
+			break;
+		case UCALL_DONE:
+			done = true;
+			break;
+		default:
+			TEST_FAIL("Unknown ucall %lu", uc.cmd);
+		}
+	}
+
+	TEST_ASSERT(nr_pml_full == 1, "Expected 1 PML_FULL exit, got %lu",
+		    nr_pml_full);
+
+	check_pml_buffer(vm, pml_gpa, pml_index);
+
+	kvm_vm_free(vm);
+}
+
 static void test_dirty_log(bool nested_tdp)
 {
 	gva_t nested_gva = 0;
@@ -282,8 +425,12 @@ int main(int argc, char *argv[])
 
 	test_dirty_log(/*nested_tdp=*/false);
 
-	if (kvm_cpu_has_tdp())
+	if (kvm_cpu_has_tdp()) {
 		test_dirty_log(/*nested_tdp=*/true);
 
+		if (kvm_cpu_has_vmx_pml())
+			test_nested_pml();
+	}
+
 	return 0;
 }
-- 
2.48.1

Re: [RFC PATCH 4/5] KVM: selftests: nVMX: Add nested PML coverage to nested_dirty_log_test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: kvm: nVMX test asserts on invalid guest PML index
--

commit 88c8168ec493a327cfe182178b11f281733406e7
Author: Nikunj A Dadhania <nikunj@amd.com>

KVM: selftests: nVMX: Add nested PML coverage to nested_dirty_log_test

This patch adds coverage for nested PML to nested_dirty_log_test. It
verifies that L1 takes a PML_FULL exit when the buffer fills, can drain it,
and can resume L2 without losing entries.

> diff --git a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
> index 7e5f9ae1cb00a..b1008724f46dc 100644
> --- a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c

[ ... ]

> @@ -192,6 +192,149 @@ static void test_handle_ucall_sync(struct kvm_vm *vm, u64 arg,
>  	}
>  }
>  
> +#define PML_LOG_NR_ENTRIES	512
> +#define PML_HEAD_INDEX		(PML_LOG_NR_ENTRIES - 1)
> +
> +#define PML_FULL_MAX_EXITS	64

[ ... ]

> +static void l1_pml_vmx_code(struct vmx_pages *vmx)
> +{
> +	int nr_pml_full = 0;
> +
> +	GUEST_ASSERT(vmx->vmcs_gpa);
> +	prepare_for_vmx_operation(vmx);
> +	load_vmcs(vmx);
> +
> +	/* PML requires EPT. */
> +	GUEST_ASSERT(vmx->eptp_gpa);
> +
> +	prepare_vmcs(vmx, l2_pml_guest_code);
> +
> +	vmwrite(PML_ADDRESS, vmx->pml_gpa);
> +	vmwrite(GUEST_PML_INDEX, PML_LOG_NR_ENTRIES);
> +	vmwrite(SECONDARY_VM_EXEC_CONTROL,
> +		vmread(SECONDARY_VM_EXEC_CONTROL) | SECONDARY_EXEC_ENABLE_PML);
> +
> +	vmlaunch();
> +
> +	for (;;) {
> +		u64 reason = vmread(VM_EXIT_REASON);
> +
> +		if (reason == EXIT_REASON_VMCALL)
> +			break;
> +
> +		GUEST_ASSERT_EQ(reason, EXIT_REASON_PML_FULL);

[Severity: Medium]
Since the test intentionally sets GUEST_PML_INDEX to 512, shouldn't this
result in a VM-entry failure instead of a successful entry followed by a
PML_FULL exit?

According to the Intel SDM (Volume 3C, Section 26.3.1.5 "Checks on Guest
Non-Register State"), the guest PML index must be less than 512 when PML
is enabled.

Does this assertion rely on KVM improperly omitting the guest state check?
If KVM is updated in the future to correctly emulate the architecturally
mandated check and fail the VM-entry, will this assertion break the test
suite?

> +		GUEST_ASSERT(++nr_pml_full < PML_FULL_MAX_EXITS);
> +
> +		vmwrite(GUEST_PML_INDEX, PML_HEAD_INDEX);
> +		vmresume();
> +	}
> +
> +	GUEST_SYNC_ARGS(vmread(GUEST_PML_INDEX), nr_pml_full, 0, 0, 0);
> +	GUEST_DONE();
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015103933.2088307-1-nikunj@amd.com?part=4

[RFC PATCH 5/5] KVM: selftests: nSVM: Add nested PML coverage to nested_dirty_log_test

[Nikunj A Dadhania]

Extend the nested PML test to cover nSVM.

Assisted-by: LLM
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 .../selftests/kvm/include/x86/processor.h     |  1 +
 tools/testing/selftests/kvm/include/x86/svm.h |  6 ++-
 .../selftests/kvm/include/x86/svm_util.h      |  4 ++
 tools/testing/selftests/kvm/lib/x86/svm.c     |  3 ++
 .../selftests/kvm/x86/nested_dirty_log_test.c | 48 +++++++++++++++++--
 5 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
index 5898a277a2e0..3f4eda041fd4 100644
--- a/tools/testing/selftests/kvm/include/x86/processor.h
+++ b/tools/testing/selftests/kvm/include/x86/processor.h
@@ -219,6 +219,7 @@ struct kvm_x86_cpu_feature {
 #define	X86_FEATURE_RDPRU		KVM_X86_CPU_FEATURE(0x80000008, 0, EBX, 4)
 #define	X86_FEATURE_AMD_IBPB		KVM_X86_CPU_FEATURE(0x80000008, 0, EBX, 12)
 #define	X86_FEATURE_EFER_LMSLE_MBZ	KVM_X86_CPU_FEATURE(0x80000008, 0, EBX, 20)
+#define	X86_FEATURE_PML			KVM_X86_CPU_FEATURE(0x8000000A, 0, ECX, 4)
 #define	X86_FEATURE_NPT			KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 0)
 #define	X86_FEATURE_LBRV		KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 1)
 #define	X86_FEATURE_NRIPS		KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 3)
diff --git a/tools/testing/selftests/kvm/include/x86/svm.h b/tools/testing/selftests/kvm/include/x86/svm.h
index c8539166270e..00b410bb18be 100644
--- a/tools/testing/selftests/kvm/include/x86/svm.h
+++ b/tools/testing/selftests/kvm/include/x86/svm.h
@@ -115,7 +115,10 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
 	u64 avic_physical_id;	/* Offset 0xf8 */
 	u8 reserved_7[8];
 	u64 vmsa_pa;		/* Used for an SEV-ES guest */
-	u8 reserved_8[720];
+	u8 reserved_8[184];	/* Offset 0x110 */
+	u64 pml_addr;		/* Offset 0x1c8 */
+	u16 pml_index;		/* Offset 0x1d0 */
+	u8 reserved_9[526];	/* Offset 0x1d2 */
 	/*
 	 * Offset 0x3e0, 32 bytes reserved
 	 * for use by hypervisor/software.
@@ -174,6 +177,7 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
 
 #define SVM_MISC_ENABLE_NP		BIT(0)
 #define SVM_MISC_ENABLE_SEV		BIT(1)
+#define SVM_MISC_ENABLE_PML		BIT(11)
 
 #define SVM_MISC2_ENABLE_V_LBR BIT_ULL(0)
 #define SVM_MISC2_ENABLE_V_VMLOAD_VMSAVE BIT_ULL(1)
diff --git a/tools/testing/selftests/kvm/include/x86/svm_util.h b/tools/testing/selftests/kvm/include/x86/svm_util.h
index c201c30485e7..62d2ca9dda34 100644
--- a/tools/testing/selftests/kvm/include/x86/svm_util.h
+++ b/tools/testing/selftests/kvm/include/x86/svm_util.h
@@ -33,6 +33,10 @@ struct svm_test_data {
 
 	/* NPT */
 	u64 ncr3_gpa;
+
+	/* PML buffer */
+	void *pml; /* gva */
+	u64 pml_gpa;
 };
 
 static inline void vmmcall(void)
diff --git a/tools/testing/selftests/kvm/lib/x86/svm.c b/tools/testing/selftests/kvm/lib/x86/svm.c
index b05be50f075d..ef22f2686b3f 100644
--- a/tools/testing/selftests/kvm/lib/x86/svm.c
+++ b/tools/testing/selftests/kvm/lib/x86/svm.c
@@ -45,6 +45,9 @@ vcpu_alloc_svm(struct kvm_vm *vm, gva_t *p_svm_gva)
 
 	svm->stack = (void *)vm_alloc_stack(vm, 1);
 
+	svm->pml = (void *)vm_alloc_page(vm);
+	svm->pml_gpa = addr_gva2gpa(vm, (uintptr_t)svm->pml);
+
 	if (vm->stage2_mmu.pgd_created)
 		svm->ncr3_gpa = vm->stage2_mmu.pgd;
 
diff --git a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
index b1008724f46d..cc49fa0c4293 100644
--- a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
@@ -250,6 +250,45 @@ static void l1_pml_vmx_code(struct vmx_pages *vmx)
 	GUEST_DONE();
 }
 
+static void l1_pml_svm_code(struct svm_test_data *svm)
+{
+	struct vmcb_control_area *ctrl = &svm->vmcb->control;
+	int nr_pml_full = 0;
+
+	/* PML requires NPT. */
+	GUEST_ASSERT(svm->ncr3_gpa);
+
+	generic_svm_setup(svm, l2_pml_guest_code);
+
+	ctrl->pml_addr = svm->pml_gpa;
+	ctrl->pml_index = PML_LOG_NR_ENTRIES;
+	ctrl->misc_ctl |= SVM_MISC_ENABLE_PML;
+
+	run_guest(svm->vmcb, svm->vmcb_gpa);
+
+	for (;;) {
+		if (ctrl->exit_code == SVM_EXIT_VMMCALL)
+			break;
+
+		GUEST_ASSERT_EQ(ctrl->exit_code, SVM_EXIT_PML_FULL);
+		GUEST_ASSERT(++nr_pml_full < PML_FULL_MAX_EXITS);
+
+		ctrl->pml_index = PML_HEAD_INDEX;
+		run_guest(svm->vmcb, svm->vmcb_gpa);
+	}
+
+	GUEST_SYNC_ARGS(ctrl->pml_index, nr_pml_full, 0, 0, 0);
+	GUEST_DONE();
+}
+
+static void l1_pml_guest_code(void *data)
+{
+	if (this_cpu_has(X86_FEATURE_VMX))
+		l1_pml_vmx_code(data);
+	else
+		l1_pml_svm_code(data);
+}
+
 static void check_pml_buffer(struct kvm_vm *vm, u64 pml_gpa, u64 pml_index)
 {
 	u64 *pml_buf = addr_gpa2hva(vm, pml_gpa);
@@ -289,10 +328,13 @@ static void test_nested_pml(void)
 
 	pr_info("Nested TDP: enabled, PML\n");
 
-	vm = vm_create_with_one_vcpu(&vcpu, l1_pml_vmx_code);
+	vm = vm_create_with_one_vcpu(&vcpu, l1_pml_guest_code);
 	vm_enable_tdp(vm);
 
-	pml_gpa = vcpu_alloc_vmx(vm, &nested_gva)->pml_gpa;
+	if (kvm_cpu_has(X86_FEATURE_VMX))
+		pml_gpa = vcpu_alloc_vmx(vm, &nested_gva)->pml_gpa;
+	else
+		pml_gpa = vcpu_alloc_svm(vm, &nested_gva)->pml_gpa;
 
 	vcpu_args_set(vcpu, 1, nested_gva);
 
@@ -428,7 +470,7 @@ int main(int argc, char *argv[])
 	if (kvm_cpu_has_tdp()) {
 		test_dirty_log(/*nested_tdp=*/true);
 
-		if (kvm_cpu_has_vmx_pml())
+		if (kvm_cpu_has_vmx_pml() || kvm_cpu_has(X86_FEATURE_PML))
 			test_nested_pml();
 	}
 
-- 
2.48.1

Re: [RFC PATCH 5/5] KVM: selftests: nSVM: Add nested PML coverage to nested_dirty_log_test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] selftests: kvm: l2_pml_guest_code executes VMX-specific vmcall() on SVM
--

commit cb64ef5e62e8854d8adff372ccbc0426babe6f16
Author: Nikunj A Dadhania <nikunj@amd.com>

KVM: selftests: nSVM: Add nested PML coverage to nested_dirty_log_test

This commit extends the existing nested PML selftest to also cover AMD's
nested Page Modification Logging (nSVM PML) by introducing an SVM-specific
L1 guest code path.

> diff --git a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
> index b1008724f46dc..cc49fa0c42937 100644
> --- a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
> @@ -250,6 +250,45 @@ static void l1_pml_vmx_code(struct vmx_pages *vmx)
>  	GUEST_DONE();
>  }
>  
> +static void l1_pml_svm_code(struct svm_test_data *svm)
> +{
> +	struct vmcb_control_area *ctrl = &svm->vmcb->control;
> +	int nr_pml_full = 0;
> +
> +	/* PML requires NPT. */
> +	GUEST_ASSERT(svm->ncr3_gpa);
> +
> +	generic_svm_setup(svm, l2_pml_guest_code);

[Severity: High]
Could this setup lead to an Invalid Opcode (#UD) exception on AMD systems?

Looking at l2_pml_guest_code(), it appears to unconditionally execute the
vmcall() instruction:

tools/testing/selftests/kvm/x86/nested_dirty_log_test.c:l2_pml_guest_code() {
    ...
    vmcall();
    ...
}

If l2_pml_guest_code() emits the Intel-specific VMCALL instruction, won't
executing VMCALL on AMD systems raise an Invalid Opcode (#UD) exception
instead of causing a VM-exit to L1?

Since generic_svm_setup() does not intercept #UD, does this exception
get delivered to the L2 guest, triggering the host's selftest abort
handler and crashing the nested AMD guest?

> +
> +	ctrl->pml_addr = svm->pml_gpa;
> +	ctrl->pml_index = PML_LOG_NR_ENTRIES;
> +	ctrl->misc_ctl |= SVM_MISC_ENABLE_PML;
> +
> +	run_guest(svm->vmcb, svm->vmcb_gpa);
> +
> +	for (;;) {
> +		if (ctrl->exit_code == SVM_EXIT_VMMCALL)
> +			break;

[Severity: High]
Will this loop exit condition ever be met?

If l2_pml_guest_code() crashes with a #UD exception before completing the
hypercall, L1 won't receive a clean SVM_EXIT_VMMCALL, causing the test
to fail on AMD.

Would it be better to use a generic wrapper (like vmmcall()) in L2 that
handles both VMX and SVM properly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015103933.2088307-1-nikunj@amd.com?part=5