Re: [Qemu-devel] [PATCH V2] pci: fixes to allow booting from extra root pci buses.

[Laszlo Ersek <lersek@redhat.com>]

On 06/11/15 15:37, Marcel Apfelbaum wrote:
> The fixes solves the following issue:
> The PXB device exposes a new  pci root bridge with the
> fw path:  /pci-root@4/..., in which 4 is the root bus number.
> Before this patch the fw path was wrongly computed:
>     /pci-root@1/pci@i0cf8/...
> Fix the above issues: Correct the bus number and remove the
> extra host bridge description.
> 
> The IEEE Std 1275-1994:
> 
>   IEEE Standard for Boot (Initialization Configuration)
>     Firmware: Core Requirements and Practices
>       3.2.1.1 Node names
>           Each node in the device tree is identified by a node name
>           using the following notation:
>               driver-name@unit-address:device-arguments
> 
>           The driver name field is a sequence of between one and 31
>           letters [...]. By convention, this name includes the name of
>           the device’s manufacturer and the device’s model name separated by
>           a “,”.
> 
>           The unit address field is the text representation of the
>           physical address of the device within the address space
>           defined by its parent node. The form of the text
>           representation is bus-dependent.
>     3.2.1.2 Path names
>         A particular node is uniquely identified by describing its position
>         in the device tree by completely specifying the
>         path from the root node through all intermediate nodes to the node
>         in question. The textual representation of a
>         such a path is called a device path. Device paths are composed as
>         follows:
>         /node-name0/node-name1/ ... /node-nameN
>         When Open Firmware is searching for a particular node, and either
>         the driver name or @unit-address portion of
>         the node name is not given, Open Firmware shall arbitrarily choose a
>         node matching the portion that is present.
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
> v1->v2:
>   - fixed a bug preventing the boot of devices behind the main host bridge
>     This approach leaves previous code paths intact so it will work the same
>     as before if no PXB is present.
>   - Added spec for fw path naming conventions. (Michael S. Tsirkin)
> 
>  src/boot.c   | 3 ++-
>  src/hw/pci.c | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/boot.c b/src/boot.c
> index ec59c37..e241d1c 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);
> +        else
> +            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..9e77af4 100644
> --- a/src/hw/pci.c
> +++ b/src/hw/pci.c
> @@ -133,7 +133,7 @@ pci_probe_devices(void)
>                  if (bus != lastbus)
>                      rootbuses++;
>                  lastbus = bus;
> -                rootbus = rootbuses;
> +                rootbus = bus;
>                  if (bus > MaxPCIBus)
>                      MaxPCIBus = bus;
>              } else {
> 

I think the commit message is somewhat overkill, but I'll leave that to
Michael. :)

Regardig the rootbus question. As far as I can see, the last hunk
changes the dev->rootbus assignment for parentless devices, so they pick
up the last bus rather than the number of buses found.

Then, the only difference this makes is in build_pci_path() -- I grepped
the tree for whole-word "rootbus".

(It's quite a serendipity that my v3 qemu patchset produces exactly this
pattern in OFW device paths, without my then-knowledge of the SeaBIOS
code. Although, admittedly, the fw_name := "pci-root" change there was
suggested by Marcel.)

So, unless I'm missing something:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>