From: "Danilo Krummrich" <dakr@kernel.org>
To: "Joel Fernandes" <joelagnelf@nvidia.com>
Cc: "Alexandre Courbot" <acourbot@nvidia.com>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Alex Gaynor" <alex.gaynor@gmail.com>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Trevor Gross" <tmgross@umich.edu>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Maxime Ripard" <mripard@kernel.org>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"John Hubbard" <jhubbard@nvidia.com>,
"Alistair Popple" <apopple@nvidia.com>,
"Timur Tabi" <ttabi@nvidia.com>,
rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v5 02/12] gpu: nova-core: move GSP boot code to a dedicated method
Date: Sat, 13 Sep 2025 21:53:16 +0200 [thread overview]
Message-ID: <DCRXOMQN3Z20.2JCNP4BDEE79T@kernel.org> (raw)
In-Reply-To: <20250913171357.GA1551194@joelbox2>
On Sat Sep 13, 2025 at 7:13 PM CEST, Joel Fernandes wrote:
> On Sat, Sep 13, 2025 at 03:30:31PM +0200, Danilo Krummrich wrote:
>> On Sat Sep 13, 2025 at 3:02 AM CEST, Joel Fernandes wrote:
>> > Any chance we can initialize the locks later? We don't need locking until
>> > after the boot process is completed, and if there's a way we can dynamically
>> > "pin", where we hypothetically pin after the boot process completed, that
>> > might also work. Though I am not sure if that's something possible in
>> > Rust/rust4linux or if it makes sense.
>>
>> We can't partially initialize structures and then rely on accessing initialized
>> data only.
>
> Yet, that is exactly what the pin initialization sequence block does? The
> whole structure is not initialized yet you need access to already initialized
> fields.
No, having a reference to a partially initialized structure is UB. But of course
you can have a reference to already initialized fields within a not yet fully
initialized structure.
>> This is one of the sources for memory bugs that Rust tries to solve.
>> You can wrap fields into Option types and initialize them later, which would
>> defer pin-init calls for the price of having Option fields around.
>
> I am not denying the need for pinning. Also regarding Option, just thinking
> out loud but if something is optional temporary, maybe needing a new type
> like TempOption, and promote it to a non-option type later, I am not seeing
> that as completely outside the world, if there is a legitimate usecase that
> needs to be Option temporarily, but not later, what's wrong with that? It is
> "Optional" for the timebeing, but not later.
That's what MaybeUninit<T> from the core library already does and promoting it
to T is fundamentally unsafe for obvious reasons.
Drivers should never use that. Having partially initialized structures is a
horrible anti-pattern that we see too often in C code (only for convinience
reasons) causing real memory bugs.
If you run into a case where you want this, 99% of the time you have a design
issue that you should fix instead.
>> However, we should never do such things. If there's the necessity to do
>> something like that, it indicates a design issue.
>>
>> In this case, there's no problem, we can use pin-init without any issues right
>> away, and should do so.
>>
>> pin-init is going to be an essential part of *every* Rust driver given that a
>> lot of the C infrastruture that we abstract requires pinned initialization, such
>> as locks and other synchronization primitives.
>
> To be honest, the pinning concept seems like an after thought for such a
> fundamental thing that we need, requiring additional macros, and bandaids on
> top of the language itself, to make it work for the kernel. I am not alone in
> that opinion. This should be first-class in a (systems) language, built into
> the language itself? I am talking about the whole pin initialization,
> accessing fields dances, etc.
Yes, that's exactly why people (Benno) are already working on making this a
language feature (here's a first step in this direction [1]).
Benno should have more details on this.
[1] https://github.com/rust-lang/rust/pull/146307
> Also I am concerned that overusage of pinning defeats a lot of optimizations
pin-init does the oposite it allows us to use a single memory allocation where
otherwise you would need multiple.
Can you please show some optimizations that can not be done in drivers due to
pin-init for dynamic allocations?
Or in other words, what are the things you want to do with a KBox<T> in drivers
that you can't do with a Pin<KBox<T>> in a more optimal way?
> that Rust may be able to perform, especially forcefully pinning stuff that
> does not need all to be pinned (except to satisfy paranoia),
Can you please present some examples where it is a major advantage to be able to
move out of a Box in drivers? I think you will have a hard time finding them.
In C code, how often do you move fields out of structures that live within a
kmalloc() allocation?
> thus generating
> suboptimal code gen. Not only does it require redesign and concerns over
> accesses to un-initialized fields,
We're not doing any accesses to uninitialized fields with pin-init, nor do we
encourage them.
> like we saw in the last 2-3 weeks, it also
> forces people into that when maybe there is a chance that underlying
> structures do not need to be pinned at all (for some usecases).
Again, what are those use-cases where you want to be able to move out of a Box
in drivers?
next prev parent reply other threads:[~2025-09-13 19:53 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-11 11:04 [PATCH v5 00/12] gpu: nova-core: process and prepare more firmwares to boot GSP Alexandre Courbot
2025-09-11 11:04 ` [PATCH v5 01/12] gpu: nova-core: require `Send` on `FalconEngine` and `FalconHal` Alexandre Courbot
2025-09-11 11:04 ` [PATCH v5 02/12] gpu: nova-core: move GSP boot code to a dedicated method Alexandre Courbot
2025-09-11 11:22 ` Danilo Krummrich
2025-09-11 12:17 ` Alexandre Courbot
2025-09-11 12:46 ` Danilo Krummrich
2025-09-11 13:26 ` Alexandre Courbot
2025-09-11 14:22 ` Benno Lossin
2025-09-13 1:02 ` Joel Fernandes
2025-09-13 13:30 ` Danilo Krummrich
2025-09-13 17:13 ` Joel Fernandes
2025-09-13 19:53 ` Danilo Krummrich [this message]
2025-09-13 23:02 ` Joel Fernandes
2025-09-14 7:58 ` Benno Lossin
2025-09-13 20:37 ` Miguel Ojeda
2025-09-13 21:16 ` Joel Fernandes
2025-09-13 21:29 ` John Hubbard
2025-09-13 22:06 ` Joel Fernandes
2025-09-14 1:49 ` Alexandre Courbot
2025-09-14 14:42 ` Benno Lossin
2025-09-15 4:59 ` Alexandre Courbot
2025-09-15 6:44 ` Benno Lossin
2025-09-11 11:04 ` [PATCH v5 03/12] gpu: nova-core: initialize Gpu structure fully in-place Alexandre Courbot
2025-09-11 11:04 ` [PATCH v5 04/12] gpu: nova-core: add Chipset::name() method Alexandre Courbot
2025-09-11 11:04 ` [PATCH v5 05/12] gpu: nova-core: firmware: move firmware request code into a function Alexandre Courbot
2025-09-11 11:23 ` Danilo Krummrich
2025-09-11 12:18 ` Alexandre Courbot
2025-09-11 11:04 ` [PATCH v5 06/12] gpu: nova-core: firmware: add support for common firmware header Alexandre Courbot
2025-09-11 11:04 ` [PATCH v5 07/12] gpu: nova-core: firmware: process Booter and patch its signature Alexandre Courbot
2025-09-11 11:04 ` [PATCH v5 08/12] gpu: nova-core: firmware: process and prepare the GSP firmware Alexandre Courbot
2025-09-11 11:27 ` Danilo Krummrich
2025-09-11 12:29 ` Alexandre Courbot
2025-09-11 12:31 ` Danilo Krummrich
2025-09-11 11:04 ` [PATCH v5 09/12] gpu: nova-core: firmware: process the GSP bootloader Alexandre Courbot
2025-09-11 11:04 ` [PATCH v5 10/12] gpu: nova-core: firmware: use 570.144 firmware Alexandre Courbot
2025-09-11 11:04 ` [PATCH v5 11/12] gpu: nova-core: Add base files for r570.144 firmware bindings Alexandre Courbot
2025-09-11 11:04 ` [PATCH v5 12/12] gpu: nova-core: compute layout of more framebuffer regions required for GSP Alexandre Courbot
2025-09-11 11:28 ` [PATCH v5 00/12] gpu: nova-core: process and prepare more firmwares to boot GSP Danilo Krummrich
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=DCRXOMQN3Z20.2JCNP4BDEE79T@kernel.org \
--to=dakr@kernel.org \
--cc=a.hindborg@kernel.org \
--cc=acourbot@nvidia.com \
--cc=airlied@gmail.com \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=apopple@nvidia.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=gary@garyguo.net \
--cc=jhubbard@nvidia.com \
--cc=joelagnelf@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=nouveau@lists.freedesktop.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=simona@ffwll.ch \
--cc=tmgross@umich.edu \
--cc=ttabi@nvidia.com \
--cc=tzimmermann@suse.de \
/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;
as well as URLs for NNTP newsgroup(s).