netdev - Re: [net PATCH v2 7/8] fbnic: Pull fbnic_fw_xmit_cap

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: <ffb6bdbb-64f6-4be2-984e-3c8be185f62c@intel.com>
Date: Tue, 6 May 2025 11:56:59 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Alexander Duyck <alexander.duyck@...il.com>, <netdev@...r.kernel.org>
CC: <davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<horms@...nel.org>
Subject: Re: [net PATCH v2 7/8] fbnic: Pull fbnic_fw_xmit_cap_msg use out of
 interrupt context



On 5/6/2025 9:00 AM, Alexander Duyck wrote:
> From: Alexander Duyck <alexanderduyck@...com>
> 
> This change pulls the call to fbnic_fw_xmit_cap_msg out of
> fbnic_mbx_init_desc_ring and instead places it in the polling function for
> getting the Tx ready. Doing that we can avoid the potential issue with an
> interrupt coming in later from the firmware that causes it to get fired in
> interrupt context.
> 
> Fixes: 20d2e88cc746 ("eth: fbnic: Add initial messaging to notify FW of our presence")
> Signed-off-by: Alexander Duyck <alexanderduyck@...com>


Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>

> @@ -393,15 +375,6 @@ static void fbnic_mbx_init_desc_ring(struct fbnic_dev *fbd, int mbx_idx)
>  		/* Enable DMA reads from the device */
>  		wr32(fbd, FBNIC_PUL_OB_TLP_HDR_AR_CFG,
>  		     FBNIC_PUL_OB_TLP_HDR_AR_CFG_BME);
> -
> -		/* Force version to 1 if we successfully requested an update
> -		 * from the firmware. This should be overwritten once we get
> -		 * the actual version from the firmware in the capabilities
> -		 * request message.
> -		 */
> -		if (!fbnic_fw_xmit_cap_msg(fbd) &&
> -		    !fbd->fw_cap.running.mgmt.version)
> -			fbd->fw_cap.running.mgmt.version = 1;

...

>  
> +	/* Request an update from the firmware. This should overwrite
> +	 * mgmt.version once we get the actual version from the firmware
> +	 * in the capabilities request message.
> +	 */
> +	err = fbnic_fw_xmit_simple_msg(fbd, FBNIC_TLV_MSG_ID_HOST_CAP_REQ);
> +	if (err)
> +		goto clean_mbx;
> +
> +	/* Use "1" to indicate we entered the state waiting for a response */
> +	fbd->fw_cap.running.mgmt.version = 1;
> +

Curious about the comment rewording here. I guess the extra information
about forcing and the value being updated to the actual version later
isn't as relevant in the new location?