From 05e6e40a869cc7e4492c5ffc8161005bbb7be986 Mon Sep 17 00:00:00 2001 From: Wentao_Liang Date: Tue, 8 Mar 2022 09:18:11 +0100 Subject: [PATCH 01/11] aspeed: Fix a potential memory leak bug in write_boot_rom() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A memory chunk is allocated with g_new0() and assigned to the variable 'storage'. However, if the branch takes true, there will be only an error report but not a free operation for 'storage' before function returns. As a result, a memory leak bug is triggered. Use g_autofree to fix the issue. Suggested-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Signed-off-by: Wentao_Liang [ clg: reworked the commit log ] Signed-off-by: Cédric Le Goater --- hw/arm/aspeed.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 11558b327b..b71bc2559b 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -246,7 +246,7 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size, Error **errp) { BlockBackend *blk = blk_by_legacy_dinfo(dinfo); - uint8_t *storage; + g_autofree void *storage = NULL; int64_t size; /* The block backend size should have already been 'validated' by @@ -262,14 +262,13 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size, rom_size = size; } - storage = g_new0(uint8_t, rom_size); + storage = g_malloc0(rom_size); if (blk_pread(blk, 0, storage, rom_size) < 0) { error_setg(errp, "failed to read the initial flash content"); return; } rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr); - g_free(storage); } static void aspeed_board_init_flashes(AspeedSMCState *s, From dcacfa945103d179e71bb1c853f2a13aca5ca4b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 8 Mar 2022 09:18:11 +0100 Subject: [PATCH 02/11] aspeed/smc: Use max number of CE instead of 'num_cs' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Aspeed SMC model uses the 'num_cs' field to allocate resources fitting the number of devices of the machine. This is a small optimization without real need in the controller. Simplify modelling and use the max_peripherals field instead. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Message-Id: <20220307071856.1410731-2-clg@kaod.org> Signed-off-by: Cédric Le Goater --- hw/ssi/aspeed_smc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index d899be17fd..a5d8bb717f 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -693,7 +693,7 @@ static void aspeed_smc_reset(DeviceState *d) } /* Unselect all peripherals */ - for (i = 0; i < s->num_cs; ++i) { + for (i = 0; i < asc->max_peripherals; ++i) { s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE; qemu_set_irq(s->cs_lines[i], true); } @@ -1042,7 +1042,7 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data, addr < s->r_timings + asc->nregs_timings) || addr == s->r_ce_ctrl) { s->regs[addr] = value; - } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) { + } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_peripherals) { int cs = addr - s->r_ctrl0; aspeed_smc_flash_update_ctrl(&s->flashes[cs], value); } else if (addr >= R_SEG_ADDR0 && @@ -1139,9 +1139,9 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp) s->spi = ssi_create_bus(dev, "spi"); /* Setup cs_lines for peripherals */ - s->cs_lines = g_new0(qemu_irq, s->num_cs); + s->cs_lines = g_new0(qemu_irq, asc->max_peripherals); - for (i = 0; i < s->num_cs; ++i) { + for (i = 0; i < asc->max_peripherals; ++i) { sysbus_init_irq(sbd, &s->cs_lines[i]); } From 9bd4ac61c7e7e5cc84d1d6fd1f70cc88b0ac95f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 8 Mar 2022 09:18:11 +0100 Subject: [PATCH 03/11] aspeed: Rework aspeed_board_init_flashes() interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, the allocation of the flash devices uses the number of slave selects configured in the SoC realize routine. It is simpler to use directly the number of FMC devices defined in the machine class and 1 for spi devices (which is what the SoC does in the back of the machine). Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Message-Id: <20220307071856.1410731-3-clg@kaod.org> Signed-off-by: Cédric Le Goater --- hw/arm/aspeed.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index b71bc2559b..6558801765 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -271,13 +271,12 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size, rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr); } -static void aspeed_board_init_flashes(AspeedSMCState *s, - const char *flashtype, - int unit0) +static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype, + unsigned int count, int unit0) { int i ; - for (i = 0; i < s->num_cs; ++i) { + for (i = 0; i < count; ++i) { DriveInfo *dinfo = drive_get(IF_MTD, 0, unit0 + i); qemu_irq cs_line; DeviceState *dev; @@ -373,10 +372,10 @@ static void aspeed_machine_init(MachineState *machine) aspeed_board_init_flashes(&bmc->soc.fmc, bmc->fmc_model ? bmc->fmc_model : amc->fmc_model, - 0); + amc->num_cs, 0); aspeed_board_init_flashes(&bmc->soc.spi[0], bmc->spi_model ? bmc->spi_model : amc->spi_model, - bmc->soc.fmc.num_cs); + 1, amc->num_cs); /* Install first FMC flash content as a boot rom. */ if (drive0) { From 8d99713bba88c2dff8d62a9d7370d026428335dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 8 Mar 2022 09:18:11 +0100 Subject: [PATCH 04/11] aspeed/smc: Remove 'num_cs' field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is not used anymore. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Message-Id: <20220307071856.1410731-4-clg@kaod.org> Signed-off-by: Cédric Le Goater --- hw/arm/aspeed.c | 2 -- hw/arm/aspeed_ast2600.c | 2 -- hw/arm/aspeed_soc.c | 2 -- hw/ssi/aspeed_smc.c | 7 ------- include/hw/ssi/aspeed_smc.h | 1 - 5 files changed, 14 deletions(-) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 6558801765..90504ee444 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -343,8 +343,6 @@ static void aspeed_machine_init(MachineState *machine) &error_abort); object_property_set_int(OBJECT(&bmc->soc), "hw-strap2", amc->hw_strap2, &error_abort); - object_property_set_int(OBJECT(&bmc->soc), "num-cs", amc->num_cs, - &error_abort); object_property_set_link(OBJECT(&bmc->soc), "dram", OBJECT(machine->ram), &error_abort); if (machine->kernel_filename) { diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index 21cd3342c5..c1e15e3773 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -163,7 +163,6 @@ static void aspeed_soc_ast2600_init(Object *obj) snprintf(typename, sizeof(typename), "aspeed.fmc-%s", socname); object_initialize_child(obj, "fmc", &s->fmc, typename); - object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs"); for (i = 0; i < sc->spis_num; i++) { snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, socname); @@ -383,7 +382,6 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) for (i = 0; i < sc->spis_num; i++) { object_property_set_link(OBJECT(&s->spi[i]), "dram", OBJECT(s->dram_mr), &error_abort); - object_property_set_int(OBJECT(&s->spi[i]), "num-cs", 1, &error_abort); if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) { return; } diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index 7d53cf2f51..58714cb2a0 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -170,7 +170,6 @@ static void aspeed_soc_init(Object *obj) snprintf(typename, sizeof(typename), "aspeed.fmc-%s", socname); object_initialize_child(obj, "fmc", &s->fmc, typename); - object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs"); for (i = 0; i < sc->spis_num; i++) { snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, socname); @@ -327,7 +326,6 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) /* SPI */ for (i = 0; i < sc->spis_num; i++) { - object_property_set_int(OBJECT(&s->spi[i]), "num-cs", 1, &error_abort); if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) { return; } diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index a5d8bb717f..6859f061c8 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -1127,12 +1127,6 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp) s->r_timings = asc->r_timings; s->conf_enable_w0 = asc->conf_enable_w0; - /* Enforce some real HW limits */ - if (s->num_cs > asc->max_peripherals) { - aspeed_smc_error("num_cs cannot exceed: %d", asc->max_peripherals); - s->num_cs = asc->max_peripherals; - } - /* DMA irq. Keep it first for the initialization in the SoC */ sysbus_init_irq(sbd, &s->irq); @@ -1211,7 +1205,6 @@ static const VMStateDescription vmstate_aspeed_smc = { }; static Property aspeed_smc_properties[] = { - DEFINE_PROP_UINT32("num-cs", AspeedSMCState, num_cs, 1), DEFINE_PROP_BOOL("inject-failure", AspeedSMCState, inject_failure, false), DEFINE_PROP_LINK("dram", AspeedSMCState, dram_mr, TYPE_MEMORY_REGION, MemoryRegion *), diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h index cad73ddc13..4a9354e13c 100644 --- a/include/hw/ssi/aspeed_smc.h +++ b/include/hw/ssi/aspeed_smc.h @@ -57,7 +57,6 @@ struct AspeedSMCState { qemu_irq irq; - uint32_t num_cs; qemu_irq *cs_lines; bool inject_failure; From ae945a00ff465f2e08be6ca6d83946fe5f9d6018 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 8 Mar 2022 09:18:11 +0100 Subject: [PATCH 05/11] aspeed/smc: Rename 'max_peripherals' to 'cs_num_max' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The naming makes more sense in a SPI controller model. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Message-Id: <20220307071856.1410731-5-clg@kaod.org> Signed-off-by: Cédric Le Goater --- hw/ssi/aspeed_smc.c | 42 ++++++++++++++++++------------------- include/hw/ssi/aspeed_smc.h | 2 +- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index 6859f061c8..f1b5644aaa 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -224,7 +224,7 @@ static bool aspeed_smc_flash_overlap(const AspeedSMCState *s, AspeedSegments seg; int i; - for (i = 0; i < asc->max_peripherals; i++) { + for (i = 0; i < asc->cs_num_max; i++) { if (i == cs) { continue; } @@ -290,7 +290,7 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs, */ if ((asc->segments == aspeed_2500_spi1_segments || asc->segments == aspeed_2500_spi2_segments) && - cs == asc->max_peripherals && + cs == asc->cs_num_max && seg.addr + seg.size != asc->segments[cs].addr + asc->segments[cs].size) { aspeed_smc_error("Tried to change CS%d end address to 0x%" @@ -693,13 +693,13 @@ static void aspeed_smc_reset(DeviceState *d) } /* Unselect all peripherals */ - for (i = 0; i < asc->max_peripherals; ++i) { + for (i = 0; i < asc->cs_num_max; ++i) { s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE; qemu_set_irq(s->cs_lines[i], true); } /* setup the default segment register values and regions for all */ - for (i = 0; i < asc->max_peripherals; ++i) { + for (i = 0; i < asc->cs_num_max; ++i) { aspeed_smc_flash_set_segment_region(s, i, asc->segment_to_reg(s, &asc->segments[i])); } @@ -729,8 +729,8 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size) (aspeed_smc_has_dma(asc) && addr == R_DMA_LEN) || (aspeed_smc_has_dma(asc) && addr == R_DMA_CHECKSUM) || (addr >= R_SEG_ADDR0 && - addr < R_SEG_ADDR0 + asc->max_peripherals) || - (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_peripherals)) { + addr < R_SEG_ADDR0 + asc->cs_num_max) || + (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->cs_num_max)) { trace_aspeed_smc_read(addr << 2, size, s->regs[addr]); @@ -1042,11 +1042,11 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data, addr < s->r_timings + asc->nregs_timings) || addr == s->r_ce_ctrl) { s->regs[addr] = value; - } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_peripherals) { + } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->cs_num_max) { int cs = addr - s->r_ctrl0; aspeed_smc_flash_update_ctrl(&s->flashes[cs], value); } else if (addr >= R_SEG_ADDR0 && - addr < R_SEG_ADDR0 + asc->max_peripherals) { + addr < R_SEG_ADDR0 + asc->cs_num_max) { int cs = addr - R_SEG_ADDR0; if (value != s->regs[R_SEG_ADDR0 + cs]) { @@ -1090,7 +1090,7 @@ static void aspeed_smc_instance_init(Object *obj) AspeedSMCClass *asc = ASPEED_SMC_GET_CLASS(s); int i; - for (i = 0; i < asc->max_peripherals; i++) { + for (i = 0; i < asc->cs_num_max; i++) { object_initialize_child(obj, "flash[*]", &s->flashes[i], TYPE_ASPEED_SMC_FLASH); } @@ -1133,9 +1133,9 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp) s->spi = ssi_create_bus(dev, "spi"); /* Setup cs_lines for peripherals */ - s->cs_lines = g_new0(qemu_irq, asc->max_peripherals); + s->cs_lines = g_new0(qemu_irq, asc->cs_num_max); - for (i = 0; i < asc->max_peripherals; ++i) { + for (i = 0; i < asc->cs_num_max; ++i) { sysbus_init_irq(sbd, &s->cs_lines[i]); } @@ -1168,7 +1168,7 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp) * module behind to handle the memory accesses. This depends on * the board configuration. */ - for (i = 0; i < asc->max_peripherals; ++i) { + for (i = 0; i < asc->cs_num_max; ++i) { AspeedSMCFlash *fl = &s->flashes[i]; if (!object_property_set_link(OBJECT(fl), "controller", OBJECT(s), @@ -1314,7 +1314,7 @@ static void aspeed_2400_smc_class_init(ObjectClass *klass, void *data) asc->r_timings = R_TIMINGS; asc->nregs_timings = 1; asc->conf_enable_w0 = CONF_ENABLE_W0; - asc->max_peripherals = 1; + asc->cs_num_max = 1; asc->segments = aspeed_2400_smc_segments; asc->flash_window_base = 0x10000000; asc->flash_window_size = 0x6000000; @@ -1359,7 +1359,7 @@ static void aspeed_2400_fmc_class_init(ObjectClass *klass, void *data) asc->r_timings = R_TIMINGS; asc->nregs_timings = 1; asc->conf_enable_w0 = CONF_ENABLE_W0; - asc->max_peripherals = 5; + asc->cs_num_max = 5; asc->segments = aspeed_2400_fmc_segments; asc->segment_addr_mask = 0xffff0000; asc->resets = aspeed_2400_fmc_resets; @@ -1401,7 +1401,7 @@ static void aspeed_2400_spi1_class_init(ObjectClass *klass, void *data) asc->r_timings = R_SPI_TIMINGS; asc->nregs_timings = 1; asc->conf_enable_w0 = SPI_CONF_ENABLE_W0; - asc->max_peripherals = 1; + asc->cs_num_max = 1; asc->segments = aspeed_2400_spi1_segments; asc->flash_window_base = 0x30000000; asc->flash_window_size = 0x10000000; @@ -1442,7 +1442,7 @@ static void aspeed_2500_fmc_class_init(ObjectClass *klass, void *data) asc->r_timings = R_TIMINGS; asc->nregs_timings = 1; asc->conf_enable_w0 = CONF_ENABLE_W0; - asc->max_peripherals = 3; + asc->cs_num_max = 3; asc->segments = aspeed_2500_fmc_segments; asc->segment_addr_mask = 0xffff0000; asc->resets = aspeed_2500_fmc_resets; @@ -1480,7 +1480,7 @@ static void aspeed_2500_spi1_class_init(ObjectClass *klass, void *data) asc->r_timings = R_TIMINGS; asc->nregs_timings = 1; asc->conf_enable_w0 = CONF_ENABLE_W0; - asc->max_peripherals = 2; + asc->cs_num_max = 2; asc->segments = aspeed_2500_spi1_segments; asc->segment_addr_mask = 0xffff0000; asc->flash_window_base = 0x30000000; @@ -1515,7 +1515,7 @@ static void aspeed_2500_spi2_class_init(ObjectClass *klass, void *data) asc->r_timings = R_TIMINGS; asc->nregs_timings = 1; asc->conf_enable_w0 = CONF_ENABLE_W0; - asc->max_peripherals = 2; + asc->cs_num_max = 2; asc->segments = aspeed_2500_spi2_segments; asc->segment_addr_mask = 0xffff0000; asc->flash_window_base = 0x38000000; @@ -1597,7 +1597,7 @@ static void aspeed_2600_fmc_class_init(ObjectClass *klass, void *data) asc->r_timings = R_TIMINGS; asc->nregs_timings = 1; asc->conf_enable_w0 = CONF_ENABLE_W0; - asc->max_peripherals = 3; + asc->cs_num_max = 3; asc->segments = aspeed_2600_fmc_segments; asc->segment_addr_mask = 0x0ff00ff0; asc->resets = aspeed_2600_fmc_resets; @@ -1636,7 +1636,7 @@ static void aspeed_2600_spi1_class_init(ObjectClass *klass, void *data) asc->r_timings = R_TIMINGS; asc->nregs_timings = 2; asc->conf_enable_w0 = CONF_ENABLE_W0; - asc->max_peripherals = 2; + asc->cs_num_max = 2; asc->segments = aspeed_2600_spi1_segments; asc->segment_addr_mask = 0x0ff00ff0; asc->flash_window_base = 0x30000000; @@ -1675,7 +1675,7 @@ static void aspeed_2600_spi2_class_init(ObjectClass *klass, void *data) asc->r_timings = R_TIMINGS; asc->nregs_timings = 3; asc->conf_enable_w0 = CONF_ENABLE_W0; - asc->max_peripherals = 3; + asc->cs_num_max = 3; asc->segments = aspeed_2600_spi2_segments; asc->segment_addr_mask = 0x0ff00ff0; asc->flash_window_base = 0x50000000; diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h index 4a9354e13c..2d5f8f3d8f 100644 --- a/include/hw/ssi/aspeed_smc.h +++ b/include/hw/ssi/aspeed_smc.h @@ -95,7 +95,7 @@ struct AspeedSMCClass { uint8_t r_timings; uint8_t nregs_timings; uint8_t conf_enable_w0; - uint8_t max_peripherals; + uint8_t cs_num_max; const uint32_t *resets; const AspeedSegments *segments; uint32_t segment_addr_mask; From 9bbdfe0569cc16199ed1a7deb77680b9d5051d1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 8 Mar 2022 09:18:11 +0100 Subject: [PATCH 06/11] aspeed/smc: Let the SSI core layer define the bus name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If no id is provided, qdev automatically assigns an unique name with the following pattern "." which avoids bus name collision when using multiple buses. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Message-Id: <20220307071856.1410731-6-clg@kaod.org> Signed-off-by: Cédric Le Goater --- hw/ssi/aspeed_smc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index f1b5644aaa..dc6d4a169d 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -1130,7 +1130,7 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp) /* DMA irq. Keep it first for the initialization in the SoC */ sysbus_init_irq(sbd, &s->irq); - s->spi = ssi_create_bus(dev, "spi"); + s->spi = ssi_create_bus(dev, NULL); /* Setup cs_lines for peripherals */ s->cs_lines = g_new0(qemu_irq, asc->cs_num_max); From c1402ea1226a4dd6f17f071dca8262e858af6008 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 8 Mar 2022 09:18:11 +0100 Subject: [PATCH 07/11] aspeed/smc: Fix error log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Message-Id: <20220307071856.1410731-7-clg@kaod.org> Signed-off-by: Cédric Le Goater --- hw/ssi/aspeed_smc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index dc6d4a169d..48305e1574 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -327,7 +327,7 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs, static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr, unsigned size) { - aspeed_smc_error("To 0x%" HWADDR_PRIx " of size %u" PRIx64, addr, size); + aspeed_smc_error("To 0x%" HWADDR_PRIx " of size %u", addr, size); return 0; } From 8fd49126dc9d3ae65f29efcc07b8e62e596fc5de Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Tue, 8 Mar 2022 09:18:11 +0100 Subject: [PATCH 08/11] hw/block: m25p80: Add support for w25q01jvq MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The w25q01jvq is a 128MB part. Support is being added to the kernel[1] and the two have been tested together. 1. https://lore.kernel.org/lkml/20220222092222.23108-1-potin.lai@quantatw.com/ Signed-off-by: Patrick Williams Cc: Potin Lai Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Francisco Iglesias Message-Id: <20220304180920.1780992-1-patrick@stwcx.xyz> Signed-off-by: Cédric Le Goater --- hw/block/m25p80.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index c6bf3c6bfa..7d3d8b12e0 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -340,6 +340,7 @@ static const FlashPartInfo known_devices[] = { { INFO("w25q80bl", 0xef4014, 0, 64 << 10, 16, ER_4K) }, { INFO("w25q256", 0xef4019, 0, 64 << 10, 512, ER_4K) }, { INFO("w25q512jv", 0xef4020, 0, 64 << 10, 1024, ER_4K) }, + { INFO("w25q01jvq", 0xef4021, 0, 64 << 10, 2048, ER_4K) }, }; typedef enum { From 179b2058829f934c17622b3bbfb398fcf267b670 Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Tue, 8 Mar 2022 09:18:11 +0100 Subject: [PATCH 09/11] hw/arm/aspeed: allow missing spi_model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generally all BMCs will use the fmc_model to hold their own flash and most will have a spi_model to hold the managed system's flash, but not all systems do. Add a simple NULL check to allow a system to set the spi_model as NULL to indicate it should not be instantiated. Signed-off-by: Patrick Williams Message-Id: <20220305000656.1944589-1-patrick@stwcx.xyz> Signed-off-by: Cédric Le Goater --- hw/arm/aspeed.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 90504ee444..088550ed33 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -274,7 +274,11 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size, static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype, unsigned int count, int unit0) { - int i ; + int i; + + if (!flashtype) { + return; + } for (i = 0; i < count; ++i) { DriveInfo *dinfo = drive_get(IF_MTD, 0, unit0 + i); From a20c54b1231dffa3fa0590905b67b2725138718e Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Tue, 8 Mar 2022 09:18:11 +0100 Subject: [PATCH 10/11] hw/arm/aspeed: add Bletchley machine type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the 'bletchley-bmc' machine type based on the kernel DTS[1] and hardware schematics available to me. The i2c model is as complete as the current QEMU models support, but in some cases I substituted devices that are close enough for present functionality. Strap registers are kept the same as the AST2600-EVB until I'm able to confirm correct values with physical hardware. This has been tested with an openbmc image built from [2] plus a kernel patch[3] for the SPI flash module. 1. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/aspeed-bmc-facebook-bletchley.dts?id=a8c729e966c4e9d033242d948b0e53c2a62d32e2 2. https://github.com/openbmc/openbmc/commit/b9432b980d7f63f7512ffbcc7124386ba896dfc6 3. https://github.com/openbmc/linux/commit/25b566b9a9d7f5d4f10c1b7304007bdb286eefd7 Signed-off-by: Patrick Williams Reviewed-by: Cédric Le Goater [ clg : increased number of FMC devices to 2 to match Linux dts ] Message-Id: <20220305000656.1944589-2-patrick@stwcx.xyz> Signed-off-by: Cédric Le Goater --- hw/arm/aspeed.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 088550ed33..d205384d98 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -167,6 +167,11 @@ struct AspeedMachineState { #define FUJI_BMC_HW_STRAP1 0x00000000 #define FUJI_BMC_HW_STRAP2 0x00000000 +/* Bletchley hardware value */ +/* TODO: Leave same as EVB for now. */ +#define BLETCHLEY_BMC_HW_STRAP1 AST2600_EVB_HW_STRAP1 +#define BLETCHLEY_BMC_HW_STRAP2 AST2600_EVB_HW_STRAP2 + /* * The max ram region is for firmwares that scan the address space * with load/store to guess how much RAM the SoC has. @@ -897,6 +902,54 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc) } } +#define TYPE_TMP421 "tmp421" + +static void bletchley_bmc_i2c_init(AspeedMachineState *bmc) +{ + AspeedSoCState *soc = &bmc->soc; + I2CBus *i2c[13] = {}; + for (int i = 0; i < 13; i++) { + if ((i == 8) || (i == 11)) { + continue; + } + i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i); + } + + /* Bus 0 - 5 all have the same config. */ + for (int i = 0; i < 6; i++) { + /* Missing model: ti,ina230 @ 0x45 */ + /* Missing model: mps,mp5023 @ 0x40 */ + i2c_slave_create_simple(i2c[i], TYPE_TMP421, 0x4f); + /* Missing model: nxp,pca9539 @ 0x76, but PCA9552 works enough */ + i2c_slave_create_simple(i2c[i], TYPE_PCA9552, 0x76); + i2c_slave_create_simple(i2c[i], TYPE_PCA9552, 0x67); + /* Missing model: fsc,fusb302 @ 0x22 */ + } + + /* Bus 6 */ + at24c_eeprom_init(i2c[6], 0x56, 65536); + /* Missing model: nxp,pcf85263 @ 0x51 , but ds1338 works enough */ + i2c_slave_create_simple(i2c[6], "ds1338", 0x51); + + + /* Bus 7 */ + at24c_eeprom_init(i2c[7], 0x54, 65536); + + /* Bus 9 */ + i2c_slave_create_simple(i2c[9], TYPE_TMP421, 0x4f); + + /* Bus 10 */ + i2c_slave_create_simple(i2c[10], TYPE_TMP421, 0x4f); + /* Missing model: ti,hdc1080 @ 0x40 */ + i2c_slave_create_simple(i2c[10], TYPE_PCA9552, 0x67); + + /* Bus 12 */ + /* Missing model: adi,adm1278 @ 0x11 */ + i2c_slave_create_simple(i2c[12], TYPE_TMP421, 0x4c); + i2c_slave_create_simple(i2c[12], TYPE_TMP421, 0x4d); + i2c_slave_create_simple(i2c[12], TYPE_PCA9552, 0x67); +} + static bool aspeed_get_mmio_exec(Object *obj, Error **errp) { return ASPEED_MACHINE(obj)->mmio_exec; @@ -1220,6 +1273,25 @@ static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data) aspeed_soc_num_cpus(amc->soc_name); }; +static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "Facebook Bletchley BMC (Cortex-A7)"; + amc->soc_name = "ast2600-a3"; + amc->hw_strap1 = BLETCHLEY_BMC_HW_STRAP1; + amc->hw_strap2 = BLETCHLEY_BMC_HW_STRAP2; + amc->fmc_model = "w25q01jvq"; + amc->spi_model = NULL; + amc->num_cs = 2; + amc->macs_mask = ASPEED_MAC2_ON; + amc->i2c_init = bletchley_bmc_i2c_init; + mc->default_ram_size = 512 * MiB; + mc->default_cpus = mc->min_cpus = mc->max_cpus = + aspeed_soc_num_cpus(amc->soc_name); +} + static const TypeInfo aspeed_machine_types[] = { { .name = MACHINE_TYPE_NAME("palmetto-bmc"), @@ -1273,6 +1345,10 @@ static const TypeInfo aspeed_machine_types[] = { .name = MACHINE_TYPE_NAME("fuji-bmc"), .parent = TYPE_ASPEED_MACHINE, .class_init = aspeed_machine_fuji_class_init, + }, { + .name = MACHINE_TYPE_NAME("bletchley-bmc"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_bletchley_class_init, }, { .name = TYPE_ASPEED_MACHINE, .parent = TYPE_MACHINE, From 46179776c292f83848df90de60da5ae1a965ce6a Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Tue, 8 Mar 2022 09:18:11 +0100 Subject: [PATCH 11/11] hw: aspeed_gpio: Cleanup stray semicolon after switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not sure how that got there. Signed-off-by: Andrew Jeffery Message-Id: <20220207150409.358888-2-andrew@aj.id.au> Signed-off-by: Cédric Le Goater --- hw/gpio/aspeed_gpio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c index 911d21c8cf..c63634d3d3 100644 --- a/hw/gpio/aspeed_gpio.c +++ b/hw/gpio/aspeed_gpio.c @@ -571,7 +571,7 @@ static uint64_t aspeed_gpio_read(void *opaque, hwaddr offset, uint32_t size) qemu_log_mask(LOG_GUEST_ERROR, "%s: no getter for offset 0x%" HWADDR_PRIx"\n", __func__, offset); return 0; - }; + } } static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data,