linux-kernel - Re: [PATCH] drm/amd/display: add a WARN() to irq service functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 17 Oct 2022 09:58:41 -0400
From:   Hamza Mahfooz <hamza.mahfooz@....com>
To:     Rodrigo Siqueira <Rodrigo.Siqueira@....com>
Cc:     Harry Wentland <harry.wentland@....com>,
        Leo Li <sunpeng.li@....com>,
        Alex Deucher <alexander.deucher@....com>,
        Christian König <christian.koenig@....com>,
        "Pan, Xinhui" <Xinhui.Pan@....com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Mario Limonciello <mario.limonciello@....com>,
        Nicholas Kazlauskas <nicholas.kazlauskas@....com>,
        Alex Hung <alex.hung@....com>, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, amd-gfx@...ts.freedesktop.org
Subject: Re: [PATCH] drm/amd/display: add a WARN() to irq service functions

On 2022-10-17 09:06, Rodrigo Siqueira wrote:
> Hi Hamza,
> 
> On 10/14/22 11:31, Hamza Mahfooz wrote:
>> Currently, if we encounter unimplemented functions, it is difficult to
>> tell what caused them just by looking at dmesg and that is compounded by
>> the fact that it is often hard to reproduce said issues. So, to have
>> access to more detailed debugging information, add a WARN() to
>> dal_irq_service_ack() and dal_irq_service_set() that only triggers when
>> we encounter an unimplemented function.
> 
> Do you know the specific issue that triggered this unimplemented 
> function? It might be useful to describe the situation in the commit 
> message where you see this problem.
> 

Ya, I'll do that in v2.

>>
>> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@....com>
>> ---
>>   drivers/gpu/drm/amd/display/dc/irq/irq_service.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/irq/irq_service.c 
>> b/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
>> index 7bad39bba86b..b895bdd8dc55 100644
>> --- a/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
>> +++ b/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
>> @@ -112,8 +112,11 @@ bool dal_irq_service_set(
>>       dal_irq_service_ack(irq_service, source);
>> -    if (info->funcs && info->funcs->set)
>> +    if (info->funcs && info->funcs->set) {
>> +        WARN(info->funcs->set == dal_irq_service_dummy_set,
>> +             "%s: src: %d, st: %d\n", __func__, source, enable);
>>           return info->funcs->set(irq_service, info, enable);
> 
> Do you know if we may hit this condition multiple times?

Yes, it is possible that it will be hit multiple times from different 
callers.

> 
>> +    }
>>       dal_irq_service_set_generic(irq_service, info, enable);
>> @@ -146,8 +149,11 @@ bool dal_irq_service_ack(
>>           return false;
>>       }
>> -    if (info->funcs && info->funcs->ack)
>> +    if (info->funcs && info->funcs->ack) {
>> +        WARN(info->funcs->ack == dal_irq_service_dummy_ack,
>> +             "%s: src: %d\n", __func__, source);
>>           return info->funcs->ack(irq_service, info);
>> +    }
>>       dal_irq_service_ack_generic(irq_service, info);
> 
> Just for curiosity, did you run some IGT tests?

No, this was encountered during manual testing.

> 
> Thanks
> Siqueira
> 

-- 
Hamza