[Qemu-devel] [PATCH] s390x: use generic cpu_model parsing

Igor Mammedov posted 1 patch 8 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1505991234-246759-1-git-send-email-imammedo@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
There is a newer version of this series
target/s390x/cpu.h         |  6 ++++--
hw/s390x/s390-virtio-ccw.c | 29 ++++-------------------------
target/s390x/cpu_models.c  | 11 -----------
target/s390x/kvm.c         |  3 +++
4 files changed, 11 insertions(+), 38 deletions(-)
[Qemu-devel] [PATCH] s390x: use generic cpu_model parsing
Posted by Igor Mammedov 8 years, 1 month ago
define default CPU type in generic way in machine class_init
and let common machine code to handle cpu_model parsing.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tested under TCG booting debian kernel,
and compile tested on s390 host

CC: Cornelia Huck <cohuck@redhat.com>
CC: Christian Borntraeger <borntraeger@de.ibm.com>
CC: Alexander Graf <agraf@suse.de>
CC: Richard Henderson <rth@twiddle.net>
---
 target/s390x/cpu.h         |  6 ++++--
 hw/s390x/s390-virtio-ccw.c | 29 ++++-------------------------
 target/s390x/cpu_models.c  | 11 -----------
 target/s390x/kvm.c         |  3 +++
 4 files changed, 11 insertions(+), 38 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 9b549dc..6cc96ae 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -684,12 +684,14 @@ static inline unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu)
 /* cpu_models.c */
 void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 #define cpu_list s390_cpu_list
-const char *s390_default_cpu_model_name(void);
-
 
 /* helper.c */
 #define cpu_init(cpu_model) cpu_generic_init(TYPE_S390_CPU, cpu_model)
 S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp);
+
+#define S390_CPU_TYPE_SUFFIX "-" TYPE_S390_CPU
+#define S390_CPU_TYPE_NAME(name) (name S390_CPU_TYPE_SUFFIX)
+
 /* you can call this signal handler from your SIGBUS and SIGSEGV
    signal handlers to inform the virtual CPU of exceptions. non zero
    is returned if the signal was handled by the virtual CPU.  */
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index fafbc6d..5546d6a 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -55,15 +55,8 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
 static void s390_init_cpus(MachineState *machine)
 {
     MachineClass *mc = MACHINE_GET_CLASS(machine);
-    const char *typename;
-    gchar **model_pieces;
-    ObjectClass *oc;
-    CPUClass *cc;
     int i;
 
-    if (machine->cpu_model == NULL) {
-        machine->cpu_model = s390_default_cpu_model_name();
-    }
     if (tcg_enabled() && max_cpus > 1) {
         error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
                      "supported by TCG (1) on s390x", max_cpus);
@@ -73,25 +66,8 @@ static void s390_init_cpus(MachineState *machine)
     /* initialize possible_cpus */
     mc->possible_cpu_arch_ids(machine);
 
-    model_pieces = g_strsplit(machine->cpu_model, ",", 2);
-    if (!model_pieces[0]) {
-        error_report("Invalid/empty CPU model name");
-        exit(1);
-    }
-
-    oc = cpu_class_by_name(TYPE_S390_CPU, model_pieces[0]);
-    if (!oc) {
-        error_report("Unable to find CPU definition: %s", model_pieces[0]);
-        exit(1);
-    }
-    typename = object_class_get_name(oc);
-    cc = CPU_CLASS(oc);
-    /* after parsing, properties will be applied to all *typename* instances */
-    cc->parse_features(typename, model_pieces[1], &error_fatal);
-    g_strfreev(model_pieces);
-
     for (i = 0; i < smp_cpus; i++) {
-        s390x_new_cpu(typename, i, &error_fatal);
+        s390x_new_cpu(machine->cpu_type, i, &error_fatal);
     }
 }
 
@@ -446,6 +422,9 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
     mc->get_hotplug_handler = s390_get_hotplug_handler;
     mc->cpu_index_to_instance_props = s390_cpu_index_to_props;
     mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids;
+    /* it is overrriden with 'host' cpu type when accel=kvm
+     * i.e. when kvm_arch_init() is executed */
+    mc->default_cpu_type = S390_CPU_TYPE_NAME("qemu");
     hc->plug = s390_machine_device_plug;
     hc->unplug_request = s390_machine_device_unplug_request;
     nc->nmi_monitor_handler = s390_nmi;
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 5169379..0bf7e14 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -1207,9 +1207,6 @@ static void s390_qemu_cpu_model_class_init(ObjectClass *oc, void *data)
                                 qemu_hw_version());
 }
 
-#define S390_CPU_TYPE_SUFFIX "-" TYPE_S390_CPU
-#define S390_CPU_TYPE_NAME(name) (name S390_CPU_TYPE_SUFFIX)
-
 /* Generate type name for a cpu model. Caller has to free the string. */
 static char *s390_cpu_type_name(const char *model_name)
 {
@@ -1232,14 +1229,6 @@ ObjectClass *s390_cpu_class_by_name(const char *name)
     return oc;
 }
 
-const char *s390_default_cpu_model_name(void)
-{
-     if (kvm_enabled()) {
-        return "host";
-     }
-     return "qemu";
-}
-
 static const TypeInfo qemu_s390_cpu_type_info = {
     .name = S390_CPU_TYPE_NAME("qemu"),
     .parent = TYPE_S390_CPU,
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 720cb1d..5dfdd96 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -287,6 +287,9 @@ void kvm_s390_crypto_reset(void)
 
 int kvm_arch_init(MachineState *ms, KVMState *s)
 {
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+
+    mc->default_cpu_type = S390_CPU_TYPE_NAME("host");
     cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
     cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
     cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
-- 
2.7.4


Re: [Qemu-devel] [PATCH] s390x: use generic cpu_model parsing
Posted by Cornelia Huck 8 years, 1 month ago
On Thu, 21 Sep 2017 12:53:54 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

cc: David

> define default CPU type in generic way in machine class_init
> and let common machine code to handle cpu_model parsing.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tested under TCG booting debian kernel,
> and compile tested on s390 host
> 
> CC: Cornelia Huck <cohuck@redhat.com>
> CC: Christian Borntraeger <borntraeger@de.ibm.com>
> CC: Alexander Graf <agraf@suse.de>
> CC: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/cpu.h         |  6 ++++--
>  hw/s390x/s390-virtio-ccw.c | 29 ++++-------------------------
>  target/s390x/cpu_models.c  | 11 -----------
>  target/s390x/kvm.c         |  3 +++
>  4 files changed, 11 insertions(+), 38 deletions(-)

At least, that's a nice diffstat :)

> 
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 9b549dc..6cc96ae 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -684,12 +684,14 @@ static inline unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu)
>  /* cpu_models.c */
>  void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf);
>  #define cpu_list s390_cpu_list
> -const char *s390_default_cpu_model_name(void);
> -
>  
>  /* helper.c */
>  #define cpu_init(cpu_model) cpu_generic_init(TYPE_S390_CPU, cpu_model)
>  S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp);
> +
> +#define S390_CPU_TYPE_SUFFIX "-" TYPE_S390_CPU
> +#define S390_CPU_TYPE_NAME(name) (name S390_CPU_TYPE_SUFFIX)
> +
>  /* you can call this signal handler from your SIGBUS and SIGSEGV
>     signal handlers to inform the virtual CPU of exceptions. non zero
>     is returned if the signal was handled by the virtual CPU.  */
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index fafbc6d..5546d6a 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -55,15 +55,8 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>  static void s390_init_cpus(MachineState *machine)
>  {
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
> -    const char *typename;
> -    gchar **model_pieces;
> -    ObjectClass *oc;
> -    CPUClass *cc;
>      int i;
>  
> -    if (machine->cpu_model == NULL) {
> -        machine->cpu_model = s390_default_cpu_model_name();
> -    }
>      if (tcg_enabled() && max_cpus > 1) {
>          error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
>                       "supported by TCG (1) on s390x", max_cpus);
> @@ -73,25 +66,8 @@ static void s390_init_cpus(MachineState *machine)
>      /* initialize possible_cpus */
>      mc->possible_cpu_arch_ids(machine);
>  
> -    model_pieces = g_strsplit(machine->cpu_model, ",", 2);
> -    if (!model_pieces[0]) {
> -        error_report("Invalid/empty CPU model name");
> -        exit(1);
> -    }
> -
> -    oc = cpu_class_by_name(TYPE_S390_CPU, model_pieces[0]);
> -    if (!oc) {
> -        error_report("Unable to find CPU definition: %s", model_pieces[0]);
> -        exit(1);
> -    }
> -    typename = object_class_get_name(oc);
> -    cc = CPU_CLASS(oc);
> -    /* after parsing, properties will be applied to all *typename* instances */
> -    cc->parse_features(typename, model_pieces[1], &error_fatal);
> -    g_strfreev(model_pieces);
> -
>      for (i = 0; i < smp_cpus; i++) {
> -        s390x_new_cpu(typename, i, &error_fatal);
> +        s390x_new_cpu(machine->cpu_type, i, &error_fatal);
>      }
>  }
>  
> @@ -446,6 +422,9 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
>      mc->get_hotplug_handler = s390_get_hotplug_handler;
>      mc->cpu_index_to_instance_props = s390_cpu_index_to_props;
>      mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids;
> +    /* it is overrriden with 'host' cpu type when accel=kvm
> +     * i.e. when kvm_arch_init() is executed */
> +    mc->default_cpu_type = S390_CPU_TYPE_NAME("qemu");
>      hc->plug = s390_machine_device_plug;
>      hc->unplug_request = s390_machine_device_unplug_request;
>      nc->nmi_monitor_handler = s390_nmi;
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 5169379..0bf7e14 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -1207,9 +1207,6 @@ static void s390_qemu_cpu_model_class_init(ObjectClass *oc, void *data)
>                                  qemu_hw_version());
>  }
>  
> -#define S390_CPU_TYPE_SUFFIX "-" TYPE_S390_CPU
> -#define S390_CPU_TYPE_NAME(name) (name S390_CPU_TYPE_SUFFIX)
> -
>  /* Generate type name for a cpu model. Caller has to free the string. */
>  static char *s390_cpu_type_name(const char *model_name)
>  {
> @@ -1232,14 +1229,6 @@ ObjectClass *s390_cpu_class_by_name(const char *name)
>      return oc;
>  }
>  
> -const char *s390_default_cpu_model_name(void)
> -{
> -     if (kvm_enabled()) {
> -        return "host";
> -     }
> -     return "qemu";
> -}
> -
>  static const TypeInfo qemu_s390_cpu_type_info = {
>      .name = S390_CPU_TYPE_NAME("qemu"),
>      .parent = TYPE_S390_CPU,
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 720cb1d..5dfdd96 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -287,6 +287,9 @@ void kvm_s390_crypto_reset(void)
>  
>  int kvm_arch_init(MachineState *ms, KVMState *s)
>  {
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +
> +    mc->default_cpu_type = S390_CPU_TYPE_NAME("host");
>      cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
>      cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
>      cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);

Re: [Qemu-devel] [PATCH] s390x: use generic cpu_model parsing
Posted by David Hildenbrand 8 years, 1 month ago
On 21.09.2017 12:53, Igor Mammedov wrote:
> define default CPU type in generic way in machine class_init
> and let common machine code to handle cpu_model parsing.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tested under TCG booting debian kernel,
> and compile tested on s390 host

Hotplugging checks (mixed CPU models) still seem to work fine.

Also parameters get applied and the model name is properly parsed.

>                       "supported by TCG (1) on s390x", max_cpus);
> @@ -73,25 +66,8 @@ static void s390_init_cpus(MachineState *machine)
>      /* initialize possible_cpus */
>      mc->possible_cpu_arch_ids(machine);
>  
> -    model_pieces = g_strsplit(machine->cpu_model, ",", 2);
> -    if (!model_pieces[0]) {
> -        error_report("Invalid/empty CPU model name");
> -        exit(1);
> -    }
> -
> -    oc = cpu_class_by_name(TYPE_S390_CPU, model_pieces[0]);
> -    if (!oc) {
> -        error_report("Unable to find CPU definition: %s", model_pieces[0]);
> -        exit(1);
> -    }
> -    typename = object_class_get_name(oc);
> -    cc = CPU_CLASS(oc);
> -    /* after parsing, properties will be applied to all *typename* instances */
> -    cc->parse_features(typename, model_pieces[1], &error_fatal);
> -    g_strfreev(model_pieces);
> -
>      for (i = 0; i < smp_cpus; i++) {
> -        s390x_new_cpu(typename, i, &error_fatal);
> +        s390x_new_cpu(machine->cpu_type, i, &error_fatal);
>      }
>  }
>  
> @@ -446,6 +422,9 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
>      mc->get_hotplug_handler = s390_get_hotplug_handler;
>      mc->cpu_index_to_instance_props = s390_cpu_index_to_props;
>      mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids;
> +    /* it is overrriden with 'host' cpu type when accel=kvm

s/overrriden/overridden/

... with 'host' cpu *in kvm_arch_init*. ?

> +     * i.e. when kvm_arch_init() is executed */
> +    mc->default_cpu_type = S390_CPU_TYPE_NAME("qemu");
>      hc->plug = s390_machine_device_plug;
>      hc->unplug_request = s390_machine_device_unplug_request;
>      nc->nmi_monitor_handler = s390_nmi;
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 5169379..0bf7e14 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -1207,9 +1207,6 @@ static void s390_qemu_cpu_model_class_init(ObjectClass *oc, void *data)
>                                  qemu_hw_version());
>  }


Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH] s390x: use generic cpu_model parsing
Posted by Igor Mammedov 8 years, 1 month ago
On Thu, 21 Sep 2017 14:34:40 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 21.09.2017 12:53, Igor Mammedov wrote:
> > define default CPU type in generic way in machine class_init
> > and let common machine code to handle cpu_model parsing.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > tested under TCG booting debian kernel,
> > and compile tested on s390 host  
> 
> Hotplugging checks (mixed CPU models) still seem to work fine.
> 
> Also parameters get applied and the model name is properly parsed.
> 
> >                       "supported by TCG (1) on s390x", max_cpus);
> > @@ -73,25 +66,8 @@ static void s390_init_cpus(MachineState *machine)
> >      /* initialize possible_cpus */
> >      mc->possible_cpu_arch_ids(machine);
> >  
> > -    model_pieces = g_strsplit(machine->cpu_model, ",", 2);
> > -    if (!model_pieces[0]) {
> > -        error_report("Invalid/empty CPU model name");
> > -        exit(1);
> > -    }
> > -
> > -    oc = cpu_class_by_name(TYPE_S390_CPU, model_pieces[0]);
> > -    if (!oc) {
> > -        error_report("Unable to find CPU definition: %s", model_pieces[0]);
> > -        exit(1);
> > -    }
> > -    typename = object_class_get_name(oc);
> > -    cc = CPU_CLASS(oc);
> > -    /* after parsing, properties will be applied to all *typename* instances */
> > -    cc->parse_features(typename, model_pieces[1], &error_fatal);
> > -    g_strfreev(model_pieces);
> > -
> >      for (i = 0; i < smp_cpus; i++) {
> > -        s390x_new_cpu(typename, i, &error_fatal);
> > +        s390x_new_cpu(machine->cpu_type, i, &error_fatal);
> >      }
> >  }
> >  
> > @@ -446,6 +422,9 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
> >      mc->get_hotplug_handler = s390_get_hotplug_handler;
> >      mc->cpu_index_to_instance_props = s390_cpu_index_to_props;
> >      mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids;
> > +    /* it is overrriden with 'host' cpu type when accel=kvm  
> 
> s/overrriden/overridden/
> 
> ... with 'host' cpu *in kvm_arch_init*. ?
thanks, fixed in v2

> 
> > +     * i.e. when kvm_arch_init() is executed */
> > +    mc->default_cpu_type = S390_CPU_TYPE_NAME("qemu");
> >      hc->plug = s390_machine_device_plug;
> >      hc->unplug_request = s390_machine_device_unplug_request;
> >      nc->nmi_monitor_handler = s390_nmi;
> > diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> > index 5169379..0bf7e14 100644
> > --- a/target/s390x/cpu_models.c
> > +++ b/target/s390x/cpu_models.c
> > @@ -1207,9 +1207,6 @@ static void s390_qemu_cpu_model_class_init(ObjectClass *oc, void *data)
> >                                  qemu_hw_version());
> >  }  
> 
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
>