[PATCH-for-8.2? v2 0/4] hw/arm/stm32xxx: Report error when incorrect CPU is used

[PATCH-for-8.2? v2 0/4] hw/arm/stm32xxx: Report error when incorrect CPU is used

[Philippe Mathieu-Daudé]

Series fully reviewed.

Hi,

While reviewing this [*] series from Arnaud and Inès,
I realized STM32 based boards don't check the requested
CPU type is correct, and ignore it. Possibly confusing
users.
Since these simple boards code is used as template,
I took the opportunity to sanitize it a bit, using the
MachineClass::valid_cpu_types which seems simpler (to me)
for newcomers.

Patches are candidate for 8.2 but this is a long
standing issue, not a regression from 8.1.

Regards,

Phil.

[*] https://lore.kernel.org/qemu-devel/170003673257.14701.8139061802716120109-0@git.sr.ht/

Gavin Shan (1):
  hw/core/machine: Constify MachineClass::valid_cpu_types[]

Philippe Mathieu-Daudé (3):
  hw/arm/stm32f405: Report error when incorrect CPU is used
  hw/arm/stm32f205: Report error when incorrect CPU is used
  hw/arm/stm32f100: Report error when incorrect CPU is used

 include/hw/arm/stm32f100_soc.h |  4 ----
 include/hw/arm/stm32f205_soc.h |  4 ----
 include/hw/arm/stm32f405_soc.h |  4 ----
 include/hw/boards.h            |  2 +-
 hw/arm/netduino2.c             |  7 ++++++-
 hw/arm/netduinoplus2.c         |  7 ++++++-
 hw/arm/olimex-stm32-h405.c     |  8 ++++++--
 hw/arm/stm32f100_soc.c         |  9 ++-------
 hw/arm/stm32f205_soc.c         |  9 ++-------
 hw/arm/stm32f405_soc.c         |  8 +-------
 hw/arm/stm32vldiscovery.c      |  7 ++++++-
 hw/hppa/machine.c              | 22 ++++++++++------------
 hw/m68k/q800.c                 | 11 +++++------
 13 files changed, 45 insertions(+), 57 deletions(-)

-- 
2.41.0

[PATCH-for-8.2? v2 1/4] hw/core/machine: Constify MachineClass::valid_cpu_types[]

[Philippe Mathieu-Daudé]

From: Gavin Shan <gshan@redhat.com>

Constify MachineClass::valid_cpu_types[i], as suggested by Richard
Henderson.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
[PMD: Constify HPPA machines,
      restrict valid_cpu_types to machine_class_init() handlers]
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/boards.h |  2 +-
 hw/hppa/machine.c   | 22 ++++++++++------------
 hw/m68k/q800.c      | 11 +++++------
 3 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index a735999298..da85f86efb 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -273,7 +273,7 @@ struct MachineClass {
     bool has_hotpluggable_cpus;
     bool ignore_memory_transaction_failures;
     int numa_mem_align_shift;
-    const char **valid_cpu_types;
+    const char * const *valid_cpu_types;
     strList *allowed_dynamic_sysbus_devices;
     bool auto_enable_numa_with_memhp;
     bool auto_enable_numa_with_memdev;
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 9d08f39490..c8da7c18d5 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -672,19 +672,18 @@ static void hppa_nmi(NMIState *n, int cpu_index, Error **errp)
     }
 }
 
-static const char *HP_B160L_machine_valid_cpu_types[] = {
-    TYPE_HPPA_CPU,
-    NULL
-};
-
 static void HP_B160L_machine_init_class_init(ObjectClass *oc, void *data)
 {
+    static const char * const valid_cpu_types[] = {
+        TYPE_HPPA_CPU,
+        NULL
+    };
     MachineClass *mc = MACHINE_CLASS(oc);
     NMIClass *nc = NMI_CLASS(oc);
 
     mc->desc = "HP B160L workstation";
     mc->default_cpu_type = TYPE_HPPA_CPU;
-    mc->valid_cpu_types = HP_B160L_machine_valid_cpu_types;
+    mc->valid_cpu_types = valid_cpu_types;
     mc->init = machine_HP_B160L_init;
     mc->reset = hppa_machine_reset;
     mc->block_default_type = IF_SCSI;
@@ -709,19 +708,18 @@ static const TypeInfo HP_B160L_machine_init_typeinfo = {
     },
 };
 
-static const char *HP_C3700_machine_valid_cpu_types[] = {
-    TYPE_HPPA64_CPU,
-    NULL
-};
-
 static void HP_C3700_machine_init_class_init(ObjectClass *oc, void *data)
 {
+    static const char * const valid_cpu_types[] = {
+        TYPE_HPPA64_CPU,
+        NULL
+    };
     MachineClass *mc = MACHINE_CLASS(oc);
     NMIClass *nc = NMI_CLASS(oc);
 
     mc->desc = "HP C3700 workstation";
     mc->default_cpu_type = TYPE_HPPA64_CPU;
-    mc->valid_cpu_types = HP_C3700_machine_valid_cpu_types;
+    mc->valid_cpu_types = valid_cpu_types;
     mc->init = machine_HP_C3700_init;
     mc->reset = hppa_machine_reset;
     mc->block_default_type = IF_SCSI;
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 1d7cd5ff1c..83d1571d02 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -726,19 +726,18 @@ static GlobalProperty hw_compat_q800[] = {
 };
 static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
 
-static const char *q800_machine_valid_cpu_types[] = {
-    M68K_CPU_TYPE_NAME("m68040"),
-    NULL
-};
-
 static void q800_machine_class_init(ObjectClass *oc, void *data)
 {
+    static const char * const valid_cpu_types[] = {
+        M68K_CPU_TYPE_NAME("m68040"),
+        NULL
+    };
     MachineClass *mc = MACHINE_CLASS(oc);
 
     mc->desc = "Macintosh Quadra 800";
     mc->init = q800_machine_init;
     mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040");
-    mc->valid_cpu_types = q800_machine_valid_cpu_types;
+    mc->valid_cpu_types = valid_cpu_types;
     mc->max_cpus = 1;
     mc->block_default_type = IF_SCSI;
     mc->default_ram_id = "m68k_mac.ram";
-- 
2.41.0

[PATCH-for-8.2? v2 2/4] hw/arm/stm32f405: Report error when incorrect CPU is used

[Philippe Mathieu-Daudé]

Both 'netduinoplus2' and 'olimex-stm32-h405' machines ignore the
CPU type requested by the command line. This might confuse users,
since the following will create a machine with a Cortex-M4 CPU:

  $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f

Set the MachineClass::valid_cpu_types field (introduced in commit
c9cf636d48 "machine: Add a valid_cpu_types property").
Remove the now unused MachineClass::default_cpu_type field.

We now get:

  $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f
  qemu-system-aarch64: Invalid CPU type: cortex-r5f-arm-cpu
  The valid types are: cortex-m4-arm-cpu

Since the SoC family can only use Cortex-M4 CPUs, hard-code the
CPU type name at the SoC level, removing the QOM property
entirely.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/arm/stm32f405_soc.h | 4 ----
 hw/arm/netduinoplus2.c         | 7 ++++++-
 hw/arm/olimex-stm32-h405.c     | 8 ++++++--
 hw/arm/stm32f405_soc.c         | 8 +-------
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/include/hw/arm/stm32f405_soc.h b/include/hw/arm/stm32f405_soc.h
index c968ce3ab2..d15c03c4b5 100644
--- a/include/hw/arm/stm32f405_soc.h
+++ b/include/hw/arm/stm32f405_soc.h
@@ -51,11 +51,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(STM32F405State, STM32F405_SOC)
 #define CCM_SIZE (64 * 1024)
 
 struct STM32F405State {
-    /*< private >*/
     SysBusDevice parent_obj;
-    /*< public >*/
-
-    char *cpu_type;
 
     ARMv7MState armv7m;
 
diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
index 515c081605..2e58984947 100644
--- a/hw/arm/netduinoplus2.c
+++ b/hw/arm/netduinoplus2.c
@@ -44,7 +44,6 @@ static void netduinoplus2_init(MachineState *machine)
     clock_set_hz(sysclk, SYSCLK_FRQ);
 
     dev = qdev_new(TYPE_STM32F405_SOC);
-    qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
     qdev_connect_clock_in(dev, "sysclk", sysclk);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
@@ -55,8 +54,14 @@ static void netduinoplus2_init(MachineState *machine)
 
 static void netduinoplus2_machine_init(MachineClass *mc)
 {
+    static const char * const valid_cpu_types[] = {
+        ARM_CPU_TYPE_NAME("cortex-m4"),
+        NULL
+    };
+
     mc->desc = "Netduino Plus 2 Machine (Cortex-M4)";
     mc->init = netduinoplus2_init;
+    mc->valid_cpu_types = valid_cpu_types;
 }
 
 DEFINE_MACHINE("netduinoplus2", netduinoplus2_machine_init)
diff --git a/hw/arm/olimex-stm32-h405.c b/hw/arm/olimex-stm32-h405.c
index 3aa61c91b7..d793de7c97 100644
--- a/hw/arm/olimex-stm32-h405.c
+++ b/hw/arm/olimex-stm32-h405.c
@@ -47,7 +47,6 @@ static void olimex_stm32_h405_init(MachineState *machine)
     clock_set_hz(sysclk, SYSCLK_FRQ);
 
     dev = qdev_new(TYPE_STM32F405_SOC);
-    qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
     qdev_connect_clock_in(dev, "sysclk", sysclk);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
@@ -58,9 +57,14 @@ static void olimex_stm32_h405_init(MachineState *machine)
 
 static void olimex_stm32_h405_machine_init(MachineClass *mc)
 {
+    static const char * const valid_cpu_types[] = {
+        ARM_CPU_TYPE_NAME("cortex-m4"),
+        NULL
+    };
+
     mc->desc = "Olimex STM32-H405 (Cortex-M4)";
     mc->init = olimex_stm32_h405_init;
-    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
+    mc->valid_cpu_types = valid_cpu_types;
 
     /* SRAM pre-allocated as part of the SoC instantiation */
     mc->default_ram_size = 0;
diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
index cef23d7ee4..a65bbe298d 100644
--- a/hw/arm/stm32f405_soc.c
+++ b/hw/arm/stm32f405_soc.c
@@ -149,7 +149,7 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
 
     armv7m = DEVICE(&s->armv7m);
     qdev_prop_set_uint32(armv7m, "num-irq", 96);
-    qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
+    qdev_prop_set_string(armv7m, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
     qdev_prop_set_bit(armv7m, "enable-bitband", true);
     qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
     qdev_connect_clock_in(armv7m, "refclk", s->refclk);
@@ -287,17 +287,11 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
     create_unimplemented_device("RNG",         0x50060800, 0x400);
 }
 
-static Property stm32f405_soc_properties[] = {
-    DEFINE_PROP_STRING("cpu-type", STM32F405State, cpu_type),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void stm32f405_soc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = stm32f405_soc_realize;
-    device_class_set_props(dc, stm32f405_soc_properties);
     /* No vmstate or reset required: device has no internal state */
 }
 
-- 
2.41.0

[PATCH-for-8.2? v2 3/4] hw/arm/stm32f205: Report error when incorrect CPU is used

[Philippe Mathieu-Daudé]

The 'netduino2' machine ignores the CPU type requested by the
command line. This might confuse users, since the following will
create a machine with a Cortex-M3 CPU:

  $ qemu-system-arm -M netduino2 -cpu cortex-a9

Set the MachineClass::valid_cpu_types field (introduced in commit
c9cf636d48 "machine: Add a valid_cpu_types property").
Remove the now unused MachineClass::default_cpu_type field.

We now get:

  $ qemu-system-arm -M netduino2 -cpu cortex-a9
  qemu-system-arm: Invalid CPU type: cortex-a9-arm-cpu
  The valid types are: cortex-m3-arm-cpu

Since the SoC family can only use Cortex-M3 CPUs, hard-code the
CPU type name at the SoC level, removing the QOM property
entirely.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/arm/stm32f205_soc.h | 4 ----
 hw/arm/netduino2.c             | 7 ++++++-
 hw/arm/stm32f205_soc.c         | 9 ++-------
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
index 5a4f776264..4f4c8bbebc 100644
--- a/include/hw/arm/stm32f205_soc.h
+++ b/include/hw/arm/stm32f205_soc.h
@@ -49,11 +49,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(STM32F205State, STM32F205_SOC)
 #define SRAM_SIZE (128 * 1024)
 
 struct STM32F205State {
-    /*< private >*/
     SysBusDevice parent_obj;
-    /*< public >*/
-
-    char *cpu_type;
 
     ARMv7MState armv7m;
 
diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
index 83753d53a3..501f63a77f 100644
--- a/hw/arm/netduino2.c
+++ b/hw/arm/netduino2.c
@@ -44,7 +44,6 @@ static void netduino2_init(MachineState *machine)
     clock_set_hz(sysclk, SYSCLK_FRQ);
 
     dev = qdev_new(TYPE_STM32F205_SOC);
-    qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
     qdev_connect_clock_in(dev, "sysclk", sysclk);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
@@ -54,8 +53,14 @@ static void netduino2_init(MachineState *machine)
 
 static void netduino2_machine_init(MachineClass *mc)
 {
+    static const char * const valid_cpu_types[] = {
+        ARM_CPU_TYPE_NAME("cortex-m3"),
+        NULL
+    };
+
     mc->desc = "Netduino 2 Machine (Cortex-M3)";
     mc->init = netduino2_init;
+    mc->valid_cpu_types = valid_cpu_types;
     mc->ignore_memory_transaction_failures = true;
 }
 
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index c6b75a381d..1a548646f6 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -127,7 +127,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
 
     armv7m = DEVICE(&s->armv7m);
     qdev_prop_set_uint32(armv7m, "num-irq", 96);
-    qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
+    qdev_prop_set_string(armv7m, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
     qdev_prop_set_bit(armv7m, "enable-bitband", true);
     qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
     qdev_connect_clock_in(armv7m, "refclk", s->refclk);
@@ -201,17 +201,12 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
     }
 }
 
-static Property stm32f205_soc_properties[] = {
-    DEFINE_PROP_STRING("cpu-type", STM32F205State, cpu_type),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void stm32f205_soc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = stm32f205_soc_realize;
-    device_class_set_props(dc, stm32f205_soc_properties);
+    /* No vmstate or reset required: device has no internal state */
 }
 
 static const TypeInfo stm32f205_soc_info = {
-- 
2.41.0

[PATCH-for-8.2? v2 4/4] hw/arm/stm32f100: Report error when incorrect CPU is used

[Philippe Mathieu-Daudé]

The 'stm32vldiscovery' machine ignores the CPU type requested by
the command line. This might confuse users, since the following
will create a machine with a Cortex-M3 CPU:

  $ qemu-system-aarch64 -M stm32vldiscovery -cpu neoverse-n1

Set the MachineClass::valid_cpu_types field (introduced in commit
c9cf636d48 "machine: Add a valid_cpu_types property").
Remove the now unused MachineClass::default_cpu_type field.

We now get:

  $ qemu-system-aarch64 -M stm32vldiscovery -cpu neoverse-n1
  qemu-system-aarch64: Invalid CPU type: neoverse-n1-arm-cpu
  The valid types are: cortex-m3-arm-cpu

Since the SoC family can only use Cortex-M3 CPUs, hard-code the
CPU type name at the SoC level, removing the QOM property
entirely.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/arm/stm32f100_soc.h | 4 ----
 hw/arm/stm32f100_soc.c         | 9 ++-------
 hw/arm/stm32vldiscovery.c      | 7 ++++++-
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/include/hw/arm/stm32f100_soc.h b/include/hw/arm/stm32f100_soc.h
index 40cd415b28..a74d7b369c 100644
--- a/include/hw/arm/stm32f100_soc.h
+++ b/include/hw/arm/stm32f100_soc.h
@@ -43,12 +43,8 @@ OBJECT_DECLARE_SIMPLE_TYPE(STM32F100State, STM32F100_SOC)
 #define SRAM_SIZE (8 * 1024)
 
 struct STM32F100State {
-    /*< private >*/
     SysBusDevice parent_obj;
 
-    /*< public >*/
-    char *cpu_type;
-
     ARMv7MState armv7m;
 
     STM32F2XXUsartState usart[STM_NUM_USARTS];
diff --git a/hw/arm/stm32f100_soc.c b/hw/arm/stm32f100_soc.c
index f7b344ba9f..b90d440d7a 100644
--- a/hw/arm/stm32f100_soc.c
+++ b/hw/arm/stm32f100_soc.c
@@ -115,7 +115,7 @@ static void stm32f100_soc_realize(DeviceState *dev_soc, Error **errp)
     /* Init ARMv7m */
     armv7m = DEVICE(&s->armv7m);
     qdev_prop_set_uint32(armv7m, "num-irq", 61);
-    qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
+    qdev_prop_set_string(armv7m, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
     qdev_prop_set_bit(armv7m, "enable-bitband", true);
     qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
     qdev_connect_clock_in(armv7m, "refclk", s->refclk);
@@ -180,17 +180,12 @@ static void stm32f100_soc_realize(DeviceState *dev_soc, Error **errp)
     create_unimplemented_device("CRC",       0x40023000, 0x400);
 }
 
-static Property stm32f100_soc_properties[] = {
-    DEFINE_PROP_STRING("cpu-type", STM32F100State, cpu_type),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void stm32f100_soc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = stm32f100_soc_realize;
-    device_class_set_props(dc, stm32f100_soc_properties);
+    /* No vmstate or reset required: device has no internal state */
 }
 
 static const TypeInfo stm32f100_soc_info = {
diff --git a/hw/arm/stm32vldiscovery.c b/hw/arm/stm32vldiscovery.c
index 67675e952f..190db6118b 100644
--- a/hw/arm/stm32vldiscovery.c
+++ b/hw/arm/stm32vldiscovery.c
@@ -47,7 +47,6 @@ static void stm32vldiscovery_init(MachineState *machine)
     clock_set_hz(sysclk, SYSCLK_FRQ);
 
     dev = qdev_new(TYPE_STM32F100_SOC);
-    qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
     qdev_connect_clock_in(dev, "sysclk", sysclk);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
@@ -58,8 +57,14 @@ static void stm32vldiscovery_init(MachineState *machine)
 
 static void stm32vldiscovery_machine_init(MachineClass *mc)
 {
+    static const char * const valid_cpu_types[] = {
+        ARM_CPU_TYPE_NAME("cortex-m3"),
+        NULL
+    };
+
     mc->desc = "ST STM32VLDISCOVERY (Cortex-M3)";
     mc->init = stm32vldiscovery_init;
+    mc->valid_cpu_types = valid_cpu_types;
 }
 
 DEFINE_MACHINE("stm32vldiscovery", stm32vldiscovery_machine_init)
-- 
2.41.0

Re: [PATCH-for-8.2? v2 2/4] hw/arm/stm32f405: Report error when incorrect CPU is used

[Gavin Shan]

On 11/17/23 17:17, Philippe Mathieu-Daudé wrote:
> Both 'netduinoplus2' and 'olimex-stm32-h405' machines ignore the
> CPU type requested by the command line. This might confuse users,
> since the following will create a machine with a Cortex-M4 CPU:
> 
>    $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f
> 
> Set the MachineClass::valid_cpu_types field (introduced in commit
> c9cf636d48 "machine: Add a valid_cpu_types property").
> Remove the now unused MachineClass::default_cpu_type field.
> 
> We now get:
> 
>    $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f
>    qemu-system-aarch64: Invalid CPU type: cortex-r5f-arm-cpu
>    The valid types are: cortex-m4-arm-cpu
> 
> Since the SoC family can only use Cortex-M4 CPUs, hard-code the
> CPU type name at the SoC level, removing the QOM property
> entirely.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/arm/stm32f405_soc.h | 4 ----
>   hw/arm/netduinoplus2.c         | 7 ++++++-
>   hw/arm/olimex-stm32-h405.c     | 8 ++++++--
>   hw/arm/stm32f405_soc.c         | 8 +-------
>   4 files changed, 13 insertions(+), 14 deletions(-)
> 

Reviewed-by: Gavin Shan <gshan@redhat.com>

Re: [PATCH-for-8.2? v2 3/4] hw/arm/stm32f205: Report error when incorrect CPU is used

[Gavin Shan]

On 11/17/23 17:17, Philippe Mathieu-Daudé wrote:
> The 'netduino2' machine ignores the CPU type requested by the
> command line. This might confuse users, since the following will
> create a machine with a Cortex-M3 CPU:
> 
>    $ qemu-system-arm -M netduino2 -cpu cortex-a9
> 
> Set the MachineClass::valid_cpu_types field (introduced in commit
> c9cf636d48 "machine: Add a valid_cpu_types property").
> Remove the now unused MachineClass::default_cpu_type field.
> 
> We now get:
> 
>    $ qemu-system-arm -M netduino2 -cpu cortex-a9
>    qemu-system-arm: Invalid CPU type: cortex-a9-arm-cpu
>    The valid types are: cortex-m3-arm-cpu
> 
> Since the SoC family can only use Cortex-M3 CPUs, hard-code the
> CPU type name at the SoC level, removing the QOM property
> entirely.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/arm/stm32f205_soc.h | 4 ----
>   hw/arm/netduino2.c             | 7 ++++++-
>   hw/arm/stm32f205_soc.c         | 9 ++-------
>   3 files changed, 8 insertions(+), 12 deletions(-)
> 

Reviewed-by: Gavin Shan <gshan@redhat.com>

Re: [PATCH-for-8.2? v2 4/4] hw/arm/stm32f100: Report error when incorrect CPU is used

[Gavin Shan]

On 11/17/23 17:17, Philippe Mathieu-Daudé wrote:
> The 'stm32vldiscovery' machine ignores the CPU type requested by
> the command line. This might confuse users, since the following
> will create a machine with a Cortex-M3 CPU:
> 
>    $ qemu-system-aarch64 -M stm32vldiscovery -cpu neoverse-n1
> 
> Set the MachineClass::valid_cpu_types field (introduced in commit
> c9cf636d48 "machine: Add a valid_cpu_types property").
> Remove the now unused MachineClass::default_cpu_type field.
> 
> We now get:
> 
>    $ qemu-system-aarch64 -M stm32vldiscovery -cpu neoverse-n1
>    qemu-system-aarch64: Invalid CPU type: neoverse-n1-arm-cpu
>    The valid types are: cortex-m3-arm-cpu
> 
> Since the SoC family can only use Cortex-M3 CPUs, hard-code the
> CPU type name at the SoC level, removing the QOM property
> entirely.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/arm/stm32f100_soc.h | 4 ----
>   hw/arm/stm32f100_soc.c         | 9 ++-------
>   hw/arm/stm32vldiscovery.c      | 7 ++++++-
>   3 files changed, 8 insertions(+), 12 deletions(-)
> 

Reviewed-by: Gavin Shan <gshan@redhat.com>

Re: [PATCH-for-8.2? v2 0/4] hw/arm/stm32xxx: Report error when incorrect CPU is used

[Peter Maydell]

On Fri, 17 Nov 2023 at 07:17, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Series fully reviewed.
>
> Hi,
>
> While reviewing this [*] series from Arnaud and Inès,
> I realized STM32 based boards don't check the requested
> CPU type is correct, and ignore it. Possibly confusing
> users.
> Since these simple boards code is used as template,
> I took the opportunity to sanitize it a bit, using the
> MachineClass::valid_cpu_types which seems simpler (to me)
> for newcomers.
>
> Patches are candidate for 8.2 but this is a long
> standing issue, not a regression from 8.1.
>



Applied to target-arm.next, thanks.

-- PMM

Re: [PATCH-for-8.2? v2 2/4] hw/arm/stm32f405: Report error when incorrect CPU is used

[Igor Mammedov]

On Fri, 17 Nov 2023 08:17:02 +0100
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> Both 'netduinoplus2' and 'olimex-stm32-h405' machines ignore the
> CPU type requested by the command line. This might confuse users,
> since the following will create a machine with a Cortex-M4 CPU:
> 
>   $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f
> 
> Set the MachineClass::valid_cpu_types field (introduced in commit
> c9cf636d48 "machine: Add a valid_cpu_types property").
> Remove the now unused MachineClass::default_cpu_type field.

Why default_cpu_type is removed?
what if user didn't user -cpu at all?

> 
> We now get:
> 
>   $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f
>   qemu-system-aarch64: Invalid CPU type: cortex-r5f-arm-cpu
>   The valid types are: cortex-m4-arm-cpu
> 
> Since the SoC family can only use Cortex-M4 CPUs, hard-code the
> CPU type name at the SoC level, removing the QOM property
> entirely.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/arm/stm32f405_soc.h | 4 ----
>  hw/arm/netduinoplus2.c         | 7 ++++++-
>  hw/arm/olimex-stm32-h405.c     | 8 ++++++--
>  hw/arm/stm32f405_soc.c         | 8 +-------
>  4 files changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/include/hw/arm/stm32f405_soc.h b/include/hw/arm/stm32f405_soc.h
> index c968ce3ab2..d15c03c4b5 100644
> --- a/include/hw/arm/stm32f405_soc.h
> +++ b/include/hw/arm/stm32f405_soc.h
> @@ -51,11 +51,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(STM32F405State, STM32F405_SOC)
>  #define CCM_SIZE (64 * 1024)
>  
>  struct STM32F405State {
> -    /*< private >*/
>      SysBusDevice parent_obj;
> -    /*< public >*/
> -
> -    char *cpu_type;
>  
>      ARMv7MState armv7m;
>  
> diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
> index 515c081605..2e58984947 100644
> --- a/hw/arm/netduinoplus2.c
> +++ b/hw/arm/netduinoplus2.c
> @@ -44,7 +44,6 @@ static void netduinoplus2_init(MachineState *machine)
>      clock_set_hz(sysclk, SYSCLK_FRQ);
>  
>      dev = qdev_new(TYPE_STM32F405_SOC);
> -    qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
>      qdev_connect_clock_in(dev, "sysclk", sysclk);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>  
> @@ -55,8 +54,14 @@ static void netduinoplus2_init(MachineState *machine)
>  
>  static void netduinoplus2_machine_init(MachineClass *mc)
>  {
> +    static const char * const valid_cpu_types[] = {
> +        ARM_CPU_TYPE_NAME("cortex-m4"),
> +        NULL
> +    };
> +
>      mc->desc = "Netduino Plus 2 Machine (Cortex-M4)";
>      mc->init = netduinoplus2_init;
> +    mc->valid_cpu_types = valid_cpu_types;
>  }
>  
>  DEFINE_MACHINE("netduinoplus2", netduinoplus2_machine_init)
> diff --git a/hw/arm/olimex-stm32-h405.c b/hw/arm/olimex-stm32-h405.c
> index 3aa61c91b7..d793de7c97 100644
> --- a/hw/arm/olimex-stm32-h405.c
> +++ b/hw/arm/olimex-stm32-h405.c
> @@ -47,7 +47,6 @@ static void olimex_stm32_h405_init(MachineState *machine)
>      clock_set_hz(sysclk, SYSCLK_FRQ);
>  
>      dev = qdev_new(TYPE_STM32F405_SOC);
> -    qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
>      qdev_connect_clock_in(dev, "sysclk", sysclk);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>  
> @@ -58,9 +57,14 @@ static void olimex_stm32_h405_init(MachineState *machine)
>  
>  static void olimex_stm32_h405_machine_init(MachineClass *mc)
>  {
> +    static const char * const valid_cpu_types[] = {
> +        ARM_CPU_TYPE_NAME("cortex-m4"),
> +        NULL
> +    };
> +
>      mc->desc = "Olimex STM32-H405 (Cortex-M4)";
>      mc->init = olimex_stm32_h405_init;
> -    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
> +    mc->valid_cpu_types = valid_cpu_types;
>  
>      /* SRAM pre-allocated as part of the SoC instantiation */
>      mc->default_ram_size = 0;
> diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
> index cef23d7ee4..a65bbe298d 100644
> --- a/hw/arm/stm32f405_soc.c
> +++ b/hw/arm/stm32f405_soc.c
> @@ -149,7 +149,7 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
>  
>      armv7m = DEVICE(&s->armv7m);
>      qdev_prop_set_uint32(armv7m, "num-irq", 96);
> -    qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
> +    qdev_prop_set_string(armv7m, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
>      qdev_prop_set_bit(armv7m, "enable-bitband", true);
>      qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
>      qdev_connect_clock_in(armv7m, "refclk", s->refclk);
> @@ -287,17 +287,11 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
>      create_unimplemented_device("RNG",         0x50060800, 0x400);
>  }
>  
> -static Property stm32f405_soc_properties[] = {
> -    DEFINE_PROP_STRING("cpu-type", STM32F405State, cpu_type),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
>  static void stm32f405_soc_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
>      dc->realize = stm32f405_soc_realize;
> -    device_class_set_props(dc, stm32f405_soc_properties);
>      /* No vmstate or reset required: device has no internal state */
>  }
>  

Re: [PATCH-for-8.2? v2 2/4] hw/arm/stm32f405: Report error when incorrect CPU is used

[Philippe Mathieu-Daudé]

Hi Igor,

On 24/11/23 14:13, Igor Mammedov wrote:
> On Fri, 17 Nov 2023 08:17:02 +0100
> Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
>> Both 'netduinoplus2' and 'olimex-stm32-h405' machines ignore the
>> CPU type requested by the command line. This might confuse users,
>> since the following will create a machine with a Cortex-M4 CPU:
>>
>>    $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f
>>
>> Set the MachineClass::valid_cpu_types field (introduced in commit
>> c9cf636d48 "machine: Add a valid_cpu_types property").
>> Remove the now unused MachineClass::default_cpu_type field.
> 
> Why default_cpu_type is removed?

"Since the SoC family can only use Cortex-M4 CPUs, hard-code the
  CPU type name at the SoC level"

> what if user didn't user -cpu at all?

The CPU is hardcoded, default value is not used.

qemu-system-arm -M olimex-stm32-h405 -S -monitor stdio
QEMU 8.1.91 monitor - type 'help' for more information
(qemu) info qtree
...
   dev: armv7m, id ""
     gpio-in "NMI" 1
     gpio-out "SYSRESETREQ" 1
     gpio-in "" 96
     clock-in "cpuclk" freq_hz=168 MHz
     clock-in "refclk" freq_hz=21 MHz
     cpu-type = "cortex-m4-arm-cpu"
     ...

> 
>>
>> We now get:
>>
>>    $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f
>>    qemu-system-aarch64: Invalid CPU type: cortex-r5f-arm-cpu
>>    The valid types are: cortex-m4-arm-cpu
>>
>> Since the SoC family can only use Cortex-M4 CPUs, hard-code the
>> CPU type name at the SoC level, removing the QOM property
>> entirely.
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/hw/arm/stm32f405_soc.h | 4 ----
>>   hw/arm/netduinoplus2.c         | 7 ++++++-
>>   hw/arm/olimex-stm32-h405.c     | 8 ++++++--
>>   hw/arm/stm32f405_soc.c         | 8 +-------
>>   4 files changed, 13 insertions(+), 14 deletions(-)

Re: [PATCH-for-8.2? v2 2/4] hw/arm/stm32f405: Report error when incorrect CPU is used

[Alistair Francis]

On Fri, Nov 17, 2023 at 5:18 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Both 'netduinoplus2' and 'olimex-stm32-h405' machines ignore the
> CPU type requested by the command line. This might confuse users,
> since the following will create a machine with a Cortex-M4 CPU:
>
>   $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f
>
> Set the MachineClass::valid_cpu_types field (introduced in commit
> c9cf636d48 "machine: Add a valid_cpu_types property").
> Remove the now unused MachineClass::default_cpu_type field.
>
> We now get:
>
>   $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f
>   qemu-system-aarch64: Invalid CPU type: cortex-r5f-arm-cpu
>   The valid types are: cortex-m4-arm-cpu
>
> Since the SoC family can only use Cortex-M4 CPUs, hard-code the
> CPU type name at the SoC level, removing the QOM property
> entirely.
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/hw/arm/stm32f405_soc.h | 4 ----
>  hw/arm/netduinoplus2.c         | 7 ++++++-
>  hw/arm/olimex-stm32-h405.c     | 8 ++++++--
>  hw/arm/stm32f405_soc.c         | 8 +-------
>  4 files changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/include/hw/arm/stm32f405_soc.h b/include/hw/arm/stm32f405_soc.h
> index c968ce3ab2..d15c03c4b5 100644
> --- a/include/hw/arm/stm32f405_soc.h
> +++ b/include/hw/arm/stm32f405_soc.h
> @@ -51,11 +51,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(STM32F405State, STM32F405_SOC)
>  #define CCM_SIZE (64 * 1024)
>
>  struct STM32F405State {
> -    /*< private >*/
>      SysBusDevice parent_obj;
> -    /*< public >*/
> -
> -    char *cpu_type;
>
>      ARMv7MState armv7m;
>
> diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
> index 515c081605..2e58984947 100644
> --- a/hw/arm/netduinoplus2.c
> +++ b/hw/arm/netduinoplus2.c
> @@ -44,7 +44,6 @@ static void netduinoplus2_init(MachineState *machine)
>      clock_set_hz(sysclk, SYSCLK_FRQ);
>
>      dev = qdev_new(TYPE_STM32F405_SOC);
> -    qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
>      qdev_connect_clock_in(dev, "sysclk", sysclk);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>
> @@ -55,8 +54,14 @@ static void netduinoplus2_init(MachineState *machine)
>
>  static void netduinoplus2_machine_init(MachineClass *mc)
>  {
> +    static const char * const valid_cpu_types[] = {
> +        ARM_CPU_TYPE_NAME("cortex-m4"),
> +        NULL
> +    };
> +
>      mc->desc = "Netduino Plus 2 Machine (Cortex-M4)";
>      mc->init = netduinoplus2_init;
> +    mc->valid_cpu_types = valid_cpu_types;
>  }
>
>  DEFINE_MACHINE("netduinoplus2", netduinoplus2_machine_init)
> diff --git a/hw/arm/olimex-stm32-h405.c b/hw/arm/olimex-stm32-h405.c
> index 3aa61c91b7..d793de7c97 100644
> --- a/hw/arm/olimex-stm32-h405.c
> +++ b/hw/arm/olimex-stm32-h405.c
> @@ -47,7 +47,6 @@ static void olimex_stm32_h405_init(MachineState *machine)
>      clock_set_hz(sysclk, SYSCLK_FRQ);
>
>      dev = qdev_new(TYPE_STM32F405_SOC);
> -    qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
>      qdev_connect_clock_in(dev, "sysclk", sysclk);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>
> @@ -58,9 +57,14 @@ static void olimex_stm32_h405_init(MachineState *machine)
>
>  static void olimex_stm32_h405_machine_init(MachineClass *mc)
>  {
> +    static const char * const valid_cpu_types[] = {
> +        ARM_CPU_TYPE_NAME("cortex-m4"),
> +        NULL
> +    };
> +
>      mc->desc = "Olimex STM32-H405 (Cortex-M4)";
>      mc->init = olimex_stm32_h405_init;
> -    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
> +    mc->valid_cpu_types = valid_cpu_types;
>
>      /* SRAM pre-allocated as part of the SoC instantiation */
>      mc->default_ram_size = 0;
> diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
> index cef23d7ee4..a65bbe298d 100644
> --- a/hw/arm/stm32f405_soc.c
> +++ b/hw/arm/stm32f405_soc.c
> @@ -149,7 +149,7 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
>
>      armv7m = DEVICE(&s->armv7m);
>      qdev_prop_set_uint32(armv7m, "num-irq", 96);
> -    qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
> +    qdev_prop_set_string(armv7m, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
>      qdev_prop_set_bit(armv7m, "enable-bitband", true);
>      qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
>      qdev_connect_clock_in(armv7m, "refclk", s->refclk);
> @@ -287,17 +287,11 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
>      create_unimplemented_device("RNG",         0x50060800, 0x400);
>  }
>
> -static Property stm32f405_soc_properties[] = {
> -    DEFINE_PROP_STRING("cpu-type", STM32F405State, cpu_type),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
>  static void stm32f405_soc_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>
>      dc->realize = stm32f405_soc_realize;
> -    device_class_set_props(dc, stm32f405_soc_properties);
>      /* No vmstate or reset required: device has no internal state */
>  }
>
> --
> 2.41.0
>
>