On 06/11/15 16:36, Marcel Apfelbaum wrote: > On 06/11/2015 05:24 PM, Kevin O'Connor wrote: >> On Thu, Jun 11, 2015 at 05:12:33PM +0300, Marcel Apfelbaum wrote: >>> On 06/11/2015 04:58 PM, Kevin O'Connor wrote: >>>> On Thu, Jun 11, 2015 at 04:37:08PM +0300, 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. >>>> >>>> Why is that wrong? The previous path looks correct to me. >>> The prev path includes both the extra root bridge and *then* the >>> usual host bridge. >>> /pci-root@1/pci@i0cf8/ ... >>> ^ new ^ regular ^ devices >>> >>> Since the new pci root bridge (and bus) is on "paralel" with the >>> regular one. >>> it is not correct to add it to the path. >>> >>> The architecture is: >>> /<host bridge>/devices... >>> /extra root bridge/devices... >>> /extra root bridge/devices... >>> And not >>> /extra root bridge//<host bridge>/devices >> >> Your patch changed both the "/extra root bridge/devices..." part and >> the "@1" part. The change of the "@1" in "/pci-root@1/" is not >> correct IMO. > Why? @1 should be the unit address which is the text representation > of the physical address, in our case the slot. Since the bus number > in our case is 4, I think /pci-root@4/ is the 'correct' address. >> >> Does open-firmware have any examples for PCI paths and in particular >> PCI paths when there are multiple root-buses? > Maybe Laszlo can say more, but we both agreed that this would be the > berst representation of extra root buses on both OVMF and Seabios.
The PCI Bus Binding to: IEEE Std 1275-1994 Standard for Boot (Initialization Configuration) Firmware document (binding) does speak about this, as far as I can see, in 2.2.1. Physical Address Formats It first gives a "Numerical Representation" in device tree format (same thing as in DTB / FDT), and then a "Text Representation" with references to "Numerical Representation". It is *completely* Greek to me. It took me minutes of staring just to vaguely understand how the current i0cf8 unit address comes together. I've always treated the OFW devpaths that QEMU generates only *syntactically* conformant to the (base) OFW spec, and never considered the particular bindings 100% binding. That said, if someone finds where the PCI binding defines unit addresses for *root* buses, please let me know, just for reference. >> It's possible to replace the "pci@i0cf8" with "pci-root@1" but that >> seems odd as the extra root bus is accessible via io accesses to >> 0x0cf8. > While this is true, /pci-root@[...]/ may represent also other kind of host > bridges not only PXBs. But we can change this of course, as long as OVMF > can also > work with it. > >> >> Another option would be to place the pci-root@1 behind the pci@i0cf8 >> as in "/pci@i0cf8/pci-root@1/...". Or, the root bus could be appended >> to the host bridge as in "/pci@i0cf8,1/...". > The latest representation makes sense to me, but "/pci@i0cf8,4/...", > after comma > the bus number. > > Laszlo, will this work for OVMF? With the v3 patchset for QEMU, we could probably easily generate the "i0cf8,4" unit address inside the PXB device model itself. (Of course exactly what number should stand after the comma remains a question.) Parsing it in OVMF is doable, albeit somewhat ugly. In any case, I'm not convinced at all why this is a better idea than the proposal in this patch. Laszlo