Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Clearing the stale page fault state without clearing the associated 
interrupt status leads to a spurious IOMMU interrupt upon unmasking if the MMU 
reset is disabled.
--

--- Patch [5]: [RFC PATCH v2 5/8] iommu/rockchip: clear stale page faults 
before enabling stall ---
commit 6445c2bf287495e424096d491a8245a58e126724
Author: Jiaxing Hu <[email protected]>

iommu/rockchip: clear stale page faults before enabling stall

Boot firmware can leave an IOMMU bank in PAGE_FAULT_ACTIVE before the
driver has configured paging. This commit acknowledges these stale faults
with CMD_PAGE_FAULT_DONE before enabling stall so all banks start clean
and avoid timing out.

> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 2ac65c0c5c0b4..e477fe79ca4a3 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -417,6 +417,19 @@ static int rk_iommu_enable_stall(struct rk_iommu *iommu)
>       if (!rk_iommu_is_paging_enabled(iommu))
>               return 0;
>  
> +     /*
> +      * Boot firmware can leave a bank in PAGE_FAULT_ACTIVE with no handler
> +      * (PAGE_FAULT_ACTIVE & !STALL_ACTIVE & IDLE).  Such a bank ignores
> +      * CMD_ENABLE_STALL and never reaches STALL_ACTIVE, timing out the poll
> +      * below.  Acknowledge any stale fault first so every bank starts clean.
> +      */
> +     for (i = 0; i < iommu->num_mmu; i++) {
> +             if (rk_iommu_read(iommu->bases[i], RK_MMU_STATUS) &
> +                 RK_MMU_STATUS_PAGE_FAULT_ACTIVE)
> +                     writel(RK_MMU_CMD_PAGE_FAULT_DONE,
> +                            iommu->bases[i] + RK_MMU_COMMAND);

[Severity: High]
Does clearing the page fault state here also require clearing the associated 
interrupt status in RK_MMU_INT_CLEAR?

If a device uses rockchip,disable-mmu-reset, the hardware reset during
rk_iommu_enable() is skipped. When interrupts are subsequently unmasked
by writing to RK_MMU_INT_MASK, an uncleared raw interrupt bit would
immediately trigger a spurious interrupt.

Could this spurious interrupt lead to report_iommu_fault() being called
prematurely, potentially panicking the system or failing device attachment?

> +     }
> +
>       rk_iommu_command(iommu, RK_MMU_CMD_ENABLE_STALL);

[ ... ]

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=5

Reply via email to