| Message ID | 20230713054502.410911-1-gshan@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | hw/arm/virt: Use generic CPU invalidation | expand |
On Thu, 13 Jul 2023 at 06:45, Gavin Shan <gshan@redhat.com> wrote: > > There is a generic CPU type invalidation in machine_run_board_init() > and we needn't a same and private invalidation for hw/arm/virt machines. > This series intends to use the generic CPU type invalidation on the > hw/arm/virt machines. > > PATCH[1] factors the CPU type invalidation logic in machine_run_board_init() > to a helper validate_cpu_type(). > PATCH[2] uses the generic CPU type invalidation for hw/arm/virt machines > PATCH[3] support "host-arm-cpu" CPU type only when KVM or HVF is visible > > Testing > ======= > > With the following command lines, the output messages are varied before > and after the series is applied. > > /home/gshan/sandbox/src/qemu/main/build/qemu-system-aarch64 \ > -accel tcg -machine virt,gic-version=3,nvdimm=on \ > -cpu cortex-a8 -smp maxcpus=2,cpus=1 \ > : > > Before the series is applied: > > qemu-system-aarch64: mach-virt: CPU type cortex-a8-arm-cpu not supported > > After the series is applied: > > qemu-system-aarch64: Invalid CPU type: cortex-a8-arm-cpu > The valid types are: cortex-a7-arm-cpu, cortex-a15-arm-cpu, \ > cortex-a35-arm-cpu, cortex-a55-arm-cpu, cortex-a72-arm-cpu, \ > cortex-a76-arm-cpu, a64fx-arm-cpu, neoverse-n1-arm-cpu, \ > neoverse-v1-arm-cpu, cortex-a53-arm-cpu, cortex-a57-arm-cpu, \ > max-arm-cpu I see this isn't a change in this patch, but given that what the user specifies is not "cortex-a8-arm-cpu" but "cortex-a8", why do we include the "-arm-cpu" suffix in the error messages? It's not valid syntax to say "-cpu cortex-a8-arm-cpu", so it's a bit misleading... -- PMM
W dniu 13.07.2023 o 13:44, Peter Maydell pisze: > I see this isn't a change in this patch, but given that > what the user specifies is not "cortex-a8-arm-cpu" but > "cortex-a8", why do we include the "-arm-cpu" suffix in > the error messages? It's not valid syntax to say > "-cpu cortex-a8-arm-cpu", so it's a bit misleading... Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for other architectures. I like the change but it (IMHO) needs to cut "-{TYPE_*_CPU}" string from names: 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-r5 qemu-system-aarch64: Invalid CPU type: cortex-r5-arm-cpu The valid types are: cortex-a7-arm-cpu, cortex-a15-arm-cpu, cortex-a35-arm-cpu, cortex-a55-arm-cpu, cortex-a72-arm-cpu, cortex-a76-arm-cpu, a64fx-arm-cpu, neoverse-n1-arm-cpu, neoverse-v1-arm-cpu, cortex-a53-arm-cpu, cortex-a57-arm-cpu, host-arm-cpu, max-arm-cpu 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-a57-arm-cpu qemu-system-aarch64: unable to find CPU model 'cortex-a57-arm-cpu' I can change sbsa-ref to follow that change but let it be userfriendly.
On Thu, 13 Jul 2023 at 12:52, Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org> wrote: > > W dniu 13.07.2023 o 13:44, Peter Maydell pisze: > > > I see this isn't a change in this patch, but given that > > what the user specifies is not "cortex-a8-arm-cpu" but > > "cortex-a8", why do we include the "-arm-cpu" suffix in > > the error messages? It's not valid syntax to say > > "-cpu cortex-a8-arm-cpu", so it's a bit misleading... > > Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for > other architectures. Yes; my question is "why are we not using the user-facing string rather than the internal type name?". -- PMM
Hi Peter and Marcin, On 7/13/23 21:52, Marcin Juszkiewicz wrote: > W dniu 13.07.2023 o 13:44, Peter Maydell pisze: > >> I see this isn't a change in this patch, but given that >> what the user specifies is not "cortex-a8-arm-cpu" but >> "cortex-a8", why do we include the "-arm-cpu" suffix in >> the error messages? It's not valid syntax to say >> "-cpu cortex-a8-arm-cpu", so it's a bit misleading... > > Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for other architectures. > > I like the change but it (IMHO) needs to cut "-{TYPE_*_CPU}" string from names: > > 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-r5 > qemu-system-aarch64: Invalid CPU type: cortex-r5-arm-cpu > The valid types are: cortex-a7-arm-cpu, cortex-a15-arm-cpu, cortex-a35-arm-cpu, cortex-a55-arm-cpu, cortex-a72-arm-cpu, cortex-a76-arm-cpu, a64fx-arm-cpu, neoverse-n1-arm-cpu, neoverse-v1-arm-cpu, cortex-a53-arm-cpu, cortex-a57-arm-cpu, host-arm-cpu, max-arm-cpu > > 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-a57-arm-cpu > qemu-system-aarch64: unable to find CPU model 'cortex-a57-arm-cpu' > The suffix of CPU types are provided in hw/arm/virt.c::valid_cpu_types in PATCH[2]. In the generic validation, the complete CPU type is used. The error message also have complete CPU type there. Peter and Marcin, how about to split the CPU types to two fields, as below? In this way, the complete CPU type will be used for validation and the 'internal' names will be used for the error messages. struct MachineClass { const char *valid_cpu_type_suffix; const char **valid_cpu_types; }; hw/arm/virt.c ------------- static const char *valid_cpu_types[] = { #ifdef CONFIG_TCG "cortex-a7", "cortex-a15", "cortex-a35", "cortex-a55", "cortex-a72", "cortex-a76", "a64fx", "neoverse-n1", "neoverse-v1", #endif "cortex-a53", "cortex-a57", "host", "max", NULL }; static void virt_machine_class_init(ObjectClass *oc, void *data) { mc->valid_cpu_type_suffix = TYPE_ARM_CPU; mc->valid_cpu_types = valid_cpu_types; } hw/core/machine.c ----------------- static void validate_cpu_type(MachineState *machine) { ObjectClass *oc = object_class_by_name(machine->cpu_type), *ret = NULL; CPUClass *cc = CPU_CLASS(oc); char *cpu_type; int i; if (!machine->cpu_type || !machine_class->valid_cpu_types) { goto out_no_check; } for (i = 0; machine_class->valid_cpu_types[i]; i++) { /* The class name is the combination of prefix + suffix */ if (machine_class->valid_cpu_type_suffix) { cpu_type = g_strdup_printf("%s-%s", machine_class->valid_cpu_types[i], machine_class->valid_cpu_type_suffix); } else { cpu_type = g_strdup(machine_class->valid_cpu_types[i]); } ret = object_class_dynamic_cast(oc, cpu_type)); g_free(cpu_type); if (ret) { break; } } if (!machine_class->valid_cpu_types[i]) { /* The user-specified CPU type is invalid */ /**** the prefix is used in the error messages ********/ 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); } /* Check if CPU type is deprecated and warn if so */ out_no_check: if (cc && cc->deprecation_note) { warn_report("CPU model %s is deprecated -- %s", machine->cpu_type, cc->deprecation_note); } } > > I can change sbsa-ref to follow that change but let it be userfriendly. > Yes, sbsa-ref needs an extra patch to use the generic invalidation. I can add one patch on the top for that in next revision if you agree, Marcin. Thanks, Gavin
Hi Peter, On 7/13/23 21:44, Peter Maydell wrote: > On Thu, 13 Jul 2023 at 06:45, Gavin Shan <gshan@redhat.com> wrote: >> >> There is a generic CPU type invalidation in machine_run_board_init() >> and we needn't a same and private invalidation for hw/arm/virt machines. >> This series intends to use the generic CPU type invalidation on the >> hw/arm/virt machines. >> >> PATCH[1] factors the CPU type invalidation logic in machine_run_board_init() >> to a helper validate_cpu_type(). >> PATCH[2] uses the generic CPU type invalidation for hw/arm/virt machines >> PATCH[3] support "host-arm-cpu" CPU type only when KVM or HVF is visible >> >> Testing >> ======= >> >> With the following command lines, the output messages are varied before >> and after the series is applied. >> >> /home/gshan/sandbox/src/qemu/main/build/qemu-system-aarch64 \ >> -accel tcg -machine virt,gic-version=3,nvdimm=on \ >> -cpu cortex-a8 -smp maxcpus=2,cpus=1 \ >> : >> >> Before the series is applied: >> >> qemu-system-aarch64: mach-virt: CPU type cortex-a8-arm-cpu not supported >> >> After the series is applied: >> >> qemu-system-aarch64: Invalid CPU type: cortex-a8-arm-cpu >> The valid types are: cortex-a7-arm-cpu, cortex-a15-arm-cpu, \ >> cortex-a35-arm-cpu, cortex-a55-arm-cpu, cortex-a72-arm-cpu, \ >> cortex-a76-arm-cpu, a64fx-arm-cpu, neoverse-n1-arm-cpu, \ >> neoverse-v1-arm-cpu, cortex-a53-arm-cpu, cortex-a57-arm-cpu, \ >> max-arm-cpu > > I see this isn't a change in this patch, but given that > what the user specifies is not "cortex-a8-arm-cpu" but > "cortex-a8", why do we include the "-arm-cpu" suffix in > the error messages? It's not valid syntax to say > "-cpu cortex-a8-arm-cpu", so it's a bit misleading... > Good point. Right, the complete CPU type names are provided by board (hw/arm/virt). The compelte CPU type names are used in hw/core/machine.c to search the object class. In the error messages in the same source file, the complete CPU type names are also used. Actually, we need 'internal' names like 'cortex-a8' to be shown in the error messages. For the solution, I've suggested to split the MachineClass::valid_cpu_types to two fields (valid_cpu_types and valid_cpu_type_suffix). Their combination is the complete CPU type name and 'valid_cpu_types[i]' corresponds to the 'internal' name, to be used in the error messages. Please take a look on that thread and reply to it. Thanks, Gavin
W dniu 13.07.2023 o 14:34, Gavin Shan pisze: > On 7/13/23 21:52, Marcin Juszkiewicz wrote: >> W dniu 13.07.2023 o 13:44, Peter Maydell pisze: >> >>> I see this isn't a change in this patch, but given that what the >>> user specifies is not "cortex-a8-arm-cpu" but "cortex-a8", why >>> do we include the "-arm-cpu" suffix in the error messages? It's >>> not valid syntax to say "-cpu cortex-a8-arm-cpu", so it's a bit >>> misleading... >> I like the change but it (IMHO) needs to cut "-{TYPE_*_CPU}" >> string from names: > Peter and Marcin, how about to split the CPU types to two fields, as > below? In this way, the complete CPU type will be used for validation > and the 'internal' names will be used for the error messages. Note that it should probably propagate to all architectures handled in QEMU, not only Arm ones. I do not know which machines have list of supported cpu types like arm/virt or arm/sbsa-ref ones. >> I can change sbsa-ref to follow that change but let it be >> userfriendly. > Yes, sbsa-ref needs an extra patch to use the generic invalidation. > I can add one patch on the top for that in next revision if you > agree, Marcin. I am fine with it.
Hi Marcin, On 7/13/23 22:44, Marcin Juszkiewicz wrote: > W dniu 13.07.2023 o 14:34, Gavin Shan pisze: >> On 7/13/23 21:52, Marcin Juszkiewicz wrote: >>> W dniu 13.07.2023 o 13:44, Peter Maydell pisze: >>> >>>> I see this isn't a change in this patch, but given that what the user specifies is not "cortex-a8-arm-cpu" but "cortex-a8", why >>>> do we include the "-arm-cpu" suffix in the error messages? It's >>>> not valid syntax to say "-cpu cortex-a8-arm-cpu", so it's a bit misleading... > >>> I like the change but it (IMHO) needs to cut "-{TYPE_*_CPU}" >>> string from names: > >> Peter and Marcin, how about to split the CPU types to two fields, as below? In this way, the complete CPU type will be used for validation >> and the 'internal' names will be used for the error messages. > > Note that it should probably propagate to all architectures handled in QEMU, not only Arm ones. I do not know which machines have list of supported cpu types like arm/virt or arm/sbsa-ref ones. Right, there are more boards eligible for this generic CPU type invalidation. I will cover all of them in next revision. Currently, the pattern of prefix + suffix, used to split the CPU type name, can work well for all cases. [gshan@gshan hw]$ git grep strcmp.*cpu_type arm/bananapi_m2u.c: if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a7")) != 0) { arm/cubieboard.c: if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a8")) != 0) { arm/mps2-tz.c: if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) { arm/mps2.c: if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) { arm/msf2-som.c: if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) { arm/musca.c: if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) { arm/npcm7xx_boards.c: if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) { arm/orangepi.c: if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a7")) != 0) { avr/boot.c: if (strcmp(elf_cpu, mcu_cpu_type)) { <<<<< needsn't CPU type validation riscv/shakti_c.c: if (strcmp(mstate->cpu_type, TYPE_RISCV_CPU_SHAKTI_C) != 0) { >>> I can change sbsa-ref to follow that change but let it be userfriendly. > >> Yes, sbsa-ref needs an extra patch to use the generic invalidation. >> I can add one patch on the top for that in next revision if you >> agree, Marcin. > > I am fine with it. > Thanks a lot for your confirm, Marcin. Thanks, Gavin
On 13/7/23 14:34, Gavin Shan wrote: > Hi Peter and Marcin, > > On 7/13/23 21:52, Marcin Juszkiewicz wrote: >> W dniu 13.07.2023 o 13:44, Peter Maydell pisze: >> >>> I see this isn't a change in this patch, but given that >>> what the user specifies is not "cortex-a8-arm-cpu" but >>> "cortex-a8", why do we include the "-arm-cpu" suffix in >>> the error messages? It's not valid syntax to say >>> "-cpu cortex-a8-arm-cpu", so it's a bit misleading... >> >> Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for >> other architectures. >> >> I like the change but it (IMHO) needs to cut "-{TYPE_*_CPU}" string >> from names: >> >> 13:37 marcin@applejack:qemu$ >> ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-r5 >> qemu-system-aarch64: Invalid CPU type: cortex-r5-arm-cpu >> The valid types are: cortex-a7-arm-cpu, cortex-a15-arm-cpu, >> cortex-a35-arm-cpu, cortex-a55-arm-cpu, cortex-a72-arm-cpu, >> cortex-a76-arm-cpu, a64fx-arm-cpu, neoverse-n1-arm-cpu, >> neoverse-v1-arm-cpu, cortex-a53-arm-cpu, cortex-a57-arm-cpu, >> host-arm-cpu, max-arm-cpu >> >> 13:37 marcin@applejack:qemu$ >> ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu >> cortex-a57-arm-cpu >> qemu-system-aarch64: unable to find CPU model 'cortex-a57-arm-cpu' >> > > The suffix of CPU types are provided in hw/arm/virt.c::valid_cpu_types > in PATCH[2]. > In the generic validation, the complete CPU type is used. The error > message also > have complete CPU type there. In some places (arm_cpu_list_entry, arm_cpu_add_definition) we use: g_strndup(typename, strlen(typename) - strlen("-" TYPE_ARM_CPU)) Maybe extract as a helper? cpu_typename_name()? :)
On 7/13/23 13:34, Gavin Shan wrote: > Hi Peter and Marcin, > > On 7/13/23 21:52, Marcin Juszkiewicz wrote: >> W dniu 13.07.2023 o 13:44, Peter Maydell pisze: >> >>> I see this isn't a change in this patch, but given that >>> what the user specifies is not "cortex-a8-arm-cpu" but >>> "cortex-a8", why do we include the "-arm-cpu" suffix in >>> the error messages? It's not valid syntax to say >>> "-cpu cortex-a8-arm-cpu", so it's a bit misleading... >> >> Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for other architectures. >> >> I like the change but it (IMHO) needs to cut "-{TYPE_*_CPU}" string from names: >> >> 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu >> cortex-r5 >> qemu-system-aarch64: Invalid CPU type: cortex-r5-arm-cpu >> The valid types are: cortex-a7-arm-cpu, cortex-a15-arm-cpu, cortex-a35-arm-cpu, >> cortex-a55-arm-cpu, cortex-a72-arm-cpu, cortex-a76-arm-cpu, a64fx-arm-cpu, >> neoverse-n1-arm-cpu, neoverse-v1-arm-cpu, cortex-a53-arm-cpu, cortex-a57-arm-cpu, >> host-arm-cpu, max-arm-cpu >> >> 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu >> cortex-a57-arm-cpu >> qemu-system-aarch64: unable to find CPU model 'cortex-a57-arm-cpu' >> > > The suffix of CPU types are provided in hw/arm/virt.c::valid_cpu_types in PATCH[2]. > In the generic validation, the complete CPU type is used. The error message also > have complete CPU type there. > > Peter and Marcin, how about to split the CPU types to two fields, as below? In this > way, the complete CPU type will be used for validation and the 'internal' names will > be used for the error messages. > > struct MachineClass { > const char *valid_cpu_type_suffix; > const char **valid_cpu_types; While you're changing this: const char * const *valid_cpu_types; > }; > > hw/arm/virt.c > ------------- > > static const char *valid_cpu_types[] = { So that you can then do static const char * const valid_cpu_types[] r~
Hi Philippe, On 7/14/23 02:29, Philippe Mathieu-Daudé wrote: > On 13/7/23 14:34, Gavin Shan wrote: >> On 7/13/23 21:52, Marcin Juszkiewicz wrote: >>> W dniu 13.07.2023 o 13:44, Peter Maydell pisze: >>> >>>> I see this isn't a change in this patch, but given that >>>> what the user specifies is not "cortex-a8-arm-cpu" but >>>> "cortex-a8", why do we include the "-arm-cpu" suffix in >>>> the error messages? It's not valid syntax to say >>>> "-cpu cortex-a8-arm-cpu", so it's a bit misleading... >>> >>> Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for other architectures. >>> >>> I like the change but it (IMHO) needs to cut "-{TYPE_*_CPU}" string from names: >>> >>> 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-r5 >>> qemu-system-aarch64: Invalid CPU type: cortex-r5-arm-cpu >>> The valid types are: cortex-a7-arm-cpu, cortex-a15-arm-cpu, cortex-a35-arm-cpu, cortex-a55-arm-cpu, cortex-a72-arm-cpu, cortex-a76-arm-cpu, a64fx-arm-cpu, neoverse-n1-arm-cpu, neoverse-v1-arm-cpu, cortex-a53-arm-cpu, cortex-a57-arm-cpu, host-arm-cpu, max-arm-cpu >>> >>> 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-a57-arm-cpu >>> qemu-system-aarch64: unable to find CPU model 'cortex-a57-arm-cpu' >>> >> >> The suffix of CPU types are provided in hw/arm/virt.c::valid_cpu_types in PATCH[2]. >> In the generic validation, the complete CPU type is used. The error message also >> have complete CPU type there. > > In some places (arm_cpu_list_entry, arm_cpu_add_definition) we use: > > g_strndup(typename, strlen(typename) - strlen("-" TYPE_ARM_CPU)) > > Maybe extract as a helper? cpu_typename_name()? :) > Yeah, it's definitely a good idea. The helper is needed by all architectures, not ARM alone. The following CPU types don't have explicit definition of XXXX_CPU_TYPE_SUFFIX. We need take "-" TYPE_CPU as the suffix. target/microblaze/cpu.c TYPE_MICROBLAZE_CPU target/hppa/cpu.c TYPE_HPPA_CPU target/nios2/cpu.c TYPE_NIOS2_CPU target/microblaze/cpu-qom.h:#define TYPE_MICROBLAZE_CPU "microblaze-cpu" target/hppa/cpu-qom.h: #define TYPE_HPPA_CPU "hppa-cpu" target/nios2/cpu.h: #define TYPE_NIOS2_CPU "nios2-cpu" I think the function name can be cpu_model_name() since we have called it as 'model' in cpu.c::parse_cpu_option(). Something like below. Please let me know if you have more comments. target/xxxx/cpu.h ----------------- static inline char *cpu_model_name(const char *typename) { return g_strndup(typename, strlen(typename) - strlen(TYPE_XXX_CPU_SUFFIX)); } Thanks, Gavin
Hi Richard, On 7/14/23 05:27, Richard Henderson wrote: > On 7/13/23 13:34, Gavin Shan wrote: >> On 7/13/23 21:52, Marcin Juszkiewicz wrote: >>> W dniu 13.07.2023 o 13:44, Peter Maydell pisze: >>> >>>> I see this isn't a change in this patch, but given that >>>> what the user specifies is not "cortex-a8-arm-cpu" but >>>> "cortex-a8", why do we include the "-arm-cpu" suffix in >>>> the error messages? It's not valid syntax to say >>>> "-cpu cortex-a8-arm-cpu", so it's a bit misleading... >>> >>> Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for other architectures. >>> >>> I like the change but it (IMHO) needs to cut "-{TYPE_*_CPU}" string from names: >>> >>> 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-r5 >>> qemu-system-aarch64: Invalid CPU type: cortex-r5-arm-cpu >>> The valid types are: cortex-a7-arm-cpu, cortex-a15-arm-cpu, cortex-a35-arm-cpu, cortex-a55-arm-cpu, cortex-a72-arm-cpu, cortex-a76-arm-cpu, a64fx-arm-cpu, neoverse-n1-arm-cpu, neoverse-v1-arm-cpu, cortex-a53-arm-cpu, cortex-a57-arm-cpu, host-arm-cpu, max-arm-cpu >>> >>> 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-a57-arm-cpu >>> qemu-system-aarch64: unable to find CPU model 'cortex-a57-arm-cpu' >>> >> >> The suffix of CPU types are provided in hw/arm/virt.c::valid_cpu_types in PATCH[2]. >> In the generic validation, the complete CPU type is used. The error message also >> have complete CPU type there. >> >> Peter and Marcin, how about to split the CPU types to two fields, as below? In this >> way, the complete CPU type will be used for validation and the 'internal' names will >> be used for the error messages. >> >> struct MachineClass { >> const char *valid_cpu_type_suffix; >> const char **valid_cpu_types; > > While you're changing this: > > const char * const *valid_cpu_types; > yes, will do. >> }; >> >> hw/arm/virt.c >> ------------- >> >> static const char *valid_cpu_types[] = { > > So that you can then do > > static const char * const valid_cpu_types[] > yes, will do. Thanks, Gavin
On 7/14/23 10:51, Gavin Shan wrote: > On 7/14/23 02:29, Philippe Mathieu-Daudé wrote: >> On 13/7/23 14:34, Gavin Shan wrote: >>> On 7/13/23 21:52, Marcin Juszkiewicz wrote: >>>> W dniu 13.07.2023 o 13:44, Peter Maydell pisze: >>>> >>>>> I see this isn't a change in this patch, but given that >>>>> what the user specifies is not "cortex-a8-arm-cpu" but >>>>> "cortex-a8", why do we include the "-arm-cpu" suffix in >>>>> the error messages? It's not valid syntax to say >>>>> "-cpu cortex-a8-arm-cpu", so it's a bit misleading... >>>> >>>> Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for other architectures. >>>> >>>> I like the change but it (IMHO) needs to cut "-{TYPE_*_CPU}" string from names: >>>> >>>> 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-r5 >>>> qemu-system-aarch64: Invalid CPU type: cortex-r5-arm-cpu >>>> The valid types are: cortex-a7-arm-cpu, cortex-a15-arm-cpu, cortex-a35-arm-cpu, cortex-a55-arm-cpu, cortex-a72-arm-cpu, cortex-a76-arm-cpu, a64fx-arm-cpu, neoverse-n1-arm-cpu, neoverse-v1-arm-cpu, cortex-a53-arm-cpu, cortex-a57-arm-cpu, host-arm-cpu, max-arm-cpu >>>> >>>> 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-a57-arm-cpu >>>> qemu-system-aarch64: unable to find CPU model 'cortex-a57-arm-cpu' >>>> >>> >>> The suffix of CPU types are provided in hw/arm/virt.c::valid_cpu_types in PATCH[2]. >>> In the generic validation, the complete CPU type is used. The error message also >>> have complete CPU type there. >> >> In some places (arm_cpu_list_entry, arm_cpu_add_definition) we use: >> >> g_strndup(typename, strlen(typename) - strlen("-" TYPE_ARM_CPU)) >> >> Maybe extract as a helper? cpu_typename_name()? :) >> > > Yeah, it's definitely a good idea. The helper is needed by all architectures, > not ARM alone. The following CPU types don't have explicit definition of > XXXX_CPU_TYPE_SUFFIX. We need take "-" TYPE_CPU as the suffix. > > target/microblaze/cpu.c TYPE_MICROBLAZE_CPU > target/hppa/cpu.c TYPE_HPPA_CPU > target/nios2/cpu.c TYPE_NIOS2_CPU > > target/microblaze/cpu-qom.h:#define TYPE_MICROBLAZE_CPU "microblaze-cpu" > target/hppa/cpu-qom.h: #define TYPE_HPPA_CPU "hppa-cpu" > target/nios2/cpu.h: #define TYPE_NIOS2_CPU "nios2-cpu" > > I think the function name can be cpu_model_name() since we have called it > as 'model' in cpu.c::parse_cpu_option(). Something like below. Please let > me know if you have more comments. > > target/xxxx/cpu.h > ----------------- > > static inline char *cpu_model_name(const char *typename) > { > return g_strndup(typename, strlen(typename) - strlen(TYPE_XXX_CPU_SUFFIX)); > } > I found the generic CPU type invalidation in hw/core/machine.c can't see functions from target/xxx/, including cpu_model_name(). In order to call this function from hw/core/machine.c, we need transit in cpu.c include/exec/cpu-common.h ------------------------- char *cpu_get_model_name(const char *name); void list_cpus(void); cpu.c ----- char *cpu_get_model_name(const char *name) { return cpu_model_name(name); } With above hunk of changes, cpu_get_model_name() can be called in hw/core/machine.c, to extract the CPU model name from the CPU type name. Thanks, Gavin
On Thu, 13 Jul 2023 12:59:55 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > On Thu, 13 Jul 2023 at 12:52, Marcin Juszkiewicz > <marcin.juszkiewicz@linaro.org> wrote: > > > > W dniu 13.07.2023 o 13:44, Peter Maydell pisze: > > > > > I see this isn't a change in this patch, but given that > > > what the user specifies is not "cortex-a8-arm-cpu" but > > > "cortex-a8", why do we include the "-arm-cpu" suffix in > > > the error messages? It's not valid syntax to say > > > "-cpu cortex-a8-arm-cpu", so it's a bit misleading... > > > > Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for > > other architectures. > > Yes; my question is "why are we not using the user-facing > string rather than the internal type name?". With other targets full CPU type name can also be valid user-facing string. Namely we use it with -device/device_add interface. Considering we would like to have CPU hotplug on ARM as well, we shouldn't not outlaw full type name. (QMP/monitor interface also mostly uses full type names) Instead it might be better to consolidate on what has been done on making CPU '-device' compatible and allow to use full CPU type name with '-cpu' on arm machines. Then later call suffix-less legacy => deprecate/drop it from user-facing side including cleanup of all the infra we've invented to keep mapping between cpu_model and typename. With that gone, listing/restricting (supported) cpu types can be done without extra processing and likely can be done in one place for all targets/cpus instead of zoo we have now. (extra bonus: all error messages that include CPU name will become consistent with the rest as well, since only CPU typename is left around) > -- PMM >
On Fri, 14 Jul 2023 at 12:50, Igor Mammedov <imammedo@redhat.com> wrote: > > On Thu, 13 Jul 2023 12:59:55 +0100 > Peter Maydell <peter.maydell@linaro.org> wrote: > > > On Thu, 13 Jul 2023 at 12:52, Marcin Juszkiewicz > > <marcin.juszkiewicz@linaro.org> wrote: > > > > > > W dniu 13.07.2023 o 13:44, Peter Maydell pisze: > > > > > > > I see this isn't a change in this patch, but given that > > > > what the user specifies is not "cortex-a8-arm-cpu" but > > > > "cortex-a8", why do we include the "-arm-cpu" suffix in > > > > the error messages? It's not valid syntax to say > > > > "-cpu cortex-a8-arm-cpu", so it's a bit misleading... > > > > > > Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for > > > other architectures. > > > > Yes; my question is "why are we not using the user-facing > > string rather than the internal type name?". > > With other targets full CPU type name can also be valid > user-facing string. Namely we use it with -device/device_add > interface. Considering we would like to have CPU hotplug > on ARM as well, we shouldn't not outlaw full type name. > (QMP/monitor interface also mostly uses full type names) You don't seem to be able to use the full type name on x86-64 either: $ ./build/all/qemu-system-x86_64 -cpu pentium-x86_64-cpu qemu-system-x86_64: unable to find CPU model 'pentium-x86_64-cpu' and '-cpu help' does not list them with the suffix. > Instead it might be better to consolidate on what has > been done on making CPU '-device' compatible and > allow to use full CPU type name with '-cpu' on arm machines. > > Then later call suffix-less legacy => deprecate/drop it from > user-facing side including cleanup of all the infra we've > invented to keep mapping between cpu_model and typename. This seems to me like a worsening of the user interface, and in practice there is not much likelihood of being able to deprecate-and-drop the nicer user-facing names, because they are baked into so many command lines and scripts. thanks -- PMM
On Fri, 14 Jul 2023 13:56:00 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > On Fri, 14 Jul 2023 at 12:50, Igor Mammedov <imammedo@redhat.com> wrote: > > > > On Thu, 13 Jul 2023 12:59:55 +0100 > > Peter Maydell <peter.maydell@linaro.org> wrote: > > > > > On Thu, 13 Jul 2023 at 12:52, Marcin Juszkiewicz > > > <marcin.juszkiewicz@linaro.org> wrote: > > > > > > > > W dniu 13.07.2023 o 13:44, Peter Maydell pisze: > > > > > > > > > I see this isn't a change in this patch, but given that > > > > > what the user specifies is not "cortex-a8-arm-cpu" but > > > > > "cortex-a8", why do we include the "-arm-cpu" suffix in > > > > > the error messages? It's not valid syntax to say > > > > > "-cpu cortex-a8-arm-cpu", so it's a bit misleading... > > > > > > > > Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for > > > > other architectures. > > > > > > Yes; my question is "why are we not using the user-facing > > > string rather than the internal type name?". > > > > With other targets full CPU type name can also be valid > > user-facing string. Namely we use it with -device/device_add > > interface. Considering we would like to have CPU hotplug > > on ARM as well, we shouldn't not outlaw full type name. > > (QMP/monitor interface also mostly uses full type names) > > You don't seem to be able to use the full type name on > x86-64 either: > > $ ./build/all/qemu-system-x86_64 -cpu pentium-x86_64-cpu > qemu-system-x86_64: unable to find CPU model 'pentium-x86_64-cpu' that's because it also tied into old cpu_model resolving routines, and I haven't added typename lookup the last time I've touched it (it was out of topic change anyway). but some targets do recognize typename, while some do a lot more juggling with cpu_model (alpha/ppc), and yet another class (garbage in => cpu type out). With the last one we could just error, while with alpha/ppc we could dumb it down to typenames only. > and '-cpu help' does not list them with the suffix. both above points are fixable, I can prepare PoC patches for that if there is no opposition to the idea. > > Instead it might be better to consolidate on what has > > been done on making CPU '-device' compatible and > > allow to use full CPU type name with '-cpu' on arm machines. > > > > Then later call suffix-less legacy => deprecate/drop it from > > user-facing side including cleanup of all the infra we've > > invented to keep mapping between cpu_model and typename. > > This seems to me like a worsening of the user interface, > and in practice there is not much likelihood of being > able to deprecate-and-drop the nicer user-facing names, > because they are baked into so many command lines and > scripts. Nice names are subjective point, I suspect in a long run once users switched to using longer names, they won't care much about that either. Also it's arguable if it is worsening UI or not. I see using consolidated typenames across the board (incl. UI) as a positive development. As for scripts/CLI users out there, yes it would be disruptive for a while but one can adapt to new naming (or use a wrapper around QEMU that does suffix adding/model mapping as a crutch). It weren't possible to drop anything before we introduced deprecation process, but with it we can do it. > thanks > -- PMM >
Hi Igor, On 7/17/23 22:44, Igor Mammedov wrote: > On Fri, 14 Jul 2023 13:56:00 +0100 > Peter Maydell <peter.maydell@linaro.org> wrote: > >> On Fri, 14 Jul 2023 at 12:50, Igor Mammedov <imammedo@redhat.com> wrote: >>> >>> On Thu, 13 Jul 2023 12:59:55 +0100 >>> Peter Maydell <peter.maydell@linaro.org> wrote: >>> >>>> On Thu, 13 Jul 2023 at 12:52, Marcin Juszkiewicz >>>> <marcin.juszkiewicz@linaro.org> wrote: >>>>> >>>>> W dniu 13.07.2023 o 13:44, Peter Maydell pisze: >>>>> >>>>>> I see this isn't a change in this patch, but given that >>>>>> what the user specifies is not "cortex-a8-arm-cpu" but >>>>>> "cortex-a8", why do we include the "-arm-cpu" suffix in >>>>>> the error messages? It's not valid syntax to say >>>>>> "-cpu cortex-a8-arm-cpu", so it's a bit misleading... >>>>> >>>>> Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for >>>>> other architectures. >>>> >>>> Yes; my question is "why are we not using the user-facing >>>> string rather than the internal type name?". >>> >>> With other targets full CPU type name can also be valid >>> user-facing string. Namely we use it with -device/device_add >>> interface. Considering we would like to have CPU hotplug >>> on ARM as well, we shouldn't not outlaw full type name. >>> (QMP/monitor interface also mostly uses full type names) >> >> You don't seem to be able to use the full type name on >> x86-64 either: >> >> $ ./build/all/qemu-system-x86_64 -cpu pentium-x86_64-cpu >> qemu-system-x86_64: unable to find CPU model 'pentium-x86_64-cpu' > > that's because it also tied into old cpu_model resolving > routines, and I haven't added typename lookup the last > time I've touched it (it was out of topic change anyway). > > but some targets do recognize typename, while some > do a lot more juggling with cpu_model (alpha/ppc), > and yet another class (garbage in => cpu type out). > > With the last one we could just error, > while with alpha/ppc we could dumb it down to typenames > only. > Your summary here is correct to me. However, I don't quiet understand the issue you're trying to resolve. As you mentioned, there are two cases where the users need to specify CPU typename: (1) In the command lines to start VM; (2) When CPU is hot added. For (1), the list of all available CPU is provided by each individual target. It's to say, each individual target is responsible for correlating the name (typename, CPU model name, or whatever else). Each individual target has its own rules for this correlation. Why do we bother to unify the rules so that only the typename is allowed? // // The output can be directly used in the command lines to start VM. // I don't see any problems we have. Note that the list of available // CPU names is printed by cpu_list(), which is a target specific // implementation. // # aarch64-softmmu/qemu-system-aarch64 -cpu help Available CPUs: a64fx arm1026 arm1136 arm1136-r2 arm1176 arm11mpcore arm926 arm946 cortex-a15 cortex-a35 cortex-a53 For (2) where CPU is hot added, the help option can also be used to dump the available CPUs. Nothing went to wrong as I can see. The rule used here to correlate names with CPUs is global: typename <-> CPU // // The printed CPU typenames can be taken as the driver directly // (qemu) device_add driver=? : CPU devices: name "a64fx-arm-cpu" name "cortex-a35-arm-cpu" name "cortex-a53-arm-cpu" name "cortex-a55-arm-cpu" name "cortex-a57-arm-cpu" name "cortex-a72-arm-cpu" name "cortex-a76-arm-cpu" name "max-arm-cpu" name "neoverse-n1-arm-cpu" >> and '-cpu help' does not list them with the suffix. > > both above points are fixable, > > I can prepare PoC patches for that if there is > no opposition to the idea. > Please refer to above for the argument. If I'm correct, there is nothing to be resolved or improved. >>> Instead it might be better to consolidate on what has >>> been done on making CPU '-device' compatible and >>> allow to use full CPU type name with '-cpu' on arm machines. >>> >>> Then later call suffix-less legacy => deprecate/drop it from >>> user-facing side including cleanup of all the infra we've >>> invented to keep mapping between cpu_model and typename. >> >> This seems to me like a worsening of the user interface, >> and in practice there is not much likelihood of being >> able to deprecate-and-drop the nicer user-facing names, >> because they are baked into so many command lines and >> scripts. > Nice names are subjective point, I suspect in a long run > once users switched to using longer names, they won't care much > about that either. > > Also it's arguable if it is worsening UI or not. > I see using consolidated typenames across the board (incl. UI) > as a positive development. > > As for scripts/CLI users out there, yes it would be disruptive > for a while but one can adapt to new naming (or use a wrapper > around QEMU that does suffix adding/model mapping as a crutch). > > It weren't possible to drop anything before we introduced > deprecation process, but with it we can do it. > Thanks, Gavin
On Tue, 18 Jul 2023 20:31:39 +1000 Gavin Shan <gshan@redhat.com> wrote: > Hi Igor, > > On 7/17/23 22:44, Igor Mammedov wrote: > > On Fri, 14 Jul 2023 13:56:00 +0100 > > Peter Maydell <peter.maydell@linaro.org> wrote: > > > >> On Fri, 14 Jul 2023 at 12:50, Igor Mammedov <imammedo@redhat.com> wrote: > >>> > >>> On Thu, 13 Jul 2023 12:59:55 +0100 > >>> Peter Maydell <peter.maydell@linaro.org> wrote: > >>> > >>>> On Thu, 13 Jul 2023 at 12:52, Marcin Juszkiewicz > >>>> <marcin.juszkiewicz@linaro.org> wrote: > >>>>> > >>>>> W dniu 13.07.2023 o 13:44, Peter Maydell pisze: > >>>>> > >>>>>> I see this isn't a change in this patch, but given that > >>>>>> what the user specifies is not "cortex-a8-arm-cpu" but > >>>>>> "cortex-a8", why do we include the "-arm-cpu" suffix in > >>>>>> the error messages? It's not valid syntax to say > >>>>>> "-cpu cortex-a8-arm-cpu", so it's a bit misleading... > >>>>> > >>>>> Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for > >>>>> other architectures. > >>>> > >>>> Yes; my question is "why are we not using the user-facing > >>>> string rather than the internal type name?". > >>> > >>> With other targets full CPU type name can also be valid > >>> user-facing string. Namely we use it with -device/device_add > >>> interface. Considering we would like to have CPU hotplug > >>> on ARM as well, we shouldn't not outlaw full type name. > >>> (QMP/monitor interface also mostly uses full type names) > >> > >> You don't seem to be able to use the full type name on > >> x86-64 either: > >> > >> $ ./build/all/qemu-system-x86_64 -cpu pentium-x86_64-cpu > >> qemu-system-x86_64: unable to find CPU model 'pentium-x86_64-cpu' > > > > that's because it also tied into old cpu_model resolving > > routines, and I haven't added typename lookup the last > > time I've touched it (it was out of topic change anyway). > > > > but some targets do recognize typename, while some > > do a lot more juggling with cpu_model (alpha/ppc), > > and yet another class (garbage in => cpu type out). > > > > With the last one we could just error, > > while with alpha/ppc we could dumb it down to typenames > > only. > > > > Your summary here is correct to me. However, I don't quiet understand > the issue you're trying to resolve. As you mentioned, there are two > cases where the users need to specify CPU typename: (1) In the command > lines to start VM; (2) When CPU is hot added. > > For (1), the list of all available CPU is provided by each individual > target. It's to say, each individual target is responsible for correlating > the name (typename, CPU model name, or whatever else). Each individual > target has its own rules for this correlation. Why do we bother to unify > the rules so that only the typename is allowed? I've seen others asking why you print type name instead of shorter cpu-model used on CLI. To do that would make you write a patch to implement reverse mapping. In some cases it's simple, in others plain impossible unless you can get access to -cpu foo stored somewhere. What I don't particularity like about adding reverse type->cpu_model mapping, is that it would complicate code to carter to QEMU's interface inconsistencies. And if you do it easy way (instead of fixing every target) touching only ARM, it will be spotty at best and just add to technical debt elsewhere -> more inconsistencies. What I'm proposing is for you to keep printing type names. So if others won't object to type names I'm more or less fine with your current approach. Instead of adding type->cpu_model mapping (it's not the first time this particular question had arisen - there were similar patches before on qemu-devel), get rid of shortened cpu_model in user interface (-cpu) altogether and use CPU type name there. Beside making UI consistent across QEMU it will also simplify QEMU codebase (cpu-model -> type resolving machinery + all legacy junk that was accumulated for years QEMU has existed). > // The output can be directly used in the command lines to start VM. > // I don't see any problems we have. Note that the list of available > // CPU names is printed by cpu_list(), which is a target specific > // implementation. > // > # aarch64-softmmu/qemu-system-aarch64 -cpu help > Available CPUs: > a64fx > arm1026 > arm1136 > arm1136-r2 > arm1176 > arm11mpcore > arm926 > arm946 > cortex-a15 > cortex-a35 > cortex-a53 > > For (2) where CPU is hot added, the help option can also be used to dump > the available CPUs. Nothing went to wrong as I can see. The rule used here > to correlate names with CPUs is global: typename <-> CPU > > // > // The printed CPU typenames can be taken as the driver directly > // > (qemu) device_add driver=? > : > CPU devices: > name "a64fx-arm-cpu" > name "cortex-a35-arm-cpu" > name "cortex-a53-arm-cpu" > name "cortex-a55-arm-cpu" > name "cortex-a57-arm-cpu" > name "cortex-a72-arm-cpu" > name "cortex-a76-arm-cpu" > name "max-arm-cpu" > name "neoverse-n1-arm-cpu" > > >> and '-cpu help' does not list them with the suffix. > > > > both above points are fixable, > > > > I can prepare PoC patches for that if there is > > no opposition to the idea. > > > > Please refer to above for the argument. If I'm correct, there is nothing > to be resolved or improved. > > >>> Instead it might be better to consolidate on what has > >>> been done on making CPU '-device' compatible and > >>> allow to use full CPU type name with '-cpu' on arm machines. > >>> > >>> Then later call suffix-less legacy => deprecate/drop it from > >>> user-facing side including cleanup of all the infra we've > >>> invented to keep mapping between cpu_model and typename. > >> > >> This seems to me like a worsening of the user interface, > >> and in practice there is not much likelihood of being > >> able to deprecate-and-drop the nicer user-facing names, > >> because they are baked into so many command lines and > >> scripts. > > Nice names are subjective point, I suspect in a long run > > once users switched to using longer names, they won't care much > > about that either. > > > > Also it's arguable if it is worsening UI or not. > > I see using consolidated typenames across the board (incl. UI) > > as a positive development. > > > > As for scripts/CLI users out there, yes it would be disruptive > > for a while but one can adapt to new naming (or use a wrapper > > around QEMU that does suffix adding/model mapping as a crutch). > > > > It weren't possible to drop anything before we introduced > > deprecation process, but with it we can do it. > > > > Thanks, > Gavin >
On Mon, 24 Jul 2023 at 16:06, Igor Mammedov <imammedo@redhat.com> wrote: > I've seen others asking why you print type name instead of shorter cpu-model > used on CLI. To do that would make you write a patch to implement reverse mapping. > In some cases it's simple, in others plain impossible unless you can get > access to -cpu foo stored somewhere. > > What I don't particularity like about adding reverse type->cpu_model mapping, > is that it would complicate code to carter to QEMU's interface inconsistencies. > And if you do it easy way (instead of fixing every target) touching only ARM, > it will be spotty at best and just add to technical debt elsewhere -> > more inconsistencies. > > What I'm proposing is for you to keep printing type names. > So if others won't object to type names I'm more or less fine with your > current approach. I do object to type names, because the UI for choosing a CPU ("-cpu whatever") does not take type names, it takes CPU names. The QOM type names that those end up being under the hood are a detail of QEMU's implementation that we shouldn't expose to users in the help messages. > Instead of adding type->cpu_model mapping (it's not the first time > this particular question had arisen - there were similar patches before > on qemu-devel), get rid of shortened cpu_model in user interface (-cpu) > altogether and use CPU type name there. I also think we should not do this, because it will break a ton of existing command lines, and it's not clear it has any benefit to users. thanks -- PMM
On Mon, 24 Jul 2023 16:14:22 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > On Mon, 24 Jul 2023 at 16:06, Igor Mammedov <imammedo@redhat.com> wrote: > > I've seen others asking why you print type name instead of shorter cpu-model > > used on CLI. To do that would make you write a patch to implement reverse mapping. > > In some cases it's simple, in others plain impossible unless you can get > > access to -cpu foo stored somewhere. > > > > What I don't particularity like about adding reverse type->cpu_model mapping, > > is that it would complicate code to carter to QEMU's interface inconsistencies. > > And if you do it easy way (instead of fixing every target) touching only ARM, > > it will be spotty at best and just add to technical debt elsewhere -> > > more inconsistencies. > > > > What I'm proposing is for you to keep printing type names. > > So if others won't object to type names I'm more or less fine with your > > current approach. > > I do object to type names, because the UI for choosing > a CPU ("-cpu whatever") does not take type names, it > takes CPU names. The QOM type names that those end up > being under the hood are a detail of QEMU's implementation > that we shouldn't expose to users in the help messages. those are exposed to users as a part of -device interface which uses typenames for any device and based on that interface in some monitor/qmp commands > > Instead of adding type->cpu_model mapping (it's not the first time > > this particular question had arisen - there were similar patches before > > on qemu-devel), get rid of shortened cpu_model in user interface (-cpu) > > altogether and use CPU type name there. > > I also think we should not do this, because it will break > a ton of existing command lines, and it's not clear it > has any benefit to users. Yes it will break existing scripts but that for users to fix up once (not to mention that could be worked around with a wrapper, QEMU can even supply that for existing cpu types). (so along with deprecation, we can provide a warning + a wrapper to use, and after deprecation make it a hard error but keep wrapper around for those that do not wish to use typenames) Consistent naming across UI and consistent name -> type handling would be beneficial to users in the long run (especially ones that have to deal with both interfaces so that they won't have to bother which use where). > thanks > -- PMM >
There is a generic CPU type invalidation in machine_run_board_init() and we needn't a same and private invalidation for hw/arm/virt machines. This series intends to use the generic CPU type invalidation on the hw/arm/virt machines. PATCH[1] factors the CPU type invalidation logic in machine_run_board_init() to a helper validate_cpu_type(). PATCH[2] uses the generic CPU type invalidation for hw/arm/virt machines PATCH[3] support "host-arm-cpu" CPU type only when KVM or HVF is visible Testing ======= With the following command lines, the output messages are varied before and after the series is applied. /home/gshan/sandbox/src/qemu/main/build/qemu-system-aarch64 \ -accel tcg -machine virt,gic-version=3,nvdimm=on \ -cpu cortex-a8 -smp maxcpus=2,cpus=1 \ : Before the series is applied: qemu-system-aarch64: mach-virt: CPU type cortex-a8-arm-cpu not supported After the series is applied: qemu-system-aarch64: Invalid CPU type: cortex-a8-arm-cpu The valid types are: cortex-a7-arm-cpu, cortex-a15-arm-cpu, \ cortex-a35-arm-cpu, cortex-a55-arm-cpu, cortex-a72-arm-cpu, \ cortex-a76-arm-cpu, a64fx-arm-cpu, neoverse-n1-arm-cpu, \ neoverse-v1-arm-cpu, cortex-a53-arm-cpu, cortex-a57-arm-cpu, \ max-arm-cpu Gavin Shan (3): machine: Factor CPU type invalidation out into helper hw/arm/virt: Use generic CPU type invalidation hw/arm/virt: Support host CPU type only when KVM or HVF is configured hw/arm/virt.c | 23 +++----------- hw/core/machine.c | 81 +++++++++++++++++++++++++---------------------- 2 files changed, 48 insertions(+), 56 deletions(-)