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

["Alexandre Courbot" <acourbot@nvidia.com>]

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:
>> On Mon Sep 14, 2026 at 10:58 AM BST, Danilo Krummrich wrote:
>>> On Mon Sep 14, 2026 at 2:45 AM CEST, Alexandre Courbot wrote:
>>>> On Mon Sep 14, 2026 at 6:08 AM JST, Matteo Kloiber wrote:
>>>>> On Mon Sep 7, 2026 at 11:43 AM JST, Alexandre Courbot wrote:
>>>>>> 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
> [2] https://lore.kernel.org/20260830193824.471089-1-dakr@kernel.org

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.