gpu: nova-core: miscellaneous improvements

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 <[email protected]>

---
 drivers/gpu/nova-core/gsp/cmdq.rs | 12 +++++++++++-

 1 file changed, 11 insertions(+), 1 deletion(-)

drivers/gpu/nova- renamed from drivers/gpu/nova-core/gsp/cmdq.rs +0 -0

core/gsp/cmdq.rs
index dab73377c526..5ce85ee1ffce 100644

drivers/gpu/nova-core/gsp/cmdq.rs +0 -0

@@ -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());

drivers/gpu/nova-core/gsp/cmdq.rs +10 -0

+
+            if res.is_ok() && !sbuffer.is_empty() {
+                dev_warn!(
+                    &self.dev,
+                    "GSP message {:?} has unprocessed data\n",
+                    function
+                );
+            }
+
+            res

         } else {
             Err(ERANGE)
         };

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 <[email protected]>

---
 drivers/gpu/nova-core/gsp/cmdq.rs |  2 +-
 drivers/gpu/nova-core/gsp/fw.rs   | 54 ---------------------------------------
 2 files changed, 1 insertion(+), 55 deletions(-)

drivers/gpu/nova-core/gsp/cmdq.rs +1 -1

index 5ce85ee1ffce..fa983a3f480c 100644
@@ -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(),

drivers/gpu/nova-core/gsp/fw.rs +0 -54

index 3baa5455cc32..24e4eaaf1265 100644
@@ -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;
 

Signature · 2 lines

-- 
2.52.0

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 <[email protected]>

---
 drivers/gpu/nova-core/gsp/cmdq.rs |  2 +- drivers/gpu/nova-core/gsp/fw.rs   | 54 -----------------------------
----------

 2 files changed, 1 insertion(+), 55 deletions(-)

drivers/gpu/nova- renamed from drivers/gpu/nova-core/gsp/cmdq.rs +0 -0

core/gsp/cmdq.rs
index 5ce85ee1ffce..fa983a3f480c 100644

drivers/gpu/nova-core/gsp/cmdq.rs +0 -0

@@ -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(),

drivers/gpu/nova- renamed from drivers/gpu/nova-core/gsp/fw.rs +0 -0

core/gsp/fw.rs
index 3baa5455cc32..24e4eaaf1265 100644

drivers/gpu/nova-core/gsp/fw.rs +0 -0

@@ -10,7 +10,6 @@

 use kernel::{
     dma::CoherentAllocation,
-    fmt,
     prelude::*,
     ptr::{
         Alignable,

drivers/gpu/nova-core/gsp/fw.rs +0 -0

@@ -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"),
-        }
-    }

drivers/gpu/nova-core/gsp/fw.rs +0 -2

-}
-

 impl TryFrom<u32> for MsgFunction {
     type Error = kernel::error::Error;

drivers/gpu/nova-core/gsp/fw.rs +0 -0

@@ -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"), -        }
-    }

drivers/gpu/nova-core/gsp/fw.rs +0 -2

-}
-

 impl TryFrom<u32> for SeqBufOpcode {
     type Error = kernel::error::Error;

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 <[email protected]>

---
 drivers/gpu/nova-core/gsp/fw.rs | 40 +++++--------------------------
---------

 1 file changed, 5 insertions(+), 35 deletions(-)

drivers/gpu/nova- renamed from drivers/gpu/nova-core/gsp/fw.rs +0 -0

core/gsp/fw.rs
index 24e4eaaf1265..d06c0fdd6154 100644

drivers/gpu/nova-core/gsp/fw.rs +0 -0

@@ -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.

drivers/gpu/nova-core/gsp/fw.rs +0 -0

@@ -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.

drivers/gpu/nova-core/gsp/fw.rs +0 -0

@@ -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.

drivers/gpu/nova-core/gsp/fw.rs +0 -0

@@ -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.

drivers/gpu/nova-core/gsp/fw.rs +0 -0

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

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

Signed-off-by Alexandre Courbot <[email protected]>

---
 drivers/gpu/nova-core/gsp/sequencer.rs | 17 ++++++-----------

 1 file changed, 6 insertions(+), 11 deletions(-)

drivers/gpu/nova-core/gsp/sequencer.rs +0 -0

index 2d0369c49092..4efa048b9d93 100644
@@ -147,12 +147,7 @@ pub(crate) struct GspSequencer<'a> {
      dev: ARef<device::Device>,
  }

drivers/gpu/nova-core/gsp/sequencer.rs +1 -6

-/// 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());

drivers/gpu/nova-core/gsp/sequencer.rs +0 -0

@@ -160,7 +155,7 @@ fn run(&self, sequencer: &GspSequencer<'_>) ->
 Result {
      }
  }

drivers/gpu/nova-core/gsp/sequencer.rs +1 -1

-impl GspSeqCmdRunner for fw::RegModifyPayload {
+impl fw::RegModifyPayload {

     fn run(&self, sequencer: &GspSequencer<'_>) -> Result { let addr = usize::from_safe_cast(self.addr());

drivers/gpu/nova-core/gsp/sequencer.rs +0 -0

@@ -172,7 +167,7 @@ fn run(&self, sequencer: &GspSequencer<'_>) ->
 Result {
      }
  }

drivers/gpu/nova-core/gsp/sequencer.rs +1 -1

-impl GspSeqCmdRunner for fw::RegPollPayload {
+impl fw::RegPollPayload {

     fn run(&self, sequencer: &GspSequencer<'_>) -> Result { let addr = usize::from_safe_cast(self.addr());

drivers/gpu/nova-core/gsp/sequencer.rs +0 -0

@@ -197,14 +192,14 @@ fn run(&self, sequencer: &GspSequencer<'_>) ->
 Result {
      }
  }

drivers/gpu/nova-core/gsp/sequencer.rs +1 -1

-impl GspSeqCmdRunner for fw::DelayUsPayload {
+impl fw::DelayUsPayload {

     fn run(&self, _sequencer: &GspSequencer<'_>) -> Result { fsleep(Delta::from_micros(i64::from(self.val()))); Ok(())
     }
 }

drivers/gpu/nova-core/gsp/sequencer.rs +1 -1

-impl GspSeqCmdRunner for fw::RegStorePayload {
+impl fw::RegStorePayload {

     fn run(&self, sequencer: &GspSequencer<'_>) -> Result { let addr = usize::from_safe_cast(self.addr());

drivers/gpu/nova-core/gsp/sequencer.rs +0 -0

@@ -212,7 +207,7 @@ fn run(&self, sequencer: &GspSequencer<'_>) ->
 Result {
      }
  }

drivers/gpu/nova-core/gsp/sequencer.rs +1 -1

-impl GspSeqCmdRunner for GspSeqCmd {
+impl GspSeqCmd {

     fn run(&self, seq: &GspSequencer<'_>) -> Result { match self { GspSeqCmd::RegWrite(cmd) => cmd.run(seq),

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

Signed-off-by Alexandre Courbot <[email protected]>

---
 drivers/gpu/nova-core/gsp/fw.rs        | 10 +++++----- drivers/gpu/nova-core/gsp/sequencer.rs |  1 +

 2 files changed, 6 insertions(+), 5 deletions(-)

drivers/gpu/nova- renamed from drivers/gpu/nova-core/gsp/fw.rs +0 -0

core/gsp/fw.rs
index d06c0fdd6154..d1fc8f111db1 100644

drivers/gpu/nova-core/gsp/fw.rs +0 -0

@@ -330,7 +330,7 @@ fn from(value: SeqBufOpcode) -> Self {

 /// Wrapper for GSP sequencer register write payload. #[repr(transparent)]

drivers/gpu/nova-core/gsp/fw.rs +1 -1

-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]

 pub(crate) struct RegWritePayload(bindings::GSP_SEQ_BUF_PAYLOAD_REG_WRITE);
 impl RegWritePayload {

drivers/gpu/nova-core/gsp/fw.rs +0 -0

@@ -353,7 +353,7 @@ unsafe impl AsBytes for RegWritePayload {}

 /// Wrapper for GSP sequencer register modify payload. #[repr(transparent)]

drivers/gpu/nova-core/gsp/fw.rs +1 -1

-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]

 pub(crate) struct RegModifyPayload(bindings::GSP_SEQ_BUF_PAYLOAD_REG_MODIFY);
 impl RegModifyPayload {

drivers/gpu/nova-core/gsp/fw.rs +0 -0

@@ -381,7 +381,7 @@ unsafe impl AsBytes for RegModifyPayload {}

 /// Wrapper for GSP sequencer register poll payload. #[repr(transparent)]

drivers/gpu/nova-core/gsp/fw.rs +1 -1

-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]

 pub(crate) struct RegPollPayload(bindings::GSP_SEQ_BUF_PAYLOAD_REG_POLL);
 impl RegPollPayload {

drivers/gpu/nova-core/gsp/fw.rs +0 -0

@@ -414,7 +414,7 @@ unsafe impl AsBytes for RegPollPayload {}

 /// Wrapper for GSP sequencer delay payload.
 #[repr(transparent)]

drivers/gpu/nova-core/gsp/fw.rs +1 -1

-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]

 pub(crate) struct DelayUsPayload(bindings::GSP_SEQ_BUF_PAYLOAD_DELAY_US);
 impl DelayUsPayload {

drivers/gpu/nova-core/gsp/fw.rs +0 -0

@@ -432,7 +432,7 @@ unsafe impl AsBytes for DelayUsPayload {}

 /// Wrapper for GSP sequencer register store payload. #[repr(transparent)]

drivers/gpu/nova-core/gsp/fw.rs +1 -1

-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]

 pub(crate) struct RegStorePayload(bindings::GSP_SEQ_BUF_PAYLOAD_REG_STORE);
 impl RegStorePayload {

drivers/gpu/nova-core/gsp/sequencer.rs +1 -0

index 4efa048b9d93..5eead7ad4cbd 100644
@@ -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),

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 <[email protected]>

---
 drivers/gpu/nova-core/gsp/fw/commands.rs | 11 +++++------

 1 file changed, 5 insertions(+), 6 deletions(-)

drivers/gpu/nova-core/gsp/fw/commands.rs +0 -0

index 21be44199693..85465521de32 100644
@@ -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);

drivers/gpu/nova-core/gsp/fw/commands.rs +4 -0

+#[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
     }
 }

drivers/gpu/nova-core/gsp/fw/commands.rs +0 -0

@@ -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 {}

drivers/gpu/nova-core/gsp/fw/commands.rs +0 -2

-
-// SAFETY: This struct only contains integer types and fixed-size

arrays for which

drivers/gpu/nova-core/gsp/fw/commands.rs +0 -2

-// all bit patterns are valid.
-unsafe impl Zeroable for GspStaticConfigInfo {}

     /// 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<()> {

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.

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.

Quoted 2 lines · John Hubbard

It is a little jumpy, as you can see above, but it does have the nice property of avoiding formatting discussions,

Apparently not.

Quoted 2 lines

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

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

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

Signed-off-by Alexandre Courbot <[email protected]>

---
 drivers/gpu/nova-core/firmware/fwsec.rs | 7 +------

 1 file changed, 1 insertion(+), 6 deletions(-)

drivers/gpu/nova-core/firmware/fwsec.rs +0 -0

index b28e34d279f4..b98b1286dc94 100644
@@ -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)

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 <[email protected]>

---
 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(-)

drivers/gpu/nova- renamed from drivers/gpu/nova-core/firmware.rs +0 -0

core/firmware.rs
index 2d2008b33fb4..672f6cd24d4b 100644

drivers/gpu/nova-core/firmware.rs +0 -1

@@ -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;

drivers/gpu/nova-core/firmware/gsp.rs +0 -0

index 0549805282ab..53fdbf1de27e 100644
@@ -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.

drivers/gpu/nova- renamed from drivers/gpu/nova-core/nova_core.rs +0 -0

core/nova_core.rs
index b98a1c03f13d..3c26cf0b7c6e 100644

drivers/gpu/nova-core/nova_core.rs +0 -0

@@ -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,

drivers/gpu/nova- renamed from drivers/gpu/nova-core/util.rs +0 -0

core/util.rs
index 4b503249a3ef..8b2a4b99c55b 100644

drivers/gpu/nova-core/util.rs +0 -0

@@ -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()

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 <[email protected]>

---
 drivers/gpu/nova-core/util.rs | 9 ++-------

 1 file changed, 2 insertions(+), 7 deletions(-)

drivers/gpu/nova- renamed from drivers/gpu/nova-core/util.rs +0 -0

core/util.rs
index 8b2a4b99c55b..2cccbce78c14 100644

drivers/gpu/nova-core/util.rs +0 -0

@@ -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.

drivers/gpu/nova-core/util.rs +0 -2

-///
-/// 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())
 }