From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: David Gibson <david@gibson.dropbear.id.au>, qemu-ppc@nongnu.org
Subject: [PATCH 4/4] smbus: Fix spd_data_generate() for number of banks > 2
Date: Mon, 20 Apr 2020 15:28:26 +0200 [thread overview]
Message-ID: <20200420132826.8879-5-armbru@redhat.com> (raw)
In-Reply-To: <20200420132826.8879-1-armbru@redhat.com>
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 <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);
--
2.21.1
next prev parent reply other threads:[~2020-04-20 13:30 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-20 13:28 [PATCH 0/4] Subject: [PATCH 0/4] smbus: SPD fixes Markus Armbruster
2020-04-20 13:28 ` [PATCH 1/4] sam460ex: Revert change to SPD memory type for <= 128 MiB Markus Armbruster
2020-04-20 14:12 ` BALATON Zoltan
2020-04-21 5:28 ` Markus Armbruster
2020-04-22 13:56 ` BALATON Zoltan
2020-04-29 5:18 ` Markus Armbruster
2020-04-20 13:28 ` [PATCH 2/4] smbus: Fix spd_data_generate() error API violation Markus Armbruster
2020-04-20 14:20 ` BALATON Zoltan
2020-04-21 5:28 ` Markus Armbruster
2020-04-22 13:43 ` BALATON Zoltan
2020-04-24 9:45 ` Markus Armbruster
2020-04-24 10:18 ` Philippe Mathieu-Daudé
2020-04-24 11:23 ` Markus Armbruster
2020-04-24 13:52 ` BALATON Zoltan
2020-04-29 5:42 ` Markus Armbruster
2020-04-20 13:28 ` [PATCH 3/4] bamboo, sam460ex: Tidy up error message for unsupported RAM size Markus Armbruster
2020-04-20 13:50 ` Philippe Mathieu-Daudé
2020-04-20 13:28 ` Markus Armbruster [this message]
2020-04-20 13:53 ` [PATCH 4/4] smbus: Fix spd_data_generate() for number of banks > 2 Philippe Mathieu-Daudé
2020-04-20 14:37 ` BALATON Zoltan
2020-04-21 4:57 ` Markus Armbruster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200420132826.8879-5-armbru@redhat.com \
--to=armbru@redhat.com \
--cc=david@gibson.dropbear.id.au \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.