Re: [PATCH v2 4/5] iommu: Add Broadcom BCM2712 IOMMU driver
Daniel Drake <[email protected]>
| Newsgroups | dev.linux.lists.iommu,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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!