Re: [PATCH net-next 2/5] gve: Add adminq extended command

[Simon Horman <horms@kernel.org>]

On Tue, May 07, 2024 at 10:59:42PM +0000, Ziwei Xiao wrote:
> From: Jeroen de Borst <jeroendb@google.com>
> 
> The adminq command is limited to 64 bytes per entry and it's 56 bytes
> for the command itself at maximum. To support larger commands, we need
> to dma_alloc a separate memory to put the command in that memory and
> send the dma memory address instead of the actual command.
> 
> This change introduces an extended adminq command to wrap the real
> command with the inner opcode and the allocated dma memory address
> specified. Once the device receives it, it can get the real command from
> the given dma memory address. As designed with the device, all the
> extended commands will use inner opcode larger than 0xFF.
> 
> Signed-off-by: Jeroen de Borst <jeroendb@google.com>
> Co-developed-by: Ziwei Xiao <ziweixiao@google.com>
> Signed-off-by: Ziwei Xiao <ziweixiao@google.com>
> Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
> Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
> Reviewed-by: Willem de Bruijn <willemb@google.com>
> ---
>  drivers/net/ethernet/google/gve/gve_adminq.c | 31 ++++++++++++++++++++
>  drivers/net/ethernet/google/gve/gve_adminq.h | 12 ++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> index 2c3ec5c3b114..514641b3ccc7 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> @@ -461,6 +461,8 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
>  
>  	memcpy(cmd, cmd_orig, sizeof(*cmd_orig));
>  	opcode = be32_to_cpu(READ_ONCE(cmd->opcode));
> +	if (opcode == GVE_ADMINQ_EXTENDED_COMMAND)
> +		opcode = be32_to_cpu(cmd->extended_command.inner_opcode);
>  
>  	switch (opcode) {
>  	case GVE_ADMINQ_DESCRIBE_DEVICE:
> @@ -537,6 +539,35 @@ static int gve_adminq_execute_cmd(struct gve_priv *priv,
>  	return err;
>  }
>  
> +static int gve_adminq_execute_extended_cmd(struct gve_priv *priv, u32 opcode,
> +					   size_t cmd_size, void *cmd_orig)

Hi Ziewi Xiaoi and Jeroen,

As of this patch, gve_adminq_execute_extended_cmd is defined but unused.
Which causes an error when compiling with W=1 using gcc-13 or clang-18.

Perhaps it would be better to squash this patch into the patch that
uses gve_adminq_execute_extended_cmd.

...