* [PATCH 0/2] gpu: nova-core: gsp: fix misc cmdq issues
@ 2026-11-11 1:37 Eliot Courtney
2026-11-11 1:37 ` [PATCH 1/2] gpu: nova-core: gsp: cmdq: wrap the sequence number on overflow Eliot Courtney
2026-11-11 1:37 ` [PATCH 2/2] gpu: nova-core: gsp: cmdq: increase MSGQ_NUM_PAGES to 64 Eliot Courtney
0 siblings, 2 replies; 5+ messages in thread
From: Eliot Courtney @ 2026-11-11 1:37 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Alexandre Courbot
Cc: John Hubbard, Alistair Popple, Timur Tabi, nova-gpu, dri-devel,
linux-kernel, Eliot Courtney
Fix two cmdq issues.
This is based on drm-rust-next.
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
Eliot Courtney (2):
gpu: nova-core: gsp: cmdq: wrap the sequence number on overflow
gpu: nova-core: gsp: cmdq: increase MSGQ_NUM_PAGES to 64
drivers/gpu/nova-core/gsp/cmdq.rs | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
---
base-commit: 7ecd9ee29f7bfbe9a293952fbc83c3f8bd7205dd
change-id: 20261109-msgq-64-pages-bab32c03d627
Best regards,
--
Eliot Courtney <ecourtney@nvidia.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/2] gpu: nova-core: gsp: cmdq: wrap the sequence number on overflow 2026-11-11 1:37 [PATCH 0/2] gpu: nova-core: gsp: fix misc cmdq issues Eliot Courtney @ 2026-11-11 1:37 ` Eliot Courtney 2026-11-11 14:43 ` Alexandre Courbot 2026-11-11 1:37 ` [PATCH 2/2] gpu: nova-core: gsp: cmdq: increase MSGQ_NUM_PAGES to 64 Eliot Courtney 1 sibling, 1 reply; 5+ messages in thread From: Eliot Courtney @ 2026-11-11 1:37 UTC (permalink / raw) To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter, Alexandre Courbot Cc: John Hubbard, Alistair Popple, Timur Tabi, nova-gpu, dri-devel, linux-kernel, Eliot Courtney The sequence number can (eventually) overflow, in which case we should just wrap it around. Fixes: 75f6b1de8133 ("gpu: nova-core: gsp: Add GSP command queue bindings and handling") Signed-off-by: Eliot Courtney <ecourtney@nvidia.com> --- drivers/gpu/nova-core/gsp/cmdq.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs index 5f3de456042e..33074bb53e8c 100644 --- a/drivers/gpu/nova-core/gsp/cmdq.rs +++ b/drivers/gpu/nova-core/gsp/cmdq.rs @@ -578,7 +578,7 @@ fn send_command_element<E: CommandElement>(&mut self, command: E) -> Result<u32> // All set - update the write pointer and inform the GSP of the new command. let elem_count = dst.header.element_count(); - self.seq += 1; + self.seq = self.seq.wrapping_add(1); self.gsp_mem.advance_cpu_write_ptr(elem_count); Ok(seq) -- 2.56.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] gpu: nova-core: gsp: cmdq: wrap the sequence number on overflow 2026-11-11 1:37 ` [PATCH 1/2] gpu: nova-core: gsp: cmdq: wrap the sequence number on overflow Eliot Courtney @ 2026-11-11 14:43 ` Alexandre Courbot 0 siblings, 0 replies; 5+ messages in thread From: Alexandre Courbot @ 2026-11-11 14:43 UTC (permalink / raw) To: Eliot Courtney Cc: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter, John Hubbard, Alistair Popple, Timur Tabi, nova-gpu, dri-devel, linux-kernel On Wed Nov 11, 2026 at 10:37 AM JST, Eliot Courtney wrote: > The sequence number can (eventually) overflow, in which case we should > just wrap it around. > > Fixes: 75f6b1de8133 ("gpu: nova-core: gsp: Add GSP command queue bindings and handling") > Signed-off-by: Eliot Courtney <ecourtney@nvidia.com> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] gpu: nova-core: gsp: cmdq: increase MSGQ_NUM_PAGES to 64 2026-11-11 1:37 [PATCH 0/2] gpu: nova-core: gsp: fix misc cmdq issues Eliot Courtney 2026-11-11 1:37 ` [PATCH 1/2] gpu: nova-core: gsp: cmdq: wrap the sequence number on overflow Eliot Courtney @ 2026-11-11 1:37 ` Eliot Courtney 2026-11-11 14:49 ` Alexandre Courbot 1 sibling, 1 reply; 5+ messages in thread From: Eliot Courtney @ 2026-11-11 1:37 UTC (permalink / raw) To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter, Alexandre Courbot Cc: John Hubbard, Alistair Popple, Timur Tabi, nova-gpu, dri-devel, linux-kernel, Eliot Courtney In the msgq v2 protocol, which the r000 firmware uses, the queue pointers are kept in u32 registers and increase monotonically until they wrap at 2^32. `MSGQ_NUM_PAGES` is currently 63 and does not divide 2^32. This means that on msgq v2, the slot can jump backwards when the counters wrap. For example: (2^32 - 2) % 63 == 2 (2^32 - 1) % 63 == 3 0 % 63 == 0 (slot 0 instead of slot 4) The writer can then overwrite slots before the reader has read them. Increase `MSGQ_NUM_PAGES` to 64 and check at compile time that `MSGQ_NUM_PAGES` divides 2^32. Signed-off-by: Eliot Courtney <ecourtney@nvidia.com> --- drivers/gpu/nova-core/gsp/cmdq.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs index 33074bb53e8c..240ab42db557 100644 --- a/drivers/gpu/nova-core/gsp/cmdq.rs +++ b/drivers/gpu/nova-core/gsp/cmdq.rs @@ -84,7 +84,15 @@ trait CommandElement { } /// Number of GSP pages making the [`Msgq`]. -pub(crate) const MSGQ_NUM_PAGES: u32 = 0x3f; +pub(crate) const MSGQ_NUM_PAGES: u32 = 64; + +// From version 2 of the msgq protocol, the queue pointers increase +// monotonically in `u32` counters until they wrap. If `MSGQ_NUM_PAGES` does not +// divide `1 << 32`, then the slot, `counter % MSGQ_NUM_PAGES`, can jump +// backwards when wrapping, which could overwrite slots that still need to be +// read. So, require that the last counter value, `u32::MAX`, maps to the last +// slot. +static_assert!(u32::MAX % MSGQ_NUM_PAGES == MSGQ_NUM_PAGES - 1); /// Circular buffer of a [`Msgq`]. /// -- 2.56.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] gpu: nova-core: gsp: cmdq: increase MSGQ_NUM_PAGES to 64 2026-11-11 1:37 ` [PATCH 2/2] gpu: nova-core: gsp: cmdq: increase MSGQ_NUM_PAGES to 64 Eliot Courtney @ 2026-11-11 14:49 ` Alexandre Courbot 0 siblings, 0 replies; 5+ messages in thread From: Alexandre Courbot @ 2026-11-11 14:49 UTC (permalink / raw) To: Eliot Courtney Cc: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter, John Hubbard, Alistair Popple, Timur Tabi, nova-gpu, dri-devel, linux-kernel On Wed Nov 11, 2026 at 10:37 AM JST, Eliot Courtney wrote: > In the msgq v2 protocol, which the r000 firmware uses, the queue > pointers are kept in u32 registers and increase monotonically until they > wrap at 2^32. > > `MSGQ_NUM_PAGES` is currently 63 and does not divide 2^32. This means > that on msgq v2, the slot can jump backwards when the counters wrap. > For example: > > (2^32 - 2) % 63 == 2 > (2^32 - 1) % 63 == 3 > 0 % 63 == 0 (slot 0 instead of slot 4) > > The writer can then overwrite slots before the reader has read them. > > Increase `MSGQ_NUM_PAGES` to 64 and check at compile time that > `MSGQ_NUM_PAGES` divides 2^32. Have we tested that this was also working on msgq v0? > > Signed-off-by: Eliot Courtney <ecourtney@nvidia.com> > --- > drivers/gpu/nova-core/gsp/cmdq.rs | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs > index 33074bb53e8c..240ab42db557 100644 > --- a/drivers/gpu/nova-core/gsp/cmdq.rs > +++ b/drivers/gpu/nova-core/gsp/cmdq.rs > @@ -84,7 +84,15 @@ trait CommandElement { > } > > /// Number of GSP pages making the [`Msgq`]. > -pub(crate) const MSGQ_NUM_PAGES: u32 = 0x3f; > +pub(crate) const MSGQ_NUM_PAGES: u32 = 64; > + > +// From version 2 of the msgq protocol, the queue pointers increase > +// monotonically in `u32` counters until they wrap. If `MSGQ_NUM_PAGES` does not > +// divide `1 << 32`, then the slot, `counter % MSGQ_NUM_PAGES`, can jump > +// backwards when wrapping, which could overwrite slots that still need to be > +// read. So, require that the last counter value, `u32::MAX`, maps to the last > +// slot. That's a pretty convoluted way to say the number of slots must be a power of 2 representable as a u32. :) > +static_assert!(u32::MAX % MSGQ_NUM_PAGES == MSGQ_NUM_PAGES - 1); Same here, can't we use `MSGQ_NUM_PAGES.is_power_of_two()`? ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-11-11 14:50 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-11-11 1:37 [PATCH 0/2] gpu: nova-core: gsp: fix misc cmdq issues Eliot Courtney 2026-11-11 1:37 ` [PATCH 1/2] gpu: nova-core: gsp: cmdq: wrap the sequence number on overflow Eliot Courtney 2026-11-11 14:43 ` Alexandre Courbot 2026-11-11 1:37 ` [PATCH 2/2] gpu: nova-core: gsp: cmdq: increase MSGQ_NUM_PAGES to 64 Eliot Courtney 2026-11-11 14:49 ` Alexandre Courbot
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®