From: Jim Quinlan <jim2101024@gmail.com> To: ard.biesheuvel@linaro.org Cc: Christoph Hellwig <hch@lst.de>, Robin Murphy <robin.murphy@arm.com>, Florian Fainelli <f.fainelli@gmail.com>, linux-kernel@vger.kernel.org, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, linux-pci <linux-pci@vger.kernel.org>, bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>, Gregory Fong <gregory.0xf0@gmail.com>, Bjorn Helgaas <bhelgaas@google.com>, Brian Norris <computersforpeace@gmail.com>, "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v5 04/12] PCI: brcmstb: add dma-range mapping for inbound traffic Date: Mon, 24 Sep 2018 11:01:09 -0400 [thread overview] Message-ID: <CANCKTBtbwSqfSU0b_d6Yi7TV3R_ijEpPE8a4bpgpkcOxJOa_zQ@mail.gmail.com> (raw) In-Reply-To: <CAKv+Gu-+OjJi8zGLqvnsOJQ9ygMot-O0EbyMKO-yyG-+k-274A@mail.gmail.com> On Mon, Sep 24, 2018 at 4:25 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On Fri, 21 Sep 2018 at 19:41, Jim Quinlan <jim2101024@gmail.com> wrote: > > > > On Thu, Sep 20, 2018 at 5:39 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > > > > > > On 09/20/2018 02:33 PM, Ard Biesheuvel wrote: > > > > On 20 September 2018 at 14:31, Florian Fainelli <f.fainelli@gmail.com> wrote: > > > >> On 09/20/2018 02:04 PM, Ard Biesheuvel wrote: > > > >>> On 20 September 2018 at 13:55, Florian Fainelli <f.fainelli@gmail.com> wrote: > > > >>>> On 09/19/2018 07:19 PM, Ard Biesheuvel wrote: > > > >>>>> On 19 September 2018 at 07:31, Jim Quinlan <jim2101024@gmail.com> wrote: > > > >>>>>> The Broadcom STB PCIe host controller is intimately related to the > > > >>>>>> memory subsystem. This close relationship adds complexity to how cpu > > > >>>>>> system memory is mapped to PCIe memory. Ideally, this mapping is an > > > >>>>>> identity mapping, or an identity mapping off by a constant. Not so in > > > >>>>>> this case. > > > >>>>>> > > > >>>>>> Consider the Broadcom reference board BCM97445LCC_4X8 which has 6 GB > > > >>>>>> of system memory. Here is how the PCIe controller maps the > > > >>>>>> system memory to PCIe memory: > > > >>>>>> > > > >>>>>> memc0-a@[ 0....3fffffff] <=> pci@[ 0....3fffffff] > > > >>>>>> memc0-b@[100000000...13fffffff] <=> pci@[ 40000000....7fffffff] > > > >>>>>> memc1-a@[ 40000000....7fffffff] <=> pci@[ 80000000....bfffffff] > > > >>>>>> memc1-b@[300000000...33fffffff] <=> pci@[ c0000000....ffffffff] > > > >>>>>> memc2-a@[ 80000000....bfffffff] <=> pci@[100000000...13fffffff] > > > >>>>>> memc2-b@[c00000000...c3fffffff] <=> pci@[140000000...17fffffff] > > > >>>>>> > > > >>>>> > > > >>>>> So is describing this as > > > >>>>> > > > >>>>> dma-ranges = <0x0 0x0 0x0 0x0 0x0 0x40000000>, > > > >>>>> <0x0 0x40000000 0x1 0x0 0x0 0x40000000>, > > > >>>>> <0x0 0x80000000 0x0 0x40000000 0x0 0x40000000>, > > > >>>>> <0x0 0xc0000000 0x3 0x0 0x0 0x40000000>, > > > >>>>> <0x1 0x0 0x0 0x80000000 0x0 0x40000000>, > > > >>>>> <0x1 0x40000000 0x0 0xc0000000 0x0 0x40000000>; > > > >>>>> > > > >>>>> not working for you? I haven't tried this myself, but since DT permits > > > >>>>> describing the inbound mappings this way, we should fix the code if it > > > >>>>> doesn't work at the moment. > > > >>>> > > > >>>> You mean encoding the memory controller index in the first cell? If that > > > >>>> works, that's indeed a much cleaner solution, though is it standard > > > >>>> compliant in any form? > > > >>> > > > >>> No those are just memory addresses (although I may have screwed up the > > > >>> order). From Documentation/devicetree/booting-without-of.txt: > > > >>> > > > >>> """ > > > >>> Optional property: > > > >>> - dma-ranges: <prop-encoded-array> encoded as arbitrary number of triplets of > > > >>> (child-bus-address, parent-bus-address, length). Each triplet specified > > > >>> describes a contiguous DMA address range. > > > >>> """ > > > >>> > > > >> > > > >> Then I am confused by your comment, that's what this patch does, it adds > > > >> support for reading "dma-ranges" from Device Tree and setting up inbound > > > >> windows using that. The only caveat is that because the PCIe root > > > >> complex has some ties with the memory bus architecture it is connected > > > >> to (SCB in our case) there is still a requirement to know the > > > >> translation between a given physical address and its backing memory > > > >> controller/aperture. > > > >> > > > > > > > > Ah ok, apologies for the noise then. > > > > > > > > I was hoping that having working support for dma-ranges would remove > > > > the need for the special phys<->dma conversion routines. > > > > > > What you describe definitively works with platform devices, but I am not > > > sure this is working for PCIe devices, although, conceptually it should, > > > yes. > > Sorry for my delay in responding. One problem is that > > of_dma_configure() only looks at the first dma-range given and then > > converts it to dev->dma_pfn_offset which is respected by the DMA API. > > However, we often have multiple dma-ranges, not just one. This is the > > big issue. > > > > Given the recent attention to getting these APIs in shape, this may be > something Robin or Christoph may care to look into? It looks like this has been brought up before in the "[RFC PATCH] of: Fix DMA configuration for non-DT masters" thread aka https://lists.linuxfoundation.org/pipermail/iommu/2017-April/021325.html In the thread "Oza Oza", a Broadcom coworker probably dealing with the same exact problem as I, enumerates three problems. #1 and #2 are the exact same ones I've just given: the "dma-ranges" prop of the RC DT node is "skipped", and of_dma_get_range() only considers the first entry in any "dma-ranges". Thanks, Jim > > In any case, the description of dma-ranges should be in sync with the > way Linux interprets it, so this is either a documentation bug or a > DMA layer bug. > > > There is another issue with of_dma_configure() being invoked by the EP > > driver on "bridge->parent->of_node", which is our RC device, > > Of_dma_configure() calls of_dma_range() on the of_get_next_parent() of > > our RC's device node and this misses the dma-ranges property which is > > contained within the RC. I think I could workaround this but there is > > no getting around the first problem. > > > > IIUC dma-ranges should be added to the parent bus of a device, which I > guess is slightly ambiguous for a root complex that incorporates a > host bridge.
WARNING: multiple messages have this Message-ID (diff)
From: jim2101024@gmail.com (Jim Quinlan) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v5 04/12] PCI: brcmstb: add dma-range mapping for inbound traffic Date: Mon, 24 Sep 2018 11:01:09 -0400 [thread overview] Message-ID: <CANCKTBtbwSqfSU0b_d6Yi7TV3R_ijEpPE8a4bpgpkcOxJOa_zQ@mail.gmail.com> (raw) In-Reply-To: <CAKv+Gu-+OjJi8zGLqvnsOJQ9ygMot-O0EbyMKO-yyG-+k-274A@mail.gmail.com> On Mon, Sep 24, 2018 at 4:25 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On Fri, 21 Sep 2018 at 19:41, Jim Quinlan <jim2101024@gmail.com> wrote: > > > > On Thu, Sep 20, 2018 at 5:39 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > > > > > > On 09/20/2018 02:33 PM, Ard Biesheuvel wrote: > > > > On 20 September 2018 at 14:31, Florian Fainelli <f.fainelli@gmail.com> wrote: > > > >> On 09/20/2018 02:04 PM, Ard Biesheuvel wrote: > > > >>> On 20 September 2018 at 13:55, Florian Fainelli <f.fainelli@gmail.com> wrote: > > > >>>> On 09/19/2018 07:19 PM, Ard Biesheuvel wrote: > > > >>>>> On 19 September 2018 at 07:31, Jim Quinlan <jim2101024@gmail.com> wrote: > > > >>>>>> The Broadcom STB PCIe host controller is intimately related to the > > > >>>>>> memory subsystem. This close relationship adds complexity to how cpu > > > >>>>>> system memory is mapped to PCIe memory. Ideally, this mapping is an > > > >>>>>> identity mapping, or an identity mapping off by a constant. Not so in > > > >>>>>> this case. > > > >>>>>> > > > >>>>>> Consider the Broadcom reference board BCM97445LCC_4X8 which has 6 GB > > > >>>>>> of system memory. Here is how the PCIe controller maps the > > > >>>>>> system memory to PCIe memory: > > > >>>>>> > > > >>>>>> memc0-a@[ 0....3fffffff] <=> pci@[ 0....3fffffff] > > > >>>>>> memc0-b@[100000000...13fffffff] <=> pci@[ 40000000....7fffffff] > > > >>>>>> memc1-a@[ 40000000....7fffffff] <=> pci@[ 80000000....bfffffff] > > > >>>>>> memc1-b@[300000000...33fffffff] <=> pci@[ c0000000....ffffffff] > > > >>>>>> memc2-a@[ 80000000....bfffffff] <=> pci@[100000000...13fffffff] > > > >>>>>> memc2-b@[c00000000...c3fffffff] <=> pci@[140000000...17fffffff] > > > >>>>>> > > > >>>>> > > > >>>>> So is describing this as > > > >>>>> > > > >>>>> dma-ranges = <0x0 0x0 0x0 0x0 0x0 0x40000000>, > > > >>>>> <0x0 0x40000000 0x1 0x0 0x0 0x40000000>, > > > >>>>> <0x0 0x80000000 0x0 0x40000000 0x0 0x40000000>, > > > >>>>> <0x0 0xc0000000 0x3 0x0 0x0 0x40000000>, > > > >>>>> <0x1 0x0 0x0 0x80000000 0x0 0x40000000>, > > > >>>>> <0x1 0x40000000 0x0 0xc0000000 0x0 0x40000000>; > > > >>>>> > > > >>>>> not working for you? I haven't tried this myself, but since DT permits > > > >>>>> describing the inbound mappings this way, we should fix the code if it > > > >>>>> doesn't work at the moment. > > > >>>> > > > >>>> You mean encoding the memory controller index in the first cell? If that > > > >>>> works, that's indeed a much cleaner solution, though is it standard > > > >>>> compliant in any form? > > > >>> > > > >>> No those are just memory addresses (although I may have screwed up the > > > >>> order). From Documentation/devicetree/booting-without-of.txt: > > > >>> > > > >>> """ > > > >>> Optional property: > > > >>> - dma-ranges: <prop-encoded-array> encoded as arbitrary number of triplets of > > > >>> (child-bus-address, parent-bus-address, length). Each triplet specified > > > >>> describes a contiguous DMA address range. > > > >>> """ > > > >>> > > > >> > > > >> Then I am confused by your comment, that's what this patch does, it adds > > > >> support for reading "dma-ranges" from Device Tree and setting up inbound > > > >> windows using that. The only caveat is that because the PCIe root > > > >> complex has some ties with the memory bus architecture it is connected > > > >> to (SCB in our case) there is still a requirement to know the > > > >> translation between a given physical address and its backing memory > > > >> controller/aperture. > > > >> > > > > > > > > Ah ok, apologies for the noise then. > > > > > > > > I was hoping that having working support for dma-ranges would remove > > > > the need for the special phys<->dma conversion routines. > > > > > > What you describe definitively works with platform devices, but I am not > > > sure this is working for PCIe devices, although, conceptually it should, > > > yes. > > Sorry for my delay in responding. One problem is that > > of_dma_configure() only looks at the first dma-range given and then > > converts it to dev->dma_pfn_offset which is respected by the DMA API. > > However, we often have multiple dma-ranges, not just one. This is the > > big issue. > > > > Given the recent attention to getting these APIs in shape, this may be > something Robin or Christoph may care to look into? It looks like this has been brought up before in the "[RFC PATCH] of: Fix DMA configuration for non-DT masters" thread aka https://lists.linuxfoundation.org/pipermail/iommu/2017-April/021325.html In the thread "Oza Oza", a Broadcom coworker probably dealing with the same exact problem as I, enumerates three problems. #1 and #2 are the exact same ones I've just given: the "dma-ranges" prop of the RC DT node is "skipped", and of_dma_get_range() only considers the first entry in any "dma-ranges". Thanks, Jim > > In any case, the description of dma-ranges should be in sync with the > way Linux interprets it, so this is either a documentation bug or a > DMA layer bug. > > > There is another issue with of_dma_configure() being invoked by the EP > > driver on "bridge->parent->of_node", which is our RC device, > > Of_dma_configure() calls of_dma_range() on the of_get_next_parent() of > > our RC's device node and this misses the dma-ranges property which is > > contained within the RC. I think I could workaround this but there is > > no getting around the first problem. > > > > IIUC dma-ranges should be added to the parent bus of a device, which I > guess is slightly ambiguous for a root complex that incorporates a > host bridge.
next prev parent reply other threads:[~2018-09-24 15:01 UTC|newest] Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-09-19 14:31 [PATCH v5 00/12] PCI: brcmstb: Add Broadcom Settopbox PCIe support (resend) Jim Quinlan 2018-09-19 14:31 ` [PATCH v5 01/12] soc: bcm: brcmstb: add memory API Jim Quinlan 2018-09-19 14:31 ` Jim Quinlan 2018-09-19 14:31 ` [PATCH v5 02/12] dt-bindings: pci: add DT docs for Brcmstb PCIe device Jim Quinlan 2018-09-19 14:31 ` Jim Quinlan 2018-09-19 14:31 ` Jim Quinlan 2018-09-20 9:06 ` Jonas Gorski 2018-09-20 9:06 ` Jonas Gorski 2018-09-20 9:06 ` Jonas Gorski 2018-09-21 18:00 ` Jim Quinlan 2018-09-21 18:00 ` Jim Quinlan 2018-09-21 18:00 ` Jim Quinlan 2018-09-19 14:31 ` [PATCH v5 03/12] PCI: brcmstb: add Broadcom STB PCIe host controller driver Jim Quinlan 2018-09-19 14:31 ` Jim Quinlan 2018-09-19 14:31 ` [PATCH v5 04/12] PCI: brcmstb: add dma-range mapping for inbound traffic Jim Quinlan 2018-09-19 14:31 ` Jim Quinlan 2018-09-19 14:31 ` Jim Quinlan 2018-09-20 2:19 ` Ard Biesheuvel 2018-09-20 2:19 ` Ard Biesheuvel 2018-09-20 20:55 ` Florian Fainelli 2018-09-20 20:55 ` Florian Fainelli 2018-09-20 21:04 ` Ard Biesheuvel 2018-09-20 21:04 ` Ard Biesheuvel 2018-09-20 21:31 ` Florian Fainelli 2018-09-20 21:31 ` Florian Fainelli 2018-09-20 21:33 ` Ard Biesheuvel 2018-09-20 21:33 ` Ard Biesheuvel 2018-09-20 21:39 ` Florian Fainelli 2018-09-20 21:39 ` Florian Fainelli 2018-09-21 17:40 ` Jim Quinlan 2018-09-21 17:40 ` Jim Quinlan 2018-09-24 8:25 ` Ard Biesheuvel 2018-09-24 8:25 ` Ard Biesheuvel 2018-09-24 15:01 ` Jim Quinlan [this message] 2018-09-24 15:01 ` Jim Quinlan 2018-09-26 2:59 ` Florian Fainelli 2018-09-26 2:59 ` Florian Fainelli 2018-09-26 7:59 ` Ard Biesheuvel 2018-09-26 7:59 ` Ard Biesheuvel 2018-09-26 10:56 ` Robin Murphy 2018-09-26 10:56 ` Robin Murphy 2018-10-01 18:13 ` Florian Fainelli 2018-10-01 18:13 ` Florian Fainelli 2019-06-27 16:55 ` Stefan Wahren 2019-06-27 16:55 ` Stefan Wahren 2018-09-19 14:32 ` [PATCH v5 05/12] PCI: brcmstb: add MSI capability Jim Quinlan 2018-09-19 14:32 ` Jim Quinlan 2018-09-19 14:32 ` [PATCH v5 06/12] MIPS: BMIPS: add dma remap for BrcmSTB PCIe Jim Quinlan 2018-09-26 22:07 ` Paul Burton 2018-09-26 22:07 ` Paul Burton 2018-09-28 21:48 ` Jim Quinlan 2018-09-19 14:32 ` [PATCH v5 07/12] PCI/MSI: enable PCI_MSI_IRQ_DOMAIN support for MIPS Jim Quinlan 2018-09-19 14:32 ` [PATCH v5 08/12] MIPS: BMIPS: add PCI bindings for 7425, 7435 Jim Quinlan 2018-09-19 14:32 ` [PATCH v5 09/12] MIPS: BMIPS: enable PCI Jim Quinlan 2018-09-19 14:32 ` [PATCH v5 10/12] ARM64: declare __phys_to_dma on ARCH_HAS_PHYS_TO_DMA Jim Quinlan 2018-09-19 14:32 ` Jim Quinlan 2018-09-19 14:37 ` Christoph Hellwig 2018-09-19 14:37 ` Christoph Hellwig 2018-09-19 14:32 ` [PATCH v5 11/12] ARM64: add dma remap for BrcmSTB PCIe Jim Quinlan 2018-09-19 14:32 ` Jim Quinlan 2018-09-19 14:32 ` Jim Quinlan 2018-09-19 14:41 ` Christoph Hellwig 2018-09-19 14:41 ` Christoph Hellwig 2018-09-19 14:41 ` Christoph Hellwig 2018-09-21 18:29 ` Jim Quinlan 2018-09-21 18:29 ` Jim Quinlan 2018-09-19 14:32 ` [PATCH v5 12/12] ARM: " Jim Quinlan 2018-09-19 14:32 ` Jim Quinlan 2019-03-20 23:15 ` [PATCH v5 00/12] PCI: brcmstb: Add Broadcom Settopbox PCIe support (resend) Bjorn Helgaas 2019-03-20 23:22 ` Florian Fainelli -- strict thread matches above, loose matches on Subject: below -- 2018-09-06 20:42 [PATCH v5 00/12] PCI: brcmstb: Add Broadcom Settopbox PCIe support Jim Quinlan 2018-09-06 20:42 ` [PATCH v5 04/12] PCI: brcmstb: add dma-range mapping for inbound traffic Jim Quinlan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CANCKTBtbwSqfSU0b_d6Yi7TV3R_ijEpPE8a4bpgpkcOxJOa_zQ@mail.gmail.com \ --to=jim2101024@gmail.com \ --cc=ard.biesheuvel@linaro.org \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=bhelgaas@google.com \ --cc=computersforpeace@gmail.com \ --cc=f.fainelli@gmail.com \ --cc=gregory.0xf0@gmail.com \ --cc=hch@lst.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=robin.murphy@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.