Am 15.03.2012 17:15, schrieb Lluís Vilanova:
> Andreas Färber writes:
>
>> Use CPUX86State etc. instead (hand-converted).
>
> Is there any reason not to move this into CPU${arch}State and instead provide
> virtual methods in CPUState?
Yes, a very simple one: This is one of the prerequisite patches to
having a QOM CPUState struct we can place such virtual methods in.
Originally, not even reset was in there, but there were comments that my
series was getting too long, so I moved it out of the ARM series and
squashed it into what is now patch 43.
> For example (a CPUState argument is implicit on all methods):
>
> int reg_get_number(const char *regname);
> const char* reg_get_name(int regnum);
>
> /* register validity for a specific instantiation must be chekced somehow
> */
> bool reg_valid(int regnum);
>
> size_t reg_get_size(int regnum);
>
> /* multiple sizes must be handled somehow */
> /* precondition: get_reg_validity(regnum) == true */
> void reg_get_value(int regnum, void *buf);
> void reg_set_value(int regnum, void *buf);
>
>
> This way, target-specific code would be moved where it belongs, partially
> merging (for example) code from both monitor.c, gdbstub.c and the target
> itself.
That is definitely the direction I want to go with QOM CPUs. I haven't
dug into gdbstub or monitor too deeply yet, beyond looking for ways to
make the CPUState -> CPUArchState conversion as small as possible.
In this case, I am not sure why the original authors chose not to place
the code into target-*. Maybe those reasons no longer apply.
Anyway, once all CPUs are converted - or if you're adventurous based on
my qom-cpu-wip branch - I'd be happy to review patches moving more stuff
into CPUState. :) Moving properties from CPUArchState into CPUState
seemed fairly mechanical, so I attacked the more challenging problem of
statically calculated offsets for TCG's icount first.
I'd also envision some of the #defines we have flying around in cpu.h to
turn into read-only CPU properties, like page size or number of MMU
modes, so that the CPU becomes more self-describing and doesn't pollute
the global namespace.
> The downside is that using pointers to set/get the value is pretty
> uncomfortable.
We can add cpu_...(CPUState, ...) wrapper functions, like cpu_reset().
Cheers,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
