From: Leon Romanovsky <leonro@nvidia.com>

Introduce new DMA APIs to perform DMA linkage of buffers
in layers higher than DMA.

In proposed API, the callers will perform the following steps.
In map path:
	if (dma_can_use_iova(...))
	    dma_iova_alloc()
	    for (page in range)
	       dma_iova_link_next(...)
	    dma_iova_sync(...)
	else
	     /* Fallback to legacy map pages */
             for (all pages)
	       dma_map_page(...)

In unmap path:
	if (dma_can_use_iova(...))
	     dma_iova_destroy()
	else
	     for (all pages)
		dma_unmap_page(...)

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/iommu/dma-iommu.c   | 256 ++++++++++++++++++++++++++++++++++++
 include/linux/dma-map-ops.h |   1 -
 include/linux/dma-mapping.h |  31 +++++
 3 files changed, 287 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index b859f93b1c17..f853762c2d54 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1833,6 +1833,262 @@ void dma_iova_free(struct device *dev, struct dma_iova_state *state)
 }
 EXPORT_SYMBOL_GPL(dma_iova_free);
 
+static int __dma_iova_link(struct device *dev, dma_addr_t addr,
+		phys_addr_t phys, size_t size, enum dma_data_direction dir,
+		unsigned long attrs)
+{
+	bool coherent = dev_is_dma_coherent(dev);
+
+	if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+		arch_sync_dma_for_device(phys, size, dir);
+
+	return iommu_map_nosync(iommu_get_dma_domain(dev), addr, phys, size,
+			dma_info_to_prot(dir, coherent, attrs), GFP_ATOMIC);
+}
+
+static int iommu_dma_iova_bounce_and_link(struct device *dev, dma_addr_t addr,
+		phys_addr_t phys, size_t bounce_len,
+		enum dma_data_direction dir, unsigned long attrs,
+		size_t iova_start_pad)
+{
+	struct iommu_domain *domain = iommu_get_dma_domain(dev);
+	struct iova_domain *iovad = &domain->iova_cookie->iovad;
+	phys_addr_t bounce_phys;
+	int error;
+
+	bounce_phys = iommu_dma_map_swiotlb(dev, phys, bounce_len, dir, attrs);
+	if (bounce_phys == DMA_MAPPING_ERROR)
+		return -ENOMEM;
+
+	error = __dma_iova_link(dev, addr - iova_start_pad,
+			bounce_phys - iova_start_pad,
+			iova_align(iovad, bounce_len), dir, attrs);
+	if (error)
+		swiotlb_tbl_unmap_single(dev, bounce_phys, bounce_len, dir,
+				attrs);
+	return error;
+}
+
+static int iommu_dma_iova_link_swiotlb(struct device *dev,
+		struct dma_iova_state *state, phys_addr_t phys, size_t offset,
+		size_t size, enum dma_data_direction dir, unsigned long attrs)
+{
+	struct iommu_domain *domain = iommu_get_dma_domain(dev);
+	struct iommu_dma_cookie *cookie = domain->iova_cookie;
+	struct iova_domain *iovad = &cookie->iovad;
+	size_t iova_start_pad = iova_offset(iovad, phys);
+	size_t iova_end_pad = iova_offset(iovad, phys + size);
+	dma_addr_t addr = state->addr + offset;
+	size_t mapped = 0;
+	int error;
+
+	if (iova_start_pad) {
+		size_t bounce_len = min(size, iovad->granule - iova_start_pad);
+
+		error = iommu_dma_iova_bounce_and_link(dev, addr, phys,
+				bounce_len, dir, attrs, iova_start_pad);
+		if (error)
+			return error;
+		state->__size |= DMA_IOVA_USE_SWIOTLB;
+
+		mapped += bounce_len;
+		size -= bounce_len;
+		if (!size)
+			return 0;
+	}
+
+	size -= iova_end_pad;
+	error = __dma_iova_link(dev, addr + mapped, phys + mapped, size, dir,
+			attrs);
+	if (error)
+		goto out_unmap;
+	mapped += size;
+
+	if (iova_end_pad) {
+		error = iommu_dma_iova_bounce_and_link(dev, addr + mapped,
+				phys + mapped, iova_end_pad, dir, attrs, 0);
+		if (error)
+			goto out_unmap;
+		state->__size |= DMA_IOVA_USE_SWIOTLB;
+	}
+
+	return 0;
+
+out_unmap:
+	dma_iova_unlink(dev, state, 0, mapped, dir, attrs);
+	return error;
+}
+
+/**
+ * dma_iova_link - Link a range of IOVA space
+ * @dev: DMA device
+ * @state: IOVA state
+ * @phys: physical address to link
+ * @offset: offset into the IOVA state to map into
+ * @size: size of the buffer
+ * @dir: DMA direction
+ * @attrs: attributes of mapping properties
+ *
+ * Link a range of IOVA space for the given IOVA state without IOTLB sync.
+ * This function is used to link multiple physical addresses in contigueous
+ * IOVA space without performing costly IOTLB sync.
+ *
+ * The caller is responsible to call to dma_iova_sync() to sync IOTLB at
+ * the end of linkage.
+ */
+int dma_iova_link(struct device *dev, struct dma_iova_state *state,
+		phys_addr_t phys, size_t offset, size_t size,
+		enum dma_data_direction dir, unsigned long attrs)
+{
+	struct iommu_domain *domain = iommu_get_dma_domain(dev);
+	struct iommu_dma_cookie *cookie = domain->iova_cookie;
+	struct iova_domain *iovad = &cookie->iovad;
+	size_t iova_start_pad = iova_offset(iovad, phys);
+
+	if (WARN_ON_ONCE(iova_start_pad && offset > 0))
+		return -EIO;
+
+	if (dev_use_swiotlb(dev, size, dir) && iova_offset(iovad, phys | size))
+		return iommu_dma_iova_link_swiotlb(dev, state, phys, offset,
+				size, dir, attrs);
+
+	return __dma_iova_link(dev, state->addr + offset - iova_start_pad,
+			phys - iova_start_pad,
+			iova_align(iovad, size + iova_start_pad), dir, attrs);
+}
+EXPORT_SYMBOL_GPL(dma_iova_link);
+
+/**
+ * dma_iova_sync - Sync IOTLB
+ * @dev: DMA device
+ * @state: IOVA state
+ * @offset: offset into the IOVA state to sync
+ * @size: size of the buffer
+ * @ret: return value from the last IOVA operation
+ *
+ * Sync IOTLB for the given IOVA state. This function should be called on
+ * the IOVA-contigous range created by one ore more dma_iova_link() calls
+ * to sync the IOTLB.
+ */
+int dma_iova_sync(struct device *dev, struct dma_iova_state *state,
+		size_t offset, size_t size, int ret)
+{
+	struct iommu_domain *domain = iommu_get_dma_domain(dev);
+	struct iommu_dma_cookie *cookie = domain->iova_cookie;
+	struct iova_domain *iovad = &cookie->iovad;
+	dma_addr_t addr = state->addr + offset;
+	size_t iova_start_pad = iova_offset(iovad, addr);
+
+	addr -= iova_start_pad;
+	size = iova_align(iovad, size + iova_start_pad);
+
+	if (!ret)
+		ret = iommu_sync_map(domain, addr, size);
+	if (ret)
+		iommu_unmap(domain, addr, size);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dma_iova_sync);
+
+static void iommu_dma_iova_unlink_range_slow(struct device *dev,
+		dma_addr_t addr, size_t size, enum dma_data_direction dir,
+		unsigned long attrs)
+{
+	struct iommu_domain *domain = iommu_get_dma_domain(dev);
+	struct iommu_dma_cookie *cookie = domain->iova_cookie;
+	struct iova_domain *iovad = &cookie->iovad;
+	size_t iova_start_pad = iova_offset(iovad, addr);
+	dma_addr_t end = addr + size;
+
+	do {
+		phys_addr_t phys;
+		size_t len;
+
+		phys = iommu_iova_to_phys(domain, addr);
+		if (WARN_ON(!phys))
+			continue;
+		len = min_t(size_t,
+			end - addr, iovad->granule - iova_start_pad);
+
+		if (!dev_is_dma_coherent(dev) &&
+		    !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+			arch_sync_dma_for_cpu(phys, len, dir);
+
+		swiotlb_tbl_unmap_single(dev, phys, len, dir, attrs);
+
+		addr += len;
+		iova_start_pad = 0;
+	} while (addr < end);
+}
+
+static void __iommu_dma_iova_unlink(struct device *dev,
+		struct dma_iova_state *state, size_t offset, size_t size,
+		enum dma_data_direction dir, unsigned long attrs,
+		bool free_iova)
+{
+	struct iommu_domain *domain = iommu_get_dma_domain(dev);
+	struct iommu_dma_cookie *cookie = domain->iova_cookie;
+	struct iova_domain *iovad = &cookie->iovad;
+	dma_addr_t addr = state->addr + offset;
+	size_t iova_start_pad = iova_offset(iovad, addr);
+	struct iommu_iotlb_gather iotlb_gather;
+	size_t unmapped;
+
+	if ((state->__size & DMA_IOVA_USE_SWIOTLB) ||
+	    (!dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC)))
+		iommu_dma_iova_unlink_range_slow(dev, addr, size, dir, attrs);
+
+	iommu_iotlb_gather_init(&iotlb_gather);
+	iotlb_gather.queued = free_iova && READ_ONCE(cookie->fq_domain);
+
+	size = iova_align(iovad, size + iova_start_pad);
+	addr -= iova_start_pad;
+	unmapped = iommu_unmap_fast(domain, addr, size, &iotlb_gather);
+	WARN_ON(unmapped != size);
+
+	if (!iotlb_gather.queued)
+		iommu_iotlb_sync(domain, &iotlb_gather);
+	if (free_iova)
+		iommu_dma_free_iova(cookie, addr, size, &iotlb_gather);
+}
+
+/**
+ * dma_iova_unlink - Unlink a range of IOVA space
+ * @dev: DMA device
+ * @state: IOVA state
+ * @offset: offset into the IOVA state to unlink
+ * @size: size of the buffer
+ * @dir: DMA direction
+ * @attrs: attributes of mapping properties
+ *
+ * Unlink a range of IOVA space for the given IOVA state.
+ */
+void dma_iova_unlink(struct device *dev, struct dma_iova_state *state,
+		size_t offset, size_t size, enum dma_data_direction dir,
+		unsigned long attrs)
+{
+	 __iommu_dma_iova_unlink(dev, state, offset, size, dir, attrs, false);
+}
+EXPORT_SYMBOL_GPL(dma_iova_unlink);
+
+/**
+ * dma_iova_destroy - Finish a DMA mapping transaction
+ * @dev: DMA device
+ * @state: IOVA state
+ * @dir: DMA direction
+ * @attrs: attributes of mapping properties
+ *
+ * Unlink whole IOVA range and free an IOVA space. The range of IOVA from
+ * dma_addr to size must all be linked, and be the only linked IOVA in state
+ */
+void dma_iova_destroy(struct device *dev, struct dma_iova_state *state,
+		enum dma_data_direction dir, unsigned long attrs)
+{
+	__iommu_dma_iova_unlink(dev, state, 0, dma_iova_size(state), dir, attrs,
+			true);
+}
+EXPORT_SYMBOL_GPL(dma_iova_destroy);
+
 void iommu_setup_dma_ops(struct device *dev)
 {
 	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index 6ee626e50708..dced37816ede 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -434,5 +434,4 @@ static inline void debug_dma_dump_mappings(struct device *dev)
 #endif /* CONFIG_DMA_API_DEBUG */
 
 extern const struct dma_map_ops dma_dummy_ops;
-
 #endif /* _LINUX_DMA_MAP_OPS_H */
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 817f11bce7bc..50f0edfe7350 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -313,6 +313,16 @@ static inline bool dma_use_iova(struct dma_iova_state *state)
 bool dma_iova_try_alloc(struct device *dev, struct dma_iova_state *state,
 		phys_addr_t phys, size_t size);
 void dma_iova_free(struct device *dev, struct dma_iova_state *state);
+void dma_iova_destroy(struct device *dev, struct dma_iova_state *state,
+		enum dma_data_direction dir, unsigned long attrs);
+int dma_iova_sync(struct device *dev, struct dma_iova_state *state,
+		size_t offset, size_t size, int ret);
+int dma_iova_link(struct device *dev, struct dma_iova_state *state,
+		phys_addr_t phys, size_t offset, size_t size,
+		enum dma_data_direction dir, unsigned long attrs);
+void dma_iova_unlink(struct device *dev, struct dma_iova_state *state,
+		size_t offset, size_t size, enum dma_data_direction dir,
+		unsigned long attrs);
 #else /* CONFIG_IOMMU_DMA */
 static inline bool dma_use_iova(struct dma_iova_state *state)
 {
@@ -327,6 +337,27 @@ static inline void dma_iova_free(struct device *dev,
 		struct dma_iova_state *state)
 {
 }
+static inline void dma_iova_destroy(struct device *dev,
+		struct dma_iova_state *state, enum dma_data_direction dir,
+		unsigned long attrs)
+{
+}
+static inline int dma_iova_sync(struct device *dev, struct dma_iova_state *state,
+		size_t offset, size_t size, int ret)
+{
+	return -EOPNOTSUPP;
+}
+static inline int dma_iova_link(struct device *dev,
+		struct dma_iova_state *state, phys_addr_t phys, size_t offset,
+		size_t size, enum dma_data_direction dir, unsigned long attrs)
+{
+	return -EOPNOTSUPP;
+}
+static inline void dma_iova_unlink(struct device *dev,
+		struct dma_iova_state *state, size_t offset, size_t size,
+		enum dma_data_direction dir, unsigned long attrs)
+{
+}
 #endif /* CONFIG_IOMMU_DMA */
 
 #if defined(CONFIG_HAS_DMA) && defined(CONFIG_DMA_NEED_SYNC)
-- 
2.46.2

On 2024/10/27 22:21, Leon Romanovsky wrote:
> +/**
> + * dma_iova_sync - Sync IOTLB
> + * @dev: DMA device
> + * @state: IOVA state
> + * @offset: offset into the IOVA state to sync
> + * @size: size of the buffer
> + * @ret: return value from the last IOVA operation
> + *
> + * Sync IOTLB for the given IOVA state. This function should be called on
> + * the IOVA-contigous range created by one ore more dma_iova_link() calls
> + * to sync the IOTLB.
> + */
> +int dma_iova_sync(struct device *dev, struct dma_iova_state *state,
> +		size_t offset, size_t size, int ret)
> +{
> +	struct iommu_domain *domain = iommu_get_dma_domain(dev);
> +	struct iommu_dma_cookie *cookie = domain->iova_cookie;
> +	struct iova_domain *iovad = &cookie->iovad;
> +	dma_addr_t addr = state->addr + offset;
> +	size_t iova_start_pad = iova_offset(iovad, addr);
> +
> +	addr -= iova_start_pad;
> +	size = iova_align(iovad, size + iova_start_pad);
> +
> +	if (!ret)
> +		ret = iommu_sync_map(domain, addr, size);
> +	if (ret)
> +		iommu_unmap(domain, addr, size);

It appears strange that mapping is not done in this helper, but
unmapping is added in the failure path. Perhaps I overlooked anything?
To my understanding, it should like below:

	return iommu_sync_map(domain, addr, size);

In the drivers that make use of this interface should do something like
below:

	ret = dma_iova_sync(...);
	if (ret)
		dma_iova_destroy(...)

> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(dma_iova_sync);

Thanks,
baolu

On Mon, Oct 28, 2024 at 10:00:25AM +0800, Baolu Lu wrote:
> On 2024/10/27 22:21, Leon Romanovsky wrote:
> > +/**
> > + * dma_iova_sync - Sync IOTLB
> > + * @dev: DMA device
> > + * @state: IOVA state
> > + * @offset: offset into the IOVA state to sync
> > + * @size: size of the buffer
> > + * @ret: return value from the last IOVA operation
> > + *
> > + * Sync IOTLB for the given IOVA state. This function should be called on
> > + * the IOVA-contigous range created by one ore more dma_iova_link() calls
> > + * to sync the IOTLB.
> > + */
> > +int dma_iova_sync(struct device *dev, struct dma_iova_state *state,
> > +		size_t offset, size_t size, int ret)
> > +{
> > +	struct iommu_domain *domain = iommu_get_dma_domain(dev);
> > +	struct iommu_dma_cookie *cookie = domain->iova_cookie;
> > +	struct iova_domain *iovad = &cookie->iovad;
> > +	dma_addr_t addr = state->addr + offset;
> > +	size_t iova_start_pad = iova_offset(iovad, addr);
> > +
> > +	addr -= iova_start_pad;
> > +	size = iova_align(iovad, size + iova_start_pad);
> > +
> > +	if (!ret)
> > +		ret = iommu_sync_map(domain, addr, size);
> > +	if (ret)
> > +		iommu_unmap(domain, addr, size);
> 
> It appears strange that mapping is not done in this helper, but
> unmapping is added in the failure path. Perhaps I overlooked anything?

Like iommu_sync_map() is performed on whole continuous range, the iommu_unmap()
should be done on the same range. So, technically you can unmap only part of
the range which called to dma_iova_link() and failed, but you will need
to make sure that iommu_sync_map() is still called for "successful" part of
iommu_map().

In that case, you will need to undo everything anyway and it means that
you will call to iommu_unmap() on the successful part of the range
anyway.

dma_iova_sync() is single operation for the whole range and
iommu_unmap() too, so they are bound together.

> To my understanding, it should like below:
> 
> 	return iommu_sync_map(domain, addr, size);
> 
> In the drivers that make use of this interface should do something like
> below:
> 
> 	ret = dma_iova_sync(...);
> 	if (ret)
> 		dma_iova_destroy(...)

It is actually what is happening in the code, but in less direct way due
to unwinding of the code.

As an simple example, see VFIO patch https://lore.kernel.org/all/0a517ddff099c14fac1ceb0e75f2f50ed183d09c.1730037276.git.leon@kernel.org/
where failed in dma_iova_sync() will trigger call to unregister_dma_pages() and that will call to dma_iova_destroy().

> 
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(dma_iova_sync);
> 
> Thanks,
> baolu
>

On Mon, Oct 28, 2024 at 08:22:52AM +0200, Leon Romanovsky wrote:
> On Mon, Oct 28, 2024 at 10:00:25AM +0800, Baolu Lu wrote:
> > On 2024/10/27 22:21, Leon Romanovsky wrote:
> > > +/**
> > > + * dma_iova_sync - Sync IOTLB
> > > + * @dev: DMA device
> > > + * @state: IOVA state
> > > + * @offset: offset into the IOVA state to sync
> > > + * @size: size of the buffer
> > > + * @ret: return value from the last IOVA operation
> > > + *
> > > + * Sync IOTLB for the given IOVA state. This function should be called on
> > > + * the IOVA-contigous range created by one ore more dma_iova_link() calls
> > > + * to sync the IOTLB.
> > > + */
> > > +int dma_iova_sync(struct device *dev, struct dma_iova_state *state,
> > > +		size_t offset, size_t size, int ret)
> > > +{
> > > +	struct iommu_domain *domain = iommu_get_dma_domain(dev);
> > > +	struct iommu_dma_cookie *cookie = domain->iova_cookie;
> > > +	struct iova_domain *iovad = &cookie->iovad;
> > > +	dma_addr_t addr = state->addr + offset;
> > > +	size_t iova_start_pad = iova_offset(iovad, addr);
> > > +
> > > +	addr -= iova_start_pad;
> > > +	size = iova_align(iovad, size + iova_start_pad);
> > > +
> > > +	if (!ret)
> > > +		ret = iommu_sync_map(domain, addr, size);
> > > +	if (ret)
> > > +		iommu_unmap(domain, addr, size);
> > 
> > It appears strange that mapping is not done in this helper, but
> > unmapping is added in the failure path. Perhaps I overlooked anything?
> 
> Like iommu_sync_map() is performed on whole continuous range, the iommu_unmap()
> should be done on the same range. So, technically you can unmap only part of
> the range which called to dma_iova_link() and failed, but you will need
> to make sure that iommu_sync_map() is still called for "successful" part of
> iommu_map().
> 
> In that case, you will need to undo everything anyway and it means that
> you will call to iommu_unmap() on the successful part of the range
> anyway.
> 
> dma_iova_sync() is single operation for the whole range and
> iommu_unmap() too, so they are bound together.
> 
> > To my understanding, it should like below:
> > 
> > 	return iommu_sync_map(domain, addr, size);
> > 
> > In the drivers that make use of this interface should do something like
> > below:
> > 
> > 	ret = dma_iova_sync(...);
> > 	if (ret)
> > 		dma_iova_destroy(...)
> 
> It is actually what is happening in the code, but in less direct way due
> to unwinding of the code.

After more thoughts on the topic, I think that it will be better to make
this dma_iova_sync() less cryptic and more direct. I will change it to be
as below in my next version:

  1972 int dma_iova_sync(struct device *dev, struct dma_iova_state *state,
  1973                 size_t offset, size_t size)
  1974 {
  1975         struct iommu_domain *domain = iommu_get_dma_domain(dev);
  1976         struct iommu_dma_cookie *cookie = domain->iova_cookie;
  1977         struct iova_domain *iovad = &cookie->iovad;
  1978         dma_addr_t addr = state->addr + offset;
  1979         size_t iova_start_pad = iova_offset(iovad, addr);
  1980
  1981         return iommu_sync_map(domain, addr - iova_start_pad,
  1982                       iova_align(iovad, size + iova_start_pad));
  1983 }
  1984 EXPORT_SYMBOL_GPL(dma_iova_sync);

Thanks