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 <cfont...@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~ >> >