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

[Marcel Apfelbaum <marcel@redhat.com>]

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

Thanks,
Marcel



>
>> 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.
>
> Note the "physical address" part in the above.  Your patch changes the
> "pci-root@" syntax to use a logical address instead of a physical
> address.  That is, unless I've missed something, SeaBIOS today uses a
> physical address (the n'th root bus) and the patch would change it to
> use a logical address.
>
> One of the goals of using an "openfirmware" like address was so that
> they would be stable across boots (the same mechanism is also used
> with coreboot).  Using a physical address is key for this, because
> simply adding or removing a PCI device could cause the logical PCI
> bridge enumeration to change - and that would mess up the bootorder
> list if it was based on logical addresses.
>
> -Kevin
>