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

[Kishen Maloor]

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)?
>
> 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? 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 :/

+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.