Received-SPF: pass (google.com: domain of linux-kernel+bounces-176579-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Sat, 11 May 2024 15:32:24 +0100
From: Simon Horman <horms@kernel.org>
To: Ziwei Xiao <ziweixiao@google.com>
Cc: netdev@vger.kernel.org, jeroendb@google.com, pkaligineedi@google.com,
	shailend@google.com, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, willemb@google.com,
	hramamurthy@google.com, rushilg@google.com, jfraker@google.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 2/5] gve: Add adminq extended command
Message-ID: <20240511143224.GI2347895@kernel.org>
References: <20240507225945.1408516-1-ziweixiao@google.com>
 <20240507225945.1408516-3-ziweixiao@google.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240507225945.1408516-3-ziweixiao@google.com>

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.

..