On Thu, Sep 24, 2026 at 02:30:44PM +0200, Boris Brezillon wrote:
> We're trying to be smart by skipping the UPDATE(UNMAPPED) step, but
> it remains to be proven it adds any noticeable overhead. Moreover, it's
> breaking the assumption that, once the VM is unbound, no access can
> happen on it.
> 
> For instance, say the panthor_mmu_as_enable() call in panthor_vm_active()
> fails after we've called panthor_vm_release_as_locked(), we're now in a
> state where the HW still points to the old page table, but SW slot points
> to the new VM, which is not truly HW-bound.
> 
> If, after the as_slots lock is released, the evicted VM itself is
> released, the HW might have access to memory that has been returned
> to the system until the reset we scheduled (because of the faulty
> AS_COMMAND) is effective.
> 
> Let's make this bullet-proof by doing a full bound -> unbound -> bound
> cycle on AS slot recycling.
> 
> Fixes: 32e593d74c39 ("drm/panthor: Make sure caches are flushed/invalidated 
> when an AS is recycled")
> Signed-off-by: Boris Brezillon <[email protected]>

Reviewed-by: Liviu Dudau <[email protected]>

Best regards,
Liviu

> ---
>  drivers/gpu/drm/panthor/panthor_mmu.c | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c 
> b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 7e98084b9a1e..038a092fd08c 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -620,8 +620,7 @@ static int panthor_mmu_as_enable(struct panthor_device 
> *ptdev, u32 as_nr,
>       return as_send_cmd_and_wait(ptdev, as_nr, AS_COMMAND_UPDATE);
>  }
>  
> -static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 as_nr,
> -                               bool recycle_slot)
> +static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 as_nr)
>  {
>       struct panthor_mmu *mmu = ptdev->mmu;
>       struct panthor_vm *vm = ptdev->mmu->as.slots[as_nr].vm;
> @@ -645,12 +644,6 @@ static int panthor_mmu_as_disable(struct panthor_device 
> *ptdev, u32 as_nr,
>                       return ret;
>       }
>  
> -     /* If the slot is going to be used immediately, don't bother changing
> -      * the config.
> -      */
> -     if (recycle_slot)
> -             return 0;
> -
>       gpu_write64(mmu->iomem, AS_TRANSTAB(as_nr), 0);
>       gpu_write64(mmu->iomem, AS_MEMATTR(as_nr), 0);
>       gpu_write64(mmu->iomem, AS_TRANSCFG(as_nr), 
> AS_TRANSCFG_ADRMODE_UNMAPPED);
> @@ -777,7 +770,7 @@ int panthor_vm_active(struct panthor_vm *vm)
>               drm_WARN_ON(&ptdev->base, 
> refcount_read(&lru_vm->as.active_cnt));
>               as = lru_vm->as.id;
>  
> -             ret = panthor_mmu_as_disable(ptdev, as, true);
> +             ret = panthor_mmu_as_disable(ptdev, as);
>               if (ret)
>                       goto out_unlock;
>  
> @@ -919,7 +912,7 @@ static void panthor_vm_declare_unusable(struct panthor_vm 
> *vm)
>       vm->unusable = true;
>       mutex_lock(&ptdev->mmu->as.slots_lock);
>       if (vm->as.id >= 0 && drm_dev_enter(&ptdev->base, &cookie)) {
> -             panthor_mmu_as_disable(ptdev, vm->as.id, false);
> +             panthor_mmu_as_disable(ptdev, vm->as.id);
>               drm_dev_exit(cookie);
>       }
>       mutex_unlock(&ptdev->mmu->as.slots_lock);
> @@ -1911,7 +1904,7 @@ static void panthor_mmu_irq_handler(struct 
> panthor_device *ptdev, u32 status)
>                       ptdev->mmu->as.slots[as].vm->unhandled_fault = true;
>  
>               /* Disable the MMU to kill jobs on this AS. */
> -             panthor_mmu_as_disable(ptdev, as, false);
> +             panthor_mmu_as_disable(ptdev, as);
>               mutex_unlock(&ptdev->mmu->as.slots_lock);
>  
>               status &= ~mask;
> @@ -1940,7 +1933,7 @@ void panthor_mmu_suspend(struct panthor_device *ptdev)
>  
>               if (vm) {
>                       drm_WARN_ON(&ptdev->base,
> -                                 panthor_mmu_as_disable(ptdev, i, false));
> +                                 panthor_mmu_as_disable(ptdev, i));
>                       panthor_vm_release_as_locked(vm);
>               }
>       }
> @@ -2065,7 +2058,7 @@ static void panthor_vm_free(struct drm_gpuvm *gpuvm)
>               int cookie;
>  
>               if (drm_dev_enter(&ptdev->base, &cookie)) {
> -                     panthor_mmu_as_disable(ptdev, vm->as.id, false);
> +                     panthor_mmu_as_disable(ptdev, vm->as.id);
>                       drm_dev_exit(cookie);
>               }
>  
> @@ -3360,7 +3353,7 @@ void panthor_mmu_unplug(struct panthor_device *ptdev)
>  
>               if (vm) {
>                       drm_WARN_ON(&ptdev->base,
> -                                 panthor_mmu_as_disable(ptdev, i, false));
> +                                 panthor_mmu_as_disable(ptdev, i));
>                       panthor_vm_release_as_locked(vm);
>               }
>       }
> 
> -- 
> 2.55.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Reply via email to