* [PATCH v2 01/10] gpu: nova-core: gsp: warn if data remains after processing a message
2025-12-16 4:27 [PATCH v2 00/10] gpu: nova-core: miscellaneous improvements Alexandre Courbot
@ 2025-12-16 4:27 ` Alexandre Courbot
2026-01-19 15:33 ` Gary Guo
2025-12-16 4:27 ` [PATCH v2 02/10] gpu: nova-core: gsp: remove unnecessary Display impls Alexandre Courbot
` (8 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Alexandre Courbot @ 2025-12-16 4:27 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Alexandre Courbot, Lyude Paul
Not processing the whole data from a received message is a strong
indicator of a bug - emit a warning when such cases are detected.
Reviewed-by: Lyude Paul <lyude@redhat.com>
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 7985a0b3f769..f0b7ac1ee759 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -661,7 +661,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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 01/10] gpu: nova-core: gsp: warn if data remains after processing a message
2025-12-16 4:27 ` [PATCH v2 01/10] gpu: nova-core: gsp: warn if data remains after processing a message Alexandre Courbot
@ 2026-01-19 15:33 ` Gary Guo
0 siblings, 0 replies; 23+ messages in thread
From: Gary Guo @ 2026-01-19 15:33 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Lyude Paul
On Tue Dec 16, 2025 at 4:27 AM GMT, 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.
>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> 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 7985a0b3f769..f0b7ac1ee759 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -661,7 +661,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
> + );
> + }
You can also do
M::read(cmd, &mut sbuffer).map_err(|e| e.info).inspect(|_| {
if !sbuffer.is_empty() {
// ...
}
})
This does feel like something that would look more elegant with try blocks... I
hope it'll be stable some day.
Best,
Gary
> +
> + res
> } else {
> Err(ERANGE)
> };
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 02/10] gpu: nova-core: gsp: remove unnecessary Display impls
2025-12-16 4:27 [PATCH v2 00/10] gpu: nova-core: miscellaneous improvements Alexandre Courbot
2025-12-16 4:27 ` [PATCH v2 01/10] gpu: nova-core: gsp: warn if data remains after processing a message Alexandre Courbot
@ 2025-12-16 4:27 ` Alexandre Courbot
2026-01-19 15:41 ` Gary Guo
2025-12-16 4:27 ` [PATCH v2 03/10] gpu: nova-core: gsp: simplify sequencer opcode parsing Alexandre Courbot
` (7 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Alexandre Courbot @ 2025-12-16 4:27 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Alexandre Courbot, Lyude Paul
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.
Reviewed-by: Lyude Paul <lyude@redhat.com>
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 f0b7ac1ee759..4dde9cc4e3c7 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 caeb0d251fe5..de251a143f7b 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 02/10] gpu: nova-core: gsp: remove unnecessary Display impls
2025-12-16 4:27 ` [PATCH v2 02/10] gpu: nova-core: gsp: remove unnecessary Display impls Alexandre Courbot
@ 2026-01-19 15:41 ` Gary Guo
0 siblings, 0 replies; 23+ messages in thread
From: Gary Guo @ 2026-01-19 15:41 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Lyude Paul
On Tue Dec 16, 2025 at 4:27 AM GMT, 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.
>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Reviewed-by: Alistair Popple <apopple@nvidia.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> drivers/gpu/nova-core/gsp/cmdq.rs | 2 +-
> drivers/gpu/nova-core/gsp/fw.rs | 54 ---------------------------------------
> 2 files changed, 1 insertion(+), 55 deletions(-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 03/10] gpu: nova-core: gsp: simplify sequencer opcode parsing
2025-12-16 4:27 [PATCH v2 00/10] gpu: nova-core: miscellaneous improvements Alexandre Courbot
2025-12-16 4:27 ` [PATCH v2 01/10] gpu: nova-core: gsp: warn if data remains after processing a message Alexandre Courbot
2025-12-16 4:27 ` [PATCH v2 02/10] gpu: nova-core: gsp: remove unnecessary Display impls Alexandre Courbot
@ 2025-12-16 4:27 ` Alexandre Courbot
2026-01-19 16:11 ` Gary Guo
2025-12-16 4:27 ` [PATCH v2 04/10] gpu: nova-core: gsp: remove unneeded sequencer trait Alexandre Courbot
` (6 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Alexandre Courbot @ 2025-12-16 4:27 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Alexandre Courbot, Lyude Paul
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`.
Reviewed-by: Lyude Paul <lyude@redhat.com>
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 de251a143f7b..1b0fcbdc77ba 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 03/10] gpu: nova-core: gsp: simplify sequencer opcode parsing
2025-12-16 4:27 ` [PATCH v2 03/10] gpu: nova-core: gsp: simplify sequencer opcode parsing Alexandre Courbot
@ 2026-01-19 16:11 ` Gary Guo
0 siblings, 0 replies; 23+ messages in thread
From: Gary Guo @ 2026-01-19 16:11 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Lyude Paul
On Tue Dec 16, 2025 at 4:27 AM GMT, 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`.
>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> drivers/gpu/nova-core/gsp/fw.rs | 40 +++++-----------------------------------
> 1 file changed, 5 insertions(+), 35 deletions(-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 04/10] gpu: nova-core: gsp: remove unneeded sequencer trait
2025-12-16 4:27 [PATCH v2 00/10] gpu: nova-core: miscellaneous improvements Alexandre Courbot
` (2 preceding siblings ...)
2025-12-16 4:27 ` [PATCH v2 03/10] gpu: nova-core: gsp: simplify sequencer opcode parsing Alexandre Courbot
@ 2025-12-16 4:27 ` Alexandre Courbot
2026-01-19 16:12 ` Gary Guo
2025-12-16 4:27 ` [PATCH v2 05/10] gpu: nova-core: gsp: derive `Debug` on more sequencer types Alexandre Courbot
` (5 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Alexandre Courbot @ 2025-12-16 4:27 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Alexandre Courbot, Lyude Paul
The `GspSeqCmdRunner` trait is never used as we never call the `run`
methods from generic code. Remove it.
Reviewed-by: Lyude Paul <lyude@redhat.com>
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 04/10] gpu: nova-core: gsp: remove unneeded sequencer trait
2025-12-16 4:27 ` [PATCH v2 04/10] gpu: nova-core: gsp: remove unneeded sequencer trait Alexandre Courbot
@ 2026-01-19 16:12 ` Gary Guo
0 siblings, 0 replies; 23+ messages in thread
From: Gary Guo @ 2026-01-19 16:12 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Lyude Paul
On Tue Dec 16, 2025 at 4:27 AM GMT, Alexandre Courbot wrote:
> The `GspSeqCmdRunner` trait is never used as we never call the `run`
> methods from generic code. Remove it.
>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> drivers/gpu/nova-core/gsp/sequencer.rs | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 05/10] gpu: nova-core: gsp: derive `Debug` on more sequencer types
2025-12-16 4:27 [PATCH v2 00/10] gpu: nova-core: miscellaneous improvements Alexandre Courbot
` (3 preceding siblings ...)
2025-12-16 4:27 ` [PATCH v2 04/10] gpu: nova-core: gsp: remove unneeded sequencer trait Alexandre Courbot
@ 2025-12-16 4:27 ` Alexandre Courbot
2026-01-19 16:12 ` Gary Guo
2025-12-16 4:27 ` [PATCH v2 06/10] gpu: nova-core: gsp: derive Zeroable for GspStaticConfigInfo Alexandre Courbot
` (4 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Alexandre Courbot @ 2025-12-16 4:27 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Alexandre Courbot, Lyude Paul
Being able to print these is useful when debugging the sequencer.
Reviewed-by: Lyude Paul <lyude@redhat.com>
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 1b0fcbdc77ba..09549f7db52d 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 05/10] gpu: nova-core: gsp: derive `Debug` on more sequencer types
2025-12-16 4:27 ` [PATCH v2 05/10] gpu: nova-core: gsp: derive `Debug` on more sequencer types Alexandre Courbot
@ 2026-01-19 16:12 ` Gary Guo
0 siblings, 0 replies; 23+ messages in thread
From: Gary Guo @ 2026-01-19 16:12 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Lyude Paul
On Tue Dec 16, 2025 at 4:27 AM GMT, Alexandre Courbot wrote:
> Being able to print these is useful when debugging the sequencer.
>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> drivers/gpu/nova-core/gsp/fw.rs | 10 +++++-----
> drivers/gpu/nova-core/gsp/sequencer.rs | 1 +
> 2 files changed, 6 insertions(+), 5 deletions(-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 06/10] gpu: nova-core: gsp: derive Zeroable for GspStaticConfigInfo
2025-12-16 4:27 [PATCH v2 00/10] gpu: nova-core: miscellaneous improvements Alexandre Courbot
` (4 preceding siblings ...)
2025-12-16 4:27 ` [PATCH v2 05/10] gpu: nova-core: gsp: derive `Debug` on more sequencer types Alexandre Courbot
@ 2025-12-16 4:27 ` Alexandre Courbot
2026-01-19 16:16 ` Gary Guo
2025-12-16 4:27 ` [PATCH v2 07/10] gpu: nova-core: firmware: fwsec: do not require bound device when unneeded Alexandre Courbot
` (3 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Alexandre Courbot @ 2025-12-16 4:27 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Alexandre Courbot, Lyude Paul
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.
Reviewed-by: Lyude Paul <lyude@redhat.com>
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 06/10] gpu: nova-core: gsp: derive Zeroable for GspStaticConfigInfo
2025-12-16 4:27 ` [PATCH v2 06/10] gpu: nova-core: gsp: derive Zeroable for GspStaticConfigInfo Alexandre Courbot
@ 2026-01-19 16:16 ` Gary Guo
0 siblings, 0 replies; 23+ messages in thread
From: Gary Guo @ 2026-01-19 16:16 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Lyude Paul
On Tue Dec 16, 2025 at 4:27 AM GMT, 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.
With latest pin-init changes you should be able to derive `Zeroable` on tuple
structs now, no need to convert.
Best,
Gary
>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> 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 {}
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 07/10] gpu: nova-core: firmware: fwsec: do not require bound device when unneeded
2025-12-16 4:27 [PATCH v2 00/10] gpu: nova-core: miscellaneous improvements Alexandre Courbot
` (5 preceding siblings ...)
2025-12-16 4:27 ` [PATCH v2 06/10] gpu: nova-core: gsp: derive Zeroable for GspStaticConfigInfo Alexandre Courbot
@ 2025-12-16 4:27 ` Alexandre Courbot
2026-01-19 15:50 ` Danilo Krummrich
2025-12-16 4:27 ` [PATCH v2 08/10] gpu: nova-core: use core library's CStr instead of kernel one Alexandre Courbot
` (2 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Alexandre Courbot @ 2025-12-16 4:27 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Alexandre Courbot, Lyude Paul
`run` doesn't require a bound device as its argument.
Reviewed-by: Lyude Paul <lyude@redhat.com>
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 07/10] gpu: nova-core: firmware: fwsec: do not require bound device when unneeded
2025-12-16 4:27 ` [PATCH v2 07/10] gpu: nova-core: firmware: fwsec: do not require bound device when unneeded Alexandre Courbot
@ 2026-01-19 15:50 ` Danilo Krummrich
0 siblings, 0 replies; 23+ messages in thread
From: Danilo Krummrich @ 2026-01-19 15:50 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Alice Ryhl, David Airlie, Simona Vetter, John Hubbard,
Alistair Popple, Joel Fernandes, Timur Tabi, Edwin Peer,
Eliot Courtney, nouveau, dri-devel, linux-kernel, rust-for-linux,
Lyude Paul
On Tue Dec 16, 2025 at 5:27 AM CET, 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,
Hm..the method also takes a &pci::Bar, so its either called from a bound context
or within a Revocable critical section.
Leaving the other argument as &Device<Bound> makes it obvious which one it is
(and should be).
> - ) -> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 08/10] gpu: nova-core: use core library's CStr instead of kernel one
2025-12-16 4:27 [PATCH v2 00/10] gpu: nova-core: miscellaneous improvements Alexandre Courbot
` (6 preceding siblings ...)
2025-12-16 4:27 ` [PATCH v2 07/10] gpu: nova-core: firmware: fwsec: do not require bound device when unneeded Alexandre Courbot
@ 2025-12-16 4:27 ` Alexandre Courbot
2026-01-19 16:17 ` Gary Guo
2025-12-16 4:27 ` [PATCH v2 09/10] gpu: nova-core: simplify str_from_null_terminated Alexandre Courbot
2025-12-16 4:27 ` [PATCH v2 10/10] gpu: nova-core: gsp: use available device reference Alexandre Courbot
9 siblings, 1 reply; 23+ messages in thread
From: Alexandre Courbot @ 2025-12-16 4:27 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Alexandre Courbot, Lyude Paul
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.
Reviewed-by: Lyude Paul <lyude@redhat.com>
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 08/10] gpu: nova-core: use core library's CStr instead of kernel one
2025-12-16 4:27 ` [PATCH v2 08/10] gpu: nova-core: use core library's CStr instead of kernel one Alexandre Courbot
@ 2026-01-19 16:17 ` Gary Guo
0 siblings, 0 replies; 23+ messages in thread
From: Gary Guo @ 2026-01-19 16:17 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Lyude Paul
On Tue Dec 16, 2025 at 4:27 AM GMT, 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.
>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> 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(-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 09/10] gpu: nova-core: simplify str_from_null_terminated
2025-12-16 4:27 [PATCH v2 00/10] gpu: nova-core: miscellaneous improvements Alexandre Courbot
` (7 preceding siblings ...)
2025-12-16 4:27 ` [PATCH v2 08/10] gpu: nova-core: use core library's CStr instead of kernel one Alexandre Courbot
@ 2025-12-16 4:27 ` Alexandre Courbot
2026-01-03 3:37 ` John Hubbard
2025-12-16 4:27 ` [PATCH v2 10/10] gpu: nova-core: gsp: use available device reference Alexandre Courbot
9 siblings, 1 reply; 23+ messages in thread
From: Alexandre Courbot @ 2025-12-16 4:27 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Alexandre Courbot, Lyude Paul
The core library's `CStr` has a `from_bytes_until_nul` method that we
can leverage to simplify this function.
Reviewed-by: Lyude Paul <lyude@redhat.com>
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 09/10] gpu: nova-core: simplify str_from_null_terminated
2025-12-16 4:27 ` [PATCH v2 09/10] gpu: nova-core: simplify str_from_null_terminated Alexandre Courbot
@ 2026-01-03 3:37 ` John Hubbard
2026-01-03 10:14 ` Danilo Krummrich
0 siblings, 1 reply; 23+ messages in thread
From: John Hubbard @ 2026-01-03 3:37 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter
Cc: Alistair Popple, Joel Fernandes, Timur Tabi, Edwin Peer,
Eliot Courtney, nouveau, dri-devel, linux-kernel, rust-for-linux,
Lyude Paul
On 12/15/25 8:27 PM, Alexandre Courbot wrote:
> The core library's `CStr` has a `from_bytes_until_nul` method that we
> can leverage to simplify this function.
>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> 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()
I guess I should have reviewed this patch, before creating my version of this.
I went so far as to delete this file entirely, see if you prefer that, it's
otherwise the same core idea, but with more cleanup. [1]
[1] https://lore.kernel.org/20260103013438.247759-1-jhubbard@nvidia.com
> .and_then(|cstr| cstr.to_str().ok())
> }
>
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 09/10] gpu: nova-core: simplify str_from_null_terminated
2026-01-03 3:37 ` John Hubbard
@ 2026-01-03 10:14 ` Danilo Krummrich
2026-01-14 13:43 ` Alexandre Courbot
0 siblings, 1 reply; 23+ messages in thread
From: Danilo Krummrich @ 2026-01-03 10:14 UTC (permalink / raw)
To: John Hubbard
Cc: Alexandre Courbot, Alice Ryhl, David Airlie, Simona Vetter,
Alistair Popple, Joel Fernandes, Timur Tabi, Edwin Peer,
Eliot Courtney, nouveau, dri-devel, linux-kernel, rust-for-linux,
Lyude Paul
On Sat Jan 3, 2026 at 4:37 AM CET, John Hubbard wrote:
> On 12/15/25 8:27 PM, Alexandre Courbot wrote:
>> The core library's `CStr` has a `from_bytes_until_nul` method that we
>> can leverage to simplify this function.
>>
>> Reviewed-by: Lyude Paul <lyude@redhat.com>
>> 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()
>
> I guess I should have reviewed this patch, before creating my version of this.
> I went so far as to delete this file entirely, see if you prefer that, it's
> otherwise the same core idea, but with more cleanup. [1]
>
> [1] https://lore.kernel.org/20260103013438.247759-1-jhubbard@nvidia.com
Yes, let's remove str_from_null_terminated() entirely.
>> .and_then(|cstr| cstr.to_str().ok())
Additionally, why do we return an Option here? While an error can only ever happen if
the given slice does not contain any NULL byte, I don't see why we discard the
error code.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 09/10] gpu: nova-core: simplify str_from_null_terminated
2026-01-03 10:14 ` Danilo Krummrich
@ 2026-01-14 13:43 ` Alexandre Courbot
0 siblings, 0 replies; 23+ messages in thread
From: Alexandre Courbot @ 2026-01-14 13:43 UTC (permalink / raw)
To: Danilo Krummrich, John Hubbard
Cc: Alexandre Courbot, Alice Ryhl, Simona Vetter, Alistair Popple,
Joel Fernandes, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux
On Sat Jan 3, 2026 at 7:14 PM JST, Danilo Krummrich wrote:
> On Sat Jan 3, 2026 at 4:37 AM CET, John Hubbard wrote:
>> On 12/15/25 8:27 PM, Alexandre Courbot wrote:
>>> The core library's `CStr` has a `from_bytes_until_nul` method that we
>>> can leverage to simplify this function.
>>>
>>> Reviewed-by: Lyude Paul <lyude@redhat.com>
>>> 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()
>>
>> I guess I should have reviewed this patch, before creating my version of this.
>> I went so far as to delete this file entirely, see if you prefer that, it's
>> otherwise the same core idea, but with more cleanup. [1]
>>
>> [1] https://lore.kernel.org/20260103013438.247759-1-jhubbard@nvidia.com
>
> Yes, let's remove str_from_null_terminated() entirely.
Removing that method is perfectly fine IMHO, it was only using in a
couple of places and is easily emulated.
>
>>> .and_then(|cstr| cstr.to_str().ok())
>
> Additionally, why do we return an Option here? While an error can only ever happen if
> the given slice does not contain any NULL byte, I don't see why we discard the
> error code.
I guess I didn't want to change the function's prototype, but yeah this
is sloppy and another good reason to get rid of it.
I'll respin the series once the patches in -fixes are visible in -next.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 10/10] gpu: nova-core: gsp: use available device reference
2025-12-16 4:27 [PATCH v2 00/10] gpu: nova-core: miscellaneous improvements Alexandre Courbot
` (8 preceding siblings ...)
2025-12-16 4:27 ` [PATCH v2 09/10] gpu: nova-core: simplify str_from_null_terminated Alexandre Courbot
@ 2025-12-16 4:27 ` Alexandre Courbot
2026-01-19 16:18 ` Gary Guo
9 siblings, 1 reply; 23+ messages in thread
From: Alexandre Courbot @ 2025-12-16 4:27 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux, Alexandre Courbot
We already have a regular reference to the `Device`, so we don't need to
repeatedly acquire one in this method.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/gsp/boot.rs | 32 +++++++-------------------------
1 file changed, 7 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 54937606b5b0..ca7efdaa753b 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -173,15 +173,10 @@ pub(crate) fn boot(
Some(libos_handle as u32),
Some((libos_handle >> 32) as u32),
)?;
- dev_dbg!(
- pdev.as_ref(),
- "GSP MBOX0: {:#x}, MBOX1: {:#x}\n",
- mbox0,
- mbox1
- );
+ dev_dbg!(dev, "GSP MBOX0: {:#x}, MBOX1: {:#x}\n", mbox0, mbox1);
dev_dbg!(
- pdev.as_ref(),
+ dev,
"Using SEC2 to load and run the booter_load firmware...\n"
);
@@ -193,19 +188,10 @@ pub(crate) fn boot(
Some(wpr_handle as u32),
Some((wpr_handle >> 32) as u32),
)?;
- dev_dbg!(
- pdev.as_ref(),
- "SEC2 MBOX0: {:#x}, MBOX1{:#x}\n",
- mbox0,
- mbox1
- );
+ dev_dbg!(dev, "SEC2 MBOX0: {:#x}, MBOX1{:#x}\n", mbox0, mbox1);
if mbox0 != 0 {
- dev_err!(
- pdev.as_ref(),
- "Booter-load failed with error {:#x}\n",
- mbox0
- );
+ dev_err!(dev, "Booter-load failed with error {:#x}\n", mbox0);
return Err(ENODEV);
}
@@ -219,11 +205,7 @@ pub(crate) fn boot(
Delta::from_secs(5),
)?;
- dev_dbg!(
- pdev.as_ref(),
- "RISC-V active? {}\n",
- gsp_falcon.is_riscv_active(bar),
- );
+ dev_dbg!(dev, "RISC-V active? {}\n", gsp_falcon.is_riscv_active(bar),);
// Create and run the GSP sequencer.
let seq_params = GspSequencerParams {
@@ -231,7 +213,7 @@ pub(crate) fn boot(
libos_dma_handle: libos_handle,
gsp_falcon,
sec2_falcon,
- dev: pdev.as_ref().into(),
+ dev: dev.into(),
bar,
};
GspSequencer::run(&mut self.cmdq, seq_params)?;
@@ -242,7 +224,7 @@ pub(crate) fn boot(
// Obtain and display basic GPU information.
let info = commands::get_gsp_info(&mut self.cmdq, bar)?;
dev_info!(
- pdev.as_ref(),
+ dev,
"GPU name: {}\n",
info.gpu_name().unwrap_or("invalid GPU name")
);
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 10/10] gpu: nova-core: gsp: use available device reference
2025-12-16 4:27 ` [PATCH v2 10/10] gpu: nova-core: gsp: use available device reference Alexandre Courbot
@ 2026-01-19 16:18 ` Gary Guo
0 siblings, 0 replies; 23+ messages in thread
From: Gary Guo @ 2026-01-19 16:18 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, Eliot Courtney, nouveau, dri-devel, linux-kernel,
rust-for-linux
On Tue Dec 16, 2025 at 4:27 AM GMT, Alexandre Courbot wrote:
> We already have a regular reference to the `Device`, so we don't need to
> repeatedly acquire one in this method.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> drivers/gpu/nova-core/gsp/boot.rs | 32 +++++++-------------------------
> 1 file changed, 7 insertions(+), 25 deletions(-)
^ permalink raw reply [flat|nested] 23+ messages in thread