Re: [PATCH v12 2/3] rust: add dma coherent allocator abstraction.

[Danilo Krummrich <dakr@kernel.org>]

On Fri, Mar 07, 2025 at 08:48:09AM -0400, Jason Gunthorpe wrote:
> On Fri, Mar 07, 2025 at 09:50:07AM +0100, Danilo Krummrich wrote:
> > > The actual critical region extends into the HW itself, it is not
> > > simple to model this with a pure SW construct of bracketing some
> > > allocation. You need to bracket the *entire lifecycle* of the
> > > dma_addr_t that has been returned and passed into HW, until the
> > > dma_addr_t is removed from HW.
> > 
> > Devres callbacks run after remove(). It's the drivers job to stop operating the
> > device latest in remove(). Which means that the design is correct.
> 
> It could be the drivers job to unmap the dma as well if you take that
> logic.

I really don't understand what you want: *You* brought up that the
CoherentAllocation is not allowed to out-live driver unbind.

We agreed and provided a way that solves this. But then you point out the
unsolvable problem of malicious (or wrongly programmed) hardware and use it to
question why we then even bother solving the problem you just pointed out, that
is solvable.

So, what do you ask for?

> You still didn't answer the question, what is the critical region of
> the DevRes for a dma_alloc_coherent() actually going to protect?

Devres, just like in C, ensures that an object can't out-live driver unbind. The
RCU read side critical section is to revoke access to the then invalid pointer
of the object.

C leaves you with an invalid pointer, whereas Rust revokes the access to the
invalid pointer for safety reasons. The pointer is never written to, except for
on driver unbind, hence RCU.

We discussed all this in other threads already.

> 
> You also have to urgently fix the synchronize_rcu() repitition of you
> plan to do this.

I mentioned this a few days ago, and I did not forget it. :-)

> 
> > Now, you ask for a step further, i.e. make it that we can enforce that a driver
> > actually stopped the device in remove().
> 
> So where do you draw the line on bugs Rust should prevent and bugs
> Rust requires the programmer to fix?

It should prevent all safety related bug, but the one above is impossible to
solve, so we have to live with it. But that doesn't mean it's a justification to
stop preventing bugs we can actually prevent? Do you disagree?