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.
Moving it to another fixed address below 0x1000 doesn't work, breaks with old qemu+new seabios and other way around.
I think easiest is to allow firmware pick it and fixup the tables provided by qemu accordingly.
Picking pmbase works in firmware today btw as both coreboot+seabios generate a fadt with the correct values. Only nit is that this depends on qemu 1.4+ as piix4 pmbase register was read-only in older qemu versions. So everybody uses 0xb000 by default for bug compatibility with older qemu versions.
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.
cheers, Gerd
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.
cheers, Gerd