diff mbox

[v6,1/7] s390x/cpu: Cleanup init in preparation for hotplug

Message ID 1456160797-832-2-git-send-email-mjrosato@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matthew Rosato Feb. 22, 2016, 5:06 p.m. UTC 
Ensure a valid cpu_model is set upfront by setting the
default value directly into the MachineState when none is
specified.  This is needed to ensure hotplugged CPUs share
the same cpu_model.

Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 2 +-
 hw/s390x/s390-virtio.c     | 8 ++++----
 hw/s390x/s390-virtio.h     | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

Comments

Andreas Färber Feb. 22, 2016, 5:46 p.m. UTC | #1 
Am 22.02.2016 um 18:06 schrieb Matthew Rosato:
> Ensure a valid cpu_model is set upfront by setting the
> default value directly into the MachineState when none is
> specified.  This is needed to ensure hotplugged CPUs share
> the same cpu_model.
> 
> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 2 +-
>  hw/s390x/s390-virtio.c     | 8 ++++----
>  hw/s390x/s390-virtio.h     | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 89f5d0d..b05ed8b 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -136,7 +136,7 @@ static void ccw_init(MachineState *machine)
>      virtio_ccw_register_hcalls();
>  
>      /* init CPUs */
> -    s390_init_cpus(machine->cpu_model);
> +    s390_init_cpus(machine);
>  
>      if (kvm_enabled()) {
>          kvm_s390_enable_css_support(s390_cpu_addr2state(0));
> diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
> index c320878..b576811 100644
> --- a/hw/s390x/s390-virtio.c
> +++ b/hw/s390x/s390-virtio.c
> @@ -95,12 +95,12 @@ void s390_init_ipl_dev(const char *kernel_filename,
>      qdev_init_nofail(dev);
>  }
>  
> -void s390_init_cpus(const char *cpu_model)
> +void s390_init_cpus(MachineState *machine)
>  {
>      int i;
>  
> -    if (cpu_model == NULL) {
> -        cpu_model = "host";
> +    if (machine->cpu_model == NULL) {
> +        machine->cpu_model = "host";

When/why is cpu_model == NULL? Could you simply set it as a default in
your machine's instance_init?

Regards,
Andreas

>      }
>  
>      ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
> @@ -109,7 +109,7 @@ void s390_init_cpus(const char *cpu_model)
>          S390CPU *cpu;
>          CPUState *cs;
>  
> -        cpu = cpu_s390x_init(cpu_model);
> +        cpu = cpu_s390x_init(machine->cpu_model);
>          cs = CPU(cpu);
>  
>          ipi_states[i] = cpu;
> diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h
> index eebce8e..ffd014c 100644
> --- a/hw/s390x/s390-virtio.h
> +++ b/hw/s390x/s390-virtio.h
> @@ -19,7 +19,7 @@
>  typedef int (*s390_virtio_fn)(const uint64_t *args);
>  void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn);
>  
> -void s390_init_cpus(const char *cpu_model);
> +void s390_init_cpus(MachineState *machine);
>  void s390_init_ipl_dev(const char *kernel_filename,
>                         const char *kernel_cmdline,
>                         const char *initrd_filename,
>
Matthew Rosato Feb. 22, 2016, 8:30 p.m. UTC | #2 
On 02/22/2016 12:46 PM, Andreas Färber wrote:
> Am 22.02.2016 um 18:06 schrieb Matthew Rosato:
>> Ensure a valid cpu_model is set upfront by setting the
>> default value directly into the MachineState when none is
>> specified.  This is needed to ensure hotplugged CPUs share
>> the same cpu_model.
>>
>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
>> ---
>>  hw/s390x/s390-virtio-ccw.c | 2 +-
>>  hw/s390x/s390-virtio.c     | 8 ++++----
>>  hw/s390x/s390-virtio.h     | 2 +-
>>  3 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 89f5d0d..b05ed8b 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -136,7 +136,7 @@ static void ccw_init(MachineState *machine)
>>      virtio_ccw_register_hcalls();
>>  
>>      /* init CPUs */
>> -    s390_init_cpus(machine->cpu_model);
>> +    s390_init_cpus(machine);
>>  
>>      if (kvm_enabled()) {
>>          kvm_s390_enable_css_support(s390_cpu_addr2state(0));
>> diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
>> index c320878..b576811 100644
>> --- a/hw/s390x/s390-virtio.c
>> +++ b/hw/s390x/s390-virtio.c
>> @@ -95,12 +95,12 @@ void s390_init_ipl_dev(const char *kernel_filename,
>>      qdev_init_nofail(dev);
>>  }
>>  
>> -void s390_init_cpus(const char *cpu_model)
>> +void s390_init_cpus(MachineState *machine)
>>  {
>>      int i;
>>  
>> -    if (cpu_model == NULL) {
>> -        cpu_model = "host";
>> +    if (machine->cpu_model == NULL) {
>> +        machine->cpu_model = "host";
> 
> When/why is cpu_model == NULL? Could you simply set it as a default in
> your machine's instance_init?
> 

Yes, we definitely did this in an earlier version. I'll set this during
machine instance_init.

Matt
diff mbox

Patch

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 89f5d0d..b05ed8b 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -136,7 +136,7 @@  static void ccw_init(MachineState *machine)
     virtio_ccw_register_hcalls();
 
     /* init CPUs */
-    s390_init_cpus(machine->cpu_model);
+    s390_init_cpus(machine);
 
     if (kvm_enabled()) {
         kvm_s390_enable_css_support(s390_cpu_addr2state(0));
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index c320878..b576811 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -95,12 +95,12 @@  void s390_init_ipl_dev(const char *kernel_filename,
     qdev_init_nofail(dev);
 }
 
-void s390_init_cpus(const char *cpu_model)
+void s390_init_cpus(MachineState *machine)
 {
     int i;
 
-    if (cpu_model == NULL) {
-        cpu_model = "host";
+    if (machine->cpu_model == NULL) {
+        machine->cpu_model = "host";
     }
 
     ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
@@ -109,7 +109,7 @@  void s390_init_cpus(const char *cpu_model)
         S390CPU *cpu;
         CPUState *cs;
 
-        cpu = cpu_s390x_init(cpu_model);
+        cpu = cpu_s390x_init(machine->cpu_model);
         cs = CPU(cpu);
 
         ipi_states[i] = cpu;
diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h
index eebce8e..ffd014c 100644
--- a/hw/s390x/s390-virtio.h
+++ b/hw/s390x/s390-virtio.h
@@ -19,7 +19,7 @@ 
 typedef int (*s390_virtio_fn)(const uint64_t *args);
 void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn);
 
-void s390_init_cpus(const char *cpu_model);
+void s390_init_cpus(MachineState *machine);
 void s390_init_ipl_dev(const char *kernel_filename,
                        const char *kernel_cmdline,
                        const char *initrd_filename,