| Message ID | 1473167287-8326-7-git-send-email-peter.maydell@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 6 September 2016 at 14:07, Peter Maydell <peter.maydell@linaro.org> wrote: > From: Cédric Le Goater <clg@kaod.org> > > Let's define an object class for each Aspeed SoC we support. A > AspeedSoCInfo struct gathers the SoC specifications which can later be > used by an instance of the class or by a board using the SoC. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > Reviewed-by: Andrew Jeffery <andrew@aj.id.au> > Message-id: 1473055209-18864-4-git-send-email-clg@kaod.org > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > @@ -222,7 +232,18 @@ static const TypeInfo aspeed_soc_type_info = { > > static void aspeed_soc_register_types(void) > { > + int i; > + > type_register_static(&aspeed_soc_type_info); > + for (i = 0; i < ARRAY_SIZE(aspeed_socs); ++i) { > + TypeInfo ti = { > + .name = aspeed_socs[i].name, > + .parent = TYPE_ASPEED_SOC, > + .class_init = aspeed_soc_class_init, This is kind of odd, because you now have the same class init function for both the subclass and the parent class. I would suggest adding ".abstract = true" to the TypeInfo for what is now your parent class (TYPE_ASPEED_SOC), so that it can't be accidentally created directly, and then you can drop its class_init entirely (letting the class init for the subclasses do the work). > + .class_data = (void *) &aspeed_socs[i], > + }; > + type_register(&ti); > + } > } > > type_init(aspeed_soc_register_types) > diff --git a/hw/arm/palmetto-bmc.c b/hw/arm/palmetto-bmc.c > index 4d11905..4319121 100644 > --- a/hw/arm/palmetto-bmc.c > +++ b/hw/arm/palmetto-bmc.c > @@ -22,8 +22,7 @@ > #include "sysemu/blockdev.h" > > static struct arm_boot_info palmetto_bmc_binfo = { > - .loader_start = AST2400_SDRAM_BASE, > - .board_id = 0, > + .board_id = -1, /* device-tree-only board */ > .nb_cpus = 1, > }; > > @@ -61,14 +60,17 @@ static void palmetto_bmc_init_flashes(AspeedSMCState *s, const char *flashtype, > static void palmetto_bmc_init(MachineState *machine) > { > PalmettoBMCState *bmc; > + AspeedSoCClass *sc; > > bmc = g_new0(PalmettoBMCState, 1); > - object_initialize(&bmc->soc, (sizeof(bmc->soc)), TYPE_ASPEED_SOC); > + object_initialize(&bmc->soc, (sizeof(bmc->soc)), "ast2400-a0"); > object_property_add_child(OBJECT(machine), "soc", OBJECT(&bmc->soc), > &error_abort); > > + sc = ASPEED_SOC_GET_CLASS(&bmc->soc); > + > memory_region_allocate_system_memory(&bmc->ram, NULL, "ram", ram_size); > - memory_region_add_subregion(get_system_memory(), AST2400_SDRAM_BASE, > + memory_region_add_subregion(get_system_memory(), sc->info->sdram_base, > &bmc->ram); > object_property_add_const_link(OBJECT(&bmc->soc), "ram", OBJECT(&bmc->ram), > &error_abort); > @@ -84,6 +86,8 @@ static void palmetto_bmc_init(MachineState *machine) > palmetto_bmc_binfo.initrd_filename = machine->initrd_filename; > palmetto_bmc_binfo.kernel_cmdline = machine->kernel_cmdline; > palmetto_bmc_binfo.ram_size = ram_size; > + palmetto_bmc_binfo.loader_start = sc->info->sdram_base; > + > arm_load_kernel(ARM_CPU(first_cpu), &palmetto_bmc_binfo); > } > > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h > index bf63ae9..0146a2a 100644 > --- a/include/hw/arm/aspeed_soc.h > +++ b/include/hw/arm/aspeed_soc.h > @@ -39,6 +39,21 @@ typedef struct AspeedSoCState { > #define TYPE_ASPEED_SOC "aspeed-soc" > #define ASPEED_SOC(obj) OBJECT_CHECK(AspeedSoCState, (obj), TYPE_ASPEED_SOC) > > -#define AST2400_SDRAM_BASE 0x40000000 > +typedef struct AspeedSoCInfo { > + const char *name; > + const char *cpu_model; > + uint32_t silicon_rev; > + hwaddr sdram_base; > +} AspeedSoCInfo; > + > +typedef struct AspeedSoCClass { > + DeviceState parent_class; This is wrong -- the first thing in the subclass's Class struct should be the Class struct of the parent type (here DeviceClass), not the state struct of the parent type. In particular on 32-bit hosts this means that the assignment to sc->info in aspeed_soc_class_init() is actually writing to where dc->props should be, and QEMU then dumps core in device_initfn() when it tries to use dc->props and it's garbage. Fixing this seems to cure the segfault, but I think all things considered I'm going to declare the series insufficiently baked and drop it from target-arm.next. Please can you fix these issues, retest and send a new version? thanks -- PMM
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index 1bec478..ec6ec35 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -37,6 +37,13 @@ static const int uart_irqs[] = { 9, 32, 33, 34, 10 }; static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, }; +#define AST2400_SDRAM_BASE 0x40000000 + +static const AspeedSoCInfo aspeed_socs[] = { + { "ast2400-a0", "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE }, + { "ast2400", "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE }, +}; + /* * IO handlers: simply catch any reads/writes to IO addresses that aren't * handled by a device mapping. @@ -65,8 +72,9 @@ static const MemoryRegionOps aspeed_soc_io_ops = { static void aspeed_soc_init(Object *obj) { AspeedSoCState *s = ASPEED_SOC(obj); + AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s); - s->cpu = cpu_arm_init("arm926"); + s->cpu = cpu_arm_init(sc->info->cpu_model); object_initialize(&s->vic, sizeof(s->vic), TYPE_ASPEED_VIC); object_property_add_child(obj, "vic", OBJECT(&s->vic), NULL); @@ -84,7 +92,7 @@ static void aspeed_soc_init(Object *obj) object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL); qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default()); qdev_prop_set_uint32(DEVICE(&s->scu), "silicon-rev", - AST2400_A0_SILICON_REV); + sc->info->silicon_rev); object_property_add_alias(obj, "hw-strap1", OBJECT(&s->scu), "hw-strap1", &error_abort); object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu), @@ -102,7 +110,7 @@ static void aspeed_soc_init(Object *obj) object_property_add_child(obj, "sdmc", OBJECT(&s->sdmc), NULL); qdev_set_parent_bus(DEVICE(&s->sdmc), sysbus_get_default()); qdev_prop_set_uint32(DEVICE(&s->sdmc), "silicon-rev", - AST2400_A0_SILICON_REV); + sc->info->silicon_rev); } static void aspeed_soc_realize(DeviceState *dev, Error **errp) @@ -202,7 +210,9 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) static void aspeed_soc_class_init(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); + AspeedSoCClass *sc = ASPEED_SOC_CLASS(oc); + sc->info = (AspeedSoCInfo *) data; dc->realize = aspeed_soc_realize; /* @@ -222,7 +232,18 @@ static const TypeInfo aspeed_soc_type_info = { static void aspeed_soc_register_types(void) { + int i; + type_register_static(&aspeed_soc_type_info); + for (i = 0; i < ARRAY_SIZE(aspeed_socs); ++i) { + TypeInfo ti = { + .name = aspeed_socs[i].name, + .parent = TYPE_ASPEED_SOC, + .class_init = aspeed_soc_class_init, + .class_data = (void *) &aspeed_socs[i], + }; + type_register(&ti); + } } type_init(aspeed_soc_register_types) diff --git a/hw/arm/palmetto-bmc.c b/hw/arm/palmetto-bmc.c index 4d11905..4319121 100644 --- a/hw/arm/palmetto-bmc.c +++ b/hw/arm/palmetto-bmc.c @@ -22,8 +22,7 @@ #include "sysemu/blockdev.h" static struct arm_boot_info palmetto_bmc_binfo = { - .loader_start = AST2400_SDRAM_BASE, - .board_id = 0, + .board_id = -1, /* device-tree-only board */ .nb_cpus = 1, }; @@ -61,14 +60,17 @@ static void palmetto_bmc_init_flashes(AspeedSMCState *s, const char *flashtype, static void palmetto_bmc_init(MachineState *machine) { PalmettoBMCState *bmc; + AspeedSoCClass *sc; bmc = g_new0(PalmettoBMCState, 1); - object_initialize(&bmc->soc, (sizeof(bmc->soc)), TYPE_ASPEED_SOC); + object_initialize(&bmc->soc, (sizeof(bmc->soc)), "ast2400-a0"); object_property_add_child(OBJECT(machine), "soc", OBJECT(&bmc->soc), &error_abort); + sc = ASPEED_SOC_GET_CLASS(&bmc->soc); + memory_region_allocate_system_memory(&bmc->ram, NULL, "ram", ram_size); - memory_region_add_subregion(get_system_memory(), AST2400_SDRAM_BASE, + memory_region_add_subregion(get_system_memory(), sc->info->sdram_base, &bmc->ram); object_property_add_const_link(OBJECT(&bmc->soc), "ram", OBJECT(&bmc->ram), &error_abort); @@ -84,6 +86,8 @@ static void palmetto_bmc_init(MachineState *machine) palmetto_bmc_binfo.initrd_filename = machine->initrd_filename; palmetto_bmc_binfo.kernel_cmdline = machine->kernel_cmdline; palmetto_bmc_binfo.ram_size = ram_size; + palmetto_bmc_binfo.loader_start = sc->info->sdram_base; + arm_load_kernel(ARM_CPU(first_cpu), &palmetto_bmc_binfo); } diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h index bf63ae9..0146a2a 100644 --- a/include/hw/arm/aspeed_soc.h +++ b/include/hw/arm/aspeed_soc.h @@ -39,6 +39,21 @@ typedef struct AspeedSoCState { #define TYPE_ASPEED_SOC "aspeed-soc" #define ASPEED_SOC(obj) OBJECT_CHECK(AspeedSoCState, (obj), TYPE_ASPEED_SOC) -#define AST2400_SDRAM_BASE 0x40000000 +typedef struct AspeedSoCInfo { + const char *name; + const char *cpu_model; + uint32_t silicon_rev; + hwaddr sdram_base; +} AspeedSoCInfo; + +typedef struct AspeedSoCClass { + DeviceState parent_class; + AspeedSoCInfo *info; +} AspeedSoCClass; + +#define ASPEED_SOC_CLASS(klass) \ + OBJECT_CLASS_CHECK(AspeedSoCClass, (klass), TYPE_ASPEED_SOC) +#define ASPEED_SOC_GET_CLASS(obj) \ + OBJECT_GET_CLASS(AspeedSoCClass, (obj), TYPE_ASPEED_SOC) #endif /* ASPEED_SOC_H */