[PATCH 0/2] rust: honor the maximum DMA segment size

[PATCH 0/2] rust: honor the maximum DMA segment size

[Matteo Kloiber]

Booting nova-core against a real GSP with CONFIG_DMA_API_DEBUG=y warns
when the ~60 MiB firmware image is mapped as a scatter-gather table:

  DMA-API: nova-core 0000:00:03.0: mapping sg segment longer than
  device claims to support [len=62914560] [max=65536]

There are two separate issues behind it.

Patch 1 has nova-core declare the segment size it actually supports. It
re-decomposes every segment into 4 KiB GSP page-table entries, so it has
no upper bound on segment length.

Furthermore, I also checked what other GPUs do, and most do the same:
set the max segment size u32::MAX. So this should be the correct
behavior for nova as well.

Patch 2 has SGTable::new() honor dma_get_max_seg_size() in addition to
dma_max_mapping_size(). The abstraction is generic, so a driver with a
real hardware segment limit would otherwise silently be handed segments
it cannot express in a single descriptor.

Tested on an RTX 5080 (GB203) passed through to a QEMU guest via VFIO
with CONFIG_DMA_API_DEBUG=y: the warning is gone, and GSP boot and RPC
still work.

Discussed on Zulip:
https://rust-for-linux.zulipchat.com/#narrow/channel/509436-Nova/topic/nova.20core.3A.20DMA-API.3A.20nova-core.200000.3A00.3A03.2E0.3A.20mapping.20sg.20segme/with/618319830

Matteo Kloiber (2):
  gpu: nova-core: declare unlimited DMA max segment size
  rust: scatterlist: honor the device's maximum segment size

 drivers/gpu/nova-core/gpu.rs |  7 +++++++
 rust/helpers/dma.c           |  5 +++++
 rust/kernel/scatterlist.rs   | 12 ++++++++++--
 3 files changed, 22 insertions(+), 2 deletions(-)

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.51.2

[PATCH 1/2] gpu: nova-core: declare unlimited DMA max segment size

[Matteo Kloiber]

The PCI default max_seg_size is 64 KiB. nova-core never raises it, so
mapping the GSP firmware image (~60 MiB) as a scatter-gather table
triggers a DMA-API debug warning when contiguous pages coalesce into
segments that exceed the default:

  DMA-API: nova-core 0000:00:03.0: mapping sg segment longer than
  device claims to support [len=62914560] [max=65536]

CONFIG_DMA_API_DEBUG=y is required to see this warning.

Signed-off-by: Matteo Kloiber <kernel@matt3o12.de>
Assisted-by: Claude:claude-opus-4-8
---
 drivers/gpu/nova-core/gpu.rs | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index fd1414004dd0..233ef2dc9688 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -343,6 +343,13 @@ pub(crate) fn new<'a>(
                 // still constructing it, so no concurrent DMA allocations can exist.
                 unsafe { pdev.dma_set_mask_and_coherent(dma_mask)? };
 
+                // Nova re-decomposes SG segments into 4 KiB page-table entries, so it
+                // has no upper bound on segment length; declare that to the DMA layer.
+                //
+                // SAFETY: same invariant as above -- still constructing, no concurrent
+                // DMA mapping can exist.
+                unsafe { pdev.dma_set_max_seg_size(u32::MAX) };
+
                 hal.wait_gfw_boot_completion(bar)
                     .inspect_err(|_| dev_err!(dev, "GFW boot did not complete\n"))?;
             },
-- 
2.51.2

Re: [PATCH 1/2] gpu: nova-core: declare unlimited DMA max segment size

[Alexandre Courbot]

On Tue Sep 1, 2026 at 8:32 AM JST, Matteo Kloiber wrote:
> The PCI default max_seg_size is 64 KiB. nova-core never raises it, so
> mapping the GSP firmware image (~60 MiB) as a scatter-gather table
> triggers a DMA-API debug warning when contiguous pages coalesce into
> segments that exceed the default:
>
>   DMA-API: nova-core 0000:00:03.0: mapping sg segment longer than
>   device claims to support [len=62914560] [max=65536]
>
> CONFIG_DMA_API_DEBUG=y is required to see this warning.
>
> Signed-off-by: Matteo Kloiber <kernel@matt3o12.de>
> Assisted-by: Claude:claude-opus-4-8

Note: new kernel policy [1] asks that the specific model is not named, so
this should just be `Assisted-by: LLM`.

(also should be placed before your `Signed-off-by`)

[1] https://docs.kernel.org/process/coding-assistants.html

> ---
>  drivers/gpu/nova-core/gpu.rs | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index fd1414004dd0..233ef2dc9688 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -343,6 +343,13 @@ pub(crate) fn new<'a>(
>                  // still constructing it, so no concurrent DMA allocations can exist.
>                  unsafe { pdev.dma_set_mask_and_coherent(dma_mask)? };
>  
> +                // Nova re-decomposes SG segments into 4 KiB page-table entries, so it
> +                // has no upper bound on segment length; declare that to the DMA layer.
> +                //
> +                // SAFETY: same invariant as above -- still constructing, no concurrent
> +                // DMA mapping can exist.
> +                unsafe { pdev.dma_set_max_seg_size(u32::MAX) };

Thanks, this patch looks correct and I would like to merge it early, but
one thing about the comment: it carries way more context than needed and
reads heavily, as is often the case when AI-generated. For instance,
"declare that to the DMA layer" is obvious from the method we are
calling. Make sure to give a human pass to such comments as they can
make the code tedious when they accumulate.

Conversely, `as above` is risky because the above in question might
change and we then lose the reference, so here it's actually better to
state the invariant in a self-contained way. Copy-pasting is ok. For
instance:

  // Nova walks SG segments to build page tables, so their length is
  // irrelevant to the device.
  //
  // SAFETY: `Gpu` owns all DMA allocations for this device, and we are
  // still constructing it, so no concurrent DMA allocations can exist.

Since this seems to be your first patch, this is a good opportunity to
practice sending a v2. :)

Re: [PATCH 1/2] gpu: nova-core: declare unlimited DMA max segment size

[Matteo Kloiber]

On Mon Sep 7, 2026 at 11:10 AM JST, Alexandre Courbot wrote:
> Note: new kernel policy [1] asks that the specific model is not named, so
> this should just be `Assisted-by: LLM`.

Got it, thanks! I was checking what other people did and noticed a lot of
mentions of Claude, etc., so I went with it. Changed for v2.

> Thanks, this patch looks correct and I would like to merge it early, but
> one thing about the comment: it carries way more context than needed and
> reads heavily, as is often the case when AI-generated. For instance,
> "declare that to the DMA layer" is obvious from the method we are
> calling. Make sure to give a human pass to such comments as they can
> make the code tedious when they accumulate.

Thanks! I agree. I was unsure about that and decided to leave a bit more
in for the review. I'll just use your comment verbatim.

> Since this seems to be your first patch, this is a good opportunity to
> practice sending a v2. :)

Yes, submitting patches to the mailing list was certainly a new experience.
Thank you for your help and time so far!

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

[Matteo Kloiber]

SGTable::new() caps segment length at dma_max_mapping_size() only, which
limits the DMA mapping path (e.g. swiotlb), not the device itself. The
per-device limit from dma_set_max_seg_size() is ignored, so contiguous
page segments can be longer than the declared max segment size,
potentially causing problems for future drivers that use this
abstraction.

nova-core declares an unlimited segment size, so this does not change
its behavior.

Fixes: 05aa6fb1c21d ("rust: scatterlist: Add abstraction for sg_table")
Signed-off-by: Matteo Kloiber <kernel@matt3o12.de>
---
 rust/helpers/dma.c         |  5 +++++
 rust/kernel/scatterlist.rs | 12 ++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/rust/helpers/dma.c b/rust/helpers/dma.c
index 9fbeb507b08c..ff8f24dae9df 100644
--- a/rust/helpers/dma.c
+++ b/rust/helpers/dma.c
@@ -49,3 +49,8 @@ __rust_helper void rust_helper_dma_set_max_seg_size(struct device *dev,
 {
 	dma_set_max_seg_size(dev, size);
 }
+
+__rust_helper unsigned int rust_helper_dma_get_max_seg_size(struct device *dev)
+{
+	return dma_get_max_seg_size(dev);
+}
diff --git a/rust/kernel/scatterlist.rs b/rust/kernel/scatterlist.rs
index b83c468b5c63..d677dcbe7aac 100644
--- a/rust/kernel/scatterlist.rs
+++ b/rust/kernel/scatterlist.rs
@@ -350,15 +350,23 @@ fn new(
             page_vec.push(page.as_ptr(), flags)?;
         }
 
+        // Cap segments at both the DMA mapping-path limit and the device's declared
+        // max segment size.
+        //
         // `dma_max_mapping_size` returns `size_t`, but `sg_alloc_table_from_pages_segment()` takes
         // an `unsigned int`.
         //
         // SAFETY: `dev.as_raw()` is a valid pointer to a `struct device`.
-        let max_segment = match unsafe { bindings::dma_max_mapping_size(dev.as_raw()) } {
+        let max_mapping = match unsafe { bindings::dma_max_mapping_size(dev.as_raw()) } {
             0 => u32::MAX,
-            max_segment => u32::try_from(max_segment).unwrap_or(u32::MAX),
+            max_mapping => u32::try_from(max_mapping).unwrap_or(u32::MAX),
         };
 
+        // SAFETY: `dev.as_raw()` is a valid pointer to a `struct device`.
+        let max_seg_size = unsafe { bindings::dma_get_max_seg_size(dev.as_raw()) };
+
+        let max_segment = max_mapping.min(max_seg_size);
+
         Ok(try_pin_init!(&this in Self {
             // SAFETY:
             // - `page_vec` is a `KVec` of valid `struct page *` obtained from `pages`.
-- 
2.51.2

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

[Alexandre Courbot]

On Tue Sep 1, 2026 at 8:32 AM JST, Matteo Kloiber wrote:
> SGTable::new() caps segment length at dma_max_mapping_size() only, which
> limits the DMA mapping path (e.g. swiotlb), not the device itself. The
> per-device limit from dma_set_max_seg_size() is ignored, so contiguous
> page segments can be longer than the declared max segment size,
> potentially causing problems for future drivers that use this
> abstraction.
>
> nova-core declares an unlimited segment size, so this does not change
> its behavior.

I guess what this last paragraph wants to state is that no user is
affected by this patch? There is another subtle user though: the Rust
DMA sample.

In any case, this patch without patch 1 wouldn't break either of those
(only waste a bit more memory in SG entries with nova-core), so maybe we
can skip it.

>
> Fixes: 05aa6fb1c21d ("rust: scatterlist: Add abstraction for sg_table")
> Signed-off-by: Matteo Kloiber <kernel@matt3o12.de>
> ---
>  rust/helpers/dma.c         |  5 +++++
>  rust/kernel/scatterlist.rs | 12 ++++++++++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/rust/helpers/dma.c b/rust/helpers/dma.c
> index 9fbeb507b08c..ff8f24dae9df 100644
> --- a/rust/helpers/dma.c
> +++ b/rust/helpers/dma.c
> @@ -49,3 +49,8 @@ __rust_helper void rust_helper_dma_set_max_seg_size(struct device *dev,
>  {
>  	dma_set_max_seg_size(dev, size);
>  }
> +
> +__rust_helper unsigned int rust_helper_dma_get_max_seg_size(struct device *dev)
> +{
> +	return dma_get_max_seg_size(dev);
> +}
> diff --git a/rust/kernel/scatterlist.rs b/rust/kernel/scatterlist.rs
> index b83c468b5c63..d677dcbe7aac 100644
> --- a/rust/kernel/scatterlist.rs
> +++ b/rust/kernel/scatterlist.rs
> @@ -350,15 +350,23 @@ fn new(
>              page_vec.push(page.as_ptr(), flags)?;
>          }
>  
> +        // Cap segments at both the DMA mapping-path limit and the device's declared
> +        // max segment size.
> +        //
>          // `dma_max_mapping_size` returns `size_t`, but `sg_alloc_table_from_pages_segment()` takes
>          // an `unsigned int`.
>          //
>          // SAFETY: `dev.as_raw()` is a valid pointer to a `struct device`.
> -        let max_segment = match unsafe { bindings::dma_max_mapping_size(dev.as_raw()) } {
> +        let max_mapping = match unsafe { bindings::dma_max_mapping_size(dev.as_raw()) } {
>              0 => u32::MAX,
> -            max_segment => u32::try_from(max_segment).unwrap_or(u32::MAX),
> +            max_mapping => u32::try_from(max_mapping).unwrap_or(u32::MAX),

nit: if we rename this variable, let's name it `max_mapping_size` for
consistency.

>          };
>  
> +        // SAFETY: `dev.as_raw()` is a valid pointer to a `struct device`.
> +        let max_seg_size = unsafe { bindings::dma_get_max_seg_size(dev.as_raw()) };
> +
> +        let max_segment = max_mapping.min(max_seg_size);

This looks like the correct thing to do indeed.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

(the following is a related thought, not a request for this patch)

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.

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

[Matteo Kloiber]

On Mon Sep 7, 2026 at 11:43 AM JST, Alexandre Courbot wrote:
> I guess what this last paragraph wants to state is that no user is
> affected by this patch? There is another subtle user though: the Rust
> DMA sample.

Yes, that's what I meant. I missed the sample, but it wouldn't make a
difference anyway. So I just removed the paragraph in v2.

> nit: if we rename this variable, let's name it `max_mapping_size` for
> consistency.

Agreed. Fixed. Let me know if everything is fixed in v2.

> 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?

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

[Alexandre Courbot]

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.

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

[Danilo Krummrich]

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.

Now, we can't really have drivers define them statically (e.g. in the driver
trait) as there may be cases where it depends on the runtime state or properties
of the device queried at runtime. Sometimes it is also defined through OF
properties (which from a kernel perspective are runtime values too).

For the same reason it is also pretty hard to invent a type state pattern for
those setters that is not getting ridiculously complex without much value, which
is why we just kept them unsafe for the time being.

The best option to get rid of the unsafe would probably be to use atomics
instead. It would however be a rather big change, what makes it a bit of a hard
sell, given that the reason of this unsafe is more on the theoretical side of
things.

Theoretically, we could also optimize the situation for when it is statically
known, e.g. some dma::Config trait that can be implemented, such that the bus
can set the before calling probe(). But we'd really want this to work per device
ID table entry, as it may differ between supported devices. But that might not
be quite straight forward without associated_type_defaults. The simplest thing I
could think of is some callback, such as

	fn dma_info(id_info: Option<&Self::IdInfo>) -> DmaInfo

which is called before probe(), but that's not great either. Maybe there is a
good solution for this, but I'd first want to exhaust getting the setters safe.

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

[Gary Guo]

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.

Best,
Gary

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

[Robin Murphy]

On 14/09/2026 11:22 am, 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?

Similar to DMA masks, the segment parameters should be set appropriately 
before any dma_map_sg() operation. Plus since they likely influence 
scatterlist geometry, that means typically they're also going to need to 
be set before building the scatterlist to be mapped in the first place.

I don't think there's any expectation that they would ever change 
*between* different mappings - especially given the underlying hardware 
properties they represent - so while that might technically be possible 
in the C API if the caller can enforce sufficient ordering, it should be 
fine to rule it out in the Rust abstraction if that makes things easier 
to reason about.

Thanks,
Robin.

> 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.
> 
> Best,
> Gary
> 

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

[Gary Guo]

On Mon Sep 14, 2026 at 1:17 PM BST, Robin Murphy wrote:
> On 14/09/2026 11:22 am, 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?
>
> Similar to DMA masks, the segment parameters should be set appropriately 
> before any dma_map_sg() operation. Plus since they likely influence 
> scatterlist geometry, that means typically they're also going to need to 
> be set before building the scatterlist to be mapped in the first place.

Right, in that case I think we have the following options:

1.  Have setters these being unsafe, with precondition that they must be done
    before creation of DMA mappings.

2.  Have a `dma_info()` method like Danilo mentioned. However, we cannot expose
    `&Device<Core<'_>>` inside such callbacks, because otherwise driver would be
    able create DMA mappings. This however is too restrictive to be the only way
    of setting DMA masks, because Nova needs to access the bar before setting the
    DMA masks.

3.  Use wrapper type instead of generics for typestate. Then, we can express the
    pattern of typestate transformation (generics cannot do this because we
    always place it behind a reference).

    For example, we can have `Device` to mean `Device<Normal>`, and then have
    `Bound<Device>` to mean `Device<Bound>` (both of which are behind a shared
    reference, like today, and `Bound<D>` can deref to `&D`).

    Then, for `Core` typestate, we can instead have

        struct Core<'a, D>(&'a Bound<Device>);

    where it's passed by value. We can then define the DMA setters methods to
    operate on `Core` like we did today, but safely. Instead of providing a
    `Deref` impl that turns `&Core<'_, D>` to `&Bound<D>`, we only implement
    forwarding functions for APIs that can work without DMA configured (e.g. PCI
    bar).

    Then, provide a `into_bound()` method which *consumes* `Core<'a, D>` and
    give out `&'a Bound<Device>` (this is why typestate generics cannot work,
    because `&'a Device<Core<'_>>` is Copy and we cannot represent the ownership
    consumption).

>
> I don't think there's any expectation that they would ever change 
> *between* different mappings - especially given the underlying hardware 
> properties they represent - so while that might technically be possible 
> in the C API if the caller can enforce sufficient ordering, it should be 
> fine to rule it out in the Rust abstraction if that makes things easier 
> to reason about.
>
> Thanks,
> Robin.
>
>> If it's the former, we can require these to require mutable reference instead,

Correction: this should say "latter".

Best,
Gary

>> 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.
>> 
>> Best,
>> Gary
>> 

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

[Danilo Krummrich]

On Mon Sep 14, 2026 at 3:17 PM CEST, Gary Guo wrote:
> On Mon Sep 14, 2026 at 1:17 PM BST, Robin Murphy wrote:
>> On 14/09/2026 11:22 am, 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?
>>
>> Similar to DMA masks, the segment parameters should be set appropriately 
>> before any dma_map_sg() operation. Plus since they likely influence 
>> scatterlist geometry, that means typically they're also going to need to 
>> be set before building the scatterlist to be mapped in the first place.
>
> Right, in that case I think we have the following options:
>
> 1.  Have setters these being unsafe, with precondition that they must be done
>     before creation of DMA mappings.

As mentioned, I don't think the safety requirement has to be before using other
DMA APIs, but just requiring to not do it concurrently. Not doing it would be a
semantic bug, but it shouldn't have any safety implications.

> 2.  Have a `dma_info()` method like Danilo mentioned. However, we cannot expose
>     `&Device<Core<'_>>` inside such callbacks, because otherwise driver would be
>     able create DMA mappings. This however is too restrictive to be the only way
>     of setting DMA masks, because Nova needs to access the bar before setting the
>     DMA masks.

Just to clarify, I mentioned this to be something we could do in addition to (1)
for drivers where those values are statically known.

> 3.  Use wrapper type instead of generics for typestate. Then, we can express the
>     pattern of typestate transformation (generics cannot do this because we
>     always place it behind a reference).
>
>     For example, we can have `Device` to mean `Device<Normal>`, and then have
>     `Bound<Device>` to mean `Device<Bound>` (both of which are behind a shared
>     reference, like today, and `Bound<D>` can deref to `&D`).
>
>     Then, for `Core` typestate, we can instead have
>
>         struct Core<'a, D>(&'a Bound<Device>);
>
>     where it's passed by value. We can then define the DMA setters methods to
>     operate on `Core` like we did today, but safely. Instead of providing a
>     `Deref` impl that turns `&Core<'_, D>` to `&Bound<D>`, we only implement
>     forwarding functions for APIs that can work without DMA configured (e.g. PCI
>     bar).
>
>     Then, provide a `into_bound()` method which *consumes* `Core<'a, D>` and
>     give out `&'a Bound<Device>` (this is why typestate generics cannot work,
>     because `&'a Device<Core<'_>>` is Copy and we cannot represent the ownership
>     consumption).

Yeah, we had this discussion in the past with similar ideas; it is the kind of
complication that we concluded not to be worth it and that I mentioned in my
initial reply.

The only safety relevant issue should really be the data race of the primitives
values being set, which is more on the theoretical side of things.

>> I don't think there's any expectation that they would ever change 
>> *between* different mappings - especially given the underlying hardware 
>> properties they represent - so while that might technically be possible 
>> in the C API if the caller can enforce sufficient ordering, it should be 
>> fine to rule it out in the Rust abstraction if that makes things easier 
>> to reason about.
>>
>>> If it's the former, we can require these to require mutable reference instead,
>
> Correction: this should say "latter".
>
>>> 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.

I think that doesn't work, there are drivers that need to perform MMIO before
knowing e.g. the DMA mask, i.e. we can't have Pin<&'bound mut Device<Core<'_>>>
at the same time as e.g. pci::Bar<'_>, which borrows a &Device<Bound>.