* [PATCH v2 0/5] gpu: nova-core: gsp: cmdq: preparatory changes for r000 support
@ 2026-11-02 12:09 Alexandre Courbot
2026-11-02 12:09 ` [PATCH v2 1/5] gpu: nova-core: fix stride of NV_PFSP_* registers Alexandre Courbot
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Alexandre Courbot @ 2026-11-02 12:09 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Benno Lossin, Gary Guo
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, linux-kernel, rust-for-linux,
Alexandre Courbot
This series contains patches 5, 6 and 8 from John's r000 series v3 [1]
that prepare the command queue so that the r000 switch applies more
cleanly. They are submitted separately to make review and merging
faster, and because they are also valuable even without the r000 switch.
Patch 1 is a drive-by fix of the `NV_PFSP` register declarations that is
included here as the `NV_PGSP` declarations mirror the `NV_PFSP` ones.
Patch 5 is another requirement for the r000 command queue.
No functional change is introduced by this series.
Applies cleanly on `drm-rust-next`, and runtime-tested successfully on
Ampere.
[1] https://lore.kernel.org/20260918010719.1176945-1-jhubbard@nvidia.com
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Changes in v2:
- Add patch returning the sequence number of posted commands.
- Add `Fixes:` tag to patch 1. (Eliot)
- Deslop a couple of commit messages. (Eliot)
- Use I/O projections in the command queue. (Gary & Eliot)
- Link to v1: https://patch.msgid.link/20261030-r000-cmdq-prep-v1-0-f2befbcd8ba9@nvidia.com
---
Alexandre Courbot (2):
gpu: nova-core: fix stride of NV_PFSP_* registers
gpu: nova-core: gsp: cmdq: expose the sequence number of sent commands
John Hubbard (3):
gpu: nova-core: regs: add msgq v2 BAR0 register declarations
gpu: nova-core: gsp: cmdq: ring the GSP doorbell from the queue memory
gpu: nova-core: gsp: cmdq: compute the queue regions from a count and a slot
drivers/gpu/nova-core/gsp/cmdq.rs | 187 ++++++++++++++++------------------
drivers/gpu/nova-core/gsp/cmdq/rpc.rs | 4 +-
drivers/gpu/nova-core/gsp/regs.rs | 22 +++-
drivers/gpu/nova-core/regs.rs | 8 +-
4 files changed, 118 insertions(+), 103 deletions(-)
---
base-commit: 9fe580fd90e20c8432832c5f9a7e92b049ae9af6
change-id: 20261030-r000-cmdq-prep-6c5644619c11
Best regards,
--
Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/5] gpu: nova-core: fix stride of NV_PFSP_* registers
2026-11-02 12:09 [PATCH v2 0/5] gpu: nova-core: gsp: cmdq: preparatory changes for r000 support Alexandre Courbot
@ 2026-11-02 12:09 ` Alexandre Courbot
2026-11-02 12:09 ` [PATCH v2 2/5] gpu: nova-core: regs: add msgq v2 BAR0 register declarations Alexandre Courbot
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Alexandre Courbot @ 2026-11-02 12:09 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Benno Lossin, Gary Guo
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, linux-kernel, rust-for-linux,
Alexandre Courbot
OpenRM defines these registers with a stride of 8 bytes, so reflect that
fact in our own declarations. That inconsistency went unnoticed because
nova-core only ever uses the first register of each array at the moment.
Fixes: ab42c32347ef ("gpu: nova-core: Hopper/Blackwell: add FSP message infrastructure")
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/regs.rs | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 88662da634dc..df2eddc58efb 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -402,19 +402,19 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
register! {
base: NovaRegisters;
- pub(crate) NV_PFSP_QUEUE_HEAD(u32)[8] @ 0x008f2c00 {
+ pub(crate) NV_PFSP_QUEUE_HEAD(u32)[8, stride = 8] @ 0x008f2c00 {
31:0 address => u32;
}
- pub(crate) NV_PFSP_QUEUE_TAIL(u32)[8] @ 0x008f2c04 {
+ pub(crate) NV_PFSP_QUEUE_TAIL(u32)[8, stride = 8] @ 0x008f2c04 {
31:0 address => u32;
}
- pub(crate) NV_PFSP_MSGQ_HEAD(u32)[8] @ 0x008f2c80 {
+ pub(crate) NV_PFSP_MSGQ_HEAD(u32)[8, stride = 8] @ 0x008f2c80 {
31:0 val => u32;
}
- pub(crate) NV_PFSP_MSGQ_TAIL(u32)[8] @ 0x008f2c84 {
+ pub(crate) NV_PFSP_MSGQ_TAIL(u32)[8, stride = 8] @ 0x008f2c84 {
31:0 val => u32;
}
}
--
2.56.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/5] gpu: nova-core: regs: add msgq v2 BAR0 register declarations
2026-11-02 12:09 [PATCH v2 0/5] gpu: nova-core: gsp: cmdq: preparatory changes for r000 support Alexandre Courbot
2026-11-02 12:09 ` [PATCH v2 1/5] gpu: nova-core: fix stride of NV_PFSP_* registers Alexandre Courbot
@ 2026-11-02 12:09 ` Alexandre Courbot
2026-11-02 12:09 ` [PATCH v2 3/5] gpu: nova-core: gsp: cmdq: ring the GSP doorbell from the queue memory Alexandre Courbot
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Alexandre Courbot @ 2026-11-02 12:09 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Benno Lossin, Gary Guo
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, linux-kernel, rust-for-linux,
Alexandre Courbot
From: John Hubbard <jhubbard@nvidia.com>
Msgq is the pair of ring buffers in shared memory through which the
driver and GSP-RM communicate. The command queue carries commands from
the driver to GSP-RM, and the message queue carries messages from GSP-RM
to the driver. Each ring buffer has a write pointer and a read pointer.
Msgq v0, which the r570 firmware uses, keeps both pointers of each ring
buffer in that ring buffer's header, in shared memory. Msgq v2, which
the r000 firmware uses, keeps the four pointers, two per ring buffer, in
four BAR0 registers instead:
NV_PGSP_QUEUE_HEAD command queue write pointer
NV_PGSP_QUEUE_TAIL command queue read pointer
NV_PGSP_MSGQ_HEAD message queue write pointer
NV_PGSP_MSGQ_TAIL message queue read pointer
The GSP numbers its queues 0 through 7, and each numbered queue is one
such pair of ring buffers with its own four pointer registers. Nova-core
uses queue 0 only.
A write to NV_PGSP_QUEUE_HEAD also interrupts the GSP. Msgq v0 uses that
write as the doorbell, so nova-core already declares NV_PGSP_QUEUE_HEAD.
Declare the other three, NV_PGSP_QUEUE_TAIL, NV_PGSP_MSGQ_HEAD and
NV_PGSP_MSGQ_TAIL.
No functional change intended.
[ acourbot: declare the registers as arrays of 8 with a stride of 8. ]
[ acourbot: use "val" for the field name of NV_PGSP_MSGQ as per OpenRM. ]
Assisted-by: LLM
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/gsp/cmdq.rs | 6 +++++-
drivers/gpu/nova-core/gsp/regs.rs | 22 +++++++++++++++++++++-
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 936406004a57..07b21a6f4504 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -15,6 +15,7 @@
io::{
io_project,
poll::read_poll_timeout,
+ register::Array,
Io, //
},
new_mutex,
@@ -496,7 +497,10 @@ fn calculate_checksum<T: Iterator<Item = u8>>(it: T) -> u32 {
/// Notifies the GSP that we have updated the command queue pointers.
fn notify_gsp(bar: Bar0<'_>) {
- bar.write_reg(regs::NV_PGSP_QUEUE_HEAD::zeroed().with_address(0u32));
+ bar.write(
+ Array::at(0),
+ regs::NV_PGSP_QUEUE_HEAD::zeroed().with_address(0u32),
+ );
}
/// Logs and consumes every message the GSP has already posted, and returns without waiting for
diff --git a/drivers/gpu/nova-core/gsp/regs.rs b/drivers/gpu/nova-core/gsp/regs.rs
index 3c410d65e8e4..e3fbbf364b7b 100644
--- a/drivers/gpu/nova-core/gsp/regs.rs
+++ b/drivers/gpu/nova-core/gsp/regs.rs
@@ -9,12 +9,32 @@
// PGSP
+// The GSP has eight numbered queues, each with its own four msgq pointer registers interleaved
+// at a stride of 8 bytes. nova-core uses queue 0 only.
+
register! {
base: NovaRegisters;
- pub(super) NV_PGSP_QUEUE_HEAD(u32) @ 0x00110c00 {
+ /// Write pointer of the CPU-to-GSP command queue, which the driver advances. A write to this
+ /// register also interrupts the GSP, which makes it the doorbell.
+ pub(super) NV_PGSP_QUEUE_HEAD(u32)[8, stride = 8] @ 0x00110c00 {
31:0 address;
}
+
+ /// Read pointer of the CPU-to-GSP command queue, which the GSP advances.
+ pub(super) NV_PGSP_QUEUE_TAIL(u32)[8, stride = 8] @ 0x00110c04 {
+ 31:0 address;
+ }
+
+ /// Write pointer of the GSP-to-CPU message queue, which the GSP advances.
+ pub(super) NV_PGSP_MSGQ_HEAD(u32)[8, stride = 8] @ 0x00110c80 {
+ 31:0 val;
+ }
+
+ /// Read pointer of the GSP-to-CPU message queue, which the driver advances.
+ pub(super) NV_PGSP_MSGQ_TAIL(u32)[8, stride = 8] @ 0x00110c84 {
+ 31:0 val;
+ }
}
// PBUS
--
2.56.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 3/5] gpu: nova-core: gsp: cmdq: ring the GSP doorbell from the queue memory
2026-11-02 12:09 [PATCH v2 0/5] gpu: nova-core: gsp: cmdq: preparatory changes for r000 support Alexandre Courbot
2026-11-02 12:09 ` [PATCH v2 1/5] gpu: nova-core: fix stride of NV_PFSP_* registers Alexandre Courbot
2026-11-02 12:09 ` [PATCH v2 2/5] gpu: nova-core: regs: add msgq v2 BAR0 register declarations Alexandre Courbot
@ 2026-11-02 12:09 ` Alexandre Courbot
2026-11-02 12:09 ` [PATCH v2 4/5] gpu: nova-core: gsp: cmdq: compute the queue regions from a count and a slot Alexandre Courbot
2026-11-02 12:09 ` [PATCH v2 5/5] gpu: nova-core: gsp: cmdq: expose the sequence number of sent commands Alexandre Courbot
4 siblings, 0 replies; 8+ messages in thread
From: Alexandre Courbot @ 2026-11-02 12:09 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Benno Lossin, Gary Guo
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, linux-kernel, rust-for-linux,
Alexandre Courbot
From: John Hubbard <jhubbard@nvidia.com>
A command reaches GSP-RM in two steps. The driver advances the command
queue's write pointer, which publishes the command, and then writes
NV_PGSP_QUEUE_HEAD, which interrupts the GSP so that it reads the queue.
In msgq v0, the write pointer is in the queue's shared-memory header, so
the two steps are two writes. In msgq v2, the write pointer is
NV_PGSP_QUEUE_HEAD itself, so one register write does both.
Nova-core split the two steps across two types. The queue memory type
advanced the write pointer in shared memory, and the command queue type
then rang the doorbell through a BAR0 mapping that it held for that one
purpose.
Move the BAR0 mapping into the queue memory type, and ring the doorbell
at the end of the operation that advances the write pointer in order to
prepare for the switch to msgq v2.
No functional change intended.
[ acourbot: slightly reword and trim commit message. ]
Assisted-by: LLM
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/gsp/cmdq.rs | 52 +++++++++++++++++++--------------------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 07b21a6f4504..90e26c4dcc28 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -157,11 +157,16 @@ unsafe impl FromBytes for GspMem {}
/// pointer and the GSP read pointer. This region is returned by [`Self::driver_write_area`].
/// * The driver owns (i.e. can read from) the part of the GSP message queue between the CPU read
/// pointer and the GSP write pointer. This region is returned by [`Self::driver_read_area`].
-struct DmaGspMem<'a>(Coherent<'a, GspMem>);
+struct DmaGspMem<'a> {
+ /// The queues, mapped for the GSP.
+ mem: Coherent<'a, GspMem>,
+ /// MMIO mapping of PCI BAR0, for the doorbell register.
+ bar: Bar0<'a>,
+}
impl<'a> DmaGspMem<'a> {
/// Allocate a new instance and map it for `dev`.
- fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
+ fn new(dev: &'a device::Device<device::Bound>, bar: Bar0<'a>) -> Result<Self> {
const MSGQ_SIZE: u32 = cv!(size_of::<Msgq>());
const RX_HDR_OFF: u32 = cv!(mem::offset_of!(Msgq, rx));
@@ -172,7 +177,7 @@ fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
let gsp_mem: Coherent<'_, _> = gsp_mem.into();
PteArray::init(io_project!(gsp_mem, .ptes), gsp_mem.dma_address())?;
- Ok(Self(gsp_mem))
+ Ok(Self { mem: gsp_mem, bar })
}
/// Returns the region of the CPU message queue that the driver is currently allowed to write
@@ -185,7 +190,7 @@ fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
let rx = self.gsp_read_ptr();
// Pointer to the first entry of the CPU message queue.
- let data = ptr::project!(mut self.0.as_mut_ptr(), .cpuq.msgq.data[build: 0]);
+ let data = ptr::project!(mut self.mem.as_mut_ptr(), .cpuq.msgq.data[build: 0]);
let (tail_end, wrap_end) = if rx == 0 {
// The write area is non-wrapping, and stops at the second-to-last entry of the command
@@ -247,7 +252,7 @@ fn driver_write_area_size(&self) -> usize {
let rx = self.cpu_read_ptr();
// Pointer to the first entry of the GSP message queue.
- let data = ptr::project!(self.0.as_ptr(), .gspq.msgq.data[build: 0]);
+ let data = ptr::project!(self.mem.as_ptr(), .gspq.msgq.data[build: 0]);
let (tail_end, wrap_end) = if rx <= tx {
// Read area is non-wrapping and stops right before `tx`.
@@ -331,7 +336,7 @@ fn allocate_command(&mut self, size: usize, timeout: Delta) -> Result<GspCommand
//
// - The returned value is within `0..MSGQ_NUM_PAGES`.
fn gsp_write_ptr(&self) -> u32 {
- let ptr = MsgqTxHeader::write_ptr(io_project!(self.0, .gspq.tx)) % MSGQ_NUM_PAGES;
+ let ptr = MsgqTxHeader::write_ptr(io_project!(self.mem, .gspq.tx)) % MSGQ_NUM_PAGES;
// ORDERING: LOAD->LOAD ordering needed to order `gsp_write_ptr` read before data read.
dma_mb(Read);
@@ -345,7 +350,7 @@ fn gsp_write_ptr(&self) -> u32 {
//
// - The returned value is within `0..MSGQ_NUM_PAGES`.
fn gsp_read_ptr(&self) -> u32 {
- let ptr = MsgqRxHeader::read_ptr(io_project!(self.0, .gspq.rx)) % MSGQ_NUM_PAGES;
+ let ptr = MsgqRxHeader::read_ptr(io_project!(self.mem, .gspq.rx)) % MSGQ_NUM_PAGES;
// ORDERING: LOAD->STORE ordering needed to order `gsp_read_ptr` read before data write.
dma_mb(Full);
@@ -359,7 +364,7 @@ fn gsp_read_ptr(&self) -> u32 {
//
// - The returned value is within `0..MSGQ_NUM_PAGES`.
fn cpu_read_ptr(&self) -> u32 {
- MsgqRxHeader::read_ptr(io_project!(self.0, .cpuq.rx)) % MSGQ_NUM_PAGES
+ MsgqRxHeader::read_ptr(io_project!(self.mem, .cpuq.rx)) % MSGQ_NUM_PAGES
}
// Informs the GSP that it can send `elem_count` new pages into the message queue.
@@ -367,7 +372,7 @@ fn advance_cpu_read_ptr(&mut self, elem_count: u32) {
// ORDERING: LOAD->STORE ordering needed to order `cpu_read_ptr` write after data read.
dma_mb(Full);
- let rx = io_project!(self.0, .cpuq.rx);
+ let rx = io_project!(self.mem, .cpuq.rx);
let rptr = MsgqRxHeader::read_ptr(rx).wrapping_add(elem_count) % MSGQ_NUM_PAGES;
MsgqRxHeader::set_read_ptr(rx, rptr)
}
@@ -378,17 +383,24 @@ fn advance_cpu_read_ptr(&mut self, elem_count: u32) {
//
// - The returned value is within `0..MSGQ_NUM_PAGES`.
fn cpu_write_ptr(&self) -> u32 {
- MsgqTxHeader::write_ptr(io_project!(self.0, .cpuq.tx)) % MSGQ_NUM_PAGES
+ MsgqTxHeader::write_ptr(io_project!(self.mem, .cpuq.tx)) % MSGQ_NUM_PAGES
}
- // Informs the GSP that it can process `elem_count` new pages from the command queue.
+ // Publishes `elem_count` more pages of the command queue to the GSP and rings the doorbell.
fn advance_cpu_write_ptr(&mut self, elem_count: u32) {
// ORDERING: STORE->STORE ordering needed to order `cpu_write_ptr` write after data write.
dma_mb(Write);
- let tx = io_project!(self.0, .cpuq.tx);
+ let tx = io_project!(self.mem, .cpuq.tx);
let wptr = MsgqTxHeader::write_ptr(tx).wrapping_add(elem_count) % MSGQ_NUM_PAGES;
MsgqTxHeader::set_write_ptr(tx, wptr);
+
+ // A write to the head register interrupts the GSP. The pointer itself is in the
+ // shared-memory header, so the value written does not matter.
+ self.bar.write(
+ Array::at(0),
+ regs::NV_PGSP_QUEUE_HEAD::zeroed().with_address(0u32),
+ );
}
}
@@ -468,13 +480,12 @@ pub(crate) fn new(
bar: Bar0<'cmdq>,
) -> impl PinInit<Self, Error> + 'cmdq {
pin_init_scope(move || {
- let gsp_mem = DmaGspMem::new(dev)?;
+ let gsp_mem = DmaGspMem::new(dev, bar)?;
Ok(try_pin_init!(Self {
- dma_addr: gsp_mem.0.dma_address(),
+ dma_addr: gsp_mem.mem.dma_address(),
inner <- new_mutex!(CmdqInner {
dev,
- bar,
gsp_mem,
seq: 0,
}),
@@ -495,14 +506,6 @@ fn calculate_checksum<T: Iterator<Item = u8>>(it: T) -> u32 {
((sum64 >> 32) as u32) ^ (sum64 as u32)
}
- /// Notifies the GSP that we have updated the command queue pointers.
- fn notify_gsp(bar: Bar0<'_>) {
- bar.write(
- Array::at(0),
- regs::NV_PGSP_QUEUE_HEAD::zeroed().with_address(0u32),
- );
- }
-
/// Logs and consumes every message the GSP has already posted, and returns without waiting for
/// more.
///
@@ -522,8 +525,6 @@ pub(crate) fn drain(&self) -> Result {
struct CmdqInner<'a> {
/// Device this command queue belongs to.
dev: &'a device::Device,
- /// MMIO mapping of PCI BAR0, for writing the GSP doorbell.
- bar: Bar0<'a>,
/// Current command sequence number.
seq: u32,
/// Memory area shared with the GSP for communicating commands and messages.
@@ -571,7 +572,6 @@ fn send_command_element<E: CommandElement>(&mut self, command: E) -> Result {
let elem_count = dst.header.element_count();
self.seq += 1;
self.gsp_mem.advance_cpu_write_ptr(elem_count);
- Cmdq::notify_gsp(self.bar);
Ok(())
}
--
2.56.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 4/5] gpu: nova-core: gsp: cmdq: compute the queue regions from a count and a slot
2026-11-02 12:09 [PATCH v2 0/5] gpu: nova-core: gsp: cmdq: preparatory changes for r000 support Alexandre Courbot
` (2 preceding siblings ...)
2026-11-02 12:09 ` [PATCH v2 3/5] gpu: nova-core: gsp: cmdq: ring the GSP doorbell from the queue memory Alexandre Courbot
@ 2026-11-02 12:09 ` Alexandre Courbot
2026-11-02 13:27 ` Eliot Courtney
2026-11-02 12:09 ` [PATCH v2 5/5] gpu: nova-core: gsp: cmdq: expose the sequence number of sent commands Alexandre Courbot
4 siblings, 1 reply; 8+ messages in thread
From: Alexandre Courbot @ 2026-11-02 12:09 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Benno Lossin, Gary Guo
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, linux-kernel, rust-for-linux,
Alexandre Courbot
From: John Hubbard <jhubbard@nvidia.com>
`driver_write_area` and `driver_read_area` both branch on the relative
position of the read and write pointers to decide where the two slices
they return end. This works for msgq v0, where the pointers are indices
into the ring, but msgq v2 (used by the r000 firmware) keeps them in
BAR0 registers as free-running counters, so the branches and their
SAFETY arguments would have to be rewritten for it.
Instead, compute the number of slots available to the driver and the
slot they start at, and derive the slices from that. The slot counts
come from one helper per queue, which are the only two places msgq v2
will need to touch.
Use I/O projections to limit the amount of unsafe code to the bare
minimum.
No functional change intended.
Assisted-by: LLM
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/gsp/cmdq.rs | 130 +++++++++++++++++---------------------
1 file changed, 58 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 90e26c4dcc28..33cb9174c1e6 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -19,8 +19,8 @@
Io, //
},
new_mutex,
+ num::casts::FromSafeCast,
prelude::*,
- ptr,
sync::{
barrier::{
dma_mb,
@@ -186,60 +186,58 @@ fn new(dev: &'a device::Device<device::Bound>, bar: Bar0<'a>) -> Result<Self> {
/// As the message queue is a circular buffer, the region may be discontiguous in memory. In
/// that case the second slice will have a non-zero length.
fn driver_write_area(&mut self) -> (&mut [[u8; GSP_PAGE_SIZE]], &mut [[u8; GSP_PAGE_SIZE]]) {
- let tx = self.cpu_write_ptr();
- let rx = self.gsp_read_ptr();
+ let avail = usize::from_safe_cast(self.cpuq_free_slots());
+ let w_slot = usize::from_safe_cast(self.cpu_write_ptr());
- // Pointer to the first entry of the CPU message queue.
- let data = ptr::project!(mut self.mem.as_mut_ptr(), .cpuq.msgq.data[build: 0]);
+ // `w_slot` is below `MSGQ_NUM_PAGES` per the invariant of `cpu_write_ptr`, so the
+ // subtraction cannot underflow.
+ let before_wrap = avail.min(cv!(MSGQ_NUM_PAGES => usize) - w_slot);
+ // Per the line above, `before_wrap` is less or equal to `avail` so the subtraction cannot
+ // underflow.
+ let after_wrap = avail - before_wrap;
- let (tail_end, wrap_end) = if rx == 0 {
- // The write area is non-wrapping, and stops at the second-to-last entry of the command
- // queue (to leave the last one empty).
- (MSGQ_NUM_PAGES - 1, 0)
- } else if rx <= tx {
- // The write area wraps and continues until `rx - 1`.
- (MSGQ_NUM_PAGES, rx - 1)
- } else {
- // The write area doesn't wrap and stops at `rx - 1`.
- (rx - 1, 0)
- };
+ // PANIC: `w_slot + before_wrap` is at most `MSGQ_NUM_PAGES` by construction of
+ // `before_wrap`.
+ let after = io_project!(self.mem, .cpuq.msgq.data[panic: w_slot..w_slot + before_wrap]);
+ // PANIC: `after_wrap` is at most `avail`, which is below `MSGQ_NUM_PAGES`.
+ let before = io_project!(self.mem, .cpuq.msgq.data[panic: ..after_wrap]);
// SAFETY:
- // - `data` was created from a valid pointer, and `rx` and `tx` are in the
- // `0..MSGQ_NUM_PAGES` range per the invariants of `cpu_write_ptr` and `gsp_read_ptr`,
- // thus the created slices are valid.
- // - The area starting at `tx` and ending at `rx - 2` modulo `MSGQ_NUM_PAGES`,
- // inclusive, belongs to the driver for writing and is not accessed concurrently by
- // the GSP.
- // - The caller holds a reference to `self` for as long as the returned slices are live,
- // meaning the CPU write pointer cannot be advanced and thus that the returned area
- // remains exclusive to the CPU for the duration of the slices.
- // - The created slices point to non-overlapping sub-ranges of `data` in all
- // branches (in the `rx <= tx` case, the second slice ends at `rx - 1` which is strictly
- // less than `tx` where the first slice starts; in the other cases the second slice is
- // empty), so creating two `&mut` references from them does not violate aliasing rules.
- unsafe {
- (
- core::slice::from_raw_parts_mut(
- data.add(num::u32_as_usize(tx)),
- num::u32_as_usize(tail_end - tx),
- ),
- core::slice::from_raw_parts_mut(data, num::u32_as_usize(wrap_end)),
- )
- }
+ // - Both regions are within the `avail` entries starting at `w_slot` that the driver owns,
+ // meaning the GSP won't touch them until the write pointer is advanced.
+ // - There are no other references possible as `self` is mutably borrowed.
+ // - The regions do not overlap: `avail` is at most `MSGQ_NUM_PAGES - 1`, so `after_wrap` is
+ // strictly less than `w_slot` whenever it is non-zero.
+ unsafe { (after.as_mut(), before.as_mut()) }
}
- /// 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 {
+ /// Returns the number of slots of the CPU message queue that the driver is currently allowed
+ /// to write to.
+ fn cpuq_free_slots(&self) -> u32 {
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
+ (rx + MSGQ_NUM_PAGES - tx - 1) % MSGQ_NUM_PAGES
+ }
+
+ /// 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 {
+ num::u32_as_usize(self.cpuq_free_slots()) * GSP_PAGE_SIZE
+ }
+
+ /// Returns the number of slots of the GSP message queue that the driver is currently allowed
+ /// to read from.
+ fn gspq_readable_slots(&self) -> u32 {
+ let tx = self.gsp_write_ptr();
+ let rx = self.cpu_read_ptr();
+
+ // `rx` and `tx` are both in `0..MSGQ_NUM_PAGES` per the invariants of `gsp_write_ptr` and
+ // `cpu_read_ptr`, so the subtraction cannot underflow.
+ (tx + MSGQ_NUM_PAGES - rx) % MSGQ_NUM_PAGES
}
/// Returns the region of the GSP message queue that the driver is currently allowed to read
@@ -248,39 +246,27 @@ fn driver_write_area_size(&self) -> usize {
/// As the message queue is a circular buffer, the region may be discontiguous in memory. In
/// that case the second slice will have a non-zero length.
fn driver_read_area(&self) -> (&[[u8; GSP_PAGE_SIZE]], &[[u8; GSP_PAGE_SIZE]]) {
- let tx = self.gsp_write_ptr();
- let rx = self.cpu_read_ptr();
+ let avail = usize::from_safe_cast(self.gspq_readable_slots());
+ let r_slot = usize::from_safe_cast(self.cpu_read_ptr());
- // Pointer to the first entry of the GSP message queue.
- let data = ptr::project!(self.mem.as_ptr(), .gspq.msgq.data[build: 0]);
+ // `r_slot` is below `MSGQ_NUM_PAGES` per the invariant of `cpu_read_ptr`, so the
+ // subtraction cannot underflow.
+ let before_wrap = avail.min(cv!(MSGQ_NUM_PAGES => usize) - r_slot);
+ // Per the line above, `before_wrap` is less or equal to `avail` so the subtraction cannot
+ // underflow.
+ let after_wrap = avail - before_wrap;
- let (tail_end, wrap_end) = if rx <= tx {
- // Read area is non-wrapping and stops right before `tx`.
- (tx, 0)
- } else {
- // Read area is wrapping and stops right before `tx`.
- (MSGQ_NUM_PAGES, tx)
- };
+ // PANIC: `r_slot + before_wrap` is at most `MSGQ_NUM_PAGES` by construction of
+ // `before_wrap`.
+ let after = io_project!(self.mem, .gspq.msgq.data[panic: r_slot..r_slot + before_wrap]);
+ // PANIC: `after_wrap` is at most `avail`, which is below `MSGQ_NUM_PAGES`.
+ let before = io_project!(self.mem, .gspq.msgq.data[panic: ..after_wrap]);
// SAFETY:
- // - `data` was created from a valid pointer, and `rx` and `tx` are in the
- // `0..MSGQ_NUM_PAGES` range per the invariants of `gsp_write_ptr` and `cpu_read_ptr`,
- // thus the created slices are valid.
- // - The area starting at `rx` and ending at `tx - 1` modulo `MSGQ_NUM_PAGES`,
- // inclusive, belongs to the driver for reading and is not accessed concurrently by
- // the GSP.
- // - The caller holds a reference to `self` for as long as the returned slices are live,
- // meaning the CPU read pointer cannot be advanced and thus that the returned area
- // remains exclusive to the CPU for the duration of the slices.
- unsafe {
- (
- core::slice::from_raw_parts(
- data.add(num::u32_as_usize(rx)),
- num::u32_as_usize(tail_end - rx),
- ),
- core::slice::from_raw_parts(data, num::u32_as_usize(wrap_end)),
- )
- }
+ // - Both regions are within the `avail` entries starting at `r_slot` that the driver owns,
+ // meaning the GSP won't touch them until the read pointer is advanced.
+ // - There are no mutable references possible as `self` is borrowed.
+ unsafe { (after.as_ref(), before.as_ref()) }
}
/// Allocates a region on the command queue that is large enough to send a command of `size`
--
2.56.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 5/5] gpu: nova-core: gsp: cmdq: expose the sequence number of sent commands
2026-11-02 12:09 [PATCH v2 0/5] gpu: nova-core: gsp: cmdq: preparatory changes for r000 support Alexandre Courbot
` (3 preceding siblings ...)
2026-11-02 12:09 ` [PATCH v2 4/5] gpu: nova-core: gsp: cmdq: compute the queue regions from a count and a slot Alexandre Courbot
@ 2026-11-02 12:09 ` Alexandre Courbot
2026-11-02 13:17 ` Eliot Courtney
4 siblings, 1 reply; 8+ messages in thread
From: Alexandre Courbot @ 2026-11-02 12:09 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Benno Lossin, Gary Guo
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, linux-kernel, rust-for-linux,
Alexandre Courbot
The r000 firmware requires matching the sequence number of sent commands
with the responses received from the GSP. Make `send_command_element`
return the sequence number to that end, and ignore it for now.
No functional change intended.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/gsp/cmdq.rs | 11 +++++++----
drivers/gpu/nova-core/gsp/cmdq/rpc.rs | 4 +++-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 33cb9174c1e6..65f25d3c4d98 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -524,27 +524,30 @@ impl CmdqInner<'_> {
/// Allocate enough space on the command queue to store `command`, initialize it with
/// [`CommandElement::write`], and send the command to the GSP.
///
+ /// Returns the sequence number of the sent element.
+ ///
/// # Errors
///
/// - `EMSGSIZE` if the command exceeds the maximum queue element size.
/// - `ETIMEDOUT` if space does not become available within the timeout.
///
/// Error codes returned by [`CommandElement::write`] are returned as-is.
- fn send_command_element<E: CommandElement>(&mut self, command: E) -> Result {
+ fn send_command_element<E: CommandElement>(&mut self, command: E) -> Result<u32> {
let size_in_bytes = command.size();
+ let seq = self.seq;
let mut dst = self
.gsp_mem
.allocate_command(size_in_bytes, Self::ALLOCATE_TIMEOUT)?;
// Fill the header.
- let msg_element_init = GspMsgElement::init(self.seq, size_in_bytes);
+ let msg_element_init = GspMsgElement::init(seq, size_in_bytes);
// SAFETY: `dst.header` is a valid reference, and is not touched if the initializer fails.
unsafe {
pin_init::raw_try_init(core::ptr::from_mut(dst.header), msg_element_init)?;
}
// Write the message payload.
- command.write(self.dev, self.seq, &mut dst)?;
+ command.write(self.dev, seq, &mut dst)?;
// Compute checksum now that the whole message is ready.
dst.header
@@ -559,7 +562,7 @@ fn send_command_element<E: CommandElement>(&mut self, command: E) -> Result {
self.seq += 1;
self.gsp_mem.advance_cpu_write_ptr(elem_count);
- Ok(())
+ Ok(seq)
}
/// Wait for a message to become available on the message queue.
diff --git a/drivers/gpu/nova-core/gsp/cmdq/rpc.rs b/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
index 2c52ec78871b..36cba7c28d3e 100644
--- a/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
@@ -354,7 +354,9 @@ fn send_rpc_command<M>(&mut self, command: M) -> Result
Error: From<M::InitError>,
{
match SplitState::new(command)? {
- SplitState::Single(command) => self.send_command_element(RpcCommandElement(command)),
+ SplitState::Single(command) => self
+ .send_command_element(RpcCommandElement(command))
+ .map(|_| ()),
SplitState::Split(command, mut continuations) => {
self.send_command_element(RpcCommandElement(command))?;
--
2.56.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 5/5] gpu: nova-core: gsp: cmdq: expose the sequence number of sent commands
2026-11-02 12:09 ` [PATCH v2 5/5] gpu: nova-core: gsp: cmdq: expose the sequence number of sent commands Alexandre Courbot
@ 2026-11-02 13:17 ` Eliot Courtney
0 siblings, 0 replies; 8+ messages in thread
From: Eliot Courtney @ 2026-11-02 13:17 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter, Benno Lossin, Gary Guo
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, linux-kernel, rust-for-linux
On Mon Nov 2, 2026 at 9:09 PM JST, Alexandre Courbot wrote:
> The r000 firmware requires matching the sequence number of sent commands
> with the responses received from the GSP. Make `send_command_element`
> return the sequence number to that end, and ignore it for now.
>
> No functional change intended.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 4/5] gpu: nova-core: gsp: cmdq: compute the queue regions from a count and a slot
2026-11-02 12:09 ` [PATCH v2 4/5] gpu: nova-core: gsp: cmdq: compute the queue regions from a count and a slot Alexandre Courbot
@ 2026-11-02 13:27 ` Eliot Courtney
0 siblings, 0 replies; 8+ messages in thread
From: Eliot Courtney @ 2026-11-02 13:27 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter, Benno Lossin, Gary Guo
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, linux-kernel, rust-for-linux
On Mon Nov 2, 2026 at 9:09 PM JST, Alexandre Courbot wrote:
> From: John Hubbard <jhubbard@nvidia.com>
>
> `driver_write_area` and `driver_read_area` both branch on the relative
> position of the read and write pointers to decide where the two slices
> they return end. This works for msgq v0, where the pointers are indices
> into the ring, but msgq v2 (used by the r000 firmware) keeps them in
> BAR0 registers as free-running counters, so the branches and their
> SAFETY arguments would have to be rewritten for it.
>
> Instead, compute the number of slots available to the driver and the
> slot they start at, and derive the slices from that. The slot counts
> come from one helper per queue, which are the only two places msgq v2
> will need to touch.
>
> Use I/O projections to limit the amount of unsafe code to the bare
> minimum.
>
> No functional change intended.
>
> Assisted-by: LLM
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> drivers/gpu/nova-core/gsp/cmdq.rs | 130 +++++++++++++++++---------------------
> 1 file changed, 58 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 90e26c4dcc28..33cb9174c1e6 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -19,8 +19,8 @@
> Io, //
> },
> new_mutex,
> + num::casts::FromSafeCast,
> prelude::*,
> - ptr,
> sync::{
> barrier::{
> dma_mb,
> @@ -186,60 +186,58 @@ fn new(dev: &'a device::Device<device::Bound>, bar: Bar0<'a>) -> Result<Self> {
> /// As the message queue is a circular buffer, the region may be discontiguous in memory. In
> /// that case the second slice will have a non-zero length.
> fn driver_write_area(&mut self) -> (&mut [[u8; GSP_PAGE_SIZE]], &mut [[u8; GSP_PAGE_SIZE]]) {
> - let tx = self.cpu_write_ptr();
> - let rx = self.gsp_read_ptr();
> + let avail = usize::from_safe_cast(self.cpuq_free_slots());
> + let w_slot = usize::from_safe_cast(self.cpu_write_ptr());
>
> - // Pointer to the first entry of the CPU message queue.
> - let data = ptr::project!(mut self.mem.as_mut_ptr(), .cpuq.msgq.data[build: 0]);
> + // `w_slot` is below `MSGQ_NUM_PAGES` per the invariant of `cpu_write_ptr`, so the
> + // subtraction cannot underflow.
> + let before_wrap = avail.min(cv!(MSGQ_NUM_PAGES => usize) - w_slot);
> + // Per the line above, `before_wrap` is less or equal to `avail` so the subtraction cannot
> + // underflow.
> + let after_wrap = avail - before_wrap;
>
> - let (tail_end, wrap_end) = if rx == 0 {
> - // The write area is non-wrapping, and stops at the second-to-last entry of the command
> - // queue (to leave the last one empty).
> - (MSGQ_NUM_PAGES - 1, 0)
> - } else if rx <= tx {
> - // The write area wraps and continues until `rx - 1`.
> - (MSGQ_NUM_PAGES, rx - 1)
> - } else {
> - // The write area doesn't wrap and stops at `rx - 1`.
> - (rx - 1, 0)
> - };
> + // PANIC: `w_slot + before_wrap` is at most `MSGQ_NUM_PAGES` by construction of
> + // `before_wrap`.
> + let after = io_project!(self.mem, .cpuq.msgq.data[panic: w_slot..w_slot + before_wrap]);
> + // PANIC: `after_wrap` is at most `avail`, which is below `MSGQ_NUM_PAGES`.
> + let before = io_project!(self.mem, .cpuq.msgq.data[panic: ..after_wrap]);
It looks like compared to v1, this kept the `after` and `before` names I
suggested but changed the indices (e.g. in_after => before_wrap). It
means that "before" and "after" are being used in two different ways in
this function ("after wrapping" and "after write pointer"). What about
picking one usage for the entire function? Either is fine imo. Same for
the other spot (read side).
With that changed:
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-11-02 13:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02 12:09 [PATCH v2 0/5] gpu: nova-core: gsp: cmdq: preparatory changes for r000 support Alexandre Courbot
2026-11-02 12:09 ` [PATCH v2 1/5] gpu: nova-core: fix stride of NV_PFSP_* registers Alexandre Courbot
2026-11-02 12:09 ` [PATCH v2 2/5] gpu: nova-core: regs: add msgq v2 BAR0 register declarations Alexandre Courbot
2026-11-02 12:09 ` [PATCH v2 3/5] gpu: nova-core: gsp: cmdq: ring the GSP doorbell from the queue memory Alexandre Courbot
2026-11-02 12:09 ` [PATCH v2 4/5] gpu: nova-core: gsp: cmdq: compute the queue regions from a count and a slot Alexandre Courbot
2026-11-02 13:27 ` Eliot Courtney
2026-11-02 12:09 ` [PATCH v2 5/5] gpu: nova-core: gsp: cmdq: expose the sequence number of sent commands Alexandre Courbot
2026-11-02 13:17 ` Eliot Courtney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®