| Message ID | 1548411231-27549-1-git-send-email-srinath.mannam@broadcom.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | PCIe Host request to reserve IOVA | expand |
Hi Bjorn, Please help to review this patch series. Thank you. Regards, Srinath. On Fri, Jan 25, 2019 at 3:44 PM Srinath Mannam <srinath.mannam@broadcom.com> wrote: > > Few SOCs have limitation that their PCIe host can't allow few inbound > address ranges. Allowed inbound address ranges are listed in dma-ranges > DT property and this address ranges are required to do IOVA mapping. > Remaining address ranges have to be reserved in IOVA mapping. > > PCIe Host driver of those SOCs has to list all address ranges which have > to reserve their IOVA address into PCIe host bridge resource entry list. > IOMMU framework will reserve these IOVAs while initializing IOMMU domain. > > This patch set is based on Linux-5.0-rc2. > > Changes from v2: > - Patch set rebased to Linux-5.0-rc2 > > Changes from v1: > - Addressed Oza review comments. > > Srinath Mannam (3): > PCI: Add dma-resv window list > iommu/dma: Reserve IOVA for PCI host reserve address list > PCI: iproc: Add dma reserve resources to host > > drivers/iommu/dma-iommu.c | 8 ++++++ > drivers/pci/controller/pcie-iproc.c | 51 ++++++++++++++++++++++++++++++++++++- > drivers/pci/probe.c | 3 +++ > include/linux/pci.h | 1 + > 4 files changed, 62 insertions(+), 1 deletion(-) > > -- > 2.7.4 >
Hi Bjorn, Could you please help to review the patch series below? Thanks, Srinath. On Thu, Feb 21, 2019 at 2:29 PM Srinath Mannam <srinath.mannam@broadcom.com> wrote: > > Hi Bjorn, > > Please help to review this patch series. > Thank you. > > Regards, > Srinath. > On Fri, Jan 25, 2019 at 3:44 PM Srinath Mannam > <srinath.mannam@broadcom.com> wrote: > > > > Few SOCs have limitation that their PCIe host can't allow few inbound > > address ranges. Allowed inbound address ranges are listed in dma-ranges > > DT property and this address ranges are required to do IOVA mapping. > > Remaining address ranges have to be reserved in IOVA mapping. > > > > PCIe Host driver of those SOCs has to list all address ranges which have > > to reserve their IOVA address into PCIe host bridge resource entry list. > > IOMMU framework will reserve these IOVAs while initializing IOMMU domain. > > > > This patch set is based on Linux-5.0-rc2. > > > > Changes from v2: > > - Patch set rebased to Linux-5.0-rc2 > > > > Changes from v1: > > - Addressed Oza review comments. > > > > Srinath Mannam (3): > > PCI: Add dma-resv window list > > iommu/dma: Reserve IOVA for PCI host reserve address list > > PCI: iproc: Add dma reserve resources to host > > > > drivers/iommu/dma-iommu.c | 8 ++++++ > > drivers/pci/controller/pcie-iproc.c | 51 ++++++++++++++++++++++++++++++++++++- > > drivers/pci/probe.c | 3 +++ > > include/linux/pci.h | 1 + > > 4 files changed, 62 insertions(+), 1 deletion(-) > > > > -- > > 2.7.4 > >
On 25/01/2019 10:13, Srinath Mannam wrote: > Few SOCs have limitation that their PCIe host can't allow few inbound > address ranges. Allowed inbound address ranges are listed in dma-ranges > DT property and this address ranges are required to do IOVA mapping. > Remaining address ranges have to be reserved in IOVA mapping. > > PCIe Host driver of those SOCs has to list all address ranges which have > to reserve their IOVA address into PCIe host bridge resource entry list. > IOMMU framework will reserve these IOVAs while initializing IOMMU domain. FWIW I'm still only interested in solving this problem generically, because in principle it's not specific to PCI, for PCI it's certainly not specific to iproc, and either way it's not specific to DT. That said, I don't care strongly enough to keep pushing back on this implementation outright, since it's not something which couldn't be cleaned up 'properly' in future. One general comment I'd make, though, is that AFAIK PCI has a concept of inbound windows much more than it has a concept of gaps-between-windows, so if the PCI layer is going to track anything it should probably be the actual windows, and leave the DMA layer to invert them into the reservations it cares about as it consumes the list. That way you can also avoid the undocumented requirement for the firmware to keep the ranges property sorted in the first place. Robin. > > This patch set is based on Linux-5.0-rc2. > > Changes from v2: > - Patch set rebased to Linux-5.0-rc2 > > Changes from v1: > - Addressed Oza review comments. > > Srinath Mannam (3): > PCI: Add dma-resv window list > iommu/dma: Reserve IOVA for PCI host reserve address list > PCI: iproc: Add dma reserve resources to host > > drivers/iommu/dma-iommu.c | 8 ++++++ > drivers/pci/controller/pcie-iproc.c | 51 ++++++++++++++++++++++++++++++++++++- > drivers/pci/probe.c | 3 +++ > include/linux/pci.h | 1 + > 4 files changed, 62 insertions(+), 1 deletion(-) >
Hi Robin, Thanks for your feedback. Please see my reply in line. On Wed, Mar 27, 2019 at 8:32 PM Robin Murphy <robin.murphy@arm.com> wrote: > > On 25/01/2019 10:13, Srinath Mannam wrote: > > Few SOCs have limitation that their PCIe host can't allow few inbound > > address ranges. Allowed inbound address ranges are listed in dma-ranges > > DT property and this address ranges are required to do IOVA mapping. > > Remaining address ranges have to be reserved in IOVA mapping. > > > > PCIe Host driver of those SOCs has to list all address ranges which have > > to reserve their IOVA address into PCIe host bridge resource entry list. > > IOMMU framework will reserve these IOVAs while initializing IOMMU domain. > > FWIW I'm still only interested in solving this problem generically, > because in principle it's not specific to PCI, for PCI it's certainly > not specific to iproc, and either way it's not specific to DT. That > said, I don't care strongly enough to keep pushing back on this > implementation outright, since it's not something which couldn't be > cleaned up 'properly' in future. Iproc PCIe host controller supports inbound address translation feature to restrict access to allowed address ranges. so that allowed memory ranges need to program to controller. allowed address ranges information is passed to controller driver through dma-ranges DT property. This feature is specific to iproc PCIe controller, so that I think this change has to specific to iproc PCIe driver and DT. Here I followed the same way how PCI IO regions are reserved "iova_reserve_pci_windows". so that this change also specific to PCI. > > One general comment I'd make, though, is that AFAIK PCI has a concept of > inbound windows much more than it has a concept of gaps-between-windows, > so if the PCI layer is going to track anything it should probably be the > actual windows, and leave the DMA layer to invert them into the > reservations it cares about as it consumes the list. That way you can > also avoid the undocumented requirement for the firmware to keep the > ranges property sorted in the first place. This implementation has three parts. 1. parsing dma-ranges and extract allowed and reserved address ranges. 2. program allowed ranges to iproc PCIe controller. 3. reserve list of reserved address ranges in IOMMU layer. #1 and #2 are done using "of_pci_dma_range_parser_init" in present iproc PCIe driver. so that, I listed reserve windows at the same place. #3 requires list of reserve windows so that I add new variable(dma_resv) to carry these reserve windows list to iommu layer from iproc driver layer. The reasons to not use DMA layer for parsing dma-ranges are, 1. This feature is not generic for all SOCs. 2. To avoid dam-ranges parsing in multiple places, already done in iproc pcie driver. 3. Need to do modify standard DMA layer source code "of_dma_configure" 4. required a carrier to pass reserved windows list from DMA layer to IOMMU layer. 5. I followed existing PCIe IO regions reserve procedure done in IOMMU layer. Regards, Srinath. > > Robin. > > > > > This patch set is based on Linux-5.0-rc2. > > > > Changes from v2: > > - Patch set rebased to Linux-5.0-rc2 > > > > Changes from v1: > > - Addressed Oza review comments. > > > > Srinath Mannam (3): > > PCI: Add dma-resv window list > > iommu/dma: Reserve IOVA for PCI host reserve address list > > PCI: iproc: Add dma reserve resources to host > > > > drivers/iommu/dma-iommu.c | 8 ++++++ > > drivers/pci/controller/pcie-iproc.c | 51 ++++++++++++++++++++++++++++++++++++- > > drivers/pci/probe.c | 3 +++ > > include/linux/pci.h | 1 + > > 4 files changed, 62 insertions(+), 1 deletion(-) > >
On 28/03/2019 10:34, Srinath Mannam wrote: > Hi Robin, > > Thanks for your feedback. Please see my reply in line. > > On Wed, Mar 27, 2019 at 8:32 PM Robin Murphy <robin.murphy@arm.com> wrote: >> >> On 25/01/2019 10:13, Srinath Mannam wrote: >>> Few SOCs have limitation that their PCIe host can't allow few inbound >>> address ranges. Allowed inbound address ranges are listed in dma-ranges >>> DT property and this address ranges are required to do IOVA mapping. >>> Remaining address ranges have to be reserved in IOVA mapping. >>> >>> PCIe Host driver of those SOCs has to list all address ranges which have >>> to reserve their IOVA address into PCIe host bridge resource entry list. >>> IOMMU framework will reserve these IOVAs while initializing IOMMU domain. >> >> FWIW I'm still only interested in solving this problem generically, >> because in principle it's not specific to PCI, for PCI it's certainly >> not specific to iproc, and either way it's not specific to DT. That >> said, I don't care strongly enough to keep pushing back on this >> implementation outright, since it's not something which couldn't be >> cleaned up 'properly' in future. > Iproc PCIe host controller supports inbound address translation > feature to restrict access > to allowed address ranges. so that allowed memory ranges need to > program to controller. Other PCIe host controllers work that way too - I know, because I've got one here. In this particular case, it's not explicit "restriction" so much as just that the window configuration controls what AXI attributes are generated on the master side of the PCIe-AXI bridge, and there is no default attribute. Thus if a PCIe transaction doesn't hit one of the windows it simply cannot propagate across to the AXI side because the RC won't know what attributes to emit. It may be conceptually a very slightly different problem statement, but it still wants the exact same solution. > allowed address ranges information is passed to controller driver > through dma-ranges DT property. And ACPI has a direct equivalent of dma-ranges in the form of the _DMA method - compare of_dma_get_range() and acpi_dma_get_range(). Again, platforms already exist which have this kind of hardware limitation and boot with both DT and ACPI. > This feature is specific to iproc PCIe controller, so that I think > this change has to specific to iproc > PCIe driver and DT. The general concept of devices having inaccessible holes within their nominal DMA mask ultimately boils down to how creative SoC designers can be with interconnect topologies, so in principle it could end up being relevant just about anywhere. But as I implied before, since the examples we know about today all seem to be PCIe IPs, it's not all that unreasonable to start with this PCI-specific workaround now, and generalise it later as necessary. > Here I followed the same way how PCI IO regions are reserved > "iova_reserve_pci_windows". so that this > change also specific to PCI. >> >> One general comment I'd make, though, is that AFAIK PCI has a concept of >> inbound windows much more than it has a concept of gaps-between-windows, >> so if the PCI layer is going to track anything it should probably be the >> actual windows, and leave the DMA layer to invert them into the >> reservations it cares about as it consumes the list. That way you can >> also avoid the undocumented requirement for the firmware to keep the >> ranges property sorted in the first place. > This implementation has three parts. > 1. parsing dma-ranges and extract allowed and reserved address ranges. > 2. program allowed ranges to iproc PCIe controller. > 3. reserve list of reserved address ranges in IOMMU layer. > #1 and #2 are done using "of_pci_dma_range_parser_init" in present > iproc PCIe driver. > so that, I listed reserve windows at the same place. > #3 requires list of reserve windows so that I add new > variable(dma_resv) to carry these > reserve windows list to iommu layer from iproc driver layer. > The reasons to not use DMA layer for parsing dma-ranges are, > 1. This feature is not generic for all SOCs. > 2. To avoid dam-ranges parsing in multiple places, already done in > iproc pcie driver. > 3. Need to do modify standard DMA layer source code "of_dma_configure" > 4. required a carrier to pass reserved windows list from DMA layer to > IOMMU layer. > 5. I followed existing PCIe IO regions reserve procedure done in IOMMU layer. Sure, I get that - sorry if it was unclear, but all I meant was simply taking the flow you currently have, i.e.: pcie-iproc: parse dma-ranges and make list of gaps between regions dma-iommu: process list and reserve entries and tweaking it into this: pcie-iproc: parse dma-ranges and make list of regions dma-iommu: process list and reserve gaps between entries which has the nice benefit of being more robust since the first step can easily construct the list in correctly-sorted order regardless of the order in which the DT ranges appear. Robin.
Hi Robin, Thanks a lot for detailed clarification. I will send next patch set with the changes you suggested. Regards, Srinath. On Thu, Mar 28, 2019 at 9:17 PM Robin Murphy <robin.murphy@arm.com> wrote: > > On 28/03/2019 10:34, Srinath Mannam wrote: > > Hi Robin, > > > > Thanks for your feedback. Please see my reply in line. > > > > On Wed, Mar 27, 2019 at 8:32 PM Robin Murphy <robin.murphy@arm.com> wrote: > >> > >> On 25/01/2019 10:13, Srinath Mannam wrote: > >>> Few SOCs have limitation that their PCIe host can't allow few inbound > >>> address ranges. Allowed inbound address ranges are listed in dma-ranges > >>> DT property and this address ranges are required to do IOVA mapping. > >>> Remaining address ranges have to be reserved in IOVA mapping. > >>> > >>> PCIe Host driver of those SOCs has to list all address ranges which have > >>> to reserve their IOVA address into PCIe host bridge resource entry list. > >>> IOMMU framework will reserve these IOVAs while initializing IOMMU domain. > >> > >> FWIW I'm still only interested in solving this problem generically, > >> because in principle it's not specific to PCI, for PCI it's certainly > >> not specific to iproc, and either way it's not specific to DT. That > >> said, I don't care strongly enough to keep pushing back on this > >> implementation outright, since it's not something which couldn't be > >> cleaned up 'properly' in future. > > Iproc PCIe host controller supports inbound address translation > > feature to restrict access > > to allowed address ranges. so that allowed memory ranges need to > > program to controller. > > Other PCIe host controllers work that way too - I know, because I've got > one here. In this particular case, it's not explicit "restriction" so > much as just that the window configuration controls what AXI attributes > are generated on the master side of the PCIe-AXI bridge, and there is no > default attribute. Thus if a PCIe transaction doesn't hit one of the > windows it simply cannot propagate across to the AXI side because the RC > won't know what attributes to emit. It may be conceptually a very > slightly different problem statement, but it still wants the exact same > solution. > > > allowed address ranges information is passed to controller driver > > through dma-ranges DT property. > > And ACPI has a direct equivalent of dma-ranges in the form of the _DMA > method - compare of_dma_get_range() and acpi_dma_get_range(). Again, > platforms already exist which have this kind of hardware limitation and > boot with both DT and ACPI. > > > This feature is specific to iproc PCIe controller, so that I think > > this change has to specific to iproc > > PCIe driver and DT. > > The general concept of devices having inaccessible holes within their > nominal DMA mask ultimately boils down to how creative SoC designers can > be with interconnect topologies, so in principle it could end up being > relevant just about anywhere. But as I implied before, since the > examples we know about today all seem to be PCIe IPs, it's not all that > unreasonable to start with this PCI-specific workaround now, and > generalise it later as necessary. > > > Here I followed the same way how PCI IO regions are reserved > > "iova_reserve_pci_windows". so that this > > change also specific to PCI. > >> > >> One general comment I'd make, though, is that AFAIK PCI has a concept of > >> inbound windows much more than it has a concept of gaps-between-windows, > >> so if the PCI layer is going to track anything it should probably be the > >> actual windows, and leave the DMA layer to invert them into the > >> reservations it cares about as it consumes the list. That way you can > >> also avoid the undocumented requirement for the firmware to keep the > >> ranges property sorted in the first place. > > This implementation has three parts. > > 1. parsing dma-ranges and extract allowed and reserved address ranges. > > 2. program allowed ranges to iproc PCIe controller. > > 3. reserve list of reserved address ranges in IOMMU layer. > > #1 and #2 are done using "of_pci_dma_range_parser_init" in present > > iproc PCIe driver. > > so that, I listed reserve windows at the same place. > > #3 requires list of reserve windows so that I add new > > variable(dma_resv) to carry these > > reserve windows list to iommu layer from iproc driver layer. > > The reasons to not use DMA layer for parsing dma-ranges are, > > 1. This feature is not generic for all SOCs. > > 2. To avoid dam-ranges parsing in multiple places, already done in > > iproc pcie driver. > > 3. Need to do modify standard DMA layer source code "of_dma_configure" > > 4. required a carrier to pass reserved windows list from DMA layer to > > IOMMU layer. > > 5. I followed existing PCIe IO regions reserve procedure done in IOMMU layer. > > Sure, I get that - sorry if it was unclear, but all I meant was simply > taking the flow you currently have, i.e.: > > pcie-iproc: parse dma-ranges and make list of gaps between regions > dma-iommu: process list and reserve entries > > and tweaking it into this: > > pcie-iproc: parse dma-ranges and make list of regions > dma-iommu: process list and reserve gaps between entries > > which has the nice benefit of being more robust since the first step can > easily construct the list in correctly-sorted order regardless of the > order in which the DT ranges appear. > > Robin.