Re: [PATCH v3 00/17] drm/panthor: Fix the unplug logic

On Thu, 13 Aug 2026 20:50:22 +0200
"Danilo Krummrich" <[email protected]> wrote:

> On Thu Aug 13, 2026 at 1:23 PM CEST, Boris Brezillon wrote:
> > Just a note I forgot to add to my cover letter. I've already spent way
> > more time than I wanted on this, not just because Sashiko keeps finding
> > new issues at each of my attempt, but also because the whole idea of
> > pretending a device on a platform bus is unplugged and can't harm us is
> > doomed. This is not an hot-pluggable bus, and the device is still there,
> > so, unless we can be absolutely sure it's inactive (which a RESET can
> > provide, but RESETs are fallible) we just have two options:  
> 
> I probably need a bit more context about which exact problem(s) you are trying
> to solve.

Sorry. You can find more context in patch 15.

> 
> > 1. prevent the device from going away until we managed to properly
> >    shutdown the GPU  
> 
> I'm not exactly sure what you mean with "device going away". If you mean
> "prevent the device from being unbound from the driver" this is essentially what
> you do by waiting for the completion of some HW teardown operation in remove().
> 
> In general, the implementation of remove() should ensure that on the one hand
> the device it torn down (or reset), so it does not mess with system resources
> anymore (e.g. attempt to do any DMA transfers) and behaves correctly on a
> subsequent probe of this or another driver.

So, that's the ideal situation, were a SOFT_RESET works. But because
SOFT_RESET is a GPU command that has to be acknowledged by the GPU, you
have no guarantee that this reset actually worked.

> 
> And on the other hand, the driver must release all device assoicated resources,
> such as DMA mappings, IRQs, I/O memory mappings, etc. and it should also ensure
> that no more driver code is reachable from any asynchronous paths, such as
> workqueues, IOCTLs, timers, etc.

Yep, we also take care of that in the nominal case (AKA RESET worked,
and we know the HW is off).

> 
> The latter obviously also depends on the subsystem and whether the lifetime of
> userspace structurs and their associated driver private data is cleanly
> decoupled (e.g. struct drm_file and ->driver_priv).
> 
> Since you also mention hot-unplug; those rules are universial regardless of
> whether remove is triggered by a hot-unplug event or because the driver is
> unbound for a different reason.

The API doesn't change, but the implication of such a removal do
change: on an hot-pluggable bus, the device is physically gone, so it
can't do any harm. On a platform bus, the device is there, and it might
remain clocked and powered even after the platform_device has been
unbound, because power domains and clks can be shared across devices,
so even the clk_disable_unprepare() & co we have in the remove path
won't guarantee that the GPU is inactive.

> The DRM API is a bit misleading about this,
> because with drm_dev_unregister() there is no way to prevent DRM IOCTLs from
> running after remove(), which wrongly suggests that this is not a potential
> issue.
> 
> Not summarizing this because I think you are not aware already, but it may
> provide a good entry point for you to point out where exactly things are getting
> tricky.

Things get tricky when we diverge from the nominal case: SOFT_RESET
didn't work, and we're either stuck in an infinite RESET loop waiting
for it to eventually work, or we just take the hit and leak any
resource the HW had access to at the time the RESET command was issued,
because we can't know for sure that the GPU is in such a bad state it
can't access memory anymore. All we know is that it's in a bad enough
state to no longer acknowledge RESET requests.

> 
> > 2. make sure all resources the HW might have its hands on at the time
> >    the failure of RESET in the unplug path happened are leaked  
> 
> I'm not sure what you mean by this. But it suggests that the problem you try to
> deal with is a misbehaving device that fails to reset?

Yes, this.

> 
> Also, what do you mean with leaking the device resources?

I mean leaking all the memory that the GPU had access to (page tables
and memory pointed by those page tables), so that it's never returned
to the system with a risk of UAF.