Daniel del Castillo
2025-Oct-15 19:50 UTC
[PATCH 1/2] nova-core: Solve mentions of `CoherentAllocation` improvements [COHA]
This patch solves the existing mentions of COHA, a task
in the Nova task list about improving the `CoherentAllocation` API.
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 the existing references to [COHA].
Signed-off-by: Daniel del Castillo <delcastillodelarosadaniel at
gmail.com>
---
drivers/gpu/nova-core/dma.rs | 20 ++---
drivers/gpu/nova-core/firmware/fwsec.rs | 104 ++++++++++--------------
2 files changed, 50 insertions(+), 74 deletions(-)
diff --git a/drivers/gpu/nova-core/dma.rs b/drivers/gpu/nova-core/dma.rs
index 94f44bcfd748..639a99cf72c4 100644
--- a/drivers/gpu/nova-core/dma.rs
+++ b/drivers/gpu/nova-core/dma.rs
@@ -25,21 +25,11 @@ pub(crate) fn new(dev:
&device::Device<device::Bound>, len: usize) -> Result<Sel
}
pub(crate) fn from_data(dev: &device::Device<device::Bound>,
data: &[u8]) -> Result<Self> {
- Self::new(dev, data.len()).map(|mut dma_obj| {
- // TODO[COHA]: replace with `CoherentAllocation::write()` once
available.
- // SAFETY:
- // - `dma_obj`'s size is at least `data.len()`.
- // - We have just created this object and there is no other user at
this stage.
- unsafe {
- core::ptr::copy_nonoverlapping(
- data.as_ptr(),
- dma_obj.dma.start_ptr_mut(),
- data.len(),
- );
- }
-
- dma_obj
- })
+ let mut dma_obj = Self::new(dev, data.len())?;
+ // SAFETY: We have just created this object and there is no other user
at this stage.
+ unsafe { dma_obj.write(data, 0)? };
+
+ Ok(dma_obj)
}
}
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs
b/drivers/gpu/nova-core/firmware/fwsec.rs
index 8edbb5c0572c..cc5a6200d0de 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;
@@ -70,6 +70,8 @@ struct FalconAppifDmemmapperV3 {
}
// SAFETY: any byte sequence is valid for this struct.
unsafe impl FromBytes for FalconAppifDmemmapperV3 {}
+// SAFETY: any byte sequence is valid and it contains no padding nor kernel
pointers.
+unsafe impl AsBytes for FalconAppifDmemmapperV3 {}
#[derive(Debug)]
#[repr(C, packed)]
@@ -82,6 +84,8 @@ struct ReadVbios {
}
// SAFETY: any byte sequence is valid for this struct.
unsafe impl FromBytes for ReadVbios {}
+// SAFETY: any byte sequence is valid and it contains no padding nor kernel
pointers.
+unsafe impl AsBytes for ReadVbios {}
#[derive(Debug)]
#[repr(C, packed)]
@@ -94,6 +98,8 @@ struct FrtsRegion {
}
// SAFETY: any byte sequence is valid for this struct.
unsafe impl FromBytes for FrtsRegion {}
+// SAFETY: any byte sequence is valid and it contains no padding nor kernel
pointers.
+unsafe impl AsBytes for FrtsRegion {}
const NVFW_FRTS_CMD_REGION_TYPE_FB: u32 = 2;
@@ -104,6 +110,8 @@ struct FrtsCmd {
}
// SAFETY: any byte sequence is valid for this struct.
unsafe impl FromBytes for FrtsCmd {}
+// SAFETY: any byte sequence is valid and it contains no padding nor kernel
pointers.
+unsafe impl AsBytes for FrtsCmd {}
const NVFW_FALCON_APPIF_DMEMMAPPER_CMD_FRTS: u32 = 0x15;
const NVFW_FALCON_APPIF_DMEMMAPPER_CMD_SB: u32 = 0x19;
@@ -149,24 +157,9 @@ impl FirmwareSignature<FwsecFirmware> for
Bcrt30Rsa3kSignature {}
///
/// Callers must ensure that the region of memory returned is not written for
as long as the
/// returned reference is alive.
-///
-/// TODO[TRSM][COHA]: Remove this and `transmute_mut` once
`CoherentAllocation::as_slice` is
-/// available and we have a way to transmute objects implementing FromBytes,
e.g.:
-/// https://lore.kernel.org/lkml/20250330234039.29814-1-christiansantoslima21
at gmail.com/
-unsafe fn transmute<'a, 'b, T: Sized + FromBytes>(
- fw: &'a DmaObject,
- offset: usize,
-) -> Result<&'b T> {
- if offset + size_of::<T>() > fw.size() {
- return Err(EINVAL);
- }
- if (fw.start_ptr() as usize + offset) % align_of::<T>() != 0 {
- return Err(EINVAL);
- }
-
- // SAFETY: we have checked that the pointer is properly aligned that its
pointed memory is
- // large enough the contains an instance of `T`, which implements
`FromBytes`.
- Ok(unsafe { &*(fw.start_ptr().add(offset).cast::<T>()) })
+unsafe fn transmute<T: Sized + FromBytes>(fw: &DmaObject, offset:
usize) -> Result<&T> {
+ // SAFETY: Guaranteed by the safety requirements of the function.
+ T::from_bytes(unsafe { fw.as_slice(offset, size_of::<T>())?
}).ok_or(EINVAL)
}
/// Reinterpret the area starting from `offset` in `fw` as a mutable instance
of `T` (which must
@@ -176,20 +169,12 @@ unsafe fn transmute<'a, 'b, T: Sized +
FromBytes>(
///
/// Callers must ensure that the region of memory returned is not read or
written for as long as
/// the returned reference is alive.
-unsafe fn transmute_mut<'a, 'b, T: Sized + FromBytes>(
- fw: &'a mut DmaObject,
+unsafe fn transmute_mut<T: Sized + FromBytes + AsBytes>(
+ fw: &mut DmaObject,
offset: usize,
-) -> Result<&'b mut T> {
- if offset + size_of::<T>() > fw.size() {
- return Err(EINVAL);
- }
- if (fw.start_ptr_mut() as usize + offset) % align_of::<T>() != 0 {
- return Err(EINVAL);
- }
-
- // SAFETY: we have checked that the pointer is properly aligned that its
pointed memory is
- // large enough the contains an instance of `T`, which implements
`FromBytes`.
- Ok(unsafe { &mut *(fw.start_ptr_mut().add(offset).cast::<T>()) })
+) -> Result<&mut T> {
+ // SAFETY: Guaranteed by the safety requirements of the function.
+ T::from_bytes_mut(unsafe { fw.as_slice_mut(offset, size_of::<T>())?
}).ok_or(EINVAL)
}
/// The FWSEC microcode, extracted from the BIOS and to be run on the GSP
falcon.
@@ -260,32 +245,38 @@ fn new_fwsec(dev: &Device<device::Bound>, bios:
&Vbios, cmd: FwsecCommand) -> Re
// Find the DMEM mapper section in the firmware.
for i in 0..hdr.entry_count as usize {
- let app: &FalconAppifV1 // SAFETY: we have
exclusive access to `dma_object`.
- unsafe {
+ let app: &FalconAppifV1 = unsafe {
transmute(
&dma_object,
- hdr_offset + hdr.header_size as usize + i * hdr.entry_size
as usize
+ hdr_offset + hdr.header_size as usize + i * hdr.entry_size
as usize,
)
}?;
if app.id != NVFW_FALCON_APPIF_ID_DMEMMAPPER {
continue;
}
+ let dmem_base = app.dmem_base;
// SAFETY: we have exclusive access to `dma_object`.
let dmem_mapper: &mut FalconAppifDmemmapperV3 = unsafe {
- transmute_mut(
- &mut dma_object,
- (desc.imem_load_size + app.dmem_base) as usize,
- )
+ transmute_mut(&mut dma_object, (desc.imem_load_size +
dmem_base) as usize)
}?;
+ dmem_mapper.init_cmd = match cmd {
+ FwsecCommand::Frts {
+ frts_addr: _,
+ frts_size: _,
+ } => NVFW_FALCON_APPIF_DMEMMAPPER_CMD_FRTS,
+ FwsecCommand::Sb => NVFW_FALCON_APPIF_DMEMMAPPER_CMD_SB,
+ };
+ let cmd_in_buffer_offset = dmem_mapper.cmd_in_buffer_offset;
+
// SAFETY: we have exclusive access to `dma_object`.
let frts_cmd: &mut FrtsCmd = unsafe {
transmute_mut(
&mut dma_object,
- (desc.imem_load_size + dmem_mapper.cmd_in_buffer_offset) as
usize,
+ (desc.imem_load_size + cmd_in_buffer_offset) as usize,
)
}?;
@@ -296,24 +287,19 @@ fn new_fwsec(dev: &Device<device::Bound>, bios:
&Vbios, cmd: FwsecCommand) -> Re
size: 0,
flags: 2,
};
-
- dmem_mapper.init_cmd = match cmd {
- FwsecCommand::Frts {
- frts_addr,
- frts_size,
- } => {
- frts_cmd.frts_region = FrtsRegion {
- ver: 1,
- hdr: size_of::<FrtsRegion>() as u32,
- addr: (frts_addr >> 12) as u32,
- size: (frts_size >> 12) as u32,
- ftype: NVFW_FRTS_CMD_REGION_TYPE_FB,
- };
-
- NVFW_FALCON_APPIF_DMEMMAPPER_CMD_FRTS
- }
- FwsecCommand::Sb => NVFW_FALCON_APPIF_DMEMMAPPER_CMD_SB,
- };
+ if let FwsecCommand::Frts {
+ frts_addr,
+ frts_size,
+ } = cmd
+ {
+ frts_cmd.frts_region = FrtsRegion {
+ ver: 1,
+ hdr: size_of::<FrtsRegion>() as u32,
+ addr: (frts_addr >> 12) as u32,
+ size: (frts_size >> 12) as u32,
+ ftype: NVFW_FRTS_CMD_REGION_TYPE_FB,
+ };
+ }
// Return early as we found and patched the DMEMMAPPER region.
return Ok(Self(dma_object, PhantomData));
--
2.51.0
This small patch updates the nova todo list to remove some tasks that have been solved lately. Signed-off-by: Daniel del Castillo <delcastillodelarosadaniel at gmail.com> --- Documentation/gpu/nova/core/todo.rst | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/Documentation/gpu/nova/core/todo.rst b/Documentation/gpu/nova/core/todo.rst index 48b20656dcb1..be8063030d44 100644 --- a/Documentation/gpu/nova/core/todo.rst +++ b/Documentation/gpu/nova/core/todo.rst @@ -44,25 +44,6 @@ automatically generates the corresponding mappings between a value and a number. | Complexity: Beginner | Link: https://docs.rs/num/latest/num/trait.FromPrimitive.html -Conversion from byte slices for types implementing FromBytes [TRSM] -------------------------------------------------------------------- - -We retrieve several structures from byte streams coming from the BIOS or loaded -firmware. At the moment converting the bytes slice into the proper type require -an inelegant `unsafe` operation; this will go away once `FromBytes` implements -a proper `from_bytes` method. - -| Complexity: Beginner - -CoherentAllocation improvements [COHA] --------------------------------------- - -`CoherentAllocation` needs a safe way to write into the allocation, and to -obtain slices within the allocation. - -| Complexity: Beginner -| Contact: Abdiel Janulgue - Generic register abstraction [REGA] ----------------------------------- -- 2.51.0
Danilo Krummrich
2025-Oct-15 20:04 UTC
[PATCH 1/2] nova-core: Solve mentions of `CoherentAllocation` improvements [COHA]
Hi Daniel, On 10/15/25 9:49 PM, Daniel del Castillo wrote:> This patch solves the existing mentions of COHA, a task > in the Nova task list about improving the `CoherentAllocation` API. > 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 the existing references to [COHA].This patch seems to address two different TODOs. Please split up the patch accordingly.> Signed-off-by: Daniel del Castillo <delcastillodelarosadaniel at gmail.com> > --- > drivers/gpu/nova-core/dma.rs | 20 ++--- > drivers/gpu/nova-core/firmware/fwsec.rs | 104 ++++++++++-------------- > 2 files changed, 50 insertions(+), 74 deletions(-) > > diff --git a/drivers/gpu/nova-core/dma.rs b/drivers/gpu/nova-core/dma.rs > index 94f44bcfd748..639a99cf72c4 100644 > --- a/drivers/gpu/nova-core/dma.rs > +++ b/drivers/gpu/nova-core/dma.rs > @@ -25,21 +25,11 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, len: usize) -> Result<Sel > } > > pub(crate) fn from_data(dev: &device::Device<device::Bound>, data: &[u8]) -> Result<Self> { > - Self::new(dev, data.len()).map(|mut dma_obj| { > - // TODO[COHA]: replace with `CoherentAllocation::write()` once available. > - // SAFETY: > - // - `dma_obj`'s size is at least `data.len()`. > - // - We have just created this object and there is no other user at this stage. > - unsafe { > - core::ptr::copy_nonoverlapping( > - data.as_ptr(), > - dma_obj.dma.start_ptr_mut(), > - data.len(), > - ); > - } > - > - dma_obj > - }) > + let mut dma_obj = Self::new(dev, data.len())?; > + // SAFETY: We have just created this object and there is no other user at this stage.The safety comment should rather confirm that it is guaranteed that the device won't access this memory concurrently.> + unsafe { dma_obj.write(data, 0)? }; > + > + Ok(dma_obj) > } > } > > diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs > index 8edbb5c0572c..cc5a6200d0de 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; > @@ -70,6 +70,8 @@ struct FalconAppifDmemmapperV3 { > } > // SAFETY: any byte sequence is valid for this struct. > unsafe impl FromBytes for FalconAppifDmemmapperV3 {} > +// SAFETY: any byte sequence is valid and it contains no padding nor kernel pointers. > +unsafe impl AsBytes for FalconAppifDmemmapperV3 {} > > #[derive(Debug)] > #[repr(C, packed)] > @@ -82,6 +84,8 @@ struct ReadVbios { > } > // SAFETY: any byte sequence is valid for this struct. > unsafe impl FromBytes for ReadVbios {} > +// SAFETY: any byte sequence is valid and it contains no padding nor kernel pointers.The full safety requirement is "Values of this type may not contain any uninitialized bytes. This type must not have interior mutability. Additional NIT: Please start sentences with a capital letter (Unfortunately the comment above missed this as well). Same for the cases below.> +unsafe impl AsBytes for ReadVbios {} > > #[derive(Debug)] > #[repr(C, packed)] > @@ -94,6 +98,8 @@ struct FrtsRegion { > } > // SAFETY: any byte sequence is valid for this struct. > unsafe impl FromBytes for FrtsRegion {} > +// SAFETY: any byte sequence is valid and it contains no padding nor kernel pointers. > +unsafe impl AsBytes for FrtsRegion {} > > const NVFW_FRTS_CMD_REGION_TYPE_FB: u32 = 2; > > @@ -104,6 +110,8 @@ struct FrtsCmd { > } > // SAFETY: any byte sequence is valid for this struct. > unsafe impl FromBytes for FrtsCmd {} > +// SAFETY: any byte sequence is valid and it contains no padding nor kernel pointers. > +unsafe impl AsBytes for FrtsCmd {} > > const NVFW_FALCON_APPIF_DMEMMAPPER_CMD_FRTS: u32 = 0x15; > const NVFW_FALCON_APPIF_DMEMMAPPER_CMD_SB: u32 = 0x19; > @@ -149,24 +157,9 @@ impl FirmwareSignature<FwsecFirmware> for Bcrt30Rsa3kSignature {} > /// > /// Callers must ensure that the region of memory returned is not written for as long as the > /// returned reference is alive. > -/// > -/// TODO[TRSM][COHA]: Remove this and `transmute_mut` once `CoherentAllocation::as_slice` is > -/// available and we have a way to transmute objects implementing FromBytes, e.g.: > -/// https://lore.kernel.org/lkml/20250330234039.29814-1-christiansantoslima21 at gmail.com/ > -unsafe fn transmute<'a, 'b, T: Sized + FromBytes>( > - fw: &'a DmaObject, > - offset: usize, > -) -> Result<&'b T> { > - if offset + size_of::<T>() > fw.size() { > - return Err(EINVAL); > - } > - if (fw.start_ptr() as usize + offset) % align_of::<T>() != 0 { > - return Err(EINVAL); > - } > - > - // SAFETY: we have checked that the pointer is properly aligned that its pointed memory is > - // large enough the contains an instance of `T`, which implements `FromBytes`. > - Ok(unsafe { &*(fw.start_ptr().add(offset).cast::<T>()) }) > +unsafe fn transmute<T: Sized + FromBytes>(fw: &DmaObject, offset: usize) -> Result<&T> { > + // SAFETY: Guaranteed by the safety requirements of the function.Please mention what is guaranteed and how it is guaranteed by the safety requirements of the function (even if it seems trivial).> + T::from_bytes(unsafe { fw.as_slice(offset, size_of::<T>())? }).ok_or(EINVAL) > } > > /// Reinterpret the area starting from `offset` in `fw` as a mutable instance of `T` (which must > @@ -176,20 +169,12 @@ unsafe fn transmute<'a, 'b, T: Sized + FromBytes>( > /// > /// Callers must ensure that the region of memory returned is not read or written for as long as > /// the returned reference is alive. > -unsafe fn transmute_mut<'a, 'b, T: Sized + FromBytes>( > - fw: &'a mut DmaObject, > +unsafe fn transmute_mut<T: Sized + FromBytes + AsBytes>( > + fw: &mut DmaObject, > offset: usize, > -) -> Result<&'b mut T> { > - if offset + size_of::<T>() > fw.size() { > - return Err(EINVAL); > - } > - if (fw.start_ptr_mut() as usize + offset) % align_of::<T>() != 0 { > - return Err(EINVAL); > - } > - > - // SAFETY: we have checked that the pointer is properly aligned that its pointed memory is > - // large enough the contains an instance of `T`, which implements `FromBytes`. > - Ok(unsafe { &mut *(fw.start_ptr_mut().add(offset).cast::<T>()) }) > +) -> Result<&mut T> { > + // SAFETY: Guaranteed by the safety requirements of the function.Same here.> + T::from_bytes_mut(unsafe { fw.as_slice_mut(offset, size_of::<T>())? }).ok_or(EINVAL) > }
Alexandre Courbot
2025-Oct-17 01:36 UTC
[PATCH 1/2] nova-core: Solve mentions of `CoherentAllocation` improvements [COHA]
On Thu Oct 16, 2025 at 4:49 AM JST, Daniel del Castillo wrote:> This patch solves the existing mentions of COHA, a task > in the Nova task list about improving the `CoherentAllocation` API. > 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 the existing references to [COHA].This mention of background discussions should be in the comment part of your commit (below the `---`).> > Signed-off-by: Daniel del Castillo <delcastillodelarosadaniel at gmail.com> > --- > drivers/gpu/nova-core/dma.rs | 20 ++--- > drivers/gpu/nova-core/firmware/fwsec.rs | 104 ++++++++++-------------- > 2 files changed, 50 insertions(+), 74 deletions(-) > > diff --git a/drivers/gpu/nova-core/dma.rs b/drivers/gpu/nova-core/dma.rs > index 94f44bcfd748..639a99cf72c4 100644 > --- a/drivers/gpu/nova-core/dma.rs > +++ b/drivers/gpu/nova-core/dma.rs > @@ -25,21 +25,11 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, len: usize) -> Result<Sel > } > > pub(crate) fn from_data(dev: &device::Device<device::Bound>, data: &[u8]) -> Result<Self> { > - Self::new(dev, data.len()).map(|mut dma_obj| { > - // TODO[COHA]: replace with `CoherentAllocation::write()` once available. > - // SAFETY: > - // - `dma_obj`'s size is at least `data.len()`. > - // - We have just created this object and there is no other user at this stage. > - unsafe { > - core::ptr::copy_nonoverlapping( > - data.as_ptr(), > - dma_obj.dma.start_ptr_mut(), > - data.len(), > - ); > - } > - > - dma_obj > - }) > + let mut dma_obj = Self::new(dev, data.len())?; > + // SAFETY: We have just created this object and there is no other user at this stage. > + unsafe { dma_obj.write(data, 0)? }; > + > + Ok(dma_obj)Can you preserve the use of `map`? This removes the need for the temporary variable. <snip>> /// The FWSEC microcode, extracted from the BIOS and to be run on the GSP falcon. > @@ -260,32 +245,38 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re > > // Find the DMEM mapper section in the firmware. > for i in 0..hdr.entry_count as usize { > - let app: &FalconAppifV1 > // SAFETY: we have exclusive access to `dma_object`. > - unsafe { > + let app: &FalconAppifV1 = unsafe { > transmute( > &dma_object, > - hdr_offset + hdr.header_size as usize + i * hdr.entry_size as usize > + hdr_offset + hdr.header_size as usize + i * hdr.entry_size as usize, > ) > }?; > > if app.id != NVFW_FALCON_APPIF_ID_DMEMMAPPER { > continue; > } > + let dmem_base = app.dmem_base; > > // SAFETY: we have exclusive access to `dma_object`. > let dmem_mapper: &mut FalconAppifDmemmapperV3 = unsafe { > - transmute_mut( > - &mut dma_object, > - (desc.imem_load_size + app.dmem_base) as usize, > - ) > + transmute_mut(&mut dma_object, (desc.imem_load_size + dmem_base) as usize) > }?; > > + dmem_mapper.init_cmd = match cmd { > + FwsecCommand::Frts { > + frts_addr: _, > + frts_size: _,Can the `{ .. }` syntax be used here?> + } => NVFW_FALCON_APPIF_DMEMMAPPER_CMD_FRTS, > + FwsecCommand::Sb => NVFW_FALCON_APPIF_DMEMMAPPER_CMD_SB, > + }; > + let cmd_in_buffer_offset = dmem_mapper.cmd_in_buffer_offset; > + > // SAFETY: we have exclusive access to `dma_object`. > let frts_cmd: &mut FrtsCmd = unsafe { > transmute_mut( > &mut dma_object, > - (desc.imem_load_size + dmem_mapper.cmd_in_buffer_offset) as usize, > + (desc.imem_load_size + cmd_in_buffer_offset) as usize, > ) > }?; > > @@ -296,24 +287,19 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re > size: 0, > flags: 2, > }; > - > - dmem_mapper.init_cmd = match cmd { > - FwsecCommand::Frts { > - frts_addr, > - frts_size, > - } => { > - frts_cmd.frts_region = FrtsRegion { > - ver: 1, > - hdr: size_of::<FrtsRegion>() as u32, > - addr: (frts_addr >> 12) as u32, > - size: (frts_size >> 12) as u32, > - ftype: NVFW_FRTS_CMD_REGION_TYPE_FB, > - }; > - > - NVFW_FALCON_APPIF_DMEMMAPPER_CMD_FRTS > - } > - FwsecCommand::Sb => NVFW_FALCON_APPIF_DMEMMAPPER_CMD_SB, > - }; > + if let FwsecCommand::Frts { > + frts_addr, > + frts_size, > + } = cmd > + { > + frts_cmd.frts_region = FrtsRegion { > + ver: 1, > + hdr: size_of::<FrtsRegion>() as u32, > + addr: (frts_addr >> 12) as u32, > + size: (frts_size >> 12) as u32, > + ftype: NVFW_FRTS_CMD_REGION_TYPE_FB, > + }; > + }I liked that the original code updated both `init_cmd` and `frts_region` in the same match block. I understand it might be difficult to preserve due to the borrowing rules, but can you try to preserve it if that's possible at all?