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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240511143224.GI2347895@kernel.org>
Date: Sat, 11 May 2024 15:32:24 +0100
From: Simon Horman <horms@...nel.org>
To: Ziwei Xiao <ziweixiao@...gle.com>
Cc: netdev@...r.kernel.org, jeroendb@...gle.com, pkaligineedi@...gle.com,
	shailend@...gle.com, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, willemb@...gle.com,
	hramamurthy@...gle.com, rushilg@...gle.com, jfraker@...gle.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 2/5] gve: Add adminq extended command

On Tue, May 07, 2024 at 10:59:42PM +0000, Ziwei Xiao wrote:
> From: Jeroen de Borst <jeroendb@...gle.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@...gle.com>
> Co-developed-by: Ziwei Xiao <ziweixiao@...gle.com>
> Signed-off-by: Ziwei Xiao <ziweixiao@...gle.com>
> Reviewed-by: Praveen Kaligineedi <pkaligineedi@...gle.com>
> Reviewed-by: Harshitha Ramamurthy <hramamurthy@...gle.com>
> Reviewed-by: Willem de Bruijn <willemb@...gle.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.

..