Re: [PATCH 17/22] KVM: x86/mmu: pull struct kvm_pagewalk out of struct kvm_mmu

[Sean Christopherson]

On Mon, May 18, 2026, Kishen Maloor wrote:
> On 5/13/26 2:36 PM, Yosry Ahmed wrote:
> > > @@ -6767,11 +6748,11 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu)
> > > 
> > >          vcpu->arch.mmu = &vcpu->arch.root_mmu;
> > > 
> > > -       ret = __kvm_mmu_create(vcpu, &vcpu->arch.guest_mmu);
> > > +       ret = __kvm_mmu_create(vcpu, &vcpu->arch.guest_mmu, &vcpu->arch.tdp_walk);
> > >          if (ret)
> > >                  return ret;
> > > 
> > > -       ret = __kvm_mmu_create(vcpu, &vcpu->arch.root_mmu);
> > > +       ret = __kvm_mmu_create(vcpu, &vcpu->arch.root_mmu, &vcpu->arch.cpu_walk);
> > >          if (ret)
> > >                  goto fail_allocate_root;
> > 
> > I admittedly did not take a close look at the series. Just wanted to
> > chime in about naming. I *love* that we are getting rid of nested_mmu
> > because I can never immediately tell the difference between guest_mmu
> > and nested_mmu, so thank you for that.
> > 
> > However, I can't immediately tell what vcpu->arch.cpu_walk is doing
> > either (compared to vcpu->arch.tdp_walk), so maybe the names can be
> > improved? If these walks are tied to these MMUs, maybe name them as
> > such (e.g. root_pg_walk and guest_pg_walk)?

They aren't tied to MMUs in that way though.  If L1 is using shadow paging for L2,
than the so called "root" MMU is used for both L1 and L2, albeit with a different
role (kvm_mmu_page_role.guest_mode will be different).

> > I also think root_mmu and guest_mmu could still use some improvement
> > but that's probably outside the scope of this series. These are
> > essentially L1 MMU and L2 MMU, right?

Almost, but not quite.  See above.

> > Maybe just mmu and nested_mmu could work? 

> > But I am not sure if we can reclaim the nested_mmu name, it's gonna screw
> > with anyone doing backports :/

Heh, could be fun.

> +1, and I wonder if it helps to name the walkers after what they walk, to
> shape one's conceptual model. E.g. cr3_walk (instead of cpu_walk) for the
> walker over CR3-rooted tables of the currently executing guest (L1's or L2's
> CR3), and nested_tdp_walk for the walker over L1's nested EPT/NPT when L0
> services L2. tdp_walk alone felt a bit vague — it read as "the TDP MMU's
> walker," whereas this specifically applies to the nested TDP path.

Or maybe legacy_walk instead of cr3_walk?  Purely because of nCR3 with NPT :-/

Good xxx_mmu names remain elusive...