[PATCH v4 7/9] gpu: nova-core: gsp: add command_size helper

Eliot Courtney posted 9 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v4 7/9] gpu: nova-core: gsp: add command_size helper
Posted by Eliot Courtney 1 month, 1 week ago
Add a helper function which computes the size of a command.

Tested-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 9f74f0898d90..4a663a5b3437 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -450,6 +450,15 @@ struct GspMessage<'a> {
     contents: (&'a [u8], &'a [u8]),
 }
 
+/// Computes the total size of the command (including its variable-length payload) without the
+/// [`GspMsgElement`] header.
+fn command_size<M>(command: &M) -> usize
+where
+    M: CommandToGsp,
+{
+    size_of::<M::Command>() + command.variable_payload_len()
+}
+
 /// GSP command queue.
 ///
 /// Provides the ability to send commands and receive messages from the GSP using a shared memory
@@ -526,7 +535,7 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
         Error: From<M::InitError>,
     {
-        let command_size = size_of::<M::Command>() + command.variable_payload_len();
+        let command_size = command_size(&command);
         let dst = self
             .gsp_mem
             .allocate_command(command_size, Delta::from_secs(1))?;

-- 
2.53.0
Re: [PATCH v4 7/9] gpu: nova-core: gsp: add command_size helper
Posted by Gary Guo 1 month, 1 week ago
On Mon Mar 2, 2026 at 11:42 AM GMT, Eliot Courtney wrote:
> Add a helper function which computes the size of a command.
>
> Tested-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 9f74f0898d90..4a663a5b3437 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -450,6 +450,15 @@ struct GspMessage<'a> {
>      contents: (&'a [u8], &'a [u8]),
>  }
>  
> +/// Computes the total size of the command (including its variable-length payload) without the
> +/// [`GspMsgElement`] header.
> +fn command_size<M>(command: &M) -> usize
> +where
> +    M: CommandToGsp,
> +{
> +    size_of::<M::Command>() + command.variable_payload_len()
> +}
> +

This could just a provided method on `CommandToGsp`?

Best,
Gary

>  /// GSP command queue.
>  ///
>  /// Provides the ability to send commands and receive messages from the GSP using a shared memory
> @@ -526,7 +535,7 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
>          // This allows all error types, including `Infallible`, to be used for `M::InitError`.
>          Error: From<M::InitError>,
>      {
> -        let command_size = size_of::<M::Command>() + command.variable_payload_len();
> +        let command_size = command_size(&command);
>          let dst = self
>              .gsp_mem
>              .allocate_command(command_size, Delta::from_secs(1))?;
Re: [PATCH v4 7/9] gpu: nova-core: gsp: add command_size helper
Posted by Eliot Courtney 1 month, 1 week ago
On Mon Mar 2, 2026 at 11:22 PM JST, Gary Guo wrote:
> On Mon Mar 2, 2026 at 11:42 AM GMT, Eliot Courtney wrote:
>> Add a helper function which computes the size of a command.
>>
>> Tested-by: Zhi Wang <zhiw@nvidia.com>
>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
>>  drivers/gpu/nova-core/gsp/cmdq.rs | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
>> index 9f74f0898d90..4a663a5b3437 100644
>> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
>> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
>> @@ -450,6 +450,15 @@ struct GspMessage<'a> {
>>      contents: (&'a [u8], &'a [u8]),
>>  }
>>  
>> +/// Computes the total size of the command (including its variable-length payload) without the
>> +/// [`GspMsgElement`] header.
>> +fn command_size<M>(command: &M) -> usize
>> +where
>> +    M: CommandToGsp,
>> +{
>> +    size_of::<M::Command>() + command.variable_payload_len()
>> +}
>> +
>
> This could just a provided method on `CommandToGsp`?

I discussed this with Alex before[1] and my idea was that it's odd to
add it as a default trait method since implementors could override it
when they really shouldn't be able to, since it needs to agree with the
size of the command and the variable payload length.

WDYT? Is there a clear convention of doing this over the kernel? It just
feels odd to me although I agree it's useful. Would be perfect if there
was an easy way to prevent it from being overridden.

[1]: https://lore.kernel.org/all/DGHSGO2E0U9F.2M8MOSKBNA9JY@nvidia.com