linux-kernel - Re: [DONOTAPPLY RFC PATCH v2 3/4] DONOTMERGE: net: mwifiex: fix timeouts with the SD8777 chip

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: <aTCvq7aN_WMts6hm@google.com>
Date: Wed, 3 Dec 2025 13:46:19 -0800
From: Brian Norris <briannorris@...omium.org>
To: Karel Balej <balejk@...fyz.cz>
Cc: Johannes Berg <johannes@...solutions.net>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Duje Mihanović <duje@...emihanovic.xyz>,
	Andrew Lunn <andrew@...n.ch>,
	Gregory Clement <gregory.clement@...tlin.com>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	Francesco Dolcini <francesco@...cini.it>,
	Ulf Hansson <ulf.hansson@...aro.org>, Frank Li <Frank.Li@....com>,
	linux-wireless@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-mmc@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht,
	phone-devel@...r.kernel.org, Jeff Chen <jeff.chen_1@....com>,
	Peng Fan <peng.fan@....com>
Subject: Re: [DONOTAPPLY RFC PATCH v2 3/4] DONOTMERGE: net: mwifiex: fix
 timeouts with the SD8777 chip

Hi,

On Sun, Oct 26, 2025 at 07:20:40PM +0100, Karel Balej wrote:
> 	[ 2101.211178] mwifiex_sdio mmc2:0001:1: info: MWIFIEX VERSION: mwifiex 1.0 (14.75.33.p119)
... 
> Afterwards, a bisect was
> performed which first indicated the commit 808bbebcc8fc ("mwifiex: add
> Tx status support for EAPOL packets") introduced in the v3.18-v3.19
> cycle.
> 
> Reverting this commit (and the following one, commit 18ca43823f3c
> ("mwifiex: add Tx status support for ACTION frames"), to facilitate a
> clean revert) fixed the timeouts for v3.19, but during the next cycle,
> v3.19-v4.0, another breakage was introduced via commit 84b313b35f81
> ("mwifiex: make tx packet 64 byte DMA aligned").
> 
> Reverting all three commits fixed the timeouts on the current mainline
> kernel also. This patch contains the minimal changes needed to achieve
> that derived from the full revert commits.
...

(Trimmed the commit message down to the breaking commits, and the
version info)

>From the looks of it, you're dealing with incompatible changes made in
the Marvell firmware API. It seems that you have a "version 14"
firmware, and the timeline of these mwifiex changes (~2014) is approx
when linux-firmware started seeing v15 and v16 firmware. It *might* be
OK to try add some versioning to these structs and padding changes, and
make a choice based on adapter->fw_release_number or
adapter->fw_cap_info. It might be ugly and error-prone, but possible...

Or if the FW versioning doesn't work out, it's possible we could
specifically flag these quirks for SD8777 somehow.

> Signed-off-by: Karel Balej <balejk@...fyz.cz>
> ---
>  drivers/net/wireless/marvell/mwifiex/fw.h     |  4 +---
>  drivers/net/wireless/marvell/mwifiex/sta_tx.c | 10 ++--------
>  2 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
> index e9e896606912..5c4c3363c7de 100644
> --- a/drivers/net/wireless/marvell/mwifiex/fw.h
> +++ b/drivers/net/wireless/marvell/mwifiex/fw.h
> @@ -690,9 +690,7 @@ struct txpd {
>  	u8 priority;
>  	u8 flags;
>  	u8 pkt_delay_2ms;
> -	u8 reserved1[2];
> -	u8 tx_token_id;
> -	u8 reserved[2];
> +	u8 reserved1;

I'm inferring that 'sizeof(struct txpd)' (also spelled
'sizeof(*local_tx_pd)' below) is relevant, and that this struct probably
should retain the smaller size for FW version 14.

Maybe you need a new 'struct txpd_v14' layout, and embed that inside
'struct txpd'.

>  } __packed;
>  
>  struct rxpd {
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_tx.c b/drivers/net/wireless/marvell/mwifiex/sta_tx.c
> index 9d0ef04ebe02..857eb22f4c24 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_tx.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_tx.c
> @@ -41,8 +41,8 @@ void mwifiex_process_sta_txpd(struct mwifiex_private *priv,
>  
>  	pkt_type = mwifiex_is_skb_mgmt_frame(skb) ? PKT_TYPE_MGMT : 0;
>  
> -	pad = ((uintptr_t)skb->data - (sizeof(*local_tx_pd) + hroom)) &
> -	       (MWIFIEX_DMA_ALIGN_SZ - 1);
> +	pad = (4 - (((void *)skb->data - NULL) & 0x3)) % 4;

It's not clear to me whether your v14 FW doesn't like the 64-byte
alignment, or if it didn't like the new txpd header size/layout, or
both. But obviously this line won't fly, with magic numbers and all. It
will need to be expressed in terms of macros (MWIFIEX_DMA_ALIGN_SZ, or a
"V14" version of that; and sizeof(...)).

> +
>  	skb_push(skb, sizeof(*local_tx_pd) + pad);
>  
>  	local_tx_pd = (struct txpd *) skb->data;
> @@ -58,12 +58,6 @@ void mwifiex_process_sta_txpd(struct mwifiex_private *priv,
>  	local_tx_pd->pkt_delay_2ms =
>  				mwifiex_wmm_compute_drv_pkt_delay(priv, skb);
>  
> -	if (tx_info->flags & MWIFIEX_BUF_FLAG_EAPOL_TX_STATUS ||
> -	    tx_info->flags & MWIFIEX_BUF_FLAG_ACTION_TX_STATUS) {
> -		local_tx_pd->tx_token_id = tx_info->ack_frame_id;
> -		local_tx_pd->flags |= MWIFIEX_TXPD_FLAGS_REQ_TX_STATUS;
> -	}

Rather than dropping this block, would it work to also check:

	adapter->fw_api_ver >= MWIFIEX_FW_V15

?

Brian

> -
>  	if (local_tx_pd->priority <
>  	    ARRAY_SIZE(priv->wmm.user_pri_pkt_tx_ctrl))
>  		/*
> -- 
> 2.51.1
>