linux-kernel - Re: [PATCH v3 06/16] KVM: selftests: Introduce struct kvm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aUsXw9m4g-Pn7LtO@google.com>
Date: Tue, 23 Dec 2025 14:29:23 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Yosry Ahmed <yosry.ahmed@...ux.dev>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 06/16] KVM: selftests: Introduce struct kvm_mmu

On Thu, Nov 27, 2025, Yosry Ahmed wrote:
> In preparation for generalizing the virt mapping functions to work with
> TDP page tables, introduce struct kvm_mmu. This struct currently only
> holds the root GPA and number of page table levels. Parameterize virt
> mapping functions by the kvm_mmu, and use the root GPA and page table
> levels instead of hardcoding vm->pgd and vm->pgtable_levels.
> 
> There's a subtle change here, instead of checking that the parent
> pointer is the address of the vm->pgd, check if the value pointed at by
> the parent pointer is the root GPA (i.e. the value of vm->pgd in this
> case). No change in behavior expected.
> 
> Opportunistically, switch the ordering of the checks in the assertion in
> virt_get_pte(), as it makes more sense to check if the parent PTE is the
> root (in which case, not a PTE) before checking the present flag.
> 
> vm->arch.mmu is dynamically allocated to avoid a circular dependency
> chain if kvm_util_arch.h includes processor.h for the struct definition:
> kvm_util_arch.h -> processor.h -> kvm_util.h -> kvm_util_arch.h
> 
> No functional change intended.
> 
> Suggested-by: Sean Christopherson <seanjc@...gle.com>
> Signed-off-by: Yosry Ahmed <yosry.ahmed@...ux.dev>
> ---
>  .../selftests/kvm/include/x86/kvm_util_arch.h |  4 ++
>  .../selftests/kvm/include/x86/processor.h     |  8 ++-
>  .../testing/selftests/kvm/lib/x86/processor.c | 61 +++++++++++++------
>  3 files changed, 53 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86/kvm_util_arch.h b/tools/testing/selftests/kvm/include/x86/kvm_util_arch.h
> index 972bb1c4ab4c..d8808fa33faa 100644
> --- a/tools/testing/selftests/kvm/include/x86/kvm_util_arch.h
> +++ b/tools/testing/selftests/kvm/include/x86/kvm_util_arch.h
> @@ -10,6 +10,8 @@
>  
>  extern bool is_forced_emulation_enabled;
>  
> +struct kvm_mmu;
> +
>  struct kvm_vm_arch {
>  	vm_vaddr_t gdt;
>  	vm_vaddr_t tss;
> @@ -19,6 +21,8 @@ struct kvm_vm_arch {
>  	uint64_t s_bit;
>  	int sev_fd;
>  	bool is_pt_protected;
> +
> +	struct kvm_mmu *mmu;

No, put kvm_mmu in common code and create kvm_vm.mmu.  This makes the "mmu" object
a weird copy of state that's already in kvm_vm (pgd, pgd_created, and pgtable_levels),
and more importantly makes it _way_ to easy to botch the x86 MMU code (speaking
from first hand experience), e.g. due to grabbing vm->pgtable_levels instead of
the mmu's version.  I don't see an easy way to _completely_ guard against goofs
like that, but it's easy-ish to audit code the code for instance of "vm->mmu.",
and adding a common kvm_mmu avoids the weird duplicate code.