* [PATCH v7 0/8] Unified CPU type check
@ 2023-11-26 23:12 Gavin Shan
2023-11-26 23:12 ` [PATCH v7 1/8] machine: Use error handling when CPU type is checked Gavin Shan
` (8 more replies)
0 siblings, 9 replies; 21+ messages in thread
From: Gavin Shan @ 2023-11-26 23:12 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, philmd,
wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
This series bases on Phil's repository because the prepatory commits
have been queued to the branch.
https://gitlab.com/philmd/qemu.git (branch: cpus-next)
There are two places where the user specified CPU type is checked to see
if it's supported or allowed by the board: machine_run_board_init() and
mc->init(). We don't have to maintain two duplicate sets of logic. This
series intends to move the check to machine_run_board_init() so that we
have unified CPU type check.
PATCH[1-3] refactors the logic to validate CPU type in machine_run_board_init()
PATCH[4-8] validates the CPU type in machine_run_board_init() for the
individual boards
v1: https://lists.nongnu.org/archive/html/qemu-arm/2023-07/msg00302.html
v2: https://lists.nongnu.org/archive/html/qemu-arm/2023-07/msg00528.html
v3: https://lists.nongnu.org/archive/html/qemu-arm/2023-09/msg00157.html
v4: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00005.html
v5: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00611.html
v6: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00768.html
Testing
=======
Before the series is applied:
[gshan@gshan q]$ ./build/qemu-system-aarch64 -M virt -cpu cortex-a8
qemu-system-aarch64: mach-virt: CPU type cortex-a8-arm-cpu not supported
[gshan@gshan q]$ ./build/qemu-system-aarch64 -M sbsa-ref -cpu cortex-a53
qemu-system-aarch64: sbsa-ref: CPU type cortex-a53-arm-cpu not supported
[gshan@gshan q]$ ./build/qemu-system-aarch64 -M sbsa-ref -cpu sa1100
qemu-system-aarch64: sbsa-ref: CPU type sa1100-arm-cpu not supported
[gshan@gshan q]$ ./build/qemu-system-aarch64 -M sbsa-ref -cpu host
qemu-system-aarch64: unable to find CPU model 'host'
After the series is applied:
[gshan@gshan q]$ ./build/qemu-system-aarch64 -M virt -cpu cortex-a8
qemu-system-aarch64: Invalid CPU type: cortex-a8
The valid types are: cortex-a7, cortex-a15, cortex-a35, cortex-a55, \
cortex-a72, cortex-a76, cortex-a710, a64fx, \
neoverse-n1, neoverse-v1, neoverse-n2, cortex-a53, \
cortex-a57, max
[gshan@gshan q]$ ./build/qemu-system-aarch64 -M sbsa-ref -cpu cortex-a53
qemu-system-aarch64: Invalid CPU type: cortex-a53
The valid types are: cortex-a57, cortex-a72, neoverse-n1, neoverse-v1, \
neoverse-n2, max
[gshan@gshan q]$ ./build/qemu-system-aarch64 -M sbsa-ref -cpu sa1100
qemu-system-aarch64: Invalid CPU type: sa1100
The valid types are: cortex-a57, cortex-a72, neoverse-n1, neoverse-v1, \
neoverse-n2, max
[gshan@gshan q]$ ./build/qemu-system-aarch64 -M sbsa-ref -cpu host
qemu-system-aarch64: unable to find CPU model 'host'
Changelog
=========
v7:
* Add 'return' after error_propagate() in machine_run_board_init()
to avoid calling mc->init() in the failing case (Marcin)
v6:
* Drop PATCH[v5 01-23], queued by Phil (Phil)
* Clearer hint if only one CPU type is supported and have
'const MachineState *' in is_cpu_type_supported() (Phil)
* Move valid_cpu_types[] to board's class_init() function (Phil)
v5:
* PATCH[v5 01] to remove CPU class 'ev67' for alpha (Ricard/Igor)
* PATCH[v5 02] to remove object_class_is_abstract() for hppa (Gavin)
* Don't move cpu_class_by_name() (Richard)
* PATCH[v5 04] to remove 'oc == NULL' since the check has
been covered in object_class_dynamic_cast() (Igor)
* Introduce generic cpu_list(), shared by most of the targets (Richard)
* Use g_str_has_suffix and g_auto_free (Richard)
* Collect r-bs from Igor and Richard (Gavin)
v4:
* Integrate Philippe's patches where cpu_class_by_name()
is consolidated and my duplicate code is dropped (Philippe)
* Simplified changelog and improvements (Thomas)
* g_assert() on the return value from cpu_model_from_type()
in is_cpu_type_supported() (Philippe)
* Collected r-bs from Philippe Mathieu-Daudé, Leif Lindholm,
Bastian Koppelmann, Daniel Henrique Barboza, Cédric Le Goater,
Gavin Shan (Gavin)
v3:
* Generic helper cpu_model_from_type() (Igor)
* Apply cpu_model_from_type() to the individual targets (Igor)
* Implement cpu_list() for the missed targets (Gavin)
* Remove mc->valid_cpu_models (Richard)
* Separate patch to constify mc->validate_cpu_types (Gavin)
v2:
* Constify mc->valid_cpu_types (Richard)
* Print the supported CPU models, instead of typenames (Peter)
* Misc improvements for the hleper to do the check (Igor)
* More patches to move the check (Marcin)
Gavin Shan (8):
machine: Use error handling when CPU type is checked
machine: Introduce helper is_cpu_type_supported()
machine: Print CPU model name instead of CPU type
hw/arm/virt: Hide host CPU model for tcg
hw/arm/virt: Check CPU type in machine_run_board_init()
hw/arm/sbsa-ref: Check CPU type in machine_run_board_init()
hw/arm: Check CPU type in machine_run_board_init()
hw/riscv/shakti_c: Check CPU type in machine_run_board_init()
hw/arm/bananapi_m2u.c | 12 ++---
hw/arm/cubieboard.c | 12 ++---
hw/arm/mps2-tz.c | 26 ++++++++---
hw/arm/mps2.c | 26 ++++++++---
hw/arm/msf2-som.c | 12 ++---
hw/arm/musca.c | 12 +++--
hw/arm/npcm7xx_boards.c | 12 +++--
hw/arm/orangepi.c | 12 ++---
hw/arm/sbsa-ref.c | 36 +++++----------
hw/arm/virt.c | 60 ++++++++++---------------
hw/core/machine.c | 98 +++++++++++++++++++++++++----------------
hw/riscv/shakti_c.c | 13 +++---
12 files changed, 174 insertions(+), 157 deletions(-)
--
2.42.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v7 1/8] machine: Use error handling when CPU type is checked
2023-11-26 23:12 [PATCH v7 0/8] Unified CPU type check Gavin Shan
@ 2023-11-26 23:12 ` Gavin Shan
2023-11-28 9:43 ` Philippe Mathieu-Daudé
2023-11-26 23:12 ` [PATCH v7 2/8] machine: Introduce helper is_cpu_type_supported() Gavin Shan
` (7 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Gavin Shan @ 2023-11-26 23:12 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, philmd,
wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
QEMU will be terminated if the specified CPU type isn't supported
in machine_run_board_init(). The list of supported CPU type names
is tracked by mc->valid_cpu_types.
The error handling can be used to propagate error messages, to be
consistent how the errors are handled for other situations in the
same function.
No functional change intended.
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
v7: Add 'return' after error_propagate() to avoid calling into
mc->init() in the failing case (Marcin)
---
hw/core/machine.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 0c17398141..b3ef325936 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1394,6 +1394,7 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
MachineClass *machine_class = MACHINE_GET_CLASS(machine);
ObjectClass *oc = object_class_by_name(machine->cpu_type);
CPUClass *cc;
+ Error *local_err = NULL;
/* This checkpoint is required by replay to separate prior clock
reading from the other reads, because timer polling functions query
@@ -1466,15 +1467,17 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
if (!machine_class->valid_cpu_types[i]) {
/* 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]);
+ error_setg(&local_err, "Invalid CPU type: %s", machine->cpu_type);
+ error_append_hint(&local_err, "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_append_hint(&local_err, ", %s",
+ machine_class->valid_cpu_types[i]);
}
- error_printf("\n");
+ error_append_hint(&local_err, "\n");
- exit(1);
+ error_propagate(errp, local_err);
+ return;
}
}
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v7 2/8] machine: Introduce helper is_cpu_type_supported()
2023-11-26 23:12 [PATCH v7 0/8] Unified CPU type check Gavin Shan
2023-11-26 23:12 ` [PATCH v7 1/8] machine: Use error handling when CPU type is checked Gavin Shan
@ 2023-11-26 23:12 ` Gavin Shan
2023-11-28 9:38 ` Philippe Mathieu-Daudé
2023-11-26 23:12 ` [PATCH v7 3/8] machine: Print CPU model name instead of CPU type Gavin Shan
` (6 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Gavin Shan @ 2023-11-26 23:12 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, philmd,
wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
The logic, to check if the specified CPU type is supported in
machine_run_board_init(), is independent enough. Factor it out into
helper is_cpu_type_supported(). machine_run_board_init() looks a bit
clean with this. Since we're here, @machine_class is renamed to @mc
to avoid multiple line spanning of code. The error messages and comments
are tweaked a bit either.
No functional change intended.
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
hw/core/machine.c | 90 +++++++++++++++++++++++++++--------------------
1 file changed, 51 insertions(+), 39 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index b3ef325936..05e1922b89 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1387,13 +1387,57 @@ out:
return r;
}
+static void is_cpu_type_supported(const MachineState *machine, Error **errp)
+{
+ MachineClass *mc = MACHINE_GET_CLASS(machine);
+ ObjectClass *oc = object_class_by_name(machine->cpu_type);
+ CPUClass *cc;
+ int i;
+
+ /*
+ * Check if the user specified CPU type is supported when the valid
+ * CPU types have been determined. Note that the user specified CPU
+ * type is provided through '-cpu' option.
+ */
+ if (mc->valid_cpu_types && machine->cpu_type) {
+ for (i = 0; mc->valid_cpu_types[i]; i++) {
+ if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
+ break;
+ }
+ }
+
+ /* The user specified CPU type isn't valid */
+ if (!mc->valid_cpu_types[i]) {
+ error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
+ if (!mc->valid_cpu_types[1]) {
+ error_append_hint(errp, "The only valid type is: %s",
+ mc->valid_cpu_types[0]);
+ } else {
+ error_append_hint(errp, "The valid types are: %s",
+ mc->valid_cpu_types[0]);
+ }
+
+ for (i = 1; mc->valid_cpu_types[i]; i++) {
+ error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
+ }
+
+ error_append_hint(errp, "\n");
+ return;
+ }
+ }
+
+ /* Check if CPU type is deprecated and warn if so */
+ cc = CPU_CLASS(oc);
+ if (cc && cc->deprecation_note) {
+ warn_report("CPU model %s is deprecated -- %s",
+ machine->cpu_type, cc->deprecation_note);
+ }
+}
void machine_run_board_init(MachineState *machine, const char *mem_path, Error **errp)
{
ERRP_GUARD();
MachineClass *machine_class = MACHINE_GET_CLASS(machine);
- ObjectClass *oc = object_class_by_name(machine->cpu_type);
- CPUClass *cc;
Error *local_err = NULL;
/* This checkpoint is required by replay to separate prior clock
@@ -1449,43 +1493,11 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
machine->ram = machine_consume_memdev(machine, machine->memdev);
}
- /* 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) {
- int i;
-
- for (i = 0; machine_class->valid_cpu_types[i]; i++) {
- if (object_class_dynamic_cast(oc,
- machine_class->valid_cpu_types[i])) {
- /* The user specified CPU is in the valid field, we are
- * good to go.
- */
- break;
- }
- }
-
- if (!machine_class->valid_cpu_types[i]) {
- /* The user specified CPU is not valid */
- error_setg(&local_err, "Invalid CPU type: %s", machine->cpu_type);
- error_append_hint(&local_err, "The valid types are: %s",
- machine_class->valid_cpu_types[0]);
- for (i = 1; machine_class->valid_cpu_types[i]; i++) {
- error_append_hint(&local_err, ", %s",
- machine_class->valid_cpu_types[i]);
- }
- error_append_hint(&local_err, "\n");
-
- error_propagate(errp, local_err);
- return;
- }
- }
-
- /* Check if CPU type is deprecated and warn if so */
- cc = CPU_CLASS(oc);
- if (cc && cc->deprecation_note) {
- warn_report("CPU model %s is deprecated -- %s", machine->cpu_type,
- cc->deprecation_note);
+ /* Check if the CPU type is supported */
+ is_cpu_type_supported(machine, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
}
if (machine->cgs) {
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v7 3/8] machine: Print CPU model name instead of CPU type
2023-11-26 23:12 [PATCH v7 0/8] Unified CPU type check Gavin Shan
2023-11-26 23:12 ` [PATCH v7 1/8] machine: Use error handling when CPU type is checked Gavin Shan
2023-11-26 23:12 ` [PATCH v7 2/8] machine: Introduce helper is_cpu_type_supported() Gavin Shan
@ 2023-11-26 23:12 ` Gavin Shan
2023-11-28 9:55 ` Philippe Mathieu-Daudé
2023-11-26 23:12 ` [PATCH v7 4/8] hw/arm/virt: Hide host CPU model for tcg Gavin Shan
` (5 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Gavin Shan @ 2023-11-26 23:12 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, philmd,
wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
The names of supported CPU models instead of CPU types should be
printed when the user specified CPU type isn't supported, to be
consistent with the output from '-cpu ?'.
Correct the error messages to print CPU model names instead of CPU
type names.
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
hw/core/machine.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 05e1922b89..898c25552a 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1392,6 +1392,7 @@ static void is_cpu_type_supported(const MachineState *machine, Error **errp)
MachineClass *mc = MACHINE_GET_CLASS(machine);
ObjectClass *oc = object_class_by_name(machine->cpu_type);
CPUClass *cc;
+ char *model;
int i;
/*
@@ -1408,17 +1409,25 @@ static void is_cpu_type_supported(const MachineState *machine, Error **errp)
/* The user specified CPU type isn't valid */
if (!mc->valid_cpu_types[i]) {
- error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
+ model = cpu_model_from_type(machine->cpu_type);
+ g_assert(model != NULL);
+ error_setg(errp, "Invalid CPU type: %s", model);
+ g_free(model);
+
+ model = cpu_model_from_type(mc->valid_cpu_types[0]);
+ g_assert(model != NULL);
if (!mc->valid_cpu_types[1]) {
- error_append_hint(errp, "The only valid type is: %s",
- mc->valid_cpu_types[0]);
+ error_append_hint(errp, "The only valid type is: %s", model);
} else {
- error_append_hint(errp, "The valid types are: %s",
- mc->valid_cpu_types[0]);
+ error_append_hint(errp, "The valid types are: %s", model);
}
+ g_free(model);
for (i = 1; mc->valid_cpu_types[i]; i++) {
- error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
+ model = cpu_model_from_type(mc->valid_cpu_types[i]);
+ g_assert(model != NULL);
+ error_append_hint(errp, ", %s", model);
+ g_free(model);
}
error_append_hint(errp, "\n");
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v7 4/8] hw/arm/virt: Hide host CPU model for tcg
2023-11-26 23:12 [PATCH v7 0/8] Unified CPU type check Gavin Shan
` (2 preceding siblings ...)
2023-11-26 23:12 ` [PATCH v7 3/8] machine: Print CPU model name instead of CPU type Gavin Shan
@ 2023-11-26 23:12 ` Gavin Shan
2023-11-28 9:29 ` Philippe Mathieu-Daudé
2023-11-26 23:12 ` [PATCH v7 5/8] hw/arm/virt: Check CPU type in machine_run_board_init() Gavin Shan
` (4 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Gavin Shan @ 2023-11-26 23:12 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, philmd,
wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
The 'host' CPU model isn't available until KVM or HVF is enabled.
For example, the following error messages are seen when the guest
is started with option '-cpu cortex-a8' on tcg after the next commit
is applied to check the CPU type in machine_run_board_init().
ERROR:../hw/core/machine.c:1423:is_cpu_type_supported: \
assertion failed: (model != NULL)
Bail out! ERROR:../hw/core/machine.c:1423:is_cpu_type_supported: \
assertion failed: (model != NULL)
Aborted (core dumped)
Hide 'host' CPU model until KVM or HVF is enabled. With this applied,
the valid CPU models can be shown.
qemu-system-aarch64: Invalid CPU type: cortex-a8
The valid types are: cortex-a7, cortex-a15, cortex-a35, \
cortex-a55, cortex-a72, cortex-a76, cortex-a710, a64fx, \
neoverse-n1, neoverse-v1, neoverse-n2, cortex-a53, \
cortex-a57, max
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/arm/virt.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index be2856c018..668c0d3194 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -220,7 +220,9 @@ static const char *valid_cpus[] = {
#endif
ARM_CPU_TYPE_NAME("cortex-a53"),
ARM_CPU_TYPE_NAME("cortex-a57"),
+#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
ARM_CPU_TYPE_NAME("host"),
+#endif
ARM_CPU_TYPE_NAME("max"),
};
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v7 5/8] hw/arm/virt: Check CPU type in machine_run_board_init()
2023-11-26 23:12 [PATCH v7 0/8] Unified CPU type check Gavin Shan
` (3 preceding siblings ...)
2023-11-26 23:12 ` [PATCH v7 4/8] hw/arm/virt: Hide host CPU model for tcg Gavin Shan
@ 2023-11-26 23:12 ` Gavin Shan
2023-11-27 10:13 ` Marcin Juszkiewicz
2023-11-26 23:12 ` [PATCH v7 6/8] hw/arm/sbsa-ref: " Gavin Shan
` (3 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Gavin Shan @ 2023-11-26 23:12 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, philmd,
wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
Set mc->valid_cpu_types so that the user specified CPU type can be
validated in machine_run_board_init(). We needn't to do the check
by ourselves.
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/arm/virt.c | 62 +++++++++++++++++++--------------------------------
1 file changed, 23 insertions(+), 39 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 668c0d3194..04f9f5fa56 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -204,40 +204,6 @@ static const int a15irqmap[] = {
[VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
};
-static const char *valid_cpus[] = {
-#ifdef CONFIG_TCG
- ARM_CPU_TYPE_NAME("cortex-a7"),
- ARM_CPU_TYPE_NAME("cortex-a15"),
- ARM_CPU_TYPE_NAME("cortex-a35"),
- ARM_CPU_TYPE_NAME("cortex-a55"),
- ARM_CPU_TYPE_NAME("cortex-a72"),
- ARM_CPU_TYPE_NAME("cortex-a76"),
- ARM_CPU_TYPE_NAME("cortex-a710"),
- ARM_CPU_TYPE_NAME("a64fx"),
- ARM_CPU_TYPE_NAME("neoverse-n1"),
- ARM_CPU_TYPE_NAME("neoverse-v1"),
- ARM_CPU_TYPE_NAME("neoverse-n2"),
-#endif
- ARM_CPU_TYPE_NAME("cortex-a53"),
- ARM_CPU_TYPE_NAME("cortex-a57"),
-#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
- ARM_CPU_TYPE_NAME("host"),
-#endif
- ARM_CPU_TYPE_NAME("max"),
-};
-
-static bool cpu_type_valid(const char *cpu)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(valid_cpus); i++) {
- if (strcmp(cpu, valid_cpus[i]) == 0) {
- return true;
- }
- }
- return false;
-}
-
static void create_randomness(MachineState *ms, const char *node)
{
struct {
@@ -2041,11 +2007,6 @@ static void machvirt_init(MachineState *machine)
unsigned int smp_cpus = machine->smp.cpus;
unsigned int max_cpus = machine->smp.max_cpus;
- if (!cpu_type_valid(machine->cpu_type)) {
- error_report("mach-virt: CPU type %s not supported", machine->cpu_type);
- exit(1);
- }
-
possible_cpus = mc->possible_cpu_arch_ids(machine);
/*
@@ -2939,6 +2900,28 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
+ static const char * const valid_cpu_types[] = {
+#ifdef CONFIG_TCG
+ ARM_CPU_TYPE_NAME("cortex-a7"),
+ ARM_CPU_TYPE_NAME("cortex-a15"),
+ ARM_CPU_TYPE_NAME("cortex-a35"),
+ ARM_CPU_TYPE_NAME("cortex-a55"),
+ ARM_CPU_TYPE_NAME("cortex-a72"),
+ ARM_CPU_TYPE_NAME("cortex-a76"),
+ ARM_CPU_TYPE_NAME("cortex-a710"),
+ ARM_CPU_TYPE_NAME("a64fx"),
+ ARM_CPU_TYPE_NAME("neoverse-n1"),
+ ARM_CPU_TYPE_NAME("neoverse-v1"),
+ ARM_CPU_TYPE_NAME("neoverse-n2"),
+#endif
+ ARM_CPU_TYPE_NAME("cortex-a53"),
+ ARM_CPU_TYPE_NAME("cortex-a57"),
+#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
+ ARM_CPU_TYPE_NAME("host"),
+#endif
+ ARM_CPU_TYPE_NAME("max"),
+ NULL
+ };
mc->init = machvirt_init;
/* Start with max_cpus set to 512, which is the maximum supported by KVM.
@@ -2965,6 +2948,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
#else
mc->default_cpu_type = ARM_CPU_TYPE_NAME("max");
#endif
+ mc->valid_cpu_types = valid_cpu_types;
mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
mc->kvm_type = virt_kvm_type;
assert(!mc->get_hotplug_handler);
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v7 6/8] hw/arm/sbsa-ref: Check CPU type in machine_run_board_init()
2023-11-26 23:12 [PATCH v7 0/8] Unified CPU type check Gavin Shan
` (4 preceding siblings ...)
2023-11-26 23:12 ` [PATCH v7 5/8] hw/arm/virt: Check CPU type in machine_run_board_init() Gavin Shan
@ 2023-11-26 23:12 ` Gavin Shan
2023-11-26 23:12 ` [PATCH v7 7/8] hw/arm: " Gavin Shan
` (2 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Gavin Shan @ 2023-11-26 23:12 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, philmd,
wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
Set mc->valid_cpu_types so that the user specified CPU type can
be validated in machine_run_board_init(). We needn't to do it
by ourselves.
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/arm/sbsa-ref.c | 36 ++++++++++--------------------------
1 file changed, 10 insertions(+), 26 deletions(-)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index f3c9704693..477dca0637 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -145,27 +145,6 @@ static const int sbsa_ref_irqmap[] = {
[SBSA_GWDT_WS0] = 16,
};
-static const char * const valid_cpus[] = {
- ARM_CPU_TYPE_NAME("cortex-a57"),
- ARM_CPU_TYPE_NAME("cortex-a72"),
- ARM_CPU_TYPE_NAME("neoverse-n1"),
- ARM_CPU_TYPE_NAME("neoverse-v1"),
- ARM_CPU_TYPE_NAME("neoverse-n2"),
- ARM_CPU_TYPE_NAME("max"),
-};
-
-static bool cpu_type_valid(const char *cpu)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(valid_cpus); i++) {
- if (strcmp(cpu, valid_cpus[i]) == 0) {
- return true;
- }
- }
- return false;
-}
-
static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
{
uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER;
@@ -733,11 +712,6 @@ static void sbsa_ref_init(MachineState *machine)
const CPUArchIdList *possible_cpus;
int n, sbsa_max_cpus;
- if (!cpu_type_valid(machine->cpu_type)) {
- error_report("sbsa-ref: CPU type %s not supported", machine->cpu_type);
- exit(1);
- }
-
if (kvm_enabled()) {
error_report("sbsa-ref: KVM is not supported for this machine");
exit(1);
@@ -898,10 +872,20 @@ static void sbsa_ref_instance_init(Object *obj)
static void sbsa_ref_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-a57"),
+ ARM_CPU_TYPE_NAME("cortex-a72"),
+ ARM_CPU_TYPE_NAME("neoverse-n1"),
+ ARM_CPU_TYPE_NAME("neoverse-v1"),
+ ARM_CPU_TYPE_NAME("neoverse-n2"),
+ ARM_CPU_TYPE_NAME("max"),
+ NULL,
+ };
mc->init = sbsa_ref_init;
mc->desc = "QEMU 'SBSA Reference' ARM Virtual Machine";
mc->default_cpu_type = ARM_CPU_TYPE_NAME("neoverse-n1");
+ mc->valid_cpu_types = valid_cpu_types;
mc->max_cpus = 512;
mc->pci_allow_0_address = true;
mc->minimum_page_bits = 12;
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v7 7/8] hw/arm: Check CPU type in machine_run_board_init()
2023-11-26 23:12 [PATCH v7 0/8] Unified CPU type check Gavin Shan
` (5 preceding siblings ...)
2023-11-26 23:12 ` [PATCH v7 6/8] hw/arm/sbsa-ref: " Gavin Shan
@ 2023-11-26 23:12 ` Gavin Shan
2023-11-28 9:31 ` Philippe Mathieu-Daudé
2023-11-26 23:12 ` [PATCH v7 8/8] hw/riscv/shakti_c: " Gavin Shan
2023-11-27 10:10 ` [PATCH v7 0/8] Unified CPU type check Marcin Juszkiewicz
8 siblings, 1 reply; 21+ messages in thread
From: Gavin Shan @ 2023-11-26 23:12 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, philmd,
wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
Set mc->valid_cpu_types so that the user specified CPU type can
be validated in machine_run_board_init(). We needn't to do it by
ourselves.
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/arm/bananapi_m2u.c | 12 ++++++------
hw/arm/cubieboard.c | 12 ++++++------
hw/arm/mps2-tz.c | 26 ++++++++++++++++++++------
hw/arm/mps2.c | 26 ++++++++++++++++++++------
hw/arm/msf2-som.c | 12 ++++++------
hw/arm/musca.c | 12 +++++-------
hw/arm/npcm7xx_boards.c | 12 +++++-------
hw/arm/orangepi.c | 12 ++++++------
8 files changed, 74 insertions(+), 50 deletions(-)
diff --git a/hw/arm/bananapi_m2u.c b/hw/arm/bananapi_m2u.c
index 8f24b18d8c..0a4b6f29b1 100644
--- a/hw/arm/bananapi_m2u.c
+++ b/hw/arm/bananapi_m2u.c
@@ -71,12 +71,6 @@ static void bpim2u_init(MachineState *machine)
exit(1);
}
- /* Only allow Cortex-A7 for this board */
- if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a7")) != 0) {
- error_report("This board can only be used with cortex-a7 CPU");
- exit(1);
- }
-
r40 = AW_R40(object_new(TYPE_AW_R40));
object_property_add_child(OBJECT(machine), "soc", OBJECT(r40));
object_unref(OBJECT(r40));
@@ -133,12 +127,18 @@ static void bpim2u_init(MachineState *machine)
static void bpim2u_machine_init(MachineClass *mc)
{
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-a7"),
+ NULL
+ };
+
mc->desc = "Bananapi M2U (Cortex-A7)";
mc->init = bpim2u_init;
mc->min_cpus = AW_R40_NUM_CPUS;
mc->max_cpus = AW_R40_NUM_CPUS;
mc->default_cpus = AW_R40_NUM_CPUS;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
+ mc->valid_cpu_types = valid_cpu_types;
mc->default_ram_size = 1 * GiB;
mc->default_ram_id = "bpim2u.ram";
}
diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index 29146f5018..b976727eef 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -52,12 +52,6 @@ static void cubieboard_init(MachineState *machine)
exit(1);
}
- /* Only allow Cortex-A8 for this board */
- if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a8")) != 0) {
- error_report("This board can only be used with cortex-a8 CPU");
- exit(1);
- }
-
a10 = AW_A10(object_new(TYPE_AW_A10));
object_property_add_child(OBJECT(machine), "soc", OBJECT(a10));
object_unref(OBJECT(a10));
@@ -114,8 +108,14 @@ static void cubieboard_init(MachineState *machine)
static void cubieboard_machine_init(MachineClass *mc)
{
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-a8"),
+ NULL
+ };
+
mc->desc = "cubietech cubieboard (Cortex-A8)";
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
+ mc->valid_cpu_types = valid_cpu_types;
mc->default_ram_size = 1 * GiB;
mc->init = cubieboard_init;
mc->block_default_type = IF_IDE;
diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index 668db5ed61..5d8cdc1a4c 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -813,12 +813,6 @@ static void mps2tz_common_init(MachineState *machine)
int num_ppcs;
int i;
- if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
- error_report("This board can only be used with CPU %s",
- mc->default_cpu_type);
- exit(1);
- }
-
if (machine->ram_size != mc->default_ram_size) {
char *sz = size_to_str(mc->default_ram_size);
error_report("Invalid RAM size, should be %s", sz);
@@ -1318,6 +1312,10 @@ static void mps2tz_an505_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_CLASS(oc);
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-m33"),
+ NULL
+ };
mc->desc = "ARM MPS2 with AN505 FPGA image for Cortex-M33";
mc->default_cpus = 1;
@@ -1325,6 +1323,7 @@ static void mps2tz_an505_class_init(ObjectClass *oc, void *data)
mc->max_cpus = mc->default_cpus;
mmc->fpga_type = FPGA_AN505;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m33");
+ mc->valid_cpu_types = valid_cpu_types;
mmc->scc_id = 0x41045050;
mmc->sysclk_frq = 20 * 1000 * 1000; /* 20MHz */
mmc->apb_periph_frq = mmc->sysclk_frq;
@@ -1347,6 +1346,10 @@ static void mps2tz_an521_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_CLASS(oc);
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-m33"),
+ NULL
+ };
mc->desc = "ARM MPS2 with AN521 FPGA image for dual Cortex-M33";
mc->default_cpus = 2;
@@ -1354,6 +1357,7 @@ static void mps2tz_an521_class_init(ObjectClass *oc, void *data)
mc->max_cpus = mc->default_cpus;
mmc->fpga_type = FPGA_AN521;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m33");
+ mc->valid_cpu_types = valid_cpu_types;
mmc->scc_id = 0x41045210;
mmc->sysclk_frq = 20 * 1000 * 1000; /* 20MHz */
mmc->apb_periph_frq = mmc->sysclk_frq;
@@ -1376,6 +1380,10 @@ static void mps3tz_an524_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_CLASS(oc);
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-m33"),
+ NULL
+ };
mc->desc = "ARM MPS3 with AN524 FPGA image for dual Cortex-M33";
mc->default_cpus = 2;
@@ -1383,6 +1391,7 @@ static void mps3tz_an524_class_init(ObjectClass *oc, void *data)
mc->max_cpus = mc->default_cpus;
mmc->fpga_type = FPGA_AN524;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m33");
+ mc->valid_cpu_types = valid_cpu_types;
mmc->scc_id = 0x41045240;
mmc->sysclk_frq = 32 * 1000 * 1000; /* 32MHz */
mmc->apb_periph_frq = mmc->sysclk_frq;
@@ -1410,6 +1419,10 @@ static void mps3tz_an547_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_CLASS(oc);
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-m55"),
+ NULL
+ };
mc->desc = "ARM MPS3 with AN547 FPGA image for Cortex-M55";
mc->default_cpus = 1;
@@ -1417,6 +1430,7 @@ static void mps3tz_an547_class_init(ObjectClass *oc, void *data)
mc->max_cpus = mc->default_cpus;
mmc->fpga_type = FPGA_AN547;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m55");
+ mc->valid_cpu_types = valid_cpu_types;
mmc->scc_id = 0x41055470;
mmc->sysclk_frq = 32 * 1000 * 1000; /* 32MHz */
mmc->apb_periph_frq = 25 * 1000 * 1000; /* 25MHz */
diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index 292a180ad2..bd873cc5de 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -142,12 +142,6 @@ static void mps2_common_init(MachineState *machine)
QList *oscclk;
int i;
- if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
- error_report("This board can only be used with CPU %s",
- mc->default_cpu_type);
- exit(1);
- }
-
if (machine->ram_size != mc->default_ram_size) {
char *sz = size_to_str(mc->default_ram_size);
error_report("Invalid RAM size, should be %s", sz);
@@ -484,10 +478,15 @@ static void mps2_an385_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
MPS2MachineClass *mmc = MPS2_MACHINE_CLASS(oc);
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-m3"),
+ NULL
+ };
mc->desc = "ARM MPS2 with AN385 FPGA image for Cortex-M3";
mmc->fpga_type = FPGA_AN385;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m3");
+ mc->valid_cpu_types = valid_cpu_types;
mmc->scc_id = 0x41043850;
mmc->psram_base = 0x21000000;
mmc->ethernet_base = 0x40200000;
@@ -498,10 +497,15 @@ static void mps2_an386_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
MPS2MachineClass *mmc = MPS2_MACHINE_CLASS(oc);
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-m4"),
+ NULL
+ };
mc->desc = "ARM MPS2 with AN386 FPGA image for Cortex-M4";
mmc->fpga_type = FPGA_AN386;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
+ mc->valid_cpu_types = valid_cpu_types;
mmc->scc_id = 0x41043860;
mmc->psram_base = 0x21000000;
mmc->ethernet_base = 0x40200000;
@@ -512,10 +516,15 @@ static void mps2_an500_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
MPS2MachineClass *mmc = MPS2_MACHINE_CLASS(oc);
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-m7"),
+ NULL
+ };
mc->desc = "ARM MPS2 with AN500 FPGA image for Cortex-M7";
mmc->fpga_type = FPGA_AN500;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m7");
+ mc->valid_cpu_types = valid_cpu_types;
mmc->scc_id = 0x41045000;
mmc->psram_base = 0x60000000;
mmc->ethernet_base = 0xa0000000;
@@ -526,10 +535,15 @@ static void mps2_an511_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
MPS2MachineClass *mmc = MPS2_MACHINE_CLASS(oc);
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-m3"),
+ NULL
+ };
mc->desc = "ARM MPS2 with AN511 DesignStart FPGA image for Cortex-M3";
mmc->fpga_type = FPGA_AN511;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m3");
+ mc->valid_cpu_types = valid_cpu_types;
mmc->scc_id = 0x41045110;
mmc->psram_base = 0x21000000;
mmc->ethernet_base = 0x40200000;
diff --git a/hw/arm/msf2-som.c b/hw/arm/msf2-som.c
index 7b3106c790..eb74b23797 100644
--- a/hw/arm/msf2-som.c
+++ b/hw/arm/msf2-som.c
@@ -55,12 +55,6 @@ static void emcraft_sf2_s2s010_init(MachineState *machine)
MemoryRegion *ddr = g_new(MemoryRegion, 1);
Clock *m3clk;
- if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
- error_report("This board can only be used with CPU %s",
- mc->default_cpu_type);
- exit(1);
- }
-
memory_region_init_ram(ddr, NULL, "ddr-ram", DDR_SIZE,
&error_fatal);
memory_region_add_subregion(sysmem, DDR_BASE_ADDRESS, ddr);
@@ -106,9 +100,15 @@ static void emcraft_sf2_s2s010_init(MachineState *machine)
static void emcraft_sf2_machine_init(MachineClass *mc)
{
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-m3"),
+ NULL
+ };
+
mc->desc = "SmartFusion2 SOM kit from Emcraft (M2S010)";
mc->init = emcraft_sf2_s2s010_init;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m3");
+ mc->valid_cpu_types = valid_cpu_types;
}
DEFINE_MACHINE("emcraft-sf2", emcraft_sf2_machine_init)
diff --git a/hw/arm/musca.c b/hw/arm/musca.c
index 6eeee57c9d..770ec1a15c 100644
--- a/hw/arm/musca.c
+++ b/hw/arm/musca.c
@@ -355,7 +355,6 @@ static void musca_init(MachineState *machine)
{
MuscaMachineState *mms = MUSCA_MACHINE(machine);
MuscaMachineClass *mmc = MUSCA_MACHINE_GET_CLASS(mms);
- MachineClass *mc = MACHINE_GET_CLASS(machine);
MemoryRegion *system_memory = get_system_memory();
DeviceState *ssedev;
DeviceState *dev_splitter;
@@ -366,12 +365,6 @@ static void musca_init(MachineState *machine)
assert(mmc->num_irqs <= MUSCA_NUMIRQ_MAX);
assert(mmc->num_mpcs <= MUSCA_MPC_MAX);
- if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
- error_report("This board can only be used with CPU %s",
- mc->default_cpu_type);
- exit(1);
- }
-
mms->sysclk = clock_new(OBJECT(machine), "SYSCLK");
clock_set_hz(mms->sysclk, SYSCLK_FRQ);
mms->s32kclk = clock_new(OBJECT(machine), "S32KCLK");
@@ -604,11 +597,16 @@ static void musca_init(MachineState *machine)
static void musca_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-m33"),
+ NULL
+ };
mc->default_cpus = 2;
mc->min_cpus = mc->default_cpus;
mc->max_cpus = mc->default_cpus;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m33");
+ mc->valid_cpu_types = valid_cpu_types;
mc->init = musca_init;
}
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index 2aef579aac..2999b8b96d 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -121,15 +121,8 @@ static NPCM7xxState *npcm7xx_create_soc(MachineState *machine,
uint32_t hw_straps)
{
NPCM7xxMachineClass *nmc = NPCM7XX_MACHINE_GET_CLASS(machine);
- MachineClass *mc = MACHINE_CLASS(nmc);
Object *obj;
- if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
- error_report("This board can only be used with %s",
- mc->default_cpu_type);
- exit(1);
- }
-
obj = object_new_with_props(nmc->soc_type, OBJECT(machine), "soc",
&error_abort, NULL);
object_property_set_uint(obj, "power-on-straps", hw_straps, &error_abort);
@@ -463,12 +456,17 @@ static void npcm7xx_set_soc_type(NPCM7xxMachineClass *nmc, const char *type)
static void npcm7xx_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-a9"),
+ NULL
+ };
mc->no_floppy = 1;
mc->no_cdrom = 1;
mc->no_parallel = 1;
mc->default_ram_id = "ram";
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
+ mc->valid_cpu_types = valid_cpu_types;
}
/*
diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
index f3784d45ca..77e328191d 100644
--- a/hw/arm/orangepi.c
+++ b/hw/arm/orangepi.c
@@ -49,12 +49,6 @@ static void orangepi_init(MachineState *machine)
exit(1);
}
- /* Only allow Cortex-A7 for this board */
- if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a7")) != 0) {
- error_report("This board can only be used with cortex-a7 CPU");
- exit(1);
- }
-
h3 = AW_H3(object_new(TYPE_AW_H3));
object_property_add_child(OBJECT(machine), "soc", OBJECT(h3));
object_unref(OBJECT(h3));
@@ -111,6 +105,11 @@ static void orangepi_init(MachineState *machine)
static void orangepi_machine_init(MachineClass *mc)
{
+ static const char * const valid_cpu_types[] = {
+ ARM_CPU_TYPE_NAME("cortex-a7"),
+ NULL
+ };
+
mc->desc = "Orange Pi PC (Cortex-A7)";
mc->init = orangepi_init;
mc->block_default_type = IF_SD;
@@ -119,6 +118,7 @@ static void orangepi_machine_init(MachineClass *mc)
mc->max_cpus = AW_H3_NUM_CPUS;
mc->default_cpus = AW_H3_NUM_CPUS;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
+ mc->valid_cpu_types = valid_cpu_types;
mc->default_ram_size = 1 * GiB;
mc->default_ram_id = "orangepi.ram";
}
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v7 8/8] hw/riscv/shakti_c: Check CPU type in machine_run_board_init()
2023-11-26 23:12 [PATCH v7 0/8] Unified CPU type check Gavin Shan
` (6 preceding siblings ...)
2023-11-26 23:12 ` [PATCH v7 7/8] hw/arm: " Gavin Shan
@ 2023-11-26 23:12 ` Gavin Shan
2023-11-27 10:10 ` [PATCH v7 0/8] Unified CPU type check Marcin Juszkiewicz
8 siblings, 0 replies; 21+ messages in thread
From: Gavin Shan @ 2023-11-26 23:12 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, philmd,
wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
Set mc->valid_cpu_types so that the user specified CPU type can
be validated in machine_run_board_init(). We needn't to do it
by ourselves.
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/riscv/shakti_c.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
index 12ea74b032..3888034c2b 100644
--- a/hw/riscv/shakti_c.c
+++ b/hw/riscv/shakti_c.c
@@ -28,7 +28,6 @@
#include "exec/address-spaces.h"
#include "hw/riscv/boot.h"
-
static const struct MemmapEntry {
hwaddr base;
hwaddr size;
@@ -47,12 +46,6 @@ static void shakti_c_machine_state_init(MachineState *mstate)
ShaktiCMachineState *sms = RISCV_SHAKTI_MACHINE(mstate);
MemoryRegion *system_memory = get_system_memory();
- /* Allow only Shakti C CPU for this platform */
- if (strcmp(mstate->cpu_type, TYPE_RISCV_CPU_SHAKTI_C) != 0) {
- error_report("This board can only be used with Shakti C CPU");
- exit(1);
- }
-
/* Initialize SoC */
object_initialize_child(OBJECT(mstate), "soc", &sms->soc,
TYPE_RISCV_SHAKTI_SOC);
@@ -82,9 +75,15 @@ static void shakti_c_machine_instance_init(Object *obj)
static void shakti_c_machine_class_init(ObjectClass *klass, void *data)
{
MachineClass *mc = MACHINE_CLASS(klass);
+ static const char * const valid_cpu_types[] = {
+ RISCV_CPU_TYPE_NAME("shakti-c"),
+ NULL
+ };
+
mc->desc = "RISC-V Board compatible with Shakti SDK";
mc->init = shakti_c_machine_state_init;
mc->default_cpu_type = TYPE_RISCV_CPU_SHAKTI_C;
+ mc->valid_cpu_types = valid_cpu_types;
mc->default_ram_id = "riscv.shakti.c.ram";
}
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v7 0/8] Unified CPU type check
2023-11-26 23:12 [PATCH v7 0/8] Unified CPU type check Gavin Shan
` (7 preceding siblings ...)
2023-11-26 23:12 ` [PATCH v7 8/8] hw/riscv/shakti_c: " Gavin Shan
@ 2023-11-27 10:10 ` Marcin Juszkiewicz
2023-11-27 10:15 ` Gavin Shan
8 siblings, 1 reply; 21+ messages in thread
From: Marcin Juszkiewicz @ 2023-11-27 10:10 UTC (permalink / raw)
To: Gavin Shan, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
eduardo, marcel.apfelbaum, philmd, wangyanan55, vijai, palmer,
alistair.francis, bin.meng, liwei1518, dbarboza, zhiwei_liu,
shan.gavin
W dniu 27.11.2023 o 00:12, Gavin Shan pisze:
> After the series is applied:
>
> [gshan@gshan q]$ ./build/qemu-system-aarch64 -M virt -cpu cortex-a8
> qemu-system-aarch64: Invalid CPU type: cortex-a8
> The valid types are: cortex-a7, cortex-a15, cortex-a35, cortex-a55, \
> cortex-a72, cortex-a76, cortex-a710, a64fx, \
> neoverse-n1, neoverse-v1, neoverse-n2, cortex-a53, \
> cortex-a57, max
Can this list have some better order? A35, A53, A55, A57, A72 feels
better than current one.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v7 5/8] hw/arm/virt: Check CPU type in machine_run_board_init()
2023-11-26 23:12 ` [PATCH v7 5/8] hw/arm/virt: Check CPU type in machine_run_board_init() Gavin Shan
@ 2023-11-27 10:13 ` Marcin Juszkiewicz
2023-11-27 10:42 ` Gavin Shan
0 siblings, 1 reply; 21+ messages in thread
From: Marcin Juszkiewicz @ 2023-11-27 10:13 UTC (permalink / raw)
To: Gavin Shan, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
eduardo, marcel.apfelbaum, philmd, wangyanan55, vijai, palmer,
alistair.francis, bin.meng, liwei1518, dbarboza, zhiwei_liu,
shan.gavin
W dniu 27.11.2023 o 00:12, Gavin Shan pisze:
> @@ -2939,6 +2900,28 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
> + static const char * const valid_cpu_types[] = {
> +#ifdef CONFIG_TCG
> + ARM_CPU_TYPE_NAME("cortex-a7"),
> + ARM_CPU_TYPE_NAME("cortex-a15"),
> + ARM_CPU_TYPE_NAME("cortex-a35"),
> + ARM_CPU_TYPE_NAME("cortex-a55"),
> + ARM_CPU_TYPE_NAME("cortex-a72"),
> + ARM_CPU_TYPE_NAME("cortex-a76"),
> + ARM_CPU_TYPE_NAME("cortex-a710"),
> + ARM_CPU_TYPE_NAME("a64fx"),
> + ARM_CPU_TYPE_NAME("neoverse-n1"),
> + ARM_CPU_TYPE_NAME("neoverse-v1"),
> + ARM_CPU_TYPE_NAME("neoverse-n2"),
> +#endif
> + ARM_CPU_TYPE_NAME("cortex-a53"),
> + ARM_CPU_TYPE_NAME("cortex-a57"),
> +#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
> + ARM_CPU_TYPE_NAME("host"),
> +#endif
> + ARM_CPU_TYPE_NAME("max"),
> + NULL
> + };
I understand that you just move list from one place to the other but
also wonder why a53/a57 were/are outside of 'ifdef CONFIG_TCG' check.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v7 0/8] Unified CPU type check
2023-11-27 10:10 ` [PATCH v7 0/8] Unified CPU type check Marcin Juszkiewicz
@ 2023-11-27 10:15 ` Gavin Shan
0 siblings, 0 replies; 21+ messages in thread
From: Gavin Shan @ 2023-11-27 10:15 UTC (permalink / raw)
To: Marcin Juszkiewicz, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
eduardo, marcel.apfelbaum, philmd, wangyanan55, vijai, palmer,
alistair.francis, bin.meng, liwei1518, dbarboza, zhiwei_liu,
shan.gavin
On 11/27/23 21:10, Marcin Juszkiewicz wrote:
> W dniu 27.11.2023 o 00:12, Gavin Shan pisze:
>> After the series is applied:
>>
>> [gshan@gshan q]$ ./build/qemu-system-aarch64 -M virt -cpu cortex-a8
>> qemu-system-aarch64: Invalid CPU type: cortex-a8
>> The valid types are: cortex-a7, cortex-a15, cortex-a35, cortex-a55, \
>> cortex-a72, cortex-a76, cortex-a710, a64fx, \
>> neoverse-n1, neoverse-v1, neoverse-n2, cortex-a53, \
>> cortex-a57, max
>
> Can this list have some better order? A35, A53, A55, A57, A72 feels better than current one.
>
Definitely a good idea. I think this series is about to be queued,
let me send one additional patch to do so after this series is
merged.
Thanks,
Gavin
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v7 5/8] hw/arm/virt: Check CPU type in machine_run_board_init()
2023-11-27 10:13 ` Marcin Juszkiewicz
@ 2023-11-27 10:42 ` Gavin Shan
0 siblings, 0 replies; 21+ messages in thread
From: Gavin Shan @ 2023-11-27 10:42 UTC (permalink / raw)
To: Marcin Juszkiewicz, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
eduardo, marcel.apfelbaum, philmd, wangyanan55, vijai, palmer,
alistair.francis, bin.meng, liwei1518, dbarboza, zhiwei_liu,
shan.gavin
On 11/27/23 21:13, Marcin Juszkiewicz wrote:
> W dniu 27.11.2023 o 00:12, Gavin Shan pisze:
>> @@ -2939,6 +2900,28 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>> {
>> MachineClass *mc = MACHINE_CLASS(oc);
>> HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>> + static const char * const valid_cpu_types[] = {
>> +#ifdef CONFIG_TCG
>> + ARM_CPU_TYPE_NAME("cortex-a7"),
>> + ARM_CPU_TYPE_NAME("cortex-a15"),
>> + ARM_CPU_TYPE_NAME("cortex-a35"),
>> + ARM_CPU_TYPE_NAME("cortex-a55"),
>> + ARM_CPU_TYPE_NAME("cortex-a72"),
>> + ARM_CPU_TYPE_NAME("cortex-a76"),
>> + ARM_CPU_TYPE_NAME("cortex-a710"),
>> + ARM_CPU_TYPE_NAME("a64fx"),
>> + ARM_CPU_TYPE_NAME("neoverse-n1"),
>> + ARM_CPU_TYPE_NAME("neoverse-v1"),
>> + ARM_CPU_TYPE_NAME("neoverse-n2"),
>> +#endif
>> + ARM_CPU_TYPE_NAME("cortex-a53"),
>> + ARM_CPU_TYPE_NAME("cortex-a57"),
>> +#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
>> + ARM_CPU_TYPE_NAME("host"),
>> +#endif
>> + ARM_CPU_TYPE_NAME("max"),
>> + NULL
>> + };
>
> I understand that you just move list from one place to the other but also wonder why a53/a57 were/are outside of 'ifdef CONFIG_TCG' check.
>
I'm not sure about HVF, but a53/a57 can be supported by KVM. The supported list of
CPUs by KVM is defined in linux/arch/arm64/include/uapi/asm/kvm.h as below
/*
* Supported CPU Targets - Adding a new target type is not recommended,
* unless there are some special registers not supported by the
* genericv8 syreg table.
*/
#define KVM_ARM_TARGET_AEM_V8 0
#define KVM_ARM_TARGET_FOUNDATION_V8 1
#define KVM_ARM_TARGET_CORTEX_A57 2
#define KVM_ARM_TARGET_XGENE_POTENZA 3
#define KVM_ARM_TARGET_CORTEX_A53 4
/* Generic ARM v8 target */
#define KVM_ARM_TARGET_GENERIC_V8 5
#define KVM_ARM_NUM_TARGETS 6
And the following QEMU commit gives more hints about it.
[gshan@gshan q]$ git show 39920a04952
commit 39920a04952b67fb1fce8fc3519ac18b7a95f3f3
Author: Fabiano Rosas <farosas@suse.de>
Date: Wed Apr 26 15:00:05 2023 -0300
target/arm: Move 64-bit TCG CPUs into tcg/
Move the 64-bit CPUs that are TCG-only:
- cortex-a35
- cortex-a55
- cortex-a72
- cortex-a76
- a64fx
- neoverse-n1
Keep the CPUs that can be used with KVM:
- cortex-a57
- cortex-a53
- max
- host
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20230426180013.14814-6-farosas@suse.de
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Thanks,
Gavin
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v7 4/8] hw/arm/virt: Hide host CPU model for tcg
2023-11-26 23:12 ` [PATCH v7 4/8] hw/arm/virt: Hide host CPU model for tcg Gavin Shan
@ 2023-11-28 9:29 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-28 9:29 UTC (permalink / raw)
To: Gavin Shan, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, wangyanan55,
vijai, palmer, alistair.francis, bin.meng, liwei1518, dbarboza,
zhiwei_liu, shan.gavin
On 27/11/23 00:12, Gavin Shan wrote:
> The 'host' CPU model isn't available until KVM or HVF is enabled.
> For example, the following error messages are seen when the guest
> is started with option '-cpu cortex-a8' on tcg after the next commit
> is applied to check the CPU type in machine_run_board_init().
>
> ERROR:../hw/core/machine.c:1423:is_cpu_type_supported: \
> assertion failed: (model != NULL)
> Bail out! ERROR:../hw/core/machine.c:1423:is_cpu_type_supported: \
> assertion failed: (model != NULL)
> Aborted (core dumped)
>
> Hide 'host' CPU model until KVM or HVF is enabled. With this applied,
> the valid CPU models can be shown.
>
> qemu-system-aarch64: Invalid CPU type: cortex-a8
> The valid types are: cortex-a7, cortex-a15, cortex-a35, \
> cortex-a55, cortex-a72, cortex-a76, cortex-a710, a64fx, \
> neoverse-n1, neoverse-v1, neoverse-n2, cortex-a53, \
> cortex-a57, max
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> hw/arm/virt.c | 2 ++
> 1 file changed, 2 insertions(+)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v7 7/8] hw/arm: Check CPU type in machine_run_board_init()
2023-11-26 23:12 ` [PATCH v7 7/8] hw/arm: " Gavin Shan
@ 2023-11-28 9:31 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-28 9:31 UTC (permalink / raw)
To: Gavin Shan, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, wangyanan55,
vijai, palmer, alistair.francis, bin.meng, liwei1518, dbarboza,
zhiwei_liu, shan.gavin
On 27/11/23 00:12, Gavin Shan wrote:
> Set mc->valid_cpu_types so that the user specified CPU type can
> be validated in machine_run_board_init(). We needn't to do it by
> ourselves.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> hw/arm/bananapi_m2u.c | 12 ++++++------
> hw/arm/cubieboard.c | 12 ++++++------
> hw/arm/mps2-tz.c | 26 ++++++++++++++++++++------
> hw/arm/mps2.c | 26 ++++++++++++++++++++------
> hw/arm/msf2-som.c | 12 ++++++------
> hw/arm/musca.c | 12 +++++-------
> hw/arm/npcm7xx_boards.c | 12 +++++-------
> hw/arm/orangepi.c | 12 ++++++------
> 8 files changed, 74 insertions(+), 50 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v7 2/8] machine: Introduce helper is_cpu_type_supported()
2023-11-26 23:12 ` [PATCH v7 2/8] machine: Introduce helper is_cpu_type_supported() Gavin Shan
@ 2023-11-28 9:38 ` Philippe Mathieu-Daudé
2023-11-28 22:55 ` Gavin Shan
0 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-28 9:38 UTC (permalink / raw)
To: Gavin Shan, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, wangyanan55,
vijai, palmer, alistair.francis, bin.meng, liwei1518, dbarboza,
zhiwei_liu, shan.gavin
Hi Gavin,
On 27/11/23 00:12, Gavin Shan wrote:
> The logic, to check if the specified CPU type is supported in
> machine_run_board_init(), is independent enough. Factor it out into
> helper is_cpu_type_supported(). machine_run_board_init() looks a bit
> clean with this. Since we're here, @machine_class is renamed to @mc
> to avoid multiple line spanning of code. The error messages and comments
> are tweaked a bit either.
>
> No functional change intended.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> hw/core/machine.c | 90 +++++++++++++++++++++++++++--------------------
> 1 file changed, 51 insertions(+), 39 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index b3ef325936..05e1922b89 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1387,13 +1387,57 @@ out:
> return r;
> }
>
> +static void is_cpu_type_supported(const MachineState *machine, Error **errp)
Functions taking an Error** last argument should return a boolean value.
> +{
> + MachineClass *mc = MACHINE_GET_CLASS(machine);
> + ObjectClass *oc = object_class_by_name(machine->cpu_type);
> + CPUClass *cc;
> + int i;
> +
> + /*
> + * Check if the user specified CPU type is supported when the valid
> + * CPU types have been determined. Note that the user specified CPU
> + * type is provided through '-cpu' option.
> + */
> + if (mc->valid_cpu_types && machine->cpu_type) {
> + for (i = 0; mc->valid_cpu_types[i]; i++) {
> + if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
> + break;
> + }
> + }
> +
> + /* The user specified CPU type isn't valid */
> + if (!mc->valid_cpu_types[i]) {
> + error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
> + if (!mc->valid_cpu_types[1]) {
> + error_append_hint(errp, "The only valid type is: %s",
> + mc->valid_cpu_types[0]);
> + } else {
> + error_append_hint(errp, "The valid types are: %s",
> + mc->valid_cpu_types[0]);
> + }
> +
> + for (i = 1; mc->valid_cpu_types[i]; i++) {
> + error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
> + }
> +
> + error_append_hint(errp, "\n");
> + return;
> + }
> + }
> +
> + /* Check if CPU type is deprecated and warn if so */
> + cc = CPU_CLASS(oc);
> + if (cc && cc->deprecation_note) {
> + warn_report("CPU model %s is deprecated -- %s",
> + machine->cpu_type, cc->deprecation_note);
Why did you move the deprecation warning within the is_supported check?
> + }
> +}
>
> void machine_run_board_init(MachineState *machine, const char *mem_path, Error **errp)
> {
> ERRP_GUARD();
> MachineClass *machine_class = MACHINE_GET_CLASS(machine);
> - ObjectClass *oc = object_class_by_name(machine->cpu_type);
> - CPUClass *cc;
> Error *local_err = NULL;
>
> /* This checkpoint is required by replay to separate prior clock
> @@ -1449,43 +1493,11 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
> machine->ram = machine_consume_memdev(machine, machine->memdev);
> }
>
> - /* 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) {
> - int i;
> -
> - for (i = 0; machine_class->valid_cpu_types[i]; i++) {
> - if (object_class_dynamic_cast(oc,
> - machine_class->valid_cpu_types[i])) {
> - /* The user specified CPU is in the valid field, we are
> - * good to go.
> - */
> - break;
> - }
> - }
> -
> - if (!machine_class->valid_cpu_types[i]) {
> - /* The user specified CPU is not valid */
> - error_setg(&local_err, "Invalid CPU type: %s", machine->cpu_type);
> - error_append_hint(&local_err, "The valid types are: %s",
> - machine_class->valid_cpu_types[0]);
> - for (i = 1; machine_class->valid_cpu_types[i]; i++) {
> - error_append_hint(&local_err, ", %s",
> - machine_class->valid_cpu_types[i]);
> - }
> - error_append_hint(&local_err, "\n");
> -
> - error_propagate(errp, local_err);
> - return;
> - }
> - }
> -
> - /* Check if CPU type is deprecated and warn if so */
> - cc = CPU_CLASS(oc);
> - if (cc && cc->deprecation_note) {
> - warn_report("CPU model %s is deprecated -- %s", machine->cpu_type,
> - cc->deprecation_note);
> + /* Check if the CPU type is supported */
> + is_cpu_type_supported(machine, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
This becomes:
if (!is_cpu_type_supported(machine, errp)) {
> + return;
> }
>
> if (machine->cgs) {
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v7 1/8] machine: Use error handling when CPU type is checked
2023-11-26 23:12 ` [PATCH v7 1/8] machine: Use error handling when CPU type is checked Gavin Shan
@ 2023-11-28 9:43 ` Philippe Mathieu-Daudé
2023-11-28 22:47 ` Gavin Shan
0 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-28 9:43 UTC (permalink / raw)
To: Gavin Shan, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, wangyanan55,
vijai, palmer, alistair.francis, bin.meng, liwei1518, dbarboza,
zhiwei_liu, shan.gavin
Hi Gavin,
On 27/11/23 00:12, Gavin Shan wrote:
> QEMU will be terminated if the specified CPU type isn't supported
> in machine_run_board_init(). The list of supported CPU type names
> is tracked by mc->valid_cpu_types.
>
> The error handling can be used to propagate error messages, to be
> consistent how the errors are handled for other situations in the
> same function.
>
> No functional change intended.
>
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> v7: Add 'return' after error_propagate() to avoid calling into
> mc->init() in the failing case (Marcin)
> ---
> hw/core/machine.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 0c17398141..b3ef325936 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1394,6 +1394,7 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
> MachineClass *machine_class = MACHINE_GET_CLASS(machine);
> ObjectClass *oc = object_class_by_name(machine->cpu_type);
> CPUClass *cc;
> + Error *local_err = NULL;
IIUC the big comment in "include/qapi/error.h", since commit ae7c80a7bd
("error: New macro ERRP_GUARD()") we want to use the new macro instead.
> /* This checkpoint is required by replay to separate prior clock
> reading from the other reads, because timer polling functions query
> @@ -1466,15 +1467,17 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
>
> if (!machine_class->valid_cpu_types[i]) {
> /* 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]);
> + error_setg(&local_err, "Invalid CPU type: %s", machine->cpu_type);
> + error_append_hint(&local_err, "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_append_hint(&local_err, ", %s",
> + machine_class->valid_cpu_types[i]);
> }
> - error_printf("\n");
> + error_append_hint(&local_err, "\n");
>
> - exit(1);
> + error_propagate(errp, local_err);
> + return;
> }
> }
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v7 3/8] machine: Print CPU model name instead of CPU type
2023-11-26 23:12 ` [PATCH v7 3/8] machine: Print CPU model name instead of CPU type Gavin Shan
@ 2023-11-28 9:55 ` Philippe Mathieu-Daudé
2023-11-29 3:53 ` Gavin Shan
0 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-28 9:55 UTC (permalink / raw)
To: Gavin Shan, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, wangyanan55,
vijai, palmer, alistair.francis, bin.meng, liwei1518, dbarboza,
zhiwei_liu, shan.gavin
Hi Gavin,
On 27/11/23 00:12, Gavin Shan wrote:
> The names of supported CPU models instead of CPU types should be
> printed when the user specified CPU type isn't supported, to be
> consistent with the output from '-cpu ?'.
>
> Correct the error messages to print CPU model names instead of CPU
> type names.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> hw/core/machine.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 05e1922b89..898c25552a 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1392,6 +1392,7 @@ static void is_cpu_type_supported(const MachineState *machine, Error **errp)
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> ObjectClass *oc = object_class_by_name(machine->cpu_type);
> CPUClass *cc;
> + char *model;
> int i;
>
> /*
> @@ -1408,17 +1409,25 @@ static void is_cpu_type_supported(const MachineState *machine, Error **errp)
>
> /* The user specified CPU type isn't valid */
> if (!mc->valid_cpu_types[i]) {
> - error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
> + model = cpu_model_from_type(machine->cpu_type);
> + g_assert(model != NULL);
> + error_setg(errp, "Invalid CPU type: %s", model);
> + g_free(model);
g_autofree char *requested = cpu_model_from_type(machine->cpu_type);
error_setg(errp, "Invalid CPU type: %s", requested);
> +
> + model = cpu_model_from_type(mc->valid_cpu_types[0]);
> + g_assert(model != NULL);
> if (!mc->valid_cpu_types[1]) {
> - error_append_hint(errp, "The only valid type is: %s",
> - mc->valid_cpu_types[0]);
> + error_append_hint(errp, "The only valid type is: %s", model);
g_autofree char *model = cpu_model_from_type(mc->valid_cpu_types[0]);
error_append_hint(errp, "The only valid type is: %s\n", model);
> } else {
> - error_append_hint(errp, "The valid types are: %s",
> - mc->valid_cpu_types[0]);
> + error_append_hint(errp, "The valid types are: %s", model);
Please move all the enumeration in this ladder, this makes the logic
simpler to follow:
error_append_hint(errp, "The valid types are: ");
for (i = 0; mc->valid_cpu_types[i]; i++) {
g_autofree char *model =
cpu_model_from_type(mc->valid_cpu_types[i]);
error_append_hint(errp, ", %s", model);
}
error_append_hint(errp, "\n");
> }
> + g_free(model);
>
> for (i = 1; mc->valid_cpu_types[i]; i++) {
> - error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
> + model = cpu_model_from_type(mc->valid_cpu_types[i]);
> + g_assert(model != NULL);
> + error_append_hint(errp, ", %s", model);
> + g_free(model);
> }
>
> error_append_hint(errp, "\n");
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v7 1/8] machine: Use error handling when CPU type is checked
2023-11-28 9:43 ` Philippe Mathieu-Daudé
@ 2023-11-28 22:47 ` Gavin Shan
0 siblings, 0 replies; 21+ messages in thread
From: Gavin Shan @ 2023-11-28 22:47 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, wangyanan55,
vijai, palmer, alistair.francis, bin.meng, liwei1518, dbarboza,
zhiwei_liu, shan.gavin
Hi Phil,
On 11/28/23 20:43, Philippe Mathieu-Daudé wrote:
> On 27/11/23 00:12, Gavin Shan wrote:
>> QEMU will be terminated if the specified CPU type isn't supported
>> in machine_run_board_init(). The list of supported CPU type names
>> is tracked by mc->valid_cpu_types.
>>
>> The error handling can be used to propagate error messages, to be
>> consistent how the errors are handled for other situations in the
>> same function.
>>
>> No functional change intended.
>>
>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>> v7: Add 'return' after error_propagate() to avoid calling into
>> mc->init() in the failing case (Marcin)
>> ---
>> hw/core/machine.c | 15 +++++++++------
>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index 0c17398141..b3ef325936 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -1394,6 +1394,7 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
>> MachineClass *machine_class = MACHINE_GET_CLASS(machine);
>> ObjectClass *oc = object_class_by_name(machine->cpu_type);
>> CPUClass *cc;
>> + Error *local_err = NULL;
>
> IIUC the big comment in "include/qapi/error.h", since commit ae7c80a7bd
> ("error: New macro ERRP_GUARD()") we want to use the new macro instead.
>
Yeah, there already has a ERRP_GUARD() in machine_run_board_init(). After
rechecking the implementation of ERRP_GUARD(), I found @local_err needs to
be dropped and use @errp below.
>> /* This checkpoint is required by replay to separate prior clock
>> reading from the other reads, because timer polling functions query
>> @@ -1466,15 +1467,17 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
>> if (!machine_class->valid_cpu_types[i]) {
>> /* 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]);
>> + error_setg(&local_err, "Invalid CPU type: %s", machine->cpu_type);
>> + error_append_hint(&local_err, "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_append_hint(&local_err, ", %s",
>> + machine_class->valid_cpu_types[i]);
>> }
>> - error_printf("\n");
>> + error_append_hint(&local_err, "\n");
>> - exit(1);
>> + error_propagate(errp, local_err);
>> + return;
>> }
>> }
@local_err needs to be replaced with @errp in this chunk of code, as mentioned
above.
Thanks,
Gavin
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v7 2/8] machine: Introduce helper is_cpu_type_supported()
2023-11-28 9:38 ` Philippe Mathieu-Daudé
@ 2023-11-28 22:55 ` Gavin Shan
0 siblings, 0 replies; 21+ messages in thread
From: Gavin Shan @ 2023-11-28 22:55 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, wangyanan55,
vijai, palmer, alistair.francis, bin.meng, liwei1518, dbarboza,
zhiwei_liu, shan.gavin
Hi Phil,
On 11/28/23 20:38, Philippe Mathieu-Daudé wrote:
> On 27/11/23 00:12, Gavin Shan wrote:
>> The logic, to check if the specified CPU type is supported in
>> machine_run_board_init(), is independent enough. Factor it out into
>> helper is_cpu_type_supported(). machine_run_board_init() looks a bit
>> clean with this. Since we're here, @machine_class is renamed to @mc
>> to avoid multiple line spanning of code. The error messages and comments
>> are tweaked a bit either.
>>
>> No functional change intended.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>> hw/core/machine.c | 90 +++++++++++++++++++++++++++--------------------
>> 1 file changed, 51 insertions(+), 39 deletions(-)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index b3ef325936..05e1922b89 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -1387,13 +1387,57 @@ out:
>> return r;
>> }
>> +static void is_cpu_type_supported(const MachineState *machine, Error **errp)
>
> Functions taking an Error** last argument should return a boolean value.
>
Correct, especially @errp instead of @local_err will be passed from
machine_run_board_init() to is_cpu_type_supported(). We needs an
indicator for machine_run_board_init() to bail immediately to avoid
calling mc->init() there in the failing cases.
>> +{
>> + MachineClass *mc = MACHINE_GET_CLASS(machine);
>> + ObjectClass *oc = object_class_by_name(machine->cpu_type);
>> + CPUClass *cc;
>> + int i;
>> +
>> + /*
>> + * Check if the user specified CPU type is supported when the valid
>> + * CPU types have been determined. Note that the user specified CPU
>> + * type is provided through '-cpu' option.
>> + */
>> + if (mc->valid_cpu_types && machine->cpu_type) {
>> + for (i = 0; mc->valid_cpu_types[i]; i++) {
>> + if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
>> + break;
>> + }
>> + }
>> +
>> + /* The user specified CPU type isn't valid */
>> + if (!mc->valid_cpu_types[i]) {
>> + error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
>> + if (!mc->valid_cpu_types[1]) {
>> + error_append_hint(errp, "The only valid type is: %s",
>> + mc->valid_cpu_types[0]);
>> + } else {
>> + error_append_hint(errp, "The valid types are: %s",
>> + mc->valid_cpu_types[0]);
>> + }
>> +
>> + for (i = 1; mc->valid_cpu_types[i]; i++) {
>> + error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
>> + }
>> +
>> + error_append_hint(errp, "\n");
>> + return;
>> + }
>> + }
>> +
>> + /* Check if CPU type is deprecated and warn if so */
>> + cc = CPU_CLASS(oc);
>> + if (cc && cc->deprecation_note) {
>> + warn_report("CPU model %s is deprecated -- %s",
>> + machine->cpu_type, cc->deprecation_note);
>
> Why did you move the deprecation warning within the is_supported check?
>
This check is more relevant to CPU type, to check if the CPU type has
been deprecated. Besides, @oc and @cc can be dropped from machine_run_board_init().
>> + }
>> +}
>> void machine_run_board_init(MachineState *machine, const char *mem_path, Error **errp)
>> {
>> ERRP_GUARD();
>> MachineClass *machine_class = MACHINE_GET_CLASS(machine);
>> - ObjectClass *oc = object_class_by_name(machine->cpu_type);
>> - CPUClass *cc;
>> Error *local_err = NULL;
>> /* This checkpoint is required by replay to separate prior clock
>> @@ -1449,43 +1493,11 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
>> machine->ram = machine_consume_memdev(machine, machine->memdev);
>> }
>> - /* 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) {
>> - int i;
>> -
>> - for (i = 0; machine_class->valid_cpu_types[i]; i++) {
>> - if (object_class_dynamic_cast(oc,
>> - machine_class->valid_cpu_types[i])) {
>> - /* The user specified CPU is in the valid field, we are
>> - * good to go.
>> - */
>> - break;
>> - }
>> - }
>> -
>> - if (!machine_class->valid_cpu_types[i]) {
>> - /* The user specified CPU is not valid */
>> - error_setg(&local_err, "Invalid CPU type: %s", machine->cpu_type);
>> - error_append_hint(&local_err, "The valid types are: %s",
>> - machine_class->valid_cpu_types[0]);
>> - for (i = 1; machine_class->valid_cpu_types[i]; i++) {
>> - error_append_hint(&local_err, ", %s",
>> - machine_class->valid_cpu_types[i]);
>> - }
>> - error_append_hint(&local_err, "\n");
>> -
>> - error_propagate(errp, local_err);
>> - return;
>> - }
>> - }
>> -
>> - /* Check if CPU type is deprecated and warn if so */
>> - cc = CPU_CLASS(oc);
>> - if (cc && cc->deprecation_note) {
>> - warn_report("CPU model %s is deprecated -- %s", machine->cpu_type,
>> - cc->deprecation_note);
>> + /* Check if the CPU type is supported */
>> + is_cpu_type_supported(machine, &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>
> This becomes:
>
> if (!is_cpu_type_supported(machine, errp)) {
>
Nod
>> + return;
>> }
>> if (machine->cgs) {
>
Thanks,
Gavin
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v7 3/8] machine: Print CPU model name instead of CPU type
2023-11-28 9:55 ` Philippe Mathieu-Daudé
@ 2023-11-29 3:53 ` Gavin Shan
0 siblings, 0 replies; 21+ messages in thread
From: Gavin Shan @ 2023-11-29 3:53 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, imammedo,
richard.henderson, quic_llindhol, b.galvani,
strahinja.p.jankovic, kfting, wuhaotsh, nieklinnenbank, rad,
marcin.juszkiewicz, eduardo, marcel.apfelbaum, wangyanan55,
vijai, palmer, alistair.francis, bin.meng, liwei1518, dbarboza,
zhiwei_liu, shan.gavin
Hi Phil,
On 11/28/23 20:55, Philippe Mathieu-Daudé wrote:
> On 27/11/23 00:12, Gavin Shan wrote:
>> The names of supported CPU models instead of CPU types should be
>> printed when the user specified CPU type isn't supported, to be
>> consistent with the output from '-cpu ?'.
>>
>> Correct the error messages to print CPU model names instead of CPU
>> type names.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>> hw/core/machine.c | 21 +++++++++++++++------
>> 1 file changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index 05e1922b89..898c25552a 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -1392,6 +1392,7 @@ static void is_cpu_type_supported(const MachineState *machine, Error **errp)
>> MachineClass *mc = MACHINE_GET_CLASS(machine);
>> ObjectClass *oc = object_class_by_name(machine->cpu_type);
>> CPUClass *cc;
>> + char *model;
>> int i;
>> /*
>> @@ -1408,17 +1409,25 @@ static void is_cpu_type_supported(const MachineState *machine, Error **errp)
>> /* The user specified CPU type isn't valid */
>> if (!mc->valid_cpu_types[i]) {
>> - error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
>> + model = cpu_model_from_type(machine->cpu_type);
>> + g_assert(model != NULL);
>> + error_setg(errp, "Invalid CPU type: %s", model);
>> + g_free(model);
>
> g_autofree char *requested = cpu_model_from_type(machine->cpu_type);
> error_setg(errp, "Invalid CPU type: %s", requested);
>
Yes, g_autofree shall be used here. Besides, "Invalid CPU type" needs to
be "Invalid CPU model".
>> +
>> + model = cpu_model_from_type(mc->valid_cpu_types[0]);
>> + g_assert(model != NULL);
>> if (!mc->valid_cpu_types[1]) {
>> - error_append_hint(errp, "The only valid type is: %s",
>> - mc->valid_cpu_types[0]);
>> + error_append_hint(errp, "The only valid type is: %s", model);
>
> g_autofree char *model = cpu_model_from_type(mc->valid_cpu_types[0]);
> error_append_hint(errp, "The only valid type is: %s\n", model);
>
Yes, as above.
>> } else {
>> - error_append_hint(errp, "The valid types are: %s",
>> - mc->valid_cpu_types[0]);
>> + error_append_hint(errp, "The valid types are: %s", model);
>
> Please move all the enumeration in this ladder, this makes the logic
> simpler to follow:
>
> error_append_hint(errp, "The valid types are: ");
> for (i = 0; mc->valid_cpu_types[i]; i++) {
> g_autofree char *model =
> cpu_model_from_type(mc->valid_cpu_types[i]);
> error_append_hint(errp, ", %s", model);
> }
> error_append_hint(errp, "\n");
>
Yes, but we still need to ensure mc->valid_cpu_types[0] != NULL in advance.
"The valid types are: " needs to be "The valid models are: ". Besides,
your proposed code needs to be adjusted a bit like below. Otherwise, we
will get output "The valid types are: , aaa, bbb"
} else {
error_append_hint(errp, "The valid types are: ");
for (i = 0; mc->valid_cpu_types[i]; i++) {
error_append_hint(errp, "%s%s",
mc->valid_cpu_types[i],
mc->valid_cpu_types[i + 1] ? ", " : "");
}
error_append_hint(errp, "\n");
}
I will have separate PATCH[v8 3/9] to have the changes, together with the
precise hint when only mc->valid_cpu_types[0] is valid.
>> }
>> + g_free(model);
>> for (i = 1; mc->valid_cpu_types[i]; i++) {
>> - error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
>> + model = cpu_model_from_type(mc->valid_cpu_types[i]);
>> + g_assert(model != NULL);
>> + error_append_hint(errp, ", %s", model);
>> + g_free(model);
>> }
>> error_append_hint(errp, "\n");
Thanks,
Gavin
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-11-29 3:54 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-26 23:12 [PATCH v7 0/8] Unified CPU type check Gavin Shan
2023-11-26 23:12 ` [PATCH v7 1/8] machine: Use error handling when CPU type is checked Gavin Shan
2023-11-28 9:43 ` Philippe Mathieu-Daudé
2023-11-28 22:47 ` Gavin Shan
2023-11-26 23:12 ` [PATCH v7 2/8] machine: Introduce helper is_cpu_type_supported() Gavin Shan
2023-11-28 9:38 ` Philippe Mathieu-Daudé
2023-11-28 22:55 ` Gavin Shan
2023-11-26 23:12 ` [PATCH v7 3/8] machine: Print CPU model name instead of CPU type Gavin Shan
2023-11-28 9:55 ` Philippe Mathieu-Daudé
2023-11-29 3:53 ` Gavin Shan
2023-11-26 23:12 ` [PATCH v7 4/8] hw/arm/virt: Hide host CPU model for tcg Gavin Shan
2023-11-28 9:29 ` Philippe Mathieu-Daudé
2023-11-26 23:12 ` [PATCH v7 5/8] hw/arm/virt: Check CPU type in machine_run_board_init() Gavin Shan
2023-11-27 10:13 ` Marcin Juszkiewicz
2023-11-27 10:42 ` Gavin Shan
2023-11-26 23:12 ` [PATCH v7 6/8] hw/arm/sbsa-ref: " Gavin Shan
2023-11-26 23:12 ` [PATCH v7 7/8] hw/arm: " Gavin Shan
2023-11-28 9:31 ` Philippe Mathieu-Daudé
2023-11-26 23:12 ` [PATCH v7 8/8] hw/riscv/shakti_c: " Gavin Shan
2023-11-27 10:10 ` [PATCH v7 0/8] Unified CPU type check Marcin Juszkiewicz
2023-11-27 10:15 ` Gavin Shan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.