[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH RFC qom-cpu 39/41] target-xtensa: Introduce Xten

From: Max Filippov
Subject: Re: [Qemu-devel] [PATCH RFC qom-cpu 39/41] target-xtensa: Introduce XtensaCPU subclasses
Date: Sat, 6 Jul 2013 23:54:28 +0400

On Sat, Jul 6, 2013 at 11:12 PM, Andreas Färber <address@hidden> wrote:
> Am 06.07.2013 20:39, schrieb Max Filippov:
>> On Sat, Jul 6, 2013 at 10:01 PM, Max Filippov <address@hidden> wrote:
>>> On Sat, Jul 6, 2013 at 4:55 PM, Andreas Färber <address@hidden> wrote:
>>>> Am 29.06.2013 22:01, schrieb Andreas Färber:
>>>>> Register a CPU type per core registered. Save the XtensaConfig in
>>>>> XtensaCPUClass instead of CPUXtensaState.
>>>>> Prepares for storing per-class GDB register count.
>>>>> Signed-off-by: Andreas Färber <address@hidden>
>>>> Ping! Can you ack? (It did not seem to break your test image.)
>>> Hi Andreas,
>>> I tried make check -C tests/tcg/xtensa with the branch you recommended
>>> and it segfaults on elf loading
>> ...and maybe a stupid question, but why moving configuration pointer away
>> from env and then changing every place that used to access it?
> Xtensa is the only target trying to implicitly access an "env" variable
> through a macro to obtain the number of registers for gdbstub. That's
> what I'm trying to fix with 40/41 in order to get rid of the #ifdeffery.
> The number of registers is not accessed by TCG, so it could go into
> XtensaCPU instead of CPUXtensaState.
> Further it does not change during vCPU runtime, so it no longer belongs
> in CPUXtensaState nor XtensaCPU but in XtensaCPUClass, which is shared
> among CPU cores and can be accessed statically.

I'm concerned about two things here: adding boilerplate code to access
CPU class and doing more work at runtime than just following a pointer.
I mean that env->config is almost always used together with env itself,
could we consider env->config to be a cache for xcc->config?

> However we only had one XtensaCPUClass but multiple XtensaConfigs.
> Therefore this patch registers one XtensaCPUClass per XtensaConfig.
> You might remember that I once tried to place the XtensaConfig fields
> directly into XtensaCPUClass but that didn't work out nicely back then.
> This is by comparison a slim/minimal conversion to subclasses, leaving
> XtensaConfig and your custom registration mechanisms in place. Cleaning
> that up to use type_init() and type_register_static() in the respective
> source files and changing -cpu ? to iterate QOM types would be a nice
> follow-up, but not needed for this gdbstub refactoring series.
> Another background is that I'm trying to get rid of cpu_init() and
> anything that blocks using -device or device_add with QOM CPUs.

-- Max

reply via email to

[Prev in Thread] Current Thread [Next in Thread]