Re: [PATCH v3 7/8] sparc64: add M3000 Oberon PCIe support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] iommu: missing IOMMU TSB clearing in dma_4u_free_coherent for non-Oberon
- [Medium] sparc64: IOMMU context leak in dma_4u_map_sg error path
--

commit 4df317d4338c546c969a0d6073e2c04be876d4a5
Author: Magnus Lindholm <linmag7@gmail.com>

sparc64: add M3000 Oberon PCIe support

This patch adds support for the Fujitsu M3000 Oberon PCIe bridge. It handles
Oberon enumeration, flushes DVMA translations, and routes legacy interrupts
using the bridge's ten-bit target ID. MSI queues are left uninitialized so
the existing architecture path uses legacy interrupts.

> diff --git a/arch/sparc/kernel/iommu.c b/arch/sparc/kernel/iommu.c
> index 7613ab0ffb89d..9132f0535ce43 100644
> --- a/arch/sparc/kernel/iommu.c
> +++ b/arch/sparc/kernel/iommu.c
[ ... ]
> @@ -253,6 +281,15 @@ static void dma_4u_free_coherent(struct device *dev, size_t size,
>  	npages = IO_PAGE_ALIGN(size) >> IO_PAGE_SHIFT;
>  	iommu = dev->archdata.iommu;
>  
> +	if (iommu->iommu_cache_flush) {
> +		iopte_t *base = iommu->page_table +
> +			((dvma - iommu->tbl.table_map_base) >> IO_PAGE_SHIFT);
> +		unsigned long i;
> +
> +		for (i = 0; i < npages; i++)
> +			iopte_make_dummy(iommu, base + i);
> +		iommu_sync_mapping(iommu, base, npages);
> +	}

[Severity: High]
Does this code inadvertently leave valid DMA translations in the hardware page
table for non-Oberon platforms?

When freeing a coherent DMA mapping here, the physical pages are subsequently
released back to the system via free_pages(). By explicitly wrapping the loop
that clears the IOMMU TSB entries with iopte_make_dummy() inside the
if (iommu->iommu_cache_flush) check, non-Oberon hardware skips this step.

This could allow a rogue or delayed DMA from the device to silently corrupt
newly allocated memory. Other unmap paths, like dma_4u_unmap_phys(), appear to
clear TSB entries for all platforms without this restriction.

>  	iommu_tbl_range_free(&iommu->tbl, dvma, npages, IOMMU_ERROR_CODE);
>  
>  	order = get_order(size);
[ ... ]
> @@ -577,6 +619,7 @@ static int dma_4u_map_sg(struct device *dev, struct scatterlist *sglist,
>  
>  			for (j = 0; j < npages; j++)
>  				iopte_make_dummy(iommu, base + j);
> +			iommu_sync_mapping(iommu, base, npages);
>  
>  			iommu_tbl_range_free(&iommu->tbl, vaddr, npages,
>  					     IOMMU_ERROR_CODE);
>  			s->dma_length = 0;
>  		}
>  		if (s == outs)
>  			break;
>  	}
>  	spin_unlock_irqrestore(&iommu->lock, flags);
>  
>  	return -EINVAL;
>  }

[Severity: Medium]
Does this error cleanup path leak the IOMMU context?

Earlier in dma_4u_map_sg(), an IOMMU context is allocated via
iommu_alloc_ctx(). If iommu_tbl_range_alloc() fails for any segment,
execution jumps to the iommu_map_failed label.

The cleanup logic unmaps the previously mapped ranges and releases the
spinlock, but it does not seem to call iommu_free_ctx() to release the
context ID. This could potentially cause a resource leak, leading to IOMMU
mapping failures once all contexts are exhausted.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012184826.986050-1-linmag7@gmail.com?part=7