From: "Benno Lossin" <lossin@kernel.org>
To: "Janne Grunau" <j@jannau.net>
Cc: "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>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Trevor Gross" <tmgross@umich.edu>,
"Danilo Krummrich" <dakr@kernel.org>,
"Fiona Behrens" <me@kloenk.dev>, "Alban Kurti" <kurti@invicto.ai>,
rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
asahi@lists.linux.dev
Subject: Re: [PATCH] rust: pin-init: add references to previously initialized fields
Date: Fri, 27 Feb 2026 16:02:43 +0100 [thread overview]
Message-ID: <DGPU3530XZA3.3SY27R9VIC9Y9@kernel.org> (raw)
In-Reply-To: <DFLXAD7L0MYF.39C9FW4KL917D@kernel.org>
On Sun Jan 11, 2026 at 6:06 PM CET, Benno Lossin wrote:
> On Sat Dec 6, 2025 at 6:02 PM CET, Janne Grunau wrote:
>> On Sat, Dec 06, 2025 at 09:23:08AM +0100, Benno Lossin wrote:
>>> On Wed Dec 3, 2025 at 11:05 PM CET, Janne Grunau wrote:
>>> > On Fri, Sep 05, 2025 at 04:00:46PM +0200, Benno Lossin wrote:
>>> >> After initializing a field in an initializer macro, create a variable
>>> >> holding a reference that points at that field. The type is either
>>> >> `Pin<&mut T>` or `&mut T` depending on the field's structural pinning
>>> >> kind.
>>> >
>>> > just as a heads up: creating references broke part of the agx firmware
>>> > init structs which uses a `#[repr(C, packed)]` struct as field in
>>> > another struct. This fails with
>>> >
>>> > | error[E0793]: reference to packed field is unaligned
>>> > | --> ../drivers/gpu/drm/asahi/initdata.rs:722:28
>>> > | |
>>> > | 722 | sub <- try_init!(raw::GlobalsSub::ver {
>>> > | | ____________________________^
>>> > | 723 | | unk_54: cfg.global_unk_54,
>>> > | 724 | | unk_56: 40,
>>> > | 725 | | unk_58: 0xffff,
>>> > | ... |
>>> > | 731 | | ..Zeroable::init_zeroed()
>>> > | 732 | | }),
>>> > | | |______________________^
>>> > | |
>>> > | = note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
>>> > | = note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
>>> > | = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
>>> > | = note: this error originates in the macro `$crate::__init_internal` which comes from the expansion of the macro `try_init` (in Nightly builds, run with -Z macro-backtrace for more info)
>>> >
>>> > This was easy enough to work around, I don't see how this embedding of a
>>> > `#[repr(C, packed)]` struct was necessary or at least helpful. The code
>>> > is not expected to be included in the upstream driver so it no worth
>>> > spending effort on this.
>>> >
>>> > I don't think it's likely that anyone else will run into this but I
>>> > thought I mention it at least.
>>> >
>>> > The asahi driver also ran into the discussed variable shadowing issue (a
>>> > variable was used to initialize a field of the same name and was later
>>> > used to initialize another field). This was trivially fixed by renaming
>>> > the variable.
>>>
>>> Thanks for the report, I expected the latter kind of error, but was not
>>> aware of the packed struct issue. If anyone needs a proper workaround
>>> from pin-init, let me know.
>>
>> I spoke too soon. The packed struct issue is also present in the capture
>> driver for Macbook microphones (aop_audio). Working around this issue
>> there is less obvious and more effort. I think it might be enough to use
>> unaligned u32 / u64 types already present in asahi [1].
>>
>> I'm not sure how prevalent packed structs are outside of Apple's
>> firmware interfaces. I was surprised running into the same issue in a
>> second driver but I shouldn't have been. There are plans for another
>> driver where this isssue will be present.
>>
>> A workaround on pin-init side would be appreciated. Due to the nature of
>> these packed structs I do not see a need to have access to previously
>> initialized fields. An optional way to supress the references would be
>> good enough for the cases I'm aware off.
>
> FYI, I am merging a workaround this cycle, see [1]. Gary had a good idea
> for a future patch series which I am tracking in [2].
Update on this situation, we have a problem: packed struct and the
current version of pin-init are unsound, so I'll sadly have to remove
the current workaround attribute. That is because the `[Pin]Init` trait
requires an aligned pointer as the input. Fixing this requires
introducing a new hierarchy for the Init trait that supports misaligned
writes.
If you really require this feature, then we can work something out.
Would you mind giving me a pointer to the code that you're currently
using or that you would like to support?
Cheers,
Benno
prev parent reply other threads:[~2026-02-27 15:02 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-05 14:00 [PATCH] rust: pin-init: add references to previously initialized fields Benno Lossin
2025-09-05 17:18 ` Benno Lossin
2025-09-05 17:44 ` Boqun Feng
2025-09-06 10:52 ` Danilo Krummrich
2025-09-07 1:57 ` Boqun Feng
2025-09-07 2:07 ` Boqun Feng
2025-09-07 8:41 ` Benno Lossin
2025-09-07 17:29 ` Danilo Krummrich
2025-09-07 21:06 ` Benno Lossin
2025-09-07 21:39 ` Danilo Krummrich
2025-09-07 22:51 ` Benno Lossin
2025-09-07 23:33 ` Danilo Krummrich
2025-09-08 2:08 ` Boqun Feng
2025-09-08 8:27 ` Benno Lossin
2025-09-08 8:57 ` Danilo Krummrich
2025-09-08 19:38 ` Boqun Feng
2025-09-08 20:31 ` Danilo Krummrich
2025-09-10 10:12 ` Benno Lossin
2025-09-05 17:21 ` Boqun Feng
2025-09-05 18:38 ` Danilo Krummrich
2025-09-06 14:23 ` kernel test robot
2025-09-11 21:35 ` Benno Lossin
2025-12-03 22:05 ` Janne Grunau
2025-12-06 8:23 ` Benno Lossin
2025-12-06 17:02 ` Janne Grunau
2026-01-11 17:06 ` Benno Lossin
2026-02-27 15:02 ` Benno Lossin [this message]
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=DGPU3530XZA3.3SY27R9VIC9Y9@kernel.org \
--to=lossin@kernel.org \
--cc=a.hindborg@kernel.org \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=asahi@lists.linux.dev \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=dakr@kernel.org \
--cc=gary@garyguo.net \
--cc=j@jannau.net \
--cc=kurti@invicto.ai \
--cc=linux-kernel@vger.kernel.org \
--cc=me@kloenk.dev \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tmgross@umich.edu \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.