[PATCH v3 09/14] gpu: nova-core: recover the GSP receive path from corrupt framing
From: John Hubbard <jhubbard@nvidia.com>
Date: 2026-09-03 03:15:39
Also in:
lkml
Subsystem:
core driver for nvidia gpus [rust], drm drivers, drm drivers and common infrastructure [rust], the rest · Maintainers:
Danilo Krummrich, Alexandre Courbot, David Airlie, Simona Vetter, Alice Ryhl, Linus Torvalds
A GSP message carries its length inside the checksummed region, so once the framing or the checksum fails, the length cannot be trusted to skip the message. Two paths left a bad message at the queue head. A framing or checksum failure returned without advancing the read pointer, so every later receive re-parsed the same message. A validly framed message whose typed payload failed to decode returned early and did the same. Poison the queue on a framing or checksum failure, log what was inconsistent, and fail every later receive, so the bad head is parsed once and recovery requires a reset. Advance the read pointer past a validly framed message whether or not its payload decodes. Assisted-by: Cursor:claude-opus-5 Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- drivers/gpu/nova-core/gsp/cmdq.rs | 96 +++++++++++++++++++++---------- 1 file changed, 66 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 5572224db233..ce4d6a111e68 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs@@ -2,7 +2,10 @@ mod continuation; -use core::mem; +use core::{ + cell::Cell, + mem, // +}; use kernel::{ device,
@@ -11,6 +14,7 @@ CoherentBox, DmaAddress, // }, + fmt, io::{ io_project, poll::read_poll_timeout,
@@ -531,6 +535,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>) -> impl PinInit<Self, Err dev: dev.into(), gsp_mem, seq: 0, + poisoned: Cell::new(false), }), })) })
@@ -624,6 +629,12 @@ struct CmdqInner { dev: ARef<device::Device>, /// Current command sequence number. seq: u32, + /// Set once a message with corrupt framing or a bad checksum is seen. Such a message has an + /// untrusted length, so the queue cannot be advanced past it, and every later receive fails + /// until the queue is torn down and reset. + /// + /// A [`Cell`], so the shared-borrow read path [`Self::wait_for_msg`] can set it. + poisoned: Cell<bool>, /// Memory area shared with the GSP for communicating commands and messages. gsp_mem: DmaGspMem, }
@@ -732,6 +743,19 @@ fn send_command<M>(&mut self, bar: Bar0<'_>, command: M) -> Result } } + /// Marks the queue unusable and returns the error every later receive fails with. + /// + /// `reason` names the inconsistency. Without it the failure is invisible, because the queue + /// just stops producing messages. + /// + /// Takes `&self` so the shared-borrow read path [`Self::wait_for_msg`] can call it. + fn poison(&self, reason: fmt::Arguments<'_>) -> Error { + dev_err!(&self.dev, "GSP RPC: receive: queue poisoned: {}\n", reason); + self.poisoned.set(true); + + EIO + } + /// Wait for a message to become available on the message queue. /// /// This works purely at the transport layer and does not interpret or validate the message
@@ -746,11 +770,13 @@ fn send_command<M>(&mut self, bar: Bar0<'_>, command: M) -> Result /// # Errors /// /// - `ETIMEDOUT` if `timeout` has elapsed before any message becomes available. - /// - `EIO` if there was some inconsistency (e.g. message shorter than advertised) on the - /// message queue. - /// - /// Error codes returned by the message constructor are propagated as-is. + /// - `EIO` if the framing or the checksum is invalid, or the queue was already poisoned by an + /// earlier such failure. Either failure poisons the queue, so recovery requires a reset. fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> { + if self.poisoned.get() { + return Err(EIO); + } + // Wait for a message to arrive from the GSP. let (slice_1, slice_2) = read_poll_timeout( || Ok(self.gsp_mem.driver_read_area()),
@@ -761,7 +787,12 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> { .map(|(slice_1, slice_2)| (slice_1.as_flattened(), slice_2.as_flattened()))?; // Extract the `GspMsgElement`. - let (header, slice_1) = GspMsgElement::from_bytes_prefix(slice_1).ok_or(EIO)?; + let Some((header, slice_1)) = GspMsgElement::from_bytes_prefix(slice_1) else { + return Err(self.poison(fmt!( + "read area of {} bytes is shorter than a message header", + slice_1.len() + ))); + }; dev_dbg!( &self.dev,
@@ -775,7 +806,11 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> { // Check that the driver read area is large enough for the message. if slice_1.len() + slice_2.len() < payload_length { - return Err(EIO); + return Err(self.poison(fmt!( + "message advertises {} payload bytes but only {} are readable", + payload_length, + slice_1.len() + slice_2.len() + ))); } // Cut the message slices down to the actual length of the message.
@@ -798,12 +833,10 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> { slice_2, ])) != 0 { - dev_err!( - &self.dev, - "GSP RPC: receive: Call {} - bad checksum\n", + return Err(self.poison(fmt!( + "message with sequence {} has a bad checksum", header.sequence() - ); - return Err(EIO); + ))); } Ok(GspMessage {
@@ -824,8 +857,8 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> { /// # Errors /// /// - `ETIMEDOUT` if `timeout` has elapsed before any message becomes available. - /// - `EIO` if there was some inconsistency (e.g. message shorter than advertised) on the - /// message queue. + /// - `EIO` if the queue is poisoned or the message fails framing or checksum validation (see + /// [`Self::wait_for_msg`]), or if the matched message is too short for `M::Message`. /// - `ERANGE` if the message was not the awaited reply. /// /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
@@ -838,23 +871,26 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M> let function = message.header.function(); let seq = message.header.sequence(); - // Bind the result rather than returning early. The read pointer must advance past this - // message on every path. + // Every path must advance the read pointer past this message, including a failed decode. let result = if matches!(function, Ok(f) if f == M::FUNCTION) { - 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()) - .inspect(|_| { - if !sbuffer.is_empty() { - dev_warn!( - &self.dev, - "GSP message {:?} has unprocessed data\n", - M::FUNCTION - ); - } - }) + match M::Message::from_bytes_prefix(message.contents.0) { + Some((cmd, contents_1)) => { + let mut sbuffer = SBufferIter::new_reader([contents_1, message.contents.1]); + + M::read(cmd, &mut sbuffer) + .map_err(|e| e.into()) + .inspect(|_| { + if !sbuffer.is_empty() { + dev_warn!( + &self.dev, + "GSP message {:?} has unprocessed data\n", + M::FUNCTION + ); + } + }) + } + None => Err(EIO), + } } else { self.classify_event(function, seq);
--
2.55.0