On 06/12/15 20:40, Kevin O'Connor wrote:
On Fri, Jun 12, 2015 at 05:45:04PM +0200, Laszlo Ersek wrote:
On 06/12/15 15:03, Kevin O'Connor wrote
As for what I would suggest - well, SeaBIOS has already supported multiple root buses for years and already has a mechanism for deterministically specifying a device on an extra root bus. (By specifying the N'th extra root bus instead of specifying the logical id given to that bus). This is by no means a perfect solution and it's certainly open to change - but the current proposed patches appear to be regressions to me.
Could we simply make this patch conditional on runningOnQEMU()?
It's possible. I'd certainly prefer to avoid adding special cases if possible.
Okay. Let's compare the two options we appear to have:
(1) A patch like this for SeaBIOS:
diff --git a/src/boot.c b/src/boot.c index ec59c37..c7fd091 100644 --- a/src/boot.c +++ b/src/boot.c @@ -114,7 +114,8 @@ build_pci_path(char *buf, int max, const char *devname, struct pci_device *pci) } else { if (pci->rootbus) p += snprintf(p, max, "/pci-root@%x", pci->rootbus);
p += snprintf(p, buf+max-p, "%s", FW_PCI_DOMAIN);
if (!runningOnQEMU() || !pci->rootbus)p += snprintf(p, buf+max-p, "%s", FW_PCI_DOMAIN);}
int dev = pci_bdf_to_dev(pci->bdf), fn = pci_bdf_to_fn(pci->bdf);
diff --git a/src/hw/pci.c b/src/hw/pci.c index 0379b55..169a040 100644 --- a/src/hw/pci.c +++ b/src/hw/pci.c @@ -13,6 +13,7 @@ #include "string.h" // memset #include "util.h" // udelay #include "x86.h" // outl +#include "fw/paravirt.h" // runningOnQEMU
void pci_config_writel(u16 bdf, u32 addr, u32 val) { @@ -133,7 +134,7 @@ pci_probe_devices(void) if (bus != lastbus) rootbuses++; lastbus = bus;
rootbus = rootbuses;
rootbus = runningOnQEMU() ? bus : rootbuses; if (bus > MaxPCIBus) MaxPCIBus = bus; } else {If we went down this path, I hope we could agree on the same prefix and thus limit the runningOnQEMU() to just the second part. Was there a concern with "/pci@i0cf8,%x/"?
I don't recall any specific concern, but if we want to present either /pci@i0cf8,%x/, or the pattern SeaBIOS currently expects, then in QEMU the same stuff has to be poked at anyway.
(2) The QEMU command line and the effects the command line has on the virtual hardware should not change. However, all of the following have to be updated:
- the "explicit_ofw_unit_address" property assignments in pxb_dev_initfn() have to be done separately, after all PXBs have been seen, and sorted. This probably requires another "machine init done" notifier.
I admit the sorting of pxb objects just to reverse engineer what SeaBIOS expects would not be fun.
Actually it's kinda fun. :)
Doesn't QEMU have to sort the buses anyway to know which secondary bus ranges are associated with each root bus?
I don't think so.
OVMF does the same probing (in the same order) as SeaBIOS for the root buses, and the intervals between each pair are handed to edk2's PCI bus driver (which is independent of the OVMF platform code). This latter driver performs the assignments / allocations from the allowed interval.
the _UID assignments in build_ssdt() need to reflect the exact same values
OVMF's root bridge driver needs to generate the same _UID values in the PciRoot() device path nodes
OVMF's boot order library must consider the /pci-root@N/pci@i0cf8/... format, where the root bus is the N'th extra root bus (in hex notation).
Basically, we need to keep the bus_nr=N user interface, and the effects it has on the virtual hardware, intact, but translate the numbers that are exposed via fw_cfg *and* ACPI (because those must match!) from "identifier" to "serial number after sorting by identifier"; in practice replicating the detection traversal that SeaBIOS does.
Why does fw_cfg and ACPI have to match?
You will like the explanation for that. I'm about to test a new QEMU series, and I'll CC you on the relevant patches. The last patch in the series answers this question.
(In a nutshell: OVMF translates OFW devpath fragments to UEFI devpath fragments, for boot option matching. The UEFI devpath fragments in question start with a PciRoot() ACPI node, and the _UID value in that initial node must match what QEMU hands down and OVMF uses in translation. Finally, the UEFI spec requires that such a _UID value, in the UEFI devpath, actually identify the valid ACPI object -- and that ACPI object comes from QEMU's generator.)
If everything goes well, we might not need to change SeaBIOS at all. (... Hope dies last :))
Thanks Laszlo