Re: [RFC PATCH 05/11] iommufd: Look up private-interconnect phys via exporter symbols

[sashiko-bot@kernel.org]

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 <dwmw@amazon.co.uk>

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.dwmw@amazon.co.uk?part=5