[Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()

[Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()

[Eduardo Habkost]

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

[Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()

[Eduardo Habkost]

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

[Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option()

[Eduardo Habkost]

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

[Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option()

[Eduardo Habkost]

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

Re: [Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option()

[David Gibson]

[-- 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 --]

Re: [Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option()

[David Gibson]

[-- 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 --]

Re: [Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option()

[Igor Mammedov]

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);
>  

Re: [Qemu-devel] [PATCH 1/5] cpu: Rename parse_cpu_model() to parse_cpu_option()

[Igor Mammedov]

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);
>  

[Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()

[Eduardo Habkost]

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

[Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()

[Eduardo Habkost]

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

Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()

[David Gibson]

[-- 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 --]

Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()

[David Gibson]

[-- 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 --]

Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()

[Markus Armbruster]

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().

[...]

Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()

[Markus Armbruster]

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().

[...]

Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()

[Eduardo Habkost]

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

Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()

[Eduardo Habkost]

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

[Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class()

[Eduardo Habkost]

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

[Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class()

[Eduardo Habkost]

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

Re: [Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class()

[David Gibson]

[-- 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 --]

Re: [Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class()

[David Gibson]

[-- 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 --]

Re: [Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class()

[Eduardo Habkost]

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

Re: [Qemu-devel] [PATCH 3/5] linux-user: Use lookup_cpu_class()

[Eduardo Habkost]

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

[Qemu-devel] [PATCH 4/5] bsd-user: Use lookup_cpu_class()

[Eduardo Habkost]

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

[Qemu-devel] [PATCH 4/5] bsd-user: Use lookup_cpu_class()

[Eduardo Habkost]

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

Re: [Qemu-devel] [PATCH 4/5] bsd-user: Use lookup_cpu_class()

[David Gibson]

[-- 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 --]

Re: [Qemu-devel] [PATCH 4/5] bsd-user: Use lookup_cpu_class()

[David Gibson]

[-- 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 --]

[Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features()

[Eduardo Habkost]

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

[Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features()

[Eduardo Habkost]

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

Re: [Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features()

[David Gibson]

[-- 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 --]

Re: [Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features()

[David Gibson]

[-- 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 --]

Re: [Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features()

[Markus Armbruster]

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.

Re: [Qemu-devel] [PATCH 5/5] cpu: Add MachineState parameter to parse_features()

[Markus Armbruster]

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.

Re: [Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()

[Markus Armbruster]

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>

Re: [Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()

[Markus Armbruster]

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>

Re: [Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()

[Eduardo Habkost]

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

Re: [Qemu-devel] [PATCH 0/5] Remove qdev_get_machine() call from ppc_cpu_parse_featurestr()

[Eduardo Habkost]

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