Re: [PATCH v5 30/31] hw/arm: Check CPU type in machine_run_board_init()

[Philippe Mathieu-Daudé]

Hi Gavin,

On 15/11/23 00:56, Gavin Shan wrote:
> Set mc->valid_cpu_types so that the user specified CPU type can
> be validated in machine_run_board_init(). We needn't to do it by
> ourselves.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   hw/arm/bananapi_m2u.c   | 12 ++++++------
>   hw/arm/cubieboard.c     | 12 ++++++------
>   hw/arm/mps2-tz.c        | 20 ++++++++++++++------
>   hw/arm/mps2.c           | 25 +++++++++++++++++++------
>   hw/arm/msf2-som.c       | 12 ++++++------
>   hw/arm/musca.c          | 13 ++++++-------
>   hw/arm/npcm7xx_boards.c | 13 ++++++-------
>   hw/arm/orangepi.c       | 12 ++++++------
>   8 files changed, 69 insertions(+), 50 deletions(-)


> diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
> index 668db5ed61..1c88e76a22 100644
> --- a/hw/arm/mps2-tz.c
> +++ b/hw/arm/mps2-tz.c
> @@ -191,6 +191,16 @@ OBJECT_DECLARE_TYPE(MPS2TZMachineState, 
> MPS2TZMachineClass, MPS2TZ_MACHINE)
>   /* For cpu{0,1}_mpu_{ns,s}, means "leave at SSE's default value" */
>   #define MPU_REGION_DEFAULT UINT32_MAX
>   
> +static const char * const valid_cpu_types[] = {

Generic variable name,

> +    ARM_CPU_TYPE_NAME("cortex-m33"),
> +    NULL
> +};
> +
> +static const char * const mps3tz_an547_valid_cpu_types[] = {

then specific, is a bit confusing.

Better declare the array in the machine_class_init() methods ...

> +    ARM_CPU_TYPE_NAME("cortex-m55"),
> +    NULL
> +};
> +
>   static const uint32_t an505_oscclk[] = {

>       40000000,
>       24580000,
> @@ -813,12 +823,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);
> @@ -1325,6 +1329,7 @@ static void mps2tz_an505_class_init(ObjectClass *oc, void 
> *data)

... here. See for example
https://lore.kernel.org/qemu-devel/20231115232154.4515-2-philmd@linaro.org/.

>       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;
> @@ -1354,6 +1359,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;
> @@ -1383,6 +1389,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;
> @@ -1417,6 +1424,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 = mps3tz_an547_valid_cpu_types;
>       mmc->scc_id = 0x41055470;
>       mmc->sysclk_frq = 32 * 1000 * 1000; /* 32MHz */
>       mmc->apb_periph_frq = 25 * 1000 * 1000; /* 25MHz */