Re: [Qemu-devel] [PATCH 01/19] exec: split cpu_exec_init()

[Laurent Vivier]

On 05/10/2016 22:13, Peter Maydell wrote:
> On 5 October 2016 at 12:38, Laurent Vivier <address@hidden> 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 <address@hidden>
>> Signed-off-by: Laurent Vivier <address@hidden>
>> ---
>>  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