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

[Danilo Krummrich]

(Cc: Gary)

On Mon Mar 16, 2026 at 12:44 PM CET, Eliot Courtney wrote:
> On Tue Mar 10, 2026 at 7:01 AM JST, Danilo Krummrich wrote:
>> On Mon Mar 9, 2026 at 10:57 PM CET, Danilo Krummrich wrote:
>>> On 2/27/2026 1:32 PM, Eliot Courtney wrote:
>>>> Add general `flush_into_vec` function. Add `flush_into_kvvec`
>>>> convenience wrapper alongside the existing `flush_into_kvec` function.
>>>> This is generally useful but immediately used for e.g. holding RM
>>>> control payloads, which can be large (~>=20 KiB).
>>>
>>> Why not just always use KVVec? It also seems that the KVec variant is not used?
>>
>> (Besides its single usage in GspSequence, which wouldn't hurt to be a KVVec.)
>>
>>> If there's no reason for having both, I'd also just call this into_vec().
>
> I think always using KVVec should be fine, thanks!
>
> For the naming, I think `read_to_vec` may be more conventional for this
> -- `into_vec` implies consuming the object, but if we want to keep the
> warning in `Cmdq::receive_msg` if not all the data is consumed we need
> to take &mut self.

I had another look at this and especially how the SBuffer you refer to is used.
Unfortunately, the underlying code is broken.

driver_read_area() creates a reference to the whole DMA object, including the
area the GSP might concurrently write to. This is undefined behavior. See also
commit commit 0073a17b4666 ("gpu: nova-core: gsp: fix UB in DmaGspMem pointer
accessors"), where I fixed something similar.

Additionally, even if it would only create a reference to the part of the buffer
that can be considerd untouched by the GSP and hence suits for creating a
reference, driver_read_area() and all subsequent callers would still need to be
unsafe as they would need to promise to not keep the reference alive beyond GSP
accessing that memory region again.

(The situation is similar for driver_write_area().)

So, unfortunately, commit 75f6b1de8133 ("gpu: nova-core: gsp: Add GSP command
queue bindings and handling") seems broken in this aspect.

This needs to be fixed first, and I think we should probably create a copy in
driver_read_area() right away.

I don't want to merge any code that builds on top of this before we have sorted
this out.