mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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

* [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 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

* 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®