Andreas Färber writes:
> 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.
Sure, I meant besides that :)
> 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.
Well, I just saw that when looking at your series.
> 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().
What I meant is that it's much more straightforward to have
type_t v = cpu_reg_get_value(REG_ARCH_REGID);
rather than
type_t v;
cpu_reg_get_value(REG_ARCH_REGID, &v);
The problem is that different registers can have different width. Even if you
decide for uint64_t (which seems reasonable), you can still have wider registers
(e.g., 128bit FP regs). But maybe those "uncommon" wider registers can be
handled separately.
Lluis
--
"And it's much the same thing with knowledge, for whenever you learn
something new, the whole world becomes that much richer."
-- The Princess of Pure Reason, as told by Norton Juster in The Phantom
Tollbooth
