[5/8,RFC,v2] s390-qemu: cpu hotplug - Introduce post-cpu-init function

From: "Jason J. Herne" <jjherne@us.ibm.com>

In preparation for treating cpus as devices we need to separate machine
initialization into two stages:
1. Initialization that needs to be done before cpu devices can be created.
2. Initialization that requires cpu devices to already be created.

This is accomplished by creating an optional post-cpu initialization function
for QEMUMachine.

Signed-off-by: Jason J. Herne <jjherne@us.ibm.com>
---
 include/hw/boards.h |    3 ++-
 vl.c                |    4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

Am 07.06.2013 19:28, schrieb Jason J. Herne:
> From: "Jason J. Herne" <jjherne@us.ibm.com>
> 
> In preparation for treating cpus as devices

CPUs *are* devices since multiple releases now, so this is badly put.

> we need to separate machine
> initialization into two stages:
> 1. Initialization that needs to be done before cpu devices can be created.
> 2. Initialization that requires cpu devices to already be created.
> 
> This is accomplished by creating an optional post-cpu initialization function
> for QEMUMachine.

Whatever you are using it for, this sounds wrong to me.

Machine init is supposed to use less code and more QOM infrastructure,
with a future goal of replacing most code with a config file
instantiating and wiring up devices.

And please don't forget to CC me on the next CPU series.

Regards,
Andreas

> 
> Signed-off-by: Jason J. Herne <jjherne@us.ibm.com>
> ---
>  include/hw/boards.h |    3 ++-
>  vl.c                |    4 ++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index fb7c6f1..ed427a1 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -19,7 +19,7 @@ typedef struct QEMUMachineInitArgs {
>  } QEMUMachineInitArgs;
>  
>  typedef void QEMUMachineInitFunc(QEMUMachineInitArgs *args);
> -
> +typedef void QEMUMachineInitPostCpusFunc(void);
>  typedef void QEMUMachineResetFunc(void);
>  
>  typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp);
> @@ -29,6 +29,7 @@ typedef struct QEMUMachine {
>      const char *alias;
>      const char *desc;
>      QEMUMachineInitFunc *init;
> +    QEMUMachineInitPostCpusFunc *post_cpu_init;
>      QEMUMachineResetFunc *reset;
>      QEMUMachineHotAddCPUFunc *hot_add_cpu;
>      BlockInterfaceType block_default_type;
> diff --git a/vl.c b/vl.c
> index 47ab45d..71e1e6d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4305,6 +4305,10 @@ int main(int argc, char **argv, char **envp)
>                                   .cpu_model = cpu_model };
>      machine->init(&args);
>  
> +    if (machine->post_cpu_init) {
> +        machine->post_cpu_init();
> +    }
> +
>      audio_init();
>  
>      cpu_synchronize_all_post_init();

On 06/08/2013 06:10 PM, Andreas Färber wrote:
> Am 07.06.2013 19:28, schrieb Jason J. Herne:
>> From: "Jason J. Herne" <jjherne@us.ibm.com>
>>
>> In preparation for treating cpus as devices
>
> CPUs *are* devices since multiple releases now, so this is badly put.
>
>> we need to separate machine
>> initialization into two stages:
>> 1. Initialization that needs to be done before cpu devices can be created.
>> 2. Initialization that requires cpu devices to already be created.
>>
>> This is accomplished by creating an optional post-cpu initialization function
>> for QEMUMachine.
>
> Whatever you are using it for, this sounds wrong to me.
>

The QEMUMachine->init() function (at least for S390) currently handles 
several tasks.  One of those tasks is the creation of cpus.  If we are 
switching to a new paradigm where QOM cpu devices are parsed and created 
in main() then QEMUMachine->init() will happen either before or after 
cpus are created.  This change is meant to split QEMUMachine->init() 
into two parts

1. Stuff that does not depend on cpu creation. Specifically, stuff that 
might be a dependency of cpu create, like allocating ipi_states.

2. Stuff that does depend on cpu creation.  Like 
vm_s390_enable_css_support() which requires CPU 0 to exist.


> Machine init is supposed to use less code and more QOM infrastructure,
> with a future goal of replacing most code with a config file
> instantiating and wiring up devices.
>

Duly noted.  I can have another look at the code.  Perhaps there is an 
easy place I can move the ipi_state initialization.  Also, perhaps there 
is a way to remove the cpu-0 dependency from 
vm_s390_enable_css_support().  Both of these changes would remove the 
need for the post_cpu_init function.

> And please don't forget to CC me on the next CPU series.
>

Sorry.  I had meant to CC you originally.