Re: [PATCH 6/9] gpu: nova-core: generalize `flush_into_kvec` to `flush_into_vec`

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

On Tue Mar 17, 2026 at 11:12 PM JST, Danilo Krummrich wrote:
> On Tue Mar 17, 2026 at 2:41 PM CET, Alexandre Courbot wrote:
>> On Tue Mar 17, 2026 at 7:49 PM JST, Danilo Krummrich wrote:
>>> On Tue Mar 17, 2026 at 2:55 AM CET, Alexandre Courbot wrote:
>>>> We shouldn't be doing that - I think we are limited by the current
>>>> CoherentAllocation API though. But IIUC this is something that I/O
>>>> projections will allow us to handle properly?
>>>
>>> Why do we need projections to avoid UB here? driver_read_area() already even
>>> peeks into the firmware abstraction layer, which is where MsgqData technically
>>> belongs into (despite being trivial).
>>>
>>> 	let gsp_mem = &unsafe { self.0.as_slice(0, 1) }.unwrap()[0];
>>> 	let data = &gsp_mem.gspq.msgq.data;
>>>
>>> Why do we need I/O projections to do raw pointer arithmetic where creating a
>>> reference is UB?
>>>
>>> (Eventually, we want to use IoView of course, as this is a textbook example of
>>> what I proposed IoSlice for.)
>>
>> Limiting the amount of `unsafe`s, but I guess we can live with that as
>> this is going to be short-term anyway.
>
> Of course it is going to be better with IoSlice, but limiting the number of
> unsafe calls regardless is a bit pointless if the "safe" ones can cause
> undefined behavior. :)

No arguing. :)

>
>>> Another option in the meantime would be / have been to use dma_read!() and
>>> extract (copy) the data right away in driver_read_area(), which I'd probably
>>> prefer over raw pointer arithmetic.
>>
>> I'd personally like to keep the current "no-copy" approach as it
>> implements the right reference discipline (i.e. you need a mutable
>> reference to update the read pointer, which cannot be done if the buffer
>> is read by the driver) and moving to copy semantics would open a window
>> of opportunity to mess with that balance further (on top of requiring
>> bigger code changes that will be temporary).
>
> I don't even know if we want them to be temporary, i.e. we can copy right away
> and IoSlice would still be an improvement in order to make the copy in the first
> place.
>
> Also, you say "no-copy", but that's not true, we do copy eventually. In fact,
> the whole point of this patch is to copy this buffer into a KVVec.
>
> So, why not copy it right away with dma_read!() (later replaced with an IoSlice
> copy) and then process it further?

So as you said we are already making one copy: `receive_msg` for
instance does not return any reference to the command buffer, but an
object created from the slices returned by `driver_read_area`. What you
are suggesting would introduce another copy in the command queue
internals, for no tangible benefit as it would not allow us to
advance the read pointer sooner anyway.

Because even if we did that extra copy using `dma_read`, we would still
need to make sure the read pointer doesn't move as we are doing it, lest
we return corrupted messages - so the situation would be more or less
the same, and the `&mut self` required to advance the read pointer would
still be needed to shield us from doing it inadvertently.

In addition, we need space to hold that additional copy. It doesn't have
a fixed size, and can cover several memory pages. Does that mean we
would need to allocate a KVec or KVVec for every message received, and
drop it even before `receive_msg` returns?

The additional code to do that allocation would likely be larger than
the fix I proposed below, and we know it is very temporary and will be
replaced by mostly safe code soon. I would understand the trade-off of
an extra copy if this resulted in simpler code, but I don't think that
would be the case here.

>
> I am also very sceptical of the "holding on to the reference prevents the read
> pointer update" argument. Once we have a copy, there is no need not to update
> the read pointer anymore in the first place, no?

Correct, and that's exactly what `receive_msg` does - as soon as it has
created the object to return from the slices, it advances the read
pointer. So the read pointer is already updated when `receive_msg`
returns, and that "holding on to the reference" thing is purely for
internal safety.

Introducing another copy won't allow us to update it sooner - copying
the bytes into a KVec is not faster than processing the slices' data
into the final returned value. Actually it may even be slower if that
involves a dynamic allocation.