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/[email protected] > [2] https://lore.kernel.org/[email protected]
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.
