On Mon, Dec 14, 2009 at 08:37:44PM -0600, Anthony Liguori wrote:
Okay, I think I've figured out how this is supposed to work. With these two patches to SeaBIOS and the patch to qemu, I can run:
I'm not sure why "Do not guard qemu shadow ram work around in CONFIG_OPTIONROMS_DEPLOYED" patch is needed. The code today is:
if (CONFIG_OPTIONROMS_DEPLOYED) { int reg = pci_config_readb(bdf, 0x5a + i); if ((reg & 0x11) != 0x11) { // Need to copy optionroms to work around qemu implementation void *mem = (void*)(BUILD_ROM_START + i * 32*1024); memcpy((void*)BUILD_BIOS_TMP_ADDR, mem, 32*1024); pci_config_writeb(bdf, 0x5a + i, 0x33); memcpy(mem, (void*)BUILD_BIOS_TMP_ADDR, 32*1024); clear = 1; } else { pci_config_writeb(bdf, 0x5a + i, 0x33); } } else { pci_config_writeb(bdf, 0x5a + i, 0x33); }
So, in the non CONFIG_OPTIONROMS_DEPLOYED case, SeaBIOS will just do the write enable call (pci_config_writeb(bdf, 0x5a + i, 0x33)). The CONFIG_OPTIONROMS_DEPLOYED case should just be to copy the roms qemu has deployed. If SeaBIOS is filling in the 0xc0000-0xf0000 space, it shouldn't matter if the contents of that space is lost during the write enable.
Let me know if I've missed something.
-Kevin
Kevin O'Connor wrote:
On Mon, Dec 14, 2009 at 08:37:44PM -0600, Anthony Liguori wrote:
Okay, I think I've figured out how this is supposed to work. With these two patches to SeaBIOS and the patch to qemu, I can run:
I'm not sure why "Do not guard qemu shadow ram work around in CONFIG_OPTIONROMS_DEPLOYED" patch is needed. The code today is:
if (CONFIG_OPTIONROMS_DEPLOYED) { int reg = pci_config_readb(bdf, 0x5a + i); if ((reg & 0x11) != 0x11) { // Need to copy optionroms to work around qemu implementation void *mem = (void*)(BUILD_ROM_START + i * 32*1024); memcpy((void*)BUILD_BIOS_TMP_ADDR, mem, 32*1024); pci_config_writeb(bdf, 0x5a + i, 0x33); memcpy(mem, (void*)BUILD_BIOS_TMP_ADDR, 32*1024); clear = 1; } else { pci_config_writeb(bdf, 0x5a + i, 0x33); } } else { pci_config_writeb(bdf, 0x5a + i, 0x33); }
So, in the non CONFIG_OPTIONROMS_DEPLOYED case, SeaBIOS will just do the write enable call (pci_config_writeb(bdf, 0x5a + i, 0x33)). The CONFIG_OPTIONROMS_DEPLOYED case should just be to copy the roms qemu has deployed. If SeaBIOS is filling in the 0xc0000-0xf0000 space, it shouldn't matter if the contents of that space is lost during the write enable.
The bios gets mapped in 0xe0000 .. 0x100000 so if SeaBIOS fills the 0xc0000-0xf0000 space it will write over half of the bios.
Regards,
Anthony Liguori
Anthony Liguori wrote:
The bios gets mapped in 0xe0000 .. 0x100000 so if SeaBIOS fills the 0xc0000-0xf0000 space it will write over half of the bios.
I'm a little confused by this. SeaBIOS seems to assume that it only has to deal with the 0xf0000 .. 0x100000 space as the bios which is certainly true (i don't think there's anything special about the 0xe0000 .. 0xf0000 region).
I'm not sure why we load the 128K worth of bios instead of just loading 64K.
Regards,
Anthony Liguori
Regards,
Anthony Liguori
On 12/15/2009 04:20 PM, Anthony Liguori wrote:
Anthony Liguori wrote:
The bios gets mapped in 0xe0000 .. 0x100000 so if SeaBIOS fills the 0xc0000-0xf0000 space it will write over half of the bios.
I'm a little confused by this. SeaBIOS seems to assume that it only has to deal with the 0xf0000 .. 0x100000 space as the bios which is certainly true (i don't think there's anything special about the 0xe0000 .. 0xf0000 region).
I'm not sure why we load the 128K worth of bios instead of just loading 64K.
bochs bios required all 128kB, so this is probably a leftover.
Avi Kivity wrote:
On 12/15/2009 04:20 PM, Anthony Liguori wrote:
Anthony Liguori wrote:
The bios gets mapped in 0xe0000 .. 0x100000 so if SeaBIOS fills the 0xc0000-0xf0000 space it will write over half of the bios.
I'm a little confused by this. SeaBIOS seems to assume that it only has to deal with the 0xf0000 .. 0x100000 space as the bios which is certainly true (i don't think there's anything special about the 0xe0000 .. 0xf0000 region).
I'm not sure why we load the 128K worth of bios instead of just loading 64K.
bochs bios required all 128kB, so this is probably a leftover.
This is apparently well defined in the PIIX spec. There is a bit of a difference between the lower half and upper half of the BIOS region though and I expect this is part of what the problem is. FYI, the following patch works. Surprisingly, we only need to restore the 0xe8000..0xe8fff region. Still trying to understand what's happening.
diff --git a/src/shadow.c b/src/shadow.c index f0f97c5..860f461 100644 --- a/src/shadow.c +++ b/src/shadow.c @@ -29,7 +29,8 @@ __make_bios_writable(u16 bdf) int clear = 0; int i; for (i=0; i<6; i++) { - if (CONFIG_OPTIONROMS_DEPLOYED) { + /* need to copy 0xe8000 bios region for qemu */ + if (i==5) { int reg = pci_config_readb(bdf, 0x5a + i); if ((reg & 0x11) != 0x11) { // Need to copy optionroms to work around qemu implementation
Regards,
Anthony Liguori
On Tue, Dec 15, 2009 at 12:35 PM, Anthony Liguori anthony@codemonkey.wswrote:
Avi Kivity wrote:
bochs bios required all 128kB, so this is probably a leftover.
This is apparently well defined in the PIIX spec. There is a bit of a difference between the lower half and upper half of the BIOS region though and I expect this is part of what the problem is. FYI, the following patch works. Surprisingly, we only need to restore the 0xe8000..0xe8fff region. Still trying to understand what's happening.
SeaBIOS is currently using over 64K of space. So, it is extending into the e-segment and thus needs to have the e-segment copied properly (I missed that in my earlier email).
SeaBIOS is only extending a few K into the e-segment, so only that part needs to be saved and restored.
Should SeaBIOS options be trimmed so that it fit under 64K, then the build would create only a 64K rom. However, since it is larger than 64K, it pads itself to 128K. As mentioned before, SeaBIOS can still copy roms into the unused parts of the e-segment, as it knows which parts of the e-segment it is using.
-Kevin
Kevin OConnor wrote:
On Tue, Dec 15, 2009 at 12:35 PM, Anthony Liguori <anthony@codemonkey.ws mailto:anthony@codemonkey.ws> wrote:
Avi Kivity wrote: bochs bios required all 128kB, so this is probably a leftover. This is apparently well defined in the PIIX spec. There is a bit of a difference between the lower half and upper half of the BIOS region though and I expect this is part of what the problem is. FYI, the following patch works. Surprisingly, we only need to restore the 0xe8000..0xe8fff region. Still trying to understand what's happening.
SeaBIOS is currently using over 64K of space. So, it is extending into the e-segment and thus needs to have the e-segment copied properly (I missed that in my earlier email).
Okay. So I assume this is something that SeaBIOS needs to do? I've been trying to understand the origin of:
// Need to copy optionroms to work around qemu implementation
What is it that we're doing wrong in the qemu implementation? If it's the lack of PCI option loading, then I guess we're okay post my changes provided that SeaBIOS can take care of what it needs in the e-segment.
Regards,
Anthony Liguori
On Tue, Dec 15, 2009 at 06:41:33PM -0600, Anthony Liguori wrote:
Kevin OConnor wrote:
SeaBIOS is currently using over 64K of space. So, it is extending into the e-segment and thus needs to have the e-segment copied properly (I missed that in my earlier email).
Okay. So I assume this is something that SeaBIOS needs to do? I've been trying to understand the origin of:
// Need to copy optionroms to work around qemu implementation
What is it that we're doing wrong in the qemu implementation?
The comment is referring to the fact that enabling write access to that area causes the existing contents to be discarded, and SeaBIOS needs to copy the contents elsewhere, enable the write, and then copy the contents back. A real machine shadow implementation wouldn't require that. Though, I don't think it's a big deal to handle this in SeaBIOS.
If it's the lack of PCI option loading, then I guess we're okay post my changes provided that SeaBIOS can take care of what it needs in the e-segment.
Yes - I think your changes are fine.
-Kevin