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

[Daniel Drake]

On 28/07/2026 23:57, Jim Quinlan wrote:
> > +#define MMMU_CTRL_OPERATING_FLAGS (\
> > +	MMMU_CTRL_CAP_EXCEEDED_ABORT_EN    | \
> > +	MMMU_CTRL_PT_INVALID_ABORT_EN      | \
> > +	MMMU_CTRL_PT_INVALID_EN            | \
> > +	MMMU_CTRL_WRITE_VIOLATION_ABORT_EN | \
> > +	MMMU_CTRL_STATS_ENABLE             | \
> > +	MMMU_CTRL_ENABLE)
>
> Hello,
>
> Why not request the MMU interrupt and use the "INT_EN" version, e.g.
> MMMU_CTRL_PT_INVALID_INT_EN?  It seems better than "ABORT", as with an
> interrupt you have a fighting chance of the handler printing out something
> helpful and shutting down gracefully.

That could indeed be useful, but the only info I have about this 
hardware is the RPi kernel's bcm2712-iommu driver which also does not 
handle this interrupt, so I don't have the required info to set up an 
interrupt handler.

Could you let me know the interrupt routing details for all of the 
bcm2712 iommu blocks? e.g. which GIC/L2 IRQ lines they connect to.
> > +	/* Shootdown register deals with 4 pages at a time */
> > +	for (page_group = iova >> (IOMMU_PAGE_SHIFT + 2);
> > +	     page_group <= iova_end >> (IOMMU_PAGE_SHIFT + 2); page_group++) {
> > +		MMU_WR(MMMU_SHOOT_DOWN_OFFSET,
> > +		       MMMU_SHOOT_DOWN_SHOOT + (page_group << 2));
> > +		readl_poll_timeout_atomic(
> > +			mmu->reg_base + MMMU_SHOOT_DOWN_OFFSET, val,
> > +			!(val & MMMU_SHOOT_DOWN_SHOOTING), 0, 1000);
>
> I think it would be prudent to have this function return the
> readl_poll_timeout_atomic() result and then do a dev_{warn,err}() call once
> you are out of the containing spinlock.  Also, why 1ms -- seems a long
> time to have a spinlock.

I'll do that. And yeah, that is way too long. I took some measurements 
and all 3 of the TLB-related cache flush codepaths take around 100ns on 
average. So I'm reducing the timeouts to 50usec.

Thanks!