drivers/net/wireless/ath/ath11k/qmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
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.
Fixes: 900730dc4705 ("wifi: ath: Use of_reserved_mem_region_to_resource() for "memory-region"")
Cc: stable@vger.kernel.org # v6.18
Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
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 =
--
2.45.1
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: stable@vger.kernel.org # v6.18
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> 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 =
On Monday, December 8, 2025 4:23:46 AM CST Baochen Qiang wrote:
> 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.
I am ready to submit the IPQ9574 support. This patch is a dependency. Should I
include this change in the series that adds IPQ9574?
> 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?
I'm sorry that I initially missed this question. I don't think we should move
&res initialization outside the loop. We also need host_ddr_sz to be
initialized by a HOST_DDR_REGION_TYPE (1) request. On IPQ9574, the firmware
doesn't make that request, so host_ddr_sz remains uninitialized. Since &res
and host_ddr_sz are used together, I think it's better to initialize them,
together.
Without patch:
ath11k c000000.wifi: qmi firmware request memory request
ath11k c000000.wifi: qmi mem seg type 4 size 409600
ath11k c000000.wifi: qmi mem seg type 2 size 262144
ath11k c000000.wifi: qmi mem seg type 3 size 1048576
...
ath11k c000000.wifi: failed to assign qmi target memory: -5
With patch:
ath11k c000000.wifi: qmi firmware request memory request
ath11k c000000.wifi: qmi mem seg type 4 size 409600
ath11k c000000.wifi: qmi mem seg type 2 size 262144
ath11k c000000.wifi: qmi mem seg type 3 size 1048576
ath11k c000000.wifi: qmi ignore invalid mem req type 3
ath11k c000000.wifi: qmi req mem_seg[0] 0x000000004ba00000 409600 4
ath11k c000000.wifi: qmi req mem_seg[1] 0x000000004b700000 262144 2
Tested on : WLAN.HK.2.9.0.1-01890-QCAHKSWPL_SILICONZ-1
Alex
On 12/25/2025 5:47 AM, Alex G. wrote: > On Monday, December 8, 2025 4:23:46 AM CST Baochen Qiang wrote: >> 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. > > I am ready to submit the IPQ9574 support. This patch is a dependency. Should I > include this change in the series that adds IPQ9574? Better not, first reason is for track purpose. And the second, since the patch is fixing a broken commit, IMO being standalone makes it easy for stable team to do backport work. > >> 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? > > I'm sorry that I initially missed this question. I don't think we should move > &res initialization outside the loop. We also need host_ddr_sz to be > initialized by a HOST_DDR_REGION_TYPE (1) request. On IPQ9574, the firmware > doesn't make that request, so host_ddr_sz remains uninitialized. Since &res > and host_ddr_sz are used together, I think it's better to initialize them, > together. fine then > > > Without patch: > > ath11k c000000.wifi: qmi firmware request memory request > ath11k c000000.wifi: qmi mem seg type 4 size 409600 > ath11k c000000.wifi: qmi mem seg type 2 size 262144 > ath11k c000000.wifi: qmi mem seg type 3 size 1048576 > ... > ath11k c000000.wifi: failed to assign qmi target memory: -5 > > > > With patch: > > ath11k c000000.wifi: qmi firmware request memory request > ath11k c000000.wifi: qmi mem seg type 4 size 409600 > ath11k c000000.wifi: qmi mem seg type 2 size 262144 > ath11k c000000.wifi: qmi mem seg type 3 size 1048576 > ath11k c000000.wifi: qmi ignore invalid mem req type 3 > ath11k c000000.wifi: qmi req mem_seg[0] 0x000000004ba00000 409600 4 > ath11k c000000.wifi: qmi req mem_seg[1] 0x000000004b700000 262144 2 > > > Tested on : WLAN.HK.2.9.0.1-01890-QCAHKSWPL_SILICONZ-1 > > Alex > > > >
On 12/6/2025 11:28 PM, 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
AFAICT, this is highly unlikely as HOST_DDR_REGION_TYPE will always be before
CALDB_MEM_REGION_TYPE.
> 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.
>
> Fixes: 900730dc4705 ("wifi: ath: Use of_reserved_mem_region_to_resource() for "memory-region"")
>
> Cc: stable@vger.kernel.org # v6.18
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> 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 =
The rest looks good.
Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>
On 12/8/2025 3:38 PM, Vasanthakumar Thiagarajan wrote:
>
>
> On 12/6/2025 11:28 PM, 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
>
> AFAICT, this is highly unlikely as HOST_DDR_REGION_TYPE will always be before
> CALDB_MEM_REGION_TYPE. >
>> 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.
>>
>> Fixes: 900730dc4705 ("wifi: ath: Use of_reserved_mem_region_to_resource() for
>> "memory-region"")
>>
>> Cc: stable@vger.kernel.org # v6.18
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>> ---
>> 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 =
>
> The rest looks good.
>
> Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>
>
Well, since CALDB_MEM_REGION_TYPE will always come only after HOST_DDR_REGION_TYPE we'll
not be running into this issue in real deployment with ath11k firmware binaries available
in public.
On 12/8/2025 3:55 PM, Vasanthakumar Thiagarajan wrote:
>
>
> On 12/8/2025 3:38 PM, Vasanthakumar Thiagarajan wrote:
>>
>>
>> On 12/6/2025 11:28 PM, 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
>>
>> AFAICT, this is highly unlikely as HOST_DDR_REGION_TYPE will always be before
>> CALDB_MEM_REGION_TYPE. >
>>> 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.
>>>
>>> Fixes: 900730dc4705 ("wifi: ath: Use of_reserved_mem_region_to_resource() for
>>> "memory-region"")
>>>
>>> Cc: stable@vger.kernel.org # v6.18
>>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>>> ---
>>> 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 =
>>
>> The rest looks good.
>>
>> Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>
>>
>
> Well, since CALDB_MEM_REGION_TYPE will always come only after HOST_DDR_REGION_TYPE we'll
> not be running into this issue in real deployment with ath11k firmware binaries available
> in public.
>
Nevertheless, I agree this is a critical issue to be fixed. There is another patch[1]
fixing the same issue a bit differently.
Vasanth
[1]: https://lore.kernel.org/linux-wireless/20251208200437.14199-1-ansuelsmth@gmail.com/
© 2016 - 2026 Red Hat, Inc.