Re: [RFC PATCH 06/11] iommufd: Plumb dma-buf memory-type (RAM vs MMIO) through the phys map

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Secondary domain attachments map MMIO DMA-bufs with BATCH_CPU_MEMORY (IOMMU_CACHE) instead of BATCH_MMIO (IOMMU_MMIO), breaking DMA routing.
--

--- Patch [6]: [RFC PATCH 06/11] iommufd: Plumb dma-buf memory-type (RAM vs MMIO) through the phys map ---
commit de7dbdc51049f527858115cc709f25b2cee585cb
Author: David Woodhouse <dwmw@amazon.co.uk>

iommufd: Plumb dma-buf memory-type (RAM vs MMIO) through the phys map

This commit plumbs the memory type of a dma-buf (CPU RAM vs MMIO) through
the IOMMU programming path. It adds an is_cpu_ram flag to dmabuf structures
to correctly choose between BATCH_CPU_MEMORY and BATCH_MMIO during mapping.
This prevents silent misrouting of device DMA on AMD-Vi when the physical
memory is RAM instead of a PCI BAR.

> diff --git a/drivers/iommu/iommufd/pages.c b/drivers/iommu/iommufd/pages.c
> index 2d4ea41460fdc..f9b2ae6d7e96c 100644
> --- a/drivers/iommu/iommufd/pages.c
> +++ b/drivers/iommu/iommufd/pages.c

[ ... ]

> @@ -1106,9 +1108,15 @@ static int pfn_reader_fill_dmabuf(struct pfn_reader_dmabuf *dmabuf,
>  	 * always filled using page size aligned PFNs just like the other types.
>  	 * If the dmabuf has been sliced on a sub page offset then the common
>  	 * batch to domain code will adjust it before mapping to the domain.
> +	 *
> +	 * The exporter's memory type (CPU RAM vs MMIO/BAR) selects the batch
> +	 * kind so downstream iommu_map sets IOMMU_CACHE for cache-coherent RAM
> +	 * or IOMMU_MMIO for BAR memory.  The kind was captured at map time by
> +	 * the exporter dispatch.
>  	 */
>  	batch_add_pfn_num(batch, PHYS_PFN(dmabuf->phys.paddr + start),
> -			  last_index - start_index + 1, BATCH_MMIO);
> +			  last_index - start_index + 1,
> +			  dmabuf->is_cpu_ram ? BATCH_CPU_MEMORY : BATCH_MMIO);
>  	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but does attaching a secondary domain map MMIO
DMA-bufs with BATCH_CPU_MEMORY instead of BATCH_MMIO?

When reading PFNs for a secondary domain, pfn_reader_fill_span() routes the
read to batch_from_domain() instead of pfn_reader_fill_dmabuf():

pfn_reader_fill_span() {
	if (span->is_used == 2) {
		/* ... */
		batch_from_domain(
			&pfns->batch, area->storage_domain, area, ...);
		return 0;
	}
}

Then batch_from_domain() extracts PFNs from the already mapped domain and
unconditionally delegates to batch_add_pfn():

batch_from_domain() {
	/* ... */
	phys = iommu_iova_to_phys(domain, iova) - page_offset;
	if (!batch_add_pfn(batch, PHYS_PFN(phys)))
		return;
}

And batch_add_pfn() explicitly hardcodes BATCH_CPU_MEMORY:

static bool batch_add_pfn(struct pfn_batch *batch, unsigned long pfn)
{
	return batch_add_pfn_num(batch, pfn, 1, BATCH_CPU_MEMORY);
}

Could this silently overwrite the original memory type (e.g., BATCH_MMIO)
for secondary domain mappings, causing device DMA to misroute on AMD-Vi
even after this patch?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/f09647f427565c1789a6979470045aab78661563.1784194290.git.dwmw@amazon.co.uk?part=6