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

[Laszlo Ersek]

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:  /address@hidden/..., in which 4 is the root bus number.
>>>>> Before this patch the fw path was wrongly computed:
>>>>>      /address@hidden/address@hidden/...
>>>>> 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.
>>>   /address@hidden/address@hidden/   ...
>>>      ^ 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 "/address@hidden/" 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 /address@hidden/ 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 "address@hidden" with "address@hidden" but that
>> seems odd as the extra root bus is accessible via io accesses to
>> 0x0cf8.
> While this is true, /address@hidden/ 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 address@hidden behind the address@hidden
>> as in "/address@hidden/address@hidden/...".  Or, the root bus could be 
>> appended
>> to the host bridge as in "/address@hidden,1/...".
> The latest representation makes sense to me,  but "/address@hidden,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