linux-kernel - Re: [PATCH 3/9] gpu: nova-core: gsp: simplify sequencer opcode parsing

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: <8482d73df0a6b3a5cbddf5381e9bb8b261190fae.camel@redhat.com>
Date: Thu, 11 Dec 2025 17:04:00 -0500
From: lyude@...hat.com
To: Alexandre Courbot <acourbot@...dia.com>, Danilo Krummrich
 <dakr@...nel.org>,  Alice Ryhl <aliceryhl@...gle.com>, David Airlie
 <airlied@...il.com>, Simona Vetter <simona@...ll.ch>
Cc: John Hubbard <jhubbard@...dia.com>, Alistair Popple
 <apopple@...dia.com>,  Joel Fernandes <joelagnelf@...dia.com>, Timur Tabi
 <ttabi@...dia.com>, Edwin Peer <epeer@...dia.com>, 
	nouveau@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH 3/9] gpu: nova-core: gsp: simplify sequencer opcode
 parsing

Reviewed-by: Lyude Paul <lyude@...hat.com>

On Mon, 2025-12-08 at 18:26 +0900, Alexandre Courbot wrote:
> The opcodes are already the right type in the C union, so we can use
> them directly instead of converting them to a byte stream and back
> again
> using `FromBytes`.
> 
> Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
> ---
>  drivers/gpu/nova-core/gsp/fw.rs | 40 +++++--------------------------
> ---------
>  1 file changed, 5 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-
> core/gsp/fw.rs
> index 24e4eaaf1265..d06c0fdd6154 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -472,13 +472,7 @@ pub(crate) fn reg_write_payload(&self) ->
> Result<RegWritePayload> {
>              return Err(EINVAL);
>          }
>          // SAFETY: Opcode is verified to be `RegWrite`, so union
> contains valid `RegWritePayload`.
> -        let payload_bytes = unsafe {
> -            core::slice::from_raw_parts(
> -               
> core::ptr::addr_of!(self.0.payload.regWrite).cast::<u8>(),
> -                core::mem::size_of::<RegWritePayload>(),
> -            )
> -        };
> -       
> Ok(*RegWritePayload::from_bytes(payload_bytes).ok_or(EINVAL)?)
> +        Ok(RegWritePayload(unsafe { self.0.payload.regWrite }))
>      }
>  
>      /// Returns the register modify payload by value.
> @@ -489,13 +483,7 @@ pub(crate) fn reg_modify_payload(&self) ->
> Result<RegModifyPayload> {
>              return Err(EINVAL);
>          }
>          // SAFETY: Opcode is verified to be `RegModify`, so union
> contains valid `RegModifyPayload`.
> -        let payload_bytes = unsafe {
> -            core::slice::from_raw_parts(
> -               
> core::ptr::addr_of!(self.0.payload.regModify).cast::<u8>(),
> -                core::mem::size_of::<RegModifyPayload>(),
> -            )
> -        };
> -       
> Ok(*RegModifyPayload::from_bytes(payload_bytes).ok_or(EINVAL)?)
> +        Ok(RegModifyPayload(unsafe { self.0.payload.regModify }))
>      }
>  
>      /// Returns the register poll payload by value.
> @@ -506,13 +494,7 @@ pub(crate) fn reg_poll_payload(&self) ->
> Result<RegPollPayload> {
>              return Err(EINVAL);
>          }
>          // SAFETY: Opcode is verified to be `RegPoll`, so union
> contains valid `RegPollPayload`.
> -        let payload_bytes = unsafe {
> -            core::slice::from_raw_parts(
> -               
> core::ptr::addr_of!(self.0.payload.regPoll).cast::<u8>(),
> -                core::mem::size_of::<RegPollPayload>(),
> -            )
> -        };
> -       
> Ok(*RegPollPayload::from_bytes(payload_bytes).ok_or(EINVAL)?)
> +        Ok(RegPollPayload(unsafe { self.0.payload.regPoll }))
>      }
>  
>      /// Returns the delay payload by value.
> @@ -523,13 +505,7 @@ pub(crate) fn delay_us_payload(&self) ->
> Result<DelayUsPayload> {
>              return Err(EINVAL);
>          }
>          // SAFETY: Opcode is verified to be `DelayUs`, so union
> contains valid `DelayUsPayload`.
> -        let payload_bytes = unsafe {
> -            core::slice::from_raw_parts(
> -               
> core::ptr::addr_of!(self.0.payload.delayUs).cast::<u8>(),
> -                core::mem::size_of::<DelayUsPayload>(),
> -            )
> -        };
> -       
> Ok(*DelayUsPayload::from_bytes(payload_bytes).ok_or(EINVAL)?)
> +        Ok(DelayUsPayload(unsafe { self.0.payload.delayUs }))
>      }
>  
>      /// Returns the register store payload by value.
> @@ -540,13 +516,7 @@ pub(crate) fn reg_store_payload(&self) ->
> Result<RegStorePayload> {
>              return Err(EINVAL);
>          }
>          // SAFETY: Opcode is verified to be `RegStore`, so union
> contains valid `RegStorePayload`.
> -        let payload_bytes = unsafe {
> -            core::slice::from_raw_parts(
> -               
> core::ptr::addr_of!(self.0.payload.regStore).cast::<u8>(),
> -                core::mem::size_of::<RegStorePayload>(),
> -            )
> -        };
> -       
> Ok(*RegStorePayload::from_bytes(payload_bytes).ok_or(EINVAL)?)
> +        Ok(RegStorePayload(unsafe { self.0.payload.regStore }))
>      }
>  }
>