On 7/20/20 7:49 PM, Philippe Mathieu-Daudé wrote: > The SDRAM Memory Controller has a 32-bit address bus, thus > supports up to 4 GiB of DRAM. There is a signed to unsigned > conversion error with the AST2600 maximum memory size: > > (uint64_t)(2048 << 20) = (uint64_t)(-2147483648) > = 0xffffffff40000000 > = 16 EiB - 2 GiB > > Fix by using the IEC suffixes which are usually safer, and add > an assertion check to verify the memory is valid. This would have > catched this bug: > > $ qemu-system-arm -M ast2600-evb > qemu-system-arm: hw/misc/aspeed_sdmc.c:258: aspeed_sdmc_realize: Assertion > `asc->max_ram_size < 4 * GiB' failed. > Aborted (core dumped) > > Fixes: 1550d72679 ("aspeed/sdmc: Add AST2600 support") > Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Reviewed-by: Cédric Le Goater <c...@kaod.org> > --- > v2: Assert (Cédric) > --- > hw/misc/aspeed_sdmc.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c > index 0737d8de81..855848b7d2 100644 > --- a/hw/misc/aspeed_sdmc.c > +++ b/hw/misc/aspeed_sdmc.c > @@ -255,6 +255,7 @@ static void aspeed_sdmc_realize(DeviceState *dev, Error > **errp) > AspeedSDMCState *s = ASPEED_SDMC(dev); > AspeedSDMCClass *asc = ASPEED_SDMC_GET_CLASS(s); > > + assert(asc->max_ram_size < 4 * GiB); /* 32-bit address bus */ > s->max_ram_size = asc->max_ram_size; > > memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_sdmc_ops, s, > @@ -341,7 +342,7 @@ static void aspeed_2400_sdmc_class_init(ObjectClass > *klass, void *data) > AspeedSDMCClass *asc = ASPEED_SDMC_CLASS(klass); > > dc->desc = "ASPEED 2400 SDRAM Memory Controller"; > - asc->max_ram_size = 512 << 20; > + asc->max_ram_size = 512 * MiB; > asc->compute_conf = aspeed_2400_sdmc_compute_conf; > asc->write = aspeed_2400_sdmc_write; > asc->valid_ram_sizes = aspeed_2400_ram_sizes; > @@ -408,7 +409,7 @@ static void aspeed_2500_sdmc_class_init(ObjectClass > *klass, void *data) > AspeedSDMCClass *asc = ASPEED_SDMC_CLASS(klass); > > dc->desc = "ASPEED 2500 SDRAM Memory Controller"; > - asc->max_ram_size = 1024 << 20; > + asc->max_ram_size = 1 * GiB; > asc->compute_conf = aspeed_2500_sdmc_compute_conf; > asc->write = aspeed_2500_sdmc_write; > asc->valid_ram_sizes = aspeed_2500_ram_sizes; > @@ -485,7 +486,7 @@ static void aspeed_2600_sdmc_class_init(ObjectClass > *klass, void *data) > AspeedSDMCClass *asc = ASPEED_SDMC_CLASS(klass); > > dc->desc = "ASPEED 2600 SDRAM Memory Controller"; > - asc->max_ram_size = 2048 << 20; > + asc->max_ram_size = 2 * GiB; > asc->compute_conf = aspeed_2600_sdmc_compute_conf; > asc->write = aspeed_2600_sdmc_write; > asc->valid_ram_sizes = aspeed_2600_ram_sizes; >