On Thu, Aug 08, 2013 at 04:56:55PM +0200, Gerd Hoffmann wrote:
On 08/08/13 16:13, Michael S. Tsirkin wrote:
On Thu, Aug 08, 2013 at 12:21:32PM +0200, Gerd Hoffmann wrote:
On 08/08/13 11:52, Michael S. Tsirkin wrote:
On Thu, Aug 08, 2013 at 10:57:44AM +0200, Gerd Hoffmann wrote:
On 08/08/13 10:37, Michael S. Tsirkin wrote:
On Thu, Aug 08, 2013 at 09:57:39AM +0200, Gerd Hoffmann wrote: > Also coreboot and seabios use different values for pmbase. coreboot on > q35 maps the pmbase below 0x1000. Which surely makes sense. When we > don't place chipset stuff at 0xb000 we can assign the 0xb000->0xbfff > window to a pci bridge instead.
Re-reading this - if this has value, can't we generalize it and make all firmware behave the same, getting values from QEMU?
pmbase is a compile-time constant (aka #define) in both seabios and coreboot, and making this runtime-configurable is non-trivial. See src/smm.c in seabios for one reason why.
I don't think SeaBIOS should modify tables provided by QEMU. That leads to confusion on the source of the data and mixed responsibilities which results in greater complexity in both QEMU and SeaBIOS.
SeaBIOS doesn't have any info that QEMU doesn't have. So, I think it's safe for QEMU to be the sole authority for the table content.
Converting src/smm.c to use a runtime value isn't hard - just change the assembler from: "mov $" __stringify(PORT_ACPI_PM_BASE) " + 0x04, %dx\n" to: "mov 4(my_acpi_base), %dx\n" and make sure to define the global variable my_acpi_base as VARFSEG.
Yes, the address ranges used for pci devices (aka 32bit + 64bit pci window) need to be there. Well, placing in SSDT, then referencing from DSDT works too, and this is what seabios does today to dynamically adjust stuff. Fixing up the SSDT using the linker is probably easier as we generate it anyway.
Yes but as I said, this makes things messy, since AML encoding for numbers isn't fixed width.
In seabios we have fixed 32bit / 64bit width today, from acpi.c:
// store pci io windows *(u32*)&ssdt_ptr[acpi_pci32_start[0]] = cpu_to_le32(pcimem_start); *(u32*)&ssdt_ptr[acpi_pci32_end[0]] = cpu_to_le32(pcimem_end - 1); if (pcimem64_start) { ssdt_ptr[acpi_pci64_valid[0]] = 1; *(u64*)&ssdt_ptr[acpi_pci64_start[0]] = cpu_to_le64(pcimem64_start); *(u64*)&ssdt_ptr[acpi_pci64_end[0]] = cpu_to_le64(pcimem64_end - 1); *(u64*)&ssdt_ptr[acpi_pci64_length[0]] = cpu_to_le64( pcimem64_end - pcimem64_start); } else { ssdt_ptr[acpi_pci64_valid[0]] = 0; }Storing fixup instructions for these fields in the linker script shouldn't be hard I think.
I don't think SeaBIOS should continue to do the above once the tables are moved to QEMU. QEMU has all the info SeaBIOS has, so it can generate the tables correctly on its own.
In all practical situations, the PCI window should be at least an order of magnatude greater than the sum of the PCI bars in the system. If the bars are actually bigger than the window, then things are going to fail - the best the firmware can do is try to fail gracefully. I don't think it's worth the complexity to design mixed ownership and advanced interfaces just so we can fail slightly better.
If this is a real worry, QEMU can sum all the PCI bars and warn the user if they don't fit.
-Kevin