All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()
@ 2019-04-17  2:59 ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17  2:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Paolo Bonzini, Riku Voipio, Eduardo Habkost,
	Like Xu, Richard Henderson, David Gibson, Thomas Huth,
	Igor Mammedov, qemu-ppc, Markus Armbruster, Peter Maydell,
	Artyom Tarasenko, Mark Cave-Ayland

My initial goal was simple: removing the qdev_get_machine() call
from ppc_cpu_parse_featurestr() because I want to make
qdev_get_machine() available only to softmmu code.

Before doing this, I had to make *-user not call
CPUClass::parse_features() anymore (it was pointless to call it,
anyway).

While doing this, I decided to rename parse_cpu_model() to
something clearer (parse_cpu_option()).

As a nice side effect, now the dependency between machine object
creation and parse_cpu_option() is not hidden anymore.

Eduardo Habkost (5):
  cpu: Rename parse_cpu_model() to parse_cpu_option()
  cpu: Extract CPU class lookup from parse_cpu_option()
  linux-user: Use lookup_cpu_class()
  bsd-user: Use lookup_cpu_class()
  cpu: Add MachineState parameter to parse_features()

 include/qom/cpu.h               | 18 ++++++++++++++----
 target/ppc/cpu-qom.h            |  3 ++-
 bsd-user/main.c                 |  4 +++-
 exec.c                          | 28 +++++++++++++++-------------
 linux-user/main.c               |  4 +++-
 qom/cpu.c                       |  3 ++-
 target/i386/cpu.c               |  3 ++-
 target/ppc/translate_init.inc.c |  7 ++++---
 target/sparc/cpu.c              |  3 ++-
 vl.c                            | 19 ++++++++++---------
 10 files changed, 57 insertions(+), 35 deletions(-)

-- 
2.18.0.rc1.1.g3f1ff2140

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()
@ 2019-04-17  2:59 ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17  2:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Thomas Huth, Eduardo Habkost, Like Xu,
	Riku Voipio, Mark Cave-Ayland, Laurent Vivier, Markus Armbruster,
	qemu-ppc, Igor Mammedov, Paolo Bonzini, David Gibson,
	Artyom Tarasenko, Richard Henderson

My initial goal was simple: removing the qdev_get_machine() call
from ppc_cpu_parse_featurestr() because I want to make
qdev_get_machine() available only to softmmu code.

Before doing this, I had to make *-user not call
CPUClass::parse_features() anymore (it was pointless to call it,
anyway).

While doing this, I decided to rename parse_cpu_model() to
something clearer (parse_cpu_option()).

As a nice side effect, now the dependency between machine object
creation and parse_cpu_option() is not hidden anymore.

Eduardo Habkost (5):
  cpu: Rename parse_cpu_model() to parse_cpu_option()
  cpu: Extract CPU class lookup from parse_cpu_option()
  linux-user: Use lookup_cpu_class()
  bsd-user: Use lookup_cpu_class()
  cpu: Add MachineState parameter to parse_features()

 include/qom/cpu.h               | 18 ++++++++++++++----
 target/ppc/cpu-qom.h            |  3 ++-
 bsd-user/main.c                 |  4 +++-
 exec.c                          | 28 +++++++++++++++-------------
 linux-user/main.c               |  4 +++-
 qom/cpu.c                       |  3 ++-
 target/i386/cpu.c               |  3 ++-
 target/ppc/translate_init.inc.c |  7 ++++---
 target/sparc/cpu.c              |  3 ++-
 vl.c                            | 19 ++++++++++---------
 10 files changed, 57 insertions(+), 35 deletions(-)

-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option()
@ 2019-04-17  2:59   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17  2:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Paolo Bonzini, Riku Voipio, Eduardo Habkost,
	Like Xu, Richard Henderson, David Gibson, Thomas Huth,
	Igor Mammedov, qemu-ppc, Markus Armbruster, Peter Maydell,
	Artyom Tarasenko, Mark Cave-Ayland

The "model[,option...]" string parsed by the function is not just
a CPU model.  Rename the function and its argument to indicate it
expects the full "-cpu" option to be provided.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/qom/cpu.h |  6 +++---
 bsd-user/main.c   |  2 +-
 exec.c            |  4 ++--
 linux-user/main.c |  2 +-
 vl.c              | 18 +++++++++---------
 5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 1d6099e5d4..d28c690b27 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -689,15 +689,15 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
 CPUState *cpu_create(const char *typename);
 
 /**
- * parse_cpu_model:
- * @cpu_model: The model string including optional parameters.
+ * parse_cpu_option:
+ * @cpu_option: The -cpu option including optional parameters.
  *
  * processes optional parameters and registers them as global properties
  *
  * Returns: type of CPU to create or prints error and terminates process
  *          if an error occurred.
  */
-const char *parse_cpu_model(const char *cpu_model);
+const char *parse_cpu_option(const char *cpu_option);
 
 /**
  * cpu_has_work:
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 0d3156974c..a6c055f5fb 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -903,7 +903,7 @@ int main(int argc, char **argv)
     /* init tcg before creating CPUs and to get qemu_host_page_size */
     tcg_exec_init(0);
 
-    cpu_type = parse_cpu_model(cpu_model);
+    cpu_type = parse_cpu_option(cpu_model);
     cpu = cpu_create(cpu_type);
     env = cpu->env_ptr;
 #if defined(TARGET_SPARC) || defined(TARGET_PPC)
diff --git a/exec.c b/exec.c
index 6ab62f4eee..840677f15f 100644
--- a/exec.c
+++ b/exec.c
@@ -982,14 +982,14 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
 #endif
 }
 
-const char *parse_cpu_model(const char *cpu_model)
+const char *parse_cpu_option(const char *cpu_option)
 {
     ObjectClass *oc;
     CPUClass *cc;
     gchar **model_pieces;
     const char *cpu_type;
 
-    model_pieces = g_strsplit(cpu_model, ",", 2);
+    model_pieces = g_strsplit(cpu_option, ",", 2);
 
     oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
     if (oc == NULL) {
diff --git a/linux-user/main.c b/linux-user/main.c
index a0aba9cb1e..20e0f51cfa 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -660,7 +660,7 @@ int main(int argc, char **argv, char **envp)
     if (cpu_model == NULL) {
         cpu_model = cpu_get_model(get_elf_eflags(execfd));
     }
-    cpu_type = parse_cpu_model(cpu_model);
+    cpu_type = parse_cpu_option(cpu_model);
 
     /* init tcg before creating CPUs and to get qemu_host_page_size */
     tcg_exec_init(0);
diff --git a/vl.c b/vl.c
index c696ad2a13..c57e28d1da 100644
--- a/vl.c
+++ b/vl.c
@@ -3002,7 +3002,7 @@ int main(int argc, char **argv, char **envp)
     const char *optarg;
     const char *loadvm = NULL;
     MachineClass *machine_class;
-    const char *cpu_model;
+    const char *cpu_option;
     const char *vga_model = NULL;
     const char *qtest_chrdev = NULL;
     const char *qtest_log = NULL;
@@ -3081,7 +3081,7 @@ int main(int argc, char **argv, char **envp)
     QLIST_INIT (&vm_change_state_head);
     os_setup_early_signal_handling();
 
-    cpu_model = NULL;
+    cpu_option = NULL;
     snapshot = 0;
 
     nb_nics = 0;
@@ -3133,7 +3133,7 @@ int main(int argc, char **argv, char **envp)
             switch(popt->index) {
             case QEMU_OPTION_cpu:
                 /* hw initialization will check this */
-                cpu_model = optarg;
+                cpu_option = optarg;
                 break;
             case QEMU_OPTION_hda:
             case QEMU_OPTION_hdb:
@@ -4050,8 +4050,8 @@ int main(int argc, char **argv, char **envp)
         qemu_set_hw_version(machine_class->hw_version);
     }
 
-    if (cpu_model && is_help_option(cpu_model)) {
-        list_cpus(stdout, &fprintf, cpu_model);
+    if (cpu_option && is_help_option(cpu_option)) {
+        list_cpus(stdout, &fprintf, cpu_option);
         exit(0);
     }
 
@@ -4299,9 +4299,9 @@ int main(int argc, char **argv, char **envp)
      * Global properties get set up by qdev_prop_register_global(),
      * called from user_register_global_props(), and certain option
      * desugaring.  Also in CPU feature desugaring (buried in
-     * parse_cpu_model()), which happens below this point, but may
+     * parse_cpu_option()), which happens below this point, but may
      * only target the CPU type, which can only be created after
-     * parse_cpu_model() returned the type.
+     * parse_cpu_option() returned the type.
      *
      * Machine compat properties: object_set_machine_compat_props().
      * Accelerator compat props: object_set_accelerator_compat_props(),
@@ -4465,8 +4465,8 @@ int main(int argc, char **argv, char **envp)
 
     /* parse features once if machine provides default cpu_type */
     current_machine->cpu_type = machine_class->default_cpu_type;
-    if (cpu_model) {
-        current_machine->cpu_type = parse_cpu_model(cpu_model);
+    if (cpu_option) {
+        current_machine->cpu_type = parse_cpu_option(cpu_option);
     }
     parse_numa_opts(current_machine);
 
-- 
2.18.0.rc1.1.g3f1ff2140

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option()
@ 2019-04-17  2:59   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17  2:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Thomas Huth, Eduardo Habkost, Like Xu,
	Riku Voipio, Mark Cave-Ayland, Laurent Vivier, Markus Armbruster,
	qemu-ppc, Igor Mammedov, Paolo Bonzini, David Gibson,
	Artyom Tarasenko, Richard Henderson

The "model[,option...]" string parsed by the function is not just
a CPU model.  Rename the function and its argument to indicate it
expects the full "-cpu" option to be provided.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/qom/cpu.h |  6 +++---
 bsd-user/main.c   |  2 +-
 exec.c            |  4 ++--
 linux-user/main.c |  2 +-
 vl.c              | 18 +++++++++---------
 5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 1d6099e5d4..d28c690b27 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -689,15 +689,15 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
 CPUState *cpu_create(const char *typename);
 
 /**
- * parse_cpu_model:
- * @cpu_model: The model string including optional parameters.
+ * parse_cpu_option:
+ * @cpu_option: The -cpu option including optional parameters.
  *
  * processes optional parameters and registers them as global properties
  *
  * Returns: type of CPU to create or prints error and terminates process
  *          if an error occurred.
  */
-const char *parse_cpu_model(const char *cpu_model);
+const char *parse_cpu_option(const char *cpu_option);
 
 /**
  * cpu_has_work:
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 0d3156974c..a6c055f5fb 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -903,7 +903,7 @@ int main(int argc, char **argv)
     /* init tcg before creating CPUs and to get qemu_host_page_size */
     tcg_exec_init(0);
 
-    cpu_type = parse_cpu_model(cpu_model);
+    cpu_type = parse_cpu_option(cpu_model);
     cpu = cpu_create(cpu_type);
     env = cpu->env_ptr;
 #if defined(TARGET_SPARC) || defined(TARGET_PPC)
diff --git a/exec.c b/exec.c
index 6ab62f4eee..840677f15f 100644
--- a/exec.c
+++ b/exec.c
@@ -982,14 +982,14 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
 #endif
 }
 
-const char *parse_cpu_model(const char *cpu_model)
+const char *parse_cpu_option(const char *cpu_option)
 {
     ObjectClass *oc;
     CPUClass *cc;
     gchar **model_pieces;
     const char *cpu_type;
 
-    model_pieces = g_strsplit(cpu_model, ",", 2);
+    model_pieces = g_strsplit(cpu_option, ",", 2);
 
     oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
     if (oc == NULL) {
diff --git a/linux-user/main.c b/linux-user/main.c
index a0aba9cb1e..20e0f51cfa 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -660,7 +660,7 @@ int main(int argc, char **argv, char **envp)
     if (cpu_model == NULL) {
         cpu_model = cpu_get_model(get_elf_eflags(execfd));
     }
-    cpu_type = parse_cpu_model(cpu_model);
+    cpu_type = parse_cpu_option(cpu_model);
 
     /* init tcg before creating CPUs and to get qemu_host_page_size */
     tcg_exec_init(0);
diff --git a/vl.c b/vl.c
index c696ad2a13..c57e28d1da 100644
--- a/vl.c
+++ b/vl.c
@@ -3002,7 +3002,7 @@ int main(int argc, char **argv, char **envp)
     const char *optarg;
     const char *loadvm = NULL;
     MachineClass *machine_class;
-    const char *cpu_model;
+    const char *cpu_option;
     const char *vga_model = NULL;
     const char *qtest_chrdev = NULL;
     const char *qtest_log = NULL;
@@ -3081,7 +3081,7 @@ int main(int argc, char **argv, char **envp)
     QLIST_INIT (&vm_change_state_head);
     os_setup_early_signal_handling();
 
-    cpu_model = NULL;
+    cpu_option = NULL;
     snapshot = 0;
 
     nb_nics = 0;
@@ -3133,7 +3133,7 @@ int main(int argc, char **argv, char **envp)
             switch(popt->index) {
             case QEMU_OPTION_cpu:
                 /* hw initialization will check this */
-                cpu_model = optarg;
+                cpu_option = optarg;
                 break;
             case QEMU_OPTION_hda:
             case QEMU_OPTION_hdb:
@@ -4050,8 +4050,8 @@ int main(int argc, char **argv, char **envp)
         qemu_set_hw_version(machine_class->hw_version);
     }
 
-    if (cpu_model && is_help_option(cpu_model)) {
-        list_cpus(stdout, &fprintf, cpu_model);
+    if (cpu_option && is_help_option(cpu_option)) {
+        list_cpus(stdout, &fprintf, cpu_option);
         exit(0);
     }
 
@@ -4299,9 +4299,9 @@ int main(int argc, char **argv, char **envp)
      * Global properties get set up by qdev_prop_register_global(),
      * called from user_register_global_props(), and certain option
      * desugaring.  Also in CPU feature desugaring (buried in
-     * parse_cpu_model()), which happens below this point, but may
+     * parse_cpu_option()), which happens below this point, but may
      * only target the CPU type, which can only be created after
-     * parse_cpu_model() returned the type.
+     * parse_cpu_option() returned the type.
      *
      * Machine compat properties: object_set_machine_compat_props().
      * Accelerator compat props: object_set_accelerator_compat_props(),
@@ -4465,8 +4465,8 @@ int main(int argc, char **argv, char **envp)
 
     /* parse features once if machine provides default cpu_type */
     current_machine->cpu_type = machine_class->default_cpu_type;
-    if (cpu_model) {
-        current_machine->cpu_type = parse_cpu_model(cpu_model);
+    if (cpu_option) {
+        current_machine->cpu_type = parse_cpu_option(cpu_option);
     }
     parse_numa_opts(current_machine);
 
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()
@ 2019-04-17  2:59   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17  2:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Paolo Bonzini, Riku Voipio, Eduardo Habkost,
	Like Xu, Richard Henderson, David Gibson, Thomas Huth,
	Igor Mammedov, qemu-ppc, Markus Armbruster, Peter Maydell,
	Artyom Tarasenko, Mark Cave-Ayland

The new function will be useful in user mode, when we already
have a CPU model and don't need to parse any extra options.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/qom/cpu.h |  9 +++++++++
 exec.c            | 22 ++++++++++++----------
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index d28c690b27..e11b14d9ac 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -699,6 +699,15 @@ CPUState *cpu_create(const char *typename);
  */
 const char *parse_cpu_option(const char *cpu_option);
 
+/**
+ * lookup_cpu_class:
+ * @cpu_model: CPU model name
+ *
+ * Look up CPU class corresponding to a given CPU model name.
+ */
+CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp);
+
+
 /**
  * cpu_has_work:
  * @cpu: The vCPU to check.
diff --git a/exec.c b/exec.c
index 840677f15f..d359e709a6 100644
--- a/exec.c
+++ b/exec.c
@@ -982,24 +982,26 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
 #endif
 }
 
+CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp)
+{
+    ObjectClass *oc = cpu_class_by_name(CPU_RESOLVING_TYPE, cpu_model);
+    if (oc == NULL) {
+        error_setg(errp, "unable to find CPU model '%s'", cpu_model);
+        return NULL;
+    }
+    return CPU_CLASS(oc);
+}
+
 const char *parse_cpu_option(const char *cpu_option)
 {
-    ObjectClass *oc;
     CPUClass *cc;
     gchar **model_pieces;
     const char *cpu_type;
 
     model_pieces = g_strsplit(cpu_option, ",", 2);
 
-    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
-    if (oc == NULL) {
-        error_report("unable to find CPU model '%s'", model_pieces[0]);
-        g_strfreev(model_pieces);
-        exit(EXIT_FAILURE);
-    }
-
-    cpu_type = object_class_get_name(oc);
-    cc = CPU_CLASS(oc);
+    cc = lookup_cpu_class(model_pieces[0], &error_fatal);
+    cpu_type = object_class_get_name(OBJECT_CLASS(cc));
     cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
     g_strfreev(model_pieces);
     return cpu_type;
-- 
2.18.0.rc1.1.g3f1ff2140

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()
@ 2019-04-17  2:59   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17  2:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Thomas Huth, Eduardo Habkost, Like Xu,
	Riku Voipio, Mark Cave-Ayland, Laurent Vivier, Markus Armbruster,
	qemu-ppc, Igor Mammedov, Paolo Bonzini, David Gibson,
	Artyom Tarasenko, Richard Henderson

The new function will be useful in user mode, when we already
have a CPU model and don't need to parse any extra options.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/qom/cpu.h |  9 +++++++++
 exec.c            | 22 ++++++++++++----------
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index d28c690b27..e11b14d9ac 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -699,6 +699,15 @@ CPUState *cpu_create(const char *typename);
  */
 const char *parse_cpu_option(const char *cpu_option);
 
+/**
+ * lookup_cpu_class:
+ * @cpu_model: CPU model name
+ *
+ * Look up CPU class corresponding to a given CPU model name.
+ */
+CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp);
+
+
 /**
  * cpu_has_work:
  * @cpu: The vCPU to check.
diff --git a/exec.c b/exec.c
index 840677f15f..d359e709a6 100644
--- a/exec.c
+++ b/exec.c
@@ -982,24 +982,26 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
 #endif
 }
 
+CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp)
+{
+    ObjectClass *oc = cpu_class_by_name(CPU_RESOLVING_TYPE, cpu_model);
+    if (oc == NULL) {
+        error_setg(errp, "unable to find CPU model '%s'", cpu_model);
+        return NULL;
+    }
+    return CPU_CLASS(oc);
+}
+
 const char *parse_cpu_option(const char *cpu_option)
 {
-    ObjectClass *oc;
     CPUClass *cc;
     gchar **model_pieces;
     const char *cpu_type;
 
     model_pieces = g_strsplit(cpu_option, ",", 2);
 
-    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
-    if (oc == NULL) {
-        error_report("unable to find CPU model '%s'", model_pieces[0]);
-        g_strfreev(model_pieces);
-        exit(EXIT_FAILURE);
-    }
-
-    cpu_type = object_class_get_name(oc);
-    cc = CPU_CLASS(oc);
+    cc = lookup_cpu_class(model_pieces[0], &error_fatal);
+    cpu_type = object_class_get_name(OBJECT_CLASS(cc));
     cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
     g_strfreev(model_pieces);
     return cpu_type;
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class()
@ 2019-04-17  2:59   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17  2:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Paolo Bonzini, Riku Voipio, Eduardo Habkost,
	Like Xu, Richard Henderson, David Gibson, Thomas Huth,
	Igor Mammedov, qemu-ppc, Markus Armbruster, Peter Maydell,
	Artyom Tarasenko, Mark Cave-Ayland

The return value of cpu_get_model() is just a CPU model name and
never includes extra options.  We don't need to call
parse_cpu_option().

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 linux-user/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 20e0f51cfa..d74108e05c 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -592,6 +592,7 @@ int main(int argc, char **argv, char **envp)
     TaskState *ts;
     CPUArchState *env;
     CPUState *cpu;
+    CPUClass *cc;
     int optind;
     char **target_environ, **wrk;
     char **target_argv;
@@ -660,7 +661,8 @@ int main(int argc, char **argv, char **envp)
     if (cpu_model == NULL) {
         cpu_model = cpu_get_model(get_elf_eflags(execfd));
     }
-    cpu_type = parse_cpu_option(cpu_model);
+    cc = lookup_cpu_class(cpu_model, &error_fatal);
+    cpu_type = object_class_get_name(OBJECT_CLASS(cc));
 
     /* init tcg before creating CPUs and to get qemu_host_page_size */
     tcg_exec_init(0);
-- 
2.18.0.rc1.1.g3f1ff2140

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class()
@ 2019-04-17  2:59   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17  2:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Thomas Huth, Eduardo Habkost, Like Xu,
	Riku Voipio, Mark Cave-Ayland, Laurent Vivier, Markus Armbruster,
	qemu-ppc, Igor Mammedov, Paolo Bonzini, David Gibson,
	Artyom Tarasenko, Richard Henderson

The return value of cpu_get_model() is just a CPU model name and
never includes extra options.  We don't need to call
parse_cpu_option().

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 linux-user/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 20e0f51cfa..d74108e05c 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -592,6 +592,7 @@ int main(int argc, char **argv, char **envp)
     TaskState *ts;
     CPUArchState *env;
     CPUState *cpu;
+    CPUClass *cc;
     int optind;
     char **target_environ, **wrk;
     char **target_argv;
@@ -660,7 +661,8 @@ int main(int argc, char **argv, char **envp)
     if (cpu_model == NULL) {
         cpu_model = cpu_get_model(get_elf_eflags(execfd));
     }
-    cpu_type = parse_cpu_option(cpu_model);
+    cc = lookup_cpu_class(cpu_model, &error_fatal);
+    cpu_type = object_class_get_name(OBJECT_CLASS(cc));
 
     /* init tcg before creating CPUs and to get qemu_host_page_size */
     tcg_exec_init(0);
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 4/5] bsd-user: Use lookup_cpu_class()
@ 2019-04-17  2:59   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17  2:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Paolo Bonzini, Riku Voipio, Eduardo Habkost,
	Like Xu, Richard Henderson, David Gibson, Thomas Huth,
	Igor Mammedov, qemu-ppc, Markus Armbruster, Peter Maydell,
	Artyom Tarasenko, Mark Cave-Ayland

The hardcoded CPU models in the code are just CPU models and
don't include any extra options.  We don't need to call
parse_cpu_options().

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 bsd-user/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index a6c055f5fb..d2915a9951 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -732,6 +732,7 @@ int main(int argc, char **argv)
     TaskState ts1, *ts = &ts1;
     CPUArchState *env;
     CPUState *cpu;
+    CPUClass *cc;
     int optind;
     const char *r;
     int gdbstub_port = 0;
@@ -903,7 +904,8 @@ int main(int argc, char **argv)
     /* init tcg before creating CPUs and to get qemu_host_page_size */
     tcg_exec_init(0);
 
-    cpu_type = parse_cpu_option(cpu_model);
+    cc = lookup_cpu_class(cpu_model, &error_fatal);
+    cpu_type = object_class_get_name(OBJECT_CLASS(cc));
     cpu = cpu_create(cpu_type);
     env = cpu->env_ptr;
 #if defined(TARGET_SPARC) || defined(TARGET_PPC)
-- 
2.18.0.rc1.1.g3f1ff2140

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 4/5] bsd-user: Use lookup_cpu_class()
@ 2019-04-17  2:59   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17  2:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Thomas Huth, Eduardo Habkost, Like Xu,
	Riku Voipio, Mark Cave-Ayland, Laurent Vivier, Markus Armbruster,
	qemu-ppc, Igor Mammedov, Paolo Bonzini, David Gibson,
	Artyom Tarasenko, Richard Henderson

The hardcoded CPU models in the code are just CPU models and
don't include any extra options.  We don't need to call
parse_cpu_options().

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 bsd-user/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index a6c055f5fb..d2915a9951 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -732,6 +732,7 @@ int main(int argc, char **argv)
     TaskState ts1, *ts = &ts1;
     CPUArchState *env;
     CPUState *cpu;
+    CPUClass *cc;
     int optind;
     const char *r;
     int gdbstub_port = 0;
@@ -903,7 +904,8 @@ int main(int argc, char **argv)
     /* init tcg before creating CPUs and to get qemu_host_page_size */
     tcg_exec_init(0);
 
-    cpu_type = parse_cpu_option(cpu_model);
+    cc = lookup_cpu_class(cpu_model, &error_fatal);
+    cpu_type = object_class_get_name(OBJECT_CLASS(cc));
     cpu = cpu_create(cpu_type);
     env = cpu->env_ptr;
 #if defined(TARGET_SPARC) || defined(TARGET_PPC)
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features()
@ 2019-04-17  2:59   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17  2:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Paolo Bonzini, Riku Voipio, Eduardo Habkost,
	Like Xu, Richard Henderson, David Gibson, Thomas Huth,
	Igor Mammedov, qemu-ppc, Markus Armbruster, Peter Maydell,
	Artyom Tarasenko, Mark Cave-Ayland

The ppc implementation of parse_features() requires the machine
object to be created before it gets called.  This is far from
obvious when reading the code at main().

Instead of making it call qdev_get_machine(), require the caller
of parse_cpu_option() to provide the machine object.

This makes the initialization dependency explicit at main(), and
will let us move qdev_get_machine() to CONFIG_SOFTMMU in the
future.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/qom/cpu.h               | 5 +++--
 target/ppc/cpu-qom.h            | 3 ++-
 exec.c                          | 4 ++--
 qom/cpu.c                       | 3 ++-
 target/i386/cpu.c               | 3 ++-
 target/ppc/translate_init.inc.c | 7 ++++---
 target/sparc/cpu.c              | 3 ++-
 vl.c                            | 3 ++-
 8 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index e11b14d9ac..cbc8e103bb 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -164,7 +164,8 @@ typedef struct CPUClass {
     /*< public >*/
 
     ObjectClass *(*class_by_name)(const char *cpu_model);
-    void (*parse_features)(const char *typename, char *str, Error **errp);
+    void (*parse_features)(MachineState *machine, const char *typename,
+                           char *str, Error **errp);
 
     void (*reset)(CPUState *cpu);
     int reset_dump_flags;
@@ -697,7 +698,7 @@ CPUState *cpu_create(const char *typename);
  * Returns: type of CPU to create or prints error and terminates process
  *          if an error occurred.
  */
-const char *parse_cpu_option(const char *cpu_option);
+const char *parse_cpu_option(MachineState *machine, const char *cpu_option);
 
 /**
  * lookup_cpu_class:
diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index be9b4c30c3..7891465554 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -167,7 +167,8 @@ typedef struct PowerPCCPUClass {
     DeviceRealize parent_realize;
     DeviceUnrealize parent_unrealize;
     void (*parent_reset)(CPUState *cpu);
-    void (*parent_parse_features)(const char *type, char *str, Error **errp);
+    void (*parent_parse_features)(MachineState *machine, const char *type,
+                                  char *str, Error **errp);
 
     uint32_t pvr;
     bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
diff --git a/exec.c b/exec.c
index d359e709a6..1ca95df9d8 100644
--- a/exec.c
+++ b/exec.c
@@ -992,7 +992,7 @@ CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp)
     return CPU_CLASS(oc);
 }
 
-const char *parse_cpu_option(const char *cpu_option)
+const char *parse_cpu_option(MachineState *machine, const char *cpu_option)
 {
     CPUClass *cc;
     gchar **model_pieces;
@@ -1002,7 +1002,7 @@ const char *parse_cpu_option(const char *cpu_option)
 
     cc = lookup_cpu_class(model_pieces[0], &error_fatal);
     cpu_type = object_class_get_name(OBJECT_CLASS(cc));
-    cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
+    cc->parse_features(machine, cpu_type, model_pieces[1], &error_fatal);
     g_strfreev(model_pieces);
     return cpu_type;
 }
diff --git a/qom/cpu.c b/qom/cpu.c
index a8d2958956..c8a7b56148 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -291,7 +291,8 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
     return cc->class_by_name(cpu_model);
 }
 
-static void cpu_common_parse_features(const char *typename, char *features,
+static void cpu_common_parse_features(MachineState *machine,
+                                      const char *typename, char *features,
                                       Error **errp)
 {
     char *val;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d6bb57d210..f5e15ac5da 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3528,7 +3528,8 @@ static gint compare_string(gconstpointer a, gconstpointer b)
 
 /* Parse "+feature,-feature,feature=foo" CPU feature string
  */
-static void x86_cpu_parse_featurestr(const char *typename, char *features,
+static void x86_cpu_parse_featurestr(MachineState *machine,
+                                     const char *typename, char *features,
                                      Error **errp)
 {
     char *featurestr; /* Single 'key=value" string being parsed */
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index 0bd555eb19..2ad223fcca 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -10119,10 +10119,11 @@ static ObjectClass *ppc_cpu_class_by_name(const char *name)
     return oc;
 }
 
-static void ppc_cpu_parse_featurestr(const char *type, char *features,
+static void ppc_cpu_parse_featurestr(MachineState *ms,
+                                     const char *type, char *features,
                                      Error **errp)
 {
-    Object *machine = qdev_get_machine();
+    Object *machine = OBJECT(ms);
     const PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(object_class_by_name(type));
 
     if (!features) {
@@ -10171,7 +10172,7 @@ static void ppc_cpu_parse_featurestr(const char *type, char *features,
     }
 
     /* do property processing with generic handler */
-    pcc->parent_parse_features(type, features, errp);
+    pcc->parent_parse_features(ms, type, features, errp);
 }
 
 PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 4a4445bdf5..7e360de5ee 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -115,7 +115,8 @@ cpu_add_feat_as_prop(const char *typename, const char *name, const char *val)
 }
 
 /* Parse "+feature,-feature,feature=foo" CPU feature string */
-static void sparc_cpu_parse_features(const char *typename, char *features,
+static void sparc_cpu_parse_features(MachineState *machine,
+                                     const char *typename, char *features,
                                      Error **errp)
 {
     GList *l, *plus_features = NULL, *minus_features = NULL;
diff --git a/vl.c b/vl.c
index c57e28d1da..e78c4d5a53 100644
--- a/vl.c
+++ b/vl.c
@@ -4466,7 +4466,8 @@ int main(int argc, char **argv, char **envp)
     /* parse features once if machine provides default cpu_type */
     current_machine->cpu_type = machine_class->default_cpu_type;
     if (cpu_option) {
-        current_machine->cpu_type = parse_cpu_option(cpu_option);
+        current_machine->cpu_type =
+            parse_cpu_option(current_machine, cpu_option);
     }
     parse_numa_opts(current_machine);
 
-- 
2.18.0.rc1.1.g3f1ff2140

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features()
@ 2019-04-17  2:59   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17  2:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Thomas Huth, Eduardo Habkost, Like Xu,
	Riku Voipio, Mark Cave-Ayland, Laurent Vivier, Markus Armbruster,
	qemu-ppc, Igor Mammedov, Paolo Bonzini, David Gibson,
	Artyom Tarasenko, Richard Henderson

The ppc implementation of parse_features() requires the machine
object to be created before it gets called.  This is far from
obvious when reading the code at main().

Instead of making it call qdev_get_machine(), require the caller
of parse_cpu_option() to provide the machine object.

This makes the initialization dependency explicit at main(), and
will let us move qdev_get_machine() to CONFIG_SOFTMMU in the
future.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/qom/cpu.h               | 5 +++--
 target/ppc/cpu-qom.h            | 3 ++-
 exec.c                          | 4 ++--
 qom/cpu.c                       | 3 ++-
 target/i386/cpu.c               | 3 ++-
 target/ppc/translate_init.inc.c | 7 ++++---
 target/sparc/cpu.c              | 3 ++-
 vl.c                            | 3 ++-
 8 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index e11b14d9ac..cbc8e103bb 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -164,7 +164,8 @@ typedef struct CPUClass {
     /*< public >*/
 
     ObjectClass *(*class_by_name)(const char *cpu_model);
-    void (*parse_features)(const char *typename, char *str, Error **errp);
+    void (*parse_features)(MachineState *machine, const char *typename,
+                           char *str, Error **errp);
 
     void (*reset)(CPUState *cpu);
     int reset_dump_flags;
@@ -697,7 +698,7 @@ CPUState *cpu_create(const char *typename);
  * Returns: type of CPU to create or prints error and terminates process
  *          if an error occurred.
  */
-const char *parse_cpu_option(const char *cpu_option);
+const char *parse_cpu_option(MachineState *machine, const char *cpu_option);
 
 /**
  * lookup_cpu_class:
diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index be9b4c30c3..7891465554 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -167,7 +167,8 @@ typedef struct PowerPCCPUClass {
     DeviceRealize parent_realize;
     DeviceUnrealize parent_unrealize;
     void (*parent_reset)(CPUState *cpu);
-    void (*parent_parse_features)(const char *type, char *str, Error **errp);
+    void (*parent_parse_features)(MachineState *machine, const char *type,
+                                  char *str, Error **errp);
 
     uint32_t pvr;
     bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
diff --git a/exec.c b/exec.c
index d359e709a6..1ca95df9d8 100644
--- a/exec.c
+++ b/exec.c
@@ -992,7 +992,7 @@ CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp)
     return CPU_CLASS(oc);
 }
 
-const char *parse_cpu_option(const char *cpu_option)
+const char *parse_cpu_option(MachineState *machine, const char *cpu_option)
 {
     CPUClass *cc;
     gchar **model_pieces;
@@ -1002,7 +1002,7 @@ const char *parse_cpu_option(const char *cpu_option)
 
     cc = lookup_cpu_class(model_pieces[0], &error_fatal);
     cpu_type = object_class_get_name(OBJECT_CLASS(cc));
-    cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
+    cc->parse_features(machine, cpu_type, model_pieces[1], &error_fatal);
     g_strfreev(model_pieces);
     return cpu_type;
 }
diff --git a/qom/cpu.c b/qom/cpu.c
index a8d2958956..c8a7b56148 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -291,7 +291,8 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
     return cc->class_by_name(cpu_model);
 }
 
-static void cpu_common_parse_features(const char *typename, char *features,
+static void cpu_common_parse_features(MachineState *machine,
+                                      const char *typename, char *features,
                                       Error **errp)
 {
     char *val;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d6bb57d210..f5e15ac5da 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3528,7 +3528,8 @@ static gint compare_string(gconstpointer a, gconstpointer b)
 
 /* Parse "+feature,-feature,feature=foo" CPU feature string
  */
-static void x86_cpu_parse_featurestr(const char *typename, char *features,
+static void x86_cpu_parse_featurestr(MachineState *machine,
+                                     const char *typename, char *features,
                                      Error **errp)
 {
     char *featurestr; /* Single 'key=value" string being parsed */
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index 0bd555eb19..2ad223fcca 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -10119,10 +10119,11 @@ static ObjectClass *ppc_cpu_class_by_name(const char *name)
     return oc;
 }
 
-static void ppc_cpu_parse_featurestr(const char *type, char *features,
+static void ppc_cpu_parse_featurestr(MachineState *ms,
+                                     const char *type, char *features,
                                      Error **errp)
 {
-    Object *machine = qdev_get_machine();
+    Object *machine = OBJECT(ms);
     const PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(object_class_by_name(type));
 
     if (!features) {
@@ -10171,7 +10172,7 @@ static void ppc_cpu_parse_featurestr(const char *type, char *features,
     }
 
     /* do property processing with generic handler */
-    pcc->parent_parse_features(type, features, errp);
+    pcc->parent_parse_features(ms, type, features, errp);
 }
 
 PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 4a4445bdf5..7e360de5ee 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -115,7 +115,8 @@ cpu_add_feat_as_prop(const char *typename, const char *name, const char *val)
 }
 
 /* Parse "+feature,-feature,feature=foo" CPU feature string */
-static void sparc_cpu_parse_features(const char *typename, char *features,
+static void sparc_cpu_parse_features(MachineState *machine,
+                                     const char *typename, char *features,
                                      Error **errp)
 {
     GList *l, *plus_features = NULL, *minus_features = NULL;
diff --git a/vl.c b/vl.c
index c57e28d1da..e78c4d5a53 100644
--- a/vl.c
+++ b/vl.c
@@ -4466,7 +4466,8 @@ int main(int argc, char **argv, char **envp)
     /* parse features once if machine provides default cpu_type */
     current_machine->cpu_type = machine_class->default_cpu_type;
     if (cpu_option) {
-        current_machine->cpu_type = parse_cpu_option(cpu_option);
+        current_machine->cpu_type =
+            parse_cpu_option(current_machine, cpu_option);
     }
     parse_numa_opts(current_machine);
 
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option()
@ 2019-04-17  5:21     ` David Gibson
  0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-04-17  5:21 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: qemu-devel, Laurent Vivier, Paolo Bonzini, Riku Voipio, Like Xu,
	Richard Henderson, Thomas Huth, Igor Mammedov, qemu-ppc,
	Markus Armbruster, Peter Maydell, Artyom Tarasenko,
	Mark Cave-Ayland

[-- Attachment #1: Type: text/plain, Size: 6043 bytes --]

On Tue, Apr 16, 2019 at 11:59:40PM -0300, Eduardo Habkost wrote:
> The "model[,option...]" string parsed by the function is not just
> a CPU model.  Rename the function and its argument to indicate it
> expects the full "-cpu" option to be provided.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/qom/cpu.h |  6 +++---
>  bsd-user/main.c   |  2 +-
>  exec.c            |  4 ++--
>  linux-user/main.c |  2 +-
>  vl.c              | 18 +++++++++---------
>  5 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 1d6099e5d4..d28c690b27 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -689,15 +689,15 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
>  CPUState *cpu_create(const char *typename);
>  
>  /**
> - * parse_cpu_model:
> - * @cpu_model: The model string including optional parameters.
> + * parse_cpu_option:
> + * @cpu_option: The -cpu option including optional parameters.
>   *
>   * processes optional parameters and registers them as global properties
>   *
>   * Returns: type of CPU to create or prints error and terminates process
>   *          if an error occurred.
>   */
> -const char *parse_cpu_model(const char *cpu_model);
> +const char *parse_cpu_option(const char *cpu_option);
>  
>  /**
>   * cpu_has_work:
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 0d3156974c..a6c055f5fb 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -903,7 +903,7 @@ int main(int argc, char **argv)
>      /* init tcg before creating CPUs and to get qemu_host_page_size */
>      tcg_exec_init(0);
>  
> -    cpu_type = parse_cpu_model(cpu_model);
> +    cpu_type = parse_cpu_option(cpu_model);
>      cpu = cpu_create(cpu_type);
>      env = cpu->env_ptr;
>  #if defined(TARGET_SPARC) || defined(TARGET_PPC)
> diff --git a/exec.c b/exec.c
> index 6ab62f4eee..840677f15f 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -982,14 +982,14 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
>  #endif
>  }
>  
> -const char *parse_cpu_model(const char *cpu_model)
> +const char *parse_cpu_option(const char *cpu_option)
>  {
>      ObjectClass *oc;
>      CPUClass *cc;
>      gchar **model_pieces;
>      const char *cpu_type;
>  
> -    model_pieces = g_strsplit(cpu_model, ",", 2);
> +    model_pieces = g_strsplit(cpu_option, ",", 2);
>  
>      oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
>      if (oc == NULL) {
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a0aba9cb1e..20e0f51cfa 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -660,7 +660,7 @@ int main(int argc, char **argv, char **envp)
>      if (cpu_model == NULL) {
>          cpu_model = cpu_get_model(get_elf_eflags(execfd));
>      }
> -    cpu_type = parse_cpu_model(cpu_model);
> +    cpu_type = parse_cpu_option(cpu_model);
>  
>      /* init tcg before creating CPUs and to get qemu_host_page_size */
>      tcg_exec_init(0);
> diff --git a/vl.c b/vl.c
> index c696ad2a13..c57e28d1da 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3002,7 +3002,7 @@ int main(int argc, char **argv, char **envp)
>      const char *optarg;
>      const char *loadvm = NULL;
>      MachineClass *machine_class;
> -    const char *cpu_model;
> +    const char *cpu_option;
>      const char *vga_model = NULL;
>      const char *qtest_chrdev = NULL;
>      const char *qtest_log = NULL;
> @@ -3081,7 +3081,7 @@ int main(int argc, char **argv, char **envp)
>      QLIST_INIT (&vm_change_state_head);
>      os_setup_early_signal_handling();
>  
> -    cpu_model = NULL;
> +    cpu_option = NULL;
>      snapshot = 0;
>  
>      nb_nics = 0;
> @@ -3133,7 +3133,7 @@ int main(int argc, char **argv, char **envp)
>              switch(popt->index) {
>              case QEMU_OPTION_cpu:
>                  /* hw initialization will check this */
> -                cpu_model = optarg;
> +                cpu_option = optarg;
>                  break;
>              case QEMU_OPTION_hda:
>              case QEMU_OPTION_hdb:
> @@ -4050,8 +4050,8 @@ int main(int argc, char **argv, char **envp)
>          qemu_set_hw_version(machine_class->hw_version);
>      }
>  
> -    if (cpu_model && is_help_option(cpu_model)) {
> -        list_cpus(stdout, &fprintf, cpu_model);
> +    if (cpu_option && is_help_option(cpu_option)) {
> +        list_cpus(stdout, &fprintf, cpu_option);
>          exit(0);
>      }
>  
> @@ -4299,9 +4299,9 @@ int main(int argc, char **argv, char **envp)
>       * Global properties get set up by qdev_prop_register_global(),
>       * called from user_register_global_props(), and certain option
>       * desugaring.  Also in CPU feature desugaring (buried in
> -     * parse_cpu_model()), which happens below this point, but may
> +     * parse_cpu_option()), which happens below this point, but may
>       * only target the CPU type, which can only be created after
> -     * parse_cpu_model() returned the type.
> +     * parse_cpu_option() returned the type.
>       *
>       * Machine compat properties: object_set_machine_compat_props().
>       * Accelerator compat props: object_set_accelerator_compat_props(),
> @@ -4465,8 +4465,8 @@ int main(int argc, char **argv, char **envp)
>  
>      /* parse features once if machine provides default cpu_type */
>      current_machine->cpu_type = machine_class->default_cpu_type;
> -    if (cpu_model) {
> -        current_machine->cpu_type = parse_cpu_model(cpu_model);
> +    if (cpu_option) {
> +        current_machine->cpu_type = parse_cpu_option(cpu_option);
>      }
>      parse_numa_opts(current_machine);
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option()
@ 2019-04-17  5:21     ` David Gibson
  0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-04-17  5:21 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Thomas Huth, Like Xu, Markus Armbruster,
	Riku Voipio, Mark Cave-Ayland, qemu-devel, Laurent Vivier,
	qemu-ppc, Igor Mammedov, Paolo Bonzini, Artyom Tarasenko,
	Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 6043 bytes --]

On Tue, Apr 16, 2019 at 11:59:40PM -0300, Eduardo Habkost wrote:
> The "model[,option...]" string parsed by the function is not just
> a CPU model.  Rename the function and its argument to indicate it
> expects the full "-cpu" option to be provided.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/qom/cpu.h |  6 +++---
>  bsd-user/main.c   |  2 +-
>  exec.c            |  4 ++--
>  linux-user/main.c |  2 +-
>  vl.c              | 18 +++++++++---------
>  5 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 1d6099e5d4..d28c690b27 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -689,15 +689,15 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
>  CPUState *cpu_create(const char *typename);
>  
>  /**
> - * parse_cpu_model:
> - * @cpu_model: The model string including optional parameters.
> + * parse_cpu_option:
> + * @cpu_option: The -cpu option including optional parameters.
>   *
>   * processes optional parameters and registers them as global properties
>   *
>   * Returns: type of CPU to create or prints error and terminates process
>   *          if an error occurred.
>   */
> -const char *parse_cpu_model(const char *cpu_model);
> +const char *parse_cpu_option(const char *cpu_option);
>  
>  /**
>   * cpu_has_work:
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 0d3156974c..a6c055f5fb 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -903,7 +903,7 @@ int main(int argc, char **argv)
>      /* init tcg before creating CPUs and to get qemu_host_page_size */
>      tcg_exec_init(0);
>  
> -    cpu_type = parse_cpu_model(cpu_model);
> +    cpu_type = parse_cpu_option(cpu_model);
>      cpu = cpu_create(cpu_type);
>      env = cpu->env_ptr;
>  #if defined(TARGET_SPARC) || defined(TARGET_PPC)
> diff --git a/exec.c b/exec.c
> index 6ab62f4eee..840677f15f 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -982,14 +982,14 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
>  #endif
>  }
>  
> -const char *parse_cpu_model(const char *cpu_model)
> +const char *parse_cpu_option(const char *cpu_option)
>  {
>      ObjectClass *oc;
>      CPUClass *cc;
>      gchar **model_pieces;
>      const char *cpu_type;
>  
> -    model_pieces = g_strsplit(cpu_model, ",", 2);
> +    model_pieces = g_strsplit(cpu_option, ",", 2);
>  
>      oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
>      if (oc == NULL) {
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a0aba9cb1e..20e0f51cfa 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -660,7 +660,7 @@ int main(int argc, char **argv, char **envp)
>      if (cpu_model == NULL) {
>          cpu_model = cpu_get_model(get_elf_eflags(execfd));
>      }
> -    cpu_type = parse_cpu_model(cpu_model);
> +    cpu_type = parse_cpu_option(cpu_model);
>  
>      /* init tcg before creating CPUs and to get qemu_host_page_size */
>      tcg_exec_init(0);
> diff --git a/vl.c b/vl.c
> index c696ad2a13..c57e28d1da 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3002,7 +3002,7 @@ int main(int argc, char **argv, char **envp)
>      const char *optarg;
>      const char *loadvm = NULL;
>      MachineClass *machine_class;
> -    const char *cpu_model;
> +    const char *cpu_option;
>      const char *vga_model = NULL;
>      const char *qtest_chrdev = NULL;
>      const char *qtest_log = NULL;
> @@ -3081,7 +3081,7 @@ int main(int argc, char **argv, char **envp)
>      QLIST_INIT (&vm_change_state_head);
>      os_setup_early_signal_handling();
>  
> -    cpu_model = NULL;
> +    cpu_option = NULL;
>      snapshot = 0;
>  
>      nb_nics = 0;
> @@ -3133,7 +3133,7 @@ int main(int argc, char **argv, char **envp)
>              switch(popt->index) {
>              case QEMU_OPTION_cpu:
>                  /* hw initialization will check this */
> -                cpu_model = optarg;
> +                cpu_option = optarg;
>                  break;
>              case QEMU_OPTION_hda:
>              case QEMU_OPTION_hdb:
> @@ -4050,8 +4050,8 @@ int main(int argc, char **argv, char **envp)
>          qemu_set_hw_version(machine_class->hw_version);
>      }
>  
> -    if (cpu_model && is_help_option(cpu_model)) {
> -        list_cpus(stdout, &fprintf, cpu_model);
> +    if (cpu_option && is_help_option(cpu_option)) {
> +        list_cpus(stdout, &fprintf, cpu_option);
>          exit(0);
>      }
>  
> @@ -4299,9 +4299,9 @@ int main(int argc, char **argv, char **envp)
>       * Global properties get set up by qdev_prop_register_global(),
>       * called from user_register_global_props(), and certain option
>       * desugaring.  Also in CPU feature desugaring (buried in
> -     * parse_cpu_model()), which happens below this point, but may
> +     * parse_cpu_option()), which happens below this point, but may
>       * only target the CPU type, which can only be created after
> -     * parse_cpu_model() returned the type.
> +     * parse_cpu_option() returned the type.
>       *
>       * Machine compat properties: object_set_machine_compat_props().
>       * Accelerator compat props: object_set_accelerator_compat_props(),
> @@ -4465,8 +4465,8 @@ int main(int argc, char **argv, char **envp)
>  
>      /* parse features once if machine provides default cpu_type */
>      current_machine->cpu_type = machine_class->default_cpu_type;
> -    if (cpu_model) {
> -        current_machine->cpu_type = parse_cpu_model(cpu_model);
> +    if (cpu_option) {
> +        current_machine->cpu_type = parse_cpu_option(cpu_option);
>      }
>      parse_numa_opts(current_machine);
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()
@ 2019-04-17  5:22     ` David Gibson
  0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-04-17  5:22 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: qemu-devel, Laurent Vivier, Paolo Bonzini, Riku Voipio, Like Xu,
	Richard Henderson, Thomas Huth, Igor Mammedov, qemu-ppc,
	Markus Armbruster, Peter Maydell, Artyom Tarasenko,
	Mark Cave-Ayland

[-- Attachment #1: Type: text/plain, Size: 2582 bytes --]

On Tue, Apr 16, 2019 at 11:59:41PM -0300, Eduardo Habkost wrote:
> The new function will be useful in user mode, when we already
> have a CPU model and don't need to parse any extra options.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/qom/cpu.h |  9 +++++++++
>  exec.c            | 22 ++++++++++++----------
>  2 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index d28c690b27..e11b14d9ac 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -699,6 +699,15 @@ CPUState *cpu_create(const char *typename);
>   */
>  const char *parse_cpu_option(const char *cpu_option);
>  
> +/**
> + * lookup_cpu_class:
> + * @cpu_model: CPU model name
> + *
> + * Look up CPU class corresponding to a given CPU model name.
> + */
> +CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp);
> +
> +
>  /**
>   * cpu_has_work:
>   * @cpu: The vCPU to check.
> diff --git a/exec.c b/exec.c
> index 840677f15f..d359e709a6 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -982,24 +982,26 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
>  #endif
>  }
>  
> +CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp)
> +{
> +    ObjectClass *oc = cpu_class_by_name(CPU_RESOLVING_TYPE, cpu_model);
> +    if (oc == NULL) {
> +        error_setg(errp, "unable to find CPU model '%s'", cpu_model);
> +        return NULL;
> +    }
> +    return CPU_CLASS(oc);
> +}
> +
>  const char *parse_cpu_option(const char *cpu_option)
>  {
> -    ObjectClass *oc;
>      CPUClass *cc;
>      gchar **model_pieces;
>      const char *cpu_type;
>  
>      model_pieces = g_strsplit(cpu_option, ",", 2);
>  
> -    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
> -    if (oc == NULL) {
> -        error_report("unable to find CPU model '%s'", model_pieces[0]);
> -        g_strfreev(model_pieces);
> -        exit(EXIT_FAILURE);
> -    }
> -
> -    cpu_type = object_class_get_name(oc);
> -    cc = CPU_CLASS(oc);
> +    cc = lookup_cpu_class(model_pieces[0], &error_fatal);
> +    cpu_type = object_class_get_name(OBJECT_CLASS(cc));
>      cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
>      g_strfreev(model_pieces);
>      return cpu_type;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()
@ 2019-04-17  5:22     ` David Gibson
  0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-04-17  5:22 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Thomas Huth, Like Xu, Markus Armbruster,
	Riku Voipio, Mark Cave-Ayland, qemu-devel, Laurent Vivier,
	qemu-ppc, Igor Mammedov, Paolo Bonzini, Artyom Tarasenko,
	Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 2582 bytes --]

On Tue, Apr 16, 2019 at 11:59:41PM -0300, Eduardo Habkost wrote:
> The new function will be useful in user mode, when we already
> have a CPU model and don't need to parse any extra options.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/qom/cpu.h |  9 +++++++++
>  exec.c            | 22 ++++++++++++----------
>  2 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index d28c690b27..e11b14d9ac 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -699,6 +699,15 @@ CPUState *cpu_create(const char *typename);
>   */
>  const char *parse_cpu_option(const char *cpu_option);
>  
> +/**
> + * lookup_cpu_class:
> + * @cpu_model: CPU model name
> + *
> + * Look up CPU class corresponding to a given CPU model name.
> + */
> +CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp);
> +
> +
>  /**
>   * cpu_has_work:
>   * @cpu: The vCPU to check.
> diff --git a/exec.c b/exec.c
> index 840677f15f..d359e709a6 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -982,24 +982,26 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
>  #endif
>  }
>  
> +CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp)
> +{
> +    ObjectClass *oc = cpu_class_by_name(CPU_RESOLVING_TYPE, cpu_model);
> +    if (oc == NULL) {
> +        error_setg(errp, "unable to find CPU model '%s'", cpu_model);
> +        return NULL;
> +    }
> +    return CPU_CLASS(oc);
> +}
> +
>  const char *parse_cpu_option(const char *cpu_option)
>  {
> -    ObjectClass *oc;
>      CPUClass *cc;
>      gchar **model_pieces;
>      const char *cpu_type;
>  
>      model_pieces = g_strsplit(cpu_option, ",", 2);
>  
> -    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
> -    if (oc == NULL) {
> -        error_report("unable to find CPU model '%s'", model_pieces[0]);
> -        g_strfreev(model_pieces);
> -        exit(EXIT_FAILURE);
> -    }
> -
> -    cpu_type = object_class_get_name(oc);
> -    cc = CPU_CLASS(oc);
> +    cc = lookup_cpu_class(model_pieces[0], &error_fatal);
> +    cpu_type = object_class_get_name(OBJECT_CLASS(cc));
>      cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
>      g_strfreev(model_pieces);
>      return cpu_type;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class()
@ 2019-04-17  5:23     ` David Gibson
  0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-04-17  5:23 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: qemu-devel, Laurent Vivier, Paolo Bonzini, Riku Voipio, Like Xu,
	Richard Henderson, Thomas Huth, Igor Mammedov, qemu-ppc,
	Markus Armbruster, Peter Maydell, Artyom Tarasenko,
	Mark Cave-Ayland

[-- Attachment #1: Type: text/plain, Size: 1463 bytes --]

On Tue, Apr 16, 2019 at 11:59:42PM -0300, Eduardo Habkost wrote:
> The return value of cpu_get_model() is just a CPU model name and
> never includes extra options.  We don't need to call
> parse_cpu_option().
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  linux-user/main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 20e0f51cfa..d74108e05c 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -592,6 +592,7 @@ int main(int argc, char **argv, char **envp)
>      TaskState *ts;
>      CPUArchState *env;
>      CPUState *cpu;
> +    CPUClass *cc;
>      int optind;
>      char **target_environ, **wrk;
>      char **target_argv;
> @@ -660,7 +661,8 @@ int main(int argc, char **argv, char **envp)
>      if (cpu_model == NULL) {
>          cpu_model = cpu_get_model(get_elf_eflags(execfd));
>      }
> -    cpu_type = parse_cpu_option(cpu_model);
> +    cc = lookup_cpu_class(cpu_model, &error_fatal);
> +    cpu_type = object_class_get_name(OBJECT_CLASS(cc));
>  
>      /* init tcg before creating CPUs and to get qemu_host_page_size */
>      tcg_exec_init(0);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class()
@ 2019-04-17  5:23     ` David Gibson
  0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-04-17  5:23 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Thomas Huth, Like Xu, Markus Armbruster,
	Riku Voipio, Mark Cave-Ayland, qemu-devel, Laurent Vivier,
	qemu-ppc, Igor Mammedov, Paolo Bonzini, Artyom Tarasenko,
	Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 1463 bytes --]

On Tue, Apr 16, 2019 at 11:59:42PM -0300, Eduardo Habkost wrote:
> The return value of cpu_get_model() is just a CPU model name and
> never includes extra options.  We don't need to call
> parse_cpu_option().
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  linux-user/main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 20e0f51cfa..d74108e05c 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -592,6 +592,7 @@ int main(int argc, char **argv, char **envp)
>      TaskState *ts;
>      CPUArchState *env;
>      CPUState *cpu;
> +    CPUClass *cc;
>      int optind;
>      char **target_environ, **wrk;
>      char **target_argv;
> @@ -660,7 +661,8 @@ int main(int argc, char **argv, char **envp)
>      if (cpu_model == NULL) {
>          cpu_model = cpu_get_model(get_elf_eflags(execfd));
>      }
> -    cpu_type = parse_cpu_option(cpu_model);
> +    cc = lookup_cpu_class(cpu_model, &error_fatal);
> +    cpu_type = object_class_get_name(OBJECT_CLASS(cc));
>  
>      /* init tcg before creating CPUs and to get qemu_host_page_size */
>      tcg_exec_init(0);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 4/5] bsd-user: Use lookup_cpu_class()
@ 2019-04-17  5:23     ` David Gibson
  0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-04-17  5:23 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: qemu-devel, Laurent Vivier, Paolo Bonzini, Riku Voipio, Like Xu,
	Richard Henderson, Thomas Huth, Igor Mammedov, qemu-ppc,
	Markus Armbruster, Peter Maydell, Artyom Tarasenko,
	Mark Cave-Ayland

[-- Attachment #1: Type: text/plain, Size: 1437 bytes --]

On Tue, Apr 16, 2019 at 11:59:43PM -0300, Eduardo Habkost wrote:
> The hardcoded CPU models in the code are just CPU models and
> don't include any extra options.  We don't need to call
> parse_cpu_options().
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  bsd-user/main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index a6c055f5fb..d2915a9951 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -732,6 +732,7 @@ int main(int argc, char **argv)
>      TaskState ts1, *ts = &ts1;
>      CPUArchState *env;
>      CPUState *cpu;
> +    CPUClass *cc;
>      int optind;
>      const char *r;
>      int gdbstub_port = 0;
> @@ -903,7 +904,8 @@ int main(int argc, char **argv)
>      /* init tcg before creating CPUs and to get qemu_host_page_size */
>      tcg_exec_init(0);
>  
> -    cpu_type = parse_cpu_option(cpu_model);
> +    cc = lookup_cpu_class(cpu_model, &error_fatal);
> +    cpu_type = object_class_get_name(OBJECT_CLASS(cc));
>      cpu = cpu_create(cpu_type);
>      env = cpu->env_ptr;
>  #if defined(TARGET_SPARC) || defined(TARGET_PPC)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 4/5] bsd-user: Use lookup_cpu_class()
@ 2019-04-17  5:23     ` David Gibson
  0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-04-17  5:23 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Thomas Huth, Like Xu, Markus Armbruster,
	Riku Voipio, Mark Cave-Ayland, qemu-devel, Laurent Vivier,
	qemu-ppc, Igor Mammedov, Paolo Bonzini, Artyom Tarasenko,
	Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 1437 bytes --]

On Tue, Apr 16, 2019 at 11:59:43PM -0300, Eduardo Habkost wrote:
> The hardcoded CPU models in the code are just CPU models and
> don't include any extra options.  We don't need to call
> parse_cpu_options().
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  bsd-user/main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index a6c055f5fb..d2915a9951 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -732,6 +732,7 @@ int main(int argc, char **argv)
>      TaskState ts1, *ts = &ts1;
>      CPUArchState *env;
>      CPUState *cpu;
> +    CPUClass *cc;
>      int optind;
>      const char *r;
>      int gdbstub_port = 0;
> @@ -903,7 +904,8 @@ int main(int argc, char **argv)
>      /* init tcg before creating CPUs and to get qemu_host_page_size */
>      tcg_exec_init(0);
>  
> -    cpu_type = parse_cpu_option(cpu_model);
> +    cc = lookup_cpu_class(cpu_model, &error_fatal);
> +    cpu_type = object_class_get_name(OBJECT_CLASS(cc));
>      cpu = cpu_create(cpu_type);
>      env = cpu->env_ptr;
>  #if defined(TARGET_SPARC) || defined(TARGET_PPC)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features()
@ 2019-04-17  5:25     ` David Gibson
  0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-04-17  5:25 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: qemu-devel, Laurent Vivier, Paolo Bonzini, Riku Voipio, Like Xu,
	Richard Henderson, Thomas Huth, Igor Mammedov, qemu-ppc,
	Markus Armbruster, Peter Maydell, Artyom Tarasenko,
	Mark Cave-Ayland

[-- Attachment #1: Type: text/plain, Size: 7414 bytes --]

On Tue, Apr 16, 2019 at 11:59:44PM -0300, Eduardo Habkost wrote:
> The ppc implementation of parse_features() requires the machine
> object to be created before it gets called.  This is far from
> obvious when reading the code at main().
> 
> Instead of making it call qdev_get_machine(), require the caller
> of parse_cpu_option() to provide the machine object.
> 
> This makes the initialization dependency explicit at main(), and
> will let us move qdev_get_machine() to CONFIG_SOFTMMU in the
> future.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

and ppc parts

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/qom/cpu.h               | 5 +++--
>  target/ppc/cpu-qom.h            | 3 ++-
>  exec.c                          | 4 ++--
>  qom/cpu.c                       | 3 ++-
>  target/i386/cpu.c               | 3 ++-
>  target/ppc/translate_init.inc.c | 7 ++++---
>  target/sparc/cpu.c              | 3 ++-
>  vl.c                            | 3 ++-
>  8 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index e11b14d9ac..cbc8e103bb 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -164,7 +164,8 @@ typedef struct CPUClass {
>      /*< public >*/
>  
>      ObjectClass *(*class_by_name)(const char *cpu_model);
> -    void (*parse_features)(const char *typename, char *str, Error **errp);
> +    void (*parse_features)(MachineState *machine, const char *typename,
> +                           char *str, Error **errp);
>  
>      void (*reset)(CPUState *cpu);
>      int reset_dump_flags;
> @@ -697,7 +698,7 @@ CPUState *cpu_create(const char *typename);
>   * Returns: type of CPU to create or prints error and terminates process
>   *          if an error occurred.
>   */
> -const char *parse_cpu_option(const char *cpu_option);
> +const char *parse_cpu_option(MachineState *machine, const char *cpu_option);
>  
>  /**
>   * lookup_cpu_class:
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index be9b4c30c3..7891465554 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -167,7 +167,8 @@ typedef struct PowerPCCPUClass {
>      DeviceRealize parent_realize;
>      DeviceUnrealize parent_unrealize;
>      void (*parent_reset)(CPUState *cpu);
> -    void (*parent_parse_features)(const char *type, char *str, Error **errp);
> +    void (*parent_parse_features)(MachineState *machine, const char *type,
> +                                  char *str, Error **errp);
>  
>      uint32_t pvr;
>      bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
> diff --git a/exec.c b/exec.c
> index d359e709a6..1ca95df9d8 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -992,7 +992,7 @@ CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp)
>      return CPU_CLASS(oc);
>  }
>  
> -const char *parse_cpu_option(const char *cpu_option)
> +const char *parse_cpu_option(MachineState *machine, const char *cpu_option)
>  {
>      CPUClass *cc;
>      gchar **model_pieces;
> @@ -1002,7 +1002,7 @@ const char *parse_cpu_option(const char *cpu_option)
>  
>      cc = lookup_cpu_class(model_pieces[0], &error_fatal);
>      cpu_type = object_class_get_name(OBJECT_CLASS(cc));
> -    cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
> +    cc->parse_features(machine, cpu_type, model_pieces[1], &error_fatal);
>      g_strfreev(model_pieces);
>      return cpu_type;
>  }
> diff --git a/qom/cpu.c b/qom/cpu.c
> index a8d2958956..c8a7b56148 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -291,7 +291,8 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
>      return cc->class_by_name(cpu_model);
>  }
>  
> -static void cpu_common_parse_features(const char *typename, char *features,
> +static void cpu_common_parse_features(MachineState *machine,
> +                                      const char *typename, char *features,
>                                        Error **errp)
>  {
>      char *val;
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index d6bb57d210..f5e15ac5da 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3528,7 +3528,8 @@ static gint compare_string(gconstpointer a, gconstpointer b)
>  
>  /* Parse "+feature,-feature,feature=foo" CPU feature string
>   */
> -static void x86_cpu_parse_featurestr(const char *typename, char *features,
> +static void x86_cpu_parse_featurestr(MachineState *machine,
> +                                     const char *typename, char *features,
>                                       Error **errp)
>  {
>      char *featurestr; /* Single 'key=value" string being parsed */
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index 0bd555eb19..2ad223fcca 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -10119,10 +10119,11 @@ static ObjectClass *ppc_cpu_class_by_name(const char *name)
>      return oc;
>  }
>  
> -static void ppc_cpu_parse_featurestr(const char *type, char *features,
> +static void ppc_cpu_parse_featurestr(MachineState *ms,
> +                                     const char *type, char *features,
>                                       Error **errp)
>  {
> -    Object *machine = qdev_get_machine();
> +    Object *machine = OBJECT(ms);
>      const PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(object_class_by_name(type));
>  
>      if (!features) {
> @@ -10171,7 +10172,7 @@ static void ppc_cpu_parse_featurestr(const char *type, char *features,
>      }
>  
>      /* do property processing with generic handler */
> -    pcc->parent_parse_features(type, features, errp);
> +    pcc->parent_parse_features(ms, type, features, errp);
>  }
>  
>  PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index 4a4445bdf5..7e360de5ee 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -115,7 +115,8 @@ cpu_add_feat_as_prop(const char *typename, const char *name, const char *val)
>  }
>  
>  /* Parse "+feature,-feature,feature=foo" CPU feature string */
> -static void sparc_cpu_parse_features(const char *typename, char *features,
> +static void sparc_cpu_parse_features(MachineState *machine,
> +                                     const char *typename, char *features,
>                                       Error **errp)
>  {
>      GList *l, *plus_features = NULL, *minus_features = NULL;
> diff --git a/vl.c b/vl.c
> index c57e28d1da..e78c4d5a53 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4466,7 +4466,8 @@ int main(int argc, char **argv, char **envp)
>      /* parse features once if machine provides default cpu_type */
>      current_machine->cpu_type = machine_class->default_cpu_type;
>      if (cpu_option) {
> -        current_machine->cpu_type = parse_cpu_option(cpu_option);
> +        current_machine->cpu_type =
> +            parse_cpu_option(current_machine, cpu_option);
>      }
>      parse_numa_opts(current_machine);
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features()
@ 2019-04-17  5:25     ` David Gibson
  0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-04-17  5:25 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Thomas Huth, Like Xu, Markus Armbruster,
	Riku Voipio, Mark Cave-Ayland, qemu-devel, Laurent Vivier,
	qemu-ppc, Igor Mammedov, Paolo Bonzini, Artyom Tarasenko,
	Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 7414 bytes --]

On Tue, Apr 16, 2019 at 11:59:44PM -0300, Eduardo Habkost wrote:
> The ppc implementation of parse_features() requires the machine
> object to be created before it gets called.  This is far from
> obvious when reading the code at main().
> 
> Instead of making it call qdev_get_machine(), require the caller
> of parse_cpu_option() to provide the machine object.
> 
> This makes the initialization dependency explicit at main(), and
> will let us move qdev_get_machine() to CONFIG_SOFTMMU in the
> future.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

and ppc parts

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/qom/cpu.h               | 5 +++--
>  target/ppc/cpu-qom.h            | 3 ++-
>  exec.c                          | 4 ++--
>  qom/cpu.c                       | 3 ++-
>  target/i386/cpu.c               | 3 ++-
>  target/ppc/translate_init.inc.c | 7 ++++---
>  target/sparc/cpu.c              | 3 ++-
>  vl.c                            | 3 ++-
>  8 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index e11b14d9ac..cbc8e103bb 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -164,7 +164,8 @@ typedef struct CPUClass {
>      /*< public >*/
>  
>      ObjectClass *(*class_by_name)(const char *cpu_model);
> -    void (*parse_features)(const char *typename, char *str, Error **errp);
> +    void (*parse_features)(MachineState *machine, const char *typename,
> +                           char *str, Error **errp);
>  
>      void (*reset)(CPUState *cpu);
>      int reset_dump_flags;
> @@ -697,7 +698,7 @@ CPUState *cpu_create(const char *typename);
>   * Returns: type of CPU to create or prints error and terminates process
>   *          if an error occurred.
>   */
> -const char *parse_cpu_option(const char *cpu_option);
> +const char *parse_cpu_option(MachineState *machine, const char *cpu_option);
>  
>  /**
>   * lookup_cpu_class:
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index be9b4c30c3..7891465554 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -167,7 +167,8 @@ typedef struct PowerPCCPUClass {
>      DeviceRealize parent_realize;
>      DeviceUnrealize parent_unrealize;
>      void (*parent_reset)(CPUState *cpu);
> -    void (*parent_parse_features)(const char *type, char *str, Error **errp);
> +    void (*parent_parse_features)(MachineState *machine, const char *type,
> +                                  char *str, Error **errp);
>  
>      uint32_t pvr;
>      bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
> diff --git a/exec.c b/exec.c
> index d359e709a6..1ca95df9d8 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -992,7 +992,7 @@ CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp)
>      return CPU_CLASS(oc);
>  }
>  
> -const char *parse_cpu_option(const char *cpu_option)
> +const char *parse_cpu_option(MachineState *machine, const char *cpu_option)
>  {
>      CPUClass *cc;
>      gchar **model_pieces;
> @@ -1002,7 +1002,7 @@ const char *parse_cpu_option(const char *cpu_option)
>  
>      cc = lookup_cpu_class(model_pieces[0], &error_fatal);
>      cpu_type = object_class_get_name(OBJECT_CLASS(cc));
> -    cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
> +    cc->parse_features(machine, cpu_type, model_pieces[1], &error_fatal);
>      g_strfreev(model_pieces);
>      return cpu_type;
>  }
> diff --git a/qom/cpu.c b/qom/cpu.c
> index a8d2958956..c8a7b56148 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -291,7 +291,8 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
>      return cc->class_by_name(cpu_model);
>  }
>  
> -static void cpu_common_parse_features(const char *typename, char *features,
> +static void cpu_common_parse_features(MachineState *machine,
> +                                      const char *typename, char *features,
>                                        Error **errp)
>  {
>      char *val;
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index d6bb57d210..f5e15ac5da 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3528,7 +3528,8 @@ static gint compare_string(gconstpointer a, gconstpointer b)
>  
>  /* Parse "+feature,-feature,feature=foo" CPU feature string
>   */
> -static void x86_cpu_parse_featurestr(const char *typename, char *features,
> +static void x86_cpu_parse_featurestr(MachineState *machine,
> +                                     const char *typename, char *features,
>                                       Error **errp)
>  {
>      char *featurestr; /* Single 'key=value" string being parsed */
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index 0bd555eb19..2ad223fcca 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -10119,10 +10119,11 @@ static ObjectClass *ppc_cpu_class_by_name(const char *name)
>      return oc;
>  }
>  
> -static void ppc_cpu_parse_featurestr(const char *type, char *features,
> +static void ppc_cpu_parse_featurestr(MachineState *ms,
> +                                     const char *type, char *features,
>                                       Error **errp)
>  {
> -    Object *machine = qdev_get_machine();
> +    Object *machine = OBJECT(ms);
>      const PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(object_class_by_name(type));
>  
>      if (!features) {
> @@ -10171,7 +10172,7 @@ static void ppc_cpu_parse_featurestr(const char *type, char *features,
>      }
>  
>      /* do property processing with generic handler */
> -    pcc->parent_parse_features(type, features, errp);
> +    pcc->parent_parse_features(ms, type, features, errp);
>  }
>  
>  PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index 4a4445bdf5..7e360de5ee 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -115,7 +115,8 @@ cpu_add_feat_as_prop(const char *typename, const char *name, const char *val)
>  }
>  
>  /* Parse "+feature,-feature,feature=foo" CPU feature string */
> -static void sparc_cpu_parse_features(const char *typename, char *features,
> +static void sparc_cpu_parse_features(MachineState *machine,
> +                                     const char *typename, char *features,
>                                       Error **errp)
>  {
>      GList *l, *plus_features = NULL, *minus_features = NULL;
> diff --git a/vl.c b/vl.c
> index c57e28d1da..e78c4d5a53 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4466,7 +4466,8 @@ int main(int argc, char **argv, char **envp)
>      /* parse features once if machine provides default cpu_type */
>      current_machine->cpu_type = machine_class->default_cpu_type;
>      if (cpu_option) {
> -        current_machine->cpu_type = parse_cpu_option(cpu_option);
> +        current_machine->cpu_type =
> +            parse_cpu_option(current_machine, cpu_option);
>      }
>      parse_numa_opts(current_machine);
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()
@ 2019-04-17  5:41     ` Markus Armbruster
  0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2019-04-17  5:41 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: qemu-devel, Peter Maydell, Thomas Huth, Like Xu, Riku Voipio,
	Mark Cave-Ayland, Laurent Vivier, qemu-ppc, Igor Mammedov,
	Paolo Bonzini, David Gibson, Artyom Tarasenko, Richard Henderson

Eduardo Habkost <ehabkost@redhat.com> writes:

> The new function will be useful in user mode, when we already
> have a CPU model and don't need to parse any extra options.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  include/qom/cpu.h |  9 +++++++++
>  exec.c            | 22 ++++++++++++----------
>  2 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index d28c690b27..e11b14d9ac 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -699,6 +699,15 @@ CPUState *cpu_create(const char *typename);
>   */
>  const char *parse_cpu_option(const char *cpu_option);
>  
> +/**
> + * lookup_cpu_class:
> + * @cpu_model: CPU model name
> + *
> + * Look up CPU class corresponding to a given CPU model name.
> + */
> +CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp);

Nitpicks, feel free to ignore.

Naming: lookup_cpu_class() makes my reading circuits stall momentarily,
because lookup is a noun. The verb is spelled look up.  No stall:
cpu_class_lookup(), look_up_cpu_class(), cpu_class_by_name_err(),
cpu_class_parse(), ...

Doc string: this is a wrapper around cpu_class_by_name().  Perhaps that
should be spelled out.

Apropos cpu_class_by_name(): it returns ObjectClass, not CPUClass.
Isn't that odd?

Position: If you put it before cpu_create() rather than after, it's next
to the function it wraps.  But perhaps you prefer to keep it next to
parse_cpu_option().

[...]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()
@ 2019-04-17  5:41     ` Markus Armbruster
  0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2019-04-17  5:41 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Thomas Huth, Like Xu, Riku Voipio,
	Mark Cave-Ayland, qemu-devel, Laurent Vivier, qemu-ppc,
	Paolo Bonzini, Igor Mammedov, Richard Henderson,
	Artyom Tarasenko, David Gibson

Eduardo Habkost <ehabkost@redhat.com> writes:

> The new function will be useful in user mode, when we already
> have a CPU model and don't need to parse any extra options.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  include/qom/cpu.h |  9 +++++++++
>  exec.c            | 22 ++++++++++++----------
>  2 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index d28c690b27..e11b14d9ac 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -699,6 +699,15 @@ CPUState *cpu_create(const char *typename);
>   */
>  const char *parse_cpu_option(const char *cpu_option);
>  
> +/**
> + * lookup_cpu_class:
> + * @cpu_model: CPU model name
> + *
> + * Look up CPU class corresponding to a given CPU model name.
> + */
> +CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp);

Nitpicks, feel free to ignore.

Naming: lookup_cpu_class() makes my reading circuits stall momentarily,
because lookup is a noun. The verb is spelled look up.  No stall:
cpu_class_lookup(), look_up_cpu_class(), cpu_class_by_name_err(),
cpu_class_parse(), ...

Doc string: this is a wrapper around cpu_class_by_name().  Perhaps that
should be spelled out.

Apropos cpu_class_by_name(): it returns ObjectClass, not CPUClass.
Isn't that odd?

Position: If you put it before cpu_create() rather than after, it's next
to the function it wraps.  But perhaps you prefer to keep it next to
parse_cpu_option().

[...]


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features()
@ 2019-04-17  5:45     ` Markus Armbruster
  0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2019-04-17  5:45 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: qemu-devel, Peter Maydell, Thomas Huth, Like Xu, Riku Voipio,
	Mark Cave-Ayland, Laurent Vivier, qemu-ppc, Igor Mammedov,
	Paolo Bonzini, David Gibson, Artyom Tarasenko, Richard Henderson

Eduardo Habkost <ehabkost@redhat.com> writes:

> The ppc implementation of parse_features() requires the machine
> object to be created before it gets called.  This is far from
> obvious when reading the code at main().
>
> Instead of making it call qdev_get_machine(), require the caller
> of parse_cpu_option() to provide the machine object.
>
> This makes the initialization dependency explicit at main(), and
> will let us move qdev_get_machine() to CONFIG_SOFTMMU in the
> future.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  include/qom/cpu.h               | 5 +++--
>  target/ppc/cpu-qom.h            | 3 ++-
>  exec.c                          | 4 ++--
>  qom/cpu.c                       | 3 ++-
>  target/i386/cpu.c               | 3 ++-
>  target/ppc/translate_init.inc.c | 7 ++++---
>  target/sparc/cpu.c              | 3 ++-
>  vl.c                            | 3 ++-
>  8 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index e11b14d9ac..cbc8e103bb 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -164,7 +164,8 @@ typedef struct CPUClass {
>      /*< public >*/
>  
>      ObjectClass *(*class_by_name)(const char *cpu_model);
> -    void (*parse_features)(const char *typename, char *str, Error **errp);
> +    void (*parse_features)(MachineState *machine, const char *typename,
> +                           char *str, Error **errp);
>  
>      void (*reset)(CPUState *cpu);
>      int reset_dump_flags;
> @@ -697,7 +698,7 @@ CPUState *cpu_create(const char *typename);
>   * Returns: type of CPU to create or prints error and terminates process
>   *          if an error occurred.
>   */
> -const char *parse_cpu_option(const char *cpu_option);
> +const char *parse_cpu_option(MachineState *machine, const char *cpu_option);
>  
>  /**
>   * lookup_cpu_class:
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index be9b4c30c3..7891465554 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -167,7 +167,8 @@ typedef struct PowerPCCPUClass {
>      DeviceRealize parent_realize;
>      DeviceUnrealize parent_unrealize;
>      void (*parent_reset)(CPUState *cpu);
> -    void (*parent_parse_features)(const char *type, char *str, Error **errp);
> +    void (*parent_parse_features)(MachineState *machine, const char *type,
> +                                  char *str, Error **errp);
>  
>      uint32_t pvr;
>      bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
> diff --git a/exec.c b/exec.c
> index d359e709a6..1ca95df9d8 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -992,7 +992,7 @@ CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp)
>      return CPU_CLASS(oc);
>  }
>  
> -const char *parse_cpu_option(const char *cpu_option)
> +const char *parse_cpu_option(MachineState *machine, const char *cpu_option)
>  {
>      CPUClass *cc;
>      gchar **model_pieces;
> @@ -1002,7 +1002,7 @@ const char *parse_cpu_option(const char *cpu_option)
>  
>      cc = lookup_cpu_class(model_pieces[0], &error_fatal);
>      cpu_type = object_class_get_name(OBJECT_CLASS(cc));
> -    cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
> +    cc->parse_features(machine, cpu_type, model_pieces[1], &error_fatal);
>      g_strfreev(model_pieces);
>      return cpu_type;
>  }
> diff --git a/qom/cpu.c b/qom/cpu.c
> index a8d2958956..c8a7b56148 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -291,7 +291,8 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
>      return cc->class_by_name(cpu_model);
>  }
>  
> -static void cpu_common_parse_features(const char *typename, char *features,
> +static void cpu_common_parse_features(MachineState *machine,
> +                                      const char *typename, char *features,
>                                        Error **errp)
>  {
>      char *val;
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index d6bb57d210..f5e15ac5da 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3528,7 +3528,8 @@ static gint compare_string(gconstpointer a, gconstpointer b)
>  
>  /* Parse "+feature,-feature,feature=foo" CPU feature string
>   */
> -static void x86_cpu_parse_featurestr(const char *typename, char *features,
> +static void x86_cpu_parse_featurestr(MachineState *machine,
> +                                     const char *typename, char *features,
>                                       Error **errp)
>  {
>      char *featurestr; /* Single 'key=value" string being parsed */
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index 0bd555eb19..2ad223fcca 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -10119,10 +10119,11 @@ static ObjectClass *ppc_cpu_class_by_name(const char *name)
>      return oc;
>  }
>  
> -static void ppc_cpu_parse_featurestr(const char *type, char *features,
> +static void ppc_cpu_parse_featurestr(MachineState *ms,
> +                                     const char *type, char *features,
>                                       Error **errp)
>  {
> -    Object *machine = qdev_get_machine();
> +    Object *machine = OBJECT(ms);
>      const PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(object_class_by_name(type));
>  
>      if (!features) {
> @@ -10171,7 +10172,7 @@ static void ppc_cpu_parse_featurestr(const char *type, char *features,
>      }
>  
>      /* do property processing with generic handler */
> -    pcc->parent_parse_features(type, features, errp);
> +    pcc->parent_parse_features(ms, type, features, errp);
>  }
>  
>  PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index 4a4445bdf5..7e360de5ee 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -115,7 +115,8 @@ cpu_add_feat_as_prop(const char *typename, const char *name, const char *val)
>  }
>  
>  /* Parse "+feature,-feature,feature=foo" CPU feature string */
> -static void sparc_cpu_parse_features(const char *typename, char *features,
> +static void sparc_cpu_parse_features(MachineState *machine,
> +                                     const char *typename, char *features,
>                                       Error **errp)
>  {
>      GList *l, *plus_features = NULL, *minus_features = NULL;
> diff --git a/vl.c b/vl.c
> index c57e28d1da..e78c4d5a53 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4466,7 +4466,8 @@ int main(int argc, char **argv, char **envp)
>      /* parse features once if machine provides default cpu_type */
>      current_machine->cpu_type = machine_class->default_cpu_type;
>      if (cpu_option) {
> -        current_machine->cpu_type = parse_cpu_option(cpu_option);
> +        current_machine->cpu_type =
> +            parse_cpu_option(current_machine, cpu_option);
>      }
>      parse_numa_opts(current_machine);

Well, it's not quite as bad as the commit message made me expect: since
we assign to current_machine->cpu_type, we already have a dependence on
machine creation.

Regardless, replacing qdev_get_machine() by explicit data flow is an
improvement.  More so if it really lets us confine qdev_get_machine() to
CONFIG_SOFTMMU.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features()
@ 2019-04-17  5:45     ` Markus Armbruster
  0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2019-04-17  5:45 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Thomas Huth, Like Xu, Riku Voipio,
	Mark Cave-Ayland, qemu-devel, Laurent Vivier, qemu-ppc,
	Paolo Bonzini, Igor Mammedov, Richard Henderson,
	Artyom Tarasenko, David Gibson

Eduardo Habkost <ehabkost@redhat.com> writes:

> The ppc implementation of parse_features() requires the machine
> object to be created before it gets called.  This is far from
> obvious when reading the code at main().
>
> Instead of making it call qdev_get_machine(), require the caller
> of parse_cpu_option() to provide the machine object.
>
> This makes the initialization dependency explicit at main(), and
> will let us move qdev_get_machine() to CONFIG_SOFTMMU in the
> future.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  include/qom/cpu.h               | 5 +++--
>  target/ppc/cpu-qom.h            | 3 ++-
>  exec.c                          | 4 ++--
>  qom/cpu.c                       | 3 ++-
>  target/i386/cpu.c               | 3 ++-
>  target/ppc/translate_init.inc.c | 7 ++++---
>  target/sparc/cpu.c              | 3 ++-
>  vl.c                            | 3 ++-
>  8 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index e11b14d9ac..cbc8e103bb 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -164,7 +164,8 @@ typedef struct CPUClass {
>      /*< public >*/
>  
>      ObjectClass *(*class_by_name)(const char *cpu_model);
> -    void (*parse_features)(const char *typename, char *str, Error **errp);
> +    void (*parse_features)(MachineState *machine, const char *typename,
> +                           char *str, Error **errp);
>  
>      void (*reset)(CPUState *cpu);
>      int reset_dump_flags;
> @@ -697,7 +698,7 @@ CPUState *cpu_create(const char *typename);
>   * Returns: type of CPU to create or prints error and terminates process
>   *          if an error occurred.
>   */
> -const char *parse_cpu_option(const char *cpu_option);
> +const char *parse_cpu_option(MachineState *machine, const char *cpu_option);
>  
>  /**
>   * lookup_cpu_class:
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index be9b4c30c3..7891465554 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -167,7 +167,8 @@ typedef struct PowerPCCPUClass {
>      DeviceRealize parent_realize;
>      DeviceUnrealize parent_unrealize;
>      void (*parent_reset)(CPUState *cpu);
> -    void (*parent_parse_features)(const char *type, char *str, Error **errp);
> +    void (*parent_parse_features)(MachineState *machine, const char *type,
> +                                  char *str, Error **errp);
>  
>      uint32_t pvr;
>      bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
> diff --git a/exec.c b/exec.c
> index d359e709a6..1ca95df9d8 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -992,7 +992,7 @@ CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp)
>      return CPU_CLASS(oc);
>  }
>  
> -const char *parse_cpu_option(const char *cpu_option)
> +const char *parse_cpu_option(MachineState *machine, const char *cpu_option)
>  {
>      CPUClass *cc;
>      gchar **model_pieces;
> @@ -1002,7 +1002,7 @@ const char *parse_cpu_option(const char *cpu_option)
>  
>      cc = lookup_cpu_class(model_pieces[0], &error_fatal);
>      cpu_type = object_class_get_name(OBJECT_CLASS(cc));
> -    cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
> +    cc->parse_features(machine, cpu_type, model_pieces[1], &error_fatal);
>      g_strfreev(model_pieces);
>      return cpu_type;
>  }
> diff --git a/qom/cpu.c b/qom/cpu.c
> index a8d2958956..c8a7b56148 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -291,7 +291,8 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
>      return cc->class_by_name(cpu_model);
>  }
>  
> -static void cpu_common_parse_features(const char *typename, char *features,
> +static void cpu_common_parse_features(MachineState *machine,
> +                                      const char *typename, char *features,
>                                        Error **errp)
>  {
>      char *val;
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index d6bb57d210..f5e15ac5da 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3528,7 +3528,8 @@ static gint compare_string(gconstpointer a, gconstpointer b)
>  
>  /* Parse "+feature,-feature,feature=foo" CPU feature string
>   */
> -static void x86_cpu_parse_featurestr(const char *typename, char *features,
> +static void x86_cpu_parse_featurestr(MachineState *machine,
> +                                     const char *typename, char *features,
>                                       Error **errp)
>  {
>      char *featurestr; /* Single 'key=value" string being parsed */
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index 0bd555eb19..2ad223fcca 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -10119,10 +10119,11 @@ static ObjectClass *ppc_cpu_class_by_name(const char *name)
>      return oc;
>  }
>  
> -static void ppc_cpu_parse_featurestr(const char *type, char *features,
> +static void ppc_cpu_parse_featurestr(MachineState *ms,
> +                                     const char *type, char *features,
>                                       Error **errp)
>  {
> -    Object *machine = qdev_get_machine();
> +    Object *machine = OBJECT(ms);
>      const PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(object_class_by_name(type));
>  
>      if (!features) {
> @@ -10171,7 +10172,7 @@ static void ppc_cpu_parse_featurestr(const char *type, char *features,
>      }
>  
>      /* do property processing with generic handler */
> -    pcc->parent_parse_features(type, features, errp);
> +    pcc->parent_parse_features(ms, type, features, errp);
>  }
>  
>  PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index 4a4445bdf5..7e360de5ee 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -115,7 +115,8 @@ cpu_add_feat_as_prop(const char *typename, const char *name, const char *val)
>  }
>  
>  /* Parse "+feature,-feature,feature=foo" CPU feature string */
> -static void sparc_cpu_parse_features(const char *typename, char *features,
> +static void sparc_cpu_parse_features(MachineState *machine,
> +                                     const char *typename, char *features,
>                                       Error **errp)
>  {
>      GList *l, *plus_features = NULL, *minus_features = NULL;
> diff --git a/vl.c b/vl.c
> index c57e28d1da..e78c4d5a53 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4466,7 +4466,8 @@ int main(int argc, char **argv, char **envp)
>      /* parse features once if machine provides default cpu_type */
>      current_machine->cpu_type = machine_class->default_cpu_type;
>      if (cpu_option) {
> -        current_machine->cpu_type = parse_cpu_option(cpu_option);
> +        current_machine->cpu_type =
> +            parse_cpu_option(current_machine, cpu_option);
>      }
>      parse_numa_opts(current_machine);

Well, it's not quite as bad as the commit message made me expect: since
we assign to current_machine->cpu_type, we already have a dependence on
machine creation.

Regardless, replacing qdev_get_machine() by explicit data flow is an
improvement.  More so if it really lets us confine qdev_get_machine() to
CONFIG_SOFTMMU.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()
@ 2019-04-17  5:45   ` Markus Armbruster
  0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2019-04-17  5:45 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: qemu-devel, Peter Maydell, Thomas Huth, Like Xu, Riku Voipio,
	Mark Cave-Ayland, Laurent Vivier, qemu-ppc, Igor Mammedov,
	Paolo Bonzini, David Gibson, Artyom Tarasenko, Richard Henderson

Eduardo Habkost <ehabkost@redhat.com> writes:

> My initial goal was simple: removing the qdev_get_machine() call
> from ppc_cpu_parse_featurestr() because I want to make
> qdev_get_machine() available only to softmmu code.
>
> Before doing this, I had to make *-user not call
> CPUClass::parse_features() anymore (it was pointless to call it,
> anyway).
>
> While doing this, I decided to rename parse_cpu_model() to
> something clearer (parse_cpu_option()).
>
> As a nice side effect, now the dependency between machine object
> creation and parse_cpu_option() is not hidden anymore.

Series
Reviewed-by: Markus Armbruster <armbru@redhat.com>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()
@ 2019-04-17  5:45   ` Markus Armbruster
  0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2019-04-17  5:45 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Thomas Huth, Like Xu, Riku Voipio,
	Mark Cave-Ayland, qemu-devel, Laurent Vivier, qemu-ppc,
	Paolo Bonzini, Igor Mammedov, Richard Henderson,
	Artyom Tarasenko, David Gibson

Eduardo Habkost <ehabkost@redhat.com> writes:

> My initial goal was simple: removing the qdev_get_machine() call
> from ppc_cpu_parse_featurestr() because I want to make
> qdev_get_machine() available only to softmmu code.
>
> Before doing this, I had to make *-user not call
> CPUClass::parse_features() anymore (it was pointless to call it,
> anyway).
>
> While doing this, I decided to rename parse_cpu_model() to
> something clearer (parse_cpu_option()).
>
> As a nice side effect, now the dependency between machine object
> creation and parse_cpu_option() is not hidden anymore.

Series
Reviewed-by: Markus Armbruster <armbru@redhat.com>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()
@ 2019-04-17 13:55       ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17 13:55 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, Peter Maydell, Thomas Huth, Like Xu, Riku Voipio,
	Mark Cave-Ayland, Laurent Vivier, qemu-ppc, Igor Mammedov,
	Paolo Bonzini, David Gibson, Artyom Tarasenko, Richard Henderson

On Wed, Apr 17, 2019 at 07:41:23AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > The new function will be useful in user mode, when we already
> > have a CPU model and don't need to parse any extra options.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  include/qom/cpu.h |  9 +++++++++
> >  exec.c            | 22 ++++++++++++----------
> >  2 files changed, 21 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> > index d28c690b27..e11b14d9ac 100644
> > --- a/include/qom/cpu.h
> > +++ b/include/qom/cpu.h
> > @@ -699,6 +699,15 @@ CPUState *cpu_create(const char *typename);
> >   */
> >  const char *parse_cpu_option(const char *cpu_option);
> >  
> > +/**
> > + * lookup_cpu_class:
> > + * @cpu_model: CPU model name
> > + *
> > + * Look up CPU class corresponding to a given CPU model name.
> > + */
> > +CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp);
> 
> Nitpicks, feel free to ignore.
> 
> Naming: lookup_cpu_class() makes my reading circuits stall momentarily,
> because lookup is a noun. The verb is spelled look up.  No stall:
> cpu_class_lookup(), look_up_cpu_class(), cpu_class_by_name_err(),
> cpu_class_parse(), ...
> 
> Doc string: this is a wrapper around cpu_class_by_name().  Perhaps that
> should be spelled out.
> 
> Apropos cpu_class_by_name(): it returns ObjectClass, not CPUClass.
> Isn't that odd?
> 
> Position: If you put it before cpu_create() rather than after, it's next
> to the function it wraps.  But perhaps you prefer to keep it next to
> parse_cpu_option().

Your suggestions make sense.  I didn't notice how close
lookup_cpu_class() was to the declaration of cpu_class_by_name().
I was seeing lookup_cpu_class() as "this portion of
parse_cpu_option() I need", and not as "a wrapper to
cpu_class_by_name()".

What about:

/**
 * arch_cpu_class_by_name:
 * @cpu_model: CPU model name
 *
 * Arch-specific wrapper around cpu_class_by_name(), uses CPU_RESOLVING_TYPE
 * for the current architecture.
 */
CPUClass *arch_cpu_class_by_name(const char *cpu_model, Error **errp);


-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()
@ 2019-04-17 13:55       ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-17 13:55 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Peter Maydell, Thomas Huth, Like Xu, Riku Voipio,
	Mark Cave-Ayland, qemu-devel, Laurent Vivier, qemu-ppc,
	Paolo Bonzini, Igor Mammedov, Richard Henderson,
	Artyom Tarasenko, David Gibson

On Wed, Apr 17, 2019 at 07:41:23AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > The new function will be useful in user mode, when we already
> > have a CPU model and don't need to parse any extra options.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  include/qom/cpu.h |  9 +++++++++
> >  exec.c            | 22 ++++++++++++----------
> >  2 files changed, 21 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> > index d28c690b27..e11b14d9ac 100644
> > --- a/include/qom/cpu.h
> > +++ b/include/qom/cpu.h
> > @@ -699,6 +699,15 @@ CPUState *cpu_create(const char *typename);
> >   */
> >  const char *parse_cpu_option(const char *cpu_option);
> >  
> > +/**
> > + * lookup_cpu_class:
> > + * @cpu_model: CPU model name
> > + *
> > + * Look up CPU class corresponding to a given CPU model name.
> > + */
> > +CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp);
> 
> Nitpicks, feel free to ignore.
> 
> Naming: lookup_cpu_class() makes my reading circuits stall momentarily,
> because lookup is a noun. The verb is spelled look up.  No stall:
> cpu_class_lookup(), look_up_cpu_class(), cpu_class_by_name_err(),
> cpu_class_parse(), ...
> 
> Doc string: this is a wrapper around cpu_class_by_name().  Perhaps that
> should be spelled out.
> 
> Apropos cpu_class_by_name(): it returns ObjectClass, not CPUClass.
> Isn't that odd?
> 
> Position: If you put it before cpu_create() rather than after, it's next
> to the function it wraps.  But perhaps you prefer to keep it next to
> parse_cpu_option().

Your suggestions make sense.  I didn't notice how close
lookup_cpu_class() was to the declaration of cpu_class_by_name().
I was seeing lookup_cpu_class() as "this portion of
parse_cpu_option() I need", and not as "a wrapper to
cpu_class_by_name()".

What about:

/**
 * arch_cpu_class_by_name:
 * @cpu_model: CPU model name
 *
 * Arch-specific wrapper around cpu_class_by_name(), uses CPU_RESOLVING_TYPE
 * for the current architecture.
 */
CPUClass *arch_cpu_class_by_name(const char *cpu_model, Error **errp);


-- 
Eduardo


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()
@ 2019-04-18  3:35     ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-18  3:35 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, Peter Maydell, Thomas Huth, Like Xu, Riku Voipio,
	Mark Cave-Ayland, Laurent Vivier, qemu-ppc, Igor Mammedov,
	Paolo Bonzini, David Gibson, Artyom Tarasenko, Richard Henderson

On Wed, Apr 17, 2019 at 07:45:24AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > My initial goal was simple: removing the qdev_get_machine() call
> > from ppc_cpu_parse_featurestr() because I want to make
> > qdev_get_machine() available only to softmmu code.
> >
> > Before doing this, I had to make *-user not call
> > CPUClass::parse_features() anymore (it was pointless to call it,
> > anyway).
> >
> > While doing this, I decided to rename parse_cpu_model() to
> > something clearer (parse_cpu_option()).
> >
> > As a nice side effect, now the dependency between machine object
> > creation and parse_cpu_option() is not hidden anymore.
> 
> Series
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks.  I'm queueing this even though I agree with the comments
at patch 2/5, because I'm already planning to send a separate
cleanup series for cpu_class_by_name() and other related code.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()
@ 2019-04-18  3:35     ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-18  3:35 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Peter Maydell, Thomas Huth, Like Xu, Riku Voipio,
	Mark Cave-Ayland, qemu-devel, Laurent Vivier, qemu-ppc,
	Paolo Bonzini, Igor Mammedov, Richard Henderson,
	Artyom Tarasenko, David Gibson

On Wed, Apr 17, 2019 at 07:45:24AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > My initial goal was simple: removing the qdev_get_machine() call
> > from ppc_cpu_parse_featurestr() because I want to make
> > qdev_get_machine() available only to softmmu code.
> >
> > Before doing this, I had to make *-user not call
> > CPUClass::parse_features() anymore (it was pointless to call it,
> > anyway).
> >
> > While doing this, I decided to rename parse_cpu_model() to
> > something clearer (parse_cpu_option()).
> >
> > As a nice side effect, now the dependency between machine object
> > creation and parse_cpu_option() is not hidden anymore.
> 
> Series
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks.  I'm queueing this even though I agree with the comments
at patch 2/5, because I'm already planning to send a separate
cleanup series for cpu_class_by_name() and other related code.

-- 
Eduardo


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class()
@ 2019-04-18  4:52     ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-18  4:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Paolo Bonzini, Riku Voipio, Like Xu,
	Richard Henderson, David Gibson, Thomas Huth, Igor Mammedov,
	qemu-ppc, Markus Armbruster, Peter Maydell, Artyom Tarasenko,
	Mark Cave-Ayland

On Tue, Apr 16, 2019 at 11:59:42PM -0300, Eduardo Habkost wrote:
> The return value of cpu_get_model() is just a CPU model name and
> never includes extra options.  We don't need to call
> parse_cpu_option().

Oops.  I was wrong.  linux-user also supports extra features in
the "-cpu" option, so we do need to call parse_cpu_option().

e.g.: this worked before:

  $ qemu-x86_64 -cpu Nehalem,+popcnt /bin/true

and now this is broken:

  $ qemu-x86_64 -cpu Nehalem,+popcnt /bin/true
  unable to find CPU model 'Nehalem,+popcnt'

I will drop patches 2-5 from my queue.  Sorry for the noise.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class()
@ 2019-04-18  4:52     ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2019-04-18  4:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Thomas Huth, Like Xu, Riku Voipio,
	Mark Cave-Ayland, Laurent Vivier, Markus Armbruster, qemu-ppc,
	Igor Mammedov, Paolo Bonzini, Richard Henderson,
	Artyom Tarasenko, David Gibson

On Tue, Apr 16, 2019 at 11:59:42PM -0300, Eduardo Habkost wrote:
> The return value of cpu_get_model() is just a CPU model name and
> never includes extra options.  We don't need to call
> parse_cpu_option().

Oops.  I was wrong.  linux-user also supports extra features in
the "-cpu" option, so we do need to call parse_cpu_option().

e.g.: this worked before:

  $ qemu-x86_64 -cpu Nehalem,+popcnt /bin/true

and now this is broken:

  $ qemu-x86_64 -cpu Nehalem,+popcnt /bin/true
  unable to find CPU model 'Nehalem,+popcnt'

I will drop patches 2-5 from my queue.  Sorry for the noise.

-- 
Eduardo


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option()
@ 2019-04-18 11:07     ` Igor Mammedov
  0 siblings, 0 replies; 36+ messages in thread
From: Igor Mammedov @ 2019-04-18 11:07 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: qemu-devel, Laurent Vivier, Paolo Bonzini, Riku Voipio, Like Xu,
	Richard Henderson, David Gibson, Thomas Huth, qemu-ppc,
	Markus Armbruster, Peter Maydell, Artyom Tarasenko,
	Mark Cave-Ayland

On Tue, 16 Apr 2019 23:59:40 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> The "model[,option...]" string parsed by the function is not just
> a CPU model.  Rename the function and its argument to indicate it
> expects the full "-cpu" option to be provided.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/qom/cpu.h |  6 +++---
>  bsd-user/main.c   |  2 +-
>  exec.c            |  4 ++--
>  linux-user/main.c |  2 +-
>  vl.c              | 18 +++++++++---------
>  5 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 1d6099e5d4..d28c690b27 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -689,15 +689,15 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
>  CPUState *cpu_create(const char *typename);
>  
>  /**
> - * parse_cpu_model:
> - * @cpu_model: The model string including optional parameters.
> + * parse_cpu_option:
> + * @cpu_option: The -cpu option including optional parameters.
>   *
>   * processes optional parameters and registers them as global properties
>   *
>   * Returns: type of CPU to create or prints error and terminates process
>   *          if an error occurred.
>   */
> -const char *parse_cpu_model(const char *cpu_model);
> +const char *parse_cpu_option(const char *cpu_option);
>  
>  /**
>   * cpu_has_work:
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 0d3156974c..a6c055f5fb 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -903,7 +903,7 @@ int main(int argc, char **argv)
>      /* init tcg before creating CPUs and to get qemu_host_page_size */
>      tcg_exec_init(0);
>  
> -    cpu_type = parse_cpu_model(cpu_model);
> +    cpu_type = parse_cpu_option(cpu_model);
>      cpu = cpu_create(cpu_type);
>      env = cpu->env_ptr;
>  #if defined(TARGET_SPARC) || defined(TARGET_PPC)
> diff --git a/exec.c b/exec.c
> index 6ab62f4eee..840677f15f 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -982,14 +982,14 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
>  #endif
>  }
>  
> -const char *parse_cpu_model(const char *cpu_model)
> +const char *parse_cpu_option(const char *cpu_option)
>  {
>      ObjectClass *oc;
>      CPUClass *cc;
>      gchar **model_pieces;
>      const char *cpu_type;
>  
> -    model_pieces = g_strsplit(cpu_model, ",", 2);
> +    model_pieces = g_strsplit(cpu_option, ",", 2);
>  
>      oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
>      if (oc == NULL) {
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a0aba9cb1e..20e0f51cfa 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -660,7 +660,7 @@ int main(int argc, char **argv, char **envp)
>      if (cpu_model == NULL) {
>          cpu_model = cpu_get_model(get_elf_eflags(execfd));
>      }
> -    cpu_type = parse_cpu_model(cpu_model);
> +    cpu_type = parse_cpu_option(cpu_model);
>  
>      /* init tcg before creating CPUs and to get qemu_host_page_size */
>      tcg_exec_init(0);
> diff --git a/vl.c b/vl.c
> index c696ad2a13..c57e28d1da 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3002,7 +3002,7 @@ int main(int argc, char **argv, char **envp)
>      const char *optarg;
>      const char *loadvm = NULL;
>      MachineClass *machine_class;
> -    const char *cpu_model;
> +    const char *cpu_option;
>      const char *vga_model = NULL;
>      const char *qtest_chrdev = NULL;
>      const char *qtest_log = NULL;
> @@ -3081,7 +3081,7 @@ int main(int argc, char **argv, char **envp)
>      QLIST_INIT (&vm_change_state_head);
>      os_setup_early_signal_handling();
>  
> -    cpu_model = NULL;
> +    cpu_option = NULL;
>      snapshot = 0;
>  
>      nb_nics = 0;
> @@ -3133,7 +3133,7 @@ int main(int argc, char **argv, char **envp)
>              switch(popt->index) {
>              case QEMU_OPTION_cpu:
>                  /* hw initialization will check this */
> -                cpu_model = optarg;
> +                cpu_option = optarg;
>                  break;
>              case QEMU_OPTION_hda:
>              case QEMU_OPTION_hdb:
> @@ -4050,8 +4050,8 @@ int main(int argc, char **argv, char **envp)
>          qemu_set_hw_version(machine_class->hw_version);
>      }
>  
> -    if (cpu_model && is_help_option(cpu_model)) {
> -        list_cpus(stdout, &fprintf, cpu_model);
> +    if (cpu_option && is_help_option(cpu_option)) {
> +        list_cpus(stdout, &fprintf, cpu_option);
>          exit(0);
>      }
>  
> @@ -4299,9 +4299,9 @@ int main(int argc, char **argv, char **envp)
>       * Global properties get set up by qdev_prop_register_global(),
>       * called from user_register_global_props(), and certain option
>       * desugaring.  Also in CPU feature desugaring (buried in
> -     * parse_cpu_model()), which happens below this point, but may
> +     * parse_cpu_option()), which happens below this point, but may
>       * only target the CPU type, which can only be created after
> -     * parse_cpu_model() returned the type.
> +     * parse_cpu_option() returned the type.
>       *
>       * Machine compat properties: object_set_machine_compat_props().
>       * Accelerator compat props: object_set_accelerator_compat_props(),
> @@ -4465,8 +4465,8 @@ int main(int argc, char **argv, char **envp)
>  
>      /* parse features once if machine provides default cpu_type */
>      current_machine->cpu_type = machine_class->default_cpu_type;
> -    if (cpu_model) {
> -        current_machine->cpu_type = parse_cpu_model(cpu_model);
> +    if (cpu_option) {
> +        current_machine->cpu_type = parse_cpu_option(cpu_option);
>      }
>      parse_numa_opts(current_machine);
>  

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option()
@ 2019-04-18 11:07     ` Igor Mammedov
  0 siblings, 0 replies; 36+ messages in thread
From: Igor Mammedov @ 2019-04-18 11:07 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Thomas Huth, Like Xu, Markus Armbruster,
	Riku Voipio, Mark Cave-Ayland, qemu-devel, Laurent Vivier,
	qemu-ppc, Paolo Bonzini, David Gibson, Artyom Tarasenko,
	Richard Henderson

On Tue, 16 Apr 2019 23:59:40 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> The "model[,option...]" string parsed by the function is not just
> a CPU model.  Rename the function and its argument to indicate it
> expects the full "-cpu" option to be provided.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/qom/cpu.h |  6 +++---
>  bsd-user/main.c   |  2 +-
>  exec.c            |  4 ++--
>  linux-user/main.c |  2 +-
>  vl.c              | 18 +++++++++---------
>  5 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 1d6099e5d4..d28c690b27 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -689,15 +689,15 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
>  CPUState *cpu_create(const char *typename);
>  
>  /**
> - * parse_cpu_model:
> - * @cpu_model: The model string including optional parameters.
> + * parse_cpu_option:
> + * @cpu_option: The -cpu option including optional parameters.
>   *
>   * processes optional parameters and registers them as global properties
>   *
>   * Returns: type of CPU to create or prints error and terminates process
>   *          if an error occurred.
>   */
> -const char *parse_cpu_model(const char *cpu_model);
> +const char *parse_cpu_option(const char *cpu_option);
>  
>  /**
>   * cpu_has_work:
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 0d3156974c..a6c055f5fb 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -903,7 +903,7 @@ int main(int argc, char **argv)
>      /* init tcg before creating CPUs and to get qemu_host_page_size */
>      tcg_exec_init(0);
>  
> -    cpu_type = parse_cpu_model(cpu_model);
> +    cpu_type = parse_cpu_option(cpu_model);
>      cpu = cpu_create(cpu_type);
>      env = cpu->env_ptr;
>  #if defined(TARGET_SPARC) || defined(TARGET_PPC)
> diff --git a/exec.c b/exec.c
> index 6ab62f4eee..840677f15f 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -982,14 +982,14 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
>  #endif
>  }
>  
> -const char *parse_cpu_model(const char *cpu_model)
> +const char *parse_cpu_option(const char *cpu_option)
>  {
>      ObjectClass *oc;
>      CPUClass *cc;
>      gchar **model_pieces;
>      const char *cpu_type;
>  
> -    model_pieces = g_strsplit(cpu_model, ",", 2);
> +    model_pieces = g_strsplit(cpu_option, ",", 2);
>  
>      oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
>      if (oc == NULL) {
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a0aba9cb1e..20e0f51cfa 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -660,7 +660,7 @@ int main(int argc, char **argv, char **envp)
>      if (cpu_model == NULL) {
>          cpu_model = cpu_get_model(get_elf_eflags(execfd));
>      }
> -    cpu_type = parse_cpu_model(cpu_model);
> +    cpu_type = parse_cpu_option(cpu_model);
>  
>      /* init tcg before creating CPUs and to get qemu_host_page_size */
>      tcg_exec_init(0);
> diff --git a/vl.c b/vl.c
> index c696ad2a13..c57e28d1da 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3002,7 +3002,7 @@ int main(int argc, char **argv, char **envp)
>      const char *optarg;
>      const char *loadvm = NULL;
>      MachineClass *machine_class;
> -    const char *cpu_model;
> +    const char *cpu_option;
>      const char *vga_model = NULL;
>      const char *qtest_chrdev = NULL;
>      const char *qtest_log = NULL;
> @@ -3081,7 +3081,7 @@ int main(int argc, char **argv, char **envp)
>      QLIST_INIT (&vm_change_state_head);
>      os_setup_early_signal_handling();
>  
> -    cpu_model = NULL;
> +    cpu_option = NULL;
>      snapshot = 0;
>  
>      nb_nics = 0;
> @@ -3133,7 +3133,7 @@ int main(int argc, char **argv, char **envp)
>              switch(popt->index) {
>              case QEMU_OPTION_cpu:
>                  /* hw initialization will check this */
> -                cpu_model = optarg;
> +                cpu_option = optarg;
>                  break;
>              case QEMU_OPTION_hda:
>              case QEMU_OPTION_hdb:
> @@ -4050,8 +4050,8 @@ int main(int argc, char **argv, char **envp)
>          qemu_set_hw_version(machine_class->hw_version);
>      }
>  
> -    if (cpu_model && is_help_option(cpu_model)) {
> -        list_cpus(stdout, &fprintf, cpu_model);
> +    if (cpu_option && is_help_option(cpu_option)) {
> +        list_cpus(stdout, &fprintf, cpu_option);
>          exit(0);
>      }
>  
> @@ -4299,9 +4299,9 @@ int main(int argc, char **argv, char **envp)
>       * Global properties get set up by qdev_prop_register_global(),
>       * called from user_register_global_props(), and certain option
>       * desugaring.  Also in CPU feature desugaring (buried in
> -     * parse_cpu_model()), which happens below this point, but may
> +     * parse_cpu_option()), which happens below this point, but may
>       * only target the CPU type, which can only be created after
> -     * parse_cpu_model() returned the type.
> +     * parse_cpu_option() returned the type.
>       *
>       * Machine compat properties: object_set_machine_compat_props().
>       * Accelerator compat props: object_set_accelerator_compat_props(),
> @@ -4465,8 +4465,8 @@ int main(int argc, char **argv, char **envp)
>  
>      /* parse features once if machine provides default cpu_type */
>      current_machine->cpu_type = machine_class->default_cpu_type;
> -    if (cpu_model) {
> -        current_machine->cpu_type = parse_cpu_model(cpu_model);
> +    if (cpu_option) {
> +        current_machine->cpu_type = parse_cpu_option(cpu_option);
>      }
>      parse_numa_opts(current_machine);
>  



^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2019-04-18 11:19 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-17  2:59 [Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr() Eduardo Habkost
2019-04-17  2:59 ` Eduardo Habkost
2019-04-17  2:59 ` [Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option() Eduardo Habkost
2019-04-17  2:59   ` Eduardo Habkost
2019-04-17  5:21   ` David Gibson
2019-04-17  5:21     ` David Gibson
2019-04-18 11:07   ` Igor Mammedov
2019-04-18 11:07     ` Igor Mammedov
2019-04-17  2:59 ` [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option() Eduardo Habkost
2019-04-17  2:59   ` Eduardo Habkost
2019-04-17  5:22   ` David Gibson
2019-04-17  5:22     ` David Gibson
2019-04-17  5:41   ` Markus Armbruster
2019-04-17  5:41     ` Markus Armbruster
2019-04-17 13:55     ` Eduardo Habkost
2019-04-17 13:55       ` Eduardo Habkost
2019-04-17  2:59 ` [Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class() Eduardo Habkost
2019-04-17  2:59   ` Eduardo Habkost
2019-04-17  5:23   ` David Gibson
2019-04-17  5:23     ` David Gibson
2019-04-18  4:52   ` Eduardo Habkost
2019-04-18  4:52     ` Eduardo Habkost
2019-04-17  2:59 ` [Qemu-devel] [PATCH 4/5] bsd-user: " Eduardo Habkost
2019-04-17  2:59   ` Eduardo Habkost
2019-04-17  5:23   ` David Gibson
2019-04-17  5:23     ` David Gibson
2019-04-17  2:59 ` [Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features() Eduardo Habkost
2019-04-17  2:59   ` Eduardo Habkost
2019-04-17  5:25   ` David Gibson
2019-04-17  5:25     ` David Gibson
2019-04-17  5:45   ` Markus Armbruster
2019-04-17  5:45     ` Markus Armbruster
2019-04-17  5:45 ` [Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr() Markus Armbruster
2019-04-17  5:45   ` Markus Armbruster
2019-04-18  3:35   ` Eduardo Habkost
2019-04-18  3:35     ` Eduardo Habkost

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.