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

[sashiko-bot@kernel.org]

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