Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Ian Campbell <Ian.Campbell@citrix.com>]

On Wed, 2015-02-18 at 14:19 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 18/02/2015 13:50, Ian Campbell wrote:
> > On Tue, 2015-02-17 at 17:33 +0000, Julien Grall wrote:
> >> Hi Ian,
> >>
> >> On 24/10/14 10:58, Ian Campbell wrote:
> >>> These properties are defined in ePAPR and the OpenFirmware PCI Bus Binding
> >>> Specification (IEEE Std 1275-1994).
> >>>
> >>> This replaces the xgene specific mapping. Tested on Mustang and on a model with
> >>> a PCI virtio controller.
> >>
> >> I'm wondering why you choose to map everything at Xen boot time rather
> >> than implementing PHYSDEVOP_pci_device_add to do the job?
> >
> > Does pci_device_add contain sufficient information to do so?
> 
> Hmmm... for the interrupt the SBDF is enough. Although for the MMIO it 
> looks like there is no difference between PCI bars.
> 
> > The regions which are being mapped are essentially the PCI host
> > controllers MMIO, IO and CFG "windows" which are then consumed by the
> > various bars of the devices on the bus.
> >
> > So mapping based on pci_device_add would require us to go from the SBDF
> > to a set of BARS which need mapping, which is a whole lot more complex
> > than just mapping all of the resources owned by the root complex through
> > to the h/w domain.
> 
> I gave a look to the code which parse the host bridge resource (see 
> of_pci_get_host_bridge_resources). They seem to re-use to the 
> of_translate_* function. Would not it be possible to do the same?
> 
> > Or perhaps I've misunderstood what you were suggesting?
> 
> I was suggesting to do it via pci_add_device but it looks like it's only 
> possible for IRQ not MMIO.

I think so, and we probably should consider the two cases separately
since the right answer could reasonably differ for different resource
types.

I am reasonably convinced that for MMIO (+IO+CFG space) we should map
everything as described by the ranges property of the top most node, it
can be considered an analogue to / extension of the reg property of that
node.

For IRQ I'm not so sure, it's possible that routing the IRQ at
pci_add_device time might be better, or fit in better with e.g. the ACPI
architecture, but mapping everything described in interrupt-map at start
of day is also an option and a reasonably simple one, probably.

(My memory is fuzzy, but I think the concerns I had with this patch were
precisely to do with IRQs and how to parse the interrupt-map without a
specific SBDF in hand -- but only because the existing helper functions
assume an SBDF is present)

> >> This would allow us to re-use most of the interrupts/mmio decoding
> >> provided in the device tree library. It would also avoid missing support
> >> of cascade ranges/interrupt-map.
> >
> > I *think* (if I'm remembering right) I decided we don't need to worry
> > about cascades of these things because the second level resources are
> > all fully contained within the first (top level) one and so with the
> > approach I've taken here are all fully mapped already. That's why I made
> > this patch stop descending into children when such a "bus node" is
> > found.
> 
> I don't understand this paragraph, sorry.
> 
> The address range you decoded via the PCI bus may be an intermediate 
> address which needs to be translated in the physical hardware address.

This isn't to do with IPA->PA translations but to do with translations
between different PA addressing regimes. i.e. the different addressing
schemes of difference busses.

Lets say we have a system with a PCI-ROOT device exposing a PCI bus,
which in turn contains a PCI-BRIDGE which for the sake of argument lets
say is a PCI-FOOBUS bridge.

Lets just consider the MMIO hole for now, but IRQ is basically the same.

The ranges property on a node describes a mapping from a "parent"
address space into a "child" address space.

For PCI-ROOT "parent" is the host physical address space and "child" is
the PCI MMIO/IO/CFG address spaces.

For PCI-BRIDGE "parent" is the PCI-ROOT's child address space (i.e. PCI
MMIO/IO/CFG) and "child" is the FOOBUS address space.

The inputs ("parents") of the PCI-BRIDGE ranges property must therefore
by definition be valid outputs of the PCI-ROOT ranges property (i.e. be
"child" addresses).

Therefore if we map all of the input/parent ranges described by
PCI-ROOT's ranges property we do not need to recurse further and
consider PCI-BRIDGE's ranges property -- we've effectively already dealt
with it.

Does that make more sense?

Ian.