Re: [Qemu-ppc] [RFC/PATCH] monitor/ppc: Access all SPRs from the monitor

[Peter Maydell]

On 27 September 2015 at 07:31, Benjamin Herrenschmidt
<address@hidden> wrote:
> We already have a table with all supported SPRs along with their names,
> so let's use that rather than a duplicate table that is perpetually
> out of sync in the monitor code.
>
> This adds a new monitor hook target_extra_monitor_def() which is called
> if nothing is found is the normal table. We still use the old mechanism
> for anything that isn't an SPR.
>
> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
> ---
>  include/monitor/hmp-target.h     |  1 +
>  monitor.c                        |  8 +++-
>  stubs/Makefile.objs              |  1 +
>  stubs/target-extra-monitor-def.c | 10 +++++
>  target-ppc/monitor.c             | 93 
> +++++++++-------------------------------
>  5 files changed, 39 insertions(+), 74 deletions(-)
>  create mode 100644 stubs/target-extra-monitor-def.c
>
> diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
> index 213566c..b946e32 100644
> --- a/include/monitor/hmp-target.h
> +++ b/include/monitor/hmp-target.h
> @@ -35,6 +35,7 @@ struct MonitorDef {
>  };
>
>  const MonitorDef *target_monitor_defs(void);
> +int target_extra_monitor_def(uint64_t *pval, const char *name);

This would be a good place to put a doc comment documenting
the semantics of this new hook.

MonitorDef structs treat the value to be obtained as
a target_long, but this uses uint64_t, which is a bit
inconsistent.

It might be better to:
 (a) fix the core monitor code to deal in int64_t rather
 than target_long
 (b) consider whether it would be better to have the ppc
 code generate a bunch of MonitorDef structs to return for the
 SPRs rather than having an extra hook function

> --- /dev/null
> +++ b/stubs/target-extra-monitor-def.c
> @@ -0,0 +1,10 @@
> +#include "stddef.h"
> +#include "qemu/typedefs.h"
> +#include <stdint.h>
> +
> +int target_extra_monitor_def(uint64_t *pval, const char *name);
> +
> +int target_extra_monitor_def(uint64_t *pval, const char *name)
> +{
> +    return -1;
> +}

It would be better to put the prototype for the hook somewhere
the stub file can include it rather than having it just rewritten
here.

thanks
-- PMM