iommu: Clean up map/unmap ops

PATCH In review Robin Murphy 5 participants 22 messages Sep 12, 2023 – Sep 26, 2023

Subsystems (7)

Arm Samsung Exynos Tegra architecture support Arm Rockchip Arm/Allwinner sunXi SoC support +3 more subsystems

lore

Share & tools

lore

Robin Murphy

author reviewer

2023-09-12 16:18 UTC #0 lore raw

cover iommu: Clean up map/unmap ops

Hi all,

Time to repay a little more from the technical debt pile and actually finalise another of our half-finished API evolutions, since it turned out that by now this was dead simple. So much so in fact that I'm in two minds whether to squash all the driver patches into one or not, as they're so very mechanical.

Note that the sun50i patch is just a placeholder to make a consistent series for now, as I'm anticipating a "proper" implementation from Jernej.

Thanks,
Robin.

Robin Murphy (8):
iommu/exynos: Update to {map,unmap}_pages
iommu/omap: Update to {map,unmap}_pages
iommu/rockchip: Update to {map,unmap}_pages
iommu/sun50i: Update to {map,unmap}_pages
iommu/tegra-gart: Update to {map,unmap}_pages
iommu/tegra-smmu: Update to {map,unmap}_pages
iommu: Retire map/unmap ops
iommu: Improve map/unmap sanity checks

 drivers/iommu/exynos-iommu.c   | 10 +++---
 drivers/iommu/iommu.c          | 66 +++++++++-------------------------
 drivers/iommu/omap-iommu.c     | 11 +++---
 drivers/iommu/rockchip-iommu.c | 11 +++---
 drivers/iommu/sun50i-iommu.c   | 10 +++---
 drivers/iommu/tegra-gart.c     | 12 ++++---
 drivers/iommu/tegra-smmu.c     | 12 ++++---
 include/linux/iommu.h          |  6 ----
 8 files changed, 58 insertions(+), 80 deletions(-)

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

Robin Murphy

author reviewer

2023-09-12 16:18 UTC #1 lore raw

PATCH 1/8 iommu/exynos: Update to {map,unmap}_pages

Trivially update map/unmap to the new interface, which is quite happy for drivers to still process just one page per call.

Signed-off-by Robin Murphy <[email protected]>

---
 drivers/iommu/exynos-iommu.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

drivers/iommu/exynos-iommu.c +6 -4

index c275fe71c4db..8892f5fb6bf0 100644
@@ -1219,7 +1219,7 @@ static int lv2set_page(sysmmu_pte_t *pent, phys_addr_t paddr, size_t size,
  */
 static int exynos_iommu_map(struct iommu_domain *iommu_domain,
 			    unsigned long l_iova, phys_addr_t paddr, size_t size,
-			    int prot, gfp_t gfp)
+			    size_t count, int prot, gfp_t gfp, size_t *mapped)
 {
 	struct exynos_iommu_domain *domain = to_exynos_domain(iommu_domain);
 	sysmmu_pte_t *entry;

@@ -1253,6 +1253,8 @@ static int exynos_iommu_map(struct iommu_domain *iommu_domain,
 	if (ret)
 		pr_err("%s: Failed(%d) to map %#zx bytes @ %#x\n",
 			__func__, ret, size, iova);
+	else
+		*mapped = size;
 
 	spin_unlock_irqrestore(&domain->pgtablelock, flags);

@@ -1274,7 +1276,7 @@ static void exynos_iommu_tlb_invalidate_entry(struct exynos_iommu_domain *domain
 }
 
 static size_t exynos_iommu_unmap(struct iommu_domain *iommu_domain,
-				 unsigned long l_iova, size_t size,
+				 unsigned long l_iova, size_t size, size_t count,
 				 struct iommu_iotlb_gather *gather)
 {
 	struct exynos_iommu_domain *domain = to_exynos_domain(iommu_domain);

@@ -1482,8 +1484,8 @@ static const struct iommu_ops exynos_iommu_ops = {
 	.of_xlate = exynos_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev	= exynos_iommu_attach_device,
-		.map		= exynos_iommu_map,
-		.unmap		= exynos_iommu_unmap,
+		.map_pages	= exynos_iommu_map,
+		.unmap_pages	= exynos_iommu_unmap,
 		.iova_to_phys	= exynos_iommu_iova_to_phys,
 		.free		= exynos_iommu_domain_free,
 	}

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

Robin Murphy

author reviewer

2023-09-12 16:18 UTC #2 lore raw

PATCH 2/8 iommu/omap: Update to {map,unmap}_pages

In reply to Robin Murphy

Trivially update map/unmap to the new interface, which is quite happy for drivers to still process just one page per call.

Signed-off-by Robin Murphy <[email protected]>

---
 drivers/iommu/omap-iommu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

drivers/iommu/omap-iommu.c +7 -4

index 537e402f9bba..9e07dc94af76 100644
@@ -1318,7 +1318,8 @@ static u32 iotlb_init_entry(struct iotlb_entry *e, u32 da, u32 pa, int pgsz)
 }
 
 static int omap_iommu_map(struct iommu_domain *domain, unsigned long da,
-			  phys_addr_t pa, size_t bytes, int prot, gfp_t gfp)
+			  phys_addr_t pa, size_t bytes, size_t count,
+			  int prot, gfp_t gfp, size_t *mapped)
 {
 	struct omap_iommu_domain *omap_domain = to_omap_domain(domain);
 	struct device *dev = omap_domain->dev;

@@ -1356,13 +1357,15 @@ static int omap_iommu_map(struct iommu_domain *domain, unsigned long da,
 			oiommu = iommu->iommu_dev;
 			iopgtable_clear_entry(oiommu, da);
 		}
+	} else {
+		*mapped = bytes;
 	}
 
 	return ret;
 }
 
 static size_t omap_iommu_unmap(struct iommu_domain *domain, unsigned long da,
-			       size_t size, struct iommu_iotlb_gather *gather)
+			       size_t size, size_t count, struct iommu_iotlb_gather *gather)
 {
 	struct omap_iommu_domain *omap_domain = to_omap_domain(domain);
 	struct device *dev = omap_domain->dev;

@@ -1740,8 +1743,8 @@ static const struct iommu_ops omap_iommu_ops = {
 	.pgsize_bitmap	= OMAP_IOMMU_PGSIZES,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev	= omap_iommu_attach_dev,
-		.map		= omap_iommu_map,
-		.unmap		= omap_iommu_unmap,
+		.map_pages	= omap_iommu_map,
+		.unmap_pages	= omap_iommu_unmap,
 		.iova_to_phys	= omap_iommu_iova_to_phys,
 		.free		= omap_iommu_domain_free,
 	}

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

Robin Murphy

author reviewer

2023-09-12 16:18 UTC #3 lore raw

PATCH 3/8 iommu/rockchip: Update to {map,unmap}_pages

In reply to Robin Murphy

Trivially update map/unmap to the new interface, which is quite happy for drivers to still process just one page per call.

Signed-off-by Robin Murphy <[email protected]>

---
 drivers/iommu/rockchip-iommu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

drivers/iommu/rockchip-iommu.c +7 -4

index 8ff69fbf9f65..bb00c67c1015 100644
@@ -817,7 +817,8 @@ static int rk_iommu_map_iova(struct rk_iommu_domain *rk_domain, u32 *pte_addr,
 }
 
 static int rk_iommu_map(struct iommu_domain *domain, unsigned long _iova,
-			phys_addr_t paddr, size_t size, int prot, gfp_t gfp)
+			phys_addr_t paddr, size_t size, size_t count,
+			int prot, gfp_t gfp, size_t *mapped)
 {
 	struct rk_iommu_domain *rk_domain = to_rk_domain(domain);
 	unsigned long flags;

@@ -850,12 +851,14 @@ static int rk_iommu_map(struct iommu_domain *domain, unsigned long _iova,
 				paddr, size, prot);
 
 	spin_unlock_irqrestore(&rk_domain->dt_lock, flags);
+	if (!ret)
+		*mapped = size;
 
 	return ret;
 }
 
 static size_t rk_iommu_unmap(struct iommu_domain *domain, unsigned long _iova,
-			     size_t size, struct iommu_iotlb_gather *gather)
+			     size_t size, size_t count, struct iommu_iotlb_gather *gather)
 {
 	struct rk_iommu_domain *rk_domain = to_rk_domain(domain);
 	unsigned long flags;

@@ -1197,8 +1200,8 @@ static const struct iommu_ops rk_iommu_ops = {
 	.of_xlate = rk_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev	= rk_iommu_attach_device,
-		.map		= rk_iommu_map,
-		.unmap		= rk_iommu_unmap,
+		.map_pages	= rk_iommu_map,
+		.unmap_pages	= rk_iommu_unmap,
 		.iova_to_phys	= rk_iommu_iova_to_phys,
 		.free		= rk_iommu_domain_free,
 	}

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

Robin Murphy

author reviewer

2023-09-12 16:18 UTC #4 lore raw

PATCH 4/8 iommu/sun50i: Update to {map,unmap}_pages

In reply to Robin Murphy

Trivially update map/unmap to the new interface, which is quite happy for drivers to still process just one page per call.

Signed-off-by Robin Murphy <[email protected]>

---
 drivers/iommu/sun50i-iommu.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

drivers/iommu/sun50i-iommu.c +6 -4

index 74c5cb93e900..cb4cbba4c22f 100644
@@ -589,7 +589,8 @@ static u32 *sun50i_dte_get_page_table(struct sun50i_iommu_domain *sun50i_domain,
 }
 
 static int sun50i_iommu_map(struct iommu_domain *domain, unsigned long iova,
-			    phys_addr_t paddr, size_t size, int prot, gfp_t gfp)
+			    phys_addr_t paddr, size_t size, size_t count,
+			    int prot, gfp_t gfp, size_t *mapped)
 {
 	struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain);
 	struct sun50i_iommu *iommu = sun50i_domain->iommu;

@@ -616,13 +617,14 @@ static int sun50i_iommu_map(struct iommu_domain *domain, unsigned long iova,
 
 	*pte_addr = sun50i_mk_pte(paddr, prot);
 	sun50i_table_flush(sun50i_domain, pte_addr, 1);
+	*mapped = size;
 
 out:
 	return ret;
 }
 
 static size_t sun50i_iommu_unmap(struct iommu_domain *domain, unsigned long iova,
-				 size_t size, struct iommu_iotlb_gather *gather)
+				 size_t size, size_t count, struct iommu_iotlb_gather *gather)
 {
 	struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain);
 	phys_addr_t pt_phys;

@@ -838,8 +840,8 @@ static const struct iommu_ops sun50i_iommu_ops = {
 		.iotlb_sync_map = sun50i_iommu_iotlb_sync_map,
 		.iotlb_sync	= sun50i_iommu_iotlb_sync,
 		.iova_to_phys	= sun50i_iommu_iova_to_phys,
-		.map		= sun50i_iommu_map,
-		.unmap		= sun50i_iommu_unmap,
+		.map_pages	= sun50i_iommu_map,
+		.unmap_pages	= sun50i_iommu_unmap,
 		.free		= sun50i_iommu_domain_free,
 	}
 };

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

Robin Murphy

author reviewer

2023-09-12 16:18 UTC #5 lore raw

PATCH 5/8 iommu/tegra-gart: Update to {map,unmap}_pages

In reply to Robin Murphy

Trivially update map/unmap to the new interface, which is quite happy for drivers to still process just one page per call.

Signed-off-by Robin Murphy <[email protected]>

---
 drivers/iommu/tegra-gart.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

drivers/iommu/tegra-gart.c +8 -4

index a482ff838b53..a0136cd881e6 100644
@@ -178,7 +178,8 @@ static inline int __gart_iommu_map(struct gart_device *gart, unsigned long iova,
 }
 
 static int gart_iommu_map(struct iommu_domain *domain, unsigned long iova,
-			  phys_addr_t pa, size_t bytes, int prot, gfp_t gfp)
+			  phys_addr_t pa, size_t bytes, size_t count,
+			  int prot, gfp_t gfp, size_t *mapped)
 {
 	struct gart_device *gart = gart_handle;
 	int ret;

@@ -190,6 +191,9 @@ static int gart_iommu_map(struct iommu_domain *domain, unsigned long iova,
 	ret = __gart_iommu_map(gart, iova, (unsigned long)pa);
 	spin_unlock(&gart->pte_lock);
 
+	if (!ret)
+		*mapped = bytes;
+
 	return ret;
 }

@@ -207,7 +211,7 @@ static inline int __gart_iommu_unmap(struct gart_device *gart,
 }
 
 static size_t gart_iommu_unmap(struct iommu_domain *domain, unsigned long iova,
-			       size_t bytes, struct iommu_iotlb_gather *gather)
+			       size_t bytes, size_t count, struct iommu_iotlb_gather *gather)
 {
 	struct gart_device *gart = gart_handle;
 	int err;

@@ -275,8 +279,8 @@ static const struct iommu_ops gart_iommu_ops = {
 	.of_xlate	= gart_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev	= gart_iommu_attach_dev,
-		.map		= gart_iommu_map,
-		.unmap		= gart_iommu_unmap,
+		.map_pages	= gart_iommu_map,
+		.unmap_pages	= gart_iommu_unmap,
 		.iova_to_phys	= gart_iommu_iova_to_phys,
 		.iotlb_sync_map	= gart_iommu_sync_map,
 		.iotlb_sync	= gart_iommu_sync,

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

Robin Murphy

author reviewer

2023-09-12 16:18 UTC #6 lore raw

PATCH 6/8 iommu/tegra-smmu: Update to {map,unmap}_pages

In reply to Robin Murphy

Trivially update map/unmap to the new interface, which is quite happy for drivers to still process just one page per call.

Signed-off-by Robin Murphy <[email protected]>

---
 drivers/iommu/tegra-smmu.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

drivers/iommu/tegra-smmu.c +8 -4

index e445f80d0226..4cd2153bb076 100644
@@ -751,7 +751,8 @@ __tegra_smmu_unmap(struct iommu_domain *domain, unsigned long iova,
 }
 
 static int tegra_smmu_map(struct iommu_domain *domain, unsigned long iova,
-			  phys_addr_t paddr, size_t size, int prot, gfp_t gfp)
+			  phys_addr_t paddr, size_t size, size_t count,
+			  int prot, gfp_t gfp, size_t *mapped)
 {
 	struct tegra_smmu_as *as = to_smmu_as(domain);
 	unsigned long flags;

@@ -761,11 +762,14 @@ static int tegra_smmu_map(struct iommu_domain *domain, unsigned long iova,
 	ret = __tegra_smmu_map(domain, iova, paddr, size, prot, gfp, &flags);
 	spin_unlock_irqrestore(&as->lock, flags);
 
+	if (!ret)
+		*mapped = size;
+
 	return ret;
 }
 
 static size_t tegra_smmu_unmap(struct iommu_domain *domain, unsigned long iova,
-			       size_t size, struct iommu_iotlb_gather *gather)
+			       size_t size, size_t count, struct iommu_iotlb_gather *gather)
 {
 	struct tegra_smmu_as *as = to_smmu_as(domain);
 	unsigned long flags;

@@ -971,8 +975,8 @@ static const struct iommu_ops tegra_smmu_ops = {
 	.pgsize_bitmap = SZ_4K,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev	= tegra_smmu_attach_dev,
-		.map		= tegra_smmu_map,
-		.unmap		= tegra_smmu_unmap,
+		.map_pages	= tegra_smmu_map,
+		.unmap_pages	= tegra_smmu_unmap,
 		.iova_to_phys	= tegra_smmu_iova_to_phys,
 		.free		= tegra_smmu_domain_free,
 	}

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

Robin Murphy

author reviewer

2023-09-12 16:19 UTC #7 lore raw

PATCH 7/8 iommu: Retire map/unmap ops

In reply to Robin Murphy

With everyone now implementing the new interfaces, clean up the last remnants of the old map/unmap ops and simplify the calling logic again.

Signed-off-by Robin Murphy <[email protected]>

---
 drivers/iommu/iommu.c | 60 ++++++++++---------------------------------
 include/linux/iommu.h |  6 -----
 2 files changed, 13 insertions(+), 53 deletions(-)

drivers/iommu/iommu.c +13 ~~-47~~

index 3bfc56df4f78..c83f2e4c56f5 100644
@@ -2417,30 +2417,6 @@ static size_t iommu_pgsize(struct iommu_domain *domain, unsigned long iova,
 	return pgsize;
 }
 
-static int __iommu_map_pages(struct iommu_domain *domain, unsigned long iova,
-			     phys_addr_t paddr, size_t size, int prot,
-			     gfp_t gfp, size_t *mapped)
-{
-	const struct iommu_domain_ops *ops = domain->ops;
-	size_t pgsize, count;
-	int ret;
-
-	pgsize = iommu_pgsize(domain, iova, paddr, size, &count);
-
-	pr_debug("mapping: iova 0x%lx pa %pa pgsize 0x%zx count %zu\n",
-		 iova, &paddr, pgsize, count);
-
-	if (ops->map_pages) {
-		ret = ops->map_pages(domain, iova, paddr, pgsize, count, prot,
-				     gfp, mapped);
-	} else {
-		ret = ops->map(domain, iova, paddr, pgsize, prot, gfp);
-		*mapped = ret ? 0 : pgsize;
-	}
-
-	return ret;
-}
-
 static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
 		       phys_addr_t paddr, size_t size, int prot, gfp_t gfp)
 {

@@ -2451,8 +2427,7 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
 	phys_addr_t orig_paddr = paddr;
 	int ret = 0;
 
-	if (unlikely(!(ops->map || ops->map_pages) ||
-		     domain->pgsize_bitmap == 0UL))
+	if (unlikely(!ops->map_pages || domain->pgsize_bitmap == 0UL))
 		return -ENODEV;
 
 	if (unlikely(!(domain->type & __IOMMU_DOMAIN_PAGING)))

@@ -2475,10 +2450,14 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
 	pr_debug("map: iova 0x%lx pa %pa size 0x%zx\n", iova, &paddr, size);
 
 	while (size) {
-		size_t mapped = 0;
+		size_t pgsize, count, mapped = 0;
 
-		ret = __iommu_map_pages(domain, iova, paddr, size, prot, gfp,
-					&mapped);
+		pgsize = iommu_pgsize(domain, iova, paddr, size, &count);
+
+		pr_debug("mapping: iova 0x%lx pa %pa pgsize 0x%zx count %zu\n",
+			 iova, &paddr, pgsize, count);
+		ret = ops->map_pages(domain, iova, paddr, pgsize, count, prot,
+				     gfp, &mapped);
 		/*
 		 * Some pages may have been mapped, even if an error occurred,
 		 * so we should account for those so they can be unmapped.

@@ -2522,19 +2501,6 @@ int iommu_map(struct iommu_domain *domain, unsigned long iova,
 }
 EXPORT_SYMBOL_GPL(iommu_map);
 
-static size_t __iommu_unmap_pages(struct iommu_domain *domain,
-				  unsigned long iova, size_t size,
-				  struct iommu_iotlb_gather *iotlb_gather)
-{
-	const struct iommu_domain_ops *ops = domain->ops;
-	size_t pgsize, count;
-
-	pgsize = iommu_pgsize(domain, iova, iova, size, &count);
-	return ops->unmap_pages ?
-	       ops->unmap_pages(domain, iova, pgsize, count, iotlb_gather) :
-	       ops->unmap(domain, iova, pgsize, iotlb_gather);
-}
-
 static size_t __iommu_unmap(struct iommu_domain *domain,
 			    unsigned long iova, size_t size,
 			    struct iommu_iotlb_gather *iotlb_gather)

@@ -2544,8 +2510,7 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
 	unsigned long orig_iova = iova;
 	unsigned int min_pagesz;
 
-	if (unlikely(!(ops->unmap || ops->unmap_pages) ||
-		     domain->pgsize_bitmap == 0UL))
+	if (unlikely(!ops->unmap_pages || domain->pgsize_bitmap == 0UL))
 		return 0;
 
 	if (unlikely(!(domain->type & __IOMMU_DOMAIN_PAGING)))

@@ -2572,9 +2537,10 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
 	 * or we hit an area that isn't mapped.
 	 */
 	while (unmapped < size) {
-		unmapped_page = __iommu_unmap_pages(domain, iova,
-						    size - unmapped,
-						    iotlb_gather);
+		size_t pgsize, count;
+
+		pgsize = iommu_pgsize(domain, iova, iova, size - unmapped, &count);
+		unmapped_page = ops->unmap_pages(domain, iova, pgsize, count, iotlb_gather);
 		if (!unmapped_page)
 			break;

include/linux/iommu.h +0 -6

index c50a769d569a..5c347029cf70 100644
@@ -312,10 +312,8 @@ struct iommu_ops {
  * * ENODEV	- device specific errors, not able to be attached
  * * <others>	- treated as ENODEV by the caller. Use is discouraged
  * @set_dev_pasid: set an iommu domain to a pasid of device
- * @map: map a physically contiguous memory region to an iommu domain
  * @map_pages: map a physically contiguous set of pages of the same size to
  *             an iommu domain.
- * @unmap: unmap a physically contiguous memory region from an iommu domain
  * @unmap_pages: unmap a number of pages of the same size from an iommu domain
  * @flush_iotlb_all: Synchronously flush all hardware TLBs for this domain
  * @iotlb_sync_map: Sync mappings created recently using @map to the hardware

@@ -334,13 +332,9 @@ struct iommu_domain_ops {
 	int (*set_dev_pasid)(struct iommu_domain *domain, struct device *dev,
 			     ioasid_t pasid);
 
-	int (*map)(struct iommu_domain *domain, unsigned long iova,
-		   phys_addr_t paddr, size_t size, int prot, gfp_t gfp);
 	int (*map_pages)(struct iommu_domain *domain, unsigned long iova,
 			 phys_addr_t paddr, size_t pgsize, size_t pgcount,
 			 int prot, gfp_t gfp, size_t *mapped);
-	size_t (*unmap)(struct iommu_domain *domain, unsigned long iova,
-			size_t size, struct iommu_iotlb_gather *iotlb_gather);
 	size_t (*unmap_pages)(struct iommu_domain *domain, unsigned long iova,
 			      size_t pgsize, size_t pgcount,
 			      struct iommu_iotlb_gather *iotlb_gather);

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

Jason Gunthorpe

Reviewed-by on v1 7/8

2023-09-13 14:40 UTC #8 lore raw

Re: [PATCH 7/8] iommu: Retire map/unmap ops

Quoted 8 lines · Robin Murphy

With everyone now implementing the new interfaces, clean up the last remnants of the old map/unmap ops and simplify the calling logic again.
Signed-off-by Robin Murphy <[email protected]>
---
 drivers/iommu/iommu.c | 60 ++++++++++---------------------------------
 include/linux/iommu.h |  6 -----
 2 files changed, 13 insertions(+), 53 deletions(-)

Reviewed-by Jason Gunthorpe <[email protected]>

Jason

Robin Murphy

author reviewer

2023-09-12 16:19 UTC #9 lore raw

PATCH 8/8 iommu: Improve map/unmap sanity checks

In reply to Robin Murphy

The current checks for the __IOMMU_DOMAIN_PAGING capability seem a bit stifled, since it is quite likely now that a non-paging domain won't have a pgsize_bitmap and/or mapping ops, and thus get caught by the earlier condition anyway. Swap them around to test the more fundamental condition first, then we can reasonably also upgrade the other to a WARN_ON, since if a driver does ever expose a paging domain without the means to actually page, it's clearly very broken.

Signed-off-by Robin Murphy <[email protected]>

---
 drivers/iommu/iommu.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/iommu/iommu.c +5 -5

index c83f2e4c56f5..391bcae4d02d 100644
@@ -2427,12 +2427,12 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
 	phys_addr_t orig_paddr = paddr;
 	int ret = 0;
 
-	if (unlikely(!ops->map_pages || domain->pgsize_bitmap == 0UL))
-		return -ENODEV;
-
 	if (unlikely(!(domain->type & __IOMMU_DOMAIN_PAGING)))
 		return -EINVAL;
 
+	if (WARN_ON(!ops->map_pages || domain->pgsize_bitmap == 0UL))
+		return -ENODEV;
+
 	/* find out the minimum page size supported */
 	min_pagesz = 1 << __ffs(domain->pgsize_bitmap);

@@ -2510,10 +2510,10 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
 	unsigned long orig_iova = iova;
 	unsigned int min_pagesz;
 
-	if (unlikely(!ops->unmap_pages || domain->pgsize_bitmap == 0UL))
+	if (unlikely(!(domain->type & __IOMMU_DOMAIN_PAGING)))
 		return 0;
 
-	if (unlikely(!(domain->type & __IOMMU_DOMAIN_PAGING)))
+	if (WARN_ON(!ops->unmap_pages || domain->pgsize_bitmap == 0UL))
 		return 0;
 
 	/* find out the minimum page size supported */

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

Jason Gunthorpe

2023-09-13 14:39 UTC #10 lore raw

Re: [PATCH 8/8] iommu: Improve map/unmap sanity checks

Quoted 18 lines · Robin Murphy

The current checks for the __IOMMU_DOMAIN_PAGING capability seem a bit stifled, since it is quite likely now that a non-paging domain won't have a pgsize_bitmap and/or mapping ops, and thus get caught by the earlier condition anyway. Swap them around to test the more fundamental condition first, then we can reasonably also upgrade the other to a WARN_ON, since if a driver does ever expose a paging domain without the means to actually page, it's clearly very broken.
Signed-off-by Robin Murphy <[email protected]>
---
 drivers/iommu/iommu.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/iommu/iommu.c +0 -0
index c83f2e4c56f5..391bcae4d02d 100644

drivers/iommu/iommu.c static int __iommu_map(struct iommu_domain *domain, unsigned long iova, · line 2431

Quoted 8 lines

drivers/iommu/iommu.c +0 -3

@@ -2427,12 +2427,12 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
 	phys_addr_t orig_paddr = paddr;
 	int ret = 0;
 
-	if (unlikely(!ops->map_pages || domain->pgsize_bitmap == 0UL))
-		return -ENODEV;
-
 	if (unlikely(!(domain->type & __IOMMU_DOMAIN_PAGING)))
 		return -EINVAL;

Why isn't this a WARN_ON? The caller is clearly lost its mind if it is calling map on a non paging domain..

drivers/iommu/iommu.c static int __iommu_map(struct iommu_domain *domain, unsigned long iova, · line 2435

Quoted 3 lines

drivers/iommu/iommu.c +3 -0

+	if (WARN_ON(!ops->map_pages || domain->pgsize_bitmap == 0UL))
+		return -ENODEV;
+

And these could be moved to after attach so they are not in any fast path, and eventually to after alloc..

Jason

Robin Murphy

author reviewer

2023-09-13 18:46 UTC #11 lore raw

Re: [PATCH 8/8] iommu: Improve map/unmap sanity checks

Quoted 22 lines · Jason Gunthorpe

Quoted 21 lines · Robin Murphy
The current checks for the __IOMMU_DOMAIN_PAGING capability seem a bit stifled, since it is quite likely now that a non-paging domain won't have a pgsize_bitmap and/or mapping ops, and thus get caught by the earlier condition anyway. Swap them around to test the more fundamental condition first, then we can reasonably also upgrade the other to a WARN_ON, since if a driver does ever expose a paging domain without the means to actually page, it's clearly very broken.
Signed-off-by Robin Murphy <[email protected]>
---
  drivers/iommu/iommu.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)
drivers/iommu/iommu.c +0 -0
index c83f2e4c56f5..391bcae4d02d 100644
@@ -2427,12 +2427,12 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
  	phys_addr_t orig_paddr = paddr;
  	int ret = 0;
 

Quoted 8 lines

drivers/iommu/iommu.c static int __iommu_map(struct iommu_domain *domain, unsigned long iova, · line 2431
Quoted 5 lines
drivers/iommu/iommu.c +0 -3
-	if (unlikely(!ops->map_pages || domain->pgsize_bitmap == 0UL))
-		return -ENODEV;
-
  	if (unlikely(!(domain->type & __IOMMU_DOMAIN_PAGING)))
  		return -EINVAL;
Why isn't this a WARN_ON? The caller is clearly lost its mind if it is calling map on a non paging domain..

Sure it's a dumb thing to do, however I don't think we can reasonably say without question that an external caller being dumb represents an unexpected and serious loss of internal consistency.

Quoted 6 lines

drivers/iommu/iommu.c static int __iommu_map(struct iommu_domain *domain, unsigned long iova, · line 2435
Quoted 3 lines
drivers/iommu/iommu.c +3 -0
+	if (WARN_ON(!ops->map_pages || domain->pgsize_bitmap == 0UL))
+		return -ENODEV;
+
And these could be moved to after attach so they are not in any fast path, and eventually to after alloc..

Perhaps, although TBH I can't imagine it making any appreciable difference - they're both values we need to load and use soon anyway, so on a sensible CPU, the additional overhead should only really be a couple of not-taken branch-if-zero instructions, which is nothing at all compared to how much goes on in the main loop and the driver op call itself.

Thanks,
Robin.

Jason Gunthorpe

2023-09-14 12:48 UTC #12 lore raw

Re: [PATCH 8/8] iommu: Improve map/unmap sanity checks

Quoted 31 lines · Robin Murphy

Quoted 14 lines · Jason Gunthorpe
Quoted 13 lines · Robin Murphy
The current checks for the __IOMMU_DOMAIN_PAGING capability seem a bit stifled, since it is quite likely now that a non-paging domain won't have a pgsize_bitmap and/or mapping ops, and thus get caught by the earlier condition anyway. Swap them around to test the more fundamental condition first, then we can reasonably also upgrade the other to a WARN_ON, since if a driver does ever expose a paging domain without the means to actually page, it's clearly very broken.
Signed-off-by Robin Murphy <[email protected]>
---
  drivers/iommu/iommu.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)
Quoted 15 lines
drivers/iommu/iommu.c static int __iommu_map(struct iommu_domain *domain, unsigned long iova, · line 2430
Quoted 12 lines
drivers/iommu/iommu.c +0 -3
index c83f2e4c56f5..391bcae4d02d 100644
@@ -2427,12 +2427,12 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
  	phys_addr_t orig_paddr = paddr;
  	int ret = 0;
-	if (unlikely(!ops->map_pages || domain->pgsize_bitmap == 0UL))
-		return -ENODEV;
-
  	if (unlikely(!(domain->type & __IOMMU_DOMAIN_PAGING)))
  		return -EINVAL;
Why isn't this a WARN_ON? The caller is clearly lost its mind if it is calling map on a non paging domain..

Quoted 3 lines

Sure it's a dumb thing to do, however I don't think we can reasonably say without question that an external caller being dumb represents an unexpected and serious loss of internal consistency.

WARN_ON is not just for "serious loss of internal consistency" it should be use in all places where invariant are violated. We can't guess why the caller is using this wrong (most likely it is UAF or memory corruption), but if this fires something definately has gone wrong with the kernel.

eg if syzkaller somehow hits this we want the WARN_ON so it reports it.

Quoted 7 lines

Quoted 6 lines
drivers/iommu/iommu.c static int __iommu_map(struct iommu_domain *domain, unsigned long iova, · line 2435
Quoted 3 lines
drivers/iommu/iommu.c +3 -0
+	if (WARN_ON(!ops->map_pages || domain->pgsize_bitmap == 0UL))
+		return -ENODEV;
+
And these could be moved to after attach so they are not in any fast path, and eventually to after alloc..

Quoted 5 lines

Perhaps, although TBH I can't imagine it making any appreciable difference - they're both values we need to load and use soon anyway, so on a sensible CPU, the additional overhead should only really be a couple of not-taken branch-if-zero instructions, which is nothing at all compared to how much goes on in the main loop and the driver op call itself.

IMHO it is a cleaner pattern to check the objects the driver creates when it creates them, not when we go to try to use them..

Jason

Robin Murphy

author reviewer

2023-09-14 14:24 UTC #13 lore raw

Re: [PATCH 8/8] iommu: Improve map/unmap sanity checks

Quoted 36 lines · Jason Gunthorpe

Quoted 31 lines · Robin Murphy
Quoted 14 lines · Jason Gunthorpe
Quoted 13 lines · Robin Murphy
The current checks for the __IOMMU_DOMAIN_PAGING capability seem a bit stifled, since it is quite likely now that a non-paging domain won't have a pgsize_bitmap and/or mapping ops, and thus get caught by the earlier condition anyway. Swap them around to test the more fundamental condition first, then we can reasonably also upgrade the other to a WARN_ON, since if a driver does ever expose a paging domain without the means to actually page, it's clearly very broken.
Signed-off-by Robin Murphy <[email protected]>
---
   drivers/iommu/iommu.c | 10 +++++-----
   1 file changed, 5 insertions(+), 5 deletions(-)
Quoted 15 lines
drivers/iommu/iommu.c static int __iommu_map(struct iommu_domain *domain, unsigned long iova, · line 2430
Quoted 12 lines
drivers/iommu/iommu.c +0 -3
index c83f2e4c56f5..391bcae4d02d 100644
@@ -2427,12 +2427,12 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
   	phys_addr_t orig_paddr = paddr;
   	int ret = 0;
-	if (unlikely(!ops->map_pages || domain->pgsize_bitmap == 0UL))
-		return -ENODEV;
-
   	if (unlikely(!(domain->type & __IOMMU_DOMAIN_PAGING)))
   		return -EINVAL;
Why isn't this a WARN_ON? The caller is clearly lost its mind if it is calling map on a non paging domain..
Quoted 3 lines
Sure it's a dumb thing to do, however I don't think we can reasonably say without question that an external caller being dumb represents an unexpected and serious loss of internal consistency.

Quoted 5 lines

WARN_ON is not just for "serious loss of internal consistency" it should be use in all places where invariant are violated. We can't guess why the caller is using this wrong (most likely it is UAF or memory corruption), but if this fires something definately has gone wrong with the kernel.

Has it? It's not *functionally* incorrect to obtain a valid domain by calling iommu_get_domain_for_dev(), pass that domain to iommu_map(), and handle any failure returned. Sure, it's *semantically* questionable, but so is calling iommu_iova_to_phys() on non-paging domains, and we have to support that, because callers are dumb, and "callers aren't dumb" is not a realistic invariant which we can uphold.

Quoted 2 lines

eg if syzkaller somehow hits this we want the WARN_ON so it reports it.

But then an "IOMMU bug" is reported to us, and we say "yeah, that's not ours, that's some code somewhere down in the middle of that callstack being dumb", and then what? I know I don't have the time or inclination to go off debugging and redesigning random other bits of the kernel for calling our API in ways that look sketchy but aren't technically invalid, do you?

iommu_map() can already fail for numerous reasons that may or may not represent a bug in the caller, like the IOVA or PA being out-of-range, or the IOVA already being mapped, or whatever. The wrong type of domain is just another reason to fail (in, as far as we're concerned, an entirely safe and manageable way). Callers have to be prepared to handle failure, but it's up to them how unexpected or serious it is; we can't second-guess them.

Thanks,
Robin.

Jason Gunthorpe

2023-09-14 16:48 UTC #14 lore raw

Re: [PATCH 8/8] iommu: Improve map/unmap sanity checks

Quoted 26 lines · Robin Murphy

Quoted 20 lines
Quoted 16 lines
Quoted 7 lines
Quoted 7 lines
drivers/iommu/iommu.c +0 -0
index c83f2e4c56f5..391bcae4d02d 100644
@@ -2427,12 +2427,12 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
   	phys_addr_t orig_paddr = paddr;
   	int ret = 0;
Quoted 8 lines
drivers/iommu/iommu.c static int __iommu_map(struct iommu_domain *domain, unsigned long iova, · line 2431
Quoted 5 lines
drivers/iommu/iommu.c +0 -3
-	if (unlikely(!ops->map_pages || domain->pgsize_bitmap == 0UL))
-		return -ENODEV;
-
   	if (unlikely(!(domain->type & __IOMMU_DOMAIN_PAGING)))
   		return -EINVAL;
Why isn't this a WARN_ON? The caller is clearly lost its mind if it is calling map on a non paging domain..
Quoted 3 lines
Sure it's a dumb thing to do, however I don't think we can reasonably say without question that an external caller being dumb represents an unexpected and serious loss of internal consistency.
Quoted 5 lines
WARN_ON is not just for "serious loss of internal consistency" it should be use in all places where invariant are violated. We can't guess why the caller is using this wrong (most likely it is UAF or memory corruption), but if this fires something definately has gone wrong with the kernel.

Quoted 3 lines

Has it? It's not *functionally* incorrect to obtain a valid domain by calling iommu_get_domain_for_dev(), pass that domain to iommu_map(), and handle any failure returned.

This is not a theoretical question - does any in-kernel code actually do that and expect it to work? I didn't notice any..

It doesn't matter that someone *could*, our task is not to make an overly general API. We can, and should, have tight invarients because it allows discovery of more bug classes, and prevents "creative" use of APIs.

Quoted 3 lines

is calling iommu_iova_to_phys() on non-paging domains, and we have to support that, because callers are dumb, and "callers aren't dumb" is not a realistic invariant which we can uphold.

Someone does that? :(

Quoted 6 lines

Quoted 2 lines
eg if syzkaller somehow hits this we want the WARN_ON so it reports it.
But then an "IOMMU bug" is reported to us, and we say "yeah, that's not ours, that's some code somewhere down in the middle of that callstack being dumb", and then what?

The report gets handed off to the part of the callstack that is making the problem.

Quoted 3 lines

iommu_map() can already fail for numerous reasons that may or may not represent a bug in the caller, like the IOVA or PA being out-of-range, or the IOVA already being mapped, or whatever.

IMHO some of those certainly could be WARN too, depends what you think the API should be.

eg iommufd and vfio both are designed to never double map/use out of aperture IOVA/etc, if they do it is a significant bug and a WARN_ON would be welcomed.

Quoted 3 lines

another reason to fail (in, as far as we're concerned, an entirely safe and manageable way). Callers have to be prepared to handle failure, but it's up to them how unexpected or serious it is; we can't second-guess them.

We can always convert things into error codes, but that can also hide bugs if no existing caller needs that functionality. IMHO it is always better to have tighter invarients, and be noisy when they fail. This gives us better understandablity, bug discoverability and discourages people from introducing new code doing crazy things.

Jason

Robin Murphy

author reviewer

2023-09-19 12:18 UTC #15 lore raw

Re: [PATCH 8/8] iommu: Improve map/unmap sanity checks

Quoted 27 lines · Jason Gunthorpe

Quoted 26 lines · Robin Murphy
Quoted 25 lines
Quoted 16 lines
Quoted 7 lines
Quoted 7 lines
drivers/iommu/iommu.c +0 -0
index c83f2e4c56f5..391bcae4d02d 100644
@@ -2427,12 +2427,12 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
    	phys_addr_t orig_paddr = paddr;
    	int ret = 0;
Quoted 8 lines
drivers/iommu/iommu.c static int __iommu_map(struct iommu_domain *domain, unsigned long iova, · line 2431
Quoted 5 lines
drivers/iommu/iommu.c +0 -3
-	if (unlikely(!ops->map_pages || domain->pgsize_bitmap == 0UL))
-		return -ENODEV;
-
    	if (unlikely(!(domain->type & __IOMMU_DOMAIN_PAGING)))
    		return -EINVAL;
Why isn't this a WARN_ON? The caller is clearly lost its mind if it is calling map on a non paging domain..
Quoted 3 lines
Sure it's a dumb thing to do, however I don't think we can reasonably say without question that an external caller being dumb represents an unexpected and serious loss of internal consistency.
WARN_ON is not just for "serious loss of internal consistency" it should be use in all places where invariant are violated. We can't guess why the caller is using this wrong (most likely it is UAF or memory corruption), but if this fires something definately has gone wrong with the kernel.

Quoted 6 lines

Quoted 3 lines
Has it? It's not *functionally* incorrect to obtain a valid domain by calling iommu_get_domain_for_dev(), pass that domain to iommu_map(), and handle any failure returned.
This is not a theoretical question - does any in-kernel code actually do that and expect it to work? I didn't notice any..

I was hoping to have to say I'd only seen it in unmerged (e.g. [1]) and downstream code, but then I found drivers/net/ipa/ :(

(and amusingly, the line I happened to look at first blamed you, but that was only for that mechanical change of the iommu_map() prototype)

Quoted 10 lines

It doesn't matter that someone *could*, our task is not to make an overly general API. We can, and should, have tight invarients because it allows discovery of more bug classes, and prevents "creative" use of APIs.
Quoted 3 lines
is calling iommu_iova_to_phys() on non-paging domains, and we have to support that, because callers are dumb, and "callers aren't dumb" is not a realistic invariant which we can uphold.
Someone does that? :(

Only every user of iommu_iova_to_phys() outside of drivers/iommu/, VFIO, and the arch/arm DMA ops...

Quoted 9 lines

Quoted 6 lines
Quoted 2 lines
eg if syzkaller somehow hits this we want the WARN_ON so it reports it.
But then an "IOMMU bug" is reported to us, and we say "yeah, that's not ours, that's some code somewhere down in the middle of that callstack being dumb", and then what?
The report gets handed off to the part of the callstack that is making the problem.

And if it's critical to them that a mapping succeeds, they should probably have their own warning already. An inappropriate domain type is far from critical to us, and is something we've already allowed to fail cleanly and gracefully for the best part of a decade.

Quoted 6 lines

Quoted 3 lines
iommu_map() can already fail for numerous reasons that may or may not represent a bug in the caller, like the IOVA or PA being out-of-range, or the IOVA already being mapped, or whatever.
IMHO some of those certainly could be WARN too, depends what you think the API should be.

We don't even have consistent behaviour across IOMMU drivers for most of them, and some callers are written around particular driver-specific behaviours (yes, even VFIO), so IMO there's no way we're in a position to suddenly decree that anyone's fundamentally wrong for doing something that we've long considered a non-critical failure, but have now decided looks sketchy.

Quoted 8 lines

eg iommufd and vfio both are designed to never double map/use out of aperture IOVA/etc, if they do it is a significant bug and a WARN_ON would be welcomed.
Quoted 3 lines
another reason to fail (in, as far as we're concerned, an entirely safe and manageable way). Callers have to be prepared to handle failure, but it's up to them how unexpected or serious it is; we can't second-guess them.

Quoted 5 lines

We can always convert things into error codes, but that can also hide bugs if no existing caller needs that functionality. IMHO it is always better to have tighter invarients, and be noisy when they fail. This gives us better understandablity, bug discoverability and discourages people from introducing new code doing crazy things.

All that said, I don't mind if you want to propose a separate patch to turn certain external caller conditions into noisy warnings, but I wouldn't want to tie it up with any other changes which would risk getting caught in the crossfire if someone does then hit a newly-introduced warning and start arguing to revert it. And I'm not going to do so in any of my patches since as above I don't personally believe it's a valuable thing to do.

Thanks,
Robin.

Jason Gunthorpe

2023-09-22 17:28 UTC #16 lore raw

Re: [PATCH 8/8] iommu: Improve map/unmap sanity checks

Quoted 5 lines · Robin Murphy

Quoted 2 lines
This is not a theoretical question - does any in-kernel code actually do that and expect it to work? I didn't notice any..
I was hoping to have to say I'd only seen it in unmerged (e.g. [1]) and downstream code, but then I found drivers/net/ipa/ :(

Aiiie, what is that doing? Mapping something into the dma-api owned domain!?!

See this is why I think stronger invarients are a good idea, we can't police the entire kernel but we can make the entry points explode if someone is doing something WRONG.

Quoted 6 lines

All that said, I don't mind if you want to propose a separate patch to turn certain external caller conditions into noisy warnings, but I wouldn't want to tie it up with any other changes which would risk getting caught in the crossfire if someone does then hit a newly-introduced warning and start arguing to revert it. And I'm not going to do so in any of my patches since as above I don't personally believe it's a valuable thing to do.

Ok, that make sense

Jason

Joerg Roedel

maintainer

2023-09-25 10:05 UTC #17 lore raw

In reply to Robin Murphy

Quoted 9 lines · Robin Murphy

Robin Murphy (8):
iommu/exynos: Update to {map,unmap}_pages
iommu/omap: Update to {map,unmap}_pages
iommu/rockchip: Update to {map,unmap}_pages
iommu/sun50i: Update to {map,unmap}_pages
iommu/tegra-gart: Update to {map,unmap}_pages
iommu/tegra-smmu: Update to {map,unmap}_pages
iommu: Retire map/unmap ops
iommu: Improve map/unmap sanity checks

Applied all except the tegra-gart patch. Tegra-gart has been removed by Jason in a previous patch. Thanks Robin.

Jernej Škrabec

maintainer

2023-09-25 15:34 UTC #18 lore raw

Dne ponedeljek, 25. september 2023 ob 12:05:35 CEST je Joerg Roedel napisal(a):

Quoted 11 lines · Robin Murphy

Quoted 9 lines
Robin Murphy (8):
iommu/exynos: Update to {map,unmap}_pages
iommu/omap: Update to {map,unmap}_pages
iommu/rockchip: Update to {map,unmap}_pages
iommu/sun50i: Update to {map,unmap}_pages
iommu/tegra-gart: Update to {map,unmap}_pages
iommu/tegra-smmu: Update to {map,unmap}_pages
iommu: Retire map/unmap ops
iommu: Improve map/unmap sanity checks

Quoted 2 lines

Applied all except the tegra-gart patch. Tegra-gart has been removed by Jason in a previous patch. Thanks Robin.

sun50i is meant to be placeholder (as mentioned in cover letter). It's replaced by:
https://lore.kernel.org/linux-sunxi/[email protected]/

But I didn't get any feedback if it's ok or there is anything to fix.

Best regards,
Jernej

Joerg Roedel

maintainer

2023-09-26 09:23 UTC #19 lore raw

Quoted 3 lines · Jernej Škrabec

sun50i is meant to be placeholder (as mentioned in cover letter). It's replaced by:
https://lore.kernel.org/linux-sunxi/[email protected]/

Is the placeholder patch actually breaking the driver or just not optimal yet?

Regards,

Joerg

Robin Murphy

author reviewer

2023-09-26 12:01 UTC #20 lore raw

Quoted 7 lines · Joerg Roedel

Quoted 3 lines · Jernej Škrabec
sun50i is meant to be placeholder (as mentioned in cover letter). It's replaced by:
https://lore.kernel.org/linux-sunxi/[email protected]/
Is the placeholder patch actually breaking the driver or just not optimal yet?

No functional issue, and TBH I think there's no harm in avoiding the dependency (I was kind of assuming you would pick this series to the core branch, so the "placeholder" status might extend that far too) - it should just mean that when you pick Jernej's patch to a topic branch you'll get a conflict merging it to core, and the correct resolution will be to take everything from that branch and replace my patch.

Cheers,
Robin.

Joerg Roedel

maintainer

2023-09-26 14:57 UTC #21 lore raw

Quoted 6 lines · Robin Murphy

No functional issue, and TBH I think there's no harm in avoiding the dependency (I was kind of assuming you would pick this series to the core branch, so the "placeholder" status might extend that far too) - it should just mean that when you pick Jernej's patch to a topic branch you'll get a conflict merging it to core, and the correct resolution will be to take everything from that branch and replace my patch.

Alright, thanks for clarifying. I hereby ask Jernej to base his new patch on the core branch, once I published it (which should be later today).

Regards,

Joerg