On Fri, Aug 09, 2013 at 12:13:06AM -0400, Kevin O'Connor wrote:
> 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
If we make it a rule that PCI is`setup before ACPI tables
are read, then QEMU can do the patching itself when
it detects BIOS reading the tables.
Gerd, Laszlo,others, does this rule work for alternative firmwares?
