Rust for Linux List
 help / color / mirror / Atom feed
* [PATCH] gpu: nova-core: parse structs via zerocopy
@ 2026-06-21 14:36 Nicolás Antinori
  2026-06-22  7:45 ` Alistair Popple
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolás Antinori @ 2026-06-21 14:36 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, Miguel Ojeda
  Cc: Nicolás Antinori, Alexandre Courbot, David Airlie,
	Shuah Khan, Simona Vetter, Gary Guo, Onur Özkan,
	Tamir Duberstein, Trevor Gross, linux-kernel,
	linux-kernel-mentees, dri-devel, rust-for-linux, nova-gpu

Replace the unsafe `kernel::transmute::FromBytes` trait implementation
for the `FalconUCodeDescV3`, `PcirStruct`, `BitHeader`, `NpdeStruct`,
and `PmuLookupTableHeader` structs with the derivable
`zerocopy::FromBytes` trait.

This change eliminates the manual unsafe implementations in favor of a
derivable trait. When this trait is derived, validity checks are
performed at compile time to make sure that the type can safely
implement `FromBytes`.

Link: https://github.com/Rust-for-Linux/linux/issues/1241
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Nicolás Antinori <nico.antinori.7@gmail.com>
---
NOTE: Compile-tested only. I don't own a piece of hardware where I can
test the nova driver.

 drivers/gpu/nova-core/firmware.rs |  6 +----
 drivers/gpu/nova-core/vbios.rs    | 38 ++++++++-----------------------
 2 files changed, 11 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index ad37994ac15a..0afd1d3fe5ad 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -86,7 +86,7 @@ pub(crate) struct FalconUCodeDescV2 {

 /// Structure used to describe some firmwares, notably FWSEC-FRTS.
 #[repr(C)]
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, FromBytes)]
 pub(crate) struct FalconUCodeDescV3 {
     /// Header defined by `NV_BIT_FALCON_UCODE_DESC_HEADER_VDESC*` in OpenRM.
     hdr: u32,
@@ -117,10 +117,6 @@ pub(crate) struct FalconUCodeDescV3 {
     _reserved: u16,
 }

-// SAFETY: all bit patterns are valid for this type, and it doesn't use
-// interior mutability.
-unsafe impl FromBytes for FalconUCodeDescV3 {}
-
 /// Enum wrapping the different versions of Falcon microcode descriptors.
 ///
 /// This allows handling both V2 and V3 descriptor formats through a
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 8b7d17a24660..754812bcbdde 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -13,11 +13,8 @@
         Alignment, //
     },
     sync::aref::ARef,
-    transmute::FromBytes,
 };

-use zerocopy::FromBytes as _;
-
 use crate::{
     driver::Bar0,
     firmware::{
@@ -301,7 +298,7 @@ pub(crate) fn fwsec_image(&self) -> &FwSecBiosImage {
 }

 /// PCI Data Structure as defined in PCI Firmware Specification
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, FromBytes)]
 #[repr(C)]
 struct PcirStruct {
     /// PCI Data Structure signature ("PCIR" or "NPDS")
@@ -330,12 +327,9 @@ struct PcirStruct {
     max_runtime_image_len: u16,
 }

-// SAFETY: all bit patterns are valid for `PcirStruct`.
-unsafe impl FromBytes for PcirStruct {}
-
 impl PcirStruct {
     fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
-        let (pcir, _) = PcirStruct::from_bytes_copy_prefix(data).ok_or(EINVAL)?;
+        let (pcir, _) = PcirStruct::read_from_prefix(data).map_err(|_| EINVAL)?;

         // Signature should be "PCIR" (0x52494350) or "NPDS" (0x5344504e).
         if &pcir.signature != b"PCIR" && &pcir.signature != b"NPDS" {
@@ -371,7 +365,7 @@ fn image_size_bytes(&self) -> usize {
 /// This is the head of the BIT table, that is used to locate the Falcon data. The BIT table (with
 /// its header) is in the [`PciAtBiosImage`] and the falcon data it is pointing to is in the
 /// [`FwSecBiosImage`].
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, FromBytes)]
 #[repr(C)]
 struct BitHeader {
     /// 0h: BIT Header Identifier (BMP=0x7FFF/BIT=0xB8FF)
@@ -390,12 +384,9 @@ struct BitHeader {
     checksum: u8,
 }

-// SAFETY: all bit patterns are valid for `BitHeader`.
-unsafe impl FromBytes for BitHeader {}
-
 impl BitHeader {
     fn new(data: &[u8]) -> Result<Self> {
-        let (header, _) = BitHeader::from_bytes_copy_prefix(data).ok_or(EINVAL)?;
+        let (header, _) = BitHeader::read_from_prefix(data).map_err(|_| EINVAL)?;

         // Check header ID and signature
         if header.id != 0xB8FF || &header.signature != b"BIT\0" {
@@ -533,7 +524,7 @@ fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
 /// PCI Data Structure. It contains some fields that are redundant with the PCI Data Structure, but
 /// are needed for traversing the BIOS images. It is expected to be present in all BIOS images
 /// except for NBSI images.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, FromBytes)]
 #[repr(C)]
 struct NpdeStruct {
     /// 00h: Signature ("NPDE")
@@ -548,12 +539,9 @@ struct NpdeStruct {
     last_image: u8,
 }

-// SAFETY: all bit patterns are valid for `NpdeStruct`.
-unsafe impl FromBytes for NpdeStruct {}
-
 impl NpdeStruct {
     fn new(dev: &device::Device, data: &[u8]) -> Option<Self> {
-        let (npde, _) = NpdeStruct::from_bytes_copy_prefix(data)?;
+        let (npde, _) = NpdeStruct::read_from_prefix(data).ok()?;

         // Signature should be "NPDE" (0x4544504E).
         if &npde.signature != b"NPDE" {
@@ -845,6 +833,7 @@ fn new(data: &[u8]) -> Result<Self> {
 }

 #[repr(C)]
+#[derive(FromBytes)]
 struct PmuLookupTableHeader {
     version: u8,
     header_len: u8,
@@ -852,9 +841,6 @@ struct PmuLookupTableHeader {
     entry_count: u8,
 }

-// SAFETY: all bit patterns are valid for `PmuLookupTableHeader`.
-unsafe impl FromBytes for PmuLookupTableHeader {}
-
 /// The [`PmuLookupTableEntry`] structure is used to find the [`PmuLookupTableEntry`] for a given
 /// application ID.
 ///
@@ -867,7 +853,7 @@ struct PmuLookupTable {

 impl PmuLookupTable {
     fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
-        let (header, _) = PmuLookupTableHeader::from_bytes_copy_prefix(data).ok_or(EINVAL)?;
+        let (header, _) = PmuLookupTableHeader::read_from_prefix(data).map_err(|_| EINVAL)?;

         let header_len = usize::from(header.header_len);
         let entry_len = usize::from(header.entry_len);
@@ -1013,15 +999,11 @@ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
         let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
         match ver {
             2 => {
-                let v2 = FalconUCodeDescV2::read_from_prefix(data)
-                    .map_err(|_| EINVAL)?
-                    .0;
+                let (v2, _) = FalconUCodeDescV2::read_from_prefix(data).map_err(|_| EINVAL)?;
                 Ok(FalconUCodeDesc::V2(v2))
             }
             3 => {
-                let v3 = FalconUCodeDescV3::from_bytes_copy_prefix(data)
-                    .ok_or(EINVAL)?
-                    .0;
+                let (v3, _) = FalconUCodeDescV3::read_from_prefix(data).map_err(|_| EINVAL)?;
                 Ok(FalconUCodeDesc::V3(v3))
             }
             _ => {
--
2.47.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] gpu: nova-core: parse structs via zerocopy
  2026-06-21 14:36 [PATCH] gpu: nova-core: parse structs via zerocopy Nicolás Antinori
@ 2026-06-22  7:45 ` Alistair Popple
  2026-06-22 20:26   ` Nicolás Antinori
  0 siblings, 1 reply; 4+ messages in thread
From: Alistair Popple @ 2026-06-22  7:45 UTC (permalink / raw)
  To: Nicolás Antinori
  Cc: Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alexandre Courbot,
	David Airlie, Shuah Khan, Simona Vetter, Gary Guo,
	Onur Özkan, Tamir Duberstein, Trevor Gross, linux-kernel,
	linux-kernel-mentees, dri-devel, rust-for-linux, nova-gpu

On 2026-06-22 at 00:36 +1000, Nicolás Antinori <nico.antinori.7@gmail.com> wrote...
> Replace the unsafe `kernel::transmute::FromBytes` trait implementation
> for the `FalconUCodeDescV3`, `PcirStruct`, `BitHeader`, `NpdeStruct`,
> and `PmuLookupTableHeader` structs with the derivable
> `zerocopy::FromBytes` trait.
> 
> This change eliminates the manual unsafe implementations in favor of a
> derivable trait. When this trait is derived, validity checks are
> performed at compile time to make sure that the type can safely
> implement `FromBytes`.

Nice, this looks good although due to some code movement this doesn't compile
when applied to drm-rust-next[1]. Would you mind rebasing on top of that? The
changes required should be relatively straight forward, they are mostly the
result of some new users of transmute::FromBytes being added to vbios.rs.

Also what is you plan here? I have also been looking at some of these
conversions, specifically those involving the generated bindings. Really
appreciate your contributions, so no problem if you want to continue with some
of the conversions, just want to make sure we aren't duplicating effort here.

 - Alistair

[1] - https://gitlab.freedesktop.org/drm/rust/kernel/-/tree/drm-rust-next

> Link: https://github.com/Rust-for-Linux/linux/issues/1241
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Nicolás Antinori <nico.antinori.7@gmail.com>
> ---
> NOTE: Compile-tested only. I don't own a piece of hardware where I can
> test the nova driver.
> 
>  drivers/gpu/nova-core/firmware.rs |  6 +----
>  drivers/gpu/nova-core/vbios.rs    | 38 ++++++++-----------------------
>  2 files changed, 11 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
> index ad37994ac15a..0afd1d3fe5ad 100644
> --- a/drivers/gpu/nova-core/firmware.rs
> +++ b/drivers/gpu/nova-core/firmware.rs
> @@ -86,7 +86,7 @@ pub(crate) struct FalconUCodeDescV2 {
> 
>  /// Structure used to describe some firmwares, notably FWSEC-FRTS.
>  #[repr(C)]
> -#[derive(Debug, Clone)]
> +#[derive(Debug, Clone, FromBytes)]
>  pub(crate) struct FalconUCodeDescV3 {
>      /// Header defined by `NV_BIT_FALCON_UCODE_DESC_HEADER_VDESC*` in OpenRM.
>      hdr: u32,
> @@ -117,10 +117,6 @@ pub(crate) struct FalconUCodeDescV3 {
>      _reserved: u16,
>  }
> 
> -// SAFETY: all bit patterns are valid for this type, and it doesn't use
> -// interior mutability.
> -unsafe impl FromBytes for FalconUCodeDescV3 {}
> -
>  /// Enum wrapping the different versions of Falcon microcode descriptors.
>  ///
>  /// This allows handling both V2 and V3 descriptor formats through a
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 8b7d17a24660..754812bcbdde 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -13,11 +13,8 @@
>          Alignment, //
>      },
>      sync::aref::ARef,
> -    transmute::FromBytes,
>  };
> 
> -use zerocopy::FromBytes as _;
> -
>  use crate::{
>      driver::Bar0,
>      firmware::{
> @@ -301,7 +298,7 @@ pub(crate) fn fwsec_image(&self) -> &FwSecBiosImage {
>  }
> 
>  /// PCI Data Structure as defined in PCI Firmware Specification
> -#[derive(Debug, Clone)]
> +#[derive(Debug, Clone, FromBytes)]
>  #[repr(C)]
>  struct PcirStruct {
>      /// PCI Data Structure signature ("PCIR" or "NPDS")
> @@ -330,12 +327,9 @@ struct PcirStruct {
>      max_runtime_image_len: u16,
>  }
> 
> -// SAFETY: all bit patterns are valid for `PcirStruct`.
> -unsafe impl FromBytes for PcirStruct {}
> -
>  impl PcirStruct {
>      fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
> -        let (pcir, _) = PcirStruct::from_bytes_copy_prefix(data).ok_or(EINVAL)?;
> +        let (pcir, _) = PcirStruct::read_from_prefix(data).map_err(|_| EINVAL)?;
> 
>          // Signature should be "PCIR" (0x52494350) or "NPDS" (0x5344504e).
>          if &pcir.signature != b"PCIR" && &pcir.signature != b"NPDS" {
> @@ -371,7 +365,7 @@ fn image_size_bytes(&self) -> usize {
>  /// This is the head of the BIT table, that is used to locate the Falcon data. The BIT table (with
>  /// its header) is in the [`PciAtBiosImage`] and the falcon data it is pointing to is in the
>  /// [`FwSecBiosImage`].
> -#[derive(Debug, Clone, Copy)]
> +#[derive(Debug, Clone, Copy, FromBytes)]
>  #[repr(C)]
>  struct BitHeader {
>      /// 0h: BIT Header Identifier (BMP=0x7FFF/BIT=0xB8FF)
> @@ -390,12 +384,9 @@ struct BitHeader {
>      checksum: u8,
>  }
> 
> -// SAFETY: all bit patterns are valid for `BitHeader`.
> -unsafe impl FromBytes for BitHeader {}
> -
>  impl BitHeader {
>      fn new(data: &[u8]) -> Result<Self> {
> -        let (header, _) = BitHeader::from_bytes_copy_prefix(data).ok_or(EINVAL)?;
> +        let (header, _) = BitHeader::read_from_prefix(data).map_err(|_| EINVAL)?;
> 
>          // Check header ID and signature
>          if header.id != 0xB8FF || &header.signature != b"BIT\0" {
> @@ -533,7 +524,7 @@ fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
>  /// PCI Data Structure. It contains some fields that are redundant with the PCI Data Structure, but
>  /// are needed for traversing the BIOS images. It is expected to be present in all BIOS images
>  /// except for NBSI images.
> -#[derive(Debug, Clone)]
> +#[derive(Debug, Clone, FromBytes)]
>  #[repr(C)]
>  struct NpdeStruct {
>      /// 00h: Signature ("NPDE")
> @@ -548,12 +539,9 @@ struct NpdeStruct {
>      last_image: u8,
>  }
> 
> -// SAFETY: all bit patterns are valid for `NpdeStruct`.
> -unsafe impl FromBytes for NpdeStruct {}
> -
>  impl NpdeStruct {
>      fn new(dev: &device::Device, data: &[u8]) -> Option<Self> {
> -        let (npde, _) = NpdeStruct::from_bytes_copy_prefix(data)?;
> +        let (npde, _) = NpdeStruct::read_from_prefix(data).ok()?;
> 
>          // Signature should be "NPDE" (0x4544504E).
>          if &npde.signature != b"NPDE" {
> @@ -845,6 +833,7 @@ fn new(data: &[u8]) -> Result<Self> {
>  }
> 
>  #[repr(C)]
> +#[derive(FromBytes)]
>  struct PmuLookupTableHeader {
>      version: u8,
>      header_len: u8,
> @@ -852,9 +841,6 @@ struct PmuLookupTableHeader {
>      entry_count: u8,
>  }
> 
> -// SAFETY: all bit patterns are valid for `PmuLookupTableHeader`.
> -unsafe impl FromBytes for PmuLookupTableHeader {}
> -
>  /// The [`PmuLookupTableEntry`] structure is used to find the [`PmuLookupTableEntry`] for a given
>  /// application ID.
>  ///
> @@ -867,7 +853,7 @@ struct PmuLookupTable {
> 
>  impl PmuLookupTable {
>      fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
> -        let (header, _) = PmuLookupTableHeader::from_bytes_copy_prefix(data).ok_or(EINVAL)?;
> +        let (header, _) = PmuLookupTableHeader::read_from_prefix(data).map_err(|_| EINVAL)?;
> 
>          let header_len = usize::from(header.header_len);
>          let entry_len = usize::from(header.entry_len);
> @@ -1013,15 +999,11 @@ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
>          let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
>          match ver {
>              2 => {
> -                let v2 = FalconUCodeDescV2::read_from_prefix(data)
> -                    .map_err(|_| EINVAL)?
> -                    .0;
> +                let (v2, _) = FalconUCodeDescV2::read_from_prefix(data).map_err(|_| EINVAL)?;
>                  Ok(FalconUCodeDesc::V2(v2))
>              }
>              3 => {
> -                let v3 = FalconUCodeDescV3::from_bytes_copy_prefix(data)
> -                    .ok_or(EINVAL)?
> -                    .0;
> +                let (v3, _) = FalconUCodeDescV3::read_from_prefix(data).map_err(|_| EINVAL)?;
>                  Ok(FalconUCodeDesc::V3(v3))
>              }
>              _ => {
> --
> 2.47.3
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] gpu: nova-core: parse structs via zerocopy
  2026-06-22  7:45 ` Alistair Popple
@ 2026-06-22 20:26   ` Nicolás Antinori
  2026-06-22 21:02     ` Danilo Krummrich
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolás Antinori @ 2026-06-22 20:26 UTC (permalink / raw)
  To: Alistair Popple
  Cc: Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alexandre Courbot,
	David Airlie, Shuah Khan, Simona Vetter, Gary Guo,
	Onur Özkan, Tamir Duberstein, Trevor Gross, linux-kernel,
	linux-kernel-mentees, dri-devel, rust-for-linux, nova-gpu

Hello Alistair,

On Mon Jun 22, 2026 at 4:45 AM -03, Alistair Popple wrote:
> On 2026-06-22 at 00:36 +1000, Nicolás Antinori <nico.antinori.7@gmail.com> wrote...
>> Replace the unsafe `kernel::transmute::FromBytes` trait implementation
>> for the `FalconUCodeDescV3`, `PcirStruct`, `BitHeader`, `NpdeStruct`,
>> and `PmuLookupTableHeader` structs with the derivable
>> `zerocopy::FromBytes` trait.
>> 
>> This change eliminates the manual unsafe implementations in favor of a
>> derivable trait. When this trait is derived, validity checks are
>> performed at compile time to make sure that the type can safely
>> implement `FromBytes`.
>
> Nice, this looks good although due to some code movement this doesn't compile
> when applied to drm-rust-next[1]. Would you mind rebasing on top of that? The
> changes required should be relatively straight forward, they are mostly the
> result of some new users of transmute::FromBytes being added to vbios.rs.

I did my work on rust-next [1] because drm-rust-next does not have the
zerocopy crate present yet [2]. linux-next contains both zerocopy [3] 
and the new users of transmute::FromBytes if I am not mistaken (BitToken,
PciRomHeader, and PmuLookupTableEntry), so I can make the changes there.

I got confused because the issue was originally in the rust-for-linux 
tree. Since zerocopy wasn't in drm-rust-next yet, I wrongly assumed the 
work should happen there and that drm-rust-next would eventually pull 
them in.

I am fairly new to kernel development, I apologize for the mix-up.

>
> Also what is you plan here? I have also been looking at some of these
> conversions, specifically those involving the generated bindings. Really
> appreciate your contributions, so no problem if you want to continue with some
> of the conversions, just want to make sure we aren't duplicating effort here.

I would be happy to handle the remaining conversions! I can send a v2 patch
that includes those as well.

Thank you

[1] https://github.com/Rust-for-Linux/linux/tree/rust-next/rust
[2] https://gitlab.freedesktop.org/drm/rust/kernel/-/tree/drm-rust-next/rust
[3] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/rust

>
>  - Alistair
>
> [1] - https://gitlab.freedesktop.org/drm/rust/kernel/-/tree/drm-rust-next
>
>> Link: https://github.com/Rust-for-Linux/linux/issues/1241
>> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
>> Signed-off-by: Nicolás Antinori <nico.antinori.7@gmail.com>
>> ---
>> NOTE: Compile-tested only. I don't own a piece of hardware where I can
>> test the nova driver.
>> 
>>  drivers/gpu/nova-core/firmware.rs |  6 +----
>>  drivers/gpu/nova-core/vbios.rs    | 38 ++++++++-----------------------
>>  2 files changed, 11 insertions(+), 33 deletions(-)
>> 
>> diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
>> index ad37994ac15a..0afd1d3fe5ad 100644
>> --- a/drivers/gpu/nova-core/firmware.rs
>> +++ b/drivers/gpu/nova-core/firmware.rs
>> @@ -86,7 +86,7 @@ pub(crate) struct FalconUCodeDescV2 {
>> 
>>  /// Structure used to describe some firmwares, notably FWSEC-FRTS.
>>  #[repr(C)]
>> -#[derive(Debug, Clone)]
>> +#[derive(Debug, Clone, FromBytes)]
>>  pub(crate) struct FalconUCodeDescV3 {
>>      /// Header defined by `NV_BIT_FALCON_UCODE_DESC_HEADER_VDESC*` in OpenRM.
>>      hdr: u32,
>> @@ -117,10 +117,6 @@ pub(crate) struct FalconUCodeDescV3 {
>>      _reserved: u16,
>>  }
>> 
>> -// SAFETY: all bit patterns are valid for this type, and it doesn't use
>> -// interior mutability.
>> -unsafe impl FromBytes for FalconUCodeDescV3 {}
>> -
>>  /// Enum wrapping the different versions of Falcon microcode descriptors.
>>  ///
>>  /// This allows handling both V2 and V3 descriptor formats through a
>> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
>> index 8b7d17a24660..754812bcbdde 100644
>> --- a/drivers/gpu/nova-core/vbios.rs
>> +++ b/drivers/gpu/nova-core/vbios.rs
>> @@ -13,11 +13,8 @@
>>          Alignment, //
>>      },
>>      sync::aref::ARef,
>> -    transmute::FromBytes,
>>  };
>> 
>> -use zerocopy::FromBytes as _;
>> -
>>  use crate::{
>>      driver::Bar0,
>>      firmware::{
>> @@ -301,7 +298,7 @@ pub(crate) fn fwsec_image(&self) -> &FwSecBiosImage {
>>  }
>> 
>>  /// PCI Data Structure as defined in PCI Firmware Specification
>> -#[derive(Debug, Clone)]
>> +#[derive(Debug, Clone, FromBytes)]
>>  #[repr(C)]
>>  struct PcirStruct {
>>      /// PCI Data Structure signature ("PCIR" or "NPDS")
>> @@ -330,12 +327,9 @@ struct PcirStruct {
>>      max_runtime_image_len: u16,
>>  }
>> 
>> -// SAFETY: all bit patterns are valid for `PcirStruct`.
>> -unsafe impl FromBytes for PcirStruct {}
>> -
>>  impl PcirStruct {
>>      fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
>> -        let (pcir, _) = PcirStruct::from_bytes_copy_prefix(data).ok_or(EINVAL)?;
>> +        let (pcir, _) = PcirStruct::read_from_prefix(data).map_err(|_| EINVAL)?;
>> 
>>          // Signature should be "PCIR" (0x52494350) or "NPDS" (0x5344504e).
>>          if &pcir.signature != b"PCIR" && &pcir.signature != b"NPDS" {
>> @@ -371,7 +365,7 @@ fn image_size_bytes(&self) -> usize {
>>  /// This is the head of the BIT table, that is used to locate the Falcon data. The BIT table (with
>>  /// its header) is in the [`PciAtBiosImage`] and the falcon data it is pointing to is in the
>>  /// [`FwSecBiosImage`].
>> -#[derive(Debug, Clone, Copy)]
>> +#[derive(Debug, Clone, Copy, FromBytes)]
>>  #[repr(C)]
>>  struct BitHeader {
>>      /// 0h: BIT Header Identifier (BMP=0x7FFF/BIT=0xB8FF)
>> @@ -390,12 +384,9 @@ struct BitHeader {
>>      checksum: u8,
>>  }
>> 
>> -// SAFETY: all bit patterns are valid for `BitHeader`.
>> -unsafe impl FromBytes for BitHeader {}
>> -
>>  impl BitHeader {
>>      fn new(data: &[u8]) -> Result<Self> {
>> -        let (header, _) = BitHeader::from_bytes_copy_prefix(data).ok_or(EINVAL)?;
>> +        let (header, _) = BitHeader::read_from_prefix(data).map_err(|_| EINVAL)?;
>> 
>>          // Check header ID and signature
>>          if header.id != 0xB8FF || &header.signature != b"BIT\0" {
>> @@ -533,7 +524,7 @@ fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
>>  /// PCI Data Structure. It contains some fields that are redundant with the PCI Data Structure, but
>>  /// are needed for traversing the BIOS images. It is expected to be present in all BIOS images
>>  /// except for NBSI images.
>> -#[derive(Debug, Clone)]
>> +#[derive(Debug, Clone, FromBytes)]
>>  #[repr(C)]
>>  struct NpdeStruct {
>>      /// 00h: Signature ("NPDE")
>> @@ -548,12 +539,9 @@ struct NpdeStruct {
>>      last_image: u8,
>>  }
>> 
>> -// SAFETY: all bit patterns are valid for `NpdeStruct`.
>> -unsafe impl FromBytes for NpdeStruct {}
>> -
>>  impl NpdeStruct {
>>      fn new(dev: &device::Device, data: &[u8]) -> Option<Self> {
>> -        let (npde, _) = NpdeStruct::from_bytes_copy_prefix(data)?;
>> +        let (npde, _) = NpdeStruct::read_from_prefix(data).ok()?;
>> 
>>          // Signature should be "NPDE" (0x4544504E).
>>          if &npde.signature != b"NPDE" {
>> @@ -845,6 +833,7 @@ fn new(data: &[u8]) -> Result<Self> {
>>  }
>> 
>>  #[repr(C)]
>> +#[derive(FromBytes)]
>>  struct PmuLookupTableHeader {
>>      version: u8,
>>      header_len: u8,
>> @@ -852,9 +841,6 @@ struct PmuLookupTableHeader {
>>      entry_count: u8,
>>  }
>> 
>> -// SAFETY: all bit patterns are valid for `PmuLookupTableHeader`.
>> -unsafe impl FromBytes for PmuLookupTableHeader {}
>> -
>>  /// The [`PmuLookupTableEntry`] structure is used to find the [`PmuLookupTableEntry`] for a given
>>  /// application ID.
>>  ///
>> @@ -867,7 +853,7 @@ struct PmuLookupTable {
>> 
>>  impl PmuLookupTable {
>>      fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
>> -        let (header, _) = PmuLookupTableHeader::from_bytes_copy_prefix(data).ok_or(EINVAL)?;
>> +        let (header, _) = PmuLookupTableHeader::read_from_prefix(data).map_err(|_| EINVAL)?;
>> 
>>          let header_len = usize::from(header.header_len);
>>          let entry_len = usize::from(header.entry_len);
>> @@ -1013,15 +999,11 @@ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
>>          let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
>>          match ver {
>>              2 => {
>> -                let v2 = FalconUCodeDescV2::read_from_prefix(data)
>> -                    .map_err(|_| EINVAL)?
>> -                    .0;
>> +                let (v2, _) = FalconUCodeDescV2::read_from_prefix(data).map_err(|_| EINVAL)?;
>>                  Ok(FalconUCodeDesc::V2(v2))
>>              }
>>              3 => {
>> -                let v3 = FalconUCodeDescV3::from_bytes_copy_prefix(data)
>> -                    .ok_or(EINVAL)?
>> -                    .0;
>> +                let (v3, _) = FalconUCodeDescV3::read_from_prefix(data).map_err(|_| EINVAL)?;
>>                  Ok(FalconUCodeDesc::V3(v3))
>>              }
>>              _ => {
>> --
>> 2.47.3
>> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] gpu: nova-core: parse structs via zerocopy
  2026-06-22 20:26   ` Nicolás Antinori
@ 2026-06-22 21:02     ` Danilo Krummrich
  0 siblings, 0 replies; 4+ messages in thread
From: Danilo Krummrich @ 2026-06-22 21:02 UTC (permalink / raw)
  To: Nicolás Antinori
  Cc: Alistair Popple, Alice Ryhl, Miguel Ojeda, Alexandre Courbot,
	David Airlie, Shuah Khan, Simona Vetter, Gary Guo,
	Onur Özkan, Tamir Duberstein, Trevor Gross, linux-kernel,
	linux-kernel-mentees, dri-devel, rust-for-linux, nova-gpu

On Mon Jun 22, 2026 at 10:26 PM CEST, Nicolás Antinori wrote:
> I did my work on rust-next [1] because drm-rust-next does not have the
> zerocopy crate present yet [2]. linux-next contains both zerocopy [3] 
> and the new users of transmute::FromBytes if I am not mistaken (BitToken,
> PciRomHeader, and PmuLookupTableEntry), so I can make the changes there.

The drm-rust tree is a bit special, as it remains open for contributions even
after it has been tagged for inclusion into Linus's (including throughout the
merge window). However, all changes staged in drm-rust-next are not going into
linux-next until -rc1 is released.

IOW, until -rc1 is released this may or may not resolve all conflicts with
drm-rust-next. Once -rc1 is released, it is backmerged into drm-rust-next and
drm-rust-next is picked up by linux-next again.

Usually all of this remains rather transparent to contributors, but you hit the
case of using a new feature introduced through another tree before drm-rust-next
caught up with Linus's tree (which will happen next Sunday).

Before drm-rust-next caught up, this patch can't be applied anyways, so all
good.

> I am fairly new to kernel development, I apologize for the mix-up.

No worries, you did nothing wrong; thanks for the contribution!

- Danilo

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-22 21:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-21 14:36 [PATCH] gpu: nova-core: parse structs via zerocopy Nicolás Antinori
2026-06-22  7:45 ` Alistair Popple
2026-06-22 20:26   ` Nicolás Antinori
2026-06-22 21:02     ` Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox