iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] IOMMU reserved region tweaks
@ 2017-03-16 17:00 Robin Murphy
       [not found] ` <cover.1489683129.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Robin Murphy @ 2017-03-16 17:00 UTC (permalink / raw)
  To: joro-zLv9SwRftAIdnm+yROfE0A
  Cc: gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
	john.garry-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8,
	shameerali.kolothum.thodi-hv44wF8Li93QT0dZR+AlfA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Joerg,

Here's v2 incorporating Eric's feedback and R-b tags, with patch #4 added
to finish the job. Whilst presented as a logical series because it's all
touching the same code, it doesn't necessarily need merging as such - I'd
be happier with patch #1 going into 4.11 before anyone starts using the
sysfs region info in anger, but it's your call; #2 is really a standalone
fix (other than context conflicts) but is non-urgent; #3 and #4 can
happily wait for 4.12.

Robin.

Robin Murphy (4):
  iommu: Disambiguate MSI region types
  iommu/dma: Don't reserve PCI I/O windows
  iommu/dma: Handle IOMMU API reserved regions
  iommu/dma: Make PCI window reservation generic

 drivers/iommu/amd_iommu.c       |   2 +-
 drivers/iommu/arm-smmu-v3.c     |   4 +-
 drivers/iommu/arm-smmu.c        |   4 +-
 drivers/iommu/dma-iommu.c       | 111 ++++++++++++++++++++++++++++++++++------
 drivers/iommu/intel-iommu.c     |   2 +-
 drivers/iommu/iommu.c           |   5 +-
 drivers/vfio/vfio_iommu_type1.c |   7 ++-
 include/linux/dma-iommu.h       |   5 ++
 include/linux/iommu.h           |  18 +++++--
 9 files changed, 127 insertions(+), 31 deletions(-)

-- 
2.11.0.dirty

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/4] iommu: Disambiguate MSI region types
       [not found] ` <cover.1489683129.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
@ 2017-03-16 17:00   ` Robin Murphy
       [not found]     ` <a56c76f7276a12120ecd0675be4c69de58aec8ff.1489683129.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
  2017-03-16 17:00   ` [PATCH v2 2/4] iommu/dma: Don't reserve PCI I/O windows Robin Murphy
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Robin Murphy @ 2017-03-16 17:00 UTC (permalink / raw)
  To: joro-zLv9SwRftAIdnm+yROfE0A
  Cc: kvm-u79uwXL29TY76Z2rM5mHXA,
	gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
	john.garry-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8,
	shameerali.kolothum.thodi-hv44wF8Li93QT0dZR+AlfA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	David Woodhouse,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

The introduction of reserved regions has left a couple of rough edges
which we could do with sorting out sooner rather than later. Since we
are not yet addressing the potential dynamic aspect of software-managed
reservations and presenting them at arbitrary fixed addresses, it is
incongruous that we end up displaying hardware vs. software-managed MSI
regions to userspace differently, especially since ARM-based systems may
actually require one or the other, or even potentially both at once,
(which iommu-dma currently has no hope of dealing with at all). Let's
resolve the former user-visible inconsistency ASAP before the ABI has
been baked into a kernel release, in a way that also lays the groundwork
for the latter shortcoming to be addressed by follow-up patches.

For clarity, rename the software-managed type to IOMMU_RESV_SW_MSI, use
IOMMU_RESV_MSI to describe the hardware type, and document everything a
little bit. Since the x86 MSI remapping hardware falls squarely under
this meaning of IOMMU_RESV_MSI, apply that type to their regions as well,
so that we tell the same story to userspace across all platforms.

Secondly, as the various region types require quite different handling,
and it really makes little sense to ever try combining them, convert the
bitfield-esque #defines to a plain enum in the process before anyone
gets the wrong impression.

Fixes: d30ddcaa7b02 ("iommu: Add a new type field in iommu_resv_region")
Reviewed-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
CC: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
CC: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
CC: kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
---

Notes:
    v2:
    - Reword commit message
    - Convert type to an enum [Eric]
    - Rename vfio_iommu_has_resv_msi() for clarity [Eric]

 drivers/iommu/amd_iommu.c       |  2 +-
 drivers/iommu/arm-smmu-v3.c     |  2 +-
 drivers/iommu/arm-smmu.c        |  2 +-
 drivers/iommu/intel-iommu.c     |  2 +-
 drivers/iommu/iommu.c           |  5 +++--
 drivers/vfio/vfio_iommu_type1.c |  7 +++----
 include/linux/iommu.h           | 18 +++++++++++++-----
 7 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 98940d1392cb..b17536d6e69b 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3202,7 +3202,7 @@ static void amd_iommu_get_resv_regions(struct device *dev,
 
 	region = iommu_alloc_resv_region(MSI_RANGE_START,
 					 MSI_RANGE_END - MSI_RANGE_START + 1,
-					 0, IOMMU_RESV_RESERVED);
+					 0, IOMMU_RESV_MSI);
 	if (!region)
 		return;
 	list_add_tail(&region->list, head);
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 5806a6acc94e..591bb96047c9 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1888,7 +1888,7 @@ static void arm_smmu_get_resv_regions(struct device *dev,
 	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
 
 	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
-					 prot, IOMMU_RESV_MSI);
+					 prot, IOMMU_RESV_SW_MSI);
 	if (!region)
 		return;
 
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index abf6496843a6..b493c99e17f7 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1608,7 +1608,7 @@ static void arm_smmu_get_resv_regions(struct device *dev,
 	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
 
 	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
-					 prot, IOMMU_RESV_MSI);
+					 prot, IOMMU_RESV_SW_MSI);
 	if (!region)
 		return;
 
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 238ad3447712..f1611fd6f5b0 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5249,7 +5249,7 @@ static void intel_iommu_get_resv_regions(struct device *device,
 
 	reg = iommu_alloc_resv_region(IOAPIC_RANGE_START,
 				      IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1,
-				      0, IOMMU_RESV_RESERVED);
+				      0, IOMMU_RESV_MSI);
 	if (!reg)
 		return;
 	list_add_tail(&reg->list, head);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 8ea14f41a979..3b67144dead2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -72,6 +72,7 @@ static const char * const iommu_group_resv_type_string[] = {
 	[IOMMU_RESV_DIRECT]	= "direct",
 	[IOMMU_RESV_RESERVED]	= "reserved",
 	[IOMMU_RESV_MSI]	= "msi",
+	[IOMMU_RESV_SW_MSI]	= "msi",
 };
 
 #define IOMMU_GROUP_ATTR(_name, _mode, _show, _store)		\
@@ -1743,8 +1744,8 @@ void iommu_put_resv_regions(struct device *dev, struct list_head *list)
 }
 
 struct iommu_resv_region *iommu_alloc_resv_region(phys_addr_t start,
-						  size_t length,
-						  int prot, int type)
+						  size_t length, int prot,
+						  enum iommu_resv_type type)
 {
 	struct iommu_resv_region *region;
 
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index c26fa1f3ed86..32d2633092a3 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1182,8 +1182,7 @@ static struct vfio_group *find_iommu_group(struct vfio_domain *domain,
 	return NULL;
 }
 
-static bool vfio_iommu_has_resv_msi(struct iommu_group *group,
-				    phys_addr_t *base)
+static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base)
 {
 	struct list_head group_resv_regions;
 	struct iommu_resv_region *region, *next;
@@ -1192,7 +1191,7 @@ static bool vfio_iommu_has_resv_msi(struct iommu_group *group,
 	INIT_LIST_HEAD(&group_resv_regions);
 	iommu_get_group_resv_regions(group, &group_resv_regions);
 	list_for_each_entry(region, &group_resv_regions, list) {
-		if (region->type & IOMMU_RESV_MSI) {
+		if (region->type == IOMMU_RESV_SW_MSI) {
 			*base = region->start;
 			ret = true;
 			goto out;
@@ -1283,7 +1282,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	if (ret)
 		goto out_domain;
 
-	resv_msi = vfio_iommu_has_resv_msi(iommu_group, &resv_msi_base);
+	resv_msi = vfio_iommu_has_sw_msi(iommu_group, &resv_msi_base);
 
 	INIT_LIST_HEAD(&domain->group_list);
 	list_add(&group->next, &domain->group_list);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 6a6de187ddc0..2e4de0deee53 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -125,9 +125,16 @@ enum iommu_attr {
 };
 
 /* These are the possible reserved region types */
-#define IOMMU_RESV_DIRECT	(1 << 0)
-#define IOMMU_RESV_RESERVED	(1 << 1)
-#define IOMMU_RESV_MSI		(1 << 2)
+enum iommu_resv_type {
+	/* Memory regions which must be mapped 1:1 at all times */
+	IOMMU_RESV_DIRECT,
+	/* Arbitrary "never map this or give it to a device" address ranges */
+	IOMMU_RESV_RESERVED,
+	/* Hardware MSI region (untranslated) */
+	IOMMU_RESV_MSI,
+	/* Software-managed MSI translation window */
+	IOMMU_RESV_SW_MSI,
+};
 
 /**
  * struct iommu_resv_region - descriptor for a reserved memory region
@@ -142,7 +149,7 @@ struct iommu_resv_region {
 	phys_addr_t		start;
 	size_t			length;
 	int			prot;
-	int			type;
+	enum iommu_resv_type	type;
 };
 
 #ifdef CONFIG_IOMMU_API
@@ -288,7 +295,8 @@ extern void iommu_get_resv_regions(struct device *dev, struct list_head *list);
 extern void iommu_put_resv_regions(struct device *dev, struct list_head *list);
 extern int iommu_request_dm_for_dev(struct device *dev);
 extern struct iommu_resv_region *
-iommu_alloc_resv_region(phys_addr_t start, size_t length, int prot, int type);
+iommu_alloc_resv_region(phys_addr_t start, size_t length, int prot,
+			enum iommu_resv_type type);
 extern int iommu_get_group_resv_regions(struct iommu_group *group,
 					struct list_head *head);
 
-- 
2.11.0.dirty

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/4] iommu/dma: Don't reserve PCI I/O windows
       [not found] ` <cover.1489683129.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
  2017-03-16 17:00   ` [PATCH v2 1/4] iommu: Disambiguate MSI region types Robin Murphy
@ 2017-03-16 17:00   ` Robin Murphy
  2017-03-16 17:00   ` [PATCH v2 3/4] iommu/dma: Handle IOMMU API reserved regions Robin Murphy
  2017-03-16 17:00   ` [PATCH v2 4/4] iommu/dma: Make PCI window reservation generic Robin Murphy
  3 siblings, 0 replies; 7+ messages in thread
From: Robin Murphy @ 2017-03-16 17:00 UTC (permalink / raw)
  To: joro-zLv9SwRftAIdnm+yROfE0A
  Cc: gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
	john.garry-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8,
	shameerali.kolothum.thodi-hv44wF8Li93QT0dZR+AlfA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Even if a host controller's CPU-side MMIO windows into PCI I/O space do
happen to leak into PCI memory space such that it might treat them as
peer addresses, trying to reserve the corresponding I/O space addresses
doesn't do anything to help solve that problem. Stop doing a silly thing.

Fixes: fade1ec055dc ("iommu/dma: Avoid PCI host bridge windows")
Reviewed-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
---

Notes:
    v2:
    - No change

 drivers/iommu/dma-iommu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 48d36ce59efb..1e0983488a8d 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -175,8 +175,7 @@ static void iova_reserve_pci_windows(struct pci_dev *dev,
 	unsigned long lo, hi;
 
 	resource_list_for_each_entry(window, &bridge->windows) {
-		if (resource_type(window->res) != IORESOURCE_MEM &&
-		    resource_type(window->res) != IORESOURCE_IO)
+		if (resource_type(window->res) != IORESOURCE_MEM)
 			continue;
 
 		lo = iova_pfn(iovad, window->res->start - window->offset);
-- 
2.11.0.dirty

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/4] iommu/dma: Handle IOMMU API reserved regions
       [not found] ` <cover.1489683129.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
  2017-03-16 17:00   ` [PATCH v2 1/4] iommu: Disambiguate MSI region types Robin Murphy
  2017-03-16 17:00   ` [PATCH v2 2/4] iommu/dma: Don't reserve PCI I/O windows Robin Murphy
@ 2017-03-16 17:00   ` Robin Murphy
  2017-03-16 17:00   ` [PATCH v2 4/4] iommu/dma: Make PCI window reservation generic Robin Murphy
  3 siblings, 0 replies; 7+ messages in thread
From: Robin Murphy @ 2017-03-16 17:00 UTC (permalink / raw)
  To: joro-zLv9SwRftAIdnm+yROfE0A
  Cc: gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
	john.garry-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8,
	shameerali.kolothum.thodi-hv44wF8Li93QT0dZR+AlfA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Now that it's simple to discover the necessary reservations for a given
device/IOMMU combination, let's wire up the appropriate handling. Basic
reserved regions and direct-mapped regions we simply have to carve out
of IOVA space (the IOMMU core having already mapped the latter before
attaching the device). For hardware MSI regions, we also pre-populate
the cookie with matching msi_pages. That way, irqchip drivers which
normally assume MSIs to require mapping at the IOMMU can keep working
without having to special-case their iommu_dma_map_msi_msg() hook, or
indeed be aware at all of quirks preventing the IOMMU from translating
certain addresses.

Reviewed-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
---

Notes:
    v2:
    - Make the IOVA alignment logic safer [Eric]
    - Don't duplicate direct region mapping [Eric]
    - Don't wait for the subsequent patch to start using the return value [Eric]
    - Fix the sneaky off-by-one for pfn_hi
    - Minor cosmetic tweaks

 drivers/iommu/dma-iommu.c | 76 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 69 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 1e0983488a8d..5787f919f4ec 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -184,6 +184,66 @@ static void iova_reserve_pci_windows(struct pci_dev *dev,
 	}
 }
 
+static int cookie_init_hw_msi_region(struct iommu_dma_cookie *cookie,
+		phys_addr_t start, phys_addr_t end)
+{
+	struct iova_domain *iovad = &cookie->iovad;
+	struct iommu_dma_msi_page *msi_page;
+	int i, num_pages;
+
+	start -= iova_offset(iovad, start);
+	num_pages = iova_align(iovad, end - start) >> iova_shift(iovad);
+
+	msi_page = kcalloc(num_pages, sizeof(*msi_page), GFP_KERNEL);
+	if (!msi_page)
+		return -ENOMEM;
+
+	for (i = 0; i < num_pages; i++) {
+		msi_page[i].phys = start;
+		msi_page[i].iova = start;
+		INIT_LIST_HEAD(&msi_page[i].list);
+		list_add(&msi_page[i].list, &cookie->msi_page_list);
+		start += iovad->granule;
+	}
+
+	return 0;
+}
+
+static int iova_reserve_iommu_regions(struct device *dev,
+		struct iommu_domain *domain)
+{
+	struct iommu_dma_cookie *cookie = domain->iova_cookie;
+	struct iova_domain *iovad = &cookie->iovad;
+	struct iommu_resv_region *region;
+	LIST_HEAD(resv_regions);
+	int ret = 0;
+
+	if (dev_is_pci(dev))
+		iova_reserve_pci_windows(to_pci_dev(dev), iovad);
+
+	iommu_get_resv_regions(dev, &resv_regions);
+	list_for_each_entry(region, &resv_regions, list) {
+		unsigned long lo, hi;
+
+		/* We ARE the software that manages these! */
+		if (region->type == IOMMU_RESV_SW_MSI)
+			continue;
+
+		lo = iova_pfn(iovad, region->start);
+		hi = iova_pfn(iovad, region->start + region->length - 1);
+		reserve_iova(iovad, lo, hi);
+
+		if (region->type == IOMMU_RESV_MSI)
+			ret = cookie_init_hw_msi_region(cookie, region->start,
+					region->start + region->length);
+		if (ret)
+			break;
+	}
+	iommu_put_resv_regions(dev, &resv_regions);
+
+	return ret;
+}
+
 /**
  * iommu_dma_init_domain - Initialise a DMA mapping domain
  * @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
@@ -202,7 +262,6 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 	struct iommu_dma_cookie *cookie = domain->iova_cookie;
 	struct iova_domain *iovad = &cookie->iovad;
 	unsigned long order, base_pfn, end_pfn;
-	bool pci = dev && dev_is_pci(dev);
 
 	if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE)
 		return -EINVAL;
@@ -232,7 +291,7 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 	 * leave the cache limit at the top of their range to save an rb_last()
 	 * traversal on every allocation.
 	 */
-	if (pci)
+	if (dev && dev_is_pci(dev))
 		end_pfn &= DMA_BIT_MASK(32) >> order;
 
 	/* start_pfn is always nonzero for an already-initialised domain */
@@ -247,12 +306,15 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 		 * area cache limit down for the benefit of the smaller one.
 		 */
 		iovad->dma_32bit_pfn = min(end_pfn, iovad->dma_32bit_pfn);
-	} else {
-		init_iova_domain(iovad, 1UL << order, base_pfn, end_pfn);
-		if (pci)
-			iova_reserve_pci_windows(to_pci_dev(dev), iovad);
+
+		return 0;
 	}
-	return 0;
+
+	init_iova_domain(iovad, 1UL << order, base_pfn, end_pfn);
+	if (!dev)
+		return 0;
+
+	return iova_reserve_iommu_regions(dev, domain);
 }
 EXPORT_SYMBOL(iommu_dma_init_domain);
 
-- 
2.11.0.dirty

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 4/4] iommu/dma: Make PCI window reservation generic
       [not found] ` <cover.1489683129.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
                     ` (2 preceding siblings ...)
  2017-03-16 17:00   ` [PATCH v2 3/4] iommu/dma: Handle IOMMU API reserved regions Robin Murphy
@ 2017-03-16 17:00   ` Robin Murphy
       [not found]     ` <3876a855b7b5ba4684ac3e3af632b817d224c753.1489683129.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
  3 siblings, 1 reply; 7+ messages in thread
From: Robin Murphy @ 2017-03-16 17:00 UTC (permalink / raw)
  To: joro-zLv9SwRftAIdnm+yROfE0A
  Cc: gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
	john.garry-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8,
	shameerali.kolothum.thodi-hv44wF8Li93QT0dZR+AlfA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Now that we're applying the IOMMU API reserved regions to our IOVA
domains, we shouldn't need to privately special-case PCI windows, or
indeed anything else which isn't specific to our iommu-dma layer.
However, since those aren't IOMMU-specific either, rather than start
duplicating code into IOMMU drivers let's transform the existing
function into an iommu_get_resv_regions() helper that they can share.

Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
---

Notes:
    v2:
    - New

 drivers/iommu/arm-smmu-v3.c |  2 ++
 drivers/iommu/arm-smmu.c    |  2 ++
 drivers/iommu/dma-iommu.c   | 38 ++++++++++++++++++++++++++++----------
 include/linux/dma-iommu.h   |  5 +++++
 4 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 591bb96047c9..bbd46efbe075 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1893,6 +1893,8 @@ static void arm_smmu_get_resv_regions(struct device *dev,
 		return;
 
 	list_add_tail(&region->list, head);
+
+	iommu_dma_get_resv_regions(dev, head);
 }
 
 static void arm_smmu_put_resv_regions(struct device *dev,
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index b493c99e17f7..9b33700b7c69 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1613,6 +1613,8 @@ static void arm_smmu_get_resv_regions(struct device *dev,
 		return;
 
 	list_add_tail(&region->list, head);
+
+	iommu_dma_get_resv_regions(dev, head);
 }
 
 static void arm_smmu_put_resv_regions(struct device *dev,
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 5787f919f4ec..85652110c8ff 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -167,22 +167,43 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
 }
 EXPORT_SYMBOL(iommu_put_dma_cookie);
 
-static void iova_reserve_pci_windows(struct pci_dev *dev,
-		struct iova_domain *iovad)
+/**
+ * iommu_dma_get_resv_regions - Reserved region driver helper
+ * @dev: Device from iommu_get_resv_regions()
+ * @list: Reserved region list from iommu_get_resv_regions()
+ *
+ * IOMMU drivers can use this to implement their .get_resv_regions callback
+ * for general non-IOMMU-specific reservations. Currently, this covers host
+ * bridge windows for PCI devices.
+ */
+void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
 {
-	struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus);
+	struct pci_host_bridge *bridge;
 	struct resource_entry *window;
-	unsigned long lo, hi;
 
+	if (!dev_is_pci(dev))
+		return;
+
+	bridge = pci_find_host_bridge(to_pci_dev(dev)->bus);
 	resource_list_for_each_entry(window, &bridge->windows) {
+		struct iommu_resv_region *region;
+		phys_addr_t start;
+		size_t length;
+
 		if (resource_type(window->res) != IORESOURCE_MEM)
 			continue;
 
-		lo = iova_pfn(iovad, window->res->start - window->offset);
-		hi = iova_pfn(iovad, window->res->end - window->offset);
-		reserve_iova(iovad, lo, hi);
+		start = window->res->start - window->offset;
+		length = window->res->end - window->res->start + 1;
+		region = iommu_alloc_resv_region(start, length, 0,
+				IOMMU_RESV_RESERVED);
+		if (!region)
+			return;
+
+		list_add_tail(&region->list, list);
 	}
 }
+EXPORT_SYMBOL(iommu_dma_get_resv_regions);
 
 static int cookie_init_hw_msi_region(struct iommu_dma_cookie *cookie,
 		phys_addr_t start, phys_addr_t end)
@@ -218,9 +239,6 @@ static int iova_reserve_iommu_regions(struct device *dev,
 	LIST_HEAD(resv_regions);
 	int ret = 0;
 
-	if (dev_is_pci(dev))
-		iova_reserve_pci_windows(to_pci_dev(dev), iovad);
-
 	iommu_get_resv_regions(dev, &resv_regions);
 	list_for_each_entry(region, &resv_regions, list) {
 		unsigned long lo, hi;
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 5725c94b1f12..b6635a46fc7c 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -71,6 +71,7 @@ int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
 
 /* The DMA API isn't _quite_ the whole story, though... */
 void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
+void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
 
 #else
 
@@ -100,6 +101,10 @@ static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
 {
 }
 
+static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
+{
+}
+
 #endif	/* CONFIG_IOMMU_DMA */
 #endif	/* __KERNEL__ */
 #endif	/* __DMA_IOMMU_H */
-- 
2.11.0.dirty

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/4] iommu: Disambiguate MSI region types
       [not found]     ` <a56c76f7276a12120ecd0675be4c69de58aec8ff.1489683129.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
@ 2017-03-22 15:16       ` Joerg Roedel
  0 siblings, 0 replies; 7+ messages in thread
From: Joerg Roedel @ 2017-03-22 15:16 UTC (permalink / raw)
  To: Robin Murphy
  Cc: kvm-u79uwXL29TY76Z2rM5mHXA,
	gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
	john.garry-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8,
	shameerali.kolothum.thodi-hv44wF8Li93QT0dZR+AlfA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	David Woodhouse,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Thu, Mar 16, 2017 at 05:00:16PM +0000, Robin Murphy wrote:
> The introduction of reserved regions has left a couple of rough edges
> which we could do with sorting out sooner rather than later. Since we
> are not yet addressing the potential dynamic aspect of software-managed
> reservations and presenting them at arbitrary fixed addresses, it is
> incongruous that we end up displaying hardware vs. software-managed MSI
> regions to userspace differently, especially since ARM-based systems may
> actually require one or the other, or even potentially both at once,
> (which iommu-dma currently has no hope of dealing with at all). Let's
> resolve the former user-visible inconsistency ASAP before the ABI has
> been baked into a kernel release, in a way that also lays the groundwork
> for the latter shortcoming to be addressed by follow-up patches.
> 
> For clarity, rename the software-managed type to IOMMU_RESV_SW_MSI, use
> IOMMU_RESV_MSI to describe the hardware type, and document everything a
> little bit. Since the x86 MSI remapping hardware falls squarely under
> this meaning of IOMMU_RESV_MSI, apply that type to their regions as well,
> so that we tell the same story to userspace across all platforms.
> 
> Secondly, as the various region types require quite different handling,
> and it really makes little sense to ever try combining them, convert the
> bitfield-esque #defines to a plain enum in the process before anyone
> gets the wrong impression.
> 
> Fixes: d30ddcaa7b02 ("iommu: Add a new type field in iommu_resv_region")
> Reviewed-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> CC: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> CC: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> CC: kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>

Applied this one to iommu/fixes.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 4/4] iommu/dma: Make PCI window reservation generic
       [not found]     ` <3876a855b7b5ba4684ac3e3af632b817d224c753.1489683129.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
@ 2017-03-22 15:19       ` Joerg Roedel
  0 siblings, 0 replies; 7+ messages in thread
From: Joerg Roedel @ 2017-03-22 15:19 UTC (permalink / raw)
  To: Robin Murphy
  Cc: gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
	john.garry-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8,
	shameerali.kolothum.thodi-hv44wF8Li93QT0dZR+AlfA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Thu, Mar 16, 2017 at 05:00:19PM +0000, Robin Murphy wrote:
> Now that we're applying the IOMMU API reserved regions to our IOVA
> domains, we shouldn't need to privately special-case PCI windows, or
> indeed anything else which isn't specific to our iommu-dma layer.
> However, since those aren't IOMMU-specific either, rather than start
> duplicating code into IOMMU drivers let's transform the existing
> function into an iommu_get_resv_regions() helper that they can share.
> 
> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> ---
> 
> Notes:
>     v2:
>     - New
> 
>  drivers/iommu/arm-smmu-v3.c |  2 ++
>  drivers/iommu/arm-smmu.c    |  2 ++
>  drivers/iommu/dma-iommu.c   | 38 ++++++++++++++++++++++++++++----------
>  include/linux/dma-iommu.h   |  5 +++++
>  4 files changed, 37 insertions(+), 10 deletions(-)

Applied patches 2-4 to my arm/core branch, thanks.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-03-22 15:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-16 17:00 [PATCH v2 0/4] IOMMU reserved region tweaks Robin Murphy
     [not found] ` <cover.1489683129.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2017-03-16 17:00   ` [PATCH v2 1/4] iommu: Disambiguate MSI region types Robin Murphy
     [not found]     ` <a56c76f7276a12120ecd0675be4c69de58aec8ff.1489683129.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2017-03-22 15:16       ` Joerg Roedel
2017-03-16 17:00   ` [PATCH v2 2/4] iommu/dma: Don't reserve PCI I/O windows Robin Murphy
2017-03-16 17:00   ` [PATCH v2 3/4] iommu/dma: Handle IOMMU API reserved regions Robin Murphy
2017-03-16 17:00   ` [PATCH v2 4/4] iommu/dma: Make PCI window reservation generic Robin Murphy
     [not found]     ` <3876a855b7b5ba4684ac3e3af632b817d224c753.1489683129.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2017-03-22 15:19       ` Joerg Roedel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).