[PATCH] drm/i915/guc: Add synchronization on interrupt enable flag

[Dong, Zhanjun]

See my comments inline below.

Regards,
Zhanjun Dong

On 2025-08-20 6:17 a.m., Andi Shyti wrote:
> Hi Zhanjun,
> 
> On Tue, Aug 19, 2025 at 12:00:10PM -0400, Zhanjun Dong wrote:
>> Boolean flag access from interrupt context might have synchronous issue on
> 
> /synchronous/synchronization/
> /issue/issues/
> 
>> multiple processor platform, flage modified by one core might be read as an
> 
> /flage/flags/
> 
>> old value by another core. This issue on interrupt enable flag might causes
>> interrupt missing or leakage.
> 
> /missing/misses/
> 
>> Fixed by change the data type as automic to add memory synchronization.
> 
> This can be re-written: "Change the interrupts.enable type to
> atomic to ensure memory synchronization."
Will follow all above comments

> 
>>
>> Fixes: a187f13d51fa0 ("drm/i915/guc: handle interrupts from media GuC")
> 
> What issue are you exactly trying to fix? And have you tested
Will add:
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14834

> that this patch is this really fixing it? Where is the flag's
CI report this issue about every 1 or 2 months and close it because no 
reproduce.
I can reproduce it in about 1000 rounds(about 9 hours) of IGT test but 
not at 100% rate, so it is hard to say really fixed because of hard to 
reproduce.
My latest test runs 2200 rounds in total has no reproduce.

> misalignment happening?
> 
> Please remember that when in interrupt context you are already
> running in atomic, so that probably you don't need to have an
> additional atomic access to variables.
Interrupt context only read it. When the flag was changed and interrupt 
was triggered in very short time, the flag read at interrupt context 
might read out old value, if other core(non interrupt context) set the 
flag and ISR read out 0, ISR will do simple return and misses interrupt 
handling, making it appear as lost interrupt; if other core clear the 
flag and ISR read out 1, ISR will continue to run and not stop as 
expected, making it appear as an interrupt leak. >
> Andi
> 
>> Signed-off-by: Zhanjun Dong <zhanjun.dong at intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>