Re: [Qemu-devel] [PATCH 06/19] aspeed: introduce a configurable number of CPU per machine
On 12/06/2019 03:32, Joel Stanley wrote: > On Sat, 25 May 2019 at 15:13, Cédric Le Goater 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 >> --- >> 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(>cpu), sizeof(s->cpu), >> -sc->info->cpu_type, _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(>cpu[i]), >> +sizeof(s->cpu[i]),
Re: [Qemu-devel] [PATCH 06/19] aspeed: introduce a configurable number of CPU per machine
On Sat, 25 May 2019 at 15:13, Cédric Le Goater 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 > --- > 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(>cpu), sizeof(s->cpu), > -sc->info->cpu_type, _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(>cpu[i]), > +sizeof(s->cpu[i]), sc->info->cpu_type, > +_abort, NULL); > +} > > sysbus_init_child_obj(obj, "scu", OBJECT(>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 */ > -
[Qemu-devel] [PATCH 06/19] aspeed: introduce a configurable number of CPU per machine
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 --- 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(>cpu), sizeof(s->cpu), -sc->info->cpu_type, _abort, NULL); +for (i = 0; i < MIN(smp_cpus, ASPEED_CPUS_NUM); i++) { +object_initialize_child(obj, "cpu[*]", OBJECT(>cpu[i]), +sizeof(s->cpu[i]), sc->info->cpu_type, +_abort, NULL); +} sysbus_init_child_obj(obj, "scu", OBJECT(>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(>cpu), true, "realized", ); -if (err) { -error_propagate(errp, err); -return; +for (i = 0; i < smp_cpus; i++) { +object_property_set_bool(OBJECT(>cpu[i]), true, "realized", ); +if (err) { +error_propagate(errp, err); +return; +} } /* SRAM */ -- 2.20.1