From 32c82f0eaf9919cb0268d18d86a94bfd7ff5d1b2 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 22 Apr 2020 15:48:15 +0200 Subject: [PATCH] smbus: Fix spd_data_generate() for number of banks > 2 spd_data_generate() splits @ram_size bytes into @nbanks RAM banks of 1 << sz_log2 MiB each, like this: size = ram_size >> 20; /* work in terms of megabytes */ [...] nbanks = 1; while (sz_log2 > max_log2 && nbanks < 8) { sz_log2--; nbanks++; } Each iteration halves the size of a bank, and increments the number of banks. Wrong: it should double the number of banks. The bug goes back all the way to commit b296b664ab "smbus: Add a helper to generate SPD EEPROM data". It can't bite because spd_data_generate()'s current users pass only @ram_size that result in *zero* iterations: machine RAM size #banks type bank size fulong2e 256 MiB 1 DDR 256 MiB sam460ex 2048 MiB 1 DDR2 2048 MiB 1024 MiB 1 DDR2 1024 MiB 512 MiB 1 DDR2 512 MiB 256 MiB 1 DDR2 256 MiB 128 MiB 1 SDR 128 MiB 64 MiB 1 SDR 64 MiB 32 MiB 1 SDR 32 MiB Apply the obvious, minimal fix. I admit I'm tempted to rip out the unused (and obviously untested) feature instead, because YAGNI. Note that this is not the final result, as spd_data_generate() next increases #banks from 1 to 2 if possible. This is done "to avoid a bug in MIPS Malta firmware". We don't even use this function with machine type malta. *Shrug* Signed-off-by: Markus Armbruster Message-Id: <20200422134815.1584-5-armbru@redhat.com> --- hw/i2c/smbus_eeprom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 07fbbf87f1..e199fc8678 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -229,7 +229,7 @@ uint8_t *spd_data_generate(enum sdram_type type, ram_addr_t ram_size) nbanks = 1; while (sz_log2 > max_log2 && nbanks < 8) { sz_log2--; - nbanks++; + nbanks *= 2; } assert(size == (1ULL << sz_log2) * nbanks);