| Message ID | 1345029140-12338-3-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Aug 15, 2012 at 01:12:20PM +0200, Markus Armbruster wrote: > pc_cmos_init() always claims 640KiB base memory, and ram_size - 1MiB > extended memory. The latter can underflow to "lots of extended > memory". Fix both, and clean up some. > > Note: SeaBIOS currently requires 1MiB of RAM, and doesn't check > whether it got enough. SeaBIOS (and Bochs BIOS before it) never checked cmos 0x15-0x18 for memory sizes. Thus, it has only ever supported passing memory in 1MiB chunks. If we really want to communicate fine grained memory sizes, I suggest passing an e820 like map via fw_cfg to SeaBIOS. I don't think it makes sense for SeaBIOS to read new magic cmos settings just to check if it should crash in a slightly different way. -Kevin
"Kevin O'Connor" <kevin@koconnor.net> writes: > On Wed, Aug 15, 2012 at 01:12:20PM +0200, Markus Armbruster wrote: >> pc_cmos_init() always claims 640KiB base memory, and ram_size - 1MiB >> extended memory. The latter can underflow to "lots of extended >> memory". Fix both, and clean up some. >> >> Note: SeaBIOS currently requires 1MiB of RAM, and doesn't check >> whether it got enough. > > SeaBIOS (and Bochs BIOS before it) never checked cmos 0x15-0x18 for > memory sizes. Thus, it has only ever supported passing memory in 1MiB > chunks. I'm not debating what SeaBIOS supports, not even what it should support, just correcting two inaccuracies: 1. SeaBIOS indeed doesn't check CMOS 0x15/0x16 (base memory, below 1MiB), but it *does* check the copy of 0x17/0x18 in 0x30/0x31 (extended memory, between 1MiB and 64MiB). 2. Not checking 0x15-0x18 does *not* imply 1MiB granularity. CMOS RAM size granularity is 64KiB above 16MiB, and 1KiB below. > If we really want to communicate fine grained memory sizes, I > suggest passing an e820 like map via fw_cfg to SeaBIOS. I don't think > it makes sense for SeaBIOS to read new magic cmos settings just to > check if it should crash in a slightly different way. Where "slightly different" means "fail POST (early, thus very limited ways to report to the user)" instead of "attempt to use non-existant RAM and crash hard". Sounds nice to have to me, but there are tons of "nice" things that are plainly not worth the trouble. Whether this one is worth the trouble for SeaBIOS is for SeaBIOS to decide. Likewise, whether switching from CMOS to fw_cfg for RAM size is worth it. A nice thing to have on the QEMU side would be refusing to set up SeaBIOS for a necessarily obscure failure by not meeting its RAM requirement. Again, this may not be worth the trouble. Is there a reliable way to recognize SeaBIOS?
diff --git a/hw/pc.c b/hw/pc.c index e8bcfc0..24df1e0 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -337,32 +337,37 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, /* various important CMOS locations needed by PC/Bochs bios */ /* memory size */ - val = 640; /* base memory in K */ + /* base memory (first MiB) */ + val = MIN(ram_size / 1024, 640); rtc_set_memory(s, 0x15, val); rtc_set_memory(s, 0x16, val >> 8); - - val = (ram_size / 1024) - 1024; + /* extended memory (next 64MiB) */ + if (ram_size > 1024 * 1024) { + val = (ram_size - 1024 * 1024) / 1024; + } else { + val = 0; + } if (val > 65535) val = 65535; rtc_set_memory(s, 0x17, val); rtc_set_memory(s, 0x18, val >> 8); rtc_set_memory(s, 0x30, val); rtc_set_memory(s, 0x31, val >> 8); - - if (above_4g_mem_size) { - rtc_set_memory(s, 0x5b, (unsigned int)above_4g_mem_size >> 16); - rtc_set_memory(s, 0x5c, (unsigned int)above_4g_mem_size >> 24); - rtc_set_memory(s, 0x5d, (uint64_t)above_4g_mem_size >> 32); - } - - if (ram_size > (16 * 1024 * 1024)) - val = (ram_size / 65536) - ((16 * 1024 * 1024) / 65536); - else + /* memory between 16MiB and 4GiB */ + if (ram_size > 16 * 1024 * 1024) { + val = (ram_size - 16 * 1024 * 1024) / 65536; + } else { val = 0; + } if (val > 65535) val = 65535; rtc_set_memory(s, 0x34, val); rtc_set_memory(s, 0x35, val >> 8); + /* memory above 4GiB */ + val = above_4g_mem_size / 65536; + rtc_set_memory(s, 0x5b, val); + rtc_set_memory(s, 0x5c, val >> 8); + rtc_set_memory(s, 0x5d, val >> 16); /* set the number of CPU */ rtc_set_memory(s, 0x5f, smp_cpus - 1);
pc_cmos_init() always claims 640KiB base memory, and ram_size - 1MiB extended memory. The latter can underflow to "lots of extended memory". Fix both, and clean up some. Note: SeaBIOS currently requires 1MiB of RAM, and doesn't check whether it got enough. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/pc.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-)