Re: [RFC PATCH] hmp: implement hmp_ram_memory_dump

[Dr. David Alan Gilbert]

* Alex Bennée (alex.bennee@linaro.org) wrote:
> While reviewing the tb-stats series I was confused by the different
> between physical and ram address. This implements the RAM dump so I
> can replicate the disassembly of "info tb". Whether or not that is a
> good idea remains to be discussed.

Do you have the definition of what the address space for info tb is?


> Based-on: 20231003183058.1639121-1-richard.henderson@linaro.org
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/monitor/hmp-target.h |  1 +
>  monitor/hmp-cmds-target.c    | 10 ++++++++++
>  hmp-commands.hx              | 19 ++++++++++++++++++-
>  3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
> index 730507bd65..a3fa7dc089 100644
> --- a/include/monitor/hmp-target.h
> +++ b/include/monitor/hmp-target.h
> @@ -50,6 +50,7 @@ void hmp_info_sgx(Monitor *mon, const QDict *qdict);
>  void hmp_info_via(Monitor *mon, const QDict *qdict);
>  void hmp_memory_dump(Monitor *mon, const QDict *qdict);
>  void hmp_physical_memory_dump(Monitor *mon, const QDict *qdict);
> +void hmp_ram_memory_dump(Monitor *mon, const QDict *qdict);
>  void hmp_info_registers(Monitor *mon, const QDict *qdict);
>  void hmp_gva2gpa(Monitor *mon, const QDict *qdict);
>  void hmp_gpa2hva(Monitor *mon, const QDict *qdict);
> diff --git a/monitor/hmp-cmds-target.c b/monitor/hmp-cmds-target.c
> index 476cf68e81..21ce790502 100644
> --- a/monitor/hmp-cmds-target.c
> +++ b/monitor/hmp-cmds-target.c
> @@ -253,6 +253,16 @@ void hmp_physical_memory_dump(Monitor *mon, const QDict 
> *qdict)
>      memory_dump(mon, count, format, size, addr, MON_DISAS_GPA);
>  }
>  
> +void hmp_ram_memory_dump(Monitor *mon, const QDict *qdict)
> +{
> +    int count = qdict_get_int(qdict, "count");
> +    int format = qdict_get_int(qdict, "format");

How does that does match your described '/countformatsize' syntax
you list below?
This just looks like 3 separate parameters; but I guess that's a copy.


> +    int size = qdict_get_int(qdict, "size");
> +    hwaddr addr = qdict_get_int(qdict, "addr");
> +
> +    memory_dump(mon, count, format, size, addr, MON_DISAS_GRA);

OK, so really the only difference is that flag.


> +}
> +
>  void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp)
>  {
>      Int128 gpa_region_size;
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e1d78ab69d..5a9ecefdcb 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -471,7 +471,10 @@ ERST
>  
>  SRST
>  ``x/``\ *fmt* *addr*
> -  Virtual memory dump starting at *addr*.
> +  Virtual memory dump starting at *addr*. See xp/ for physical memory and 
> xr/ for ram
> +
> +  *fmt* is a format which tells the command how to format the
> +  data. Its syntax is: ``/{count}{format}{size}``
>  ERST
>  
>      {
> @@ -530,7 +533,21 @@ SRST
>      0x000b8090: 0x0720 0x0720 0x0720 0x0720 0x0720 0x0720 0x0720 0x0720
>  
>  ERST
> +    {
> +        .name       = "xr",
> +        .args_type  = "fmt:/,addr:l",
> +        .params     = "/fmt addr",
> +        .help       = "ram memory dump starting at 'addr'",
> +        .cmd        = hmp_ram_memory_dump,
> +    },
> +
> +SRST
> +``xr /``\ *fmt* *addr*
> +  RAM memory dump starting at *addr*. See xp/ for physical and x/ for 
> virtual memory
>  
> +  *fmt* is a format which tells the command how to format the
> +  data. Its syntax is: ``/{count}{format}{size}``
> +ERST
>      {
>          .name       = "gpa2hva",
>          .args_type  = "addr:l",
> -- 
> 2.39.2
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/