Re: [PATCH ath-next 09/17] wifi: ath12k: add support for unified data-region memory allocation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] wifi: ath12k: qmi target mem data-region misallocation
--

commit 889e203c71e56c91154908ff956ce486dc098cf4
Author: Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>

wifi: ath12k: add support for unified data-region memory allocation

Add support for devices such as QCN6432 that provide the M3 and
calibration memory through a unified data-region reserved-memory node.
If the data-region is found, allocate each memory chunk from it sequentially.

> diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
> index 817bd820623d2..cc488cfe2b2fb 100644
> --- a/drivers/net/wireless/ath/ath12k/qmi.c
> +++ b/drivers/net/wireless/ath/ath12k/qmi.c

[ ... ]

> @@ -2851,6 +2871,48 @@ static int ath12k_qmi_assign_target_mem_chunk(struct ath12k_base *ab)
>  	const char *rname;
>  	int i, idx, ret;
>  
> +	ret = of_reserved_mem_region_to_resource_byname(np, "data-region", &res);
> +	if (ret && ret != -ENODATA)
> +		return ret;
> +
> +	if (!ret) {
> +		avail_rmem_size = resource_size(&res);
> +		for (i = 0, idx = 0; i < ab->qmi.mem_seg_count; i++) {
> +			chunk = &ab->qmi.target_mem[i];
> +
> +			if (chunk->type == CALDB_MEM_REGION_TYPE) {
> +				ret = ath12k_qmi_map_target_mem(ab, idx, 0, chunk->size,
> +								chunk->type);

[Severity: High]
The commit message indicates that calibration memory is allocated sequentially
from data-region. However, paddr 0 is passed here and the region's offset is
not consumed. Is CALDB intended to be allocated from the data-region or
mapped to 0?

> +				if (ret)
> +					goto out;
> +
> +				idx++;
> +				continue;
> +			}
> +
> +			if (offset > avail_rmem_size ||
> +			    avail_rmem_size - offset < chunk->size) {
> +				ath12k_err(ab, "failed to assign mem type %u req size %u avail size %zu\n",
> +					   chunk->type, chunk->size,
> +					   offset > avail_rmem_size ? 0 :
> +					   avail_rmem_size - offset);
> +				ret = -EINVAL;
> +				goto out;
> +			}
> +
> +			ret = ath12k_qmi_map_target_mem(ab, idx, res.start + offset,
> +							chunk->size, chunk->type);

[Severity: High]
This loop maps every chunk type (other than CALDB) sequentially into the
data-region without filtering by chunk type.

Since the commit message states the device tree continues to provide
separate q6-region and mlo-global-mem regions, won't this incorrectly map
types like HOST_DDR_REGION_TYPE and MLO_GLOBAL_MEM_REGION_TYPE into the
data-region instead of their dedicated DT regions?

> +			if (ret)
> +				goto out;
> +
> +			offset += chunk->size;
> +			idx++;
> +		}
> +
> +		ab->qmi.mem_seg_count = idx;
> +		return 0;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-mpp_bringup-v1-0-2c5797b5e4ac@oss.qualcomm.com?part=9