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

[Daniel Drake <dan@reactivated.net>]

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