Re: [PATCH v6 01/16] iommu/dma: Prepare MSI physical address lists
From: Robin Murphy
Date: Fri Oct 02 2026 - 12:28:08 EST
On 25/09/2026 4:16 pm, Andrew Jones wrote:
Software MSI mappings may cover an ordered list of physical addresses
which must be mapped into one contiguous IOVA range. Extend the internal
DMA-IOMMU mapping helpers to accept an address array, address count, and
required mapping granule.
Teach iommu_dma_get_msi_page() to map the list into one contiguous IOVA
allocation. Keep the existing per-page cache entries and mark the first
entry with the size of the allocation so an identical list can reuse the
mapping without changing the existing cache representation.
This prepares for the forthcoming iommu_dma_prepare_msi_list() API.
Sorry, but this looks pretty bonkers, even before we get to patch #8. You already end up adding what is effectively a RISC-V-specific entrypoint, so you may as well just carry that all the way through to its own effectively RISC-V-specific implementation, without making an unmaintainable mess of the existing code.
The current design for both DMA_IOVA and DMA_MSI cookies is based on the (Arm-centric) notion that different devices may be associated with different MSI controllers that are independent of each other, so we only map what we know we need (and have a way to get the address of at all), and the list lookup is to save redundant mappings and IOVA space when devices do happen to share. Your requirement is almost the complete opposite, where *any* device needs *every* possible MSI controller mapped up-front, plus the MSI controller driver has to be in on this notion too, so it really doesn't fit the same logic well at all. In fact IIUC it should be far simpler - you shouldn't need a list, nor even really care about the addresses, it should merely be a case of whether a) this is the first call for the given cookie so everything needs mapping, or b) it's not the first call, so everything must already be mapped and we can just return the IOVA. If trying to cram these opposing notions down the same path results in a bunch of weird complexity for pretending to support distinct and overlapping values of "everything", which neither case needs, that seems like a pretty clear sign of it being a bad idea IMO.
Note that since the core cookie rework, untangling the DMA_MSI implementation from DMA_IOVA has been on the table, at which point adding more top-level types of MSI-only cookies would clearly be straightforward. However for DMA_IOVA that could end up getting a bit combinatorial, and keeping an internal sub-type (like for flush queues) would probably be simpler, so it may well make sense to take the latter approach for both, at least to start with. But getting rid of those odd special cases in iommu_dma_{alloc,free}_iova() and the clunkiness of cookie_msi_{granule,pages}() would still be nice either way...
At very worst, a separate hook to just pre-populate msi_page_list with a regular page for each IMSIC address, such that the existing reuse mechanism keeps working as-is, could probably suffice without any other major structural changes; it's only really the IOVA allocation and the fact that it all has to be done under a single lock acquisition that's special.
Thanks,
Robin.
Signed-off-by: Andrew Jones <andrew.jones@xxxxxxxxxxxxxxxx>
Tested-by: Fangyu Yu <fangyu.yu@xxxxxxxxxxxxxxxxx>
---
drivers/iommu/dma-iommu.c | 116 +++++++++++++++++++++++++++++---------
1 file changed, 88 insertions(+), 28 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 58c624513cd4..0b4eb47d1a95 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -42,6 +42,7 @@ struct iommu_dma_msi_page {
struct list_head list;
dma_addr_t iova;
phys_addr_t phys;
+ size_t range_size; /* IOVA range size, or 0 if not the first map */
};
enum iommu_dma_queue_type {
@@ -481,7 +482,7 @@ static int cookie_init_hw_msi_region(struct iommu_dma_cookie *cookie,
num_pages = iova_align(iovad, end - start) >> iova_shift(iovad);
for (i = 0; i < num_pages; i++) {
- msi_page = kmalloc_obj(*msi_page);
+ msi_page = kzalloc_obj(*msi_page);
if (!msi_page)
return -ENOMEM;
@@ -2191,15 +2192,42 @@ static struct list_head *cookie_msi_pages(const struct iommu_domain *domain)
}
}
+static bool iommu_dma_msi_range_matches(struct list_head *msi_page_list,
+ const struct iommu_dma_msi_page *base_page,
+ const phys_addr_t *phys_addrs,
+ unsigned int nr_addrs, size_t granule)
+{
+ const struct iommu_dma_msi_page *msi_page;
+ unsigned int nr_found = 0;
+ dma_addr_t offset;
+
+ list_for_each_entry(msi_page, msi_page_list, list) {
+ if (msi_page->iova < base_page->iova)
+ continue;
+ offset = msi_page->iova - base_page->iova;
+ if (offset >= base_page->range_size)
+ continue;
+ if (!IS_ALIGNED((size_t)offset, granule) ||
+ msi_page->phys != phys_addrs[(size_t)offset / granule])
+ return false;
+ nr_found++;
+ }
+
+ return nr_found == nr_addrs;
+}
+
static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
- phys_addr_t msi_addr, struct iommu_domain *domain)
+ const phys_addr_t *phys_addrs, unsigned int nr_addrs, size_t granule,
+ struct iommu_domain *domain)
{
struct list_head *msi_page_list = cookie_msi_pages(domain);
- struct iommu_dma_msi_page *msi_page;
- dma_addr_t iova;
+ struct iommu_dma_msi_page *msi_page, *first_page = NULL;
int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
- size_t size = cookie_msi_granule(domain);
static DEFINE_MUTEX(msi_prepare_lock);
+ LIST_HEAD(new_msi_pages);
+ dma_addr_t base_iova;
+ unsigned int i;
+ size_t size;
/*
* Normally a device's default domain is only ever attached to that
@@ -2213,32 +2241,60 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
*/
guard(mutex)(&msi_prepare_lock);
- msi_addr &= ~(phys_addr_t)(size - 1);
- list_for_each_entry(msi_page, msi_page_list, list)
- if (msi_page->phys == msi_addr)
+ if (!nr_addrs || nr_addrs > SIZE_MAX / granule)
+ return NULL;
+ size = nr_addrs * granule;
+
+ list_for_each_entry(msi_page, msi_page_list, list) {
+ if (msi_page->phys != phys_addrs[0])
+ continue;
+ if (nr_addrs == 1)
+ return msi_page;
+ if (msi_page->range_size == size &&
+ iommu_dma_msi_range_matches(msi_page_list, msi_page, phys_addrs,
+ nr_addrs, granule))
return msi_page;
+ }
- msi_page = kzalloc_obj(*msi_page);
- if (!msi_page)
- return NULL;
+ for (i = 0; i < nr_addrs; i++) {
+ msi_page = kzalloc_obj(*msi_page);
+ if (!msi_page)
+ goto out_free_pages;
+ list_add_tail(&msi_page->list, &new_msi_pages);
+ }
- iova = iommu_dma_alloc_iova(domain, size, dma_get_mask(dev), dev);
- if (!iova)
- goto out_free_page;
+ base_iova = iommu_dma_alloc_iova(domain, size, dma_get_mask(dev), dev);
+ if (!base_iova)
+ goto out_free_pages;
- if (iommu_map(domain, iova, msi_addr, size, prot, GFP_KERNEL))
- goto out_free_iova;
+ i = 0;
+ list_for_each_entry(msi_page, &new_msi_pages, list) {
+ msi_page->phys = phys_addrs[i];
+ msi_page->iova = base_iova + i * granule;
+ if (!i)
+ first_page = msi_page;
+ if (iommu_map(domain, msi_page->iova, msi_page->phys, granule, prot, GFP_KERNEL))
+ goto out_unmap;
+ i++;
+ }
- INIT_LIST_HEAD(&msi_page->list);
- msi_page->phys = msi_addr;
- msi_page->iova = iova;
- list_add(&msi_page->list, msi_page_list);
- return msi_page;
+ first_page->range_size = size;
+ list_splice(&new_msi_pages, msi_page_list);
+ return first_page;
-out_free_iova:
- iommu_dma_free_iova(domain, iova, size, NULL);
-out_free_page:
- kfree(msi_page);
+out_unmap:
+ if (i) {
+ size_t unmap = iommu_unmap(domain, base_iova, i * granule);
+
+ WARN_ON_ONCE(unmap != i * granule);
+ }
+ iommu_dma_free_iova(domain, base_iova, size, NULL);
+out_free_pages:
+ while (!list_empty(&new_msi_pages)) {
+ msi_page = list_first_entry(&new_msi_pages, typeof(*msi_page), list);
+ list_del(&msi_page->list);
+ kfree(msi_page);
+ }
return NULL;
}
@@ -2247,19 +2303,23 @@ int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
{
struct device *dev = msi_desc_to_dev(desc);
const struct iommu_dma_msi_page *msi_page;
+ phys_addr_t phys_addr;
+ size_t granule;
if (!has_msi_cookie(domain)) {
msi_desc_set_iommu_msi_iova(desc, 0, 0);
return 0;
}
+ granule = cookie_msi_granule(domain);
+ phys_addr = ALIGN_DOWN(msi_addr, granule);
+
iommu_group_mutex_assert(dev);
- msi_page = iommu_dma_get_msi_page(dev, msi_addr, domain);
+ msi_page = iommu_dma_get_msi_page(dev, &phys_addr, 1, granule, domain);
if (!msi_page)
return -ENOMEM;
- msi_desc_set_iommu_msi_iova(desc, msi_page->iova,
- ilog2(cookie_msi_granule(domain)));
+ msi_desc_set_iommu_msi_iova(desc, msi_page->iova, ilog2(granule));
return 0;
}