diff mbox series

[PATCH-for-9.2] hw/avr/arduino: Check for CPU types in machine_run_board_init()

Message ID 20241118130109.7838-1-philmd@linaro.org
State New
Headers show
Series [PATCH-for-9.2] hw/avr/arduino: Check for CPU types in machine_run_board_init() | expand

Commit Message

Philippe Mathieu-Daudé Nov. 18, 2024, 1:01 p.m. UTC 
Leverage the common code introduced in commit c9cf636d48 ("machine:
Add a valid_cpu_types property") to check for the single valid CPU
type. This allows reporting an error for invalid CPUs:

  $ qemu-system-avr -M 2009 -cpu avr51-avr-cpu
  qemu-system-avr: Invalid CPU model: avr51
  The only valid type is: avr5

Reported-by: Iris Artin <iris@artins.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/avr/arduino.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Peter Maydell Nov. 18, 2024, 1:07 p.m. UTC | #1 
On Mon, 18 Nov 2024 at 13:02, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Leverage the common code introduced in commit c9cf636d48 ("machine:
> Add a valid_cpu_types property") to check for the single valid CPU
> type. This allows reporting an error for invalid CPUs:
>
>   $ qemu-system-avr -M 2009 -cpu avr51-avr-cpu
>   qemu-system-avr: Invalid CPU model: avr51
>   The only valid type is: avr5

This is a nice user-convenience feature, but why for 9.2?
We haven't bothered to error-check the user specified CPU
before, and we still don't on many (non-avr) board types.

thanks
-- PMM
Philippe Mathieu-Daudé Nov. 18, 2024, 2:40 p.m. UTC | #2 
On 18/11/24 13:07, Peter Maydell wrote:
> On Mon, 18 Nov 2024 at 13:02, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Leverage the common code introduced in commit c9cf636d48 ("machine:
>> Add a valid_cpu_types property") to check for the single valid CPU
>> type. This allows reporting an error for invalid CPUs:
>>
>>    $ qemu-system-avr -M 2009 -cpu avr51-avr-cpu
>>    qemu-system-avr: Invalid CPU model: avr51
>>    The only valid type is: avr5
> 
> This is a nice user-convenience feature, but why for 9.2?
> We haven't bothered to error-check the user specified CPU
> before, and we still don't on many (non-avr) board types.

OK to postpone for 10.0 then.
Philippe Mathieu-Daudé Dec. 30, 2024, 5 p.m. UTC | #3 
Hi Peter,

On 18/11/24 14:07, Peter Maydell wrote:
> On Mon, 18 Nov 2024 at 13:02, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Leverage the common code introduced in commit c9cf636d48 ("machine:
>> Add a valid_cpu_types property") to check for the single valid CPU
>> type. This allows reporting an error for invalid CPUs:
>>
>>    $ qemu-system-avr -M 2009 -cpu avr51-avr-cpu
>>    qemu-system-avr: Invalid CPU model: avr51
>>    The only valid type is: avr5
> 
> This is a nice user-convenience feature, but why for 9.2?
> We haven't bothered to error-check the user specified CPU
> before, and we still don't on many (non-avr) board types.

This patch was motivated by a confused user:
https://lore.kernel.org/qemu-devel/84975169-3c87-42c8-96e3-7ae724cc4692@linaro.org/

Not checking valid CPUs on boards which only support a limited
set looks like an open gate for more user complains.
IMHO checking them should be the rule, not the exceptions.

Back to this patch, do you object to it?

Regards,

Phil.
diff mbox series

Patch

diff --git a/hw/avr/arduino.c b/hw/avr/arduino.c
index 48ef478346..a8da2728f0 100644
--- a/hw/avr/arduino.c
+++ b/hw/avr/arduino.c
@@ -71,6 +71,10 @@  static void arduino_machine_class_init(ObjectClass *oc, void *data)
 
 static void arduino_duemilanove_class_init(ObjectClass *oc, void *data)
 {
+    static const char * const valid_cpu_types[] = {
+        AVR_CPU_TYPE_NAME("avr5"),
+        NULL
+    };
     MachineClass *mc = MACHINE_CLASS(oc);
     ArduinoMachineClass *amc = ARDUINO_MACHINE_CLASS(oc);
 
@@ -80,12 +84,17 @@  static void arduino_duemilanove_class_init(ObjectClass *oc, void *data)
      */
     mc->desc        = "Arduino Duemilanove (ATmega168)",
     mc->alias       = "2009";
+    mc->valid_cpu_types = valid_cpu_types;
     amc->mcu_type   = TYPE_ATMEGA168_MCU;
     amc->xtal_hz    = 16 * 1000 * 1000;
 };
 
 static void arduino_uno_class_init(ObjectClass *oc, void *data)
 {
+    static const char * const valid_cpu_types[] = {
+        AVR_CPU_TYPE_NAME("avr5"),
+        NULL
+    };
     MachineClass *mc = MACHINE_CLASS(oc);
     ArduinoMachineClass *amc = ARDUINO_MACHINE_CLASS(oc);
 
@@ -95,12 +104,17 @@  static void arduino_uno_class_init(ObjectClass *oc, void *data)
      */
     mc->desc        = "Arduino UNO (ATmega328P)";
     mc->alias       = "uno";
+    mc->valid_cpu_types = valid_cpu_types;
     amc->mcu_type   = TYPE_ATMEGA328_MCU;
     amc->xtal_hz    = 16 * 1000 * 1000;
 };
 
 static void arduino_mega_class_init(ObjectClass *oc, void *data)
 {
+    static const char * const valid_cpu_types[] = {
+        AVR_CPU_TYPE_NAME("avr51"),
+        NULL
+    };
     MachineClass *mc = MACHINE_CLASS(oc);
     ArduinoMachineClass *amc = ARDUINO_MACHINE_CLASS(oc);
 
@@ -110,12 +124,17 @@  static void arduino_mega_class_init(ObjectClass *oc, void *data)
      */
     mc->desc        = "Arduino Mega (ATmega1280)";
     mc->alias       = "mega";
+    mc->valid_cpu_types = valid_cpu_types;
     amc->mcu_type   = TYPE_ATMEGA1280_MCU;
     amc->xtal_hz    = 16 * 1000 * 1000;
 };
 
 static void arduino_mega2560_class_init(ObjectClass *oc, void *data)
 {
+    static const char * const valid_cpu_types[] = {
+        AVR_CPU_TYPE_NAME("avr6"),
+        NULL
+    };
     MachineClass *mc = MACHINE_CLASS(oc);
     ArduinoMachineClass *amc = ARDUINO_MACHINE_CLASS(oc);
 
@@ -125,6 +144,7 @@  static void arduino_mega2560_class_init(ObjectClass *oc, void *data)
      */
     mc->desc        = "Arduino Mega 2560 (ATmega2560)";
     mc->alias       = "mega2560";
+    mc->valid_cpu_types = valid_cpu_types;
     amc->mcu_type   = TYPE_ATMEGA2560_MCU;
     amc->xtal_hz    = 16 * 1000 * 1000; /* CSTCE16M0V53-R0 */
 };