Re: [Qemu-devel] [PATCH 01/32] target-arm: initial coprocessor register framework

[Peter Maydell]

On 14 May 2012 11:34, Peter Maydell <address@hidden> wrote:
> On 13 May 2012 23:57, Andreas Färber <address@hidden> wrote:
>> I'm aware this series predates the QOM era, but I'm not really happy how
>> this aligns with my CPUState overhaul. Independent of what needs to be
>> fixed for cpu_copy(), I would like to see new non-TCG fields such as
>> this hashtable added to ARMCPU after the env field, not to the old
>> CPUARMState (using fieldoffset +/- from env is correct though). By
>> consequence the API should be changed to take ARMCPU *cpu rather than
>> CPUARMState *env to avoid the QOM casts that you so loathe and to avoid
>> us refactoring this new API in a few weeks again.
>
> Yes, makes sense; as you say I wrote most of this before the ARMCPU
> changes went in. I think that would allow us to sidestep the cpu_copy()
> issues and only leave us needing to free the hashtable on cpu deletion...

So I've updated to put the hashtable in ARMCPU, and changed the APIs
for "define new register" so they take an ARMCPU* rather than the
CPUARMState*, which all looks good. Do you think the CPReadFn/CPWriteFn/
CPResetFn access function prototypes should also change to take an
ARMCPU*? I could do it, but it would mean that all the implementations
of these functions would just grow an extra "ARMCPUState *env = &cpu->env;"
or equivalent, since they have no need to get at the ARMCPU* but do need
the ARMCPUState*. If that's the thing that makes more long term sense
I'm happy to do it but I thought I'd check before I updated all
these patches :-)

thanks
-- PMM