[PATCH v2 0/9] gpu: nova-core: gsp: add continuation record support

[PATCH v2 0/9] gpu: nova-core: gsp: add continuation record support

[Eliot Courtney]

GSP commands over 16 pages need to be sent using "continuation records"
which essentially means splitting the payload over multiple commands.

This series adds a command type `ContinuationRecord` which just writes
its header and whatever payload it is given. It also adds types
`SplitState` and `SplitCommand` which support splitting a large RPC
into smaller ones while transparently letting regular sized RPCs be sent
without extra copies.

The send pathway uses `SplitState` to send all commands, but if
the command fits into 16 pages, it still writes directly into the
command queue. If it is larger than 16 pages and needs continuation
records, it writes into a staging buffer, so there is one copy.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
Changes in v2:
- Added doccoments
- Renamed driver_bytes_available_to_write to driver_write_area_size
- allocate_command_with_timeout merged allocate_command with timeout parameter
- Replaced hardcoded GSP_PAGE_SIZE * 16 with bindings
- Changed oversized command error from EIO to EMSGSIZE
- Added EMSGSIZE to kernel/error.rs
- Split WrappingCommand functionality into SplitState + SplitCommand enum
- Made max_size a const (MAX_CMD_SIZE)
- Removed send_continuation_record + added comment for type inference
- send_single_command now consumes the command
- Extracted command_size + used in SplitState 
- Link to v1: https://lore.kernel.org/r/20260212-cmdq-continuation-v1-0-73079ded55e6@nvidia.com

---
Eliot Courtney (9):
      gpu: nova-core: gsp: sort MsgFunction variants alphabetically
      gpu: nova-core: gsp: add mechanism to wait for space on command queue
      rust: add EMSGSIZE error code
      gpu: nova-core: gsp: add checking oversized commands
      gpu: nova-core: gsp: clarify invariant on command queue
      gpu: nova-core: gsp: unconditionally call variable payload handling
      gpu: nova-core: gsp: add command_size helper
      gpu: nova-core: gsp: support large RPCs via continuation record
      gpu: nova-core: gsp: add tests for SplitState

 drivers/gpu/nova-core/gsp/cmdq.rs                 | 118 +++++++---
 drivers/gpu/nova-core/gsp/commands.rs             | 256 +++++++++++++++++++++-
 drivers/gpu/nova-core/gsp/fw.rs                   | 106 +++++----
 drivers/gpu/nova-core/gsp/fw/r570_144/bindings.rs |   1 +
 rust/kernel/error.rs                              |   1 +
 5 files changed, 408 insertions(+), 74 deletions(-)
---
base-commit: 956b9cbd7f156c8672dac94a00de3c6a0939c692
change-id: 20260203-cmdq-continuation-b99f3d5966c3

Best regards,
-- 
Eliot Courtney <ecourtney@nvidia.com>

[PATCH v2 1/9] gpu: nova-core: gsp: sort MsgFunction variants alphabetically

[Eliot Courtney]

There is no particular order required here and keeping them alphabetical
will help preventing future mistakes.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/fw.rs | 97 +++++++++++++++++++++--------------------
 1 file changed, 50 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 83ff91614e36..5b96f5e622a4 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -192,34 +192,34 @@ pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
 #[repr(u32)]
 pub(crate) enum MsgFunction {
     // Common function codes
-    Nop = bindings::NV_VGPU_MSG_FUNCTION_NOP,
-    SetGuestSystemInfo = bindings::NV_VGPU_MSG_FUNCTION_SET_GUEST_SYSTEM_INFO,
-    AllocRoot = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_ROOT,
+    AllocChannelDma = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_CHANNEL_DMA,
+    AllocCtxDma = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_CTX_DMA,
     AllocDevice = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_DEVICE,
     AllocMemory = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_MEMORY,
-    AllocCtxDma = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_CTX_DMA,
-    AllocChannelDma = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_CHANNEL_DMA,
-    MapMemory = bindings::NV_VGPU_MSG_FUNCTION_MAP_MEMORY,
-    BindCtxDma = bindings::NV_VGPU_MSG_FUNCTION_BIND_CTX_DMA,
     AllocObject = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_OBJECT,
+    AllocRoot = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_ROOT,
+    BindCtxDma = bindings::NV_VGPU_MSG_FUNCTION_BIND_CTX_DMA,
     Free = bindings::NV_VGPU_MSG_FUNCTION_FREE,
-    Log = bindings::NV_VGPU_MSG_FUNCTION_LOG,
     GetGspStaticInfo = bindings::NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO,
-    SetRegistry = bindings::NV_VGPU_MSG_FUNCTION_SET_REGISTRY,
-    GspSetSystemInfo = bindings::NV_VGPU_MSG_FUNCTION_GSP_SET_SYSTEM_INFO,
+    GetStaticInfo = bindings::NV_VGPU_MSG_FUNCTION_GET_STATIC_INFO,
     GspInitPostObjGpu = bindings::NV_VGPU_MSG_FUNCTION_GSP_INIT_POST_OBJGPU,
     GspRmControl = bindings::NV_VGPU_MSG_FUNCTION_GSP_RM_CONTROL,
-    GetStaticInfo = bindings::NV_VGPU_MSG_FUNCTION_GET_STATIC_INFO,
+    GspSetSystemInfo = bindings::NV_VGPU_MSG_FUNCTION_GSP_SET_SYSTEM_INFO,
+    Log = bindings::NV_VGPU_MSG_FUNCTION_LOG,
+    MapMemory = bindings::NV_VGPU_MSG_FUNCTION_MAP_MEMORY,
+    Nop = bindings::NV_VGPU_MSG_FUNCTION_NOP,
+    SetGuestSystemInfo = bindings::NV_VGPU_MSG_FUNCTION_SET_GUEST_SYSTEM_INFO,
+    SetRegistry = bindings::NV_VGPU_MSG_FUNCTION_SET_REGISTRY,
 
     // Event codes
     GspInitDone = bindings::NV_VGPU_MSG_EVENT_GSP_INIT_DONE,
+    GspLockdownNotice = bindings::NV_VGPU_MSG_EVENT_GSP_LOCKDOWN_NOTICE,
+    GspPostNoCat = bindings::NV_VGPU_MSG_EVENT_GSP_POST_NOCAT_RECORD,
     GspRunCpuSequencer = bindings::NV_VGPU_MSG_EVENT_GSP_RUN_CPU_SEQUENCER,
-    PostEvent = bindings::NV_VGPU_MSG_EVENT_POST_EVENT,
-    RcTriggered = bindings::NV_VGPU_MSG_EVENT_RC_TRIGGERED,
     MmuFaultQueued = bindings::NV_VGPU_MSG_EVENT_MMU_FAULT_QUEUED,
     OsErrorLog = bindings::NV_VGPU_MSG_EVENT_OS_ERROR_LOG,
-    GspPostNoCat = bindings::NV_VGPU_MSG_EVENT_GSP_POST_NOCAT_RECORD,
-    GspLockdownNotice = bindings::NV_VGPU_MSG_EVENT_GSP_LOCKDOWN_NOTICE,
+    PostEvent = bindings::NV_VGPU_MSG_EVENT_POST_EVENT,
+    RcTriggered = bindings::NV_VGPU_MSG_EVENT_RC_TRIGGERED,
     UcodeLibOsPrint = bindings::NV_VGPU_MSG_EVENT_UCODE_LIBOS_PRINT,
 }
 
@@ -227,34 +227,34 @@ 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::AllocChannelDma => write!(f, "ALLOC_CHANNEL_DMA"),
+            MsgFunction::AllocCtxDma => write!(f, "ALLOC_CTX_DMA"),
             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::AllocRoot => write!(f, "ALLOC_ROOT"),
+            MsgFunction::BindCtxDma => write!(f, "BIND_CTX_DMA"),
             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::GetStaticInfo => write!(f, "GET_STATIC_INFO"),
             MsgFunction::GspInitPostObjGpu => write!(f, "GSP_INIT_POST_OBJGPU"),
             MsgFunction::GspRmControl => write!(f, "GSP_RM_CONTROL"),
-            MsgFunction::GetStaticInfo => write!(f, "GET_STATIC_INFO"),
+            MsgFunction::GspSetSystemInfo => write!(f, "GSP_SET_SYSTEM_INFO"),
+            MsgFunction::Log => write!(f, "LOG"),
+            MsgFunction::MapMemory => write!(f, "MAP_MEMORY"),
+            MsgFunction::Nop => write!(f, "NOP"),
+            MsgFunction::SetGuestSystemInfo => write!(f, "SET_GUEST_SYSTEM_INFO"),
+            MsgFunction::SetRegistry => write!(f, "SET_REGISTRY"),
 
             // Event codes
             MsgFunction::GspInitDone => write!(f, "INIT_DONE"),
+            MsgFunction::GspLockdownNotice => write!(f, "LOCKDOWN_NOTICE"),
+            MsgFunction::GspPostNoCat => write!(f, "NOCAT"),
             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::PostEvent => write!(f, "POST_EVENT"),
+            MsgFunction::RcTriggered => write!(f, "RC_TRIGGERED"),
             MsgFunction::UcodeLibOsPrint => write!(f, "LIBOS_PRINT"),
         }
     }
@@ -265,38 +265,41 @@ impl TryFrom<u32> for MsgFunction {
 
     fn try_from(value: u32) -> Result<MsgFunction> {
         match value {
-            bindings::NV_VGPU_MSG_FUNCTION_NOP => Ok(MsgFunction::Nop),
-            bindings::NV_VGPU_MSG_FUNCTION_SET_GUEST_SYSTEM_INFO => {
-                Ok(MsgFunction::SetGuestSystemInfo)
-            }
-            bindings::NV_VGPU_MSG_FUNCTION_ALLOC_ROOT => Ok(MsgFunction::AllocRoot),
+            // Common function codes
+            bindings::NV_VGPU_MSG_FUNCTION_ALLOC_CHANNEL_DMA => Ok(MsgFunction::AllocChannelDma),
+            bindings::NV_VGPU_MSG_FUNCTION_ALLOC_CTX_DMA => Ok(MsgFunction::AllocCtxDma),
             bindings::NV_VGPU_MSG_FUNCTION_ALLOC_DEVICE => Ok(MsgFunction::AllocDevice),
             bindings::NV_VGPU_MSG_FUNCTION_ALLOC_MEMORY => Ok(MsgFunction::AllocMemory),
-            bindings::NV_VGPU_MSG_FUNCTION_ALLOC_CTX_DMA => Ok(MsgFunction::AllocCtxDma),
-            bindings::NV_VGPU_MSG_FUNCTION_ALLOC_CHANNEL_DMA => Ok(MsgFunction::AllocChannelDma),
-            bindings::NV_VGPU_MSG_FUNCTION_MAP_MEMORY => Ok(MsgFunction::MapMemory),
-            bindings::NV_VGPU_MSG_FUNCTION_BIND_CTX_DMA => Ok(MsgFunction::BindCtxDma),
             bindings::NV_VGPU_MSG_FUNCTION_ALLOC_OBJECT => Ok(MsgFunction::AllocObject),
+            bindings::NV_VGPU_MSG_FUNCTION_ALLOC_ROOT => Ok(MsgFunction::AllocRoot),
+            bindings::NV_VGPU_MSG_FUNCTION_BIND_CTX_DMA => Ok(MsgFunction::BindCtxDma),
             bindings::NV_VGPU_MSG_FUNCTION_FREE => Ok(MsgFunction::Free),
-            bindings::NV_VGPU_MSG_FUNCTION_LOG => Ok(MsgFunction::Log),
             bindings::NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO => Ok(MsgFunction::GetGspStaticInfo),
-            bindings::NV_VGPU_MSG_FUNCTION_SET_REGISTRY => Ok(MsgFunction::SetRegistry),
-            bindings::NV_VGPU_MSG_FUNCTION_GSP_SET_SYSTEM_INFO => Ok(MsgFunction::GspSetSystemInfo),
+            bindings::NV_VGPU_MSG_FUNCTION_GET_STATIC_INFO => Ok(MsgFunction::GetStaticInfo),
             bindings::NV_VGPU_MSG_FUNCTION_GSP_INIT_POST_OBJGPU => {
                 Ok(MsgFunction::GspInitPostObjGpu)
             }
             bindings::NV_VGPU_MSG_FUNCTION_GSP_RM_CONTROL => Ok(MsgFunction::GspRmControl),
-            bindings::NV_VGPU_MSG_FUNCTION_GET_STATIC_INFO => Ok(MsgFunction::GetStaticInfo),
+            bindings::NV_VGPU_MSG_FUNCTION_GSP_SET_SYSTEM_INFO => Ok(MsgFunction::GspSetSystemInfo),
+            bindings::NV_VGPU_MSG_FUNCTION_LOG => Ok(MsgFunction::Log),
+            bindings::NV_VGPU_MSG_FUNCTION_MAP_MEMORY => Ok(MsgFunction::MapMemory),
+            bindings::NV_VGPU_MSG_FUNCTION_NOP => Ok(MsgFunction::Nop),
+            bindings::NV_VGPU_MSG_FUNCTION_SET_GUEST_SYSTEM_INFO => {
+                Ok(MsgFunction::SetGuestSystemInfo)
+            }
+            bindings::NV_VGPU_MSG_FUNCTION_SET_REGISTRY => Ok(MsgFunction::SetRegistry),
+
+            // Event codes
             bindings::NV_VGPU_MSG_EVENT_GSP_INIT_DONE => Ok(MsgFunction::GspInitDone),
+            bindings::NV_VGPU_MSG_EVENT_GSP_LOCKDOWN_NOTICE => Ok(MsgFunction::GspLockdownNotice),
+            bindings::NV_VGPU_MSG_EVENT_GSP_POST_NOCAT_RECORD => Ok(MsgFunction::GspPostNoCat),
             bindings::NV_VGPU_MSG_EVENT_GSP_RUN_CPU_SEQUENCER => {
                 Ok(MsgFunction::GspRunCpuSequencer)
             }
-            bindings::NV_VGPU_MSG_EVENT_POST_EVENT => Ok(MsgFunction::PostEvent),
-            bindings::NV_VGPU_MSG_EVENT_RC_TRIGGERED => Ok(MsgFunction::RcTriggered),
             bindings::NV_VGPU_MSG_EVENT_MMU_FAULT_QUEUED => Ok(MsgFunction::MmuFaultQueued),
             bindings::NV_VGPU_MSG_EVENT_OS_ERROR_LOG => Ok(MsgFunction::OsErrorLog),
-            bindings::NV_VGPU_MSG_EVENT_GSP_POST_NOCAT_RECORD => Ok(MsgFunction::GspPostNoCat),
-            bindings::NV_VGPU_MSG_EVENT_GSP_LOCKDOWN_NOTICE => Ok(MsgFunction::GspLockdownNotice),
+            bindings::NV_VGPU_MSG_EVENT_POST_EVENT => Ok(MsgFunction::PostEvent),
+            bindings::NV_VGPU_MSG_EVENT_RC_TRIGGERED => Ok(MsgFunction::RcTriggered),
             bindings::NV_VGPU_MSG_EVENT_UCODE_LIBOS_PRINT => Ok(MsgFunction::UcodeLibOsPrint),
             _ => Err(EINVAL),
         }

-- 
2.53.0

[PATCH v2 2/9] gpu: nova-core: gsp: add mechanism to wait for space on command queue

[Eliot Courtney]

Add a timeout to `allocate_command` which waits for space on the GSP
command queue. It uses a similar timeout to nouveau.

This lets `send_command` wait for space to free up in the command queue.
This is required to support continuation records which can fill up the
queue.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 46819a82a51a..f7f3cf2c7443 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -243,6 +243,19 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
         }
     }
 
+    /// Returns the size of the region of the CPU message queue that the driver is currently allowed
+    /// to write to, in bytes.
+    fn driver_write_area_size(&self) -> usize {
+        let tx = self.cpu_write_ptr();
+        let rx = self.gsp_read_ptr();
+
+        // `rx` and `tx` are both in `0..MSGQ_NUM_PAGES` per the invariants of `gsp_read_ptr` and
+        // `cpu_write_ptr`. The minimum value case is where `rx == 0` and `tx == MSGQ_NUM_PAGES -
+        // 1`, which gives `0 + MSGQ_NUM_PAGES - (MSGQ_NUM_PAGES - 1) - 1 == 0`.
+        let slots = (rx + MSGQ_NUM_PAGES - tx - 1) % MSGQ_NUM_PAGES;
+        num::u32_as_usize(slots) * GSP_PAGE_SIZE
+    }
+
     /// Returns the region of the GSP message queue that the driver is currently allowed to read
     /// from.
     ///
@@ -268,15 +281,22 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
     }
 
     /// Allocates a region on the command queue that is large enough to send a command of `size`
-    /// bytes.
+    /// bytes, waiting for space to become available based on the provided timeout.
     ///
     /// This returns a [`GspCommand`] ready to be written to by the caller.
     ///
     /// # Errors
     ///
-    /// - `EAGAIN` if the driver area is too small to hold the requested command.
+    /// - `ETIMEDOUT` if space does not become available within the timeout.
     /// - `EIO` if the command header is not properly aligned.
-    fn allocate_command(&mut self, size: usize) -> Result<GspCommand<'_>> {
+    fn allocate_command(&mut self, size: usize, timeout: Delta) -> Result<GspCommand<'_>> {
+        read_poll_timeout(
+            || Ok(self.driver_write_area_size()),
+            |available_bytes| *available_bytes >= size_of::<GspMsgElement>() + size,
+            Delta::ZERO,
+            timeout,
+        )?;
+
         // Get the current writable area as an array of bytes.
         let (slice_1, slice_2) = {
             let (slice_1, slice_2) = self.driver_write_area();
@@ -285,13 +305,6 @@ fn allocate_command(&mut self, size: usize) -> Result<GspCommand<'_>> {
             (slice_1.as_flattened_mut(), slice_2.as_flattened_mut())
         };
 
-        // If the GSP is still processing previous messages the shared region
-        // may be full in which case we will have to retry once the GSP has
-        // processed the existing commands.
-        if size_of::<GspMsgElement>() + size > slice_1.len() + slice_2.len() {
-            return Err(EAGAIN);
-        }
-
         // Extract area for the `GspMsgElement`.
         let (header, slice_1) = GspMsgElement::from_bytes_mut_prefix(slice_1).ok_or(EIO)?;
 
@@ -484,7 +497,7 @@ fn notify_gsp(bar: &Bar0) {
     ///
     /// # Errors
     ///
-    /// - `EAGAIN` if there was not enough space in the command queue to send the command.
+    /// - `ETIMEDOUT` if space does not become available within the timeout.
     /// - `EIO` if the variable payload requested by the command has not been entirely
     ///   written to by its [`CommandToGsp::init_variable_payload`] method.
     ///
@@ -496,7 +509,9 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
         Error: From<M::InitError>,
     {
         let command_size = size_of::<M::Command>() + command.variable_payload_len();
-        let dst = self.gsp_mem.allocate_command(command_size)?;
+        let dst = self
+            .gsp_mem
+            .allocate_command(command_size, Delta::from_secs(1))?;
 
         // Extract area for the command itself.
         let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;

-- 
2.53.0

[PATCH v2 3/9] rust: add EMSGSIZE error code

[Eliot Courtney]

Add the EMSGSIZE error code, which indicates that a message is too
long.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 rust/kernel/error.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 258b12afdcba..10fcf1f0404d 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -67,6 +67,7 @@ macro_rules! declare_err {
     declare_err!(EDOM, "Math argument out of domain of func.");
     declare_err!(ERANGE, "Math result not representable.");
     declare_err!(EOVERFLOW, "Value too large for defined data type.");
+    declare_err!(EMSGSIZE, "Message too long.");
     declare_err!(ETIMEDOUT, "Connection timed out.");
     declare_err!(ERESTARTSYS, "Restart the system call.");
     declare_err!(ERESTARTNOINTR, "System call was interrupted by a signal and will be restarted.");

-- 
2.53.0

[PATCH v2 4/9] gpu: nova-core: gsp: add checking oversized commands

[Eliot Courtney]

The limit is 16 pages for a single command sent to the GSP. Return an
error if `allocate_command` is called with a too large size.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs                 | 7 ++++++-
 drivers/gpu/nova-core/gsp/fw.rs                   | 4 ++++
 drivers/gpu/nova-core/gsp/fw/r570_144/bindings.rs | 1 +
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index f7f3cf2c7443..ae4aa7562dee 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -33,7 +33,8 @@
             GspMsgElement,
             MsgFunction,
             MsgqRxHeader,
-            MsgqTxHeader, //
+            MsgqTxHeader,
+            GSP_MSG_QUEUE_ELEMENT_SIZE_MAX, //
         },
         PteArray,
         GSP_PAGE_SHIFT,
@@ -287,9 +288,13 @@ fn driver_write_area_size(&self) -> usize {
     ///
     /// # Errors
     ///
+    /// - `EMSGSIZE` if the command is larger than [`GSP_MSG_QUEUE_ELEMENT_SIZE_MAX`].
     /// - `ETIMEDOUT` if space does not become available within the timeout.
     /// - `EIO` if the command header is not properly aligned.
     fn allocate_command(&mut self, size: usize, timeout: Delta) -> Result<GspCommand<'_>> {
+        if size_of::<GspMsgElement>() + size > GSP_MSG_QUEUE_ELEMENT_SIZE_MAX {
+            return Err(EMSGSIZE);
+        }
         read_poll_timeout(
             || Ok(self.driver_write_area_size()),
             |available_bytes| *available_bytes >= size_of::<GspMsgElement>() + size,
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 5b96f5e622a4..c604f423fff3 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -40,6 +40,10 @@
     },
 };
 
+/// Maximum size of a single GSP message queue element in bytes.
+pub(crate) const GSP_MSG_QUEUE_ELEMENT_SIZE_MAX: usize =
+    num::u32_as_usize(bindings::GSP_MSG_QUEUE_ELEMENT_SIZE_MAX);
+
 /// Empty type to group methods related to heap parameters for running the GSP firmware.
 enum GspFwHeapParams {}
 
diff --git a/drivers/gpu/nova-core/gsp/fw/r570_144/bindings.rs b/drivers/gpu/nova-core/gsp/fw/r570_144/bindings.rs
index 6d25fe0bffa9..334e8be5fde8 100644
--- a/drivers/gpu/nova-core/gsp/fw/r570_144/bindings.rs
+++ b/drivers/gpu/nova-core/gsp/fw/r570_144/bindings.rs
@@ -43,6 +43,7 @@ fn fmt(&self, fmt: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
 pub const GSP_FW_WPR_META_REVISION: u32 = 1;
 pub const GSP_FW_WPR_META_MAGIC: i64 = -2577556379034558285;
 pub const REGISTRY_TABLE_ENTRY_TYPE_DWORD: u32 = 1;
+pub const GSP_MSG_QUEUE_ELEMENT_SIZE_MAX: u32 = 65536;
 pub type __u8 = ffi::c_uchar;
 pub type __u16 = ffi::c_ushort;
 pub type __u32 = ffi::c_uint;

-- 
2.53.0

[PATCH v2 5/9] gpu: nova-core: gsp: clarify invariant on command queue

[Eliot Courtney]

Clarify why using only the first returned slice from allocate_command
for the message headers is okay.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index ae4aa7562dee..08b8c04909a6 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -518,7 +518,9 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
             .gsp_mem
             .allocate_command(command_size, Delta::from_secs(1))?;
 
-        // Extract area for the command itself.
+        // Extract area for the command itself. The GSP message header and the command header
+        // together are guaranteed to fit entirely into a single page, so it's ok to only look
+        // at `dst.contents.0` here.
         let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;
 
         // Fill the header and command in-place.

-- 
2.53.0

[PATCH v2 6/9] gpu: nova-core: gsp: unconditionally call variable payload handling

[Eliot Courtney]

Unconditionally call the variable length payload code, which is a no-op
if there is no such payload but could defensively catch some coding
errors by e.g. checking that the allocated size is completely filled.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 08b8c04909a6..1547687834f8 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -532,16 +532,14 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
             command.init().__init(core::ptr::from_mut(cmd))?;
         }
 
-        // Fill the variable-length payload.
-        if command_size > size_of::<M::Command>() {
-            let mut sbuffer =
-                SBufferIter::new_writer([&mut payload_1[..], &mut dst.contents.1[..]]);
-            command.init_variable_payload(&mut sbuffer)?;
-
-            if !sbuffer.is_empty() {
-                return Err(EIO);
-            }
+        // Fill the variable-length payload, which may be empty.
+        let mut sbuffer = SBufferIter::new_writer([&mut payload_1[..], &mut dst.contents.1[..]]);
+        command.init_variable_payload(&mut sbuffer)?;
+
+        if !sbuffer.is_empty() {
+            return Err(EIO);
         }
+        drop(sbuffer);
 
         // Compute checksum now that the whole message is ready.
         dst.header

-- 
2.53.0

[PATCH v2 7/9] gpu: nova-core: gsp: add command_size helper

[Eliot Courtney]

Add a helper function which computes the size of a command.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 1547687834f8..f832252ae45c 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -437,6 +437,15 @@ struct GspMessage<'a> {
     contents: (&'a [u8], &'a [u8]),
 }
 
+/// Computes the total size of the command (not including the [`GspMsgElement`] header), including
+/// its variable-length payload.
+fn command_size<M>(command: &M) -> usize
+where
+    M: CommandToGsp,
+{
+    size_of::<M::Command>() + command.variable_payload_len()
+}
+
 /// GSP command queue.
 ///
 /// Provides the ability to send commands and receive messages from the GSP using a shared memory
@@ -513,7 +522,7 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
         Error: From<M::InitError>,
     {
-        let command_size = size_of::<M::Command>() + command.variable_payload_len();
+        let command_size = command_size(&command);
         let dst = self
             .gsp_mem
             .allocate_command(command_size, Delta::from_secs(1))?;

-- 
2.53.0

[PATCH v2 8/9] gpu: nova-core: gsp: support large RPCs via continuation record

[Eliot Courtney]

Splits large RPCs if necessary and sends the remaining parts using
continuation records. RPCs that do not need continuation records
continue to write directly into the command buffer. Ones that do write
into a staging buffer first, so there is one copy.

Continuation record for receive is not necessary to support at the
moment because those replies do not need to be read and are currently
drained by retrying `receive_msg` on ERANGE.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs     |  43 +++++++++-
 drivers/gpu/nova-core/gsp/commands.rs | 143 +++++++++++++++++++++++++++++++++-
 drivers/gpu/nova-core/gsp/fw.rs       |   5 ++
 3 files changed, 187 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index f832252ae45c..402b0b9418ab 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -29,6 +29,10 @@
 use crate::{
     driver::Bar0,
     gsp::{
+        commands::{
+            ContinuationRecord,
+            SplitState, //
+        },
         fw::{
             GspMsgElement,
             MsgFunction,
@@ -439,7 +443,7 @@ struct GspMessage<'a> {
 
 /// Computes the total size of the command (not including the [`GspMsgElement`] header), including
 /// its variable-length payload.
-fn command_size<M>(command: &M) -> usize
+pub(crate) fn command_size<M>(command: &M) -> usize
 where
     M: CommandToGsp,
 {
@@ -507,7 +511,7 @@ fn notify_gsp(bar: &Bar0) {
             .write(bar);
     }
 
-    /// Sends `command` to the GSP.
+    /// Sends `command` to the GSP, without splitting it.
     ///
     /// # Errors
     ///
@@ -516,7 +520,7 @@ fn notify_gsp(bar: &Bar0) {
     ///   written to by its [`CommandToGsp::init_variable_payload`] method.
     ///
     /// Error codes returned by the command initializers are propagated as-is.
-    pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
+    fn send_single_command<M>(&mut self, bar: &Bar0, command: M) -> Result
     where
         M: CommandToGsp,
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
@@ -575,6 +579,39 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
         Ok(())
     }
 
+    /// Sends `command` to the GSP.
+    ///
+    /// The command may be split into multiple messages if it is large.
+    ///
+    /// # Errors
+    ///
+    /// - `ETIMEDOUT` if space does not become available within the timeout.
+    /// - `EIO` if the variable payload requested by the command has not been entirely
+    ///   written to by its [`CommandToGsp::init_variable_payload`] method.
+    ///
+    /// Error codes returned by the command initializers are propagated as-is.
+    pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
+    where
+        M: CommandToGsp,
+        Error: From<M::InitError>,
+    {
+        let mut state = SplitState::new(&command)?;
+
+        self.send_single_command(bar, state.command(command))?;
+
+        while let Some(continuation) = state.next_continuation_record() {
+            dev_dbg!(
+                &self.dev,
+                "GSP RPC: send continuation: size=0x{:x}\n",
+                command_size(&continuation),
+            );
+            // Turbofish needed because the compiler cannot infer M here.
+            self.send_single_command::<ContinuationRecord<'_>>(bar, continuation)?;
+        }
+
+        Ok(())
+    }
+
     /// 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
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 8f270eca33be..e722c6091ec8 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -4,6 +4,7 @@
     array,
     convert::Infallible,
     ffi::FromBytesUntilNulError,
+    marker::PhantomData,
     str::Utf8Error, //
 };
 
@@ -22,13 +23,16 @@
     driver::Bar0,
     gsp::{
         cmdq::{
+            command_size,
             Cmdq,
             CommandToGsp,
             MessageFromGsp, //
         },
         fw::{
             commands::*,
-            MsgFunction, //
+            GspMsgElement,
+            MsgFunction,
+            GSP_MSG_QUEUE_ELEMENT_SIZE_MAX, //
         },
     },
     sbuffer::SBufferIter,
@@ -242,3 +246,140 @@ pub(crate) fn get_gsp_info(cmdq: &mut Cmdq, bar: &Bar0) -> Result<GetGspStaticIn
         }
     }
 }
+
+/// The `ContinuationRecord` command.
+pub(crate) struct ContinuationRecord<'a> {
+    data: &'a [u8],
+}
+
+impl<'a> ContinuationRecord<'a> {
+    /// Creates a new `ContinuationRecord` command with the given data.
+    pub(crate) fn new(data: &'a [u8]) -> Self {
+        Self { data }
+    }
+}
+
+impl<'a> CommandToGsp for ContinuationRecord<'a> {
+    const FUNCTION: MsgFunction = MsgFunction::ContinuationRecord;
+    type Command = ();
+    type InitError = Infallible;
+
+    fn init(&self) -> impl Init<Self::Command, Self::InitError> {
+        <()>::init_zeroed()
+    }
+
+    fn variable_payload_len(&self) -> usize {
+        self.data.len()
+    }
+
+    fn init_variable_payload(
+        &self,
+        dst: &mut SBufferIter<core::array::IntoIter<&mut [u8], 2>>,
+    ) -> Result {
+        dst.write_all(self.data)
+    }
+}
+
+/// Wrapper that splits a command across continuation records if needed.
+pub(crate) struct SplitState<C: CommandToGsp> {
+    state: Option<(KVVec<u8>, usize)>,
+    _phantom: PhantomData<C>,
+}
+
+impl<C: CommandToGsp> SplitState<C> {
+    /// Maximum command size that fits in a single queue element.
+    const MAX_CMD_SIZE: usize = GSP_MSG_QUEUE_ELEMENT_SIZE_MAX - size_of::<GspMsgElement>();
+
+    /// Maximum size of the variable payload that can be sent in the main command.
+    const MAX_FIRST_PAYLOAD_SIZE: usize = Self::MAX_CMD_SIZE - size_of::<C::Command>();
+
+    /// Creates a new `SplitState` for the given command.
+    ///
+    /// If the command is too large, it will be split into a main command and some number of
+    /// continuation records.
+    pub(crate) fn new(inner: &C) -> Result<Self> {
+        if command_size(inner) > Self::MAX_CMD_SIZE {
+            let mut staging =
+                KVVec::<u8>::from_elem(0u8, inner.variable_payload_len(), GFP_KERNEL)?;
+            let mut sbuffer = SBufferIter::new_writer([staging.as_mut_slice(), &mut []]);
+            inner.init_variable_payload(&mut sbuffer)?;
+            if !sbuffer.is_empty() {
+                return Err(EIO);
+            }
+            drop(sbuffer);
+
+            Ok(Self {
+                state: Some((staging, Self::MAX_FIRST_PAYLOAD_SIZE)),
+                _phantom: PhantomData,
+            })
+        } else {
+            Ok(Self {
+                state: None,
+                _phantom: PhantomData,
+            })
+        }
+    }
+
+    /// Returns the main command.
+    pub(crate) fn command(&self, inner: C) -> SplitCommand<'_, C> {
+        if let Some((staging, _)) = &self.state {
+            SplitCommand::Split(inner, staging)
+        } else {
+            SplitCommand::Single(inner)
+        }
+    }
+
+    /// Returns the next continuation record, or `None` if there are no more.
+    pub(crate) fn next_continuation_record(&mut self) -> Option<ContinuationRecord<'_>> {
+        let (staging, offset) = self.state.as_mut()?;
+
+        let remaining = staging.len() - *offset;
+        if remaining > 0 {
+            let chunk_size = remaining.min(Self::MAX_CMD_SIZE);
+            let record = ContinuationRecord::new(&staging[*offset..(*offset + chunk_size)]);
+            *offset += chunk_size;
+            Some(record)
+        } else {
+            None
+        }
+    }
+}
+
+/// Wrapper enum that represents either a single command or a split command with its staging buffer.
+pub(crate) enum SplitCommand<'a, C: CommandToGsp> {
+    Single(C),
+    Split(C, &'a [u8]),
+}
+
+impl<'a, C: CommandToGsp> CommandToGsp for SplitCommand<'a, C> {
+    const FUNCTION: MsgFunction = C::FUNCTION;
+    type Command = C::Command;
+    type InitError = C::InitError;
+
+    fn init(&self) -> impl Init<Self::Command, Self::InitError> {
+        match self {
+            SplitCommand::Single(cmd) => cmd.init(),
+            SplitCommand::Split(cmd, _) => cmd.init(),
+        }
+    }
+
+    fn variable_payload_len(&self) -> usize {
+        match self {
+            SplitCommand::Single(cmd) => cmd.variable_payload_len(),
+            SplitCommand::Split(_, _) => SplitState::<C>::MAX_FIRST_PAYLOAD_SIZE,
+        }
+    }
+
+    fn init_variable_payload(
+        &self,
+        dst: &mut SBufferIter<core::array::IntoIter<&mut [u8], 2>>,
+    ) -> Result {
+        match self {
+            SplitCommand::Single(cmd) => cmd.init_variable_payload(dst),
+            SplitCommand::Split(_, staging) => {
+                dst.write_all(&staging[..self.variable_payload_len()])
+            }
+        }
+    }
+}
+
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index c604f423fff3..1a317dfb0bc9 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -203,6 +203,7 @@ pub(crate) enum MsgFunction {
     AllocObject = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_OBJECT,
     AllocRoot = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_ROOT,
     BindCtxDma = bindings::NV_VGPU_MSG_FUNCTION_BIND_CTX_DMA,
+    ContinuationRecord = bindings::NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD,
     Free = bindings::NV_VGPU_MSG_FUNCTION_FREE,
     GetGspStaticInfo = bindings::NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO,
     GetStaticInfo = bindings::NV_VGPU_MSG_FUNCTION_GET_STATIC_INFO,
@@ -238,6 +239,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
             MsgFunction::AllocObject => write!(f, "ALLOC_OBJECT"),
             MsgFunction::AllocRoot => write!(f, "ALLOC_ROOT"),
             MsgFunction::BindCtxDma => write!(f, "BIND_CTX_DMA"),
+            MsgFunction::ContinuationRecord => write!(f, "CONTINUATION_RECORD"),
             MsgFunction::Free => write!(f, "FREE"),
             MsgFunction::GetGspStaticInfo => write!(f, "GET_GSP_STATIC_INFO"),
             MsgFunction::GetStaticInfo => write!(f, "GET_STATIC_INFO"),
@@ -277,6 +279,9 @@ fn try_from(value: u32) -> Result<MsgFunction> {
             bindings::NV_VGPU_MSG_FUNCTION_ALLOC_OBJECT => Ok(MsgFunction::AllocObject),
             bindings::NV_VGPU_MSG_FUNCTION_ALLOC_ROOT => Ok(MsgFunction::AllocRoot),
             bindings::NV_VGPU_MSG_FUNCTION_BIND_CTX_DMA => Ok(MsgFunction::BindCtxDma),
+            bindings::NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD => {
+                Ok(MsgFunction::ContinuationRecord)
+            }
             bindings::NV_VGPU_MSG_FUNCTION_FREE => Ok(MsgFunction::Free),
             bindings::NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO => Ok(MsgFunction::GetGspStaticInfo),
             bindings::NV_VGPU_MSG_FUNCTION_GET_STATIC_INFO => Ok(MsgFunction::GetStaticInfo),

-- 
2.53.0

[PATCH v2 9/9] gpu: nova-core: gsp: add tests for SplitState

[Eliot Courtney]

Add tests for SplitState. They cover boundary conditions at the split
points to make sure the right number of continuation records are made.
They also check that the data concatenated is correct.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/commands.rs | 113 ++++++++++++++++++++++++++++++++++
 1 file changed, 113 insertions(+)

diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index e722c6091ec8..1683ebb4c685 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -383,3 +383,116 @@ fn init_variable_payload(
     }
 }
 
+#[kunit_tests(nova_core_gsp_commands)]
+mod tests {
+    use super::*;
+
+    struct TestPayload {
+        data: KVVec<u8>,
+    }
+
+    impl TestPayload {
+        fn generate_pattern(len: usize) -> Result<KVVec<u8>> {
+            let mut data = KVVec::with_capacity(len, GFP_KERNEL)?;
+            for i in 0..len {
+                data.push(i as u8, GFP_KERNEL)?;
+            }
+            Ok(data)
+        }
+
+        fn new(len: usize) -> Result<Self> {
+            Ok(Self {
+                data: Self::generate_pattern(len)?,
+            })
+        }
+    }
+
+    impl CommandToGsp for TestPayload {
+        const FUNCTION: MsgFunction = MsgFunction::Nop;
+        type Command = ();
+        type InitError = Infallible;
+
+        fn init(&self) -> impl Init<Self::Command, Self::InitError> {
+            <()>::init_zeroed()
+        }
+
+        fn variable_payload_len(&self) -> usize {
+            self.data.len()
+        }
+
+        fn init_variable_payload(
+            &self,
+            dst: &mut SBufferIter<core::array::IntoIter<&mut [u8], 2>>,
+        ) -> Result {
+            dst.write_all(self.data.as_slice())
+        }
+    }
+
+    const MAX_CMD_SIZE: usize = SplitState::<TestPayload>::MAX_CMD_SIZE;
+
+    fn read_payload(cmd: &impl CommandToGsp) -> Result<KVVec<u8>> {
+        let len = cmd.variable_payload_len();
+        let mut buf = KVVec::from_elem(0u8, len, GFP_KERNEL)?;
+        let mut sbuf = SBufferIter::new_writer([buf.as_mut_slice(), &mut []]);
+        cmd.init_variable_payload(&mut sbuf)?;
+        drop(sbuf);
+        Ok(buf)
+    }
+
+    struct SplitTest {
+        payload_size: usize,
+        num_continuations: usize,
+    }
+
+    fn check_split(t: SplitTest) -> Result {
+        let payload = TestPayload::new(t.payload_size)?;
+        let mut state = SplitState::new(&payload)?;
+
+        let mut buf = read_payload(&state.command(payload))?;
+        assert!(buf.len() <= MAX_CMD_SIZE);
+
+        let mut num_continuations = 0;
+        while let Some(cont) = state.next_continuation_record() {
+            let payload = read_payload(&cont)?;
+            assert!(payload.len() <= MAX_CMD_SIZE);
+            buf.extend_from_slice(&payload, GFP_KERNEL)?;
+            num_continuations += 1;
+        }
+
+        assert_eq!(num_continuations, t.num_continuations);
+        assert_eq!(
+            buf.as_slice(),
+            TestPayload::generate_pattern(t.payload_size)?.as_slice()
+        );
+        Ok(())
+    }
+
+    #[test]
+    fn split_command() -> Result {
+        check_split(SplitTest {
+            payload_size: 0,
+            num_continuations: 0,
+        })?;
+        check_split(SplitTest {
+            payload_size: MAX_CMD_SIZE,
+            num_continuations: 0,
+        })?;
+        check_split(SplitTest {
+            payload_size: MAX_CMD_SIZE + 1,
+            num_continuations: 1,
+        })?;
+        check_split(SplitTest {
+            payload_size: MAX_CMD_SIZE * 2,
+            num_continuations: 1,
+        })?;
+        check_split(SplitTest {
+            payload_size: MAX_CMD_SIZE * 2 + 1,
+            num_continuations: 2,
+        })?;
+        check_split(SplitTest {
+            payload_size: MAX_CMD_SIZE * 3 + MAX_CMD_SIZE / 2,
+            num_continuations: 3,
+        })?;
+        Ok(())
+    }
+}

-- 
2.53.0

Claude review: gpu: nova-core: gsp: add continuation record support

[Claude Code Review Bot]

Overall Series Review

Subject: gpu: nova-core: gsp: add continuation record support
Author: Eliot Courtney <ecourtney@nvidia.com>
Patches: 10
Reviewed: 2026-02-19T18:43:06.950624

---

This 9-patch series adds continuation record support to the nova-core GSP command queue. GSP commands larger than 16 pages (64KB) need to be split across multiple queue elements, with the overflow sent as "continuation records." The series takes a well-structured incremental approach: first sorting enum variants, then adding the wait-for-space mechanism, adding size checking, refactoring the send path, and finally introducing the split logic with tests.

The overall architecture is sound. The `SplitState`/`SplitCommand` design cleanly separates the splitting logic from the transport layer. Commands that fit in a single element still write directly into the command queue with no extra copies, while oversized commands stage their variable payload into a `KVVec` buffer. The TOCTOU between `driver_write_area_size()` and `driver_write_area()` is safe because the GSP read pointer only advances (growing the writable area) and `&mut self` prevents concurrent driver-side writes.

One issue worth discussing is around the `read_poll_timeout` and `driver_write_area` interaction -- see the patch 2 review below. The tests in patch 9 provide good boundary coverage for the split logic. The series is clean and the patches are well-ordered for review.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: sort MsgFunction variants alphabetically

[Claude Code Review Bot]

Patch Review

Pure mechanical reordering of enum variants, Display impl arms, and TryFrom arms into alphabetical order within their "Common function codes" and "Event codes" sections. No issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: add mechanism to wait for space on command queue

[Claude Code Review Bot]

Patch Review

This patch adds `driver_write_area_size()` and changes `allocate_command` to poll-wait for space using `read_poll_timeout` instead of returning `EAGAIN` immediately.

> +    fn driver_write_area_size(&self) -> usize {
> +        let tx = self.cpu_write_ptr();
> +        let rx = self.gsp_read_ptr();
> +
> +        // `rx` and `tx` are both in `0..MSGQ_NUM_PAGES` per the invariants of `gsp_read_ptr` and
> +        // `cpu_write_ptr`. The minimum value case is where `rx == 0` and `tx == MSGQ_NUM_PAGES -
> +        // 1`, which gives `0 + MSGQ_NUM_PAGES - (MSGQ_NUM_PAGES - 1) - 1 == 0`.
> +        let slots = (rx + MSGQ_NUM_PAGES - tx - 1) % MSGQ_NUM_PAGES;
> +        num::u32_as_usize(slots) * GSP_PAGE_SIZE
> +    }

The comment says `rx` and `tx` are both in `0..MSGQ_NUM_PAGES`, but the invariant comments on `gsp_read_ptr` and `cpu_write_ptr` say "between `0` and `MSGQ_NUM_PAGES`" which is ambiguous about whether `MSGQ_NUM_PAGES` itself is included. Looking at the code, both functions use `% MSGQ_NUM_PAGES`, so the range is `0..MSGQ_NUM_PAGES` (exclusive of `MSGQ_NUM_PAGES`). The comment here is consistent and the math checks out.

> +        read_poll_timeout(
> +            || Ok(self.driver_write_area_size()),
> +            |available_bytes| *available_bytes >= size_of::<GspMsgElement>() + size,
> +            Delta::ZERO,
> +            timeout,
> +        )?;

After `read_poll_timeout` succeeds, the code calls `self.driver_write_area()` to get the actual mutable slices. Since `gsp_read_ptr` can only advance between the two calls (the GSP consumes data), the actual area is at least as large as what was polled, so no underflow can occur. The `&mut self` receiver prevents concurrent driver-side calls. This is safe.

The 1-second timeout used in `send_command` matches the nouveau driver pattern. No issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: rust: add EMSGSIZE error code

[Claude Code Review Bot]

Patch Review

> +    declare_err!(EMSGSIZE, "Message too long.");

Straightforward addition of a missing error code to the Rust kernel error declarations. No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: add checking oversized commands

[Claude Code Review Bot]

Patch Review

> +        if size_of::<GspMsgElement>() + size > GSP_MSG_QUEUE_ELEMENT_SIZE_MAX {
> +            return Err(EMSGSIZE);
> +        }

This adds an early check in `allocate_command` that rejects commands larger than `GSP_MSG_QUEUE_ELEMENT_SIZE_MAX` (64KB). This is the right place for this check -- it prevents the poll loop from waiting forever for space that can never be sufficient. After patch 8, `send_command` uses `SplitState` so `allocate_command` should never actually see oversized commands, but the check still serves as a useful invariant guard. No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: clarify invariant on command queue

[Claude Code Review Bot]

Patch Review

> +        // Extract area for the command itself. The GSP message header and the command header
> +        // together are guaranteed to fit entirely into a single page, so it's ok to only look
> +        // at `dst.contents.0` here.

Comment-only change adding useful context about why `from_bytes_mut_prefix` only uses the first content slice. No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: unconditionally call variable payload handling

[Claude Code Review Bot]

Patch Review

> -        // Fill the variable-length payload.
> -        if command_size > size_of::<M::Command>() {
> -            let mut sbuffer =
> -                SBufferIter::new_writer([&mut payload_1[..], &mut dst.contents.1[..]]);
> -            command.init_variable_payload(&mut sbuffer)?;
> -
> -            if !sbuffer.is_empty() {
> -                return Err(EIO);
> -            }
> +        // Fill the variable-length payload, which may be empty.
> +        let mut sbuffer = SBufferIter::new_writer([&mut payload_1[..], &mut dst.contents.1[..]]);
> +        command.init_variable_payload(&mut sbuffer)?;
> +
> +        if !sbuffer.is_empty() {
> +            return Err(EIO);
>          }
> +        drop(sbuffer);

The conditional was guarding a no-op path (when there's no variable payload, `init_variable_payload` does nothing and the `SBufferIter` with zero remaining bytes would pass the `is_empty` check). Removing the conditional simplifies the code and the `drop(sbuffer)` is needed to end the mutable borrow before the checksum computation accesses `dst.contents`. No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: add command_size helper

[Claude Code Review Bot]

Patch Review

> +fn command_size<M>(command: &M) -> usize
> +where
> +    M: CommandToGsp,
> +{
> +    size_of::<M::Command>() + command.variable_payload_len()
> +}

Simple extraction of a repeated pattern into a helper. Initially private, made `pub(crate)` in patch 8. No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: support large RPCs via continuation record

[Claude Code Review Bot]

Patch Review

This is the main patch. A few observations:

> +impl<C: CommandToGsp> SplitState<C> {
> +    const MAX_CMD_SIZE: usize = GSP_MSG_QUEUE_ELEMENT_SIZE_MAX - size_of::<GspMsgElement>();
> +    const MAX_FIRST_PAYLOAD_SIZE: usize = Self::MAX_CMD_SIZE - size_of::<C::Command>();

`MAX_FIRST_PAYLOAD_SIZE` could underflow if `size_of::<C::Command>() > MAX_CMD_SIZE`. In practice this would mean a command header larger than ~64KB which seems impossible for any real GSP command, and `SplitState` is only instantiated with concrete types the driver controls, so this is not a real concern.

> +    pub(crate) fn new(inner: &C) -> Result<Self> {
> +        if command_size(inner) > Self::MAX_CMD_SIZE {
> +            let mut staging =
> +                KVVec::<u8>::from_elem(0u8, inner.variable_payload_len(), GFP_KERNEL)?;
> +            let mut sbuffer = SBufferIter::new_writer([staging.as_mut_slice(), &mut []]);
> +            inner.init_variable_payload(&mut sbuffer)?;
> +            if !sbuffer.is_empty() {
> +                return Err(EIO);
> +            }
> +            drop(sbuffer);
> +
> +            Ok(Self {
> +                state: Some((staging, Self::MAX_FIRST_PAYLOAD_SIZE)),
> +                _phantom: PhantomData,
> +            })

When a split is needed, the entire variable payload is serialized into a staging buffer. This means `init_variable_payload` is called once during `SplitState::new`, and then the staging buffer is read from when writing the main command and continuation records. This avoids calling `init_variable_payload` multiple times, which is correct.

> +    pub(crate) fn command(&self, inner: C) -> SplitCommand<'_, C> {
> +        if let Some((staging, _)) = &self.state {
> +            SplitCommand::Split(inner, staging)
> +        } else {
> +            SplitCommand::Single(inner)
> +        }
> +    }

`command()` takes `&self` but consumes `inner`. This means after calling `command()`, the caller no longer has the original command, but `SplitState` still holds the staging buffer for continuation records. The borrow relationships look correct.

> +    pub(crate) fn next_continuation_record(&mut self) -> Option<ContinuationRecord<'_>> {
> +        let (staging, offset) = self.state.as_mut()?;
> +
> +        let remaining = staging.len() - *offset;
> +        if remaining > 0 {
> +            let chunk_size = remaining.min(Self::MAX_CMD_SIZE);
> +            let record = ContinuationRecord::new(&staging[*offset..(*offset + chunk_size)]);
> +            *offset += chunk_size;
> +            Some(record)
> +        } else {
> +            None
> +        }
> +    }

The staging buffer length is `inner.variable_payload_len()` and the initial offset is `MAX_FIRST_PAYLOAD_SIZE`. Could `MAX_FIRST_PAYLOAD_SIZE > staging.len()` (i.e. `variable_payload_len()`)? That would cause `remaining` to underflow (panic in debug, wrap in release). Let's check: we only enter the split path when `command_size(inner) > MAX_CMD_SIZE`, which means `size_of::<C::Command>() + variable_payload_len() > MAX_CMD_SIZE`, so `variable_payload_len() > MAX_CMD_SIZE - size_of::<C::Command>() = MAX_FIRST_PAYLOAD_SIZE`. So the staging buffer is always strictly larger than `MAX_FIRST_PAYLOAD_SIZE`, meaning `remaining` is always at least 1 on the first call. No underflow.

> +    fn variable_payload_len(&self) -> usize {
> +        match self {
> +            SplitCommand::Single(cmd) => cmd.variable_payload_len(),
> +            SplitCommand::Split(_, _) => SplitState::<C>::MAX_FIRST_PAYLOAD_SIZE,
> +        }
> +    }

For the `Split` variant, the variable payload is exactly `MAX_FIRST_PAYLOAD_SIZE` bytes, which is the truncated first chunk of the full payload. This means `send_single_command` will allocate `size_of::<C::Command>() + MAX_FIRST_PAYLOAD_SIZE = MAX_CMD_SIZE`, which fits within one element. Correct.

> +    fn init_variable_payload(
> +        &self,
> +        dst: &mut SBufferIter<core::array::IntoIter<&mut [u8], 2>>,
> +    ) -> Result {
> +        match self {
> +            SplitCommand::Single(cmd) => cmd.init_variable_payload(dst),
> +            SplitCommand::Split(_, staging) => {
> +                dst.write_all(&staging[..self.variable_payload_len()])
> +            }
> +        }
> +    }

For `Split`, this writes the first `MAX_FIRST_PAYLOAD_SIZE` bytes of the staging buffer. `self.variable_payload_len()` calls back into the match and returns `MAX_FIRST_PAYLOAD_SIZE`. The `SBufferIter` was constructed with capacity `MAX_FIRST_PAYLOAD_SIZE`, so after `write_all` it should be exactly empty, passing the `is_empty` check in `send_single_command`. Correct.

In `send_command`:

> +        let mut state = SplitState::new(&command)?;
> +
> +        self.send_single_command(bar, state.command(command))?;

`SplitState::new` takes `&command` and `state.command` takes `command` by value. If the command is not split, the original `command` is passed through to `send_single_command` without staging. If it is split, the staging buffer was already populated in `new`, and `command` is moved into `SplitCommand::Split` where its `init()` method is still called but its `init_variable_payload` is not (the staging buffer is used instead). This is correct.

One minor observation: the `SplitState` is declared `mut` but is only mutated during `next_continuation_record`. For the non-split path, `state` is not mutated at all. This is fine; the compiler would optimize it.

No bugs found in this patch.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: add tests for SplitState

[Claude Code Review Bot]

Patch Review

> +    impl CommandToGsp for TestPayload {
> +        const FUNCTION: MsgFunction = MsgFunction::Nop;
> +        type Command = ();

The tests use `Command = ()` (zero-sized type), which means `MAX_FIRST_PAYLOAD_SIZE == MAX_CMD_SIZE`. This simplifies the boundary testing. It would be nice to also have a test with a non-zero-sized `Command` to exercise the case where `MAX_FIRST_PAYLOAD_SIZE < MAX_CMD_SIZE`, but this is a minor suggestion, not a bug.

> +    fn check_split(t: SplitTest) -> Result {
> +        let payload = TestPayload::new(t.payload_size)?;
> +        let mut state = SplitState::new(&payload)?;
> +
> +        let mut buf = read_payload(&state.command(payload))?;
> +        assert!(buf.len() <= MAX_CMD_SIZE);

The test correctly verifies both the structure (correct number of continuation records, each within the size limit) and the data integrity (reassembled data matches the original pattern). The boundary conditions at `MAX_CMD_SIZE`, `MAX_CMD_SIZE + 1`, `MAX_CMD_SIZE * 2`, and `MAX_CMD_SIZE * 2 + 1` are well-chosen. No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: add mechanism to wait for space on command queue

[Claude Code Review Bot]

Patch Review

Adds `driver_write_area_size()` and changes `allocate_command` from returning `EAGAIN` to polling with `read_poll_timeout`. The 1-second timeout matches nouveau's approach.

The circular buffer arithmetic in `driver_write_area_size()` looks correct:

```rust
let slots = (rx + MSGQ_NUM_PAGES - tx - 1) % MSGQ_NUM_PAGES;
```

The `-1` reserves one slot to distinguish full from empty, which is standard for circular buffers.

**Minor observation**: The `Delta::ZERO` sleep period in `read_poll_timeout` means this is a busy-poll. For a 1-second timeout that's acceptable in this context (the GSP should drain commands quickly), but if this is ever used with longer timeouts it could waste CPU. Not a problem for the current use case.

The error code change from `EAGAIN` to `ETIMEDOUT` is appropriate since the caller no longer retries.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: add mechanism to wait for space on command queue

[Claude Code Review Bot]

Patch Review

Adds `driver_write_area_size()` and changes `allocate_command()` to poll-wait with a timeout instead of immediately returning `EAGAIN`.

The available-space calculation looks correct:

```rust
let slots = (rx + MSGQ_NUM_PAGES - tx - 1) % MSGQ_NUM_PAGES;
num::u32_as_usize(slots) * GSP_PAGE_SIZE
```

The comment accurately describes the minimum case (rx==0, tx==MSGQ_NUM_PAGES-1 → 0 slots).

**Minor observation:** The poll interval is `Delta::ZERO`:

```rust
read_poll_timeout(
    || Ok(self.driver_write_area_size()),
    |available_bytes| *available_bytes >= size_of::<GspMsgElement>() + size,
    Delta::ZERO,
    timeout,
)?;
```

This means busy-polling with no sleep between checks. For the typical case where the queue has space, this returns immediately (one iteration). For the rare case where the queue is full, this busy-spins for up to 1 second. Depending on how `read_poll_timeout` is implemented in the Rust kernel bindings, this could consume significant CPU. Consider whether a small sleep interval (e.g., `Delta::from_millis(1)`, matching the pattern used in `wait_for_msg`) would be more appropriate for the queue-full case.

The 1-second timeout at the call site (`Delta::from_secs(1)`) seems reasonable and matches nouveau's approach.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: add mechanism to wait for space on command queue

[Claude Code Review Bot]

Patch Review

This replaces the immediate `EAGAIN` return with a `read_poll_timeout` loop, which is the right approach for supporting continuation records that can fill the queue.

The `driver_write_area_size` method is a good addition that cleanly computes the available space without needing to materialize the slices. The comment explaining the minimum value is helpful.

One observation: the `read_poll_timeout` uses `Delta::ZERO` for the poll interval:

```rust
read_poll_timeout(
    || Ok(self.driver_write_area_size()),
    |available_bytes| *available_bytes >= size_of::<GspMsgElement>() + size,
    Delta::ZERO,
    timeout,
)?;
```

A zero poll interval means busy-waiting. Other callers in the same tree (e.g., `wait_for_msg` at cmdq.rs:572) use `Delta::from_millis(1)`. This could be intentional since command queue space should free up very quickly after GSP processes a command, but it might be worth adding a brief comment explaining the choice or using a small interval to avoid CPU spinning under load.

The timeout of 1 second (`ALLOCATE_TIMEOUT`) matches nouveau's approach, which is good.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: add mechanism to wait for space on command queue

[Claude Code Review Bot]

Patch Review

Replaces the immediate `EAGAIN` failure in `allocate_command` with a `read_poll_timeout` loop, adding a 1-second timeout. This is required because continuation records can fill up the queue and need the GSP to drain it.

The new `driver_write_area_size()` method computes the writable area size without materializing the slices, which is the right approach for a polling check.

The poll delay of `Delta::from_micros(1)` (changed from zero in v5 to 1us in v6) is reasonable — it avoids a busy spin while still being responsive.

The `ALLOCATE_TIMEOUT` of 1 second matches nouveau's behavior per the commit message.

One minor note: the existing `driver_write_area` method has essentially the same slot calculation logic. There's some duplication but it's justified since `driver_write_area` returns actual slices while `driver_write_area_size` just returns the size, and combining them would be awkward.

No issues.

---
Generated by Claude Code Patch Reviewer