linux-kernel - Re: [PATCH v3] wifi: ath11k: Fix MHI target memory reuse logic

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 for Android: free password hash cracker in your pocket

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

Message-ID: <5f51e283-f29a-4b3f-ad36-d6d851baeb7f@collabora.com>
Date: Fri, 25 Apr 2025 16:02:15 +0500
From: Muhammad Usama Anjum <usama.anjum@...labora.com>
To: Baochen Qiang <quic_bqiang@...cinc.com>, jeff.johnson@....qualcomm.com,
 Jeff Johnson <jjohnson@...nel.org>
Cc: usama.anjum@...labora.com, kernel@...labora.com,
 linux-wireless@...r.kernel.org, ath11k@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] wifi: ath11k: Fix MHI target memory reuse logic

On 4/25/25 3:36 PM, Baochen Qiang wrote:
> 
> 
> On 4/25/2025 1:51 PM, Muhammad Usama Anjum wrote:
>> Firmware requests 2 segments at first. The first segment is of 6799360
>> whose allocation fails due to dma remapping not available. The success
>> is returned to firmware. Then firmware asks for 22 smaller segments
>> instead of 2 big ones. Those get allocated successfully. At suspend/
>> hibernation time, these segments aren't freed as they will be reused
>> by firmware after resuming.
>>
>> After resuming, the firmware asks for the 2 segments again with the
>> first segment of 6799360 size. Since chunk->vaddr is not NULL, the
>> type and size are compared with the previous type and size to know if
>> it can be reused or not. Unfortunately, it is detected that it cannot
>> be reused and this first smaller segment is freed. Then we continue to
>> allocate 6799360 size memory which fails and ath11k_qmi_free_target_mem_chunk()
>> is called which frees the second smaller segment as well. Later success
>> is returned to firmware which asks for 22 smaller segments again. But
>> as we had freed 2 segments already, we'll allocate the first 2 new
>> smaller segments again and reuse the remaining 20. Hence 20 small
>> segments are being reused instead of 22.
>>
>> Add skip logic when vaddr is set, but size/type don't match. Use the
>> same skip and success logic as used when dma_alloc_coherent() fails
> 
> 
> till here it is good.
> 
> but from below
> 
>> without freeing the memory area. The following error are being fixed at
>> resume:
>>
>> 	kernel: ath11k_pci 0000:03:00.0: failed to allocate dma memory for qmi (524288 B type 1)
>> 	ath11k_pci 0000:03:00.0: failed to allocate qmi target memory: -22
>>
>> Those failures aren't because of the bigger chunk allocation failure as
>> they are skipped. Rather these failures are because of smaller chunk
>> allocation failures. This patch fixes freeing and allocation of 2 smaller
>> chunks.
> 
> to here not good to me, as it gives me the impression that you are fixing a kernel
> allocation failure.
> 
> How about rephrase like:
> 
> By skipping, the possibility of resume failure due to kernel failing to allocate memory
> for QMI can be avoided.
I don't know why do you feel like that even in v3.

Anyways, I'll send v4 and hope that you'll ack that one.

> 
>>
>> Tested-on: WCN6855 WLAN.HSP.1.1-03926.13-QCAHSPSWPL_V2_SILICONZ_CE-2.52297.6
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
>> ---
>> Changes since v1:
>> - Update description
>>
>> Changes since v2:
>> - Update description
>> ---
>>  drivers/net/wireless/ath/ath11k/qmi.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
>> index 47b9d4126d3a9..2782f4723e413 100644
>> --- a/drivers/net/wireless/ath/ath11k/qmi.c
>> +++ b/drivers/net/wireless/ath/ath11k/qmi.c
>> @@ -1993,6 +1993,15 @@ static int ath11k_qmi_alloc_target_mem_chunk(struct ath11k_base *ab)
>>  			    chunk->prev_size == chunk->size)
>>  				continue;
>>  
>> +			if (ab->qmi.mem_seg_count <= ATH11K_QMI_FW_MEM_REQ_SEGMENT_CNT) {
>> +				ath11k_dbg(ab, ATH11K_DBG_QMI,
>> +					   "size/type mismatch (current %d %u) (prev %d %u), try later with small size\n",
>> +					    chunk->size, chunk->type,
>> +					    chunk->prev_size, chunk->prev_type);
>> +				ab->qmi.target_mem_delayed = true;
>> +				return 0;
>> +			}
>> +
>>  			/* cannot reuse the existing chunk */
>>  			dma_free_coherent(ab->dev, chunk->prev_size,
>>  					  chunk->vaddr, chunk->paddr);
> 


-- 
Regards,
Usama