Re: [Qemu-devel] [PATCH next v2 00/74] QOM CPUState, part 3: CPU reset

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

Am 14.05.2012 23:22, schrieb Blue Swirl:
> On Mon, May 14, 2012 at 8:59 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Am 14.05.2012 21:54, schrieb Blue Swirl:
>>> On Mon, May 14, 2012 at 4:15 PM, Andreas Färber <afaerber@suse.de> wrote:
>>>> Am 10.05.2012 02:13, schrieb Andreas Färber:
>>>>> Andreas Färber (74):
>>>> [...]
>>>>>   target-sparc: Let cpu_sparc_init() return SPARCCPU
>>>>>   sun4m: Use cpu_sparc_init() to obtain SPARCCPU
>>>>>   sun4m: Pass SPARCCPU to {main,secondary}_cpu_reset()
>>>>>   sun4u: Use cpu_sparc_init() to obtain SPARCCPU
>>>>>   sun4u: Let cpu_devinit() return SPARCCPU
>>>>>   sun4u: Store SPARCCPU in ResetData
>>>>>   leon3: Use cpu_sparc_init() to obtain SPARCCPU
>>>>>   leon3: Store SPARCCPU in ResetData
>>>> [...]
>>
>> Forgot to mention the bsd-user patch.
>>
>>>>>   Kill off cpu_state_reset()
>>>>
>>>> Ping! Blue, can you ack please?
>>>
>>> What was again the purpose of all these changes, 00/74 only mentions
>>> killing cpu_state_reset()?
[...]
>> Cf. http://wiki.qemu.org/Features/QOM/CPU
>>
>> CPUSPARCState is what still makes having subclasses of SPARCCPU with
>> additional fields tricky - they would get added to the end after some of
>> the large CPU_COMMON fields, so accessing them from TCG gets ugly.
> 
> Can't we move the TLB to common code?

The TLB depends on both target_ulong and target_phys_addr_t, so we get a
matrix of 32/32, 32/64, 64/64 at least, for which we don't have a
common-code compilation mechanism to date.

> Why can't SPARCCPU contain
> everything that CPUSPARCState had minus common items?

The common items I'm working on and it became clearer that cpu_copy() is
broken as it is, so I won't hold back that series any longer.

If we move all fields from CPUSPARCState to SPARCCPU, what pointer would
you pass as AREG0?

In C code a pointer to the start of the CPU would certainly be handier
but it was brought up that any large, rarely needed common fields at the
start (CPUState) would contribute to imm overflows for, e.g., SPARCCPU
fields such as GPRs etc.

> Also, TCG
> softmmu functions could use pointers to CPUTLB directly, then the slow
> path could recover CPUState or others with container_of().

Having messed in TCG code not too long ago, I would appreciate your
help. The current ppc/ppc64 code at least relies on indexed TLB loads
from AREG0 aka env. Pointer dereferences in generated code to get to an
external TLB object might impact performance...

>>> Otherwise the patches look pretty safe ("if it compiles, it works").

Was I supposed to interpret that as an Acked-by? ;)

Thanks,
Andreas

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