From: "Danilo Krummrich" <dakr@kernel.org>
To: "Eliot Courtney" <ecourtney@nvidia.com>
Cc: "Alice Ryhl" <aliceryhl@google.com>,
"Alexandre Courbot" <acourbot@nvidia.com>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
<rust-for-linux@vger.kernel.org>, <nouveau@lists.freedesktop.org>,
<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
"dri-devel" <dri-devel-bounces@lists.freedesktop.org>,
"Gary Guo" <gary@garyguo.net>
Subject: Re: [PATCH 6/9] gpu: nova-core: generalize `flush_into_kvec` to `flush_into_vec`
Date: Mon, 16 Mar 2026 13:21:23 +0100 [thread overview]
Message-ID: <DH47AVPEKN06.3BERUSJIB4M1R@kernel.org> (raw)
In-Reply-To: <DH46IKESDQ3B.2GEUC479WDK3N@nvidia.com>
(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.
next prev parent reply other threads:[~2026-03-16 13:21 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-27 12:32 [PATCH 0/9] gpu: nova-core: gsp: add RM control command infrastructure Eliot Courtney
2026-02-27 12:32 ` [PATCH 1/9] gpu: nova-core: gsp: add NV_STATUS error code bindings Eliot Courtney
2026-02-27 12:32 ` [PATCH 2/9] gpu: nova-core: gsp: add NvStatus enum for RM control errors Eliot Courtney
2026-02-27 12:32 ` [PATCH 3/9] gpu: nova-core: gsp: expose GSP-RM internal client and subdevice handles Eliot Courtney
2026-03-09 21:22 ` Joel Fernandes
2026-03-09 23:41 ` Joel Fernandes
2026-03-10 0:06 ` John Hubbard
2026-03-10 2:17 ` Joel Fernandes
2026-03-10 2:29 ` John Hubbard
2026-03-10 18:48 ` Joel Fernandes
2026-03-10 2:36 ` Alexandre Courbot
2026-03-10 4:02 ` Eliot Courtney
2026-03-10 10:35 ` Danilo Krummrich
2026-02-27 12:32 ` [PATCH 4/9] gpu: nova-core: gsp: add RM control RPC structure binding Eliot Courtney
2026-02-27 12:32 ` [PATCH 5/9] gpu: nova-core: gsp: add types for RM control RPCs Eliot Courtney
2026-03-09 21:45 ` Joel Fernandes
2026-03-16 11:42 ` Eliot Courtney
2026-02-27 12:32 ` [PATCH 6/9] gpu: nova-core: generalize `flush_into_kvec` to `flush_into_vec` Eliot Courtney
2026-03-09 21:53 ` Joel Fernandes
2026-03-09 21:57 ` Danilo Krummrich
2026-03-09 22:01 ` Danilo Krummrich
2026-03-16 11:44 ` Eliot Courtney
2026-03-16 12:21 ` Danilo Krummrich [this message]
2026-03-17 1:55 ` Alexandre Courbot
2026-03-17 10:49 ` Danilo Krummrich
2026-03-17 13:41 ` Alexandre Courbot
2026-03-17 14:12 ` Danilo Krummrich
2026-03-18 1:52 ` Alexandre Courbot
2026-02-27 12:32 ` [PATCH 7/9] gpu: nova-core: gsp: add RM control command infrastructure Eliot Courtney
2026-03-02 8:00 ` Zhi Wang
2026-03-09 22:08 ` Joel Fernandes
2026-03-13 15:40 ` Danilo Krummrich
2026-03-16 12:06 ` Eliot Courtney
2026-02-27 12:32 ` [PATCH 8/9] gpu: nova-core: gsp: add CE fault method buffer size bindings Eliot Courtney
2026-03-09 22:08 ` Joel Fernandes
2026-02-27 12:32 ` [PATCH 9/9] gpu: nova-core: gsp: add CeGetFaultMethodBufferSize RM control command Eliot Courtney
2026-03-09 22:23 ` Joel Fernandes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DH47AVPEKN06.3BERUSJIB4M1R@kernel.org \
--to=dakr@kernel.org \
--cc=acourbot@nvidia.com \
--cc=airlied@gmail.com \
--cc=aliceryhl@google.com \
--cc=dri-devel-bounces@lists.freedesktop.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=ecourtney@nvidia.com \
--cc=gary@garyguo.net \
--cc=linux-kernel@vger.kernel.org \
--cc=nouveau@lists.freedesktop.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=simona@ffwll.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox