RE: [PATCH v2 3/5] RISC-V: 'info gmem' to show hypervisor guest -> physical address translations

[Schwarz, Konrad]

> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Tuesday, January 4, 2022 23:03
> On Wed, Jan 5, 2022 at 1:55 AM Konrad Schwarz
> <konrad.schwarz@siemens.com> wrote:
> >
> > This is analog to the existing 'info mem' command and is implemented
> > using the same machinery.

> >  hmp-commands-info.hx         |  16 +++++
> >  include/monitor/hmp-target.h |   2 +
> >  target/riscv/monitor.c       | 135 +++++++++++++++++++++++++----------
> >  3 files changed, 117 insertions(+), 36 deletions(-)
> >
> > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> > index 407a1da800..fa519f0129 100644
> > --- a/hmp-commands-info.hx
> > +++ b/hmp-commands-info.hx
> > @@ -237,6 +237,22 @@ SRST
> >      Show the active virtual memory mappings.
> >  ERST
> >
> > +#if defined TARGET_RISCV
> > +    {
> > +        .name       = "gmem",
> > +        .args_type  = "",
> > +        .params     = "",
> > +        .help       = "show the hypervisor guest's physical address"
> > +                   " translation",
> > +        .cmd        = hmp_info_gmem,
> > +    },
> > +#endif
> 
> I don't think we want RISC-V specific commands. Could we not just
> extend `info mem` instead?

Considering that the similar commands `info tlb' and `info mem'
are target dependent (i.e. specific to I386, SH4, SPARC,
PCC, XTENSA and M68K, respectively to I386 and RISC-V),
I honestly do not see a problem here.

Obviously, other architectures are free to add their own implementation
of the `info gmem' functionality, so the list of architectures
supporting this command might grow in future.  The command itself
is not specific to RISC-V.

PS: I will be taking your other points to heart on the re-roll,
I just wanted to get this out of the way before attempting it.

Regards,
Konrad