[RFC,v2,1/2] machine: Add a valid_cpu_types property

Commit Message

Alistair Francis Sept. 21, 2017, 11:41 p.m. UTC

Signed-off-by: Alistair Francis <[e-mail]>
---

RFC v2:
 - Rebase on Igor's cpu_type work
 - Use object_class_dynamic_cast()
 - Use a NULL terminated cahr** list
 - Do the check before the machine_class init() is called


 hw/core/machine.c   | 35 +++++++++++++++++++++++++++++++++++
 include/hw/boards.h |  1 +
 2 files changed, 36 insertions(+)

Comments

Eduardo Habkost Oct. 3, 2017, 2:10 p.m. UTC | #1

On Thu, Sep 21, 2017 at 04:41:50PM -0700, Alistair Francis wrote:
> Signed-off-by: Alistair Francis <[e-mail]>
> ---
> 
> RFC v2:
>  - Rebase on Igor's cpu_type work
>  - Use object_class_dynamic_cast()
>  - Use a NULL terminated cahr** list
>  - Do the check before the machine_class init() is called
> 
> 
>  hw/core/machine.c   | 35 +++++++++++++++++++++++++++++++++++
>  include/hw/boards.h |  1 +
>  2 files changed, 36 insertions(+)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 80647edc2a..abebfabdb8 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -758,6 +758,41 @@ void machine_run_board_init(MachineState *machine)
>      if (nb_numa_nodes) {
>          machine_numa_finish_init(machine);
>      }
> +
> +    if (machine_class->valid_cpu_types && machine->cpu_type) {
> +        int i;
> +
> +        for (i = 0; machine_class->valid_cpu_types[i]; i++) {
> +            ObjectClass *class = object_class_by_name(machine->cpu_type);
> +
> +            if (!class) {
> +                break;
> +            }
> +
> +            if (object_class_dynamic_cast(class,
> +                                          machine_class->valid_cpu_types[i])) {
> +                /* The user specificed CPU is in the valid field, we are
> +                 * good to go.
> +                 */
> +                goto done;

I would move the object_class_by_name() call outside the for loop
and remove the "goto", like this:

    if (machine->cpu_type && machine_class->valid_cpu_types) {
        ObjectClass *class = object_class_by_name(machine->cpu_type);
        int i;

        /* machine->cpu_type is supposed to be always a valid QOM type */
        assert(class);

        for (i = 0; machine_class->valid_cpu_types[i]; i++) {
            if (object_class_dynamic_cast(class,
                                          machine_class->valid_cpu_types[i])) {
                /* Valid CPU type, we're good to go */
                break;
            }
        }
        if (!machine_class->valid_cpu_types[i]) {
            error_report(...);
            ...
        }
    }

    machine_class->init(machine);

> +            }
> +        }
> +
> +        /* The user specified CPU must not be a valid CPU, print a sane
> +         * error
> +         */
> +        error_report("Invalid CPU: %s", machine->cpu_type);
> +        error_printf("The valid options are: %s",
> +                     machine_class->valid_cpu_types[0]);
> +        for (i = 1; machine_class->valid_cpu_types[i]; i++) {
> +            error_printf(", %s", machine_class->valid_cpu_types[i]);
> +        }
> +        error_printf("\n");

I would still like to make this share code with
query-cpu-definitions one day, but I'm OK with this
implementation.

I would just rewrite the message as "valid types are:" instead of
"valid options are:" and "Invalid CPU type:" instead of "Invalid
CPU:", because the -cpu option doesn't need to match a string in
valid_cpu_types exactly, it just needs to resolve to a type that
implements a valid type.


> +
> +        exit(1);
> +    }
> +
> +done:
>      machine_class->init(machine);
>  }
>  
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 156e0a5701..191a5b3cd8 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -191,6 +191,7 @@ struct MachineClass {
>      bool has_hotpluggable_cpus;
>      bool ignore_memory_transaction_failures;
>      int numa_mem_align_shift;
> +    const char **valid_cpu_types;
>      void (*numa_auto_assign_ram)(MachineClass *mc, NodeInfo *nodes,
>                                   int nb_nodes, ram_addr_t size);
>  
> -- 
> 2.11.0
> 
>

Patch

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 80647edc2a..abebfabdb8 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -758,6 +758,41 @@  void machine_run_board_init(MachineState *machine)
     if (nb_numa_nodes) {
         machine_numa_finish_init(machine);
     }
+
+    if (machine_class->valid_cpu_types && machine->cpu_type) {
+        int i;
+
+        for (i = 0; machine_class->valid_cpu_types[i]; i++) {
+            ObjectClass *class = object_class_by_name(machine->cpu_type);
+
+            if (!class) {
+                break;
+            }
+
+            if (object_class_dynamic_cast(class,
+                                          machine_class->valid_cpu_types[i])) {
+                /* The user specificed CPU is in the valid field, we are
+                 * good to go.
+                 */
+                goto done;
+            }
+        }
+
+        /* The user specified CPU must not be a valid CPU, print a sane
+         * error
+         */
+        error_report("Invalid CPU: %s", machine->cpu_type);
+        error_printf("The valid options are: %s",
+                     machine_class->valid_cpu_types[0]);
+        for (i = 1; machine_class->valid_cpu_types[i]; i++) {
+            error_printf(", %s", machine_class->valid_cpu_types[i]);
+        }
+        error_printf("\n");
+
+        exit(1);
+    }
+
+done:
     machine_class->init(machine);
 }
 
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 156e0a5701..191a5b3cd8 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -191,6 +191,7 @@  struct MachineClass {
     bool has_hotpluggable_cpus;
     bool ignore_memory_transaction_failures;
     int numa_mem_align_shift;
+    const char **valid_cpu_types;
     void (*numa_auto_assign_ram)(MachineClass *mc, NodeInfo *nodes,
                                  int nb_nodes, ram_addr_t size);