[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC/PATCH] monitor/ppc: Access all SPRs from the monit
From: |
Benjamin Herrenschmidt |
Subject: |
Re: [Qemu-devel] [RFC/PATCH] monitor/ppc: Access all SPRs from the monitor |
Date: |
Thu, 01 Oct 2015 06:41:09 +1000 |
On Wed, 2015-09-30 at 12:47 +0100, Peter Maydell wrote:
> > 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.
I couldn't get the definition of target_ulong in the stubs.
[ Note: Alexey has a different approach which completely replaces
monitor_defs() with a CPU specific one via a method in the CPU class.
I don't care which way you prefer as long as the functionality is there
]
> 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
That sounds like the most bloated way to handle this :)
> > --- /dev/null
> > +++ b/stubs/target-extra-monitor-def.c
> > @@ -0,0 +1,10 @@
> > +#include "stddef.h"
> > +#include "qemu/typedefs.h"
> > +#include
> > +
> > +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.
I just copied the existing practice in get_monitor_defs()... but yes, I
agree.
Ben.
> thanks
> -- PMM