Thread (16 messages) flat view 16 messages, 5 authors, 8d ago

Re: [PATCH 2/2] rust: scatterlist: honor the device's maximum segment size

From: "Alexandre Courbot" <acourbot@nvidia.com>
Date: 2026-09-15 04:52:43
Also in: dri-devel, driver-core, lkml, nova-gpu

On Tue Sep 15, 2026 at 1:44 PM JST, Alexandre Courbot wrote:
On Mon Sep 14, 2026 at 7:22 PM JST, Gary Guo wrote:
quoted
On Mon Sep 14, 2026 at 10:58 AM BST, Danilo Krummrich wrote:
quoted
On Mon Sep 14, 2026 at 2:45 AM CEST, Alexandre Courbot wrote:
quoted
On Mon Sep 14, 2026 at 6:08 AM JST, Matteo Kloiber wrote:
quoted
On Mon Sep 7, 2026 at 11:43 AM JST, Alexandre Courbot wrote:
quoted
It also means that without patch 1, nova-core would split the firmware
into hundreds of 64KB SG entries, which is not breaking but still
something we want to avoid. The correct fix is to make sure that
`dma_set_max_seg_size` is called by the driver, and while we are at it
we also want every driver to call `dma_set_mask_and_coherent`. Ideally
we would use the type system to make sure that both functions are called
before any DMA operation can take place (using a safe interface), but
I'm not quite sure yet how we can do this.
This sounds sensible indeed. Should I open a thread regarding that on Zulip?
Probably not necessary, the mailing-list has a larger audience and is
the right place for this. I expect people will jump in here with their
thoughts.
The problem with those is not that they must strictly be called before
allocating DMA memory, but they must not be called concurrently with other DMA
operations, such as allocating DMA memory, as it would technically be a data
race.
Do they really have to be called *before* allocating DMA memory, not do they
just need not be called *concurrent* to DMA memory allocation?

If it's the former, we can require these to require mutable reference instead,
so the probe takes `Pin<&'bound mut Device<Core<'_>>>` which still derefs to
`&'bound Device<Bound>`, but Rust will require the shared reference to not
co-exist with the mutable reference.
As Danilo pointed out [1] from a strict safety perspective they must not
be called concurrently to DMA allocations. So there is not a hard need
to call them before.

But my point is different: until patch 1 of this series, nova-core
didn't do anything particularly wrong but was operating with an
inaccurate DMA segment size. Which is currently inconsequential as long
as you don't set `CONFIG_DMA_API_DEBUG`, but will become sub-optimal if
patch 2 gets merged (which it should be, otherwise more DMA-limited
devices could start misbehaving).

The thing is that every driver *really* should set their DMA mask and
max segment size ASAP, but at the moment this is done through two unsafe
functions that are very easy to overlook.

And practically speaking, is there a reason *not* to do that setup
before allocating objects?

So I am thinking that maybe we could have a kind of `DeviceDma` type
which construction involves setting the core DMA parameters (or
acknowledging the defaults), and to which all allocations would be tied.
IOW, Danilo's DMA allocations rework [2] would tie to that `DeviceDma`
object (which itself would be tied to the bound device) instead of the
device itself.

[1] https://lore.kernel.org/DLEY8AHWZFCQ.QUHP2NHJW1QX@kernel.org (local)
[2] https://lore.kernel.org/20260830193824.471089-1-dakr@kernel.org (local)
Sorry, I typed this message while travelling (and offline) and sent it
before noticing there has been new replies that make most of it
irrelevant.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help