On Tue, Apr 29, 2025 at 02:21:56PM +0100, Peter Maydell wrote: > Currently we call gdb_init_cpu() in cpu_common_initfn(), which is > very early in the CPU object's init->realize creation sequence. In > particular this happens before the architecture-specific subclass's > init fn has even run. This means that gdb_init_cpu() can only do > things that depend strictly on the class, not on the object, because > the CPUState* that it is passed is currently half-initialized. > > In commit a1f728ecc90cf6c6 we accidentally broke this rule, by adding > a call to the gdb_get_core_xml_file method which takes the CPUState. > At the moment we get away with this because the only implementation > doesn't actually look at the pointer it is passed. However the whole > reason we created that method was so that we could make the "which > XML file?" decision based on a property of the CPU object, and we > currently can't change the Arm implementation of the method to do > what we want without causing wrong behaviour or a crash. > > The ordering restrictions here are: > * we must call gdb_init_cpu before: > - any call to gdb_register_coprocessor() > - any use of the gdb_num_regs field (this is only used > in code that's about to call gdb_register_coprocessor() > and wants to know the first register number of the > set of registers it's about to add) > * we must call gdb_init_cpu after CPU properties have been > set, which is to say somewhere in realize > > The function cpu_exec_realizefn() meets both of these requirements, > as it is called by the architecture-specific CPU realize function > early in realize, before any calls ot gdb_register_coprocessor(). > Move the gdb_init_cpu() call to there.
Sounds good to me: Reviewed-by: Edgar E. Iglesias <edgar.igles...@amd.com> > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > hw/core/cpu-common.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c > index 92c40b6bf83..39e674aca21 100644 > --- a/hw/core/cpu-common.c > +++ b/hw/core/cpu-common.c > @@ -234,6 +234,8 @@ bool cpu_exec_realizefn(CPUState *cpu, Error **errp) > return false; > } > > + gdb_init_cpu(cpu); > + > /* Wait until cpu initialization complete before exposing cpu. */ > cpu_list_add(cpu); > > @@ -304,7 +306,6 @@ static void cpu_common_initfn(Object *obj) > /* cache the cpu class for the hotpath */ > cpu->cc = CPU_GET_CLASS(cpu); > > - gdb_init_cpu(cpu); > cpu->cpu_index = UNASSIGNED_CPU_INDEX; > cpu->cluster_index = UNASSIGNED_CLUSTER_INDEX; > cpu->as = NULL; > -- > 2.43.0 >
