[SeaBIOS] [PATCH V2] pci: fixes to allow booting from extra root pci buses.

[Kevin O'Connor]

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 at i0cf8,%x/"?

> (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.  Doesn't QEMU have to sort the buses
anyway to know which secondary bus ranges are associated with each
root bus?

> - 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 at N/pci at 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?

-Kevin