[PATCH 0/9] gpu: nova-core: miscellaneous improvements

[PATCH 0/9] gpu: nova-core: miscellaneous improvements

[Alexandre Courbot]

A few simple, loosely-related small improvements for nova-core,
including reporting unprocessed data in GSP messages, removal of
unnecessary code in GSP and the sequencer, and leveraging the Zeroable
derive macro and core library's CStr. Probably nothing too
controversial.

This series is based on the fixup patch series for this cycle [1].

[1] https://lore.kernel.org/all/20251123-nova-fixes-v2-0-33d86092cf6a@nvidia.com/

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Alexandre Courbot (9):
      gpu: nova-core: gsp: warn if data remains after processing a message
      gpu: nova-core: gsp: remove unnecessary Display impls
      gpu: nova-core: gsp: simplify sequencer opcode parsing
      gpu: nova-core: gsp: remove unneeded sequencer trait
      gpu: nova-core: gsp: derive `Debug` on more sequencer types
      gpu: nova-core: gsp: derive Zeroable for GspStaticConfigInfo
      gpu: nova-core: firmware: fwsec: do not require bound device when unneeded
      gpu: nova-core: use core library's CStr instead of kernel one
      gpu: nova-core: simplify str_from_null_terminated

 drivers/gpu/nova-core/firmware.rs        |   2 +-
 drivers/gpu/nova-core/firmware/fwsec.rs  |   7 +--
 drivers/gpu/nova-core/firmware/gsp.rs    |   6 +-
 drivers/gpu/nova-core/gsp/cmdq.rs        |  14 ++++-
 drivers/gpu/nova-core/gsp/fw.rs          | 104 +++----------------------------
 drivers/gpu/nova-core/gsp/fw/commands.rs |  11 ++--
 drivers/gpu/nova-core/gsp/sequencer.rs   |  18 +++---
 drivers/gpu/nova-core/nova_core.rs       |   2 +-
 drivers/gpu/nova-core/util.rs            |  11 +---
 9 files changed, 44 insertions(+), 131 deletions(-)
---
base-commit: 449c67daceeda195c0553ca890d6944a054ff4d8
change-id: 20251208-nova-misc-1d797b5d64f2

Best regards,
-- 
Alexandre Courbot <acourbot@nvidia.com>

[PATCH 1/9] gpu: nova-core: gsp: warn if data remains after processing a message

[Alexandre Courbot]

Not processing the whole data from a received message is a strong
indicator of a bug - emit a warning when such cases are detected.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index dab73377c526..5ce85ee1ffce 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -662,7 +662,17 @@ pub(crate) fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Resul
             let (cmd, contents_1) = M::Message::from_bytes_prefix(message.contents.0).ok_or(EIO)?;
             let mut sbuffer = SBufferIter::new_reader([contents_1, message.contents.1]);
 
-            M::read(cmd, &mut sbuffer).map_err(|e| e.into())
+            let res = M::read(cmd, &mut sbuffer).map_err(|e| e.into());
+
+            if res.is_ok() && !sbuffer.is_empty() {
+                dev_warn!(
+                    &self.dev,
+                    "GSP message {:?} has unprocessed data\n",
+                    function
+                );
+            }
+
+            res
         } else {
             Err(ERANGE)
         };

-- 
2.52.0

[PATCH 2/9] gpu: nova-core: gsp: remove unnecessary Display impls

[Alexandre Courbot]

We only ever display these in debug context, for which the automatically
derived `Debug` impls work just fine - so use them and remove these
boilerplate-looking implementations.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs |  2 +-
 drivers/gpu/nova-core/gsp/fw.rs   | 54 ---------------------------------------
 2 files changed, 1 insertion(+), 55 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 5ce85ee1ffce..fa983a3f480c 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -531,7 +531,7 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
 
         dev_dbg!(
             &self.dev,
-            "GSP RPC: send: seq# {}, function={}, length=0x{:x}\n",
+            "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
             self.seq,
             M::FUNCTION,
             dst.header.length(),
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 3baa5455cc32..24e4eaaf1265 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -10,7 +10,6 @@
 
 use kernel::{
     dma::CoherentAllocation,
-    fmt,
     prelude::*,
     ptr::{
         Alignable,
@@ -223,43 +222,6 @@ pub(crate) enum MsgFunction {
     UcodeLibOsPrint = bindings::NV_VGPU_MSG_EVENT_UCODE_LIBOS_PRINT,
 }
 
-impl fmt::Display for MsgFunction {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            // Common function codes
-            MsgFunction::Nop => write!(f, "NOP"),
-            MsgFunction::SetGuestSystemInfo => write!(f, "SET_GUEST_SYSTEM_INFO"),
-            MsgFunction::AllocRoot => write!(f, "ALLOC_ROOT"),
-            MsgFunction::AllocDevice => write!(f, "ALLOC_DEVICE"),
-            MsgFunction::AllocMemory => write!(f, "ALLOC_MEMORY"),
-            MsgFunction::AllocCtxDma => write!(f, "ALLOC_CTX_DMA"),
-            MsgFunction::AllocChannelDma => write!(f, "ALLOC_CHANNEL_DMA"),
-            MsgFunction::MapMemory => write!(f, "MAP_MEMORY"),
-            MsgFunction::BindCtxDma => write!(f, "BIND_CTX_DMA"),
-            MsgFunction::AllocObject => write!(f, "ALLOC_OBJECT"),
-            MsgFunction::Free => write!(f, "FREE"),
-            MsgFunction::Log => write!(f, "LOG"),
-            MsgFunction::GetGspStaticInfo => write!(f, "GET_GSP_STATIC_INFO"),
-            MsgFunction::SetRegistry => write!(f, "SET_REGISTRY"),
-            MsgFunction::GspSetSystemInfo => write!(f, "GSP_SET_SYSTEM_INFO"),
-            MsgFunction::GspInitPostObjGpu => write!(f, "GSP_INIT_POST_OBJGPU"),
-            MsgFunction::GspRmControl => write!(f, "GSP_RM_CONTROL"),
-            MsgFunction::GetStaticInfo => write!(f, "GET_STATIC_INFO"),
-
-            // Event codes
-            MsgFunction::GspInitDone => write!(f, "INIT_DONE"),
-            MsgFunction::GspRunCpuSequencer => write!(f, "RUN_CPU_SEQUENCER"),
-            MsgFunction::PostEvent => write!(f, "POST_EVENT"),
-            MsgFunction::RcTriggered => write!(f, "RC_TRIGGERED"),
-            MsgFunction::MmuFaultQueued => write!(f, "MMU_FAULT_QUEUED"),
-            MsgFunction::OsErrorLog => write!(f, "OS_ERROR_LOG"),
-            MsgFunction::GspPostNoCat => write!(f, "NOCAT"),
-            MsgFunction::GspLockdownNotice => write!(f, "LOCKDOWN_NOTICE"),
-            MsgFunction::UcodeLibOsPrint => write!(f, "LIBOS_PRINT"),
-        }
-    }
-}
-
 impl TryFrom<u32> for MsgFunction {
     type Error = kernel::error::Error;
 
@@ -330,22 +292,6 @@ pub(crate) enum SeqBufOpcode {
     RegWrite = bindings::GSP_SEQ_BUF_OPCODE_GSP_SEQ_BUF_OPCODE_REG_WRITE,
 }
 
-impl fmt::Display for SeqBufOpcode {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            SeqBufOpcode::CoreReset => write!(f, "CORE_RESET"),
-            SeqBufOpcode::CoreResume => write!(f, "CORE_RESUME"),
-            SeqBufOpcode::CoreStart => write!(f, "CORE_START"),
-            SeqBufOpcode::CoreWaitForHalt => write!(f, "CORE_WAIT_FOR_HALT"),
-            SeqBufOpcode::DelayUs => write!(f, "DELAY_US"),
-            SeqBufOpcode::RegModify => write!(f, "REG_MODIFY"),
-            SeqBufOpcode::RegPoll => write!(f, "REG_POLL"),
-            SeqBufOpcode::RegStore => write!(f, "REG_STORE"),
-            SeqBufOpcode::RegWrite => write!(f, "REG_WRITE"),
-        }
-    }
-}
-
 impl TryFrom<u32> for SeqBufOpcode {
     type Error = kernel::error::Error;
 

-- 
2.52.0

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

[Alexandre Courbot]

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@nvidia.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 }))
     }
 }
 

-- 
2.52.0

[PATCH 4/9] gpu: nova-core: gsp: remove unneeded sequencer trait

[Alexandre Courbot]

The `GspSeqCmdRunner` trait is never used as we never call the `run`
methods from generic code. Remove it.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/sequencer.rs | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs
index 2d0369c49092..4efa048b9d93 100644
--- a/drivers/gpu/nova-core/gsp/sequencer.rs
+++ b/drivers/gpu/nova-core/gsp/sequencer.rs
@@ -147,12 +147,7 @@ pub(crate) struct GspSequencer<'a> {
     dev: ARef<device::Device>,
 }
 
-/// Trait for running sequencer commands.
-pub(crate) trait GspSeqCmdRunner {
-    fn run(&self, sequencer: &GspSequencer<'_>) -> Result;
-}
-
-impl GspSeqCmdRunner for fw::RegWritePayload {
+impl fw::RegWritePayload {
     fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
         let addr = usize::from_safe_cast(self.addr());
 
@@ -160,7 +155,7 @@ fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
     }
 }
 
-impl GspSeqCmdRunner for fw::RegModifyPayload {
+impl fw::RegModifyPayload {
     fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
         let addr = usize::from_safe_cast(self.addr());
 
@@ -172,7 +167,7 @@ fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
     }
 }
 
-impl GspSeqCmdRunner for fw::RegPollPayload {
+impl fw::RegPollPayload {
     fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
         let addr = usize::from_safe_cast(self.addr());
 
@@ -197,14 +192,14 @@ fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
     }
 }
 
-impl GspSeqCmdRunner for fw::DelayUsPayload {
+impl fw::DelayUsPayload {
     fn run(&self, _sequencer: &GspSequencer<'_>) -> Result {
         fsleep(Delta::from_micros(i64::from(self.val())));
         Ok(())
     }
 }
 
-impl GspSeqCmdRunner for fw::RegStorePayload {
+impl fw::RegStorePayload {
     fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
         let addr = usize::from_safe_cast(self.addr());
 
@@ -212,7 +207,7 @@ fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
     }
 }
 
-impl GspSeqCmdRunner for GspSeqCmd {
+impl GspSeqCmd {
     fn run(&self, seq: &GspSequencer<'_>) -> Result {
         match self {
             GspSeqCmd::RegWrite(cmd) => cmd.run(seq),

-- 
2.52.0

[PATCH 5/9] gpu: nova-core: gsp: derive `Debug` on more sequencer types

[Alexandre Courbot]

Being able to print these is useful when debugging the sequencer.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/fw.rs        | 10 +++++-----
 drivers/gpu/nova-core/gsp/sequencer.rs |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index d06c0fdd6154..d1fc8f111db1 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -330,7 +330,7 @@ fn from(value: SeqBufOpcode) -> Self {
 
 /// Wrapper for GSP sequencer register write payload.
 #[repr(transparent)]
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub(crate) struct RegWritePayload(bindings::GSP_SEQ_BUF_PAYLOAD_REG_WRITE);
 
 impl RegWritePayload {
@@ -353,7 +353,7 @@ unsafe impl AsBytes for RegWritePayload {}
 
 /// Wrapper for GSP sequencer register modify payload.
 #[repr(transparent)]
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub(crate) struct RegModifyPayload(bindings::GSP_SEQ_BUF_PAYLOAD_REG_MODIFY);
 
 impl RegModifyPayload {
@@ -381,7 +381,7 @@ unsafe impl AsBytes for RegModifyPayload {}
 
 /// Wrapper for GSP sequencer register poll payload.
 #[repr(transparent)]
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub(crate) struct RegPollPayload(bindings::GSP_SEQ_BUF_PAYLOAD_REG_POLL);
 
 impl RegPollPayload {
@@ -414,7 +414,7 @@ unsafe impl AsBytes for RegPollPayload {}
 
 /// Wrapper for GSP sequencer delay payload.
 #[repr(transparent)]
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub(crate) struct DelayUsPayload(bindings::GSP_SEQ_BUF_PAYLOAD_DELAY_US);
 
 impl DelayUsPayload {
@@ -432,7 +432,7 @@ unsafe impl AsBytes for DelayUsPayload {}
 
 /// Wrapper for GSP sequencer register store payload.
 #[repr(transparent)]
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub(crate) struct RegStorePayload(bindings::GSP_SEQ_BUF_PAYLOAD_REG_STORE);
 
 impl RegStorePayload {
diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs
index 4efa048b9d93..5eead7ad4cbd 100644
--- a/drivers/gpu/nova-core/gsp/sequencer.rs
+++ b/drivers/gpu/nova-core/gsp/sequencer.rs
@@ -70,6 +70,7 @@ fn read(
 /// GSP Sequencer Command types with payload data.
 /// Commands have an opcode and an opcode-dependent struct.
 #[allow(clippy::enum_variant_names)]
+#[derive(Debug)]
 pub(crate) enum GspSeqCmd {
     RegWrite(fw::RegWritePayload),
     RegModify(fw::RegModifyPayload),

-- 
2.52.0

[PATCH 6/9] gpu: nova-core: gsp: derive Zeroable for GspStaticConfigInfo

[Alexandre Courbot]

We can derive `Zeroable` automatically instead of implementing it
ourselves if we convert it from a tuple struct into a regular one. This
removes an `unsafe` statement.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/fw/commands.rs | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
index 21be44199693..85465521de32 100644
--- a/drivers/gpu/nova-core/gsp/fw/commands.rs
+++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
@@ -107,12 +107,15 @@ unsafe impl FromBytes for PackedRegistryTable {}
 
 /// Payload of the `GetGspStaticInfo` command and message.
 #[repr(transparent)]
-pub(crate) struct GspStaticConfigInfo(bindings::GspStaticConfigInfo_t);
+#[derive(Zeroable)]
+pub(crate) struct GspStaticConfigInfo {
+    inner: bindings::GspStaticConfigInfo_t,
+}
 
 impl GspStaticConfigInfo {
     /// Returns a bytes array containing the (hopefully) zero-terminated name of this GPU.
     pub(crate) fn gpu_name_str(&self) -> [u8; 64] {
-        self.0.gpuNameString
+        self.inner.gpuNameString
     }
 }
 
@@ -122,7 +125,3 @@ unsafe impl AsBytes for GspStaticConfigInfo {}
 // SAFETY: This struct only contains integer types for which all bit patterns
 // are valid.
 unsafe impl FromBytes for GspStaticConfigInfo {}
-
-// SAFETY: This struct only contains integer types and fixed-size arrays for which
-// all bit patterns are valid.
-unsafe impl Zeroable for GspStaticConfigInfo {}

-- 
2.52.0

[PATCH 7/9] gpu: nova-core: firmware: fwsec: do not require bound device when unneeded

[Alexandre Courbot]

`run` doesn't require a bound device as its argument.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/firmware/fwsec.rs | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index b28e34d279f4..b98b1286dc94 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -412,12 +412,7 @@ pub(crate) fn new(
     }
 
     /// Loads the FWSEC firmware into `falcon` and execute it.
-    pub(crate) fn run(
-        &self,
-        dev: &Device<device::Bound>,
-        falcon: &Falcon<Gsp>,
-        bar: &Bar0,
-    ) -> Result<()> {
+    pub(crate) fn run(&self, dev: &Device, falcon: &Falcon<Gsp>, bar: &Bar0) -> Result<()> {
         // Reset falcon, load the firmware, and run it.
         falcon
             .reset(bar)

-- 
2.52.0

[PATCH 8/9] gpu: nova-core: use core library's CStr instead of kernel one

[Alexandre Courbot]

The kernel's own CStr type has been replaced by the one in the core
library, and is now an alias to the latter. Change our imports to
directly reference the actual type.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/firmware.rs     | 2 +-
 drivers/gpu/nova-core/firmware/gsp.rs | 6 ++++--
 drivers/gpu/nova-core/nova_core.rs    | 2 +-
 drivers/gpu/nova-core/util.rs         | 4 ++--
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 2d2008b33fb4..672f6cd24d4b 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -229,7 +229,7 @@ const fn make_entry_chipset(self, chipset: &str) -> Self {
     }
 
     pub(crate) const fn create(
-        module_name: &'static kernel::str::CStr,
+        module_name: &'static core::ffi::CStr,
     ) -> firmware::ModInfoBuilder<N> {
         let mut this = Self(firmware::ModInfoBuilder::new(module_name));
         let mut i = 0;
diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
index 0549805282ab..53fdbf1de27e 100644
--- a/drivers/gpu/nova-core/firmware/gsp.rs
+++ b/drivers/gpu/nova-core/firmware/gsp.rs
@@ -34,10 +34,12 @@
 /// that scheme before nova-core becomes stable, which means this module will eventually be
 /// removed.
 mod elf {
-    use core::mem::size_of;
+    use core::{
+        ffi::CStr,
+        mem::size_of, //
+    };
 
     use kernel::bindings;
-    use kernel::str::CStr;
     use kernel::transmute::FromBytes;
 
     /// Newtype to provide a [`FromBytes`] implementation.
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index b98a1c03f13d..3c26cf0b7c6e 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -19,7 +19,7 @@
 mod util;
 mod vbios;
 
-pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
+pub(crate) const MODULE_NAME: &core::ffi::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
 
 kernel::module_pci_driver! {
     type: driver::NovaCore,
diff --git a/drivers/gpu/nova-core/util.rs b/drivers/gpu/nova-core/util.rs
index 4b503249a3ef..8b2a4b99c55b 100644
--- a/drivers/gpu/nova-core/util.rs
+++ b/drivers/gpu/nova-core/util.rs
@@ -3,10 +3,10 @@
 /// Converts a null-terminated byte slice to a string, or `None` if the array does not
 /// contains any null byte or contains invalid characters.
 ///
-/// Contrary to [`kernel::str::CStr::from_bytes_with_nul`], the null byte can be anywhere in the
+/// Contrary to [`core::ffi::CStr::from_bytes_with_nul`], the null byte can be anywhere in the
 /// slice, and not only in the last position.
 pub(crate) fn str_from_null_terminated(bytes: &[u8]) -> Option<&str> {
-    use kernel::str::CStr;
+    use core::ffi::CStr;
 
     bytes
         .iter()

-- 
2.52.0

[PATCH 9/9] gpu: nova-core: simplify str_from_null_terminated

[Alexandre Courbot]

The core library's `CStr` has a `from_bytes_until_nul` method that we
can leverage to simplify this function.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/util.rs | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/nova-core/util.rs b/drivers/gpu/nova-core/util.rs
index 8b2a4b99c55b..2cccbce78c14 100644
--- a/drivers/gpu/nova-core/util.rs
+++ b/drivers/gpu/nova-core/util.rs
@@ -2,15 +2,10 @@
 
 /// Converts a null-terminated byte slice to a string, or `None` if the array does not
 /// contains any null byte or contains invalid characters.
-///
-/// Contrary to [`core::ffi::CStr::from_bytes_with_nul`], the null byte can be anywhere in the
-/// slice, and not only in the last position.
 pub(crate) fn str_from_null_terminated(bytes: &[u8]) -> Option<&str> {
     use core::ffi::CStr;
 
-    bytes
-        .iter()
-        .position(|&b| b == 0)
-        .and_then(|null_pos| CStr::from_bytes_with_nul(&bytes[..=null_pos]).ok())
+    CStr::from_bytes_until_nul(bytes)
+        .ok()
         .and_then(|cstr| cstr.to_str().ok())
 }

-- 
2.52.0

Re: [PATCH 7/9] gpu: nova-core: firmware: fwsec: do not require bound device when unneeded

[Timur Tabi]

On Mon, 2025-12-08 at 18:26 +0900, Alexandre Courbot wrote:
>      /// Loads the FWSEC firmware into `falcon` and execute it.
> -    pub(crate) fn run(
> -        &self,
> -        dev: &Device<device::Bound>,
> -        falcon: &Falcon<Gsp>,
> -        bar: &Bar0,
> -    ) -> Result<()> {
> +    pub(crate) fn run(&self, dev: &Device, falcon: &Falcon<Gsp>, bar: &Bar0) -> Result<()> {

I frequently see patches that, when they change the function signature, rearrange the parameters
from one line to multiple lines.  Here, you are doing the opposite.  Not only that, but it seems
unnecessary because you're actually just changing one parameter, so you should only be replacing
one line.

It seems to me that some people have their editors configured to prefer one line, and others
have their editor configured to prefer multiple lines, so whenever there's a signature change,
we get diffs like this.

Re: [PATCH 7/9] gpu: nova-core: firmware: fwsec: do not require bound device when unneeded

[John Hubbard]

On 12/8/25 8:18 AM, Timur Tabi wrote:
> On Mon, 2025-12-08 at 18:26 +0900, Alexandre Courbot wrote:
...
> I frequently see patches that, when they change the function signature, rearrange the parameters
> from one line to multiple lines.  Here, you are doing the opposite.  Not only that, but it seems
> unnecessary because you're actually just changing one parameter, so you should only be replacing
> one line.
> 
> It seems to me that some people have their editors configured to prefer one line, and others
> have their editor configured to prefer multiple lines, so whenever there's a signature change,
> we get diffs like this.

Nope, what's actually happening is that (nearly) everyone has
their editor set up to run rustfmt(1) upon file save. That's
the convention used in Rust for Linux. (Failing that, one is
expected to run rustfmt before posting.)

It is a little jumpy, as you can see above, but it does have
the nice property of avoiding formatting discussions, since
there is only one way for things to end up.

thanks,
-- 
John Hubbard

Re: [PATCH 7/9] gpu: nova-core: firmware: fwsec: do not require bound device when unneeded

[Timur Tabi]

On Mon, 2025-12-08 at 08:51 -0800, John Hubbard wrote:
> It is a little jumpy, as you can see above, but it does have
> the nice property of avoiding formatting discussions, 

Apparently not.

> since
> there is only one way for things to end up.

That's not my observation.  Like I said, it appears to oscillate.

Re: [PATCH 2/9] gpu: nova-core: gsp: remove unnecessary Display impls

[Alistair Popple]

On 2025-12-08 at 20:26 +1100, Alexandre Courbot <acourbot@nvidia.com> wrote...
> We only ever display these in debug context, for which the automatically
> derived `Debug` impls work just fine - so use them and remove these
> boilerplate-looking implementations.

Nice.

Reviewed-by: Alistair Popple <apopple@nvidia.com>

> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs |  2 +-
>  drivers/gpu/nova-core/gsp/fw.rs   | 54 ---------------------------------------
>  2 files changed, 1 insertion(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 5ce85ee1ffce..fa983a3f480c 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -531,7 +531,7 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
>  
>          dev_dbg!(
>              &self.dev,
> -            "GSP RPC: send: seq# {}, function={}, length=0x{:x}\n",
> +            "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
>              self.seq,
>              M::FUNCTION,
>              dst.header.length(),
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
> index 3baa5455cc32..24e4eaaf1265 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -10,7 +10,6 @@
>  
>  use kernel::{
>      dma::CoherentAllocation,
> -    fmt,
>      prelude::*,
>      ptr::{
>          Alignable,
> @@ -223,43 +222,6 @@ pub(crate) enum MsgFunction {
>      UcodeLibOsPrint = bindings::NV_VGPU_MSG_EVENT_UCODE_LIBOS_PRINT,
>  }
>  
> -impl fmt::Display for MsgFunction {
> -    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> -        match self {
> -            // Common function codes
> -            MsgFunction::Nop => write!(f, "NOP"),
> -            MsgFunction::SetGuestSystemInfo => write!(f, "SET_GUEST_SYSTEM_INFO"),
> -            MsgFunction::AllocRoot => write!(f, "ALLOC_ROOT"),
> -            MsgFunction::AllocDevice => write!(f, "ALLOC_DEVICE"),
> -            MsgFunction::AllocMemory => write!(f, "ALLOC_MEMORY"),
> -            MsgFunction::AllocCtxDma => write!(f, "ALLOC_CTX_DMA"),
> -            MsgFunction::AllocChannelDma => write!(f, "ALLOC_CHANNEL_DMA"),
> -            MsgFunction::MapMemory => write!(f, "MAP_MEMORY"),
> -            MsgFunction::BindCtxDma => write!(f, "BIND_CTX_DMA"),
> -            MsgFunction::AllocObject => write!(f, "ALLOC_OBJECT"),
> -            MsgFunction::Free => write!(f, "FREE"),
> -            MsgFunction::Log => write!(f, "LOG"),
> -            MsgFunction::GetGspStaticInfo => write!(f, "GET_GSP_STATIC_INFO"),
> -            MsgFunction::SetRegistry => write!(f, "SET_REGISTRY"),
> -            MsgFunction::GspSetSystemInfo => write!(f, "GSP_SET_SYSTEM_INFO"),
> -            MsgFunction::GspInitPostObjGpu => write!(f, "GSP_INIT_POST_OBJGPU"),
> -            MsgFunction::GspRmControl => write!(f, "GSP_RM_CONTROL"),
> -            MsgFunction::GetStaticInfo => write!(f, "GET_STATIC_INFO"),
> -
> -            // Event codes
> -            MsgFunction::GspInitDone => write!(f, "INIT_DONE"),
> -            MsgFunction::GspRunCpuSequencer => write!(f, "RUN_CPU_SEQUENCER"),
> -            MsgFunction::PostEvent => write!(f, "POST_EVENT"),
> -            MsgFunction::RcTriggered => write!(f, "RC_TRIGGERED"),
> -            MsgFunction::MmuFaultQueued => write!(f, "MMU_FAULT_QUEUED"),
> -            MsgFunction::OsErrorLog => write!(f, "OS_ERROR_LOG"),
> -            MsgFunction::GspPostNoCat => write!(f, "NOCAT"),
> -            MsgFunction::GspLockdownNotice => write!(f, "LOCKDOWN_NOTICE"),
> -            MsgFunction::UcodeLibOsPrint => write!(f, "LIBOS_PRINT"),
> -        }
> -    }
> -}
> -
>  impl TryFrom<u32> for MsgFunction {
>      type Error = kernel::error::Error;
>  
> @@ -330,22 +292,6 @@ pub(crate) enum SeqBufOpcode {
>      RegWrite = bindings::GSP_SEQ_BUF_OPCODE_GSP_SEQ_BUF_OPCODE_REG_WRITE,
>  }
>  
> -impl fmt::Display for SeqBufOpcode {
> -    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> -        match self {
> -            SeqBufOpcode::CoreReset => write!(f, "CORE_RESET"),
> -            SeqBufOpcode::CoreResume => write!(f, "CORE_RESUME"),
> -            SeqBufOpcode::CoreStart => write!(f, "CORE_START"),
> -            SeqBufOpcode::CoreWaitForHalt => write!(f, "CORE_WAIT_FOR_HALT"),
> -            SeqBufOpcode::DelayUs => write!(f, "DELAY_US"),
> -            SeqBufOpcode::RegModify => write!(f, "REG_MODIFY"),
> -            SeqBufOpcode::RegPoll => write!(f, "REG_POLL"),
> -            SeqBufOpcode::RegStore => write!(f, "REG_STORE"),
> -            SeqBufOpcode::RegWrite => write!(f, "REG_WRITE"),
> -        }
> -    }
> -}
> -
>  impl TryFrom<u32> for SeqBufOpcode {
>      type Error = kernel::error::Error;
>  
> 
> -- 
> 2.52.0
> 

Re: [PATCH 7/9] gpu: nova-core: firmware: fwsec: do not require bound device when unneeded

[Alexandre Courbot]

On Tue Dec 9, 2025 at 2:55 AM JST, Timur Tabi wrote:
> On Mon, 2025-12-08 at 08:51 -0800, John Hubbard wrote:
>> It is a little jumpy, as you can see above, but it does have
>> the nice property of avoiding formatting discussions, 
>
> Apparently not.
>
>> since
>> there is only one way for things to end up.
>
> That's not my observation.  Like I said, it appears to oscillate.

rustfmt is a requirement for merging patches, so that's really all there
is to say on the topic.

Re: [PATCH 1/9] gpu: nova-core: gsp: warn if data remains after processing a message

[lyude]

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

On Mon, 2025-12-08 at 18:26 +0900, Alexandre Courbot wrote:
> Not processing the whole data from a received message is a strong
> indicator of a bug - emit a warning when such cases are detected.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-
> core/gsp/cmdq.rs
> index dab73377c526..5ce85ee1ffce 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -662,7 +662,17 @@ pub(crate) fn receive_msg<M:
> MessageFromGsp>(&mut self, timeout: Delta) -> Resul
>              let (cmd, contents_1) =
> M::Message::from_bytes_prefix(message.contents.0).ok_or(EIO)?;
>              let mut sbuffer = SBufferIter::new_reader([contents_1,
> message.contents.1]);
>  
> -            M::read(cmd, &mut sbuffer).map_err(|e| e.into())
> +            let res = M::read(cmd, &mut sbuffer).map_err(|e|
> e.into());
> +
> +            if res.is_ok() && !sbuffer.is_empty() {
> +                dev_warn!(
> +                    &self.dev,
> +                    "GSP message {:?} has unprocessed data\n",
> +                    function
> +                );
> +            }
> +
> +            res
>          } else {
>              Err(ERANGE)
>          };

Re: [PATCH 2/9] gpu: nova-core: gsp: remove unnecessary Display impls

[lyude]

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

On Mon, 2025-12-08 at 18:26 +0900, Alexandre Courbot wrote:
> We only ever display these in debug context, for which the
> automatically
> derived `Debug` impls work just fine - so use them and remove these
> boilerplate-looking implementations.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs |  2 +-
>  drivers/gpu/nova-core/gsp/fw.rs   | 54 -----------------------------
> ----------
>  2 files changed, 1 insertion(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-
> core/gsp/cmdq.rs
> index 5ce85ee1ffce..fa983a3f480c 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -531,7 +531,7 @@ pub(crate) fn send_command<M>(&mut self, bar:
> &Bar0, command: M) -> Result
>  
>          dev_dbg!(
>              &self.dev,
> -            "GSP RPC: send: seq# {}, function={}, length=0x{:x}\n",
> +            "GSP RPC: send: seq# {}, function={:?},
> length=0x{:x}\n",
>              self.seq,
>              M::FUNCTION,
>              dst.header.length(),
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-
> core/gsp/fw.rs
> index 3baa5455cc32..24e4eaaf1265 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -10,7 +10,6 @@
>  
>  use kernel::{
>      dma::CoherentAllocation,
> -    fmt,
>      prelude::*,
>      ptr::{
>          Alignable,
> @@ -223,43 +222,6 @@ pub(crate) enum MsgFunction {
>      UcodeLibOsPrint = bindings::NV_VGPU_MSG_EVENT_UCODE_LIBOS_PRINT,
>  }
>  
> -impl fmt::Display for MsgFunction {
> -    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> -        match self {
> -            // Common function codes
> -            MsgFunction::Nop => write!(f, "NOP"),
> -            MsgFunction::SetGuestSystemInfo => write!(f,
> "SET_GUEST_SYSTEM_INFO"),
> -            MsgFunction::AllocRoot => write!(f, "ALLOC_ROOT"),
> -            MsgFunction::AllocDevice => write!(f, "ALLOC_DEVICE"),
> -            MsgFunction::AllocMemory => write!(f, "ALLOC_MEMORY"),
> -            MsgFunction::AllocCtxDma => write!(f, "ALLOC_CTX_DMA"),
> -            MsgFunction::AllocChannelDma => write!(f,
> "ALLOC_CHANNEL_DMA"),
> -            MsgFunction::MapMemory => write!(f, "MAP_MEMORY"),
> -            MsgFunction::BindCtxDma => write!(f, "BIND_CTX_DMA"),
> -            MsgFunction::AllocObject => write!(f, "ALLOC_OBJECT"),
> -            MsgFunction::Free => write!(f, "FREE"),
> -            MsgFunction::Log => write!(f, "LOG"),
> -            MsgFunction::GetGspStaticInfo => write!(f,
> "GET_GSP_STATIC_INFO"),
> -            MsgFunction::SetRegistry => write!(f, "SET_REGISTRY"),
> -            MsgFunction::GspSetSystemInfo => write!(f,
> "GSP_SET_SYSTEM_INFO"),
> -            MsgFunction::GspInitPostObjGpu => write!(f,
> "GSP_INIT_POST_OBJGPU"),
> -            MsgFunction::GspRmControl => write!(f,
> "GSP_RM_CONTROL"),
> -            MsgFunction::GetStaticInfo => write!(f,
> "GET_STATIC_INFO"),
> -
> -            // Event codes
> -            MsgFunction::GspInitDone => write!(f, "INIT_DONE"),
> -            MsgFunction::GspRunCpuSequencer => write!(f,
> "RUN_CPU_SEQUENCER"),
> -            MsgFunction::PostEvent => write!(f, "POST_EVENT"),
> -            MsgFunction::RcTriggered => write!(f, "RC_TRIGGERED"),
> -            MsgFunction::MmuFaultQueued => write!(f,
> "MMU_FAULT_QUEUED"),
> -            MsgFunction::OsErrorLog => write!(f, "OS_ERROR_LOG"),
> -            MsgFunction::GspPostNoCat => write!(f, "NOCAT"),
> -            MsgFunction::GspLockdownNotice => write!(f,
> "LOCKDOWN_NOTICE"),
> -            MsgFunction::UcodeLibOsPrint => write!(f,
> "LIBOS_PRINT"),
> -        }
> -    }
> -}
> -
>  impl TryFrom<u32> for MsgFunction {
>      type Error = kernel::error::Error;
>  
> @@ -330,22 +292,6 @@ pub(crate) enum SeqBufOpcode {
>      RegWrite =
> bindings::GSP_SEQ_BUF_OPCODE_GSP_SEQ_BUF_OPCODE_REG_WRITE,
>  }
>  
> -impl fmt::Display for SeqBufOpcode {
> -    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> -        match self {
> -            SeqBufOpcode::CoreReset => write!(f, "CORE_RESET"),
> -            SeqBufOpcode::CoreResume => write!(f, "CORE_RESUME"),
> -            SeqBufOpcode::CoreStart => write!(f, "CORE_START"),
> -            SeqBufOpcode::CoreWaitForHalt => write!(f,
> "CORE_WAIT_FOR_HALT"),
> -            SeqBufOpcode::DelayUs => write!(f, "DELAY_US"),
> -            SeqBufOpcode::RegModify => write!(f, "REG_MODIFY"),
> -            SeqBufOpcode::RegPoll => write!(f, "REG_POLL"),
> -            SeqBufOpcode::RegStore => write!(f, "REG_STORE"),
> -            SeqBufOpcode::RegWrite => write!(f, "REG_WRITE"),
> -        }
> -    }
> -}
> -
>  impl TryFrom<u32> for SeqBufOpcode {
>      type Error = kernel::error::Error;
>  

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

[lyude]

Reviewed-by: Lyude Paul <lyude@redhat.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@nvidia.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 }))
>      }
>  }
>  

Re: [PATCH 4/9] gpu: nova-core: gsp: remove unneeded sequencer trait

[lyude]

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

On Mon, 2025-12-08 at 18:26 +0900, Alexandre Courbot wrote:
> The `GspSeqCmdRunner` trait is never used as we never call the `run`
> methods from generic code. Remove it.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/sequencer.rs | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs
> b/drivers/gpu/nova-core/gsp/sequencer.rs
> index 2d0369c49092..4efa048b9d93 100644
> --- a/drivers/gpu/nova-core/gsp/sequencer.rs
> +++ b/drivers/gpu/nova-core/gsp/sequencer.rs
> @@ -147,12 +147,7 @@ pub(crate) struct GspSequencer<'a> {
>      dev: ARef<device::Device>,
>  }
>  
> -/// Trait for running sequencer commands.
> -pub(crate) trait GspSeqCmdRunner {
> -    fn run(&self, sequencer: &GspSequencer<'_>) -> Result;
> -}
> -
> -impl GspSeqCmdRunner for fw::RegWritePayload {
> +impl fw::RegWritePayload {
>      fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
>          let addr = usize::from_safe_cast(self.addr());
>  
> @@ -160,7 +155,7 @@ fn run(&self, sequencer: &GspSequencer<'_>) ->
> Result {
>      }
>  }
>  
> -impl GspSeqCmdRunner for fw::RegModifyPayload {
> +impl fw::RegModifyPayload {
>      fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
>          let addr = usize::from_safe_cast(self.addr());
>  
> @@ -172,7 +167,7 @@ fn run(&self, sequencer: &GspSequencer<'_>) ->
> Result {
>      }
>  }
>  
> -impl GspSeqCmdRunner for fw::RegPollPayload {
> +impl fw::RegPollPayload {
>      fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
>          let addr = usize::from_safe_cast(self.addr());
>  
> @@ -197,14 +192,14 @@ fn run(&self, sequencer: &GspSequencer<'_>) ->
> Result {
>      }
>  }
>  
> -impl GspSeqCmdRunner for fw::DelayUsPayload {
> +impl fw::DelayUsPayload {
>      fn run(&self, _sequencer: &GspSequencer<'_>) -> Result {
>          fsleep(Delta::from_micros(i64::from(self.val())));
>          Ok(())
>      }
>  }
>  
> -impl GspSeqCmdRunner for fw::RegStorePayload {
> +impl fw::RegStorePayload {
>      fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
>          let addr = usize::from_safe_cast(self.addr());
>  
> @@ -212,7 +207,7 @@ fn run(&self, sequencer: &GspSequencer<'_>) ->
> Result {
>      }
>  }
>  
> -impl GspSeqCmdRunner for GspSeqCmd {
> +impl GspSeqCmd {
>      fn run(&self, seq: &GspSequencer<'_>) -> Result {
>          match self {
>              GspSeqCmd::RegWrite(cmd) => cmd.run(seq),

Re: [PATCH 5/9] gpu: nova-core: gsp: derive `Debug` on more sequencer types

[lyude]

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

On Mon, 2025-12-08 at 18:26 +0900, Alexandre Courbot wrote:
> Being able to print these is useful when debugging the sequencer.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/fw.rs        | 10 +++++-----
>  drivers/gpu/nova-core/gsp/sequencer.rs |  1 +
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-
> core/gsp/fw.rs
> index d06c0fdd6154..d1fc8f111db1 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -330,7 +330,7 @@ fn from(value: SeqBufOpcode) -> Self {
>  
>  /// Wrapper for GSP sequencer register write payload.
>  #[repr(transparent)]
> -#[derive(Copy, Clone)]
> +#[derive(Copy, Clone, Debug)]
>  pub(crate) struct
> RegWritePayload(bindings::GSP_SEQ_BUF_PAYLOAD_REG_WRITE);
>  
>  impl RegWritePayload {
> @@ -353,7 +353,7 @@ unsafe impl AsBytes for RegWritePayload {}
>  
>  /// Wrapper for GSP sequencer register modify payload.
>  #[repr(transparent)]
> -#[derive(Copy, Clone)]
> +#[derive(Copy, Clone, Debug)]
>  pub(crate) struct
> RegModifyPayload(bindings::GSP_SEQ_BUF_PAYLOAD_REG_MODIFY);
>  
>  impl RegModifyPayload {
> @@ -381,7 +381,7 @@ unsafe impl AsBytes for RegModifyPayload {}
>  
>  /// Wrapper for GSP sequencer register poll payload.
>  #[repr(transparent)]
> -#[derive(Copy, Clone)]
> +#[derive(Copy, Clone, Debug)]
>  pub(crate) struct
> RegPollPayload(bindings::GSP_SEQ_BUF_PAYLOAD_REG_POLL);
>  
>  impl RegPollPayload {
> @@ -414,7 +414,7 @@ unsafe impl AsBytes for RegPollPayload {}
>  
>  /// Wrapper for GSP sequencer delay payload.
>  #[repr(transparent)]
> -#[derive(Copy, Clone)]
> +#[derive(Copy, Clone, Debug)]
>  pub(crate) struct
> DelayUsPayload(bindings::GSP_SEQ_BUF_PAYLOAD_DELAY_US);
>  
>  impl DelayUsPayload {
> @@ -432,7 +432,7 @@ unsafe impl AsBytes for DelayUsPayload {}
>  
>  /// Wrapper for GSP sequencer register store payload.
>  #[repr(transparent)]
> -#[derive(Copy, Clone)]
> +#[derive(Copy, Clone, Debug)]
>  pub(crate) struct
> RegStorePayload(bindings::GSP_SEQ_BUF_PAYLOAD_REG_STORE);
>  
>  impl RegStorePayload {
> diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs
> b/drivers/gpu/nova-core/gsp/sequencer.rs
> index 4efa048b9d93..5eead7ad4cbd 100644
> --- a/drivers/gpu/nova-core/gsp/sequencer.rs
> +++ b/drivers/gpu/nova-core/gsp/sequencer.rs
> @@ -70,6 +70,7 @@ fn read(
>  /// GSP Sequencer Command types with payload data.
>  /// Commands have an opcode and an opcode-dependent struct.
>  #[allow(clippy::enum_variant_names)]
> +#[derive(Debug)]
>  pub(crate) enum GspSeqCmd {
>      RegWrite(fw::RegWritePayload),
>      RegModify(fw::RegModifyPayload),

Re: [PATCH 6/9] gpu: nova-core: gsp: derive Zeroable for GspStaticConfigInfo

[lyude]

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

On Mon, 2025-12-08 at 18:26 +0900, Alexandre Courbot wrote:
> We can derive `Zeroable` automatically instead of implementing it
> ourselves if we convert it from a tuple struct into a regular one.
> This
> removes an `unsafe` statement.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/fw/commands.rs | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs
> b/drivers/gpu/nova-core/gsp/fw/commands.rs
> index 21be44199693..85465521de32 100644
> --- a/drivers/gpu/nova-core/gsp/fw/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
> @@ -107,12 +107,15 @@ unsafe impl FromBytes for PackedRegistryTable
> {}
>  
>  /// Payload of the `GetGspStaticInfo` command and message.
>  #[repr(transparent)]
> -pub(crate) struct
> GspStaticConfigInfo(bindings::GspStaticConfigInfo_t);
> +#[derive(Zeroable)]
> +pub(crate) struct GspStaticConfigInfo {
> +    inner: bindings::GspStaticConfigInfo_t,
> +}
>  
>  impl GspStaticConfigInfo {
>      /// Returns a bytes array containing the (hopefully) zero-
> terminated name of this GPU.
>      pub(crate) fn gpu_name_str(&self) -> [u8; 64] {
> -        self.0.gpuNameString
> +        self.inner.gpuNameString
>      }
>  }
>  
> @@ -122,7 +125,3 @@ unsafe impl AsBytes for GspStaticConfigInfo {}
>  // SAFETY: This struct only contains integer types for which all bit
> patterns
>  // are valid.
>  unsafe impl FromBytes for GspStaticConfigInfo {}
> -
> -// SAFETY: This struct only contains integer types and fixed-size
> arrays for which
> -// all bit patterns are valid.
> -unsafe impl Zeroable for GspStaticConfigInfo {}

Re: [PATCH 7/9] gpu: nova-core: firmware: fwsec: do not require bound device when unneeded

[lyude]

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

On Mon, 2025-12-08 at 18:26 +0900, Alexandre Courbot wrote:
> `run` doesn't require a bound device as its argument.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/firmware/fwsec.rs | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs
> b/drivers/gpu/nova-core/firmware/fwsec.rs
> index b28e34d279f4..b98b1286dc94 100644
> --- a/drivers/gpu/nova-core/firmware/fwsec.rs
> +++ b/drivers/gpu/nova-core/firmware/fwsec.rs
> @@ -412,12 +412,7 @@ pub(crate) fn new(
>      }
>  
>      /// Loads the FWSEC firmware into `falcon` and execute it.
> -    pub(crate) fn run(
> -        &self,
> -        dev: &Device<device::Bound>,
> -        falcon: &Falcon<Gsp>,
> -        bar: &Bar0,
> -    ) -> Result<()> {
> +    pub(crate) fn run(&self, dev: &Device, falcon: &Falcon<Gsp>,
> bar: &Bar0) -> Result<()> {
>          // Reset falcon, load the firmware, and run it.
>          falcon
>              .reset(bar)

Re: [PATCH 8/9] gpu: nova-core: use core library's CStr instead of kernel one

[lyude]

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

On Mon, 2025-12-08 at 18:27 +0900, Alexandre Courbot wrote:
> The kernel's own CStr type has been replaced by the one in the core
> library, and is now an alias to the latter. Change our imports to
> directly reference the actual type.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/firmware.rs     | 2 +-
>  drivers/gpu/nova-core/firmware/gsp.rs | 6 ++++--
>  drivers/gpu/nova-core/nova_core.rs    | 2 +-
>  drivers/gpu/nova-core/util.rs         | 4 ++--
>  4 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-
> core/firmware.rs
> index 2d2008b33fb4..672f6cd24d4b 100644
> --- a/drivers/gpu/nova-core/firmware.rs
> +++ b/drivers/gpu/nova-core/firmware.rs
> @@ -229,7 +229,7 @@ const fn make_entry_chipset(self, chipset: &str)
> -> Self {
>      }
>  
>      pub(crate) const fn create(
> -        module_name: &'static kernel::str::CStr,
> +        module_name: &'static core::ffi::CStr,
>      ) -> firmware::ModInfoBuilder<N> {
>          let mut this =
> Self(firmware::ModInfoBuilder::new(module_name));
>          let mut i = 0;
> diff --git a/drivers/gpu/nova-core/firmware/gsp.rs
> b/drivers/gpu/nova-core/firmware/gsp.rs
> index 0549805282ab..53fdbf1de27e 100644
> --- a/drivers/gpu/nova-core/firmware/gsp.rs
> +++ b/drivers/gpu/nova-core/firmware/gsp.rs
> @@ -34,10 +34,12 @@
>  /// that scheme before nova-core becomes stable, which means this
> module will eventually be
>  /// removed.
>  mod elf {
> -    use core::mem::size_of;
> +    use core::{
> +        ffi::CStr,
> +        mem::size_of, //
> +    };
>  
>      use kernel::bindings;
> -    use kernel::str::CStr;
>      use kernel::transmute::FromBytes;
>  
>      /// Newtype to provide a [`FromBytes`] implementation.
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-
> core/nova_core.rs
> index b98a1c03f13d..3c26cf0b7c6e 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -19,7 +19,7 @@
>  mod util;
>  mod vbios;
>  
> -pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as
> kernel::ModuleMetadata>::NAME;
> +pub(crate) const MODULE_NAME: &core::ffi::CStr = <LocalModule as
> kernel::ModuleMetadata>::NAME;
>  
>  kernel::module_pci_driver! {
>      type: driver::NovaCore,
> diff --git a/drivers/gpu/nova-core/util.rs b/drivers/gpu/nova-
> core/util.rs
> index 4b503249a3ef..8b2a4b99c55b 100644
> --- a/drivers/gpu/nova-core/util.rs
> +++ b/drivers/gpu/nova-core/util.rs
> @@ -3,10 +3,10 @@
>  /// Converts a null-terminated byte slice to a string, or `None` if
> the array does not
>  /// contains any null byte or contains invalid characters.
>  ///
> -/// Contrary to [`kernel::str::CStr::from_bytes_with_nul`], the null
> byte can be anywhere in the
> +/// Contrary to [`core::ffi::CStr::from_bytes_with_nul`], the null
> byte can be anywhere in the
>  /// slice, and not only in the last position.
>  pub(crate) fn str_from_null_terminated(bytes: &[u8]) -> Option<&str>
> {
> -    use kernel::str::CStr;
> +    use core::ffi::CStr;
>  
>      bytes
>          .iter()

Re: [PATCH 9/9] gpu: nova-core: simplify str_from_null_terminated

[lyude]

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

On Mon, 2025-12-08 at 18:27 +0900, Alexandre Courbot wrote:
> The core library's `CStr` has a `from_bytes_until_nul` method that we
> can leverage to simplify this function.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/util.rs | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/util.rs b/drivers/gpu/nova-
> core/util.rs
> index 8b2a4b99c55b..2cccbce78c14 100644
> --- a/drivers/gpu/nova-core/util.rs
> +++ b/drivers/gpu/nova-core/util.rs
> @@ -2,15 +2,10 @@
>  
>  /// Converts a null-terminated byte slice to a string, or `None` if
> the array does not
>  /// contains any null byte or contains invalid characters.
> -///
> -/// Contrary to [`core::ffi::CStr::from_bytes_with_nul`], the null
> byte can be anywhere in the
> -/// slice, and not only in the last position.
>  pub(crate) fn str_from_null_terminated(bytes: &[u8]) -> Option<&str>
> {
>      use core::ffi::CStr;
>  
> -    bytes
> -        .iter()
> -        .position(|&b| b == 0)
> -        .and_then(|null_pos|
> CStr::from_bytes_with_nul(&bytes[..=null_pos]).ok())
> +    CStr::from_bytes_until_nul(bytes)
> +        .ok()
>          .and_then(|cstr| cstr.to_str().ok())
>  }