linux-kernel - Re: [PATCH v2 1/3] nova-core: Simplify `transmute` and `transmute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DDXC17HXPXFZ.3TIX7FHUJHAI7@nvidia.com>
Date: Sat, 01 Nov 2025 20:47:16 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Daniel del Castillo" <delcastillodelarosadaniel@...il.com>, "Danilo
 Krummrich" <dakr@...nel.org>, "Alexandre Courbot" <acourbot@...dia.com>,
 "David Airlie" <airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>,
 "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>
Cc: <nouveau@...ts.freedesktop.org>, <dri-devel@...ts.freedesktop.org>,
 <linux-kernel@...r.kernel.org>, "Boqun Feng" <boqun.feng@...il.com>, "Gary
 Guo" <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, <rust-for-linux@...r.kernel.org>,
 "dri-devel" <dri-devel-bounces@...ts.freedesktop.org>
Subject: Re: [PATCH v2 1/3] nova-core: Simplify `transmute` and
 `transmute_mut` in fwsec.rs

On Fri Oct 24, 2025 at 5:51 AM JST, Daniel del Castillo wrote:
> This patch solves one of the existing mentions of COHA, a task
> in the Nova task list about improving the `CoherentAllocation` API.
> It uses the new `from_bytes` method from the `FromBytes` trait as
> well as the `as_slice` and `as_slice_mut` methods from
> `CoherentAllocation`.
>
> Signed-off-by: Daniel del Castillo <delcastillodelarosadaniel@...il.com>
>
> ---
>
> I confirmed by talking to Alexandre Courbot, that the reading/writing
> methods in `CoherentAllocation` can never be safe, so
> this patch doesn't actually change `CoherentAllocation`, but rather
> tries to solve one of the existing references to [COHA].
>
> V1 -> V2: Split previous patch into two. One per reference to COHA.
>           Improved comments. Let me know if they are okay now.
>           Use of `{...}` syntax for the `if let`
>
>  drivers/gpu/nova-core/firmware/fwsec.rs | 129 +++++++++++-------------
>  1 file changed, 60 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
> index 8edbb5c0572c..507ef3868565 100644
> --- a/drivers/gpu/nova-core/firmware/fwsec.rs
> +++ b/drivers/gpu/nova-core/firmware/fwsec.rs
> @@ -11,12 +11,12 @@
>  //! - The ucode signature, so the GSP falcon can run FWSEC in HS mode.
>  
>  use core::marker::PhantomData;
> -use core::mem::{align_of, size_of};
> +use core::mem::size_of;
>  use core::ops::Deref;
>  
>  use kernel::device::{self, Device};
>  use kernel::prelude::*;
> -use kernel::transmute::FromBytes;
> +use kernel::transmute::{AsBytes, FromBytes};
>  
>  use crate::dma::DmaObject;
>  use crate::driver::Bar0;
> @@ -35,7 +35,7 @@ struct FalconAppifHdrV1 {
>      entry_size: u8,
>      entry_count: u8,
>  }
> -// SAFETY: any byte sequence is valid for this struct.
> +// SAFETY: Any byte sequence is valid for this struct.
>  unsafe impl FromBytes for FalconAppifHdrV1 {}
>  
>  #[repr(C, packed)]
> @@ -44,7 +44,7 @@ struct FalconAppifV1 {
>      id: u32,
>      dmem_base: u32,
>  }
> -// SAFETY: any byte sequence is valid for this struct.
> +// SAFETY: Any byte sequence is valid for this struct.
>  unsafe impl FromBytes for FalconAppifV1 {}
>  
>  #[derive(Debug)]
> @@ -68,8 +68,10 @@ struct FalconAppifDmemmapperV3 {
>      ucode_cmd_mask1: u32,
>      multi_tgt_tbl: u32,
>  }
> -// SAFETY: any byte sequence is valid for this struct.
> +// SAFETY: Any byte sequence is valid for this struct.

I appreciate the capitalization, but these changes are a bit
distracting. :) If you absolutely want to do this, let it be its own
patch so the current one stays focused on what it actually does.

>  unsafe impl FromBytes for FalconAppifDmemmapperV3 {}
> +// SAFETY: This struct doesn't contain unitialized bytes and doesn't have interior mutability.

Typo: s/unitialized/uninitialized (and in other comments as well).

Otherwise this looks ok - it doesn't apply cleanly on drm-rust-next
though, could you rebase for the next version?

Thanks for the cleanup!