|
From: | Gavin Shan |
Subject: | Re: [PATCH 0/3] hw/arm/virt: Use generic CPU invalidation |
Date: | Fri, 14 Jul 2023 10:51:29 +1000 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 |
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
[Prev in Thread] | Current Thread | [Next in Thread] |