From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties Date: Wed, 18 Feb 2015 14:19:01 +0000 Message-ID: <54E49F55.2010402@linaro.org> References: <1414144694.15687.31.camel@citrix.com> <1414144717-32328-5-git-send-email-ian.campbell@citrix.com> <54E37B62.4020101@linaro.org> <1424267412.27775.62.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1424267412.27775.62.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Pranavkumar Sawargaonkar , Clark Laughlin , tim@xen.org, stefano.stabellini@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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. >> 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. Regards, -- Julien Grall