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 <[email protected]> Signed-off-by: Nicolás Antinori <[email protected]> --- 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
