Alexandre Courbot
2025-Nov-01 11:47 UTC
[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 at gmail.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!