Re: [PATCH 6/8] hw/arm/aspeed/2600: Check for CPU types in machine_run_board_init()

[Cédric Le Goater]

On 1/23/24 07:38, Philippe Mathieu-Daudé wrote:
> Restrict MachineClass::valid_cpu_types[] to the single
> valid CPU type.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/aspeed.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index df627096d2..393c97d55e 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -1157,6 +1157,11 @@ static const char * const ast2500_a1_valid_cpu_types[] = 
> {
>       NULL
>   };
>   
> +static const char * const ast2600_a3_valid_cpu_types[] = {
> +    ARM_CPU_TYPE_NAME("cortex-a9"),

This should be "cortex-a7"

Looking closer, the CPU information is under AspeedSoCClass. Why not build the
valid_cpu_types array with something like :

    struct AspeedMachineClass {
        ...
        const char *valid_cpu_types[2];
    };

    static void aspeed_machine_set_valid_cpu_types(MachineClass *mc)
    {
        AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(mc);
        AspeedSoCClass *sc = 
ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name));
    
        amc->valid_cpu_types[0] = sc->cpu_type;
        amc->valid_cpu_types[1] = NULL;
        mc->valid_cpu_types = amc->valid_cpu_types;
    };


Thanks,

C.



> +    NULL
> +};
> +
>   static void aspeed_machine_class_init(ObjectClass *oc, void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> @@ -1373,6 +1378,7 @@ static void 
> aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
>       amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON | ASPEED_MAC2_ON |
>                        ASPEED_MAC3_ON;
>       amc->i2c_init  = ast2600_evb_i2c_init;
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = 1 * GiB;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);
> @@ -1392,6 +1398,7 @@ static void aspeed_machine_tacoma_class_init(ObjectClass 
> *oc, void *data)
>       amc->num_cs    = 2;
>       amc->macs_mask  = ASPEED_MAC2_ON;
>       amc->i2c_init  = witherspoon_bmc_i2c_init; /* Same board layout */
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = 1 * GiB;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);
> @@ -1449,6 +1456,7 @@ static void aspeed_machine_rainier_class_init(ObjectClass 
> *oc, void *data)
>       amc->num_cs    = 2;
>       amc->macs_mask  = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
>       amc->i2c_init  = rainier_bmc_i2c_init;
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = 1 * GiB;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);
> @@ -1471,6 +1479,7 @@ static void aspeed_machine_fuji_class_init(ObjectClass 
> *oc, void *data)
>       amc->macs_mask = ASPEED_MAC3_ON;
>       amc->i2c_init = fuji_bmc_i2c_init;
>       amc->uart_default = ASPEED_DEV_UART1;
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = FUJI_BMC_RAM_SIZE;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);
> @@ -1492,6 +1501,7 @@ static void 
> aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
>       amc->num_cs    = 2;
>       amc->macs_mask = ASPEED_MAC2_ON;
>       amc->i2c_init  = bletchley_bmc_i2c_init;
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = BLETCHLEY_BMC_RAM_SIZE;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);
> @@ -1631,6 +1641,7 @@ static void 
> aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
>       amc->num_cs    = 2;
>       amc->macs_mask = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
>       amc->i2c_init  = qcom_dc_scm_bmc_i2c_init;
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = 1 * GiB;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);
> @@ -1651,6 +1662,7 @@ static void 
> aspeed_machine_qcom_firework_class_init(ObjectClass *oc,
>       amc->num_cs    = 2;
>       amc->macs_mask = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
>       amc->i2c_init  = qcom_dc_scm_firework_i2c_init;
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = 1 * GiB;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);