Re: [PATCH] wifi: ath11k: fix qmi memory allocation logic for CALDB region

Baochen Qiang <[email protected]>
Newsgroups org.infradead.lists.ath11k,org.kernel.vger.linux-kernel,org.kernel.vger.linux-wireless,org.kernel.vger.stable
Message-ID <[email protected]>

On 12/7/2025 1:58 AM, Alexandru Gagniuc wrote:
> Memory region assignment in ath11k_qmi_assign_target_mem_chunk()
> assumes that:
>   1. firmware will make a HOST_DDR_REGION_TYPE request, and
>   2. this request is processed before CALDB_MEM_REGION_TYPE
> 
> In this case CALDB_MEM_REGION_TYPE, can safely be assigned immediately
> after the host region.
> 
> However, if the HOST_DDR_REGION_TYPE request is not made, or the
> reserved-memory node is not present, then res.start and res.end are 0,
> and host_ddr_sz remains uninitialized. The physical address should
> fall back to ATH11K_QMI_CALDB_ADDRESS. That doesn't happen:
> 
> resource_size(&res) returns 1 for an empty resource, and thus the if
> clause never takes the fallback path. ab->qmi.target_mem[idx].paddr
> is assigned the uninitialized value of host_ddr_sz + 0 (res.start).
> 
> Use "if (res.end > res.start)" for the predicate, which correctly
> falls back to ATH11K_QMI_CALDB_ADDRESS.

In addition, does it make sense to do of_reserved_mem_region_to_resource() before the
loop, which may give CALDB_MEM_REGION_TYPE a chance even HOST_DDR_REGION_TYPE request is
not made?

> 
> Fixes: 900730dc4705 ("wifi: ath: Use of_reserved_mem_region_to_resource() for "memory-region"")
> 
> Cc: [email protected] # v6.18
> Signed-off-by: Alexandru Gagniuc <[email protected]>
> ---
>  drivers/net/wireless/ath/ath11k/qmi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
> index aea56c38bf8f3..6cc26d1c1e2a4 100644
> --- a/drivers/net/wireless/ath/ath11k/qmi.c
> +++ b/drivers/net/wireless/ath/ath11k/qmi.c
> @@ -2054,7 +2054,7 @@ static int ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab)
>  				return ret;
>  			}
>  
> -			if (res.end - res.start + 1 < ab->qmi.target_mem[i].size) {
> +			if (resource_size(&res) < ab->qmi.target_mem[i].size) {
>  				ath11k_dbg(ab, ATH11K_DBG_QMI,
>  					   "fail to assign memory of sz\n");
>  				return -EINVAL;
> @@ -2086,7 +2086,7 @@ static int ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab)
>  			}
>  
>  			if (ath11k_core_coldboot_cal_support(ab)) {
> -				if (resource_size(&res)) {
> +				if (res.end > res.start) {
>  					ab->qmi.target_mem[idx].paddr =
>  							res.start + host_ddr_sz;
>  					ab->qmi.target_mem[idx].iaddr =
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.