netdev - Re: [PATCH net 2/2] net: libwx: Fix firmware interaction failure

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] [day] [month] [year] [list]

Message-ID: <20250512105712.GW3339421@horms.kernel.org>
Date: Mon, 12 May 2025 11:57:12 +0100
From: Simon Horman <horms@...nel.org>
To: Jiawen Wu <jiawenwu@...stnetic.com>
Cc: netdev@...r.kernel.org, andrew+netdev@...n.ch, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	mengyuanlou@...-swift.com
Subject: Re: [PATCH net 2/2] net: libwx: Fix firmware interaction failure

On Fri, May 09, 2025 at 06:00:03PM +0800, Jiawen Wu wrote:
> There are two issues that need to be fixed on the new SW-FW interaction.
> 
> The timeout waiting for the firmware to return is too short. So that
> some mailbox commands cannot be completed. Use the 'timeout' parameter
> instead of fixed timeout value for flexible configuration.
> 
> Missing the error return if there is an unknown command. It causes the
> driver to mistakenly believe that the interaction is complete. This
> problem occurs when new driver is paired with old firmware, which does
> not support the new mailbox commands.

Hi Jiawen Wu,

Please split this patch so that each issue is fixed in a separate patch:
the rule of thumb is one patch per fix.

> 
> Fixes: 2e5af6b2ae85 ("net: txgbe: Add basic support for new AML devices")
> Signed-off-by: Jiawen Wu <jiawenwu@...stnetic.com>
> ---
>  drivers/net/ethernet/wangxun/libwx/wx_hw.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> index 3c3aa5f4ebbf..de06467898de 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> @@ -435,7 +435,7 @@ static int wx_host_interface_command_r(struct wx *wx, u32 *buffer,
>  	wr32m(wx, WX_SW2FW_MBOX_CMD, WX_SW2FW_MBOX_CMD_VLD, WX_SW2FW_MBOX_CMD_VLD);
>  
>  	/* polling reply from FW */
> -	err = read_poll_timeout(wx_poll_fw_reply, reply, reply, 1000, 50000,
> +	err = read_poll_timeout(wx_poll_fw_reply, reply, reply, 2000, timeout * 1000,
>  				true, wx, buffer, send_cmd);

nit: Please line-wrap so that lines remain no wider than 80 columns.

>  	if (err) {
>  		wx_err(wx, "Polling from FW messages timeout, cmd: 0x%x, index: %d\n",
> @@ -443,6 +443,12 @@ static int wx_host_interface_command_r(struct wx *wx, u32 *buffer,
>  		goto rel_out;
>  	}
>  
> +	if (hdr->cmd_or_resp.ret_status == 0x80) {
> +		wx_err(wx, "Unknown FW command: 0x%x\n", send_cmd);
> +		err = -EINVAL;
> +		goto rel_out;
> +	}
> +
>  	/* expect no reply from FW then return */
>  	if (!return_data)
>  		goto rel_out;

-- 
pw-bot: changes-requested