Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Mapping guest_memfd CPU RAM as uncached MMIO creates memory attribute aliasing, breaking bisection. --
--- Patch [5]: [RFC PATCH 05/11] iommufd: Look up private-interconnect phys via exporter symbols --- commit a4bfbccbc028813d0ddd8502c6dc498ce23a16cb Author: David Woodhouse <[email protected]> iommufd: Look up private-interconnect phys via exporter symbols This commit widens the dma-buf phys-map dispatch in iommufd to support additional exporters by looking up a well-known symbol convention. It initially tries a sample gmem provider before falling back to VFIO PCI. > diff --git a/drivers/iommu/iommufd/pages.c b/drivers/iommu/iommufd/pages.c > index 03c8379bbc347..2d4ea41460fdc 100644 > --- a/drivers/iommu/iommufd/pages.c > +++ b/drivers/iommu/iommufd/pages.c [ ... ] > @@ -1470,6 +1470,26 @@ sym_vfio_pci_dma_buf_iommufd_map(struct > dma_buf_attachment *attachment, > if (rc != -EOPNOTSUPP) > return rc; > > + /* > + * Prototype: try the sample gmem provider's dma-buf exporter. This > + * mirrors the vfio-pci private-interconnect hook, and (like it) is > + * meant to be replaced by a formal negotiated exporter op returning > + * phys for iommufd. > + */ > + { > + extern int gmem_provider_dma_buf_iommufd_map( > + struct dma_buf_attachment *, struct phys_vec *); > + typeof(&gmem_provider_dma_buf_iommufd_map) gfn; > + > + gfn = symbol_get(gmem_provider_dma_buf_iommufd_map); > + if (gfn) { > + rc = gfn(attachment, phys); > + symbol_put(gmem_provider_dma_buf_iommufd_map); > + if (rc != -EOPNOTSUPP) > + return rc; [Severity: High] Will mapping guest_memfd CPU RAM as uncached MMIO cause memory attribute aliasing issues here? When gmem_provider_dma_buf_iommufd_map returns a physical address for RAM, pfn_reader_fill_dmabuf() will hardcode it as BATCH_MMIO: drivers/iommu/iommufd/pages.c:pfn_reader_fill_dmabuf() { ... batch_add_pfn_num(batch, PHYS_PFN(dmabuf->phys.paddr + start), last_index - start_index + 1, BATCH_MMIO); ... } This causes batch_to_domain() to forcefully strip IOMMU_CACHE and apply IOMMU_MMIO: drivers/iommu/iommufd/pages.c:batch_to_domain() { ... if (batch->kind == BATCH_MMIO) { iommu_prot &= ~IOMMU_CACHE; iommu_prot |= IOMMU_MMIO; } ... } On architectures like ARM64, this memory attribute aliasing between the CPU (cacheable) and the IOMMU (uncached) can lead to a loss of coherency, data corruption, or fatal SErrors. Since the memory type plumbing is deferred to a later patch, could this intermediate state introduce a bisection breakage if the sample provider is utilized? > + } > + } > + > if (!IS_ENABLED(CONFIG_VFIO_PCI_DMABUF)) > return -EOPNOTSUPP; > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/f09647f427565c1789a6979470045aab78661563.1784194290.git.d...@amazon.co.uk?part=5
