[PATCH] cxl/events: Return IRQ_NONE when no event is pending

[PATCH] cxl/events: Return IRQ_NONE when no event is pending

[Shaikh Kamaluddin]

CXL event interrupts may share an MSI/MSI-X vector with other event
logs or device features. Consequently, cxl_event_thread() is registered
with IRQF_SHARED and must determine whether an interrupt belongs to the
event-log facility.

The handler masks the Device Event Status register to the event logs
supported by the driver. However, when no supported status bit is set,
it exits the processing loop and still returns IRQ_HANDLED.

Track whether at least one supported event status bit was observed.
Return IRQ_NONE when there was no event to service, while continuing to
return IRQ_HANDLED after processing one or more event logs.

Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")

Signed-off-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
---
 drivers/cxl/pci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index c7c91e8dc51d..8b560cae91f2 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -515,6 +515,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
 	struct cxl_dev_id *dev_id = id;
 	struct cxl_dev_state *cxlds = dev_id->cxlds;
 	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
+	bool handled = false;
 	u32 status;
 
 	do {
@@ -527,11 +528,13 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
 		status &= CXLDEV_EVENT_STATUS_ALL;
 		if (!status)
 			break;
+
+		handled = true;
 		cxl_mem_get_event_records(mds, status);
 		cond_resched();
 	} while (status);
 
-	return IRQ_HANDLED;
+	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
 static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)

base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
prerequisite-patch-id: 92e40cd60a697020faac475dcc77ba63b33434ea
prerequisite-patch-id: 10027ad5d9aed85806047f807b3a76273b6c4a77
-- 
2.43.0

Re: [PATCH] cxl/events: Return IRQ_NONE when no event is pending

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The `cxl_event_thread` can enter an infinite loop on device surprise removal or mailbox hardware failure, hanging the CPU and permanently masking a shared IRQ line.
--

commit f6cda8d59c7a077a8328222a63290241e9b118c0
Author: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
cxl/events: Return IRQ_NONE when no event is pending

This commit tracks whether at least one supported event status bit was observed
during interrupt handling. It returns IRQ_NONE when there was no event to
service, allowing the shared interrupt handler to behave correctly.

> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index c7c91e8dc51dc..8b560cae91f2e 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -515,6 +515,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)

[ ... ]

>  		status &= CXLDEV_EVENT_STATUS_ALL;
>  		if (!status)
>  			break;
> +
> +		handled = true;
>  		cxl_mem_get_event_records(mds, status);
>  		cond_resched();
>  	} while (status);

[Severity: High]
This is a pre-existing issue, but can cxl_event_thread() get stuck in an
infinite loop here?

If a CXL device undergoes a surprise removal or a mailbox failure,
cxl_mem_get_records_log() can abort early without clearing the hardware
event status:

drivers/cxl/core/mbox.c:cxl_mem_get_records_log() {
    ...
		if (rc) {
			dev_err_ratelimited(dev,
				"Event log '%d': Failed to query event records : %d",
				type, rc);
			break;
		}
    ...
}

Because the local status variable evaluated in the while condition remains
non-zero, the loop will restart. If the memory-mapped read at the top of the
loop then returns 0xFFFFFFFF or the stuck status, the loop will spin endlessly,
permanently masking the shared interrupt line.

Should we add a mechanism to break out of this loop or track whether the
hardware status is actively being cleared?

>  
> -	return IRQ_HANDLED;
> +	return handled ? IRQ_HANDLED : IRQ_NONE;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906155705.13252-1-shaikhkamal2012@gmail.com?part=1

Re: [PATCH] cxl/events: Return IRQ_NONE when no event is pending

[Jonathan Cameron]

On Sun, 06 Sep 2026 16:09:00 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The `cxl_event_thread` can enter an infinite loop on device surprise removal or mailbox hardware failure, hanging the CPU and permanently masking a shared IRQ line.

FWIW I'd be doubtful about fixing this unless we have some evidence
it is actually useful in practice.  Today we do not support surprise
removal and I'm not sure that will change any time soon.

Jonathan

> --
> 
> commit f6cda8d59c7a077a8328222a63290241e9b118c0
> Author: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
> cxl/events: Return IRQ_NONE when no event is pending
> 
> This commit tracks whether at least one supported event status bit was observed
> during interrupt handling. It returns IRQ_NONE when there was no event to
> service, allowing the shared interrupt handler to behave correctly.
> 
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index c7c91e8dc51dc..8b560cae91f2e 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -515,6 +515,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)  
> 
> [ ... ]
> 
> >  		status &= CXLDEV_EVENT_STATUS_ALL;
> >  		if (!status)
> >  			break;
> > +
> > +		handled = true;
> >  		cxl_mem_get_event_records(mds, status);
> >  		cond_resched();
> >  	} while (status);  
> 
> [Severity: High]
> This is a pre-existing issue, but can cxl_event_thread() get stuck in an
> infinite loop here?
> 
> If a CXL device undergoes a surprise removal or a mailbox failure,
> cxl_mem_get_records_log() can abort early without clearing the hardware
> event status:
> 
> drivers/cxl/core/mbox.c:cxl_mem_get_records_log() {
>     ...
> 		if (rc) {
> 			dev_err_ratelimited(dev,
> 				"Event log '%d': Failed to query event records : %d",
> 				type, rc);
> 			break;
> 		}
>     ...
> }
> 
> Because the local status variable evaluated in the while condition remains
> non-zero, the loop will restart. If the memory-mapped read at the top of the
> loop then returns 0xFFFFFFFF or the stuck status, the loop will spin endlessly,
> permanently masking the shared interrupt line.
> 
> Should we add a mechanism to break out of this loop or track whether the
> hardware status is actively being cleared?
> 
> >  
> > -	return IRQ_HANDLED;
> > +	return handled ? IRQ_HANDLED : IRQ_NONE;
> >  }  
> 

Re: [PATCH] cxl/events: Return IRQ_NONE when no event is pending

[Jonathan Cameron]

On Sun,  6 Sep 2026 21:27:05 +0530
Shaikh Kamaluddin <shaikhkamal2012@gmail.com> wrote:

> CXL event interrupts may share an MSI/MSI-X vector with other event
> logs or device features. Consequently, cxl_event_thread() is registered
> with IRQF_SHARED and must determine whether an interrupt belongs to the
> event-log facility.
> 
> The handler masks the Device Event Status register to the event logs
> supported by the driver. However, when no supported status bit is set,
> it exits the processing loop and still returns IRQ_HANDLED.
> 
> Track whether at least one supported event status bit was observed.
> Return IRQ_NONE when there was no event to service, while continuing to
> return IRQ_HANDLED after processing one or more event logs.

What is the practical result of this change?  
I think it will only affect the spurious interrupt detection so
to me it looks like a cleanup rather than a fix.  Is there
any path by which we actually lose interrupts as a result of this?

> 
> Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
> 

No blank lines in tag blocks.  There are a number of tools that rely
on there not being any and as such there are also scripts that run
on various upstream trees that will send us annoying emails if this
blank line is still here!  I'm not sure why this mistake in patch
formatting is so common as there are plenty of docs without the
blank lines!

> Signed-off-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>

A suggestion for an alternative implementation below.

> ---
>  drivers/cxl/pci.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index c7c91e8dc51d..8b560cae91f2 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -515,6 +515,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
>  	struct cxl_dev_id *dev_id = id;
>  	struct cxl_dev_state *cxlds = dev_id->cxlds;
>  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> +	bool handled = false;
>  	u32 status;
>  
>  	do {
> @@ -527,11 +528,13 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
>  		status &= CXLDEV_EVENT_STATUS_ALL;
>  		if (!status)

			return IRQ_NONE;

And don't need the other changes.

>  			break;
> +
> +		handled = true;
>  		cxl_mem_get_event_records(mds, status);
>  		cond_resched();
>  	} while (status);
>  
> -	return IRQ_HANDLED;
> +	return handled ? IRQ_HANDLED : IRQ_NONE;
>  }
>  
>  static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)
> 
> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> prerequisite-patch-id: 92e40cd60a697020faac475dcc77ba63b33434ea
> prerequisite-patch-id: 10027ad5d9aed85806047f807b3a76273b6c4a77

Re: [PATCH] cxl/events: Return IRQ_NONE when no event is pending

[Anisa Su]

On Sun, Sep 06, 2026 at 09:27:05PM +0530, Shaikh Kamaluddin wrote:
> CXL event interrupts may share an MSI/MSI-X vector with other event
> logs or device features. Consequently, cxl_event_thread() is registered
> with IRQF_SHARED and must determine whether an interrupt belongs to the
> event-log facility.
> 
> The handler masks the Device Event Status register to the event logs
> supported by the driver. However, when no supported status bit is set,
> it exits the processing loop and still returns IRQ_HANDLED.
> 
> Track whether at least one supported event status bit was observed.
> Return IRQ_NONE when there was no event to service, while continuing to
> return IRQ_HANDLED after processing one or more event logs.
> 
> Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
> 
> Signed-off-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
Hello,

This looks similar to a patch I sent last week:
[PATCH v2 4/4] cxl/events: Return IRQ_NONE when no events were processed
https://lore.kernel.org/linux-cxl/8e22c1c2-8098-492a-8162-3dd27507c853@amd.com/T/#ma4cf2b2dddaeebc06cb73f81647817bdbb51963c

It was dropped because I received the feedback that in general, the driver does
not need to handle device-side errors/spec-violating behaviors. I believe it
would apply here as well. But if I misunderstood the intent of this patch,
let me know.

Thanks,
Anisa
> ---
>  drivers/cxl/pci.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index c7c91e8dc51d..8b560cae91f2 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -515,6 +515,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
>  	struct cxl_dev_id *dev_id = id;
>  	struct cxl_dev_state *cxlds = dev_id->cxlds;
>  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> +	bool handled = false;
>  	u32 status;
>  
>  	do {
> @@ -527,11 +528,13 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
>  		status &= CXLDEV_EVENT_STATUS_ALL;
>  		if (!status)
>  			break;
> +
> +		handled = true;
>  		cxl_mem_get_event_records(mds, status);
>  		cond_resched();
>  	} while (status);
>  
> -	return IRQ_HANDLED;
> +	return handled ? IRQ_HANDLED : IRQ_NONE;
>  }
>  
>  static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)
> 
> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> prerequisite-patch-id: 92e40cd60a697020faac475dcc77ba63b33434ea
> prerequisite-patch-id: 10027ad5d9aed85806047f807b3a76273b6c4a77
> -- 
> 2.43.0
> 

Re: [PATCH] cxl/events: Return IRQ_NONE when no event is pending

[Li Ming]

在 2026/9/8 02:48, Jonathan Cameron 写道:
> On Sun,  6 Sep 2026 21:27:05 +0530
> Shaikh Kamaluddin <shaikhkamal2012@gmail.com> wrote:
>
>> CXL event interrupts may share an MSI/MSI-X vector with other event
>> logs or device features. Consequently, cxl_event_thread() is registered
>> with IRQF_SHARED and must determine whether an interrupt belongs to the
>> event-log facility.
>>
>> The handler masks the Device Event Status register to the event logs
>> supported by the driver. However, when no supported status bit is set,
>> it exits the processing loop and still returns IRQ_HANDLED.
>>
>> Track whether at least one supported event status bit was observed.
>> Return IRQ_NONE when there was no event to service, while continuing to
>> return IRQ_HANDLED after processing one or more event logs.
> What is the practical result of this change?
> I think it will only affect the spurious interrupt detection so
> to me it looks like a cleanup rather than a fix.  Is there
> any path by which we actually lose interrupts as a result of this?
>
>> Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
>>
> No blank lines in tag blocks.  There are a number of tools that rely
> on there not being any and as such there are also scripts that run
> on various upstream trees that will send us annoying emails if this
> blank line is still here!  I'm not sure why this mistake in patch
> formatting is so common as there are plenty of docs without the
> blank lines!
>
>> Signed-off-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
> A suggestion for an alternative implementation below.
>
>> ---
>>   drivers/cxl/pci.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>> index c7c91e8dc51d..8b560cae91f2 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -515,6 +515,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
>>   	struct cxl_dev_id *dev_id = id;
>>   	struct cxl_dev_state *cxlds = dev_id->cxlds;
>>   	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
>> +	bool handled = false;
>>   	u32 status;
>>   
>>   	do {
>> @@ -527,11 +528,13 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
>>   		status &= CXLDEV_EVENT_STATUS_ALL;
>>   		if (!status)
> 			return IRQ_NONE;
>
> And don't need the other changes.

Hi Jonathan,


I think we should not return IRQ_NONE here, because the loop could run 
multiple times untill no event record, so we always get a status with 
0x0 value in the last loop. In that case, we still need to return 
IRQ_HANDLED.


Ming

>
>>   			break;
>> +
>> +		handled = true;
>>   		cxl_mem_get_event_records(mds, status);
>>   		cond_resched();
>>   	} while (status);
>>   
>> -	return IRQ_HANDLED;
>> +	return handled ? IRQ_HANDLED : IRQ_NONE;
>>   }
>>   
>>   static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)
>>
>> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
>> prerequisite-patch-id: 92e40cd60a697020faac475dcc77ba63b33434ea
>> prerequisite-patch-id: 10027ad5d9aed85806047f807b3a76273b6c4a77

Re: [PATCH] cxl/events: Return IRQ_NONE when no event is pending

[Shaikh Kamaluddin]

On Mon, Sep 07, 2026 at 07:48:24PM +0100, Jonathan Cameron wrote:
> On Sun,  6 Sep 2026 21:27:05 +0530
> Shaikh Kamaluddin <shaikhkamal2012@gmail.com> wrote:
> 
> > CXL event interrupts may share an MSI/MSI-X vector with other event
> > logs or device features. Consequently, cxl_event_thread() is registered
> > with IRQF_SHARED and must determine whether an interrupt belongs to the
> > event-log facility.
> > 
> > The handler masks the Device Event Status register to the event logs
> > supported by the driver. However, when no supported status bit is set,
> > it exits the processing loop and still returns IRQ_HANDLED.
> > 
> > Track whether at least one supported event status bit was observed.
> > Return IRQ_NONE when there was no event to service, while continuing to
> > return IRQ_HANDLED after processing one or more event logs.
> 
> What is the practical result of this change?  
> I think it will only affect the spurious interrupt detection so
> to me it looks like a cleanup rather than a fix.  Is there
> any path by which we actually lose interrupts as a result of this?
>
Hi Jonathan,
No interrupts are lost. I agree that it only affects spurious
interrupt detection.

When note_interrupt() receives IRQ_WAKE_THREAD, it sets
SPURIOUS_DEFERRED and checks on the next interrupt whether
desc->threads_handled changed. Currently, cxl_event_thread() returns
IRQ_HANDLED even for zero event status, incrementing that counter and
making an unrelated interrupt appear handled.

Returning IRQ_NONE for zero status leaves the counter unchanged. Another
handler sharing the vector can still claim the interrupt; otherwise,
note_interrupt() can account it as unhandled. Normal CXL event handling
is unchanged, so I agree this should be presented as a cleanup without a Fixes tag.

> > 
> > Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
> > 
> 
> No blank lines in tag blocks.  There are a number of tools that rely
> on there not being any and as such there are also scripts that run
> on various upstream trees that will send us annoying emails if this
> blank line is still here!  I'm not sure why this mistake in patch
> formatting is so common as there are plenty of docs without the
> blank lines!
>

Sorry It was my mistake

> > Signed-off-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
> 
> A suggestion for an alternative implementation below.
> 
> > ---
> >  drivers/cxl/pci.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index c7c91e8dc51d..8b560cae91f2 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -515,6 +515,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> >  	struct cxl_dev_id *dev_id = id;
> >  	struct cxl_dev_state *cxlds = dev_id->cxlds;
> >  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> > +	bool handled = false;
> >  	u32 status;
> >  
> >  	do {
> > @@ -527,11 +528,13 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> >  		status &= CXLDEV_EVENT_STATUS_ALL;
> >  		if (!status)
> 
> 			return IRQ_NONE;
> 
> And don't need the other changes.
> 
> >  			break;
> > +
> > +		handled = true;
> >  		cxl_mem_get_event_records(mds, status);
> >  		cond_resched();
> >  	} while (status);
> >  
> > -	return IRQ_HANDLED;
> > +	return handled ? IRQ_HANDLED : IRQ_NONE;
> >  }
> >  
> >  static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)
> > 
> > base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> > prerequisite-patch-id: 92e40cd60a697020faac475dcc77ba63b33434ea
> > prerequisite-patch-id: 10027ad5d9aed85806047f807b3a76273b6c4a77
> 

Re: [PATCH] cxl/events: Return IRQ_NONE when no event is pending

[Shaikh Kamaluddin]

On Tue, Sep 08, 2026 at 11:22:09AM -0700, Anisa Su wrote:
> On Sun, Sep 06, 2026 at 09:27:05PM +0530, Shaikh Kamaluddin wrote:
> > CXL event interrupts may share an MSI/MSI-X vector with other event
> > logs or device features. Consequently, cxl_event_thread() is registered
> > with IRQF_SHARED and must determine whether an interrupt belongs to the
> > event-log facility.
> > 
> > The handler masks the Device Event Status register to the event logs
> > supported by the driver. However, when no supported status bit is set,
> > it exits the processing loop and still returns IRQ_HANDLED.
> > 
> > Track whether at least one supported event status bit was observed.
> > Return IRQ_NONE when there was no event to service, while continuing to
> > return IRQ_HANDLED after processing one or more event logs.
> > 
> > Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
> > 
> > Signed-off-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
> Hello,
> 
> This looks similar to a patch I sent last week:
> [PATCH v2 4/4] cxl/events: Return IRQ_NONE when no events were processed
> https://lore.kernel.org/linux-cxl/8e22c1c2-8098-492a-8162-3dd27507c853@amd.com/T/#ma4cf2b2dddaeebc06cb73f81647817bdbb51963c
> 
> It was dropped because I received the feedback that in general, the driver does
> not need to handle device-side errors/spec-violating behaviors. I believe it
> would apply here as well. But if I misunderstood the intent of this patch,
> let me know.
>

Hi Anisa,

Thanks for pointing this out. I came across your v2 patch 4 only after I
had sent my patch.

Your patch 4 is based on the preceding robustness changes in the series.
By that point, cxl_event_thread() has been changed from the original
do-while loop to a while (mask) loop, and
cxl_mem_get_event_records() reports both an error return and the set of
drained logs. The handler also uses the drained and stuck masks to
control further retries.

My patch leaves the existing event-record retrieval and do-while loop
unchanged. It only tracks whether a supported event-status bit was
observed and uses that information to select IRQ_HANDLED or IRQ_NONE.
It therefore does not change the handling of mailbox errors, undrained
logs, or retry behavior.

The handled tracking in both cases addresses the same zero-status
shared-vector case. My motivation was its effect on generic IRQ
spurious-interrupt detection. Jonathan agreed that this is a cleanup
rather than an interrupt-loss fix, so the Fixes tag should be dropped.

Since your patch was posted first, would you like to repost the IRQ
return change as a standalone cleanup? If so, I am happy to step back.
If you no longer plan to pursue it, I can continue with a v2
incorporating the review feedback.

Please let me know which you prefer.

Thanks,
Shaikh




> Thanks,
> Anisa
> > ---
> >  drivers/cxl/pci.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index c7c91e8dc51d..8b560cae91f2 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -515,6 +515,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> >  	struct cxl_dev_id *dev_id = id;
> >  	struct cxl_dev_state *cxlds = dev_id->cxlds;
> >  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> > +	bool handled = false;
> >  	u32 status;
> >  
> >  	do {
> > @@ -527,11 +528,13 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> >  		status &= CXLDEV_EVENT_STATUS_ALL;
> >  		if (!status)
> >  			break;
> > +
> > +		handled = true;
> >  		cxl_mem_get_event_records(mds, status);
> >  		cond_resched();
> >  	} while (status);
> >  
> > -	return IRQ_HANDLED;
> > +	return handled ? IRQ_HANDLED : IRQ_NONE;
> >  }
> >  
> >  static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)
> > 
> > base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> > prerequisite-patch-id: 92e40cd60a697020faac475dcc77ba63b33434ea
> > prerequisite-patch-id: 10027ad5d9aed85806047f807b3a76273b6c4a77
> > -- 
> > 2.43.0
> > 

Re: [PATCH] cxl/events: Return IRQ_NONE when no event is pending

[Anisa Su]

On Wed, Sep 09, 2026 at 10:25:50PM +0530, Shaikh Kamaluddin wrote:
> On Tue, Sep 08, 2026 at 11:22:09AM -0700, Anisa Su wrote:
> > On Sun, Sep 06, 2026 at 09:27:05PM +0530, Shaikh Kamaluddin wrote:
> > > CXL event interrupts may share an MSI/MSI-X vector with other event
> > > logs or device features. Consequently, cxl_event_thread() is registered
> > > with IRQF_SHARED and must determine whether an interrupt belongs to the
> > > event-log facility.
> > > 
> > > The handler masks the Device Event Status register to the event logs
> > > supported by the driver. However, when no supported status bit is set,
> > > it exits the processing loop and still returns IRQ_HANDLED.
> > > 
> > > Track whether at least one supported event status bit was observed.
> > > Return IRQ_NONE when there was no event to service, while continuing to
> > > return IRQ_HANDLED after processing one or more event logs.
> > > 
> > > Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
> > > 
> > > Signed-off-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
> > Hello,
> > 
> > This looks similar to a patch I sent last week:
> > [PATCH v2 4/4] cxl/events: Return IRQ_NONE when no events were processed
> > https://lore.kernel.org/linux-cxl/8e22c1c2-8098-492a-8162-3dd27507c853@amd.com/T/#ma4cf2b2dddaeebc06cb73f81647817bdbb51963c
> > 
> > It was dropped because I received the feedback that in general, the driver does
> > not need to handle device-side errors/spec-violating behaviors. I believe it
> > would apply here as well. But if I misunderstood the intent of this patch,
> > let me know.
> >
> 
> Hi Anisa,
> 
> Thanks for pointing this out. I came across your v2 patch 4 only after I
> had sent my patch.
> 
> Your patch 4 is based on the preceding robustness changes in the series.
> By that point, cxl_event_thread() has been changed from the original
> do-while loop to a while (mask) loop, and
> cxl_mem_get_event_records() reports both an error return and the set of
> drained logs. The handler also uses the drained and stuck masks to
> control further retries.
> 
> My patch leaves the existing event-record retrieval and do-while loop
> unchanged. It only tracks whether a supported event-status bit was
> observed and uses that information to select IRQ_HANDLED or IRQ_NONE.
> It therefore does not change the handling of mailbox errors, undrained
> logs, or retry behavior.
> 
> The handled tracking in both cases addresses the same zero-status
> shared-vector case. My motivation was its effect on generic IRQ
> spurious-interrupt detection. Jonathan agreed that this is a cleanup
> rather than an interrupt-loss fix, so the Fixes tag should be dropped.
> 
> Since your patch was posted first, would you like to repost the IRQ
> return change as a standalone cleanup? If so, I am happy to step back.
> If you no longer plan to pursue it, I can continue with a v2
> incorporating the review feedback.
> 
> Please let me know which you prefer.
> 
Hello Shaikh,

Thank you for your clarification. Please feel free to continue with v2.
Can you add me to the CC list for v2? I am working on some prepatory
patches for DCD an there would be a minor conflict with this patch,
so I will need to rebase on your changes.

Also Ben Cheatham gave the feedback to mention that "MSI will eventually
be disabled" as a consequence of this patch in the commit message on my patch,
which I think may be valuable here as well:
https://lore.kernel.org/linux-cxl/20260901002912.958-1-anisa.su@samsung.com/T/#m866a4fdb159553039d8d47b6ee7f2312caa9ed73

Thanks,
Anisa

> Thanks,
> Shaikh
> 
> 
> 
> 
> > Thanks,
> > Anisa
> > > ---
> > >  drivers/cxl/pci.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > > index c7c91e8dc51d..8b560cae91f2 100644
> > > --- a/drivers/cxl/pci.c
> > > +++ b/drivers/cxl/pci.c
> > > @@ -515,6 +515,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> > >  	struct cxl_dev_id *dev_id = id;
> > >  	struct cxl_dev_state *cxlds = dev_id->cxlds;
> > >  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> > > +	bool handled = false;
> > >  	u32 status;
> > >  
> > >  	do {
> > > @@ -527,11 +528,13 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> > >  		status &= CXLDEV_EVENT_STATUS_ALL;
> > >  		if (!status)
> > >  			break;
> > > +
> > > +		handled = true;
> > >  		cxl_mem_get_event_records(mds, status);
> > >  		cond_resched();
> > >  	} while (status);
> > >  
> > > -	return IRQ_HANDLED;
> > > +	return handled ? IRQ_HANDLED : IRQ_NONE;
> > >  }
> > >  
> > >  static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)
> > > 
> > > base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> > > prerequisite-patch-id: 92e40cd60a697020faac475dcc77ba63b33434ea
> > > prerequisite-patch-id: 10027ad5d9aed85806047f807b3a76273b6c4a77
> > > -- 
> > > 2.43.0
> > > 

Re: [PATCH] cxl/events: Return IRQ_NONE when no event is pending

[Jonathan Cameron]

On Wed, 9 Sep 2026 10:08:14 +0800
Li Ming <ming.li@zohomail.com> wrote:

> 在 2026/9/8 02:48, Jonathan Cameron 写道:
> > On Sun,  6 Sep 2026 21:27:05 +0530
> > Shaikh Kamaluddin <shaikhkamal2012@gmail.com> wrote:
> >  
> >> CXL event interrupts may share an MSI/MSI-X vector with other event
> >> logs or device features. Consequently, cxl_event_thread() is registered
> >> with IRQF_SHARED and must determine whether an interrupt belongs to the
> >> event-log facility.
> >>
> >> The handler masks the Device Event Status register to the event logs
> >> supported by the driver. However, when no supported status bit is set,
> >> it exits the processing loop and still returns IRQ_HANDLED.
> >>
> >> Track whether at least one supported event status bit was observed.
> >> Return IRQ_NONE when there was no event to service, while continuing to
> >> return IRQ_HANDLED after processing one or more event logs.  
> > What is the practical result of this change?
> > I think it will only affect the spurious interrupt detection so
> > to me it looks like a cleanup rather than a fix.  Is there
> > any path by which we actually lose interrupts as a result of this?
> >  
> >> Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
> >>  
> > No blank lines in tag blocks.  There are a number of tools that rely
> > on there not being any and as such there are also scripts that run
> > on various upstream trees that will send us annoying emails if this
> > blank line is still here!  I'm not sure why this mistake in patch
> > formatting is so common as there are plenty of docs without the
> > blank lines!
> >  
> >> Signed-off-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>  
> > A suggestion for an alternative implementation below.
> >  
> >> ---
> >>   drivers/cxl/pci.c | 5 ++++-
> >>   1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> >> index c7c91e8dc51d..8b560cae91f2 100644
> >> --- a/drivers/cxl/pci.c
> >> +++ b/drivers/cxl/pci.c
> >> @@ -515,6 +515,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> >>   	struct cxl_dev_id *dev_id = id;
> >>   	struct cxl_dev_state *cxlds = dev_id->cxlds;
> >>   	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> >> +	bool handled = false;
> >>   	u32 status;
> >>   
> >>   	do {
> >> @@ -527,11 +528,13 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> >>   		status &= CXLDEV_EVENT_STATUS_ALL;
> >>   		if (!status)  
> > 			return IRQ_NONE;
> >
> > And don't need the other changes.  
> 
> Hi Jonathan,
> 
> 
> I think we should not return IRQ_NONE here, because the loop could run 
> multiple times untill no event record, so we always get a status with 
> 0x0 value in the last loop. In that case, we still need to return 
> IRQ_HANDLED.

Ah excellent point.  Ignore me.  Any dance to return here needs
a handled variable to be checked so ends up just being moving the
return.  Maybe that's worth doing but not important.

Jonathan

> 
> 
> Ming
> 
> >  
> >>   			break;
> >> +
> >> +		handled = true;
> >>   		cxl_mem_get_event_records(mds, status);
> >>   		cond_resched();
> >>   	} while (status);
> >>   
> >> -	return IRQ_HANDLED;
> >> +	return handled ? IRQ_HANDLED : IRQ_NONE;
> >>   }
> >>   
> >>   static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)
> >>
> >> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> >> prerequisite-patch-id: 92e40cd60a697020faac475dcc77ba63b33434ea
> >> prerequisite-patch-id: 10027ad5d9aed85806047f807b3a76273b6c4a77  

Re: [PATCH] cxl/events: Return IRQ_NONE when no event is pending

[Shaikh Kamaluddin]

On Thu, Sep 10, 2026 at 03:43:57AM +0900, Anisa Su wrote:
> On Wed, Sep 09, 2026 at 10:25:50PM +0530, Shaikh Kamaluddin wrote:
> > On Tue, Sep 08, 2026 at 11:22:09AM -0700, Anisa Su wrote:
> > > On Sun, Sep 06, 2026 at 09:27:05PM +0530, Shaikh Kamaluddin wrote:
> > > > CXL event interrupts may share an MSI/MSI-X vector with other event
> > > > logs or device features. Consequently, cxl_event_thread() is registered
> > > > with IRQF_SHARED and must determine whether an interrupt belongs to the
> > > > event-log facility.
> > > > 
> > > > The handler masks the Device Event Status register to the event logs
> > > > supported by the driver. However, when no supported status bit is set,
> > > > it exits the processing loop and still returns IRQ_HANDLED.
> > > > 
> > > > Track whether at least one supported event status bit was observed.
> > > > Return IRQ_NONE when there was no event to service, while continuing to
> > > > return IRQ_HANDLED after processing one or more event logs.
> > > > 
> > > > Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
> > > > 
> > > > Signed-off-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
> > > Hello,
> > > 
> > > This looks similar to a patch I sent last week:
> > > [PATCH v2 4/4] cxl/events: Return IRQ_NONE when no events were processed
> > > https://lore.kernel.org/linux-cxl/8e22c1c2-8098-492a-8162-3dd27507c853@amd.com/T/#ma4cf2b2dddaeebc06cb73f81647817bdbb51963c
> > > 
> > > It was dropped because I received the feedback that in general, the driver does
> > > not need to handle device-side errors/spec-violating behaviors. I believe it
> > > would apply here as well. But if I misunderstood the intent of this patch,
> > > let me know.
> > >
> > 
> > Hi Anisa,
> > 
> > Thanks for pointing this out. I came across your v2 patch 4 only after I
> > had sent my patch.
> > 
> > Your patch 4 is based on the preceding robustness changes in the series.
> > By that point, cxl_event_thread() has been changed from the original
> > do-while loop to a while (mask) loop, and
> > cxl_mem_get_event_records() reports both an error return and the set of
> > drained logs. The handler also uses the drained and stuck masks to
> > control further retries.
> > 
> > My patch leaves the existing event-record retrieval and do-while loop
> > unchanged. It only tracks whether a supported event-status bit was
> > observed and uses that information to select IRQ_HANDLED or IRQ_NONE.
> > It therefore does not change the handling of mailbox errors, undrained
> > logs, or retry behavior.
> > 
> > The handled tracking in both cases addresses the same zero-status
> > shared-vector case. My motivation was its effect on generic IRQ
> > spurious-interrupt detection. Jonathan agreed that this is a cleanup
> > rather than an interrupt-loss fix, so the Fixes tag should be dropped.
> > 
> > Since your patch was posted first, would you like to repost the IRQ
> > return change as a standalone cleanup? If so, I am happy to step back.
> > If you no longer plan to pursue it, I can continue with a v2
> > incorporating the review feedback.
> > 
> > Please let me know which you prefer.
> > 
> Hello Shaikh,
> 
> Thank you for your clarification. Please feel free to continue with v2.
> Can you add me to the CC list for v2? I am working on some prepatory
> patches for DCD an there would be a minor conflict with this patch,
> so I will need to rebase on your changes.
>

Hi Anisa,

Thank you for offering the handoff. Since you posted this change first,
I appreciate you allowing me to carry the standalone cleanup forward. I
will keep you in the Cc list for v2, so you can track the final change
and rebase your DCD preparation patches accordingly.


> Also Ben Cheatham gave the feedback to mention that "MSI will eventually
> be disabled" as a consequence of this patch in the commit message on my patch,
> which I think may be valuable here as well:
> https://lore.kernel.org/linux-cxl/20260901002912.958-1-anisa.su@samsung.com/T/#m866a4fdb159553039d8d47b6ee7f2312caa9ed73
> 

Hi Ben,

Thank you for the feedback on Anisa's earlier Patch. I will update the
commit message to describe the consequence more clearly. When the CXL
evnet thread returns IRQ_NONE, irq_thread_fn() does not increment
desc->threads_handled. On a subsequent interrupt, note_interrupt()
observes that the counter didn't change and accounts the previous
interrupt as unhandled.If more than 99,900 interrupts in its 100,000
interrupt detection window are unhandled, the generic IRQ code calls
irq_disable() and disables the affected MSI/MSI-X vector.

Thanks,
Shaikh 

> Thanks,
> Anisa
> 


> > Thanks,
> > Shaikh
> > 
> > 
> > 
> > 
> > > Thanks,
> > > Anisa
> > > > ---
> > > >  drivers/cxl/pci.c | 5 ++++-
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > > > index c7c91e8dc51d..8b560cae91f2 100644
> > > > --- a/drivers/cxl/pci.c
> > > > +++ b/drivers/cxl/pci.c
> > > > @@ -515,6 +515,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> > > >  	struct cxl_dev_id *dev_id = id;
> > > >  	struct cxl_dev_state *cxlds = dev_id->cxlds;
> > > >  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> > > > +	bool handled = false;
> > > >  	u32 status;
> > > >  
> > > >  	do {
> > > > @@ -527,11 +528,13 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> > > >  		status &= CXLDEV_EVENT_STATUS_ALL;
> > > >  		if (!status)
> > > >  			break;
> > > > +
> > > > +		handled = true;
> > > >  		cxl_mem_get_event_records(mds, status);
> > > >  		cond_resched();
> > > >  	} while (status);
> > > >  
> > > > -	return IRQ_HANDLED;
> > > > +	return handled ? IRQ_HANDLED : IRQ_NONE;
> > > >  }
> > > >  
> > > >  static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)
> > > > 
> > > > base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> > > > prerequisite-patch-id: 92e40cd60a697020faac475dcc77ba63b33434ea
> > > > prerequisite-patch-id: 10027ad5d9aed85806047f807b3a76273b6c4a77
> > > > -- 
> > > > 2.43.0
> > > >