[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aVKdOU8scPANWb1h@google.com>
Date: Mon, 29 Dec 2025 07:24:41 -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 Tue, Dec 23, 2025, Yosry Ahmed wrote:
> On Tue, Dec 23, 2025 at 02:29:23PM -0800, Sean Christopherson wrote:
> > On Thu, Nov 27, 2025, Yosry Ahmed wrote:
> > > 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.
>
> Do you mean move pgd, pgd_created, and pgtable_levels into kvm_mmu?
Yep, exactly.
> If yes, that makes sense to me and is obviously an improvement over what it's
> in this patch.
>
> I didn't immediately make the connection, but in hindsight it's obvious
> that having some of the state in kvm_vm_arch and some in kvm_vm is
> fragile.
Powered by blists - more mailing lists