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 <[email protected]> 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.d...@amazon.co.uk?part=6
