diff mbox

[09/10] arm: virt: parse cpu_model only once

Message ID 1465226212-254093-10-git-send-email-imammedo@redhat.com
State New
Headers show

Commit Message

Igor Mammedov June 6, 2016, 3:16 p.m. UTC
considering that features are converted to
global properties and global properties are
automatically applied to every new instance
of created CPU (at object_new() time), there
is no point in parsing cpu_model string every
time a CPU created.
So move parsing outside CPU creation loop and
do it only once.
Parsing also should be done before any CPU is
created so that features would affect the first
CPU a well.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/arm/virt.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

Comments

Eduardo Habkost June 8, 2016, 4:55 p.m. UTC | #1
On Mon, Jun 06, 2016 at 05:16:51PM +0200, Igor Mammedov wrote:
> considering that features are converted to
> global properties and global properties are
> automatically applied to every new instance
> of created CPU (at object_new() time), there
> is no point in parsing cpu_model string every
> time a CPU created.
> So move parsing outside CPU creation loop and
> do it only once.
> Parsing also should be done before any CPU is
> created so that features would affect the first
> CPU a well.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Peter, do you prefer to get this included through my tree with
the rest of the series, or wait to included it in your tree after
the other patches get merged to master?

> ---
>  hw/arm/virt.c | 42 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 473e439..0dbee47 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1112,6 +1112,10 @@ static void machvirt_init(MachineState *machine)
>      VirtGuestInfoState *guest_info_state = g_malloc0(sizeof *guest_info_state);
>      VirtGuestInfo *guest_info = &guest_info_state->info;
>      char **cpustr;
> +    ObjectClass *oc;
> +    const char *typename;
> +    CPUClass *cc;
> +    Error *err = NULL;
>      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
>  
>      if (!cpu_model) {
> @@ -1191,27 +1195,24 @@ static void machvirt_init(MachineState *machine)
>  
>      create_fdt(vbi);
>  
> -    for (n = 0; n < smp_cpus; n++) {
> -        ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
> -        const char *typename = object_class_get_name(oc);
> -        CPUClass *cc = CPU_CLASS(oc);
> -        Object *cpuobj;
> -        Error *err = NULL;
> -        char *cpuopts = g_strdup(cpustr[1]);
> -
> -        if (!oc) {
> -            error_report("Unable to find CPU definition");
> -            exit(1);
> -        }
> -        /* convert -smp CPU options specified by the user into global props */
> -        cc->parse_features(typename, cpuopts, &err);
> -        cpuobj = object_new(typename);
> +    oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
> +    if (!oc) {
> +        error_report("Unable to find CPU definition");
> +        exit(1);
> +    }
> +    typename = object_class_get_name(oc);
>  
> -        g_free(cpuopts);
> -        if (err) {
> -            error_report_err(err);
> -            exit(1);
> -        }
> +    /* convert -smp CPU options specified by the user into global props */
> +    cc = CPU_CLASS(oc);
> +    cc->parse_features(typename, cpustr[1], &err);
> +    g_strfreev(cpustr);
> +    if (err) {
> +        error_report_err(err);
> +        exit(1);
> +    }
> +
> +    for (n = 0; n < smp_cpus; n++) {
> +        Object *cpuobj = object_new(typename);
>  
>          if (!vms->secure) {
>              object_property_set_bool(cpuobj, false, "has_el3", NULL);
> @@ -1242,7 +1243,6 @@ static void machvirt_init(MachineState *machine)
>  
>          object_property_set_bool(cpuobj, true, "realized", NULL);
>      }
> -    g_strfreev(cpustr);
>      fdt_add_timer_nodes(vbi, gic_version);
>      fdt_add_cpu_nodes(vbi);
>      fdt_add_psci_node(vbi);
> -- 
> 1.8.3.1
>
Peter Maydell June 8, 2016, 5:25 p.m. UTC | #2
On 8 June 2016 at 17:55, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Jun 06, 2016 at 05:16:51PM +0200, Igor Mammedov wrote:
>> considering that features are converted to
>> global properties and global properties are
>> automatically applied to every new instance
>> of created CPU (at object_new() time), there
>> is no point in parsing cpu_model string every
>> time a CPU created.
>> So move parsing outside CPU creation loop and
>> do it only once.
>> Parsing also should be done before any CPU is
>> created so that features would affect the first
>> CPU a well.
>>
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>
> Peter, do you prefer to get this included through my tree with
> the rest of the series, or wait to included it in your tree after
> the other patches get merged to master?

Seems easiest for you to take it.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
diff mbox

Patch

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 473e439..0dbee47 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1112,6 +1112,10 @@  static void machvirt_init(MachineState *machine)
     VirtGuestInfoState *guest_info_state = g_malloc0(sizeof *guest_info_state);
     VirtGuestInfo *guest_info = &guest_info_state->info;
     char **cpustr;
+    ObjectClass *oc;
+    const char *typename;
+    CPUClass *cc;
+    Error *err = NULL;
     bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
 
     if (!cpu_model) {
@@ -1191,27 +1195,24 @@  static void machvirt_init(MachineState *machine)
 
     create_fdt(vbi);
 
-    for (n = 0; n < smp_cpus; n++) {
-        ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
-        const char *typename = object_class_get_name(oc);
-        CPUClass *cc = CPU_CLASS(oc);
-        Object *cpuobj;
-        Error *err = NULL;
-        char *cpuopts = g_strdup(cpustr[1]);
-
-        if (!oc) {
-            error_report("Unable to find CPU definition");
-            exit(1);
-        }
-        /* convert -smp CPU options specified by the user into global props */
-        cc->parse_features(typename, cpuopts, &err);
-        cpuobj = object_new(typename);
+    oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
+    if (!oc) {
+        error_report("Unable to find CPU definition");
+        exit(1);
+    }
+    typename = object_class_get_name(oc);
 
-        g_free(cpuopts);
-        if (err) {
-            error_report_err(err);
-            exit(1);
-        }
+    /* convert -smp CPU options specified by the user into global props */
+    cc = CPU_CLASS(oc);
+    cc->parse_features(typename, cpustr[1], &err);
+    g_strfreev(cpustr);
+    if (err) {
+        error_report_err(err);
+        exit(1);
+    }
+
+    for (n = 0; n < smp_cpus; n++) {
+        Object *cpuobj = object_new(typename);
 
         if (!vms->secure) {
             object_property_set_bool(cpuobj, false, "has_el3", NULL);
@@ -1242,7 +1243,6 @@  static void machvirt_init(MachineState *machine)
 
         object_property_set_bool(cpuobj, true, "realized", NULL);
     }
-    g_strfreev(cpustr);
     fdt_add_timer_nodes(vbi, gic_version);
     fdt_add_cpu_nodes(vbi);
     fdt_add_psci_node(vbi);