* [PATCH v9 0/9] Unified CPU type check
@ 2023-12-04 0:47 Gavin Shan
2023-12-04 0:47 ` [PATCH v9 1/9] machine: Use error handling when CPU type is checked Gavin Shan
` (9 more replies)
0 siblings, 10 replies; 19+ messages in thread
From: Gavin Shan @ 2023-12-04 0:47 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
philmd, armbru, 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.
This series can be checked out from:
git@github.com:gwshan/qemu.git (branch: kvm/cpu-type)
PATCH[1-4] refactors and improves the logic to validate CPU type in
machine_run_board_init()
PATCH[5-9] 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
v7: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01045.html
v8: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01168.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 model: cortex-a8
The valid models 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 model: cortex-a53
The valid models 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 model: sa1100
The valid models 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
=========
v9:
* Pick r-bs from Markus and Phil (Gavin)
* Improved change log for PATCH[v9 1/9] (Markus)
* assert(mc->valid_cpu_types[0] != NULL) and assert(cc != NULL)
in is_cpu_type_supported() of PATCH[v9 3/9] (Phil)
v8:
* Pick r-bs from Phil (Gavin)
* Drop @local_err and use @errp in machine_run_board_init() (Phil)
* Add PATCH[v8 3/9] to improve is_cpu_type_supported() (Phil)
* Use g_autofree and replace 'type' with 'model' for the
error messages in is_cpu_type_supported() (Phil)
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)
Gavin Shan (9):
machine: Use error handling when CPU type is checked
machine: Introduce helper is_cpu_type_supported()
machine: Improve 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 | 94 +++++++++++++++++++++++++----------------
hw/riscv/shakti_c.c | 13 +++---
12 files changed, 170 insertions(+), 157 deletions(-)
--
2.42.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v9 1/9] machine: Use error handling when CPU type is checked
2023-12-04 0:47 [PATCH v9 0/9] Unified CPU type check Gavin Shan
@ 2023-12-04 0:47 ` Gavin Shan
2024-01-05 11:00 ` Philippe Mathieu-Daudé
2023-12-04 0:47 ` [PATCH v9 2/9] machine: Introduce helper is_cpu_type_supported() Gavin Shan
` (8 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Gavin Shan @ 2023-12-04 0:47 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
philmd, armbru, wangyanan55, vijai, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu, shan.gavin
Functions that use an Error **errp parameter to return errors should
not also report them to the user, because reporting is the caller's
job. The principle is violated by machine_run_board_init() because
it calls error_report(), error_printf(), and exit(1) when the machine
doesn't support the requested CPU type.
Clean this up by using error_setg() and error_append_hint() instead.
No functional change, as the only caller passes &error_fatal.
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
v9: Improved change log (Markus)
---
hw/core/machine.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 0c17398141..bde7f4af6d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1466,15 +1466,16 @@ 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(errp, "Invalid CPU type: %s", machine->cpu_type);
+ error_append_hint(errp, "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(errp, ", %s",
+ machine_class->valid_cpu_types[i]);
}
- error_printf("\n");
- exit(1);
+ error_append_hint(&errp, "\n");
+ return;
}
}
--
2.42.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v9 2/9] machine: Introduce helper is_cpu_type_supported()
2023-12-04 0:47 [PATCH v9 0/9] Unified CPU type check Gavin Shan
2023-12-04 0:47 ` [PATCH v9 1/9] machine: Use error handling when CPU type is checked Gavin Shan
@ 2023-12-04 0:47 ` Gavin Shan
2023-12-04 0:47 ` [PATCH v9 3/9] machine: Improve is_cpu_type_supported() Gavin Shan
` (7 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Gavin Shan @ 2023-12-04 0:47 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
philmd, armbru, 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 comments are tweaked a bit
either.
No functional change intended.
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/core/machine.c | 83 +++++++++++++++++++++++++----------------------
1 file changed, 45 insertions(+), 38 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index bde7f4af6d..1797e002f9 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1387,13 +1387,53 @@ out:
return r;
}
+static bool 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);
+ 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 false;
+ }
+ }
+
+ /* 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);
+ }
+
+ return true;
+}
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;
/* This checkpoint is required by replay to separate prior clock
reading from the other reads, because timer polling functions query
@@ -1448,42 +1488,9 @@ 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(errp, "Invalid CPU type: %s", machine->cpu_type);
- error_append_hint(errp, "The valid types are: %s",
- machine_class->valid_cpu_types[0]);
- for (i = 1; machine_class->valid_cpu_types[i]; i++) {
- error_append_hint(errp, ", %s",
- machine_class->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);
+ /* Check if the CPU type is supported */
+ if (!is_cpu_type_supported(machine, errp)) {
+ return;
}
if (machine->cgs) {
--
2.42.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v9 3/9] machine: Improve is_cpu_type_supported()
2023-12-04 0:47 [PATCH v9 0/9] Unified CPU type check Gavin Shan
2023-12-04 0:47 ` [PATCH v9 1/9] machine: Use error handling when CPU type is checked Gavin Shan
2023-12-04 0:47 ` [PATCH v9 2/9] machine: Introduce helper is_cpu_type_supported() Gavin Shan
@ 2023-12-04 0:47 ` Gavin Shan
2024-01-05 22:09 ` Philippe Mathieu-Daudé
2023-12-04 0:47 ` [PATCH v9 4/9] machine: Print CPU model name instead of CPU type Gavin Shan
` (6 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Gavin Shan @ 2023-12-04 0:47 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
philmd, armbru, wangyanan55, vijai, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu, shan.gavin
It's no sense to check the CPU type when mc->valid_cpu_types[0] is
NULL, which is a program error. Raise an assert on this.
A precise hint for the error message is given when mc->valid_cpu_types[0]
is the only valid entry. Besides, enumeration on mc->valid_cpu_types[0]
when we have mutiple valid entries there is avoided to increase the code
readability, as suggested by Philippe Mathieu-Daudé.
Besides, @cc comes from machine->cpu_type or mc->default_cpu_type. For
the later case, it can be NULL and it's also a program error. We should
use assert() in this case.
Signed-off-by: Gavin Shan <gshan@redhat.com>
v9: assert(mc->valid_cpu_types[0] != NULL) (Phil)
assert(cc != NULL); (Phil)
---
hw/core/machine.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1797e002f9..4ae9aaee8e 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1400,6 +1400,7 @@ static bool is_cpu_type_supported(const MachineState *machine, Error **errp)
* type is provided through '-cpu' option.
*/
if (mc->valid_cpu_types && machine->cpu_type) {
+ assert(mc->valid_cpu_types[0] != NULL);
for (i = 0; mc->valid_cpu_types[i]; i++) {
if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
break;
@@ -1409,20 +1410,27 @@ static bool 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);
- 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]);
+ if (!mc->valid_cpu_types[1]) {
+ error_append_hint(errp, "The only valid type is: %s\n",
+ mc->valid_cpu_types[0]);
+ } 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");
}
- error_append_hint(errp, "\n");
return false;
}
}
/* Check if CPU type is deprecated and warn if so */
cc = CPU_CLASS(oc);
- if (cc && cc->deprecation_note) {
+ assert(cc != NULL);
+ if (cc->deprecation_note) {
warn_report("CPU model %s is deprecated -- %s",
machine->cpu_type, cc->deprecation_note);
}
--
2.42.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v9 4/9] machine: Print CPU model name instead of CPU type
2023-12-04 0:47 [PATCH v9 0/9] Unified CPU type check Gavin Shan
` (2 preceding siblings ...)
2023-12-04 0:47 ` [PATCH v9 3/9] machine: Improve is_cpu_type_supported() Gavin Shan
@ 2023-12-04 0:47 ` Gavin Shan
2023-12-04 0:47 ` [PATCH v9 5/9] hw/arm/virt: Hide host CPU model for tcg Gavin Shan
` (5 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Gavin Shan @ 2023-12-04 0:47 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
philmd, armbru, 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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/core/machine.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 4ae9aaee8e..bc9c3e7dcd 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1409,15 +1409,19 @@ static bool 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);
+ g_autofree char *requested = cpu_model_from_type(machine->cpu_type);
+ error_setg(errp, "Invalid CPU model: %s", requested);
if (!mc->valid_cpu_types[1]) {
- error_append_hint(errp, "The only valid type is: %s\n",
- mc->valid_cpu_types[0]);
+ 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: ");
+ error_append_hint(errp, "The valid models 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%s",
- mc->valid_cpu_types[i],
+ model,
mc->valid_cpu_types[i + 1] ? ", " : "");
}
error_append_hint(errp, "\n");
--
2.42.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v9 5/9] hw/arm/virt: Hide host CPU model for tcg
2023-12-04 0:47 [PATCH v9 0/9] Unified CPU type check Gavin Shan
` (3 preceding siblings ...)
2023-12-04 0:47 ` [PATCH v9 4/9] machine: Print CPU model name instead of CPU type Gavin Shan
@ 2023-12-04 0:47 ` Gavin Shan
2023-12-04 0:47 ` [PATCH v9 6/9] hw/arm/virt: Check CPU type in machine_run_board_init() Gavin Shan
` (4 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Gavin Shan @ 2023-12-04 0:47 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
philmd, armbru, 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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@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] 19+ messages in thread
* [PATCH v9 6/9] hw/arm/virt: Check CPU type in machine_run_board_init()
2023-12-04 0:47 [PATCH v9 0/9] Unified CPU type check Gavin Shan
` (4 preceding siblings ...)
2023-12-04 0:47 ` [PATCH v9 5/9] hw/arm/virt: Hide host CPU model for tcg Gavin Shan
@ 2023-12-04 0:47 ` Gavin Shan
2023-12-04 0:47 ` [PATCH v9 7/9] hw/arm/sbsa-ref: " Gavin Shan
` (3 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Gavin Shan @ 2023-12-04 0:47 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
philmd, armbru, 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] 19+ messages in thread
* [PATCH v9 7/9] hw/arm/sbsa-ref: Check CPU type in machine_run_board_init()
2023-12-04 0:47 [PATCH v9 0/9] Unified CPU type check Gavin Shan
` (5 preceding siblings ...)
2023-12-04 0:47 ` [PATCH v9 6/9] hw/arm/virt: Check CPU type in machine_run_board_init() Gavin Shan
@ 2023-12-04 0:47 ` Gavin Shan
2023-12-04 0:47 ` [PATCH v9 8/9] hw/arm: " Gavin Shan
` (2 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Gavin Shan @ 2023-12-04 0:47 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
philmd, armbru, 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] 19+ messages in thread
* [PATCH v9 8/9] hw/arm: Check CPU type in machine_run_board_init()
2023-12-04 0:47 [PATCH v9 0/9] Unified CPU type check Gavin Shan
` (6 preceding siblings ...)
2023-12-04 0:47 ` [PATCH v9 7/9] hw/arm/sbsa-ref: " Gavin Shan
@ 2023-12-04 0:47 ` Gavin Shan
2023-12-04 0:47 ` [PATCH v9 9/9] hw/riscv/shakti_c: " Gavin Shan
2023-12-12 4:55 ` [PATCH v9 0/9] Unified CPU type check Gavin Shan
9 siblings, 0 replies; 19+ messages in thread
From: Gavin Shan @ 2023-12-04 0:47 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
philmd, armbru, 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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@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] 19+ messages in thread
* [PATCH v9 9/9] hw/riscv/shakti_c: Check CPU type in machine_run_board_init()
2023-12-04 0:47 [PATCH v9 0/9] Unified CPU type check Gavin Shan
` (7 preceding siblings ...)
2023-12-04 0:47 ` [PATCH v9 8/9] hw/arm: " Gavin Shan
@ 2023-12-04 0:47 ` Gavin Shan
2023-12-12 4:55 ` [PATCH v9 0/9] Unified CPU type check Gavin Shan
9 siblings, 0 replies; 19+ messages in thread
From: Gavin Shan @ 2023-12-04 0:47 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
philmd, armbru, 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] 19+ messages in thread
* Re: [PATCH v9 0/9] Unified CPU type check
2023-12-04 0:47 [PATCH v9 0/9] Unified CPU type check Gavin Shan
` (8 preceding siblings ...)
2023-12-04 0:47 ` [PATCH v9 9/9] hw/riscv/shakti_c: " Gavin Shan
@ 2023-12-12 4:55 ` Gavin Shan
2023-12-13 10:08 ` Philippe Mathieu-Daudé
9 siblings, 1 reply; 19+ messages in thread
From: Gavin Shan @ 2023-12-12 4:55 UTC (permalink / raw)
To: qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
philmd, armbru, wangyanan55, vijai, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu, shan.gavin
Hi Phil,
On 12/4/23 10:47, Gavin Shan wrote:
> 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.
>
> This series can be checked out from:
>
> git@github.com:gwshan/qemu.git (branch: kvm/cpu-type)
>
> PATCH[1-4] refactors and improves the logic to validate CPU type in
> machine_run_board_init()
> PATCH[5-9] validates the CPU type in machine_run_board_init() for the
> individual boards
>
> v6: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00768.html
> v7: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01045.html
> v8: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01168.html
>
Ping to see if there is a chance to queue it up before the Chrismas? :)
Thanks,
Gavin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v9 0/9] Unified CPU type check
2023-12-12 4:55 ` [PATCH v9 0/9] Unified CPU type check Gavin Shan
@ 2023-12-13 10:08 ` Philippe Mathieu-Daudé
2023-12-13 10:54 ` Gavin Shan
0 siblings, 1 reply; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-12-13 10:08 UTC (permalink / raw)
To: Gavin Shan, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
armbru, wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
On 12/12/23 05:55, Gavin Shan wrote:
> Hi Phil,
>
> On 12/4/23 10:47, Gavin Shan wrote:
>> 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.
>>
>> This series can be checked out from:
>>
>> git@github.com:gwshan/qemu.git (branch: kvm/cpu-type)
>>
>> PATCH[1-4] refactors and improves the logic to validate CPU type in
>> machine_run_board_init()
>> PATCH[5-9] validates the CPU type in machine_run_board_init() for the
>> individual boards
>>
>> v6: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00768.html
>> v7: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01045.html
>> v8: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01168.html
>>
>
> Ping to see if there is a chance to queue it up before the Chrismas? :)
Series queued. "Before" Christmas will depend on the final release tag.
Thanks for the various iterations,
Phil.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v9 0/9] Unified CPU type check
2023-12-13 10:08 ` Philippe Mathieu-Daudé
@ 2023-12-13 10:54 ` Gavin Shan
2024-01-05 22:12 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 19+ messages in thread
From: Gavin Shan @ 2023-12-13 10:54 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
armbru, wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
On 12/13/23 20:08, Philippe Mathieu-Daudé wrote:
> On 12/12/23 05:55, Gavin Shan wrote:
>> On 12/4/23 10:47, Gavin Shan wrote:
>>> 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.
>>>
>>> This series can be checked out from:
>>>
>>> git@github.com:gwshan/qemu.git (branch: kvm/cpu-type)
>>>
>>> PATCH[1-4] refactors and improves the logic to validate CPU type in
>>> machine_run_board_init()
>>> PATCH[5-9] validates the CPU type in machine_run_board_init() for the
>>> individual boards
>>>
>>> v6: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00768.html
>>> v7: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01045.html
>>> v8: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01168.html
>>>
>>
>> Ping to see if there is a chance to queue it up before the Chrismas? :)
>
> Series queued. "Before" Christmas will depend on the final release tag.
>
> Thanks for the various iterations,
>
Phil, thank you for you continuous reviews and valuable comments.
Yes, the final merge to master branch depends on the release plan.
'queue' meant to merge the series to your 'cpus-next' branch ;-)
Thanks,
Gavin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v9 1/9] machine: Use error handling when CPU type is checked
2023-12-04 0:47 ` [PATCH v9 1/9] machine: Use error handling when CPU type is checked Gavin Shan
@ 2024-01-05 11:00 ` Philippe Mathieu-Daudé
2024-01-08 1:11 ` Gavin Shan
0 siblings, 1 reply; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-05 11:00 UTC (permalink / raw)
To: Gavin Shan, armbru
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
wangyanan55, vijai, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, shan.gavin, qemu-arm
On 4/12/23 01:47, Gavin Shan wrote:
> Functions that use an Error **errp parameter to return errors should
> not also report them to the user, because reporting is the caller's
> job. The principle is violated by machine_run_board_init() because
> it calls error_report(), error_printf(), and exit(1) when the machine
> doesn't support the requested CPU type.
>
> Clean this up by using error_setg() and error_append_hint() instead.
> No functional change, as the only caller passes &error_fatal.
>
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> ---
> v9: Improved change log (Markus)
> ---
> hw/core/machine.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 0c17398141..bde7f4af6d 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1466,15 +1466,16 @@ 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(errp, "Invalid CPU type: %s", machine->cpu_type);
> + error_append_hint(errp, "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(errp, ", %s",
> + machine_class->valid_cpu_types[i]);
> }
> - error_printf("\n");
>
> - exit(1);
> + error_append_hint(&errp, "\n");
This doesn't build:
hw/core/machine.c:1488:31: error: incompatible pointer types passing
'Error ***' (aka 'struct Error ***') to parameter of type 'Error *const
*' (aka 'struct Error *const *'); remove &
[-Werror,-Wincompatible-pointer-types]
error_append_hint(&errp, "\n");
^~~~~
> + return;
> }
> }
>
Squashing:
-- >8 --
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 021044aaaf..1898d1d1d7 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1487,3 +1487,3 @@ void machine_run_board_init(MachineState *machine,
const char *mem_path, Error *
- error_append_hint(&errp, "\n");
+ error_append_hint(errp, "\n");
return;
---
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v9 3/9] machine: Improve is_cpu_type_supported()
2023-12-04 0:47 ` [PATCH v9 3/9] machine: Improve is_cpu_type_supported() Gavin Shan
@ 2024-01-05 22:09 ` Philippe Mathieu-Daudé
2024-01-08 1:21 ` Gavin Shan
0 siblings, 1 reply; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-05 22:09 UTC (permalink / raw)
To: Gavin Shan, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
armbru, wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
On 4/12/23 01:47, Gavin Shan wrote:
> It's no sense to check the CPU type when mc->valid_cpu_types[0] is
> NULL, which is a program error. Raise an assert on this.
>
> A precise hint for the error message is given when mc->valid_cpu_types[0]
> is the only valid entry. Besides, enumeration on mc->valid_cpu_types[0]
> when we have mutiple valid entries there is avoided to increase the code
> readability, as suggested by Philippe Mathieu-Daudé.
>
> Besides, @cc comes from machine->cpu_type or mc->default_cpu_type. For
> the later case, it can be NULL and it's also a program error. We should
> use assert() in this case.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> v9: assert(mc->valid_cpu_types[0] != NULL) (Phil)
> assert(cc != NULL); (Phil)
> ---
> hw/core/machine.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 1797e002f9..4ae9aaee8e 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1400,6 +1400,7 @@ static bool is_cpu_type_supported(const MachineState *machine, Error **errp)
> * type is provided through '-cpu' option.
> */
> if (mc->valid_cpu_types && machine->cpu_type) {
> + assert(mc->valid_cpu_types[0] != NULL);
> for (i = 0; mc->valid_cpu_types[i]; i++) {
> if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
> break;
> @@ -1409,20 +1410,27 @@ static bool 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);
> - 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]);
> + if (!mc->valid_cpu_types[1]) {
> + error_append_hint(errp, "The only valid type is: %s\n",
> + mc->valid_cpu_types[0]);
> + } 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");
> }
>
> - error_append_hint(errp, "\n");
> return false;
> }
> }
>
> /* Check if CPU type is deprecated and warn if so */
> cc = CPU_CLASS(oc);
> - if (cc && cc->deprecation_note) {
> + assert(cc != NULL);
> + if (cc->deprecation_note) {
> warn_report("CPU model %s is deprecated -- %s",
> machine->cpu_type, cc->deprecation_note);
> }
Since we were getting:
$ qemu-system-s390x -M none
QEMU 8.2.50 monitor - type 'help' for more information
qemu-system-s390x: ../../hw/core/machine.c:1444: _Bool
is_cpu_type_supported(const MachineState *, Error **): Assertion `cc !=
NULL' failed.
Aborted (core dumped)
I added a check on mc->valid_cpu_types before calling
is_cpu_type_supported() in the previous patch. See commit acbadc5a29.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v9 0/9] Unified CPU type check
2023-12-13 10:54 ` Gavin Shan
@ 2024-01-05 22:12 ` Philippe Mathieu-Daudé
2024-01-08 1:24 ` Gavin Shan
0 siblings, 1 reply; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-05 22:12 UTC (permalink / raw)
To: Gavin Shan, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
armbru, wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
Hi Gavin,
On 13/12/23 11:54, Gavin Shan wrote:
> On 12/13/23 20:08, Philippe Mathieu-Daudé wrote:
>> On 12/12/23 05:55, Gavin Shan wrote:
>>> On 12/4/23 10:47, Gavin Shan wrote:
>>>> 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.
>>>>
>>>> This series can be checked out from:
>>>>
>>>> git@github.com:gwshan/qemu.git (branch: kvm/cpu-type)
>>>>
>>>> PATCH[1-4] refactors and improves the logic to validate CPU type in
>>>> machine_run_board_init()
>>>> PATCH[5-9] validates the CPU type in machine_run_board_init() for the
>>>> individual boards
>>>>
>>>> v6:
>>>> https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00768.html
>>>> v7:
>>>> https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01045.html
>>>> v8:
>>>> https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01168.html
>>>>
>>>
>>> Ping to see if there is a chance to queue it up before the Chrismas? :)
>>
>> Series queued. "Before" Christmas will depend on the final release tag.
>>
>> Thanks for the various iterations,
>>
>
> Phil, thank you for you continuous reviews and valuable comments.
>
> Yes, the final merge to master branch depends on the release plan.
> 'queue' meant to merge the series to your 'cpus-next' branch ;-)
I had to fix 3 different issues caught by our CI. Next time please
run your series on GitLab CI, you just have to push your branch and
wait for the result :)
Now merged as 445946f4dd..cd75cc6337.
Happy new year!
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v9 1/9] machine: Use error handling when CPU type is checked
2024-01-05 11:00 ` Philippe Mathieu-Daudé
@ 2024-01-08 1:11 ` Gavin Shan
0 siblings, 0 replies; 19+ messages in thread
From: Gavin Shan @ 2024-01-08 1:11 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, armbru
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
wangyanan55, vijai, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, shan.gavin, qemu-arm
On 1/5/24 21:00, Philippe Mathieu-Daudé wrote:
> On 4/12/23 01:47, Gavin Shan wrote:
>> Functions that use an Error **errp parameter to return errors should
>> not also report them to the user, because reporting is the caller's
>> job. The principle is violated by machine_run_board_init() because
>> it calls error_report(), error_printf(), and exit(1) when the machine
>> doesn't support the requested CPU type.
>>
>> Clean this up by using error_setg() and error_append_hint() instead.
>> No functional change, as the only caller passes &error_fatal.
>>
>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> v9: Improved change log (Markus)
>> ---
>> hw/core/machine.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index 0c17398141..bde7f4af6d 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -1466,15 +1466,16 @@ 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(errp, "Invalid CPU type: %s", machine->cpu_type);
>> + error_append_hint(errp, "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(errp, ", %s",
>> + machine_class->valid_cpu_types[i]);
>> }
>> - error_printf("\n");
>> - exit(1);
>> + error_append_hint(&errp, "\n");
>
> This doesn't build:
>
> hw/core/machine.c:1488:31: error: incompatible pointer types passing 'Error ***' (aka 'struct Error ***') to parameter of type 'Error *const *' (aka 'struct Error *const *'); remove & [-Werror,-Wincompatible-pointer-types]
> error_append_hint(&errp, "\n");
> ^~~~~
>
Yes, &errp should have been errp. The problematic code was carried from
previous revisions and has been corrected by PATCH[2/9]. It's how I missed
the building error. Thanks for fixing it up!
Thanks,
Gavin
>> + return;
>> }
>> }
>
> Squashing:
> -- >8 --
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 021044aaaf..1898d1d1d7 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1487,3 +1487,3 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
>
> - error_append_hint(&errp, "\n");
> + error_append_hint(errp, "\n");
> return;
> ---
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v9 3/9] machine: Improve is_cpu_type_supported()
2024-01-05 22:09 ` Philippe Mathieu-Daudé
@ 2024-01-08 1:21 ` Gavin Shan
0 siblings, 0 replies; 19+ messages in thread
From: Gavin Shan @ 2024-01-08 1:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
armbru, wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
On 1/6/24 08:09, Philippe Mathieu-Daudé wrote:
> On 4/12/23 01:47, Gavin Shan wrote:
>> It's no sense to check the CPU type when mc->valid_cpu_types[0] is
>> NULL, which is a program error. Raise an assert on this.
>>
>> A precise hint for the error message is given when mc->valid_cpu_types[0]
>> is the only valid entry. Besides, enumeration on mc->valid_cpu_types[0]
>> when we have mutiple valid entries there is avoided to increase the code
>> readability, as suggested by Philippe Mathieu-Daudé.
>>
>> Besides, @cc comes from machine->cpu_type or mc->default_cpu_type. For
>> the later case, it can be NULL and it's also a program error. We should
>> use assert() in this case.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> v9: assert(mc->valid_cpu_types[0] != NULL) (Phil)
>> assert(cc != NULL); (Phil)
>> ---
>> hw/core/machine.c | 20 ++++++++++++++------
>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index 1797e002f9..4ae9aaee8e 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -1400,6 +1400,7 @@ static bool is_cpu_type_supported(const MachineState *machine, Error **errp)
>> * type is provided through '-cpu' option.
>> */
>> if (mc->valid_cpu_types && machine->cpu_type) {
>> + assert(mc->valid_cpu_types[0] != NULL);
>> for (i = 0; mc->valid_cpu_types[i]; i++) {
>> if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
>> break;
>> @@ -1409,20 +1410,27 @@ static bool 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);
>> - 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]);
>> + if (!mc->valid_cpu_types[1]) {
>> + error_append_hint(errp, "The only valid type is: %s\n",
>> + mc->valid_cpu_types[0]);
>> + } 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");
>> }
>> - error_append_hint(errp, "\n");
>> return false;
>> }
>> }
>> /* Check if CPU type is deprecated and warn if so */
>> cc = CPU_CLASS(oc);
>> - if (cc && cc->deprecation_note) {
>> + assert(cc != NULL);
>> + if (cc->deprecation_note) {
>> warn_report("CPU model %s is deprecated -- %s",
>> machine->cpu_type, cc->deprecation_note);
>> }
>
> Since we were getting:
>
> $ qemu-system-s390x -M none
> QEMU 8.2.50 monitor - type 'help' for more information
> qemu-system-s390x: ../../hw/core/machine.c:1444: _Bool is_cpu_type_supported(const MachineState *, Error **): Assertion `cc != NULL' failed.
> Aborted (core dumped)
>
> I added a check on mc->valid_cpu_types before calling
> is_cpu_type_supported() in the previous patch. See commit acbadc5a29.
>
Phil, thanks for fixing it up and it looks good to me.
Thanks,
Gavin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v9 0/9] Unified CPU type check
2024-01-05 22:12 ` Philippe Mathieu-Daudé
@ 2024-01-08 1:24 ` Gavin Shan
0 siblings, 0 replies; 19+ messages in thread
From: Gavin Shan @ 2024-01-08 1:24 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-arm
Cc: qemu-devel, qemu-riscv, peter.maydell, b.galvani,
strahinja.p.jankovic, imammedo, kfting, wuhaotsh, nieklinnenbank,
rad, quic_llindhol, marcin.juszkiewicz, eduardo, marcel.apfelbaum,
armbru, wangyanan55, vijai, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu, shan.gavin
Hi Phil,
On 1/6/24 08:12, Philippe Mathieu-Daudé wrote:
> On 13/12/23 11:54, Gavin Shan wrote:
>> On 12/13/23 20:08, Philippe Mathieu-Daudé wrote:
>>> On 12/12/23 05:55, Gavin Shan wrote:
>>>> On 12/4/23 10:47, Gavin Shan wrote:
>>>>> 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.
>>>>>
>>>>> This series can be checked out from:
>>>>>
>>>>> git@github.com:gwshan/qemu.git (branch: kvm/cpu-type)
>>>>>
>>>>> PATCH[1-4] refactors and improves the logic to validate CPU type in
>>>>> machine_run_board_init()
>>>>> PATCH[5-9] validates the CPU type in machine_run_board_init() for the
>>>>> individual boards
>>>>>
>>>>> v6: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00768.html
>>>>> v7: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01045.html
>>>>> v8: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01168.html
>>>>>
>>>>
>>>> Ping to see if there is a chance to queue it up before the Chrismas? :)
>>>
>>> Series queued. "Before" Christmas will depend on the final release tag.
>>>
>>> Thanks for the various iterations,
>>>
>>
>> Phil, thank you for you continuous reviews and valuable comments.
>>
>> Yes, the final merge to master branch depends on the release plan.
>> 'queue' meant to merge the series to your 'cpus-next' branch ;-)
>
> I had to fix 3 different issues caught by our CI. Next time please
> run your series on GitLab CI, you just have to push your branch and
> wait for the result :)
>
> Now merged as 445946f4dd..cd75cc6337.
>
> Happy new year!
>
Happy new year. I just came back from holiday.
Thanks for the reminder for GitLab CI, which I don't know how to run yet.
I will learn how to run it from gitlab :)
Thanks,
Gavin
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2024-01-08 1:25 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-04 0:47 [PATCH v9 0/9] Unified CPU type check Gavin Shan
2023-12-04 0:47 ` [PATCH v9 1/9] machine: Use error handling when CPU type is checked Gavin Shan
2024-01-05 11:00 ` Philippe Mathieu-Daudé
2024-01-08 1:11 ` Gavin Shan
2023-12-04 0:47 ` [PATCH v9 2/9] machine: Introduce helper is_cpu_type_supported() Gavin Shan
2023-12-04 0:47 ` [PATCH v9 3/9] machine: Improve is_cpu_type_supported() Gavin Shan
2024-01-05 22:09 ` Philippe Mathieu-Daudé
2024-01-08 1:21 ` Gavin Shan
2023-12-04 0:47 ` [PATCH v9 4/9] machine: Print CPU model name instead of CPU type Gavin Shan
2023-12-04 0:47 ` [PATCH v9 5/9] hw/arm/virt: Hide host CPU model for tcg Gavin Shan
2023-12-04 0:47 ` [PATCH v9 6/9] hw/arm/virt: Check CPU type in machine_run_board_init() Gavin Shan
2023-12-04 0:47 ` [PATCH v9 7/9] hw/arm/sbsa-ref: " Gavin Shan
2023-12-04 0:47 ` [PATCH v9 8/9] hw/arm: " Gavin Shan
2023-12-04 0:47 ` [PATCH v9 9/9] hw/riscv/shakti_c: " Gavin Shan
2023-12-12 4:55 ` [PATCH v9 0/9] Unified CPU type check Gavin Shan
2023-12-13 10:08 ` Philippe Mathieu-Daudé
2023-12-13 10:54 ` Gavin Shan
2024-01-05 22:12 ` Philippe Mathieu-Daudé
2024-01-08 1:24 ` Gavin Shan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).