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

[Paolo Bonzini]

Il mer 13 mag 2026, 23:36 Yosry Ahmed <yosry@xxxxxxxxxx> ha scritto:
>
> 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)?

No, cpu_walk is always GVA->(n)GPA and tdp_walk is the optional
nGPA->GPA stage. While there is a 1:1 mapping from a struct kvm_mmu to
kvm_pagewalk when doing shadow paging, for *emulation* purposes
cpu_walk is used for both L1 and L2 and It replaces the
vcpu->arch.walk_mmu pointer from the old code (which led to either
root_mmu or nested_mmu). In fact the main change in the series is the
removal of walk_mmu, with cpu_walk always representing the CR0/CR3/CR4
page tables.

I could call them gva_walk and ngpa_walk, but I think the current name
are also self-explanatory (especially once you understand that
walk_mmu is no more and cpu_walk can be used for both L1 and L2). The
confusion comes more if you look at the walkers from the POV of struct
kvm_mmu. Which leads to the other half...

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

And even more important vcpu->arch.mmu is the pointer to either
root_mmu or guest_mmu. I wouldn't reclaim either mmu or nested_mmu.

guest_mmu is not L2 MMU if L1 does not use two-dimensional paging, so
l1_mmu and l2_mmu does not cut it entirely. And root_mmu can be either
GVA->HPA or GPA->HPA, therefore applying the idea above (e.g., gpa_mmu
and ngpa_mmu) would not work well.

I suppose guest_mmu could be ngpa_mmu or shadow_tdp_mmu, but another
possibility/refactoring would be to adjust the code and call the two
MMUs direct_mmu and shadow_mmu. I haven't looked into what this means
for the code but it would definitely make for the clearest naming.
Having direct_mmu->w be NULL would be nice, so it seems promising but
not something I was going to do soon (the *real* reason to submit this
patch now is to get rid of is_executable_pte() half-assed support for
MBEC/GMET, and that's where I stopped).

Even with the small complication that CR0.PG=0 is a direct mapping but
would use shadow_mmu, it's still a GVA->HPA mapping and thus pretty
understandable.

tl;dr: naming is hard so I tried to change as little as possible in
this respect...

Paolo

>