On Tue, Oct 3, 2017 at 1:23 PM, Eduardo Habkost <ehabk...@redhat.com> wrote:
> On Tue, Oct 03, 2017 at 01:05:09PM -0700, Alistair Francis wrote:
>> This patch add a MachineClass element that can be set in the machine C
>> code to specify a list of supported CPU types. If the supported CPU
>> types are specified the user enter CPU (by -cpu at runtime) is checked
>> against the supported types and QEMU exits if they aren't supported.
>>
>> Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com>
>> ---
>
> Thanks!
>
> Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
>
> However, I will squash the following changes before queueing,
> because:
>
> * object_class_dynamic_cast() is safe even if class is NULL,
> so there's no need to validate cpu_type here.
> * "must not be valid" sounds like the CPU is not allowed to be a
> valid CPU, so I rewrote the comment.
>
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 3afc6a7b5b..36c2fb069c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -766,9 +766,6 @@ void machine_run_board_init(MachineState *machine)
> 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])) {
> @@ -780,7 +777,7 @@ void machine_run_board_init(MachineState *machine)
> }
>
> if (!machine_class->valid_cpu_types[i]) {
> - /* The user specified CPU must not be a valid CPU */
> + /* The user specified CPU is not valid */
> error_report("Invalid CPU type: %s", machine->cpu_type);
> error_printf("The valid types are: %s",
> machine_class->valid_cpu_types[0]);
Looks good to me.
Does that mean you are taking the whole series now?
Thanks,
Alistair
>>
>> V1:
>> - Use 'type' in the error message
>> - Move a uneeded operation out of the for loop
>> - Remove the goto
>> 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..3afc6a7b5b 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 the machine supports the valid_cpu_types check and the user
>> + * specified a CPU with -cpu check here that the user CPU is supported.
>> + */
>> + if (machine_class->valid_cpu_types && machine->cpu_type) {
>> + 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])) {
>> + /* The user specificed CPU is in the valid field, we are
>> + * good to go.
>> + */
>> + break;
>> + }
>> + }
>> +
>> + if (!machine_class->valid_cpu_types[i]) {
>> + /* The user specified CPU must not be a valid CPU */
>> + error_report("Invalid CPU type: %s", machine->cpu_type);
>> + error_printf("The valid types 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);
>> + }
>> + }
>> +
>> 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
>>
>
> --
> Eduardo
