On 05/10/2016 22:13, Peter Maydell wrote: > On 5 October 2016 at 12:38, Laurent Vivier <lviv...@redhat.com> wrote: >> Extract the realize part to cpu_exec_realize(), update all >> calls to cpu_exec_init() to add cpu_exec_realize() to >> have no functionnal change. >> >> Put in cpu_exec_init() what initializes the CPU, >> in cpu_exec_realize() what adds it to the environment. >> >> CC: Paolo Bonzini <pbonz...@redhat.com> >> Signed-off-by: Laurent Vivier <lviv...@redhat.com> >> --- >> exec.c | 8 +++++--- >> include/exec/exec-all.h | 1 + >> target-alpha/cpu.c | 1 + >> target-arm/cpu.c | 1 + >> target-cris/cpu.c | 1 + >> target-i386/cpu.c | 1 + >> target-lm32/cpu.c | 1 + >> target-m68k/cpu.c | 1 + >> target-microblaze/cpu.c | 1 + >> target-mips/cpu.c | 1 + >> target-moxie/cpu.c | 1 + >> target-openrisc/cpu.c | 1 + >> target-ppc/translate_init.c | 5 +++++ >> target-s390x/cpu.c | 4 ++++ >> target-sh4/cpu.c | 1 + >> target-sparc/cpu.c | 1 + >> target-tilegx/cpu.c | 1 + >> target-tricore/cpu.c | 1 + >> target-unicore32/cpu.c | 1 + >> target-xtensa/cpu.c | 1 + >> 20 files changed, 31 insertions(+), 3 deletions(-) >> >> diff --git a/exec.c b/exec.c >> index c8389f9..95b0aee 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -614,9 +614,6 @@ void cpu_exec_exit(CPUState *cpu) >> >> void cpu_exec_init(CPUState *cpu, Error **errp) >> { >> - CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu); >> - Error *local_err ATTRIBUTE_UNUSED = NULL; >> - >> cpu->as = NULL; >> cpu->num_ases = 0; >> >> @@ -637,6 +634,11 @@ void cpu_exec_init(CPUState *cpu, Error **errp) >> cpu->memory = system_memory; >> object_ref(OBJECT(cpu->memory)); >> #endif >> +} >> + >> +void cpu_exec_realize(CPUState *cpu, Error **errp) >> +{ >> + CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu); >> >> cpu_list_add(cpu); > > I think cpu_list_add() needs to be in init, because > this is where we set cpu->cpu_index, and (after patch 5) > target-arm assumes that cpu_index has been set by > init but before realize. So I guess we should do it > in init and then roll back in the destructor?
I think in some case cpu_list_add() can fail (no more index available), it's why it has to be in realize. In the case of hotplug we must not kill the machine. See "5a790cc cpu: Add Error argument to cpu_exec_init()" Laurent