This version of the 'info pg' command adopts Peter Maydell's request
to write some guest-agnostic page table iterator and accessor code,
along with architecture-specific hooks.  The first patch in this
series contributes a generic page table iterator and an x86
instantiation.  As a client, we first introduce an 'info pg' monitor
command, as well as a compressing callback hook for creating succinct
page table representations.

After this, each successive patch replaces an exisitng x86 page table
walker with a use of common iterator code.

I could use advice on how to ensure this is sufficiently well tested.
I used 'make check' and 'make check-avocado', which both pass; what is
the typical standard for testing something like a page table related
change?

As far as generality, I have only attempted this on x86, but I expect
the design would work for any similar radix-tree style page table.

I am still new enough to the code base that I wasn't certain about
where to put the generic code, as well as naming conventions.

Per David Gilbert's suggestion, I was careful to ensure that monitor
calls do not perturb TLB state (see the read-only flag in some
functions).

I appreciate Nadav's suggestion about other ways to pursue the same
goal: I ended up deciding I would like to try my hand at consolidating
the x86 page table code.

Don Porter (6):
  Add an "info pg" command that prints the current page tables
  Convert 'info tlb' to use generic iterator
  Convert 'info mem' to use generic iterator
  Convert x86_cpu_get_memory_mapping() to use generic iterators
  Move tcg implementation of x86 get_physical_address into common helper
    code.
  Convert x86_mmu_translate() to use common code.

 hmp-commands-info.hx                 |  26 +
 include/monitor/hmp-target.h         |   1 +
 target/i386/arch_memory_mapping.c    | 735 +++++++++++++++-------
 target/i386/cpu.h                    |  66 ++
 target/i386/helper.c                 | 518 ++++++++++++----
 target/i386/monitor.c                | 883 +++++++++++++--------------
 target/i386/tcg/sysemu/excp_helper.c | 555 +----------------
 7 files changed, 1439 insertions(+), 1345 deletions(-)

--
2.34.1

On Fri, 24 May 2024 at 18:08, Don Porter <porter@cs.unc.edu> wrote:
>
> This version of the 'info pg' command adopts Peter Maydell's request
> to write some guest-agnostic page table iterator and accessor code,
> along with architecture-specific hooks.  The first patch in this
> series contributes a generic page table iterator and an x86
> instantiation.  As a client, we first introduce an 'info pg' monitor
> command, as well as a compressing callback hook for creating succinct
> page table representations.
>
> After this, each successive patch replaces an exisitng x86 page table
> walker with a use of common iterator code.
>
> I could use advice on how to ensure this is sufficiently well tested.
> I used 'make check' and 'make check-avocado', which both pass; what is
> the typical standard for testing something like a page table related
> change?
>
> As far as generality, I have only attempted this on x86, but I expect
> the design would work for any similar radix-tree style page table.
>
> I am still new enough to the code base that I wasn't certain about
> where to put the generic code, as well as naming conventions.
>
> Per David Gilbert's suggestion, I was careful to ensure that monitor
> calls do not perturb TLB state (see the read-only flag in some
> functions).
>
> I appreciate Nadav's suggestion about other ways to pursue the same
> goal: I ended up deciding I would like to try my hand at consolidating
> the x86 page table code.
>
> Don Porter (6):
>   Add an "info pg" command that prints the current page tables
>   Convert 'info tlb' to use generic iterator
>   Convert 'info mem' to use generic iterator
>   Convert x86_cpu_get_memory_mapping() to use generic iterators
>   Move tcg implementation of x86 get_physical_address into common helper
>     code.
>   Convert x86_mmu_translate() to use common code.

Thanks for doing this work -- I like the diffstats for patches 2 and
3 a lot :-)

I have been digging out from under a big backlog of unreviewed
patches, but in an ideal world I might get to this next week.
Hopefully the x86 maintainers will have a look at it too.

thanks
-- PMM

On Fri, 31 May 2024 at 14:48, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 24 May 2024 at 18:08, Don Porter <porter@cs.unc.edu> wrote:
> >
> > This version of the 'info pg' command adopts Peter Maydell's request
> > to write some guest-agnostic page table iterator and accessor code,
> > along with architecture-specific hooks.  The first patch in this
> > series contributes a generic page table iterator and an x86
> > instantiation.  As a client, we first introduce an 'info pg' monitor
> > command, as well as a compressing callback hook for creating succinct
> > page table representations.
> >
> > After this, each successive patch replaces an exisitng x86 page table
> > walker with a use of common iterator code.
> >
> > I could use advice on how to ensure this is sufficiently well tested.
> > I used 'make check' and 'make check-avocado', which both pass; what is
> > the typical standard for testing something like a page table related
> > change?
> >
> > As far as generality, I have only attempted this on x86, but I expect
> > the design would work for any similar radix-tree style page table.
> >
> > I am still new enough to the code base that I wasn't certain about
> > where to put the generic code, as well as naming conventions.
> >
> > Per David Gilbert's suggestion, I was careful to ensure that monitor
> > calls do not perturb TLB state (see the read-only flag in some
> > functions).
> >
> > I appreciate Nadav's suggestion about other ways to pursue the same
> > goal: I ended up deciding I would like to try my hand at consolidating
> > the x86 page table code.
> >
> > Don Porter (6):
> >   Add an "info pg" command that prints the current page tables
> >   Convert 'info tlb' to use generic iterator
> >   Convert 'info mem' to use generic iterator
> >   Convert x86_cpu_get_memory_mapping() to use generic iterators
> >   Move tcg implementation of x86 get_physical_address into common helper
> >     code.
> >   Convert x86_mmu_translate() to use common code.
>
> Thanks for doing this work -- I like the diffstats for patches 2 and
> 3 a lot :-)
>
> I have been digging out from under a big backlog of unreviewed
> patches, but in an ideal world I might get to this next week.
> Hopefully the x86 maintainers will have a look at it too.

PS: forgot to mention, but if you eventually send a v2 of this
series, could you send it as its own fresh email, not as
a reply to an existing email like this series? Patches and
patchsets that are replies to existing threads tend to get
overlooked, and our automated patch tooling doesn't spot them
either.

thanks
-- PMM