Re: [PATCH 5/6] iommu: Add Broadcom BCM2712 IOMMU driver

[Daniel Drake]

Hi,

On 12/07/2026 23:11, Jason Gunthorpe wrote:
> > +static int bcm2712_iommu_map_pages(struct iommu_domain *domain,
> > +				   unsigned long iova, phys_addr_t paddr,
> > +				   size_t pgsize, size_t pgcount, int prot,
> > +				   gfp_t gfp, size_t *mapped)
> > +{
> > +	struct bcm2712_iommu_domain *mydomain = to_bcm2712_domain(domain);
> > +	struct pt_iommu *pt = &mydomain->pt.iommu;
> > +
> > +	return pt->ops->map_range(pt, bcm2712_iova_to_offset(mydomain, iova),
> > +				  paddr, pgsize * pgcount, prot, gfp, mapped);
> > +}
>
> These ops should not be present, the iommpt provides the ops directly
> through a macro;
>
> > +static const struct iommu_ops bcm2712_iommu_ops = {
> > +	.identity_domain = &bcm2712_identity_domain,
> > +	.domain_alloc_paging = bcm2712_iommu_domain_alloc,
> > +	.probe_device = bcm2712_iommu_probe_device,
> > +	.device_group = generic_single_device_group,
> > +	.of_xlate = bcm2712_iommu_of_xlate,
> > +	.default_domain_ops = &(const struct iommu_domain_ops) {
> > +		.attach_dev	 = bcm2712_iommu_attach_dev,
> > +		.iotlb_sync      = bcm2712_iommu_sync,
> > +		.iotlb_sync_map  = bcm2712_iommu_sync_map,
> > +		.flush_iotlb_all = bcm2712_iommu_sync_all,
> > +		.free		 = bcm2712_iommu_domain_free,
> > +		.map_pages       = bcm2712_iommu_map_pages,
> > +		.unmap_pages     = bcm2712_iommu_unmap_pages,
> > +		.iova_to_phys    = bcm2712_iova_to_phys,
> > +	},
>
> Then use something like:
>
> 		IOMMU_PT_DOMAIN_OPS(bcm2712),
>
> To define all the page table related ops automatically.
I don't think I can use that because this setup uses an IOVA aperture at 
base 0xA00000000, whereas generic_pt assumes it is managing a 0-indexed 
virtual address space. So the driver has to intercept every incoming 
IOVA and translate for the aperture, see how map_pages calls:

 static inline unsigned long
 bcm2712_iova_to_offset(struct bcm2712_iommu_domain *domain, unsigned 
long iova)
 {
 	return iova - domain->mmu->aperture_start;
 }

That's why I also set is_iommupt=false. I will add a comment to make 
this clear. Let me know if you see a better approach. I was wondering 
about making iommupt understand apertures and translate accordingly, but 
I imagine you would want to keep that kind of thing out of the generic 
fast path?

Thanks for all the other feedback too - working on it!

Daniel