Re: [RFC v12 27/65] target/arm: split a15 cpu model and 32bit class functions to cpu32.c

[Claudio Fontana]

On 4/8/21 12:23 PM, Claudio Fontana wrote:
> On 3/28/21 6:18 PM, Richard Henderson wrote:
>> On 3/26/21 1:36 PM, Claudio Fontana wrote:
>>> provide helper functions there to initialize 32bit models,
>>> and export the a15 cpu model.
>>>
>>> We still need to keep around a15 until we sort out the board configurations.
>>>
>>> cpu.c will continue to contain the common parts between
>>> 32 and 64.
>>>
>>> Note that we need to build cpu32 also for TARGET_AARCH64,
>>> because qemu-system-aarch64 is supposed to be able to run
>>> non-aarch64 cpus too.
>>>
>>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>>
>> Dump state has nothing to do with a15 or the 32-bit models.
> 
> The relationship I see here is that 32-bit builds,
> 
> only really need to use aarch32 version of the dump state, and they do not 
> need to see the aarch64 version.
> 
>> Moving that should be a separate change.
>>
>> The gdbstub changes are also a separate change, afaics.
> 
> 


To hopefully clarify things a bit more here,

the current hierarchy seems not right for ARM CPUs also in this respect.

TYPE_ARM_CPU is the ancestor of all ARM CPUs, ok.
TYPE_AARCH64_CPU is child of TYPE_ARM_CPU, ok.

There is however no TYPE_AARCH32_CPU, or TYPE_ARM32_CPU.

So what ends up being necessary here (both in the mainline code and in this 
patch, which just makes it more explicit),
is to make the ancestor TYPE_ARM_CPU effectively a 32bit CPU class, with 
TYPE_AARCH64_CPU overriding class methods in order to morph it into a 64bit CPU 
class.

What this patch does is to make it explicit, by creating a cpu32.c module that 
contains this non-explicit 32bit ARM CPU class methods, and the registration 
function to register 32bit ARM cpus.

Thanks,

Claudio

> But the main concern is to split more, I understand: will do.
> 
>>
>> I don't understand the a15 comment above.  Is it really relevant to moving 
>> the 
>> 32-bit cpu models?
> 
> 
> The point there was that we keep around a15 for KVM for the moment, instead 
> of relegating it to the set of "tcg-only" models,
> so that virt board and qtest continue to function also in the KVM-only build.
> 
> Mainly for this code here a question from my side: is the current code 
> actually already "wrong"?
> 
> I mean, we unconditionally set the aarch64-capable cpu classes to all use 
> aarch64_gdb_arch_name and gdbstub64,
> but what about an aarch64-capable cpu running in 32bit mode?
> 
> Why don't we have, like tentatively done here for arm_cpu_dump_state,
> 
> an arm_gdb_arch_name and an arm_cpu_gdb_read_register that check is_a64() and 
> call aaarch32_cpu_gdb_read_register or aarch64_cpu_gdb_read_register 
> accordingly?
> 
> 
>>
>>
>> r~
>>
>