[PATCH net] net: ethernet: ti: cpts: drop stale events before the misc IRQ can fire

[PATCH net] net: ethernet: ti: cpts: drop stale events before the misc IRQ can fire

[Hung-Chun Tseng]

cpsw requests its misc IRQ at probe, but the CPTS event pool is only set
up later, in cpts_register() from ndo_open. That is fine from a cold boot,
where the CPTS comes out of reset stopped with its interrupt masked. After
a kexec it is not: the previous kernel leaves the CPTS running with
TS_PEND_EN set and events in its FIFO, so the interrupt fires as soon as
it is requested and cpts_fifo_read() takes an entry from the still empty
pool list:

  Unable to handle kernel NULL pointer dereference at virtual address 00000010 when write
  PC is at cpts_fifo_read+0xa4/0x3c8
  Call trace:
   cpts_fifo_read from cpsw_misc_interrupt+0x30/0x4c
   cpsw_misc_interrupt from __handle_irq_event_percpu+0x74/0x378
   ...
   __setup_irq from request_threaded_irq+0xbc/0x170
   request_threaded_irq from devm_request_threaded_irq+0x70/0x104

On a BeagleBone Black the CPTS registers read control=0x1, int_enable=0x1
and intstat_raw=0x1 in cpts_create() after the kexec, with one event
queued.

Mask the CPTS interrupt and drain the FIFO in cpts_create(), before
either cpsw driver requests the IRQ. cpts_register() unmasks it again
once the pool exists. The CPTS is left enabled: clearing CPTS_EN at this
point made TS_PEND_RAW read back as set, which would hand the handler a
bogus event.

Masking alone is not enough, as the interrupt may already be latched by
then, and clearing the wrapper's misc_en before requesting the IRQ does
not help either, since cpsw_misc_interrupt() sets it again on the way
out.

Fixes: 84ea9c0a95d7 ("net: ethernet: ti: cpsw: enable cpts irq")
Signed-off-by: Hung-Chun Tseng <alan.tseng.cs@gmail.com>
---
Tested on a BeagleBone Black (AM335x, cpsw_new) on top of net 7076f684d5:

 - Cold boot from U-Boot: probe and link come up and /dev/ptp0 is
   registered, as before.
 - kexec_load from a kernel without this patch into one with it, and from
   a patched kernel into itself: no oops, link comes up, /dev/ptp0 is
   registered and its time advances.

Without the patch the kexec'd kernel oopses as quoted above on every boot.

Not tested: the legacy cpsw driver, and Keystone netcp_ethss, which also
calls cpts_create(). The CPTS_EN behaviour described above was only
observed on AM335x.

How this was made: the investigation, the fix and this description were
done with an AI coding assistant (Claude Code) driving my board. Three
earlier attempts (clearing misc_en at probe, clearing CPTS_EN, draining and
then clearing CPTS_EN) were tried on the hardware and dropped; the register
values quoted above come from a debug print added for that. I have
reviewed the patch and can answer for it.

 drivers/net/ethernet/ti/cpts.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index 2ba4c8795d..3cc908ac90 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -747,7 +747,8 @@ struct cpts *cpts_create(struct device *dev, void __iomem *regs,
 			 struct device_node *node, u32 n_ext_ts)
 {
 	struct cpts *cpts;
-	int ret;
+	u32 high, low;
+	int ret, i;
 
 	cpts = devm_kzalloc(dev, sizeof(*cpts), GFP_KERNEL);
 	if (!cpts)
@@ -779,6 +780,17 @@ struct cpts *cpts_create(struct device *dev, void __iomem *regs,
 	if (ret)
 		return ERR_PTR(ret);
 
+	/* After a kexec the CPTS may still be running from the previous
+	 * kernel, with its interrupt enabled and events in the FIFO. The misc
+	 * IRQ is requested before cpts_register() sets up the event pool, so
+	 * mask the interrupt and drop the stale events now.
+	 */
+	cpts_write32(cpts, 0, int_enable);
+	for (i = 0; i < CPTS_FIFO_DEPTH; i++) {
+		if (cpts_fifo_pop(cpts, &high, &low))
+			break;
+	}
+
 	cpts->cc.read = cpts_systim_read;
 	cpts->cc.mask = CLOCKSOURCE_MASK(32);
 	cpts->info = cpts_info;
-- 
2.43.0

Re: [PATCH net] net: ethernet: ti: cpts: drop stale events before the misc IRQ can fire

[Siddharth Vadapalli]

On 10/26/26 12:53 PM, Hung-Chun Tseng wrote:
> cpsw requests its misc IRQ at probe, but the CPTS event pool is only set
> up later, in cpts_register() from ndo_open. That is fine from a cold boot,
> where the CPTS comes out of reset stopped with its interrupt masked. After
> a kexec it is not: the previous kernel leaves the CPTS running with
> TS_PEND_EN set and events in its FIFO, so the interrupt fires as soon as
> it is requested and cpts_fifo_read() takes an entry from the still empty
> pool list:
> 
>    Unable to handle kernel NULL pointer dereference at virtual address 00000010 when write
>    PC is at cpts_fifo_read+0xa4/0x3c8
>    Call trace:
>     cpts_fifo_read from cpsw_misc_interrupt+0x30/0x4c
>     cpsw_misc_interrupt from __handle_irq_event_percpu+0x74/0x378
>     ...
>     __setup_irq from request_threaded_irq+0xbc/0x170
>     request_threaded_irq from devm_request_threaded_irq+0x70/0x104
> 
> On a BeagleBone Black the CPTS registers read control=0x1, int_enable=0x1
> and intstat_raw=0x1 in cpts_create() after the kexec, with one event
> queued.
> 
> Mask the CPTS interrupt and drain the FIFO in cpts_create(), before
> either cpsw driver requests the IRQ. cpts_register() unmasks it again
> once the pool exists. The CPTS is left enabled: clearing CPTS_EN at this
> point made TS_PEND_RAW read back as set, which would hand the handler a
> bogus event.
> 
> Masking alone is not enough, as the interrupt may already be latched by
> then, and clearing the wrapper's misc_en before requesting the IRQ does
> not help either, since cpsw_misc_interrupt() sets it again on the way
> out.
> 
> Fixes: 84ea9c0a95d7 ("net: ethernet: ti: cpsw: enable cpts irq")
> Signed-off-by: Hung-Chun Tseng <alan.tseng.cs@gmail.com>
> ---
> Tested on a BeagleBone Black (AM335x, cpsw_new) on top of net 7076f684d5:
> 
>   - Cold boot from U-Boot: probe and link come up and /dev/ptp0 is
>     registered, as before.
>   - kexec_load from a kernel without this patch into one with it, and from
>     a patched kernel into itself: no oops, link comes up, /dev/ptp0 is
>     registered and its time advances.
> 
> Without the patch the kexec'd kernel oopses as quoted above on every boot.
> 
> Not tested: the legacy cpsw driver, and Keystone netcp_ethss, which also
> calls cpts_create(). The CPTS_EN behaviour described above was only
> observed on AM335x.
> 
> How this was made: the investigation, the fix and this description were
> done with an AI coding assistant (Claude Code) driving my board. Three
> earlier attempts (clearing misc_en at probe, clearing CPTS_EN, draining and
> then clearing CPTS_EN) were tried on the hardware and dropped; the register
> values quoted above come from a debug print added for that. I have
> reviewed the patch and can answer for it.
> 
>   drivers/net/ethernet/ti/cpts.c | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
> index 2ba4c8795d..3cc908ac90 100644
> --- a/drivers/net/ethernet/ti/cpts.c
> +++ b/drivers/net/ethernet/ti/cpts.c
> @@ -747,7 +747,8 @@ struct cpts *cpts_create(struct device *dev, void __iomem *regs,
>   			 struct device_node *node, u32 n_ext_ts)
>   {
>   	struct cpts *cpts;
> -	int ret;
> +	u32 high, low;
> +	int ret, i;
>   
>   	cpts = devm_kzalloc(dev, sizeof(*cpts), GFP_KERNEL);
>   	if (!cpts)
> @@ -779,6 +780,17 @@ struct cpts *cpts_create(struct device *dev, void __iomem *regs,
>   	if (ret)
>   		return ERR_PTR(ret);
>   
> +	/* After a kexec the CPTS may still be running from the previous
> +	 * kernel, with its interrupt enabled and events in the FIFO. The misc
> +	 * IRQ is requested before cpts_register() sets up the event pool, so
> +	 * mask the interrupt and drop the stale events now.
> +	 */
> +	cpts_write32(cpts, 0, int_enable);
> +	for (i = 0; i < CPTS_FIFO_DEPTH; i++) {
> +		if (cpts_fifo_pop(cpts, &high, &low))
> +			break;
> +	}

Thank you for posting this patch to fix the issue. While it will solve this 
particular issue, I believe that the underlying problem is that CPTS 
remains powered-on during the kexec sequence. Therefore the proper fix will 
be to power-off and power-on the CPTS module (to perform a complete reset) 
rather than fixing this particular symptom (please note that the remaining 
registers of CPTS retain their state as well, and a subset of the 
underlying cause is addressed by this patch).

Regards,
Siddharth.

Re: [PATCH net] net: ethernet: ti: cpts: drop stale events before the misc IRQ can fire

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix narrows the window but leaves the NULL-pointer write in
  cpts_fifo_read() reachable.

--

net: ethernet: ti: cpts: drop stale events before the misc IRQ can fire

After a kexec, the previous kernel can leave a misc IRQ pending. That IRQ
can run cpts_fifo_read() before cpts_register() has set up the event
pool. This patch masks the CPTS interrupt and drains the CPTS event FIFO
in cpts_create() to avoid that. CPTS_EN is deliberately left set.

> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
> index 2ba4c8795d605..3cc908ac90e2b 100644
> --- a/drivers/net/ethernet/ti/cpts.c
> +++ b/drivers/net/ethernet/ti/cpts.c

[ ... ]

> @@ -779,6 +780,17 @@ struct cpts *cpts_create(struct device *dev, void __iomem *regs,
>  	if (ret)
>  		return ERR_PTR(ret);
>  
> +	/* After a kexec the CPTS may still be running from the previous
> +	 * kernel, with its interrupt enabled and events in the FIFO. The misc
> +	 * IRQ is requested before cpts_register() sets up the event pool, so
> +	 * mask the interrupt and drop the stale events now.
> +	 */
> +	cpts_write32(cpts, 0, int_enable);
> +	for (i = 0; i < CPTS_FIFO_DEPTH; i++) {
> +		if (cpts_fifo_pop(cpts, &high, &low))
> +			break;
> +	}

[Severity: High]
Does this close the window completely, or only make it smaller?

CPTS_EN stays set, so the CPTS can keep queueing events after this loop
finishes. cpts->pool and cpts->events are still zeroed from devm_kzalloc().
Only cpts_register() initializes them, and it runs later from ndo_open.

A fair amount of work happens between cpts_create() and the misc IRQ
request. In cpsw_new.c:cpsw_probe():

    cpsw_init_common()        <- calls cpts_create()
    ...
    cpsw_create_ports()
    ...
    devm_request_irq(dev, cpsw->misc_irq, cpsw_misc_interrupt, ...)

In cpsw.c:cpsw_probe() the same window also covers register_netdev() and
cpsw_probe_dual_emac().

Several things could add new events to the FIFO in that window:

 - half-rollover and rollover events from the free-running counter
   (CPTS_EV_HALF / CPTS_EV_ROLL)
 - HW_TS_PUSH events, if the previous kernel left HWx_TS_PUSH_EN set
 - RX/TX timestamp events, if the previous kernel left port timestamping
   enabled

Port timestamping is only cleared by cpsw_soft_reset() in ndo_open, and
cpsw has no .shutdown handler. So after a kexec, a peer that sends PTP
event frames would keep generating RX events.

The commit message notes that the interrupt may already be latched. If
it is delivered when devm_request_irq() runs, the handler does:

cpsw_misc_interrupt()
  cpts_misc_interrupt()
    cpts_fifo_read()
      cpts_fifo_pop()

cpts_fifo_pop() tests intstat_raw & TS_PEND_RAW, so writing 0 to
int_enable does not stop the pop. Then in cpts_fifo_read():

		if (list_empty(&cpts->pool) && cpts_purge_events(cpts)) {
			dev_warn(cpts->dev, "cpts: event pool empty\n");
			break;
		}

		event = list_first_entry(&cpts->pool, struct cpts_event, list);
		event->high = hi;

On a zeroed list_head, list_empty() returns false, because NULL is not
equal to &cpts->pool. list_first_entry() then gives
container_of(NULL, struct cpts_event, list), so the write to event->high
lands near address 0 in hardirq context.

Isn't that the same oops the commit message describes?

Would it be more robust to make the handler safe to run before
cpts_register()? One option is to move INIT_LIST_HEAD(&cpts->events) and
INIT_LIST_HEAD(&cpts->pool) into cpts_create(). Another is to have
cpts_misc_interrupt() return early until registration has happened.
Either would replace the drain approach rather than extend it.

>  	cpts->cc.read = cpts_systim_read;
>  	cpts->cc.mask = CLOCKSOURCE_MASK(32);
>  	cpts->info = cpts_info;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261026072314.663980-1-alan.tseng.cs%40gmail.com