drm/msm: adreno: fix deferencing ifpc_reglist when not declared

[PATCH] drm/msm: adreno: fix deferencing ifpc_reglist when not declared

Posted by Neil Armstrong 2 weeks ago

On plaforms with an a7xx GPU not supporting IFPC, the ifpc_reglist
if still deferenced in a7xx_patch_pwrup_reglist() which causes
a kernel crash:
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
...
pc : a6xx_hw_init+0x155c/0x1e4c [msm]
lr : a6xx_hw_init+0x9a8/0x1e4c [msm]
...
Call trace:
  a6xx_hw_init+0x155c/0x1e4c [msm] (P)
  msm_gpu_hw_init+0x58/0x88 [msm]
  adreno_load_gpu+0x94/0x1fc [msm]
  msm_open+0xe4/0xf4 [msm]
  drm_file_alloc+0x1a0/0x2e4 [drm]
  drm_client_init+0x7c/0x104 [drm]
  drm_fbdev_client_setup+0x94/0xcf0 [drm_client_lib]
  drm_client_setup+0xb4/0xd8 [drm_client_lib]
  msm_drm_kms_post_init+0x2c/0x3c [msm]
  msm_drm_init+0x1a4/0x228 [msm]
  msm_drm_bind+0x30/0x3c [msm]
...

Check the validity of ifpc_reglist before deferencing the table
to setup the register values.

Fixes: a6a0157cc68e ("drm/msm/a6xx: Enable IFPC on Adreno X1-85")
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index b8f8ae940b55..7ee15f339438 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -831,15 +831,17 @@ static void a7xx_patch_pwrup_reglist(struct msm_gpu *gpu)
 	lock->gpu_req = lock->cpu_req = lock->turn = 0;
 
 	reglist = adreno_gpu->info->a6xx->ifpc_reglist;
-	lock->ifpc_list_len = reglist->count;
+	if (reglist) {
+		lock->ifpc_list_len = reglist->count;
 
-	/*
-	 * For each entry in each of the lists, write the offset and the current
-	 * register value into the GPU buffer
-	 */
-	for (i = 0; i < reglist->count; i++) {
-		*dest++ = reglist->regs[i];
-		*dest++ = gpu_read(gpu, reglist->regs[i]);
+		/*
+		 * For each entry in each of the lists, write the offset and the current
+		 * register value into the GPU buffer
+		 */
+		for (i = 0; i < reglist->count; i++) {
+			*dest++ = reglist->regs[i];
+			*dest++ = gpu_read(gpu, reglist->regs[i]);
+		}
 	}
 
 	reglist = adreno_gpu->info->a6xx->pwrup_reglist;

---
base-commit: d6556aa99b8839819267e8167f04b715cfb5d4cf
change-id: 20251117-topic-sm8x50-fix-a6xx-non-ifpc-e89753af3696

Best regards,
-- 
Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH] drm/msm: adreno: fix deferencing ifpc_reglist when not declared

Posted by Akhil P Oommen 1 week, 6 days ago

On 11/17/2025 8:21 PM, Neil Armstrong wrote:
> On plaforms with an a7xx GPU not supporting IFPC, the ifpc_reglist
> if still deferenced in a7xx_patch_pwrup_reglist() which causes
> a kernel crash:
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
> ...
> pc : a6xx_hw_init+0x155c/0x1e4c [msm]
> lr : a6xx_hw_init+0x9a8/0x1e4c [msm]
> ...
> Call trace:
>   a6xx_hw_init+0x155c/0x1e4c [msm] (P)
>   msm_gpu_hw_init+0x58/0x88 [msm]
>   adreno_load_gpu+0x94/0x1fc [msm]
>   msm_open+0xe4/0xf4 [msm]
>   drm_file_alloc+0x1a0/0x2e4 [drm]
>   drm_client_init+0x7c/0x104 [drm]
>   drm_fbdev_client_setup+0x94/0xcf0 [drm_client_lib]
>   drm_client_setup+0xb4/0xd8 [drm_client_lib]
>   msm_drm_kms_post_init+0x2c/0x3c [msm]
>   msm_drm_init+0x1a4/0x228 [msm]
>   msm_drm_bind+0x30/0x3c [msm]
> ...
> 
> Check the validity of ifpc_reglist before deferencing the table
> to setup the register values.
> 
> Fixes: a6a0157cc68e ("drm/msm/a6xx: Enable IFPC on Adreno X1-85")
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

Reviewed-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>

Thanks for fixing this. Chia reported this issue a couple of weeks back.
Probably, we can add his S-b tag.
https://lore.kernel.org/lkml/CAPaKu7QvBZmx0b8SDBY1H3u-Em+0py0BrGkmrfGUXkx1uQrg3A@mail.gmail.com/

-Akhil

Re: [PATCH] drm/msm: adreno: fix deferencing ifpc_reglist when not declared

Posted by Akhil P Oommen 1 week, 6 days ago

On 11/18/2025 1:05 PM, Akhil P Oommen wrote:
> On 11/17/2025 8:21 PM, Neil Armstrong wrote:
>> On plaforms with an a7xx GPU not supporting IFPC, the ifpc_reglist
>> if still deferenced in a7xx_patch_pwrup_reglist() which causes
>> a kernel crash:
>> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
>> ...
>> pc : a6xx_hw_init+0x155c/0x1e4c [msm]
>> lr : a6xx_hw_init+0x9a8/0x1e4c [msm]
>> ...
>> Call trace:
>>   a6xx_hw_init+0x155c/0x1e4c [msm] (P)
>>   msm_gpu_hw_init+0x58/0x88 [msm]
>>   adreno_load_gpu+0x94/0x1fc [msm]
>>   msm_open+0xe4/0xf4 [msm]
>>   drm_file_alloc+0x1a0/0x2e4 [drm]
>>   drm_client_init+0x7c/0x104 [drm]
>>   drm_fbdev_client_setup+0x94/0xcf0 [drm_client_lib]
>>   drm_client_setup+0xb4/0xd8 [drm_client_lib]
>>   msm_drm_kms_post_init+0x2c/0x3c [msm]
>>   msm_drm_init+0x1a4/0x228 [msm]
>>   msm_drm_bind+0x30/0x3c [msm]
>> ...
>>
>> Check the validity of ifpc_reglist before deferencing the table
>> to setup the register values.
>>
>> Fixes: a6a0157cc68e ("drm/msm/a6xx: Enable IFPC on Adreno X1-85")
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> 
> Reviewed-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> 
> Thanks for fixing this. Chia reported this issue a couple of weeks back.
> Probably, we can add his S-b tag.

Sorry, I meant R-b tag.

> https://lore.kernel.org/lkml/CAPaKu7QvBZmx0b8SDBY1H3u-Em+0py0BrGkmrfGUXkx1uQrg3A@mail.gmail.com/
> 
> -Akhil
>

Re: [PATCH] drm/msm: adreno: fix deferencing ifpc_reglist when not declared

Posted by Konrad Dybcio 2 weeks ago

On 11/17/25 3:51 PM, Neil Armstrong wrote:
> On plaforms with an a7xx GPU not supporting IFPC, the ifpc_reglist
> if still deferenced in a7xx_patch_pwrup_reglist() which causes
> a kernel crash:
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
> ...
> pc : a6xx_hw_init+0x155c/0x1e4c [msm]
> lr : a6xx_hw_init+0x9a8/0x1e4c [msm]
> ...
> Call trace:
>   a6xx_hw_init+0x155c/0x1e4c [msm] (P)
>   msm_gpu_hw_init+0x58/0x88 [msm]
>   adreno_load_gpu+0x94/0x1fc [msm]
>   msm_open+0xe4/0xf4 [msm]
>   drm_file_alloc+0x1a0/0x2e4 [drm]
>   drm_client_init+0x7c/0x104 [drm]
>   drm_fbdev_client_setup+0x94/0xcf0 [drm_client_lib]
>   drm_client_setup+0xb4/0xd8 [drm_client_lib]
>   msm_drm_kms_post_init+0x2c/0x3c [msm]
>   msm_drm_init+0x1a4/0x228 [msm]
>   msm_drm_bind+0x30/0x3c [msm]
> ...
> 
> Check the validity of ifpc_reglist before deferencing the table
> to setup the register values.
> 
> Fixes: a6a0157cc68e ("drm/msm/a6xx: Enable IFPC on Adreno X1-85")
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---

I think it should be fine to skip calling this func altogether
if !ifpc || !pwrup_reglist

Although ifpc && !pwrup_reglist should probably scream very loud

Konrad

Re: [PATCH] drm/msm: adreno: fix deferencing ifpc_reglist when not declared

Posted by Neil Armstrong 2 weeks ago

On 11/17/25 16:02, Konrad Dybcio wrote:
> On 11/17/25 3:51 PM, Neil Armstrong wrote:
>> On plaforms with an a7xx GPU not supporting IFPC, the ifpc_reglist
>> if still deferenced in a7xx_patch_pwrup_reglist() which causes
>> a kernel crash:
>> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
>> ...
>> pc : a6xx_hw_init+0x155c/0x1e4c [msm]
>> lr : a6xx_hw_init+0x9a8/0x1e4c [msm]
>> ...
>> Call trace:
>>    a6xx_hw_init+0x155c/0x1e4c [msm] (P)
>>    msm_gpu_hw_init+0x58/0x88 [msm]
>>    adreno_load_gpu+0x94/0x1fc [msm]
>>    msm_open+0xe4/0xf4 [msm]
>>    drm_file_alloc+0x1a0/0x2e4 [drm]
>>    drm_client_init+0x7c/0x104 [drm]
>>    drm_fbdev_client_setup+0x94/0xcf0 [drm_client_lib]
>>    drm_client_setup+0xb4/0xd8 [drm_client_lib]
>>    msm_drm_kms_post_init+0x2c/0x3c [msm]
>>    msm_drm_init+0x1a4/0x228 [msm]
>>    msm_drm_bind+0x30/0x3c [msm]
>> ...
>>
>> Check the validity of ifpc_reglist before deferencing the table
>> to setup the register values.
>>
>> Fixes: a6a0157cc68e ("drm/msm/a6xx: Enable IFPC on Adreno X1-85")
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
> 
> I think it should be fine to skip calling this func altogether
> if !ifpc || !pwrup_reglist
> 
> Although ifpc && !pwrup_reglist should probably scream very loud

Sorry but why? pwrup_reglist was introduced way earlier than IFPC.

Why would we be skipping the a7xx_patch_pwrup_reglist() because ifpc_reglist is not declared ???

Neil

> 
> Konrad

Re: [PATCH] drm/msm: adreno: fix deferencing ifpc_reglist when not declared

Posted by Akhil P Oommen 1 week, 6 days ago

On 11/17/2025 9:17 PM, Neil Armstrong wrote:
> On 11/17/25 16:02, Konrad Dybcio wrote:
>> On 11/17/25 3:51 PM, Neil Armstrong wrote:
>>> On plaforms with an a7xx GPU not supporting IFPC, the ifpc_reglist
>>> if still deferenced in a7xx_patch_pwrup_reglist() which causes
>>> a kernel crash:
>>> Unable to handle kernel NULL pointer dereference at virtual address
>>> 0000000000000008
>>> ...
>>> pc : a6xx_hw_init+0x155c/0x1e4c [msm]
>>> lr : a6xx_hw_init+0x9a8/0x1e4c [msm]
>>> ...
>>> Call trace:
>>>    a6xx_hw_init+0x155c/0x1e4c [msm] (P)
>>>    msm_gpu_hw_init+0x58/0x88 [msm]
>>>    adreno_load_gpu+0x94/0x1fc [msm]
>>>    msm_open+0xe4/0xf4 [msm]
>>>    drm_file_alloc+0x1a0/0x2e4 [drm]
>>>    drm_client_init+0x7c/0x104 [drm]
>>>    drm_fbdev_client_setup+0x94/0xcf0 [drm_client_lib]
>>>    drm_client_setup+0xb4/0xd8 [drm_client_lib]
>>>    msm_drm_kms_post_init+0x2c/0x3c [msm]
>>>    msm_drm_init+0x1a4/0x228 [msm]
>>>    msm_drm_bind+0x30/0x3c [msm]
>>> ...
>>>
>>> Check the validity of ifpc_reglist before deferencing the table
>>> to setup the register values.
>>>
>>> Fixes: a6a0157cc68e ("drm/msm/a6xx: Enable IFPC on Adreno X1-85")
>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>> ---
>>
>> I think it should be fine to skip calling this func altogether
>> if !ifpc || !pwrup_reglist
>>
>> Although ifpc && !pwrup_reglist should probably scream very loud

That's correct. It is a weird combo.

> 
> Sorry but why? pwrup_reglist was introduced way earlier than IFPC.

Pwrup list is shared with preemption feature too. And preemption support
was introduced before IFPC.

nit: It is nice to have a similar NULL check for pwrup reglist too. We
sometimes skip these during initial GPU bringup (like A840 for eg:).

-Akhil

> 
> Why would we be skipping the a7xx_patch_pwrup_reglist() because
> ifpc_reglist is not declared ???
> 
> Neil
> 
>>
>> Konrad
>

Re: [PATCH] drm/msm: adreno: fix deferencing ifpc_reglist when not declared

Posted by Konrad Dybcio 1 week, 6 days ago

On 11/18/25 8:47 AM, Akhil P Oommen wrote:
> On 11/17/2025 9:17 PM, Neil Armstrong wrote:
>> On 11/17/25 16:02, Konrad Dybcio wrote:
>>> On 11/17/25 3:51 PM, Neil Armstrong wrote:
>>>> On plaforms with an a7xx GPU not supporting IFPC, the ifpc_reglist
>>>> if still deferenced in a7xx_patch_pwrup_reglist() which causes
>>>> a kernel crash:
>>>> Unable to handle kernel NULL pointer dereference at virtual address
>>>> 0000000000000008
>>>> ...
>>>> pc : a6xx_hw_init+0x155c/0x1e4c [msm]
>>>> lr : a6xx_hw_init+0x9a8/0x1e4c [msm]
>>>> ...
>>>> Call trace:
>>>>    a6xx_hw_init+0x155c/0x1e4c [msm] (P)
>>>>    msm_gpu_hw_init+0x58/0x88 [msm]
>>>>    adreno_load_gpu+0x94/0x1fc [msm]
>>>>    msm_open+0xe4/0xf4 [msm]
>>>>    drm_file_alloc+0x1a0/0x2e4 [drm]
>>>>    drm_client_init+0x7c/0x104 [drm]
>>>>    drm_fbdev_client_setup+0x94/0xcf0 [drm_client_lib]
>>>>    drm_client_setup+0xb4/0xd8 [drm_client_lib]
>>>>    msm_drm_kms_post_init+0x2c/0x3c [msm]
>>>>    msm_drm_init+0x1a4/0x228 [msm]
>>>>    msm_drm_bind+0x30/0x3c [msm]
>>>> ...
>>>>
>>>> Check the validity of ifpc_reglist before deferencing the table
>>>> to setup the register values.
>>>>
>>>> Fixes: a6a0157cc68e ("drm/msm/a6xx: Enable IFPC on Adreno X1-85")
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> ---
>>>
>>> I think it should be fine to skip calling this func altogether
>>> if !ifpc || !pwrup_reglist
>>>
>>> Although ifpc && !pwrup_reglist should probably scream very loud
> 
> That's correct. It is a weird combo.
> 
>>
>> Sorry but why? pwrup_reglist was introduced way earlier than IFPC.
> 
> Pwrup list is shared with preemption feature too. And preemption support
> was introduced before IFPC.

Sorry, I somehow erased that from my memory

Konrad