[PATCH 0/4] gpu: nova-core: debug logging essentials for r000

[PATCH 0/4] gpu: nova-core: debug logging essentials for r000

[John Hubbard]

Following up on the r000 v3 patchset that I just posted, this provides
some key logging enhancements for dmesg output, as well as re-enabling
decoding of GSP logs with the new r000 firmware (which has a new log
system that needs a little bit more information; specifically, Build
IDs).

These are a reworked and refreshed version of the debug-logging patches
of the r000 v2 series [1], which Alex asked to be moved into their own
patchset.

* Each log dump carries the header that the log decoder expects, with
  the firmware's build ID, so the decoder matches the dump to the right
  firmware ELF instead of the user guessing.

* Each GMC debug line prints the command by name and labels a received
  message as an event or a response, so that this line:

      GSP GMC: receive: seq# 9223372036854775808, command_id=0x10002, length=0xb0

  becomes this one:

      GSP GMC: event: seq# 0, command=EXEC_GENERIC_BOOTLOADER (0x10002), length=0xb0

A small preparatory patch keeps the NV_PMC_BOOT_42 implementation
field in the GPU spec, so that the log header takes the GPU's
architecture and implementation from the register fields, as Open RM
does.

The series applies on top of v3 of the r000 series [2], which in turn
applies on top of drm-rust-next plus:

* the interrupt tree series for GIN, the GPU Interrupt and Notification
  unit [3]
* Eliot Courtney's NVKV codec series [4]
* the three-patch radix3 cleanup [5]
* two lint fixes for the generated bindings, both already in mainline

A branch with everything applied:

    https://github.com/johnhubbard/linux/tree/nova-core-r615-plus-log-decoding-v0/

[1] https://lore.kernel.org/all/20260822015448.238214-1-jhubbard@nvidia.com/
[2] https://github.com/johnhubbard/linux/tree/nova-core-run-on-r615-or-later-v3/
[3] https://lore.kernel.org/all/20260912044400.677097-1-jhubbard@nvidia.com/
[4] https://lore.kernel.org/all/20260827-b4-nvkv-v2-0-0de9d5c8658c@nvidia.com/
[5] https://lore.kernel.org/all/20260913195413.742143-1-jhubbard@nvidia.com/

John Hubbard (4):
  gpu: nova-core: record the BOOT_42 implementation field in the GPU
    spec
  gpu: nova-core: enable decoding of the GSP firmware logs
  gpu: nova-core: gsp: print GMC command names in debug logs
  gpu: nova-core: gsp: distinguish GMC events from responses in debug
    logs

 drivers/gpu/nova-core/firmware/gsp.rs |  54 ++++++-
 drivers/gpu/nova-core/gpu.rs          |   9 +-
 drivers/gpu/nova-core/gsp.rs          | 212 ++++++++++++++++++++++----
 drivers/gpu/nova-core/gsp/boot.rs     |  11 +-
 drivers/gpu/nova-core/gsp/cmdq.rs     |  42 +++--
 drivers/gpu/nova-core/gsp/fw.rs       |  81 +++++++++-
 6 files changed, 355 insertions(+), 54 deletions(-)

-- 
2.55.0

[PATCH 1/4] gpu: nova-core: record the BOOT_42 implementation field in the GPU spec

[John Hubbard]

NV_PMC_BOOT_42, the register that identifies the GPU, carries an
architecture field and an implementation field. Open RM keeps the two
fields as it reads them, and the GSP log headers that Open RM writes
name the GPU by both.

Nova-core combined the two fields into one chipset code at probe and
kept only that code, so the implementation number could be recovered
only by undoing the combination.

Record the implementation field in the GPU spec beside the chipset
code.

Assisted-by: LLM
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/gpu.rs | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index fd1a74913d7c..cecd8e79430d 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -213,10 +213,15 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
     }
 }
 
-/// Structure holding a basic description of the GPU: `Chipset` and `Revision`.
+/// Structure holding a basic description of the GPU: `Chipset`, the implementation field of
+/// `NV_PMC_BOOT_42`, and `Revision`.
 #[derive(Clone, Copy)]
 pub(crate) struct Spec {
     pub(crate) chipset: Chipset,
+    /// Implementation field of `NV_PMC_BOOT_42`, which distinguishes this chipset from the other
+    /// chipsets of its architecture.
+    #[expect(dead_code)]
+    pub(crate) implementation: u8,
     revision: Revision,
 }
 
@@ -259,6 +264,7 @@ impl TryFrom<regs::NV_PMC_BOOT_42> for Spec {
     fn try_from(boot42: regs::NV_PMC_BOOT_42) -> Result<Self> {
         Ok(Self {
             chipset: boot42.chipset()?,
+            implementation: boot42.implementation().into(),
             revision: boot42.into(),
         })
     }
-- 
2.55.0

[PATCH 2/4] gpu: nova-core: enable decoding of the GSP firmware logs

[John Hubbard]

Nova-core exports the GSP-RM log buffers over debugfs, and Open RM's
log decoder turns a dump into text with the ELF of the firmware that
wrote it. The decoder identifies that firmware by a header ahead of
each buffer. The header, in the LIBOS_LOG_NVLOG_BUFFER_V2 layout, names
the GPU, the firmware task that wrote the buffer, and the GNU build ID
of the firmware.

Nova-core wrote no such header, so a dump could not be matched to its
firmware, and a decode with the wrong ELF produced garbage rather than
an error.

Prepend the header to each debugfs dump, with the build ID read from
the BLID tag of gsp.tlv. The log buffers are created before boot loads
the firmware from that file, so request the file once when the buffers
are created and hand it to the boot sequence. A file without a usable
build ID is reported with a warning, and its dumps carry no header.

Also, r000 has several more log buffers that must be provisioned, so do
that too.

Assisted-by: LLM
Reviewed-by: Timur Tabi <ttabi@nvidia.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/firmware/gsp.rs |  54 ++++++-
 drivers/gpu/nova-core/gpu.rs          |   3 +-
 drivers/gpu/nova-core/gsp.rs          | 212 ++++++++++++++++++++++----
 drivers/gpu/nova-core/gsp/boot.rs     |   2 +-
 4 files changed, 234 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
index a2db7b6ba131..e6ca8d559d7b 100644
--- a/drivers/gpu/nova-core/firmware/gsp.rs
+++ b/drivers/gpu/nova-core/firmware/gsp.rs
@@ -6,6 +6,7 @@
         Coherent,
         DmaAddress, //
     },
+    firmware,
     prelude::*, //
 };
 
@@ -21,6 +22,48 @@
     gpu::Chipset, //
 };
 
+/// Longest build ID that a log dump header carries, matching Open RM's `BUILD_ID_MAX_LENGTH`.
+pub(crate) const BUILD_ID_MAX_LENGTH: usize = 32;
+
+/// Build ID of the GSP firmware, from the `BLID` tag of its TLV.
+pub(crate) struct BuildId {
+    /// The ID, zero-padded to [`BUILD_ID_MAX_LENGTH`] bytes.
+    bytes: [u8; BUILD_ID_MAX_LENGTH],
+    /// Number of valid bytes in `bytes`.
+    len: u32,
+}
+
+impl BuildId {
+    /// Reads the build ID from the `BLID` tag of `tlv`.
+    ///
+    /// Returns `EINVAL` if the tag is absent or empty, or if its value is longer than
+    /// [`BUILD_ID_MAX_LENGTH`] bytes.
+    pub(crate) fn from_tlv(tlv: &Tlv<'_>) -> Result<Self> {
+        let value = tlv.get_bytes(b"BLID")?;
+
+        let mut bytes = [0; BUILD_ID_MAX_LENGTH];
+        bytes
+            .get_mut(..value.len())
+            .ok_or(EINVAL)?
+            .copy_from_slice(value);
+
+        Ok(Self {
+            bytes,
+            len: u32::try_from(value.len())?,
+        })
+    }
+
+    /// Returns the ID, zero-padded to [`BUILD_ID_MAX_LENGTH`] bytes.
+    pub(crate) fn padded(&self) -> &[u8; BUILD_ID_MAX_LENGTH] {
+        &self.bytes
+    }
+
+    /// Returns the number of bytes in the ID.
+    pub(crate) fn len(&self) -> u32 {
+        self.len
+    }
+}
+
 /// The GSP firmware image, its signatures, and the GSP bootloader.
 #[pin_data]
 pub(crate) struct GspFirmware<'a> {
@@ -36,13 +79,16 @@ pub(crate) struct GspFirmware<'a> {
 impl<'a> GspFirmware<'a> {
     /// Loads the GSP firmware binaries, map them into `dev`'s address-space, and creates the page
     /// tables expected by the GSP bootloader to load it.
-    pub(crate) fn new(
+    ///
+    /// `gsp_tlv` is the TLV of the GSP firmware, which names the image file and carries the
+    /// signatures.
+    pub(crate) fn new<'tlv>(
         dev: &'a device::Device<device::Bound>,
         chipset: Chipset,
-    ) -> impl PinInit<Self, Error> + 'a {
+        gsp_tlv: &'tlv firmware::Firmware,
+    ) -> impl PinInit<Self, Error> + use<'a, 'tlv> {
         pin_init::pin_init_scope(move || {
-            let firmware = request_tlv(dev, chipset, "gsp")?;
-            let tlv = Tlv::new(firmware.data())?;
+            let tlv = Tlv::new(gsp_tlv.data())?;
             dev_dbg!(dev, "loaded gsp firmware v{}\n", tlv.get_string(b"VERS")?);
 
             let fw_vvec = tlv.load_file(dev, chipset)?;
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index cecd8e79430d..173c57179b39 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -220,7 +220,6 @@ pub(crate) struct Spec {
     pub(crate) chipset: Chipset,
     /// Implementation field of `NV_PMC_BOOT_42`, which distinguishes this chipset from the other
     /// chipsets of its architecture.
-    #[expect(dead_code)]
     pub(crate) implementation: u8,
     revision: Revision,
 }
@@ -424,7 +423,7 @@ pub(crate) fn new<'a>(
 
                 vgpu: VgpuManager::new(pdev, spec.chipset, fsp.as_mut()),
 
-                gsp <- Gsp::new(pdev, bar),
+                gsp <- Gsp::new(pdev, *spec, bar),
 
                 // This member must be initialized last, so that the unload bundle can never be
                 // dropped from outside the constructed `GspResources`, and the unload sequence runs
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 85967119cd7e..f8c675288d58 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -12,13 +12,17 @@
         CoherentView,
         DmaAddress, //
     },
+    firmware,
+    fs::file,
     io::{
         io_project,
         io_write,
         Io, //
     },
     pci,
-    prelude::*, //
+    prelude::*,
+    transmute::AsBytes,
+    uaccess::UserSliceWriter, //
 };
 
 pub(crate) mod cmdq;
@@ -42,8 +46,21 @@
         sec2::Sec2 as Sec2Falcon,
         Falcon, //
     },
+    firmware::{
+        gsp::{
+            BuildId,
+            BUILD_ID_MAX_LENGTH, //
+        },
+        tlv::{
+            request_tlv,
+            Tlv, //
+        },
+    },
     fsp::Fsp,
-    gpu::Chipset,
+    gpu::{
+        Chipset,
+        Spec, //
+    },
     gsp::{
         cmdq::Cmdq,
         fw::GspArgumentsPadded, //
@@ -100,6 +117,68 @@ fn init(view: CoherentView<'_, Self>, start: DmaAddress) -> Result<()> {
     }
 }
 
+/// Longest task name in a log dump header, matching Open RM's `TASK_NAME_MAX_LENGTH`.
+const TASK_NAME_MAX_LENGTH: usize = 8;
+
+/// Header ahead of the log data in a debugfs dump, in the layout of Open RM's
+/// `LIBOS_LOG_NVLOG_BUFFER_V2`.
+///
+/// The decoder uses the header only when `build_id` matches the build ID of its GSP firmware ELF.
+#[repr(C)]
+struct LogBufferHeader {
+    /// Architecture code of the GPU, from the `NV_PMC_BOOT_42` architecture field.
+    gpu_arch: u32,
+    /// Implementation number of the GPU within its architecture.
+    gpu_impl: u32,
+    version: u32,
+    /// Number of valid bytes in `build_id`.
+    build_id_length: u32,
+    /// Name of the LIBOS3 task, printed ahead of each decoded line.
+    task_prefix: [u8; TASK_NAME_MAX_LENGTH],
+    /// Value that the decoder adds to each timestamp, zero when unknown.
+    local_to_global_timer_delta: u64,
+    /// Build ID of the GSP firmware, zero-padded.
+    build_id: [u8; BUILD_ID_MAX_LENGTH],
+    /// `LIBOS_LOG_NVLOG_BUFFER_FLAG_*` bits.
+    flags: u32,
+    reserved: u32,
+}
+
+// SAFETY: `LogBufferHeader` is `repr(C)`, its integer and byte array fields leave no padding
+// between or after them, and it has no interior mutability.
+unsafe impl AsBytes for LogBufferHeader {}
+
+impl LogBufferHeader {
+    /// The `LIBOS_LOG_NVLOG_BUFFER_V2` layout.
+    const VERSION: u32 = 2;
+    /// `LIBOS_LOG_NVLOG_BUFFER_FLAG_PACKED_METADATA`: each log entry packs its argument count and
+    /// task id into the word that holds its metadata address.
+    const FLAG_PACKED_METADATA: u32 = 0x1;
+
+    /// Builds the header for a dump of the `task_prefix` task's log, on the GPU that `spec`
+    /// describes, whose GSP runs the firmware with `build_id`.
+    fn new(spec: Spec, build_id: &BuildId, task_prefix: &str) -> Self {
+        let mut header = Self {
+            gpu_arch: spec.chipset.arch() as u32,
+            gpu_impl: u32::from(spec.implementation),
+            version: Self::VERSION,
+            build_id_length: build_id.len(),
+            task_prefix: [0; TASK_NAME_MAX_LENGTH],
+            local_to_global_timer_delta: 0,
+            build_id: *build_id.padded(),
+            flags: Self::FLAG_PACKED_METADATA,
+            reserved: 0,
+        };
+
+        // The last byte stays zero, so that the name is NUL-terminated for the decoder.
+        let prefix = task_prefix.as_bytes();
+        let len = prefix.len().min(TASK_NAME_MAX_LENGTH - 1);
+        header.task_prefix[..len].copy_from_slice(&prefix[..len]);
+
+        header
+    }
+}
+
 /// The logging buffers are byte queues that contain encoded printf-like
 /// messages from GSP-RM.  They need to be decoded by a special application
 /// that can parse the buffers.
@@ -114,7 +193,13 @@ fn init(view: CoherentView<'_, Self>, start: DmaAddress) -> Result<()> {
 /// then pp points to index into the buffer where the next logging entry will
 /// be written. Therefore, the logging data is valid if:
 ///   1 <= pp < sizeof(buffer)/sizeof(u64)
-struct LogBuffer<'a, const NUM_PAGES: usize>(Coherent<'a, [[u8; GSP_PAGE_SIZE]; NUM_PAGES]>);
+struct LogBuffer<'a, const NUM_PAGES: usize> {
+    /// Header that a debugfs dump carries ahead of the data, present when the GSP firmware's
+    /// build ID is known.
+    header: Option<LogBufferHeader>,
+    /// The buffer that GSP-RM logs into.
+    buffer: Coherent<'a, [[u8; GSP_PAGE_SIZE]; NUM_PAGES]>,
+}
 
 /// A log buffer at the default size, [`RM_LOG_BUFFER_NUM_PAGES`] pages.
 ///
@@ -127,20 +212,66 @@ fn init(view: CoherentView<'_, Self>, start: DmaAddress) -> Result<()> {
 type SmallLogBuffer<'a> = LogBuffer<'a, 1>;
 
 impl<'a, const NUM_PAGES: usize> LogBuffer<'a, NUM_PAGES> {
-    /// Creates a new `LogBuffer` mapped on `dev`.
-    fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
-        let obj = Self(Coherent::zeroed(dev, GFP_KERNEL)?);
+    /// Creates a new `LogBuffer` mapped on `dev`, whose debugfs dump opens with `header`.
+    fn new(
+        dev: &'a device::Device<device::Bound>,
+        header: Option<LogBufferHeader>,
+    ) -> Result<Self> {
+        let buffer = Coherent::zeroed(dev, GFP_KERNEL)?;
 
-        let start_addr = obj.0.dma_address();
+        let start_addr = buffer.dma_address();
 
         let pte_view = io_project!(
-            obj.0,
+            buffer,
             [build: 0][build: size_of::<u64>()..][build: ..NUM_PAGES * size_of::<u64>()]
         )
         .try_cast::<PteArray<NUM_PAGES>>()?;
         PteArray::init(pte_view, start_addr)?;
 
-        Ok(obj)
+        Ok(Self { header, buffer })
+    }
+}
+
+impl<const NUM_PAGES: usize> debugfs::BinaryWriter for LogBuffer<'_, NUM_PAGES> {
+    fn write_to_slice(
+        &self,
+        writer: &mut UserSliceWriter,
+        offset: &mut file::Offset,
+    ) -> Result<usize> {
+        if offset.is_negative() {
+            return Err(EINVAL);
+        }
+
+        // An offset too large for a `usize` is past the end of the dump.
+        let Ok(offset_val) = usize::try_from(*offset) else {
+            return Ok(0);
+        };
+
+        let header = self
+            .header
+            .as_ref()
+            .map_or(&[][..], |header| header.as_bytes());
+        let total = header.len() + self.buffer.size();
+        if offset_val >= total {
+            return Ok(0);
+        }
+
+        let count = (total - offset_val).min(writer.len());
+        let mut written = 0;
+
+        // The header comes first, and a read that reaches past it continues in the buffer.
+        if let Some(header_rest) = header.get(offset_val..) {
+            written = header_rest.len().min(count);
+            writer.write_slice(&header_rest[..written])?;
+        }
+        if written < count {
+            let buffer_offset = offset_val + written - header.len();
+            writer.write_dma(&self.buffer, buffer_offset, count - written)?;
+            written = count;
+        }
+
+        *offset += i64::try_from(written)?;
+        Ok(written)
     }
 }
 
@@ -164,15 +295,22 @@ impl<'a> LogBuffers<'a> {
     /// Number of log buffers.
     const COUNT: usize = 6;
 
-    /// Allocates the six log buffers, mapped on `dev`.
-    fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
+    /// Allocates the six log buffers, mapped on `dev`, and gives each one the dump header for
+    /// the GPU that `spec` describes when `build_id` is known.
+    fn new(
+        dev: &'a device::Device<device::Bound>,
+        spec: Spec,
+        build_id: Option<&BuildId>,
+    ) -> Result<Self> {
+        let header = |task_prefix| build_id.map(|id| LogBufferHeader::new(spec, id, task_prefix));
+
         Ok(Self {
-            loginit: TaskLogBuffer::new(dev)?,
-            logintr: TaskLogBuffer::new(dev)?,
-            logrm: TaskLogBuffer::new(dev)?,
-            logmnoc: TaskLogBuffer::new(dev)?,
-            logroot: SmallLogBuffer::new(dev)?,
-            logrmon: SmallLogBuffer::new(dev)?,
+            loginit: TaskLogBuffer::new(dev, header("INIT"))?,
+            logintr: TaskLogBuffer::new(dev, header("INTR"))?,
+            logrm: TaskLogBuffer::new(dev, header("RM"))?,
+            logmnoc: TaskLogBuffer::new(dev, header("MNOC"))?,
+            logroot: SmallLogBuffer::new(dev, header("ROOT"))?,
+            logrmon: SmallLogBuffer::new(dev, header("RMON"))?,
         })
     }
 
@@ -184,27 +322,27 @@ fn init_arguments(
     ) -> Result {
         libos.init_at(
             0,
-            LibosMemoryRegionInitArgument::new("LOGINIT", &self.loginit.0),
+            LibosMemoryRegionInitArgument::new("LOGINIT", &self.loginit.buffer),
         )?;
         libos.init_at(
             1,
-            LibosMemoryRegionInitArgument::new("LOGINTR", &self.logintr.0),
+            LibosMemoryRegionInitArgument::new("LOGINTR", &self.logintr.buffer),
         )?;
         libos.init_at(
             2,
-            LibosMemoryRegionInitArgument::new("LOGRM", &self.logrm.0),
+            LibosMemoryRegionInitArgument::new("LOGRM", &self.logrm.buffer),
         )?;
         libos.init_at(
             3,
-            LibosMemoryRegionInitArgument::new("LOGMNOC", &self.logmnoc.0),
+            LibosMemoryRegionInitArgument::new("LOGMNOC", &self.logmnoc.buffer),
         )?;
         libos.init_at(
             4,
-            LibosMemoryRegionInitArgument::new("LOGROOT", &self.logroot.0),
+            LibosMemoryRegionInitArgument::new("LOGROOT", &self.logroot.buffer),
         )?;
         libos.init_at(
             5,
-            LibosMemoryRegionInitArgument::new("LOGRMON", &self.logrmon.0),
+            LibosMemoryRegionInitArgument::new("LOGRMON", &self.logrmon.buffer),
         )?;
 
         Ok(())
@@ -212,18 +350,20 @@ fn init_arguments(
 
     /// Exposes each log buffer as a binary file in `dir`, under the lowercase form of its name.
     fn register_debugfs<'data>(&'data self, dir: &debugfs::ScopedDir<'data, '_>) {
-        dir.read_binary_file(c"loginit", &self.loginit.0);
-        dir.read_binary_file(c"logintr", &self.logintr.0);
-        dir.read_binary_file(c"logrm", &self.logrm.0);
-        dir.read_binary_file(c"logmnoc", &self.logmnoc.0);
-        dir.read_binary_file(c"logroot", &self.logroot.0);
-        dir.read_binary_file(c"logrmon", &self.logrmon.0);
+        dir.read_binary_file(c"loginit", &self.loginit);
+        dir.read_binary_file(c"logintr", &self.logintr);
+        dir.read_binary_file(c"logrm", &self.logrm);
+        dir.read_binary_file(c"logmnoc", &self.logmnoc);
+        dir.read_binary_file(c"logroot", &self.logroot);
+        dir.read_binary_file(c"logrmon", &self.logrmon);
     }
 }
 
 /// GSP runtime data.
 #[pin_data]
 pub(crate) struct Gsp<'gsp> {
+    /// The GSP firmware's TLV.
+    gsp_tlv: firmware::Firmware,
     /// Libos arguments.
     pub(crate) libos: Coherent<'gsp, [LibosMemoryRegionInitArgument]>,
     /// Log buffers, optionally exposed via debugfs.
@@ -242,13 +382,25 @@ impl<'gsp> Gsp<'gsp> {
     // Creates an in-place initializer for a `Gsp` manager for `pdev`.
     pub(crate) fn new(
         pdev: &'gsp pci::Device<device::Bound>,
+        spec: Spec,
         bar: Bar0<'gsp>,
     ) -> impl PinInit<Self, Error> + 'gsp {
         pin_init::pin_init_scope(move || {
             let dev = pdev.as_ref();
-            let log_buffers = LogBuffers::new(dev)?;
+
+            let gsp_tlv = request_tlv(dev, spec.chipset, "gsp")?;
+            let build_id = BuildId::from_tlv(&Tlv::new(gsp_tlv.data())?)
+                .inspect_err(|_| {
+                    dev_warn!(
+                        dev,
+                        "no build ID in the GSP firmware TLV, so its log dumps carry no header\n"
+                    )
+                })
+                .ok();
+            let log_buffers = LogBuffers::new(dev, spec, build_id.as_ref())?;
 
             Ok(try_pin_init!(Self {
+                gsp_tlv,
                 cmdq <- Cmdq::new(dev, bar),
                 rm_state_monitor: Coherent::zeroed(dev, GFP_KERNEL)?,
                 rmargs: Coherent::init(
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 5da3d134eec7..6f8f330a0297 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -362,7 +362,7 @@ pub(crate) fn boot(
         let dev = pdev.as_ref();
         let hal = super::hal::gsp_hal(chipset);
 
-        let gsp_fw = KBox::pin_init(GspFirmware::new(dev, chipset), GFP_KERNEL)?;
+        let gsp_fw = KBox::pin_init(GspFirmware::new(dev, chipset, &self.gsp_tlv), GFP_KERNEL)?;
 
         let generic_bootloader = hal.generic_bootloader(dev, chipset, gsp_falcon.imem_size())?;
 
-- 
2.55.0

[PATCH 3/4] gpu: nova-core: gsp: print GMC command names in debug logs

[John Hubbard]

The GMC (GPU Management Controller) API names each command in the
GMCAPI_COMMANDS enum of the firmware bindings, and a message carries
the enum's numeric id.

The GMC send, receive and error lines printed the bare id, so a reader
had to look 0x10001 up in the bindings to learn that it was GSP_INIT:

    GSP GMC: send: seq# 0, command_id=0x10001, length=0xf8

Print the name beside the id, from a type that wraps an id for
formatting, so that the ids stay plain integers at the send and receive
interfaces:

    GSP GMC: send: seq# 0, command=GSP_INIT (0x10001), length=0xf8

Assisted-by: LLM
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/gsp/boot.rs |  9 ++--
 drivers/gpu/nova-core/gsp/cmdq.rs | 13 +++---
 drivers/gpu/nova-core/gsp/fw.rs   | 70 +++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 6f8f330a0297..8848feebb9db 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -42,6 +42,7 @@
         cmdq::Cmdq,
         commands,
         fw::{
+            GmcCommand,
             GspArgumentsPadded,
             GMCAPI_CMD_EXEC_GENERIC_BOOTLOADER,
             GMCAPI_CMD_EXEC_HS_BINARY, //
@@ -169,8 +170,8 @@ fn dispatch_gmc_boot_event(
             _ => {
                 dev_err!(
                     self.dev,
-                    "Unexpected GMC boot event: command_id={:#010x}\n",
-                    command_id
+                    "Unexpected GMC boot event: command={}\n",
+                    GmcCommand(command_id)
                 );
                 return Err(EINVAL);
             }
@@ -179,8 +180,8 @@ fn dispatch_gmc_boot_event(
         handled.and_then(|()| self.core_resume()).inspect_err(|e| {
             dev_err!(
                 self.dev,
-                "GMC boot event {:#010x} failed: {:?}\n",
-                command_id,
+                "GMC boot event {} failed: {:?}\n",
+                GmcCommand(command_id),
                 e
             );
         })
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index dd0b3dfc6f37..2b53b1e6cebf 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -52,6 +52,7 @@
     driver::Bar0,
     gsp::{
         fw::{
+            GmcCommand,
             GspGmcMsgElement,
             GspMsgElement,
             MsgFunction,
@@ -884,9 +885,9 @@ fn send_gmc(&mut self, command_id: u32, payload: &[u8], max_response_size: u32)
 
         dev_dbg!(
             &self.dev,
-            "GSP GMC: send: seq# {}, command_id=0x{:x}, length=0x{:x}\n",
+            "GSP GMC: send: seq# {}, command={}, length=0x{:x}\n",
             rpc_seq,
-            command_id,
+            GmcCommand(command_id),
             dst.header.length(),
         );
 
@@ -1063,9 +1064,9 @@ fn log_event(&self, function: Result<MsgFunction, u32>, seq: u32) {
     fn log_gmc_event(&self, header: &GspGmcMsgElement) {
         dev_warn!(
             &self.dev,
-            "GSP GMC: dropping unclaimed message (seq {}, command_id=0x{:x})\n",
+            "GSP GMC: dropping unclaimed message (seq {}, command={})\n",
             header.gmc.sequence,
-            header.gmc.command_id(),
+            GmcCommand(header.gmc.command_id()),
         );
     }
 
@@ -1269,9 +1270,9 @@ fn consume_element<R>(
             ),
             QueueElement::Gmc(message) => dev_dbg!(
                 &self.dev,
-                "GSP GMC: receive: seq# {}, command_id=0x{:x}, length=0x{:x}\n",
+                "GSP GMC: receive: seq# {}, command={}, length=0x{:x}\n",
                 message.header.gmc.sequence,
-                message.header.gmc.command_id(),
+                GmcCommand(message.header.gmc.command_id()),
                 message.header.length(),
             ),
         }
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index f7e83e75e53e..106d71f95fc3 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -12,6 +12,7 @@
 use kernel::{
     bitfield,
     dma::Coherent,
+    fmt,
     io::io_write,
     prelude::*,
     ptr::{
@@ -802,6 +803,75 @@ unsafe impl AsBytes for GmcApiHeader {}
 // SAFETY: All fields are integer types for which all bit patterns are valid.
 unsafe impl FromBytes for GmcApiHeader {}
 
+/// A GMC command id that formats with its name.
+///
+/// [`fmt::Display`] writes the name that the bindings give the id, then the id itself, as in
+/// `GSP_INIT (0x10001)`. An id that the bindings do not name writes `UNKNOWN`.
+#[derive(Copy, Clone)]
+pub(crate) struct GmcCommand(pub(crate) u32);
+
+impl GmcCommand {
+    /// Returns the name of the id in the `GMCAPI_COMMANDS` enum of the bindings.
+    fn name(self) -> &'static str {
+        match self.0 {
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_GSP_INIT => "GSP_INIT",
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_EXEC_GENERIC_BOOTLOADER => {
+                "EXEC_GENERIC_BOOTLOADER"
+            }
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_EXEC_HS_BINARY => "EXEC_HS_BINARY",
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_GSP_SUSPEND => "GSP_SUSPEND",
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_GSP_RESUME_DONE => "GSP_RESUME_DONE",
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_ADD_VGPU_TYPE => "ADD_VGPU_TYPE",
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_QUERY_SUPPORTED_VGPU_TYPES => {
+                "QUERY_SUPPORTED_VGPU_TYPES"
+            }
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_QUERY_CREATABLE_VGPU_TYPES => {
+                "QUERY_CREATABLE_VGPU_TYPES"
+            }
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_ASSIGN_VGPU_TYPE => "ASSIGN_VGPU_TYPE",
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_DEASSIGN_VGPU_TYPE => "DEASSIGN_VGPU_TYPE",
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_QUERY_VGPU_PROPERTIES => "QUERY_VGPU_PROPERTIES",
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_QUERY_ASSIGNED_VF_VGPU_TYPE => {
+                "QUERY_ASSIGNED_VF_VGPU_TYPE"
+            }
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_BOOTLOAD_GSP_VGPU_PLUGIN_TASK => {
+                "BOOTLOAD_GSP_VGPU_PLUGIN_TASK"
+            }
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_SHUTDOWN_GSP_VGPU_PLUGIN_TASK => {
+                "SHUTDOWN_GSP_VGPU_PLUGIN_TASK"
+            }
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_SHUTDOWN_GSP_VGPU_PLUGIN_TASK_COMPLETE => {
+                "SHUTDOWN_GSP_VGPU_PLUGIN_TASK_COMPLETE"
+            }
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_CLEANUP_GSP_VGPU_PLUGIN_RESOURCES => {
+                "CLEANUP_GSP_VGPU_PLUGIN_RESOURCES"
+            }
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_VGPU_PLUGIN_TRIGGERED_EVENT => {
+                "VGPU_PLUGIN_TRIGGERED_EVENT"
+            }
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_VGPU_MGR_SCRUB_GUEST_FB => {
+                "VGPU_MGR_SCRUB_GUEST_FB"
+            }
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_VGPU_MGR_ALLOC_GSP_CEUTILS => {
+                "VGPU_MGR_ALLOC_GSP_CEUTILS"
+            }
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_VGPU_MGR_FREE_GSP_CEUTILS => {
+                "VGPU_MGR_FREE_GSP_CEUTILS"
+            }
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_SCHED_CONTROL => "SCHED_CONTROL",
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_MMU_FAULT_QUEUED => "MMU_FAULT_QUEUED",
+            bindings::GMCAPI_COMMANDS_GMCAPI_CMD_INVALID => "INVALID",
+            _ => "UNKNOWN",
+        }
+    }
+}
+
+impl fmt::Display for GmcCommand {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{} (0x{:x})", self.name(), self.0)
+    }
+}
+
 /// The headers that open a GMC API queue element: the queue element header and the GMC API
 /// header.
 #[repr(C)]
-- 
2.55.0

[PATCH 4/4] gpu: nova-core: gsp: distinguish GMC events from responses in debug logs

[John Hubbard]

GSP-RM numbers its GMC events from bit 63 upward, apart from the
driver's request numbers, and marks a response with a flag in the
command word.

The GMC receive debug line printed the raw sequence and labeled every
element a receive:

    GSP GMC: receive: seq# 9223372036854775808, command=EXEC_GENERIC_BOOTLOADER (0x10002), length=0xb0
    GSP GMC: receive: seq# 0, command=GSP_INIT (0x10001), length=0x450

Print the sequence with bit 63 cleared, and label each received element
an event or a response by the flag:

    GSP GMC: event: seq# 0, command=EXEC_GENERIC_BOOTLOADER (0x10002), length=0xb0
    GSP GMC: response: seq# 0, command=GSP_INIT (0x10001), length=0x450

Assisted-by: LLM
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 35 +++++++++++++++++++++++--------
 drivers/gpu/nova-core/gsp/fw.rs   | 11 +++++++++-
 2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 2b53b1e6cebf..530ad51304ce 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -1062,10 +1062,17 @@ fn log_event(&self, function: Result<MsgFunction, u32>, seq: u32) {
     /// Logs a GMC message that no caller is waiting for: a response to a request that has already
     /// timed out, or an event that arrives outside the boot sequence.
     fn log_gmc_event(&self, header: &GspGmcMsgElement) {
+        let kind = if header.gmc.is_response() {
+            "response"
+        } else {
+            "event"
+        };
+
         dev_warn!(
             &self.dev,
-            "GSP GMC: dropping unclaimed message (seq {}, command={})\n",
-            header.gmc.sequence,
+            "GSP GMC: dropping unclaimed {} (seq# {}, command={})\n",
+            kind,
+            header.gmc.sequence_number(),
             GmcCommand(header.gmc.command_id()),
         );
     }
@@ -1268,13 +1275,23 @@ fn consume_element<R>(
                 message.header.function(),
                 message.header.length(),
             ),
-            QueueElement::Gmc(message) => dev_dbg!(
-                &self.dev,
-                "GSP GMC: receive: seq# {}, command={}, length=0x{:x}\n",
-                message.header.gmc.sequence,
-                GmcCommand(message.header.gmc.command_id()),
-                message.header.length(),
-            ),
+            QueueElement::Gmc(message) => {
+                let gmc = &message.header.gmc;
+                let kind = if gmc.is_response() {
+                    "response"
+                } else {
+                    "event"
+                };
+
+                dev_dbg!(
+                    &self.dev,
+                    "GSP GMC: {}: seq# {}, command={}, length=0x{:x}\n",
+                    kind,
+                    gmc.sequence_number(),
+                    GmcCommand(gmc.command_id()),
+                    message.header.length(),
+                )
+            }
         }
 
         let result = f(self, element);
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 106d71f95fc3..ec474a175ee6 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -729,6 +729,10 @@ pub(crate) struct GmcApiHeader {
 /// Flag bit of [`GmcApiHeader::command`] that GSP-RM sets on a response.
 const GMCAPI_COMMAND_FLAGS_RESPONSE: u32 = 0x0100_0000;
 
+/// First sequence number of the space that GSP-RM numbers its events from, so that an event's
+/// number cannot collide with a request's. Requests are numbered from zero.
+const GMC_EVENT_SEQUENCE_BASE: u64 = 1 << 63;
+
 /// GMC request that carries the system information and registry keys to GSP-RM. GSP-RM answers
 /// it with the static GPU configuration once it has finished starting.
 pub(crate) const GMCAPI_CMD_GSP_INIT: u32 = bindings::GMCAPI_COMMANDS_GMCAPI_CMD_GSP_INIT;
@@ -776,10 +780,15 @@ pub(crate) fn command_id(&self) -> u32 {
     }
 
     /// Returns `true` if GSP-RM sent this header as a response rather than an event.
-    fn is_response(&self) -> bool {
+    pub(crate) fn is_response(&self) -> bool {
         self.command & GMCAPI_COMMAND_FLAGS_RESPONSE != 0
     }
 
+    /// Returns the sequence number without the bit that marks an event's sequence space.
+    pub(crate) fn sequence_number(&self) -> u64 {
+        self.sequence & !GMC_EVENT_SEQUENCE_BASE
+    }
+
     /// Returns the `NV_STATUS` that a response carries.
     ///
     /// The value is meaningful only when [`Self::is_response`] is `true`. In a request, the same
-- 
2.55.0