Re: [PATCH ath-next v2 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] ath12k: data-region loop incorrectly intercepts all memory chunk types
--

commit 1f2202b158e5c8168f0cae90faae57bce35a9612
Author: Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>
wifi: ath12k: add support for unified data-region memory allocation

This commit adds support for devices that provide M3 and calibration
memory through a unified data-region reserved-memory node. If the
data-region lookup fails, it falls back to the existing per-memory-type
allocation scheme.

> diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
> --- 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);
> +				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]
The commit message states that only M3 and calibration memory should be
allocated from the unified data-region, while other segments continue to use
separate regions.

Since there is no check for the chunk type here, won't this unconditionally
map all memory segments (including HOST_DDR_REGION_TYPE, BDF_MEM_REGION_TYPE,
and MLO_GLOBAL_MEM_REGION_TYPE) sequentially from the data-region?

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

[Severity: High]
Because the function returns 0 here after iterating all requested memory
segments, does this entirely bypass the fallback code below that maps the
dedicated q6-region and mlo-global-mem regions?

Could this lead to device initialization failure if the data-region is
exhausted, or place hardware-specific memory segments at incorrect physical
addresses?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-mpp-bringup-v2-0-f510c33bfcf5@oss.qualcomm.com?part=9