Re: [Qemu-devel] [PATCH RFC 7/7] target-arm: Embed CPUARMState in QOM ARMCPU

["Andreas Färber" <afaerber@suse.de>]

Am 30.01.2012 03:22, schrieb Anthony Liguori:
> On 01/29/2012 07:25 AM, Andreas Färber wrote:
>> +#define ENV_GET_OBJECT(e) \
>> +    (Object *)((void *)(e) - offsetof(ARMCPU, env))
> 
> sizeof(CPU) should be sizeof(void *).

Not following... CPU is a struct, so:
sizeof(ARMCPU) > sizeof(CPU) >= sizeof(Object) == 2x sizeof(void *)

The above formula is calculating the address of the ARMCPU object the
CPUState *env pointer belongs to. Maybe I should make it an inline
function for readability, I just followed the QOM naming conventions.

> Presumably it's okay to add 8
> bytes to the beginning of CPUState?

The whole problem with today's CPUState is that CPU_COMMON fields are at
the back or in the middle, with target-specific offset from the pointer.
We can of course add an Object *obj field to the start of each
CPU*State, but rth's (and Peter's?) worry was that adding things to the
front may push fields in a (TCG) hot path out of range for
immediate-offset load/stores. Thus if we just do it in the back for ARM
I don't see a benefit over the above solution. Per target we know the
offset.

Fields in the front of CPUState are usually
memset(env, 0, offsetof(CPUState, breakpoints));
on reset - one issue this series is working towards cleaning up. :) If
we add an Object* to the front we also have to save and restore it on
reset per target.

Again, the idea here is an interim solution to get moving in the right
direction. Things that are target-specific should not be accessed from
generic code but indirected through CPUClass methods, dispatching to
target code (e.g., reset), at some point obsoleting the aliased
cpu_reset() functions with one real function in cpu.c.
If it turns out that the TLB is common code modulo target_ulong or so
then it may be moved from CPU*State to CPU, making common code work with
CPU rather than CPUState. Long-term, today's CPUState would remain
what's needed for TCG and it's AREG0 + offset(CPUState, x) calculations.

Regards,
Andreas

> If so, that would make things much nicer from a QOM perspective.
> 
> Regards,
> 
> Anthony Liguori

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg