| Message ID | 20190525151241.5017-7-clg@kaod.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | aspeed: machine extensions and fixes | expand |
On Sat, 25 May 2019 at 15:13, Cédric Le Goater <clg@kaod.org> wrote: > > The current models of the Aspeed SoCs only have one CPU but future > ones will support SMP. Introduce a way to configure the maximum number > of CPU per machine. SMP support will be activated when models for such > SoCs are implemented. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > include/hw/arm/aspeed.h | 1 + > include/hw/arm/aspeed_soc.h | 3 ++- > hw/arm/aspeed.c | 8 ++++++-- > hw/arm/aspeed_soc.c | 17 +++++++++++------ > 4 files changed, 20 insertions(+), 9 deletions(-) > > diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h > index 02073a6b4d61..f2f238ea83cc 100644 > --- a/include/hw/arm/aspeed.h > +++ b/include/hw/arm/aspeed.h > @@ -23,6 +23,7 @@ typedef struct AspeedBoardConfig { > uint32_t num_cs; > void (*i2c_init)(AspeedBoardState *bmc); > uint32_t ram; > + uint32_t num_cpus; > } AspeedBoardConfig; > > #define TYPE_ASPEED_MACHINE MACHINE_TYPE_NAME("aspeed") > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h > index fa0ba957a611..7247f6da2505 100644 > --- a/include/hw/arm/aspeed_soc.h > +++ b/include/hw/arm/aspeed_soc.h > @@ -24,13 +24,14 @@ > > #define ASPEED_SPIS_NUM 2 > #define ASPEED_WDTS_NUM 3 > +#define ASPEED_CPUS_NUM 2 > > typedef struct AspeedSoCState { > /*< private >*/ > DeviceState parent; > > /*< public >*/ > - ARMCPU cpu; > + ARMCPU cpu[ASPEED_CPUS_NUM]; > MemoryRegion sram; > AspeedVICState vic; > AspeedRtcState rtc; > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index 10ba3f50481a..004b0c318951 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -29,7 +29,6 @@ > > static struct arm_boot_info aspeed_board_binfo = { > .board_id = -1, /* device-tree-only board */ > - .nb_cpus = 1, > }; > > struct AspeedBoardState { > @@ -231,6 +230,7 @@ static void aspeed_board_init(MachineState *machine, > aspeed_board_binfo.kernel_cmdline = machine->kernel_cmdline; > aspeed_board_binfo.ram_size = ram_size; > aspeed_board_binfo.loader_start = sc->info->memmap[ASPEED_SDRAM]; > + aspeed_board_binfo.nb_cpus = cfg->num_cpus; > > if (cfg->i2c_init) { > cfg->i2c_init(bmc); > @@ -327,7 +327,7 @@ static void aspeed_machine_class_init(ObjectClass *oc, void *data) > > mc->desc = board->desc; > mc->init = aspeed_machine_init; > - mc->max_cpus = 1; > + mc->max_cpus = ASPEED_CPUS_NUM; > mc->no_sdcard = 1; > mc->no_floppy = 1; > mc->no_cdrom = 1; > @@ -357,6 +357,7 @@ static const AspeedBoardConfig aspeed_boards[] = { > .num_cs = 1, > .i2c_init = palmetto_bmc_i2c_init, > .ram = 256 * MiB, > + .num_cpus = 1, > }, { > .name = MACHINE_TYPE_NAME("ast2500-evb"), > .desc = "Aspeed AST2500 EVB (ARM1176)", > @@ -367,6 +368,7 @@ static const AspeedBoardConfig aspeed_boards[] = { > .num_cs = 1, > .i2c_init = ast2500_evb_i2c_init, > .ram = 512 * MiB, > + .num_cpus = 1, > }, { > .name = MACHINE_TYPE_NAME("romulus-bmc"), > .desc = "OpenPOWER Romulus BMC (ARM1176)", > @@ -377,6 +379,7 @@ static const AspeedBoardConfig aspeed_boards[] = { > .num_cs = 2, > .i2c_init = romulus_bmc_i2c_init, > .ram = 512 * MiB, > + .num_cpus = 1, > }, { > .name = MACHINE_TYPE_NAME("witherspoon-bmc"), > .desc = "OpenPOWER Witherspoon BMC (ARM1176)", > @@ -387,6 +390,7 @@ static const AspeedBoardConfig aspeed_boards[] = { > .num_cs = 2, > .i2c_init = witherspoon_bmc_i2c_init, > .ram = 512 * MiB, > + .num_cpus = 1, > }, > }; > > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c > index d1dc8f03f35c..b983d5efc5d1 100644 > --- a/hw/arm/aspeed_soc.c > +++ b/hw/arm/aspeed_soc.c > @@ -172,8 +172,11 @@ static void aspeed_soc_init(Object *obj) > AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s); > int i; > > - object_initialize_child(obj, "cpu", OBJECT(&s->cpu), sizeof(s->cpu), > - sc->info->cpu_type, &error_abort, NULL); > + for (i = 0; i < MIN(smp_cpus, ASPEED_CPUS_NUM); i++) { What's the intent of this test? If we're checking that the user hasn't requested more CPUs that the SoC supports, shoudln't it be testing against ->num_cpus instead of ASPEED_CPUS_NUM? > + object_initialize_child(obj, "cpu[*]", OBJECT(&s->cpu[i]), > + sizeof(s->cpu[i]), sc->info->cpu_type, > + &error_abort, NULL); > + } > > sysbus_init_child_obj(obj, "scu", OBJECT(&s->scu), sizeof(s->scu), > TYPE_ASPEED_SCU); > @@ -242,10 +245,12 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) > ASPEED_SOC_IOMEM_SIZE); > > /* CPU */ > - object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err); > - if (err) { > - error_propagate(errp, err); > - return; > + for (i = 0; i < smp_cpus; i++) { Can you explain why we use smp_cpus instead of ->num_cpus? > + object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > } > > /* SRAM */ > -- > 2.20.1 >
On 12/06/2019 03:32, Joel Stanley wrote: > On Sat, 25 May 2019 at 15:13, Cédric Le Goater <clg@kaod.org> wrote: >> >> The current models of the Aspeed SoCs only have one CPU but future >> ones will support SMP. Introduce a way to configure the maximum number >> of CPU per machine. SMP support will be activated when models for such >> SoCs are implemented. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> include/hw/arm/aspeed.h | 1 + >> include/hw/arm/aspeed_soc.h | 3 ++- >> hw/arm/aspeed.c | 8 ++++++-- >> hw/arm/aspeed_soc.c | 17 +++++++++++------ >> 4 files changed, 20 insertions(+), 9 deletions(-) >> >> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h >> index 02073a6b4d61..f2f238ea83cc 100644 >> --- a/include/hw/arm/aspeed.h >> +++ b/include/hw/arm/aspeed.h >> @@ -23,6 +23,7 @@ typedef struct AspeedBoardConfig { >> uint32_t num_cs; >> void (*i2c_init)(AspeedBoardState *bmc); >> uint32_t ram; >> + uint32_t num_cpus; >> } AspeedBoardConfig; >> >> #define TYPE_ASPEED_MACHINE MACHINE_TYPE_NAME("aspeed") >> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h >> index fa0ba957a611..7247f6da2505 100644 >> --- a/include/hw/arm/aspeed_soc.h >> +++ b/include/hw/arm/aspeed_soc.h >> @@ -24,13 +24,14 @@ >> >> #define ASPEED_SPIS_NUM 2 >> #define ASPEED_WDTS_NUM 3 >> +#define ASPEED_CPUS_NUM 2 >> >> typedef struct AspeedSoCState { >> /*< private >*/ >> DeviceState parent; >> >> /*< public >*/ >> - ARMCPU cpu; >> + ARMCPU cpu[ASPEED_CPUS_NUM]; >> MemoryRegion sram; >> AspeedVICState vic; >> AspeedRtcState rtc; >> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c >> index 10ba3f50481a..004b0c318951 100644 >> --- a/hw/arm/aspeed.c >> +++ b/hw/arm/aspeed.c >> @@ -29,7 +29,6 @@ >> >> static struct arm_boot_info aspeed_board_binfo = { >> .board_id = -1, /* device-tree-only board */ >> - .nb_cpus = 1, >> }; >> >> struct AspeedBoardState { >> @@ -231,6 +230,7 @@ static void aspeed_board_init(MachineState *machine, >> aspeed_board_binfo.kernel_cmdline = machine->kernel_cmdline; >> aspeed_board_binfo.ram_size = ram_size; >> aspeed_board_binfo.loader_start = sc->info->memmap[ASPEED_SDRAM]; >> + aspeed_board_binfo.nb_cpus = cfg->num_cpus; >> >> if (cfg->i2c_init) { >> cfg->i2c_init(bmc); >> @@ -327,7 +327,7 @@ static void aspeed_machine_class_init(ObjectClass *oc, void *data) >> >> mc->desc = board->desc; >> mc->init = aspeed_machine_init; >> - mc->max_cpus = 1; >> + mc->max_cpus = ASPEED_CPUS_NUM; >> mc->no_sdcard = 1; >> mc->no_floppy = 1; >> mc->no_cdrom = 1; >> @@ -357,6 +357,7 @@ static const AspeedBoardConfig aspeed_boards[] = { >> .num_cs = 1, >> .i2c_init = palmetto_bmc_i2c_init, >> .ram = 256 * MiB, >> + .num_cpus = 1, >> }, { >> .name = MACHINE_TYPE_NAME("ast2500-evb"), >> .desc = "Aspeed AST2500 EVB (ARM1176)", >> @@ -367,6 +368,7 @@ static const AspeedBoardConfig aspeed_boards[] = { >> .num_cs = 1, >> .i2c_init = ast2500_evb_i2c_init, >> .ram = 512 * MiB, >> + .num_cpus = 1, >> }, { >> .name = MACHINE_TYPE_NAME("romulus-bmc"), >> .desc = "OpenPOWER Romulus BMC (ARM1176)", >> @@ -377,6 +379,7 @@ static const AspeedBoardConfig aspeed_boards[] = { >> .num_cs = 2, >> .i2c_init = romulus_bmc_i2c_init, >> .ram = 512 * MiB, >> + .num_cpus = 1, >> }, { >> .name = MACHINE_TYPE_NAME("witherspoon-bmc"), >> .desc = "OpenPOWER Witherspoon BMC (ARM1176)", >> @@ -387,6 +390,7 @@ static const AspeedBoardConfig aspeed_boards[] = { >> .num_cs = 2, >> .i2c_init = witherspoon_bmc_i2c_init, >> .ram = 512 * MiB, >> + .num_cpus = 1, >> }, >> }; >> >> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c >> index d1dc8f03f35c..b983d5efc5d1 100644 >> --- a/hw/arm/aspeed_soc.c >> +++ b/hw/arm/aspeed_soc.c >> @@ -172,8 +172,11 @@ static void aspeed_soc_init(Object *obj) >> AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s); >> int i; >> >> - object_initialize_child(obj, "cpu", OBJECT(&s->cpu), sizeof(s->cpu), >> - sc->info->cpu_type, &error_abort, NULL); >> + for (i = 0; i < MIN(smp_cpus, ASPEED_CPUS_NUM); i++) { > > What's the intent of this test? > > If we're checking that the user hasn't requested more CPUs that the > SoC supports, shoudln't it be testing against ->num_cpus instead of > ASPEED_CPUS_NUM? yes but we are in the SoC and ->num_cpus is currently a board level information. As we are in the _init routine I can't use a SoC property. This can be improved. I will send a v2. Thanks, C. > >> + object_initialize_child(obj, "cpu[*]", OBJECT(&s->cpu[i]), >> + sizeof(s->cpu[i]), sc->info->cpu_type, >> + &error_abort, NULL); >> + } >> >> sysbus_init_child_obj(obj, "scu", OBJECT(&s->scu), sizeof(s->scu), >> TYPE_ASPEED_SCU); >> @@ -242,10 +245,12 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) >> ASPEED_SOC_IOMEM_SIZE); >> >> /* CPU */ >> - object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err); >> - if (err) { >> - error_propagate(errp, err); >> - return; >> + for (i = 0; i < smp_cpus; i++) { > > Can you explain why we use smp_cpus instead of ->num_cpus? > >> + object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err); >> + if (err) { >> + error_propagate(errp, err); >> + return; >> + } >> } >> >> /* SRAM */ >> -- >> 2.20.1 >>
diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h index 02073a6b4d61..f2f238ea83cc 100644 --- a/include/hw/arm/aspeed.h +++ b/include/hw/arm/aspeed.h @@ -23,6 +23,7 @@ typedef struct AspeedBoardConfig { uint32_t num_cs; void (*i2c_init)(AspeedBoardState *bmc); uint32_t ram; + uint32_t num_cpus; } AspeedBoardConfig; #define TYPE_ASPEED_MACHINE MACHINE_TYPE_NAME("aspeed") diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h index fa0ba957a611..7247f6da2505 100644 --- a/include/hw/arm/aspeed_soc.h +++ b/include/hw/arm/aspeed_soc.h @@ -24,13 +24,14 @@ #define ASPEED_SPIS_NUM 2 #define ASPEED_WDTS_NUM 3 +#define ASPEED_CPUS_NUM 2 typedef struct AspeedSoCState { /*< private >*/ DeviceState parent; /*< public >*/ - ARMCPU cpu; + ARMCPU cpu[ASPEED_CPUS_NUM]; MemoryRegion sram; AspeedVICState vic; AspeedRtcState rtc; diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 10ba3f50481a..004b0c318951 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -29,7 +29,6 @@ static struct arm_boot_info aspeed_board_binfo = { .board_id = -1, /* device-tree-only board */ - .nb_cpus = 1, }; struct AspeedBoardState { @@ -231,6 +230,7 @@ static void aspeed_board_init(MachineState *machine, aspeed_board_binfo.kernel_cmdline = machine->kernel_cmdline; aspeed_board_binfo.ram_size = ram_size; aspeed_board_binfo.loader_start = sc->info->memmap[ASPEED_SDRAM]; + aspeed_board_binfo.nb_cpus = cfg->num_cpus; if (cfg->i2c_init) { cfg->i2c_init(bmc); @@ -327,7 +327,7 @@ static void aspeed_machine_class_init(ObjectClass *oc, void *data) mc->desc = board->desc; mc->init = aspeed_machine_init; - mc->max_cpus = 1; + mc->max_cpus = ASPEED_CPUS_NUM; mc->no_sdcard = 1; mc->no_floppy = 1; mc->no_cdrom = 1; @@ -357,6 +357,7 @@ static const AspeedBoardConfig aspeed_boards[] = { .num_cs = 1, .i2c_init = palmetto_bmc_i2c_init, .ram = 256 * MiB, + .num_cpus = 1, }, { .name = MACHINE_TYPE_NAME("ast2500-evb"), .desc = "Aspeed AST2500 EVB (ARM1176)", @@ -367,6 +368,7 @@ static const AspeedBoardConfig aspeed_boards[] = { .num_cs = 1, .i2c_init = ast2500_evb_i2c_init, .ram = 512 * MiB, + .num_cpus = 1, }, { .name = MACHINE_TYPE_NAME("romulus-bmc"), .desc = "OpenPOWER Romulus BMC (ARM1176)", @@ -377,6 +379,7 @@ static const AspeedBoardConfig aspeed_boards[] = { .num_cs = 2, .i2c_init = romulus_bmc_i2c_init, .ram = 512 * MiB, + .num_cpus = 1, }, { .name = MACHINE_TYPE_NAME("witherspoon-bmc"), .desc = "OpenPOWER Witherspoon BMC (ARM1176)", @@ -387,6 +390,7 @@ static const AspeedBoardConfig aspeed_boards[] = { .num_cs = 2, .i2c_init = witherspoon_bmc_i2c_init, .ram = 512 * MiB, + .num_cpus = 1, }, }; diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index d1dc8f03f35c..b983d5efc5d1 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -172,8 +172,11 @@ static void aspeed_soc_init(Object *obj) AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s); int i; - object_initialize_child(obj, "cpu", OBJECT(&s->cpu), sizeof(s->cpu), - sc->info->cpu_type, &error_abort, NULL); + for (i = 0; i < MIN(smp_cpus, ASPEED_CPUS_NUM); i++) { + object_initialize_child(obj, "cpu[*]", OBJECT(&s->cpu[i]), + sizeof(s->cpu[i]), sc->info->cpu_type, + &error_abort, NULL); + } sysbus_init_child_obj(obj, "scu", OBJECT(&s->scu), sizeof(s->scu), TYPE_ASPEED_SCU); @@ -242,10 +245,12 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) ASPEED_SOC_IOMEM_SIZE); /* CPU */ - object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err); - if (err) { - error_propagate(errp, err); - return; + for (i = 0; i < smp_cpus; i++) { + object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err); + if (err) { + error_propagate(errp, err); + return; + } } /* SRAM */
The current models of the Aspeed SoCs only have one CPU but future ones will support SMP. Introduce a way to configure the maximum number of CPU per machine. SMP support will be activated when models for such SoCs are implemented. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- include/hw/arm/aspeed.h | 1 + include/hw/arm/aspeed_soc.h | 3 ++- hw/arm/aspeed.c | 8 ++++++-- hw/arm/aspeed_soc.c | 17 +++++++++++------ 4 files changed, 20 insertions(+), 9 deletions(-)