Re: [Qemu-devel] [PATCH] hw/arm/virt: Fix address in PCIe device tree node's unit name

[Peter Maydell]

On 23 October 2015 at 16:26, Alexander Gordeev <address@hidden> wrote:
> On Fri, Oct 23, 2015 at 03:18:34PM +0100, Peter Maydell wrote:
>> On 21 October 2015 at 23:01, Alexander Gordeev <address@hidden> wrote:
>> > On Wed, Oct 21, 2015 at 10:26:27PM +0100, Peter Maydell wrote:
>> >> On 21 October 2015 at 21:43, Alexander Gordeev <address@hidden> wrote:
>> >> > PCIe device tree unit name is address@hidden - which denotes
>> >> > IO space base address. However, the corresponding node's
>> >> > "reg" property points to PCI configuration space base address
>> >> > 0x3f000000.
>> >> >
>> >> > Set the unit name to address@hidden which is not only correct,
>> >> > but also conforms to Open Firmware (IEEE 1275).
>> >>
>> >> Nothing should actually care about the address in the
>> >> nodename, though, right -- it's just for human readability
>> >> and debugging (and guests will be looking at the regs
>> >> etc properties of the node to figure out where it is)?
>> >> Or have I misunderstood this and there's an actual visible
>> >> consequence to this bug?
>> >
>> > I do not think there are actual consequences out there.
>> > It is just misleading and does not honour the standard.
>>
>> Do you have a more precise reference than just "IEEE 1275" ?
>> I found the bit that says node names should be address@hidden,
>> and unit address is the "text representation of the physical address
>> of the device", but it seems to me that our current choice of
>> "the lowest physical address where you can find any part of this
>> device" is closer to that than deciding that we should use the
>> address of the config space window instead.
>
> No, I do not have it other than indirectly (though I probably could locate 
> it).
> I relied on Linux Documentation/devicetree/bindings/pci/host-generic-pci.txt
> instead:
>
> - reg            : The Configuration Space base address and size, as accessed
>                    from the parent bus. [...]

Yes, we do this correctly. This has nothing to do with the nodename,
though, does it?

> IMHO it perfectly make sense - either io or memory mapped regions are rather
> used to map attached devices, while the unit name for a host bridge is implied
> to "address" the bridge itself, isn't it?

The config space is also just talking to attached devices.
The 'generic' host bridge itself has no registers at all.

Well, the only purpose as far as I can tell for the addr part of
the node name is:
 * to make sure it's unique
 * as a reasonably easy way to remember which one it is if there
   are several of a device in the system

And using the lowest address in the map that the PCI device uses
seems as good a way as any other.

So unless you have a really strong definition from the standard
for why this should be the config space in particular I don't
really think there's any need to change it.

thanks
-- PMM