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 <[email protected]>
>
> ---
>
> 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!
