On Tue, Mar 22, 2022 at 12:27 PM Robin Murphy <robin.murphy@xxxxxxx> wrote: > > Originally, creating the dma_ranges resource list in pre-sorted fashion > was the simplest and most efficient way to enforce the order required by > iova_reserve_pci_windows(). However since then at least one PCI host > driver is now re-sorting the list for its own probe-time processing, > which doesn't seem entirely unreasonable, so that basic assumption no > longer holds. Make iommu-dma robust and get the sort order it needs by > explicitly sorting, which means we can also save the effort at creation > time and just build the list in whatever natural order the DT had. > > Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx> > --- > > Looking at this area off the back of the XGene thread[1] made me realise > that we need to do it anyway, regardless of whether it might also happen > to restore the previous XGene behaviour or not. Presumably nobody's > tried to use pcie-cadence-host behind an IOMMU yet... > > Boot-tested on Juno to make sure I hadn't got the sort comparison > backwards. > > Robin. > > [1] https://lore.kernel.org/linux-pci/20220321104843.949645-1-maz@xxxxxxxxxx/ > > drivers/iommu/dma-iommu.c | 13 ++++++++++++- > drivers/pci/of.c | 7 +------ > 2 files changed, 13 insertions(+), 7 deletions(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index b22034975301..91d134c0c9b1 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -20,6 +20,7 @@ > #include <linux/iommu.h> > #include <linux/iova.h> > #include <linux/irq.h> > +#include <linux/list_sort.h> > #include <linux/mm.h> > #include <linux/mutex.h> > #include <linux/pci.h> > @@ -414,6 +415,15 @@ static int cookie_init_hw_msi_region(struct iommu_dma_cookie *cookie, > return 0; > } > > +static int iommu_dma_ranges_sort(void *priv, const struct list_head *a, > + const struct list_head *b) > +{ > + struct resource_entry *res_a = list_entry(a, typeof(*res_a), node); > + struct resource_entry *res_b = list_entry(b, typeof(*res_b), node); > + > + return res_a->res->start > res_b->res->start; > +} > + > static int iova_reserve_pci_windows(struct pci_dev *dev, > struct iova_domain *iovad) > { > @@ -432,6 +442,7 @@ static int iova_reserve_pci_windows(struct pci_dev *dev, > } > > /* Get reserved DMA windows from host bridge */ > + list_sort(NULL, &bridge->dma_ranges, iommu_dma_ranges_sort); > resource_list_for_each_entry(window, &bridge->dma_ranges) { > end = window->res->start - window->offset; > resv_iova: > @@ -440,7 +451,7 @@ static int iova_reserve_pci_windows(struct pci_dev *dev, > hi = iova_pfn(iovad, end); > reserve_iova(iovad, lo, hi); > } else if (end < start) { > - /* dma_ranges list should be sorted */ > + /* DMA ranges should be non-overlapping */ > dev_err(&dev->dev, > "Failed to reserve IOVA [%pa-%pa]\n", > &start, &end); > diff --git a/drivers/pci/of.c b/drivers/pci/of.c > index cb2e8351c2cc..d176b4bc6193 100644 > --- a/drivers/pci/of.c > +++ b/drivers/pci/of.c > @@ -393,12 +393,7 @@ static int devm_of_pci_get_host_bridge_resources(struct device *dev, > goto failed; > } > > - /* Keep the resource list sorted */ > - resource_list_for_each_entry(entry, ib_resources) > - if (entry->res->start > res->start) > - break; > - > - pci_add_resource_offset(&entry->node, res, entry is now unused and causes a warning. > + pci_add_resource_offset(ib_resources, res, > res->start - range.pci_addr); > } > > -- > 2.28.0.dirty >
