Re: [PATCH 2/2] gpu: nova-core: gsp: cmdq: increase MSGQ_NUM_PAGES to 64

["Eliot Courtney" <ecourtney@nvidia.com>]

On Wed Nov 11, 2026 at 11:49 PM JST, Alexandre Courbot wrote:
> 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?

Yerp I tested boot on turing, ampere, ada, blackwell.

>
>>
>> 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()`?

I actually had this originally but I think that this form expresses the
condition more directly. The condition/issue is what happens when you
wrap the counter - it should go to the next slot (which necessarily
needs to be zero since a wrapped counter is 0). So you want
MSGQ_NUM_PAGES to divide 2^32. So if you write
MSGQ_NUM_PAGES.is_power_of_two() you are implicitly relying on all
divisors being a power of two, which is an extra logical step.

But in the end I don't really mind, so lmk wdyt.