On Thu, Jan 6, 2022 at 12:34 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Mon, Dec 13, 2021, David Matlack wrote:
> > Instead of passing a pointer to the root page table and the root level
> > separately, pass in a pointer to the kvm_mmu_page that backs the root.
> > This reduces the number of arguments by 1, cutting down on line lengths.
> >
> > No functional change intended.
> >
> > Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>
> > Reviewed-by: Ben Gardon <bgardon@xxxxxxxxxx>
> > ---
> > arch/x86/kvm/mmu/tdp_iter.c | 5 ++++-
> > arch/x86/kvm/mmu/tdp_iter.h | 10 +++++-----
> > arch/x86/kvm/mmu/tdp_mmu.c | 14 +++++---------
> > 3 files changed, 14 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/x86/kvm/mmu/tdp_iter.c b/arch/x86/kvm/mmu/tdp_iter.c
> > index b3ed302c1a35..92b3a075525a 100644
> > --- a/arch/x86/kvm/mmu/tdp_iter.c
> > +++ b/arch/x86/kvm/mmu/tdp_iter.c
> > @@ -39,9 +39,12 @@ void tdp_iter_restart(struct tdp_iter *iter)
> > * Sets a TDP iterator to walk a pre-order traversal of the paging structure
> > * rooted at root_pt, starting with the walk to translate next_last_level_gfn.
> > */
> > -void tdp_iter_start(struct tdp_iter *iter, u64 *root_pt, int root_level,
> > +void tdp_iter_start(struct tdp_iter *iter, struct kvm_mmu_page *root,
> > int min_level, gfn_t next_last_level_gfn)
> > {
> > + u64 *root_pt = root->spt;
> > + int root_level = root->role.level;
>
> Uber nit, arch/x86/ prefers reverse fir tree, though I've yet to get Paolo fully
> on board :-)
>
> But looking at the usage of root_pt, even better would be
>
> void tdp_iter_start(struct tdp_iter *iter, struct kvm_mmu_page *root,
> int min_level, gfn_t next_last_level_gfn)
> {
> int root_level = root->role.level;
>
> WARN_ON(root_level < 1);
> WARN_ON(root_level > PT64_ROOT_MAX_LEVEL);
>
> iter->next_last_level_gfn = next_last_level_gfn;
> iter->root_level = root_level;
> iter->min_level = min_level;
> iter->pt_path[iter->root_level - 1] = (tdp_ptep_t)root->spt;
> iter->as_id = kvm_mmu_page_as_id(root);
>
> tdp_iter_restart(iter);
> }
>
> to avoid the pointless sptep_to_sp() and eliminate the motivation for root_pt.
Will do.
