linux-kernel - Re: [PATCH] drm/i915/gsc: mei interrupt top half should be in irq disabled context

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <87zfg6yq1k.fsf@intel.com>
Date: Thu, 24 Apr 2025 10:30:47 +0300
From: Jani Nikula <jani.nikula@...ux.intel.com>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Junxiao Chang
 <junxiao.chang@...el.com>
Cc: Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>, Rodrigo Vivi
 <rodrigo.vivi@...el.com>, Tvrtko Ursulin <tursulin@...ulin.net>, David
 Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Clark
 Williams <clrkwllms@...nel.org>, Steven Rostedt <rostedt@...dmis.org>,
 intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev
Subject: Re: [PATCH] drm/i915/gsc: mei interrupt top half should be in irq
 disabled context

On Thu, 24 Apr 2025, Sebastian Andrzej Siewior <bigeasy@...utronix.de> wrote:
> On 2025-04-24 14:56:08 [+0800], Junxiao Chang wrote:
>> MEI GSC interrupt comes from i915. It has top half and bottom half.
>> Top half is called from i915 interrupt handler. It should be in
>> irq disabled context.
>> 
>> With RT kernel, by default i915 IRQ handler is in threaded IRQ. MEI GSC
>> top half might be in threaded IRQ context. In this case, local IRQ
>> should be disabled for MEI GSC interrupt top half.
>> 
>> This change fixes A380/A770 GPU boot hang issue with RT kernel.
>
> This should have a Fixes when generic_handle_irq() was introduced. 
>
>> Signed-off-by: Junxiao Chang <junxiao.chang@...el.com>
>> ---
>>  drivers/gpu/drm/i915/gt/intel_gsc.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.c b/drivers/gpu/drm/i915/gt/intel_gsc.c
>> index 1e925c75fb080..9c72117263f78 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gsc.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gsc.c
>> @@ -270,6 +270,9 @@ static void gsc_init_one(struct drm_i915_private *i915, struct intel_gsc *gsc,
>>  static void gsc_irq_handler(struct intel_gt *gt, unsigned int intf_id)
>>  {
>>  	int ret;
>> +#ifdef CONFIG_PREEMPT_RT
>> +	int irq_disabled_flag;
>> +#endif
>>  
>>  	if (intf_id >= INTEL_GSC_NUM_INTERFACES) {
>>  		gt_warn_once(gt, "GSC irq: intf_id %d is out of range", intf_id);
>> @@ -284,7 +287,18 @@ static void gsc_irq_handler(struct intel_gt *gt, unsigned int intf_id)
>>  	if (gt->gsc.intf[intf_id].irq < 0)
>>  		return;
>>  
>> +#ifdef CONFIG_PREEMPT_RT
>> +	/* mei interrupt top half should run in irq disabled context */
>> +	irq_disabled_flag = irqs_disabled();
>> +	if (!irq_disabled_flag)
>> +		local_irq_disable();
>> +#endif
>>  	ret = generic_handle_irq(gt->gsc.intf[intf_id].irq);
>
> What about generic_handle_irq_safe() instead the whole ifdef show?

Anything without the ifdefs would be welcome.

BR,
Jani.

>
>> +#ifdef CONFIG_PREEMPT_RT
>> +	if (!irq_disabled_flag)
>> +		local_irq_enable();
>> +#endif
>> +
>>  	if (ret)
>>  		gt_err_ratelimited(gt, "error handling GSC irq: %d\n", ret);
>>  }
>
> Sebastian

-- 
Jani Nikula, Intel