All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Hubbard <jhubbard@nvidia.com>
To: Danilo Krummrich <dakr@kernel.org>,
	Joel Fernandes <joel@joelfernandes.org>,
	Alexandre Courbot <acourbot@nvidia.com>
Cc: "Timur Tabi" <ttabi@nvidia.com>,
	"Alistair Popple" <apopple@nvidia.com>,
	"Eliot Courtney" <ecourtney@nvidia.com>,
	"Shashank Sharma" <shashanks@nvidia.com>,
	"Zhi Wang" <zhiw@nvidia.com>, "David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	nova-gpu@lists.linux.dev, LKML <linux-kernel@vger.kernel.org>,
	"John Hubbard" <jhubbard@nvidia.com>
Subject: [PATCH 12/17] gpu: nova-core: recover the GSP receive path from corrupt framing
Date: Fri,  7 Aug 2026 20:11:14 -0700	[thread overview]
Message-ID: <20260808031120.363869-13-jhubbard@nvidia.com> (raw)
In-Reply-To: <20260808031120.363869-1-jhubbard@nvidia.com>

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, 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 | 63 ++++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 3224079abf7e..fc4c229b8b9a 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -3,6 +3,7 @@
 mod continuation;
 
 use core::{
+    cell::Cell,
     mem,
     sync::atomic::{
         fence,
@@ -523,6 +524,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>) -> impl PinInit<Self, Err
                     gsp_mem,
                     elem_seq: 0,
                     rpc_seq: 0,
+                    poisoned: Cell::new(false),
                 }),
             }))
         })
@@ -622,6 +624,12 @@ struct CmdqInner {
     /// [`CmdqInner::receive_msg`] match that reply to the awaiting command. Advances once per
     /// logical command.
     rpc_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,
 }
@@ -748,11 +756,13 @@ fn send_command<M>(&mut self, bar: Bar0<'_>, command: M) -> Result<u32>
     /// # 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()),
@@ -763,7 +773,10 @@ 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 {
+            self.poisoned.set(true);
+            return Err(EIO);
+        };
 
         dev_dbg!(
             &self.dev,
@@ -777,6 +790,7 @@ 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 {
+            self.poisoned.set(true);
             return Err(EIO);
         }
 
@@ -805,6 +819,7 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
                 "GSP RPC: receive: Call {} - bad checksum\n",
                 header.sequence()
             );
+            self.poisoned.set(true);
             return Err(EIO);
         }
 
@@ -830,8 +845,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.
@@ -850,22 +865,26 @@ fn receive_msg<M: MessageFromGsp>(
         let func_matches = matches!(function, Ok(f) if f == M::FUNCTION);
         let matched = func_matches && expected_seq.is_none_or(|expected| seq == expected);
 
-        // Every path must advance the read pointer past this message.
+        // Every path must advance the read pointer past this message, including a failed decode.
         let result = if matched {
-            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 {
             Err(ERANGE)
         };
-- 
2.55.0


  parent reply	other threads:[~2026-08-08  3:11 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-08  3:11 [PATCH 00/17] nova-core: GPU interrupt support and GSP event delivery John Hubbard
2026-08-08  3:11 ` [PATCH 01/17] rust: sync: completion: add wait_for_completion_timeout() John Hubbard
2026-08-09  2:40   ` Alexandre Courbot
2026-08-09 21:43     ` John Hubbard
2026-08-08  3:11 ` [PATCH 02/17] rust: pci: expose the whole interrupt vector allocation John Hubbard
2026-08-09 13:27   ` Danilo Krummrich
2026-08-08  3:11 ` [PATCH 03/17] rust: pci: expose the allocated interrupt type John Hubbard
2026-08-09 13:24   ` Danilo Krummrich
2026-08-09 21:42     ` John Hubbard
2026-08-10 22:53       ` Danilo Krummrich
2026-08-10 22:55         ` John Hubbard
2026-08-11  3:09         ` John Hubbard
2026-08-08  3:11 ` [PATCH 04/17] gpu: nova-core: allocate PCI MSI vector during probe John Hubbard
2026-08-08  3:11 ` [PATCH 05/17] gpu: nova-core: add the GIN CPU interrupt tree and MSI EOI registers John Hubbard
2026-08-08  3:11 ` [PATCH 06/17] gpu: nova-core: add the GIN interrupt tree API John Hubbard
2026-08-08  3:11 ` [PATCH 07/17] gpu: nova-core: add the per-architecture GIN CPU interrupt HAL John Hubbard
2026-08-08  3:11 ` [PATCH 08/17] gpu: nova-core: allocate interrupt vectors for the serviced subtrees John Hubbard
2026-08-08  3:11 ` [PATCH 09/17] gpu: nova-core: add an interrupt delivery self-test John Hubbard
2026-08-08  3:11 ` [PATCH 10/17] gpu: nova-core: dispatch GSP events instead of discarding them John Hubbard
2026-08-08  3:11 ` [PATCH 11/17] gpu: nova-core: match GSP RPC replies by sequence, not just function John Hubbard
2026-08-08  3:11 ` John Hubbard [this message]
2026-08-08  3:11 ` [PATCH 13/17] gpu: nova-core: bound a GSP wait by a single deadline John Hubbard
2026-08-08  3:11 ` [PATCH 14/17] gpu: nova-core: drive GSP events with the SWGEN0 interrupt John Hubbard
2026-08-08  3:11 ` [PATCH 15/17] gpu: nova-core: retrigger the GSP falcon and clear every latched cause John Hubbard
2026-08-08  3:11 ` [PATCH 16/17] gpu: nova-core: add KUnit tests for the interrupt tree and HALs John Hubbard
2026-08-08  3:11 ` [PATCH 17/17] gpu: nova-core: document the GIN interrupt controller and GSP events John Hubbard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260808031120.363869-13-jhubbard@nvidia.com \
    --to=jhubbard@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=ecourtney@nvidia.com \
    --cc=gary@garyguo.net \
    --cc=joel@joelfernandes.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=nova-gpu@lists.linux.dev \
    --cc=ojeda@kernel.org \
    --cc=shashanks@nvidia.com \
    --cc=simona@ffwll.ch \
    --cc=tmgross@umich.edu \
    --cc=ttabi@nvidia.com \
    --cc=zhiw@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.