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 resource entries of allowed
address ranges given in dma-ranges DT property in sorted order. This
sorted list of resources will be processed and reserve IOVA address for
inaccessible address holes while initializing IOMMU domain.
This patch set is based on Linux-5.0-rc2.
Changes from v3:
- Addressed Robin Murphy review comments.
- pcie-iproc: parse dma-ranges and make sorted resource list.
- dma-iommu: process list and reserve gaps between entries
Changes from v2:
- Patch set rebased to Linux-5.0-rc2
Changes from v1:
- Addressed Oza review comments.
Srinath Mannam (3):
PCI: Add dma_ranges window list
iommu/dma: Reserve IOVA for PCIe inaccessible DMA address
PCI: iproc: Add sorted dma ranges resource entries to host bridge
drivers/iommu/dma-iommu.c | 19 ++++++++++++++++
drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
drivers/pci/probe.c | 3 +++
include/linux/pci.h | 1 +
4 files changed, 66 insertions(+), 1 deletion(-)
--
2.7.4
Add a dma_ranges field in PCI host bridge structure to hold resource
entries list of memory regions in sorted order given through dma-ranges
DT property.
While initializing IOMMU domain of PCI EPs connected to that host bridge
This list of resources will be processed and IOVAs for the address holes
will be reserved.
Signed-off-by: Srinath Mannam <[email protected]>
Based-on-patch-by: Oza Pawandeep <[email protected]>
Reviewed-by: Oza Pawandeep <[email protected]>
---
drivers/pci/probe.c | 3 +++
include/linux/pci.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 257b9f6..ce5505f 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -544,6 +544,7 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
return NULL;
INIT_LIST_HEAD(&bridge->windows);
+ INIT_LIST_HEAD(&bridge->dma_ranges);
bridge->dev.release = pci_release_host_bridge_dev;
/*
@@ -572,6 +573,7 @@ struct pci_host_bridge *devm_pci_alloc_host_bridge(struct device *dev,
return NULL;
INIT_LIST_HEAD(&bridge->windows);
+ INIT_LIST_HEAD(&bridge->dma_ranges);
bridge->dev.release = devm_pci_release_host_bridge_dev;
return bridge;
@@ -581,6 +583,7 @@ EXPORT_SYMBOL(devm_pci_alloc_host_bridge);
void pci_free_host_bridge(struct pci_host_bridge *bridge)
{
pci_free_resource_list(&bridge->windows);
+ pci_free_resource_list(&bridge->dma_ranges);
kfree(bridge);
}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 65f1d8c..016a044 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -487,6 +487,7 @@ struct pci_host_bridge {
void *sysdata;
int busnr;
struct list_head windows; /* resource_entry */
+ struct list_head dma_ranges; /* dma ranges resource list */
u8 (*swizzle_irq)(struct pci_dev *, u8 *); /* Platform IRQ swizzler */
int (*map_irq)(const struct pci_dev *, u8, u8);
void (*release_fn)(struct pci_host_bridge *);
--
2.7.4
IPROC host has the limitation that it can use only those address ranges
given by dma-ranges property as inbound address. So that the memory
address holes in dma-ranges should be reserved to allocate as DMA address.
Inbound address of host accessed by PCIe devices will not be translated
before it comes to IOMMU or directly to PE. But the limitation of this
host is, access to few address ranges are ignored. So that IOVA ranges
for these address ranges have to be reserved.
All allowed address ranges are listed in dma-ranges DT parameter. These
address ranges are converted as resource entries and listed in sorted
order add added to dma_ranges list of PCI host bridge structure.
Ex:
dma-ranges = < \
0x43000000 0x00 0x80000000 0x00 0x80000000 0x00 0x80000000 \
0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
0x43000000 0x80 0x00000000 0x80 0x00000000 0x40 0x00000000>
In the above example of dma-ranges, memory address from
0x0 - 0x80000000,
0x100000000 - 0x800000000,
0x1000000000 - 0x8000000000 and
0x10000000000 - 0xffffffffffffffff.
are not allowed to use as inbound addresses.
Signed-off-by: Srinath Mannam <[email protected]>
Based-on-patch-by: Oza Pawandeep <[email protected]>
Reviewed-by: Oza Pawandeep <[email protected]>
---
drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c
index c20fd6b..94ba5c0 100644
--- a/drivers/pci/controller/pcie-iproc.c
+++ b/drivers/pci/controller/pcie-iproc.c
@@ -1146,11 +1146,43 @@ static int iproc_pcie_setup_ib(struct iproc_pcie *pcie,
return ret;
}
+static int
+iproc_pcie_add_dma_range(struct device *dev, struct list_head *resources,
+ struct of_pci_range *range)
+{
+ struct resource *res;
+ struct resource_entry *entry, *tmp;
+ struct list_head *head = resources;
+
+ res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
+ if (!res)
+ return -ENOMEM;
+
+ resource_list_for_each_entry(tmp, resources) {
+ if (tmp->res->start < range->cpu_addr)
+ head = &tmp->node;
+ }
+
+ res->start = range->cpu_addr;
+ res->end = res->start + range->size - 1;
+
+ entry = resource_list_create_entry(res, 0);
+ if (!entry)
+ return -ENOMEM;
+
+ entry->offset = res->start - range->cpu_addr;
+ resource_list_add(entry, head);
+
+ return 0;
+}
+
static int iproc_pcie_map_dma_ranges(struct iproc_pcie *pcie)
{
+ struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
struct of_pci_range range;
struct of_pci_range_parser parser;
int ret;
+ LIST_HEAD(resources);
/* Get the dma-ranges from DT */
ret = of_pci_dma_range_parser_init(&parser, pcie->dev->of_node);
@@ -1158,13 +1190,23 @@ static int iproc_pcie_map_dma_ranges(struct iproc_pcie *pcie)
return ret;
for_each_of_pci_range(&parser, &range) {
+ ret = iproc_pcie_add_dma_range(pcie->dev,
+ &resources,
+ &range);
+ if (ret)
+ goto out;
/* Each range entry corresponds to an inbound mapping region */
ret = iproc_pcie_setup_ib(pcie, &range, IPROC_PCIE_IB_MAP_MEM);
if (ret)
- return ret;
+ goto out;
}
+ list_splice_init(&resources, &host->dma_ranges);
+
return 0;
+out:
+ pci_free_resource_list(&resources);
+ return ret;
}
static int iproce_pcie_get_msi(struct iproc_pcie *pcie,
--
2.7.4
dma_ranges field of PCI host bridge structure has resource entries in
sorted order of address range given through dma-ranges DT property. This
list is the accessible DMA address range. So that this resource list will
be processed and reserve IOVA address to the inaccessible address holes in
the list.
This method is similar to PCI IO resources address ranges reserving in
IOMMU for each EP connected to host bridge.
Signed-off-by: Srinath Mannam <[email protected]>
Based-on-patch-by: Oza Pawandeep <[email protected]>
Reviewed-by: Oza Pawandeep <[email protected]>
---
drivers/iommu/dma-iommu.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d19f3d6..fb42d7c 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -212,6 +212,7 @@ static void iova_reserve_pci_windows(struct pci_dev *dev,
struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus);
struct resource_entry *window;
unsigned long lo, hi;
+ phys_addr_t start = 0, end;
resource_list_for_each_entry(window, &bridge->windows) {
if (resource_type(window->res) != IORESOURCE_MEM)
@@ -221,6 +222,24 @@ static void iova_reserve_pci_windows(struct pci_dev *dev,
hi = iova_pfn(iovad, window->res->end - window->offset);
reserve_iova(iovad, lo, hi);
}
+
+ /* Get reserved DMA windows from host bridge */
+ resource_list_for_each_entry(window, &bridge->dma_ranges) {
+ end = window->res->start - window->offset;
+resv_iova:
+ if (end - start) {
+ lo = iova_pfn(iovad, start);
+ hi = iova_pfn(iovad, end);
+ reserve_iova(iovad, lo, hi);
+ }
+ start = window->res->end - window->offset + 1;
+ /* If window is last entry */
+ if (window->node.next == &bridge->dma_ranges &&
+ end != DMA_BIT_MASK(sizeof(dma_addr_t) * BITS_PER_BYTE)) {
+ end = DMA_BIT_MASK(sizeof(dma_addr_t) * BITS_PER_BYTE);
+ goto resv_iova;
+ }
+ }
}
static int iova_reserve_iommu_regions(struct device *dev,
--
2.7.4
On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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.
If I understand correctly, devices below these PCIe host bridges can
DMA only to the listed address ranges, and you prevent devices from
doing DMA to the holes between the listed ranges by reserving the
holes in dma-iommu.
Apparently there's something that makes sure driver dma_map_*() always
goes through dma-iommu? I traced as far as seeing that dma-iommu
depends on CONFIG_IOMMU_DMA, and that arm64 selects CONFIG_IOMMU_DMA
if CONFIG_IOMMU_SUPPORT, but then the trail got cold. I didn't see
what selects CONFIG_IOMMU_SUPPORT.
This does look like what Robin suggested, as far as I can tell.
Hopefully he'll take a look and give his reviewed-by. Thanks for
persevering!
> PCIe Host driver of those SOCs has to list resource entries of allowed
> address ranges given in dma-ranges DT property in sorted order. This
> sorted list of resources will be processed and reserve IOVA address for
> inaccessible address holes while initializing IOMMU domain.
>
> This patch set is based on Linux-5.0-rc2.
>
> Changes from v3:
> - Addressed Robin Murphy review comments.
> - pcie-iproc: parse dma-ranges and make sorted resource list.
> - dma-iommu: process list and reserve gaps between entries
>
> Changes from v2:
> - Patch set rebased to Linux-5.0-rc2
>
> Changes from v1:
> - Addressed Oza review comments.
>
> Srinath Mannam (3):
> PCI: Add dma_ranges window list
> iommu/dma: Reserve IOVA for PCIe inaccessible DMA address
> PCI: iproc: Add sorted dma ranges resource entries to host bridge
>
> drivers/iommu/dma-iommu.c | 19 ++++++++++++++++
> drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> drivers/pci/probe.c | 3 +++
> include/linux/pci.h | 1 +
> 4 files changed, 66 insertions(+), 1 deletion(-)
>
> --
> 2.7.4
>
Hi Bjorn,
Thanks for review. Please find my reply below.
On Sat, Apr 13, 2019 at 4:04 AM Bjorn Helgaas <[email protected]> wrote:
>
> On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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.
>
> If I understand correctly, devices below these PCIe host bridges can
> DMA only to the listed address ranges, and you prevent devices from
> doing DMA to the holes between the listed ranges by reserving the
> holes in dma-iommu.
Yes, devices below these PCIe host bridges can DMA only to the listed
address ranges,
and this patch prevents to allocate DMA(IOVA) addresses in the holes
of listed ranges.
>
> Apparently there's something that makes sure driver dma_map_*() always
> goes through dma-iommu? I traced as far as seeing that dma-iommu
> depends on CONFIG_IOMMU_DMA, and that arm64 selects CONFIG_IOMMU_DMA
> if CONFIG_IOMMU_SUPPORT, but then the trail got cold. I didn't see
> what selects CONFIG_IOMMU_SUPPORT.
IOMMU_SUPPORT depends on MMU.
>
> This does look like what Robin suggested, as far as I can tell.
> Hopefully he'll take a look and give his reviewed-by. Thanks for
> persevering!
Thank you.
Regards,
Srinath.
>
> > PCIe Host driver of those SOCs has to list resource entries of allowed
> > address ranges given in dma-ranges DT property in sorted order. This
> > sorted list of resources will be processed and reserve IOVA address for
> > inaccessible address holes while initializing IOMMU domain.
> >
> > This patch set is based on Linux-5.0-rc2.
> >
> > Changes from v3:
> > - Addressed Robin Murphy review comments.
> > - pcie-iproc: parse dma-ranges and make sorted resource list.
> > - dma-iommu: process list and reserve gaps between entries
> >
> > Changes from v2:
> > - Patch set rebased to Linux-5.0-rc2
> >
> > Changes from v1:
> > - Addressed Oza review comments.
> >
> > Srinath Mannam (3):
> > PCI: Add dma_ranges window list
> > iommu/dma: Reserve IOVA for PCIe inaccessible DMA address
> > PCI: iproc: Add sorted dma ranges resource entries to host bridge
> >
> > drivers/iommu/dma-iommu.c | 19 ++++++++++++++++
> > drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> > drivers/pci/probe.c | 3 +++
> > include/linux/pci.h | 1 +
> > 4 files changed, 66 insertions(+), 1 deletion(-)
> >
> > --
> > 2.7.4
> >
On Tue, Apr 16, 2019 at 05:28:36PM +0530, Srinath Mannam wrote:
> On Sat, Apr 13, 2019 at 4:04 AM Bjorn Helgaas <[email protected]> wrote:
> > On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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.
>
> > If I understand correctly, devices below these PCIe host bridges can
> > DMA only to the listed address ranges, and you prevent devices from
> > doing DMA to the holes between the listed ranges by reserving the
> > holes in dma-iommu.
>
> Yes, devices below these PCIe host bridges can DMA only to the listed
> address ranges,
> and this patch prevents to allocate DMA(IOVA) addresses in the holes
> of listed ranges.
>
> > Apparently there's something that makes sure driver dma_map_*() always
> > goes through dma-iommu? I traced as far as seeing that dma-iommu
> > depends on CONFIG_IOMMU_DMA, and that arm64 selects CONFIG_IOMMU_DMA
> > if CONFIG_IOMMU_SUPPORT, but then the trail got cold. I didn't see
> > what selects CONFIG_IOMMU_SUPPORT.
>
> IOMMU_SUPPORT depends on MMU.
Yes, I see that IOMMU_SUPPORT depends on MMU (in
drivers/iommu/Kconfig). But that doesn't *select* IOMMU_SUPPORT; it
only means you *can't* select it unless MMU has already been selected.
I think you only get dma-iommu if you choose to select IOMMU_SUPPORT
via menuconfig or whatever, and the current config rules allow you to
turn that off. Maybe that's OK, I dunno. If you do turn it off, I
guess we'll ignore the holes in "dma-ranges" and devices will be able
to DMA to the holes.
> > This does look like what Robin suggested, as far as I can tell.
> > Hopefully he'll take a look and give his reviewed-by. Thanks for
> > persevering!
> Thank you.
>
> Regards,
> Srinath.
> >
> > > PCIe Host driver of those SOCs has to list resource entries of allowed
> > > address ranges given in dma-ranges DT property in sorted order. This
> > > sorted list of resources will be processed and reserve IOVA address for
> > > inaccessible address holes while initializing IOMMU domain.
> > >
> > > This patch set is based on Linux-5.0-rc2.
> > >
> > > Changes from v3:
> > > - Addressed Robin Murphy review comments.
> > > - pcie-iproc: parse dma-ranges and make sorted resource list.
> > > - dma-iommu: process list and reserve gaps between entries
> > >
> > > Changes from v2:
> > > - Patch set rebased to Linux-5.0-rc2
> > >
> > > Changes from v1:
> > > - Addressed Oza review comments.
> > >
> > > Srinath Mannam (3):
> > > PCI: Add dma_ranges window list
> > > iommu/dma: Reserve IOVA for PCIe inaccessible DMA address
> > > PCI: iproc: Add sorted dma ranges resource entries to host bridge
> > >
> > > drivers/iommu/dma-iommu.c | 19 ++++++++++++++++
> > > drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> > > drivers/pci/probe.c | 3 +++
> > > include/linux/pci.h | 1 +
> > > 4 files changed, 66 insertions(+), 1 deletion(-)
> > >
> > > --
> > > 2.7.4
> > >
On Fri, Apr 12, 2019 at 08:43:33AM +0530, Srinath Mannam wrote:
> Add a dma_ranges field in PCI host bridge structure to hold resource
> entries list of memory regions in sorted order given through dma-ranges
> DT property.
>
> While initializing IOMMU domain of PCI EPs connected to that host bridge
> This list of resources will be processed and IOVAs for the address holes
> will be reserved.
s/bridge This list/bridge, this list/
> Signed-off-by: Srinath Mannam <[email protected]>
> Based-on-patch-by: Oza Pawandeep <[email protected]>
> Reviewed-by: Oza Pawandeep <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
> ---
> drivers/pci/probe.c | 3 +++
> include/linux/pci.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 257b9f6..ce5505f 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -544,6 +544,7 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
> return NULL;
>
> INIT_LIST_HEAD(&bridge->windows);
> + INIT_LIST_HEAD(&bridge->dma_ranges);
> bridge->dev.release = pci_release_host_bridge_dev;
>
> /*
> @@ -572,6 +573,7 @@ struct pci_host_bridge *devm_pci_alloc_host_bridge(struct device *dev,
> return NULL;
>
> INIT_LIST_HEAD(&bridge->windows);
> + INIT_LIST_HEAD(&bridge->dma_ranges);
> bridge->dev.release = devm_pci_release_host_bridge_dev;
>
> return bridge;
> @@ -581,6 +583,7 @@ EXPORT_SYMBOL(devm_pci_alloc_host_bridge);
> void pci_free_host_bridge(struct pci_host_bridge *bridge)
> {
> pci_free_resource_list(&bridge->windows);
> + pci_free_resource_list(&bridge->dma_ranges);
>
> kfree(bridge);
> }
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 65f1d8c..016a044 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -487,6 +487,7 @@ struct pci_host_bridge {
> void *sysdata;
> int busnr;
> struct list_head windows; /* resource_entry */
> + struct list_head dma_ranges; /* dma ranges resource list */
> u8 (*swizzle_irq)(struct pci_dev *, u8 *); /* Platform IRQ swizzler */
> int (*map_irq)(const struct pci_dev *, u8, u8);
> void (*release_fn)(struct pci_host_bridge *);
> --
> 2.7.4
>
[+cc Scott]
On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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 resource entries of allowed
> address ranges given in dma-ranges DT property in sorted order. This
> sorted list of resources will be processed and reserve IOVA address for
> inaccessible address holes while initializing IOMMU domain.
>
> This patch set is based on Linux-5.0-rc2.
>
> Changes from v3:
> - Addressed Robin Murphy review comments.
> - pcie-iproc: parse dma-ranges and make sorted resource list.
> - dma-iommu: process list and reserve gaps between entries
>
> Changes from v2:
> - Patch set rebased to Linux-5.0-rc2
>
> Changes from v1:
> - Addressed Oza review comments.
>
> Srinath Mannam (3):
> PCI: Add dma_ranges window list
> iommu/dma: Reserve IOVA for PCIe inaccessible DMA address
> PCI: iproc: Add sorted dma ranges resource entries to host bridge
>
> drivers/iommu/dma-iommu.c | 19 ++++++++++++++++
> drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> drivers/pci/probe.c | 3 +++
> include/linux/pci.h | 1 +
> 4 files changed, 66 insertions(+), 1 deletion(-)
To make progress on this, I think we need an ack from Joerg for the
dma-iommu.c part, an ack from Ray or Scott for the pcie-iproc.c part,
and an ack from Robin for the thing as a whole.
Then I would say Lorenzo should take a look and merge if he approves, since
pcie-iproc.c contains the most changes.
Bjorn
On Thu, Apr 18, 2019 at 06:48:28PM -0500, Bjorn Helgaas wrote:
> To make progress on this, I think we need an ack from Joerg for the
> dma-iommu.c part, an ack from Ray or Scott for the pcie-iproc.c part,
> and an ack from Robin for the thing as a whole.
I wait for Robin to review the dma-iommu change. If he is okay with it,
I am too.
Joerg
On 12/04/2019 04:13, Srinath Mannam wrote:
> dma_ranges field of PCI host bridge structure has resource entries in
> sorted order of address range given through dma-ranges DT property. This
> list is the accessible DMA address range. So that this resource list will
> be processed and reserve IOVA address to the inaccessible address holes in
> the list.
>
> This method is similar to PCI IO resources address ranges reserving in
> IOMMU for each EP connected to host bridge.
>
> Signed-off-by: Srinath Mannam <[email protected]>
> Based-on-patch-by: Oza Pawandeep <[email protected]>
> Reviewed-by: Oza Pawandeep <[email protected]>
> ---
> drivers/iommu/dma-iommu.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index d19f3d6..fb42d7c 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -212,6 +212,7 @@ static void iova_reserve_pci_windows(struct pci_dev *dev,
> struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus);
> struct resource_entry *window;
> unsigned long lo, hi;
> + phys_addr_t start = 0, end;
>
> resource_list_for_each_entry(window, &bridge->windows) {
> if (resource_type(window->res) != IORESOURCE_MEM)
> @@ -221,6 +222,24 @@ static void iova_reserve_pci_windows(struct pci_dev *dev,
> hi = iova_pfn(iovad, window->res->end - window->offset);
> reserve_iova(iovad, lo, hi);
> }
> +
> + /* Get reserved DMA windows from host bridge */
> + resource_list_for_each_entry(window, &bridge->dma_ranges) {
> + end = window->res->start - window->offset;
> +resv_iova:
> + if (end - start) {
> + lo = iova_pfn(iovad, start);
> + hi = iova_pfn(iovad, end);
> + reserve_iova(iovad, lo, hi);
> + }
> + start = window->res->end - window->offset + 1;
> + /* If window is last entry */
> + if (window->node.next == &bridge->dma_ranges &&
> + end != DMA_BIT_MASK(sizeof(dma_addr_t) * BITS_PER_BYTE)) {
I still think that's a very silly way to write "~(dma_addr_t)0", but
otherwise,
Acked-by: Robin Murphy <[email protected]>
> + end = DMA_BIT_MASK(sizeof(dma_addr_t) * BITS_PER_BYTE);
> + goto resv_iova;
> + }
> + }
> }
>
> static int iova_reserve_iommu_regions(struct device *dev,
>
Hi Srinath,
On 4/12/19 5:13 AM, Srinath Mannam wrote:
> IPROC host has the limitation that it can use only those address ranges
> given by dma-ranges property as inbound address. So that the memory
> address holes in dma-ranges should be reserved to allocate as DMA address.
>
> Inbound address of host accessed by PCIe devices will not be translated
> before it comes to IOMMU or directly to PE. But the limitation of this
> host is, access to few address ranges are ignored. So that IOVA ranges
> for these address ranges have to be reserved.
>
> All allowed address ranges are listed in dma-ranges DT parameter. These
> address ranges are converted as resource entries and listed in sorted
> order add added to dma_ranges list of PCI host bridge structure.
s/add added/and added
>
> Ex:
> dma-ranges = < \
> 0x43000000 0x00 0x80000000 0x00 0x80000000 0x00 0x80000000 \
> 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
> 0x43000000 0x80 0x00000000 0x80 0x00000000 0x40 0x00000000>
>
> In the above example of dma-ranges, memory address from
> 0x0 - 0x80000000,
> 0x100000000 - 0x800000000,
> 0x1000000000 - 0x8000000000 and
> 0x10000000000 - 0xffffffffffffffff.
> are not allowed to use as inbound addresses.
s/to use/to be used
>
> Signed-off-by: Srinath Mannam <[email protected]>
> Based-on-patch-by: Oza Pawandeep <[email protected]>
> Reviewed-by: Oza Pawandeep <[email protected]>
> ---
> drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c
> index c20fd6b..94ba5c0 100644
> --- a/drivers/pci/controller/pcie-iproc.c
> +++ b/drivers/pci/controller/pcie-iproc.c
> @@ -1146,11 +1146,43 @@ static int iproc_pcie_setup_ib(struct iproc_pcie *pcie,
> return ret;
> }
>
> +static int
> +iproc_pcie_add_dma_range(struct device *dev, struct list_head *resources,
> + struct of_pci_range *range)
> +{
> + struct resource *res;
> + struct resource_entry *entry, *tmp;
> + struct list_head *head = resources;
> +
> + res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
> + if (!res)
> + return -ENOMEM;
> +
> + resource_list_for_each_entry(tmp, resources) {
> + if (tmp->res->start < range->cpu_addr)
> + head = &tmp->node;
> + }
> +
> + res->start = range->cpu_addr;
> + res->end = res->start + range->size - 1;
> +
> + entry = resource_list_create_entry(res, 0);
> + if (!entry)
> + return -ENOMEM;
> +
> + entry->offset = res->start - range->cpu_addr;
> + resource_list_add(entry, head);
> +
> + return 0;
> +}
> +
> static int iproc_pcie_map_dma_ranges(struct iproc_pcie *pcie)
> {
> + struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
> struct of_pci_range range;
> struct of_pci_range_parser parser;
> int ret;
> + LIST_HEAD(resources);
>
> /* Get the dma-ranges from DT */
> ret = of_pci_dma_range_parser_init(&parser, pcie->dev->of_node);
> @@ -1158,13 +1190,23 @@ static int iproc_pcie_map_dma_ranges(struct iproc_pcie *pcie)
> return ret;
>
> for_each_of_pci_range(&parser, &range) {
> + ret = iproc_pcie_add_dma_range(pcie->dev,
> + &resources,
> + &range);
> + if (ret)
> + goto out;
> /* Each range entry corresponds to an inbound mapping region */
> ret = iproc_pcie_setup_ib(pcie, &range, IPROC_PCIE_IB_MAP_MEM);
> if (ret)
> - return ret;
> + goto out;
> }
>
> + list_splice_init(&resources, &host->dma_ranges);
> +
> return 0;
> +out:
> + pci_free_resource_list(&resources);
> + return ret;
> }
>
> static int iproce_pcie_get_msi(struct iproc_pcie *pcie,
>
Reviewed-by: Eric Auger <[email protected]>
Thanks
Eric
On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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 resource entries of allowed
> address ranges given in dma-ranges DT property in sorted order. This
> sorted list of resources will be processed and reserve IOVA address for
> inaccessible address holes while initializing IOMMU domain.
>
> This patch set is based on Linux-5.0-rc2.
>
> Changes from v3:
> - Addressed Robin Murphy review comments.
> - pcie-iproc: parse dma-ranges and make sorted resource list.
> - dma-iommu: process list and reserve gaps between entries
>
> Changes from v2:
> - Patch set rebased to Linux-5.0-rc2
>
> Changes from v1:
> - Addressed Oza review comments.
>
> Srinath Mannam (3):
> PCI: Add dma_ranges window list
> iommu/dma: Reserve IOVA for PCIe inaccessible DMA address
> PCI: iproc: Add sorted dma ranges resource entries to host bridge
>
> drivers/iommu/dma-iommu.c | 19 ++++++++++++++++
> drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> drivers/pci/probe.c | 3 +++
> include/linux/pci.h | 1 +
> 4 files changed, 66 insertions(+), 1 deletion(-)
Bjorn, Joerg,
this series should not affect anything in the mainline other than its
consumer (ie patch 3); if that's the case should we consider it for v5.2
and if yes how are we going to merge it ?
Thanks,
Lorenzo
On Wed, May 01, 2019 at 12:30:38PM +0100, Lorenzo Pieralisi wrote:
> On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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 resource entries of allowed
> > address ranges given in dma-ranges DT property in sorted order. This
> > sorted list of resources will be processed and reserve IOVA address for
> > inaccessible address holes while initializing IOMMU domain.
> >
> > This patch set is based on Linux-5.0-rc2.
> >
> > Changes from v3:
> > - Addressed Robin Murphy review comments.
> > - pcie-iproc: parse dma-ranges and make sorted resource list.
> > - dma-iommu: process list and reserve gaps between entries
> >
> > Changes from v2:
> > - Patch set rebased to Linux-5.0-rc2
> >
> > Changes from v1:
> > - Addressed Oza review comments.
> >
> > Srinath Mannam (3):
> > PCI: Add dma_ranges window list
> > iommu/dma: Reserve IOVA for PCIe inaccessible DMA address
> > PCI: iproc: Add sorted dma ranges resource entries to host bridge
> >
> > drivers/iommu/dma-iommu.c | 19 ++++++++++++++++
> > drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> > drivers/pci/probe.c | 3 +++
> > include/linux/pci.h | 1 +
> > 4 files changed, 66 insertions(+), 1 deletion(-)
>
> Bjorn, Joerg,
>
> this series should not affect anything in the mainline other than its
> consumer (ie patch 3); if that's the case should we consider it for v5.2
> and if yes how are we going to merge it ?
I acked the first one
Robin reviewed the second
(https://lore.kernel.org/lkml/[email protected])
(though I do agree with his comment about DMA_BIT_MASK()), Joerg was OK
with it if Robin was
(https://lore.kernel.org/lkml/[email protected]).
Eric reviewed the third (and pointed out a typo).
My Kconfiggery never got fully answered -- it looks to me as though it's
possible to build pcie-iproc without the DMA hole support, and I thought
the whole point of this series was to deal with those holes
(https://lore.kernel.org/lkml/[email protected]). I would
have expected something like making pcie-iproc depend on IOMMU_SUPPORT.
But Srinath didn't respond to that, so maybe it's not an issue and it
should only affect pcie-iproc anyway.
So bottom line, I'm fine with merging it for v5.2. Do you want to merge
it, Lorenzo, or ...?
Bjorn
On 2019-05-01 1:55 pm, Bjorn Helgaas wrote:
> On Wed, May 01, 2019 at 12:30:38PM +0100, Lorenzo Pieralisi wrote:
>> On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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 resource entries of allowed
>>> address ranges given in dma-ranges DT property in sorted order. This
>>> sorted list of resources will be processed and reserve IOVA address for
>>> inaccessible address holes while initializing IOMMU domain.
>>>
>>> This patch set is based on Linux-5.0-rc2.
>>>
>>> Changes from v3:
>>> - Addressed Robin Murphy review comments.
>>> - pcie-iproc: parse dma-ranges and make sorted resource list.
>>> - dma-iommu: process list and reserve gaps between entries
>>>
>>> Changes from v2:
>>> - Patch set rebased to Linux-5.0-rc2
>>>
>>> Changes from v1:
>>> - Addressed Oza review comments.
>>>
>>> Srinath Mannam (3):
>>> PCI: Add dma_ranges window list
>>> iommu/dma: Reserve IOVA for PCIe inaccessible DMA address
>>> PCI: iproc: Add sorted dma ranges resource entries to host bridge
>>>
>>> drivers/iommu/dma-iommu.c | 19 ++++++++++++++++
>>> drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
>>> drivers/pci/probe.c | 3 +++
>>> include/linux/pci.h | 1 +
>>> 4 files changed, 66 insertions(+), 1 deletion(-)
>>
>> Bjorn, Joerg,
>>
>> this series should not affect anything in the mainline other than its
>> consumer (ie patch 3); if that's the case should we consider it for v5.2
>> and if yes how are we going to merge it ?
>
> I acked the first one
>
> Robin reviewed the second
> (https://lore.kernel.org/lkml/[email protected])
> (though I do agree with his comment about DMA_BIT_MASK()), Joerg was OK
> with it if Robin was
> (https://lore.kernel.org/lkml/[email protected]).
>
> Eric reviewed the third (and pointed out a typo).
>
> My Kconfiggery never got fully answered -- it looks to me as though it's
> possible to build pcie-iproc without the DMA hole support, and I thought
> the whole point of this series was to deal with those holes
> (https://lore.kernel.org/lkml/[email protected]). I would
> have expected something like making pcie-iproc depend on IOMMU_SUPPORT.
> But Srinath didn't respond to that, so maybe it's not an issue and it
> should only affect pcie-iproc anyway.
Hmm, I'm sure I had at least half-written a reply on that point, but I
can't seem to find it now... anyway, the gist is that these inbound
windows are generally set up to cover the physical address ranges of
DRAM and anything else that devices might need to DMA to. Thus if you're
not using an IOMMU, the fact that devices can't access the gaps in
between doesn't matter because there won't be anything there anyway; it
only needs mitigating if you do use an IOMMU and start giving arbitrary
non-physical addresses to the endpoint.
> So bottom line, I'm fine with merging it for v5.2. Do you want to merge
> it, Lorenzo, or ...?
This doesn't look like it will conflict with the other DMA ops and MSI
mapping changes currently in-flight for iommu-dma, so I have no
objection to it going through the PCI tree for 5.2.
Robin.
On Wed, May 01, 2019 at 02:20:56PM +0100, Robin Murphy wrote:
> On 2019-05-01 1:55 pm, Bjorn Helgaas wrote:
> > On Wed, May 01, 2019 at 12:30:38PM +0100, Lorenzo Pieralisi wrote:
> > > On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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 resource entries of allowed
> > > > address ranges given in dma-ranges DT property in sorted order. This
> > > > sorted list of resources will be processed and reserve IOVA address for
> > > > inaccessible address holes while initializing IOMMU domain.
> > > >
> > > > This patch set is based on Linux-5.0-rc2.
> > > >
> > > > Changes from v3:
> > > > - Addressed Robin Murphy review comments.
> > > > - pcie-iproc: parse dma-ranges and make sorted resource list.
> > > > - dma-iommu: process list and reserve gaps between entries
> > > >
> > > > Changes from v2:
> > > > - Patch set rebased to Linux-5.0-rc2
> > > >
> > > > Changes from v1:
> > > > - Addressed Oza review comments.
> > > >
> > > > Srinath Mannam (3):
> > > > PCI: Add dma_ranges window list
> > > > iommu/dma: Reserve IOVA for PCIe inaccessible DMA address
> > > > PCI: iproc: Add sorted dma ranges resource entries to host bridge
> > > >
> > > > drivers/iommu/dma-iommu.c | 19 ++++++++++++++++
> > > > drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> > > > drivers/pci/probe.c | 3 +++
> > > > include/linux/pci.h | 1 +
> > > > 4 files changed, 66 insertions(+), 1 deletion(-)
> > >
> > > Bjorn, Joerg,
> > >
> > > this series should not affect anything in the mainline other than its
> > > consumer (ie patch 3); if that's the case should we consider it for v5.2
> > > and if yes how are we going to merge it ?
> >
> > I acked the first one
> >
> > Robin reviewed the second
> > (https://lore.kernel.org/lkml/[email protected])
> > (though I do agree with his comment about DMA_BIT_MASK()), Joerg was OK
> > with it if Robin was
> > (https://lore.kernel.org/lkml/[email protected]).
> >
> > Eric reviewed the third (and pointed out a typo).
> >
> > My Kconfiggery never got fully answered -- it looks to me as though it's
> > possible to build pcie-iproc without the DMA hole support, and I thought
> > the whole point of this series was to deal with those holes
> > (https://lore.kernel.org/lkml/[email protected]). I would
> > have expected something like making pcie-iproc depend on IOMMU_SUPPORT.
> > But Srinath didn't respond to that, so maybe it's not an issue and it
> > should only affect pcie-iproc anyway.
>
> Hmm, I'm sure I had at least half-written a reply on that point, but I
> can't seem to find it now... anyway, the gist is that these inbound
> windows are generally set up to cover the physical address ranges of DRAM
> and anything else that devices might need to DMA to. Thus if you're not
> using an IOMMU, the fact that devices can't access the gaps in between
> doesn't matter because there won't be anything there anyway; it only
> needs mitigating if you do use an IOMMU and start giving arbitrary
> non-physical addresses to the endpoint.
So basically there is no strict IOMMU_SUPPORT dependency.
> > So bottom line, I'm fine with merging it for v5.2. Do you want to merge
> > it, Lorenzo, or ...?
>
> This doesn't look like it will conflict with the other DMA ops and MSI
> mapping changes currently in-flight for iommu-dma, so I have no
> objection to it going through the PCI tree for 5.2.
I will update the DMA_BIT_MASK() according to your review and fix the
typo Eric pointed out and push out a branch - we shall see if we can
include it for v5.2.
Thanks,
Lorenzo
On Fri, Apr 12, 2019 at 08:43:35AM +0530, Srinath Mannam wrote:
> IPROC host has the limitation that it can use only those address ranges
> given by dma-ranges property as inbound address. So that the memory
> address holes in dma-ranges should be reserved to allocate as DMA address.
>
> Inbound address of host accessed by PCIe devices will not be translated
> before it comes to IOMMU or directly to PE.
What does that mean "directly to PE" ?
IIUC all you want to say is that there is no entity translating
PCI memory transactions addresses before they it the PCI host
controller inbound regions address decoder.
> But the limitation of this host is, access to few address ranges are
> ignored. So that IOVA ranges for these address ranges have to be
> reserved.
>
> All allowed address ranges are listed in dma-ranges DT parameter. These
> address ranges are converted as resource entries and listed in sorted
> order add added to dma_ranges list of PCI host bridge structure.
>
> Ex:
> dma-ranges = < \
> 0x43000000 0x00 0x80000000 0x00 0x80000000 0x00 0x80000000 \
> 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
> 0x43000000 0x80 0x00000000 0x80 0x00000000 0x40 0x00000000>
>
> In the above example of dma-ranges, memory address from
> 0x0 - 0x80000000,
> 0x100000000 - 0x800000000,
> 0x1000000000 - 0x8000000000 and
> 0x10000000000 - 0xffffffffffffffff.
> are not allowed to use as inbound addresses.
>
> Signed-off-by: Srinath Mannam <[email protected]>
> Based-on-patch-by: Oza Pawandeep <[email protected]>
> Reviewed-by: Oza Pawandeep <[email protected]>
> ---
> drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c
> index c20fd6b..94ba5c0 100644
> --- a/drivers/pci/controller/pcie-iproc.c
> +++ b/drivers/pci/controller/pcie-iproc.c
> @@ -1146,11 +1146,43 @@ static int iproc_pcie_setup_ib(struct iproc_pcie *pcie,
> return ret;
> }
>
> +static int
> +iproc_pcie_add_dma_range(struct device *dev, struct list_head *resources,
> + struct of_pci_range *range)
> +{
> + struct resource *res;
> + struct resource_entry *entry, *tmp;
> + struct list_head *head = resources;
> +
> + res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
> + if (!res)
> + return -ENOMEM;
> +
> + resource_list_for_each_entry(tmp, resources) {
> + if (tmp->res->start < range->cpu_addr)
> + head = &tmp->node;
> + }
> +
> + res->start = range->cpu_addr;
> + res->end = res->start + range->size - 1;
> +
> + entry = resource_list_create_entry(res, 0);
> + if (!entry)
> + return -ENOMEM;
> +
> + entry->offset = res->start - range->cpu_addr;
> + resource_list_add(entry, head);
> +
> + return 0;
> +}
> +
> static int iproc_pcie_map_dma_ranges(struct iproc_pcie *pcie)
> {
> + struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
> struct of_pci_range range;
> struct of_pci_range_parser parser;
> int ret;
> + LIST_HEAD(resources);
>
> /* Get the dma-ranges from DT */
> ret = of_pci_dma_range_parser_init(&parser, pcie->dev->of_node);
> @@ -1158,13 +1190,23 @@ static int iproc_pcie_map_dma_ranges(struct iproc_pcie *pcie)
> return ret;
>
> for_each_of_pci_range(&parser, &range) {
> + ret = iproc_pcie_add_dma_range(pcie->dev,
> + &resources,
> + &range);
> + if (ret)
> + goto out;
> /* Each range entry corresponds to an inbound mapping region */
> ret = iproc_pcie_setup_ib(pcie, &range, IPROC_PCIE_IB_MAP_MEM);
> if (ret)
> - return ret;
> + goto out;
> }
>
> + list_splice_init(&resources, &host->dma_ranges);
> +
> return 0;
> +out:
> + pci_free_resource_list(&resources);
> + return ret;
> }
>
> static int iproce_pcie_get_msi(struct iproc_pcie *pcie,
> --
> 2.7.4
>
Hi Lorenzo,
Thanks a lot. Please see my reply below.
On Wed, May 1, 2019 at 7:24 PM Lorenzo Pieralisi
<[email protected]> wrote:
>
> On Wed, May 01, 2019 at 02:20:56PM +0100, Robin Murphy wrote:
> > On 2019-05-01 1:55 pm, Bjorn Helgaas wrote:
> > > On Wed, May 01, 2019 at 12:30:38PM +0100, Lorenzo Pieralisi wrote:
> > > > On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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 resource entries of allowed
> > > > > address ranges given in dma-ranges DT property in sorted order. This
> > > > > sorted list of resources will be processed and reserve IOVA address for
> > > > > inaccessible address holes while initializing IOMMU domain.
> > > > >
> > > > > This patch set is based on Linux-5.0-rc2.
> > > > >
> > > > > Changes from v3:
> > > > > - Addressed Robin Murphy review comments.
> > > > > - pcie-iproc: parse dma-ranges and make sorted resource list.
> > > > > - dma-iommu: process list and reserve gaps between entries
> > > > >
> > > > > Changes from v2:
> > > > > - Patch set rebased to Linux-5.0-rc2
> > > > >
> > > > > Changes from v1:
> > > > > - Addressed Oza review comments.
> > > > >
> > > > > Srinath Mannam (3):
> > > > > PCI: Add dma_ranges window list
> > > > > iommu/dma: Reserve IOVA for PCIe inaccessible DMA address
> > > > > PCI: iproc: Add sorted dma ranges resource entries to host bridge
> > > > >
> > > > > drivers/iommu/dma-iommu.c | 19 ++++++++++++++++
> > > > > drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> > > > > drivers/pci/probe.c | 3 +++
> > > > > include/linux/pci.h | 1 +
> > > > > 4 files changed, 66 insertions(+), 1 deletion(-)
> > > >
> > > > Bjorn, Joerg,
> > > >
> > > > this series should not affect anything in the mainline other than its
> > > > consumer (ie patch 3); if that's the case should we consider it for v5.2
> > > > and if yes how are we going to merge it ?
> > >
> > > I acked the first one
> > >
> > > Robin reviewed the second
> > > (https://lore.kernel.org/lkml/[email protected])
> > > (though I do agree with his comment about DMA_BIT_MASK()), Joerg was OK
> > > with it if Robin was
> > > (https://lore.kernel.org/lkml/[email protected]).
> > >
> > > Eric reviewed the third (and pointed out a typo).
> > >
> > > My Kconfiggery never got fully answered -- it looks to me as though it's
> > > possible to build pcie-iproc without the DMA hole support, and I thought
> > > the whole point of this series was to deal with those holes
> > > (https://lore.kernel.org/lkml/[email protected]). I would
> > > have expected something like making pcie-iproc depend on IOMMU_SUPPORT.
> > > But Srinath didn't respond to that, so maybe it's not an issue and it
> > > should only affect pcie-iproc anyway.
> >
> > Hmm, I'm sure I had at least half-written a reply on that point, but I
> > can't seem to find it now... anyway, the gist is that these inbound
> > windows are generally set up to cover the physical address ranges of DRAM
> > and anything else that devices might need to DMA to. Thus if you're not
> > using an IOMMU, the fact that devices can't access the gaps in between
> > doesn't matter because there won't be anything there anyway; it only
> > needs mitigating if you do use an IOMMU and start giving arbitrary
> > non-physical addresses to the endpoint.
>
> So basically there is no strict IOMMU_SUPPORT dependency.
Yes, without IOMMU_SUPPORT, all inbound addresses will fall inside dma-ranges.
Issue is only in the case of IOMMU enable, this patch will address by
reserving non-allowed
address (holes of dma-ranges) by reserving them.
>
> > > So bottom line, I'm fine with merging it for v5.2. Do you want to merge
> > > it, Lorenzo, or ...?
> >
> > This doesn't look like it will conflict with the other DMA ops and MSI
> > mapping changes currently in-flight for iommu-dma, so I have no
> > objection to it going through the PCI tree for 5.2.
>
> I will update the DMA_BIT_MASK() according to your review and fix the
> typo Eric pointed out and push out a branch - we shall see if we can
> include it for v5.2.
I will send new patches with the change DMA_BIT_MASK() and typo along
with Bjorn's comment in PATCH-1.
Regards,
Srinath.
>
> Thanks,
> Lorenzo
Hi Bjorn,
Thank you. Please find my reply below.
On Wed, May 1, 2019 at 6:25 PM Bjorn Helgaas <[email protected]> wrote:
>
> On Wed, May 01, 2019 at 12:30:38PM +0100, Lorenzo Pieralisi wrote:
> > On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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 resource entries of allowed
> > > address ranges given in dma-ranges DT property in sorted order. This
> > > sorted list of resources will be processed and reserve IOVA address for
> > > inaccessible address holes while initializing IOMMU domain.
> > >
> > > This patch set is based on Linux-5.0-rc2.
> > >
> > > Changes from v3:
> > > - Addressed Robin Murphy review comments.
> > > - pcie-iproc: parse dma-ranges and make sorted resource list.
> > > - dma-iommu: process list and reserve gaps between entries
> > >
> > > Changes from v2:
> > > - Patch set rebased to Linux-5.0-rc2
> > >
> > > Changes from v1:
> > > - Addressed Oza review comments.
> > >
> > > Srinath Mannam (3):
> > > PCI: Add dma_ranges window list
> > > iommu/dma: Reserve IOVA for PCIe inaccessible DMA address
> > > PCI: iproc: Add sorted dma ranges resource entries to host bridge
> > >
> > > drivers/iommu/dma-iommu.c | 19 ++++++++++++++++
> > > drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> > > drivers/pci/probe.c | 3 +++
> > > include/linux/pci.h | 1 +
> > > 4 files changed, 66 insertions(+), 1 deletion(-)
> >
> > Bjorn, Joerg,
> >
> > this series should not affect anything in the mainline other than its
> > consumer (ie patch 3); if that's the case should we consider it for v5.2
> > and if yes how are we going to merge it ?
>
> I acked the first one
I will send new patch with change in commit message as per your comment.
"s/bridge This list/bridge, this list/"
>
> Robin reviewed the second
> (https://lore.kernel.org/lkml/[email protected])
> (though I do agree with his comment about DMA_BIT_MASK()), Joerg was OK
> with it if Robin was
> (https://lore.kernel.org/lkml/[email protected]).
>
I will send patch, for "DMA_BIT_MASK(sizeof(dma_addr_t) *
BITS_PER_BYTE)" change to "~(dma_addr_t)0".
> Eric reviewed the third (and pointed out a typo).
I will send a new patch to address this typo.
>
> My Kconfiggery never got fully answered -- it looks to me as though it's
> possible to build pcie-iproc without the DMA hole support, and I thought
> the whole point of this series was to deal with those holes
> (https://lore.kernel.org/lkml/[email protected]). I would
> have expected something like making pcie-iproc depend on IOMMU_SUPPORT.
> But Srinath didn't respond to that, so maybe it's not an issue and it
> should only affect pcie-iproc anyway.
I am sorry to miss to respond..
In NO-IOMMU case, All inbound addresses allocated from dma APIs are
physical addresses of DDR.
All DDR physical addresses are fall inside given dma-ranges. so that,
without IOMMU_SUPPORT, will not
be any issue.
Regards,
Srinath.
>
> So bottom line, I'm fine with merging it for v5.2. Do you want to merge
> it, Lorenzo, or ...?
>
> Bjorn
Hi Robin,
Thank you so much for all the information.
Regards,
Srinath.
On Wed, May 1, 2019 at 6:51 PM Robin Murphy <[email protected]> wrote:
>
> On 2019-05-01 1:55 pm, Bjorn Helgaas wrote:
> > On Wed, May 01, 2019 at 12:30:38PM +0100, Lorenzo Pieralisi wrote:
> >> On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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 resource entries of allowed
> >>> address ranges given in dma-ranges DT property in sorted order. This
> >>> sorted list of resources will be processed and reserve IOVA address for
> >>> inaccessible address holes while initializing IOMMU domain.
> >>>
> >>> This patch set is based on Linux-5.0-rc2.
> >>>
> >>> Changes from v3:
> >>> - Addressed Robin Murphy review comments.
> >>> - pcie-iproc: parse dma-ranges and make sorted resource list.
> >>> - dma-iommu: process list and reserve gaps between entries
> >>>
> >>> Changes from v2:
> >>> - Patch set rebased to Linux-5.0-rc2
> >>>
> >>> Changes from v1:
> >>> - Addressed Oza review comments.
> >>>
> >>> Srinath Mannam (3):
> >>> PCI: Add dma_ranges window list
> >>> iommu/dma: Reserve IOVA for PCIe inaccessible DMA address
> >>> PCI: iproc: Add sorted dma ranges resource entries to host bridge
> >>>
> >>> drivers/iommu/dma-iommu.c | 19 ++++++++++++++++
> >>> drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> >>> drivers/pci/probe.c | 3 +++
> >>> include/linux/pci.h | 1 +
> >>> 4 files changed, 66 insertions(+), 1 deletion(-)
> >>
> >> Bjorn, Joerg,
> >>
> >> this series should not affect anything in the mainline other than its
> >> consumer (ie patch 3); if that's the case should we consider it for v5.2
> >> and if yes how are we going to merge it ?
> >
> > I acked the first one
> >
> > Robin reviewed the second
> > (https://lore.kernel.org/lkml/[email protected])
> > (though I do agree with his comment about DMA_BIT_MASK()), Joerg was OK
> > with it if Robin was
> > (https://lore.kernel.org/lkml/[email protected]).
> >
> > Eric reviewed the third (and pointed out a typo).
> >
> > My Kconfiggery never got fully answered -- it looks to me as though it's
> > possible to build pcie-iproc without the DMA hole support, and I thought
> > the whole point of this series was to deal with those holes
> > (https://lore.kernel.org/lkml/[email protected]). I would
> > have expected something like making pcie-iproc depend on IOMMU_SUPPORT.
> > But Srinath didn't respond to that, so maybe it's not an issue and it
> > should only affect pcie-iproc anyway.
>
> Hmm, I'm sure I had at least half-written a reply on that point, but I
> can't seem to find it now... anyway, the gist is that these inbound
> windows are generally set up to cover the physical address ranges of
> DRAM and anything else that devices might need to DMA to. Thus if you're
> not using an IOMMU, the fact that devices can't access the gaps in
> between doesn't matter because there won't be anything there anyway; it
> only needs mitigating if you do use an IOMMU and start giving arbitrary
> non-physical addresses to the endpoint.
>
> > So bottom line, I'm fine with merging it for v5.2. Do you want to merge
> > it, Lorenzo, or ...?
>
> This doesn't look like it will conflict with the other DMA ops and MSI
> mapping changes currently in-flight for iommu-dma, so I have no
> objection to it going through the PCI tree for 5.2.
>
> Robin.
Hi Lorenzo,
Please see my reply below.
On Wed, May 1, 2019 at 8:07 PM Lorenzo Pieralisi
<[email protected]> wrote:
>
> On Fri, Apr 12, 2019 at 08:43:35AM +0530, Srinath Mannam wrote:
> > IPROC host has the limitation that it can use only those address ranges
> > given by dma-ranges property as inbound address. So that the memory
> > address holes in dma-ranges should be reserved to allocate as DMA address.
> >
> > Inbound address of host accessed by PCIe devices will not be translated
> > before it comes to IOMMU or directly to PE.
>
> What does that mean "directly to PE" ?
In general, with IOMMU enable case, inbound address access of endpoint
will come to IOMMU.
If IOMMU disable then it comes to PE (processing element - ARM).
>
> IIUC all you want to say is that there is no entity translating
> PCI memory transactions addresses before they it the PCI host
> controller inbound regions address decoder.
In our SOC we have an entity (Inside PCIe RC) which will translate
inbound address before it goes to
IOMMU or PE. In other SOCs this will not be the case, all inbound
address access will go to IOMMU or
PE.
Regards,
Srinath.
>
> > But the limitation of this host is, access to few address ranges are
> > ignored. So that IOVA ranges for these address ranges have to be
> > reserved.
> >
> > All allowed address ranges are listed in dma-ranges DT parameter. These
> > address ranges are converted as resource entries and listed in sorted
> > order add added to dma_ranges list of PCI host bridge structure.
> >
> > Ex:
> > dma-ranges = < \
> > 0x43000000 0x00 0x80000000 0x00 0x80000000 0x00 0x80000000 \
> > 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
> > 0x43000000 0x80 0x00000000 0x80 0x00000000 0x40 0x00000000>
> >
> > In the above example of dma-ranges, memory address from
> > 0x0 - 0x80000000,
> > 0x100000000 - 0x800000000,
> > 0x1000000000 - 0x8000000000 and
> > 0x10000000000 - 0xffffffffffffffff.
> > are not allowed to use as inbound addresses.
> >
> > Signed-off-by: Srinath Mannam <[email protected]>
> > Based-on-patch-by: Oza Pawandeep <[email protected]>
> > Reviewed-by: Oza Pawandeep <[email protected]>
> > ---
> > drivers/pci/controller/pcie-iproc.c | 44 ++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c
> > index c20fd6b..94ba5c0 100644
> > --- a/drivers/pci/controller/pcie-iproc.c
> > +++ b/drivers/pci/controller/pcie-iproc.c
> > @@ -1146,11 +1146,43 @@ static int iproc_pcie_setup_ib(struct iproc_pcie *pcie,
> > return ret;
> > }
> >
> > +static int
> > +iproc_pcie_add_dma_range(struct device *dev, struct list_head *resources,
> > + struct of_pci_range *range)
> > +{
> > + struct resource *res;
> > + struct resource_entry *entry, *tmp;
> > + struct list_head *head = resources;
> > +
> > + res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
> > + if (!res)
> > + return -ENOMEM;
> > +
> > + resource_list_for_each_entry(tmp, resources) {
> > + if (tmp->res->start < range->cpu_addr)
> > + head = &tmp->node;
> > + }
> > +
> > + res->start = range->cpu_addr;
> > + res->end = res->start + range->size - 1;
> > +
> > + entry = resource_list_create_entry(res, 0);
> > + if (!entry)
> > + return -ENOMEM;
> > +
> > + entry->offset = res->start - range->cpu_addr;
> > + resource_list_add(entry, head);
> > +
> > + return 0;
> > +}
> > +
> > static int iproc_pcie_map_dma_ranges(struct iproc_pcie *pcie)
> > {
> > + struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
> > struct of_pci_range range;
> > struct of_pci_range_parser parser;
> > int ret;
> > + LIST_HEAD(resources);
> >
> > /* Get the dma-ranges from DT */
> > ret = of_pci_dma_range_parser_init(&parser, pcie->dev->of_node);
> > @@ -1158,13 +1190,23 @@ static int iproc_pcie_map_dma_ranges(struct iproc_pcie *pcie)
> > return ret;
> >
> > for_each_of_pci_range(&parser, &range) {
> > + ret = iproc_pcie_add_dma_range(pcie->dev,
> > + &resources,
> > + &range);
> > + if (ret)
> > + goto out;
> > /* Each range entry corresponds to an inbound mapping region */
> > ret = iproc_pcie_setup_ib(pcie, &range, IPROC_PCIE_IB_MAP_MEM);
> > if (ret)
> > - return ret;
> > + goto out;
> > }
> >
> > + list_splice_init(&resources, &host->dma_ranges);
> > +
> > return 0;
> > +out:
> > + pci_free_resource_list(&resources);
> > + return ret;
> > }
> >
> > static int iproce_pcie_get_msi(struct iproc_pcie *pcie,
> > --
> > 2.7.4
> >
From: Srinath Mannam
> Sent: 01 May 2019 16:23
...
> > > On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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.
You probably want to fix the english in the first sentence.
The sense is reversed.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Hi David,
Thanks for review, I will fix in next version of this patch set.
Regards,
Srinath.
On Thu, May 2, 2019 at 3:24 PM David Laight <[email protected]> wrote:
>
> From: Srinath Mannam
> > Sent: 01 May 2019 16:23
> ...
> > > > On Fri, Apr 12, 2019 at 08:43:32AM +0530, 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.
>
> You probably want to fix the english in the first sentence.
> The sense is reversed.
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)