[PATCH 0/7] Input: gscps2: cleanups and locking fixes

[PATCH 0/7] Input: gscps2: cleanups and locking fixes

[Dmitry Torokhov]

This series cleans up the PA-RISC GSC PS/2 driver (gscps2) and addresses
several concurrency, locking, and interrupt handling issues.

The series performs code style and structural cleanups, including removing
forward declarations, converting printk calls to dev_* logging helpers,
and simplifying enable/disable handling.

Additionally, it resolves several concurrency, locking, and interrupt
handling issues:
- Protect ps2port_list traversal with RCU and manage port registration in
  open/close rather than probe/remove to avoid use-after-free during
  device unregistration.
- Protect ring buffer indices with ps2port->lock in read and report
  helpers while releasing the lock before calling serio_interrupt().
- Assert and enforce ps2port->lock across gscps2_flush() callers.
- Return IRQ_NONE when no data was handled on shared interrupt lines to
  preserve core spurious interrupt detection.
- Serialize concurrent interrupt handlers using a try-lock to guarantee
  in-order byte delivery.
- Drop the 6 ms busy-wait and manual interrupt pump on transmit in favor
  of standard asynchronous hardware interrupt delivery.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Dmitry Torokhov (7):
      Input: gscps2 - clean up driver code style and structure
      Input: gscps2 - use RCU for ps2port_list and manage it in open/close
      Input: gscps2 - protect buffer access in read and report helpers
      Input: gscps2 - serialize hardware and buffer access in gscps2_flush()
      Input: gscps2 - return IRQ_NONE when interrupt is not handled
      Input: gscps2 - serialize concurrent interrupt handlers
      Input: gscps2 - drop busy-wait and manual interrupt pump on transmit

 drivers/input/serio/gscps2.c | 277 ++++++++++++++++++++++---------------------
 1 file changed, 139 insertions(+), 138 deletions(-)
---
base-commit: e30626823a406725ce29bc75cb8ec467d3e1e326
change-id: 20260809-gscps2-affec5caa7f9

Thanks.

-- 
Dmitry

[PATCH 1/7] Input: gscps2 - clean up driver code style and structure

[Dmitry Torokhov]

Clean up code style issues and function ordering in the gscps2 driver:
- Reorder functions to place gscps2_interrupt() before its callers,
  allowing removal of its forward declaration.
- Change gscps2_enable() to accept a boolean parameter and remove the
  ENABLE and DISABLE macro definitions.
- Convert printk() calls to dev_dbg() and dev_warn().
- Fix operator spacing and multi-variable assignment.
- Add spinlock comment and use cpu_relax() in spin-wait loop.

Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 208 +++++++++++++++++++------------------------
 1 file changed, 91 insertions(+), 117 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index 43453ec533b2..b82c56ba8ff7 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -1,6 +1,4 @@
 /*
- * drivers/input/serio/gscps2.c
- *
  * Copyright (c) 2004-2006 Helge Deller <deller@gmx.de>
  * Copyright (c) 2002 Laurent Canet <canetl@esiee.fr>
  * Copyright (c) 2002 Thibaut Varene <varenet@parisc-linux.org>
@@ -37,16 +35,10 @@ MODULE_AUTHOR("Laurent Canet <canetl@esiee.fr>, Thibaut Varene <varenet@parisc-l
 MODULE_DESCRIPTION("HP GSC PS2 port driver");
 MODULE_LICENSE("GPL");
 
-#define PFX "gscps2.c: "
-
 /*
  * Driver constants
  */
 
-/* various constants */
-#define ENABLE			1
-#define DISABLE			0
-
 #define GSC_DINO_OFFSET		0x0800	/* offset for DINO controller versus LASI one */
 
 /* PS/2 IO port offsets */
@@ -77,23 +69,21 @@ MODULE_LICENSE("GPL");
 #define GSC_ID_KEYBOARD		0	/* device ID values */
 #define GSC_ID_MOUSE		1
 
-#ifndef CONFIG_SERIO_GSCPS2_RDI_KEYCODES
-# define CONFLICT(x, y) x
-#else
-# define CONFLICT(x, y) y
-#endif
+#define RDI_CONFLICT(x, y) \
+	((x) * !IS_ENABLED(CONFIG_SERIO_GSCPS2_RDI_KEYCODES) + \
+	 (y) * IS_ENABLED(CONFIG_SERIO_GSCPS2_RDI_KEYCODES))
 
 /*
  * Sadly RDI (Tadpole) decided to ship a different keyboard layout
  * than HP for their PS/2 laptop keyboard which leads to conflicting
  * keycodes between a normal HP PS/2 keyboard and a RDI PrecisionBook.
- *                       HP:            RDI:
+ *                           HP:                RDI:
  */
-#define C_07	CONFLICT(KEY_F12,	KEY_F1)
-#define C_11	CONFLICT(KEY_LEFTALT,	KEY_LEFTCTRL)
-#define C_14	CONFLICT(KEY_LEFTCTRL,	KEY_CAPSLOCK)
-#define C_58	CONFLICT(KEY_CAPSLOCK,	KEY_RIGHTCTRL)
-#define C_61	CONFLICT(KEY_102ND,	KEY_LEFT)
+#define C_07	RDI_CONFLICT(KEY_F12,		KEY_F1)
+#define C_11	RDI_CONFLICT(KEY_LEFTALT,	KEY_LEFTCTRL)
+#define C_14	RDI_CONFLICT(KEY_LEFTCTRL,	KEY_CAPSLOCK)
+#define C_58	RDI_CONFLICT(KEY_CAPSLOCK,	KEY_RIGHTCTRL)
+#define C_61	RDI_CONFLICT(KEY_102ND,		KEY_LEFT)
 
 /*
  * Special keycode value recognized by atkbd (ATKBD_KEY_NULL) to silently
@@ -188,8 +178,6 @@ static const struct software_node gscps2_keyboard_node = {
 	.properties = gscps2_props,
 };
 
-static irqreturn_t gscps2_interrupt(int irq, void *dev);
-
 #define BUFFER_SIZE 0x0f
 
 /* GSC PS/2 port device struct */
@@ -197,33 +185,34 @@ struct gscps2port {
 	struct list_head node;
 	struct parisc_device *padev;
 	struct serio *port;
-	spinlock_t lock;
+	spinlock_t lock; /* serializes HW access */
 	char __iomem *addr;
 	u8 act, append; /* position in buffer[] */
 	struct {
 		u8 data;
 		u8 str;
-	} buffer[BUFFER_SIZE+1];
+	} buffer[BUFFER_SIZE + 1];
 	int id;
 };
 
+static LIST_HEAD(ps2port_list);
+
 /*
  * Various HW level routines
  */
 
-#define gscps2_readb_input(x)		readb((x)+GSC_RCVDATA)
-#define gscps2_readb_control(x)		readb((x)+GSC_CONTROL)
-#define gscps2_readb_status(x)		readb((x)+GSC_STATUS)
-#define gscps2_writeb_control(x, y)	writeb((x), (y)+GSC_CONTROL)
-
+#define gscps2_readb_input(x)		readb((x) + GSC_RCVDATA)
+#define gscps2_readb_control(x)		readb((x) + GSC_CONTROL)
+#define gscps2_readb_status(x)		readb((x) + GSC_STATUS)
+#define gscps2_writeb_control(x, y)	writeb((x), (y) + GSC_CONTROL)
 
 /*
  * wait_TBE() - wait for Transmit Buffer Empty
  */
-
 static int wait_TBE(char __iomem *addr)
 {
 	int timeout = 25000; /* device is expected to react within 250 msec */
+
 	while (gscps2_readb_status(addr) & GSC_STAT_TBNE) {
 		if (!--timeout)
 			return 0;	/* This should not happen */
@@ -232,88 +221,17 @@ static int wait_TBE(char __iomem *addr)
 	return 1;
 }
 
-
 /*
  * gscps2_flush() - flush the receive buffer
  */
-
 static void gscps2_flush(struct gscps2port *ps2port)
 {
 	while (gscps2_readb_status(ps2port->addr) & GSC_STAT_RBNE)
 		gscps2_readb_input(ps2port->addr);
-	ps2port->act = ps2port->append = 0;
-}
-
-/*
- * gscps2_writeb_output() - write a byte to the port
- *
- * returns 1 on success, 0 on error
- */
-
-static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
-{
-	char __iomem *addr = ps2port->addr;
-
-	if (!wait_TBE(addr)) {
-		printk(KERN_DEBUG PFX "timeout - could not write byte %#x\n", data);
-		return 0;
-	}
-
-	while (gscps2_readb_status(addr) & GSC_STAT_RBNE)
-		/* wait */;
-
-	scoped_guard(spinlock_irqsave, &ps2port->lock)
-		writeb(data, addr+GSC_XMTDATA);
-
-	/* this is ugly, but due to timing of the port it seems to be necessary. */
-	mdelay(6);
-
-	/* make sure any received data is returned as fast as possible */
-	/* this is important e.g. when we set the LEDs on the keyboard */
-	gscps2_interrupt(0, NULL);
-
-	return 1;
-}
-
-
-/*
- * gscps2_enable() - enables or disables the port
- */
-
-static void gscps2_enable(struct gscps2port *ps2port, int enable)
-{
-	u8 data;
-
-	/* now enable/disable the port */
-	scoped_guard(spinlock_irqsave, &ps2port->lock) {
-		gscps2_flush(ps2port);
-		data = gscps2_readb_control(ps2port->addr);
-		if (enable)
-			data |= GSC_CTRL_ENBL;
-		else
-			data &= ~GSC_CTRL_ENBL;
-		gscps2_writeb_control(data, ps2port->addr);
-	}
-
-	wait_TBE(ps2port->addr);
-	gscps2_flush(ps2port);
-}
-
-/*
- * gscps2_reset() - resets the PS/2 port
- */
-
-static void gscps2_reset(struct gscps2port *ps2port)
-{
-	/* reset the interface */
-	guard(spinlock_irqsave)(&ps2port->lock);
-	gscps2_flush(ps2port);
-	writeb(0xff, ps2port->addr + GSC_RESET);
-	gscps2_flush(ps2port);
+	ps2port->act = 0;
+	ps2port->append = 0;
 }
 
-static LIST_HEAD(ps2port_list);
-
 static void gscps2_read_data(struct gscps2port *ps2port)
 {
 	u8 status;
@@ -348,8 +266,8 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
 		data   = ps2port->buffer[ps2port->act].data;
 
 		ps2port->act = (ps2port->act + 1) & BUFFER_SIZE;
-		rxflags = ((status & GSC_STAT_TERR) ? SERIO_TIMEOUT : 0 ) |
-			  ((status & GSC_STAT_PERR) ? SERIO_PARITY  : 0 );
+		rxflags = ((status & GSC_STAT_TERR) ? SERIO_TIMEOUT : 0) |
+			  ((status & GSC_STAT_PERR) ? SERIO_PARITY  : 0);
 
 		serio_interrupt(ps2port->port, data, rxflags);
 	}
@@ -370,7 +288,6 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
  * the data as fast as possible and handle the reporting to the upper layer
  * later.
  */
-
 static irqreturn_t gscps2_interrupt(int irq, void *dev)
 {
 	struct gscps2port *ps2port;
@@ -392,17 +309,79 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+/*
+ * gscps2_writeb_output() - write a byte to the port
+ *
+ * returns 1 on success, 0 on error
+ */
+static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
+{
+	char __iomem *addr = ps2port->addr;
+
+	if (!wait_TBE(addr)) {
+		dev_dbg(&ps2port->padev->dev, "timeout - could not write byte %#x\n", data);
+		return 0;
+	}
+
+	while (gscps2_readb_status(addr) & GSC_STAT_RBNE)
+		cpu_relax();
+
+	scoped_guard(spinlock_irqsave, &ps2port->lock)
+		writeb(data, addr + GSC_XMTDATA);
+
+	/* this is ugly, but due to timing of the port it seems to be necessary. */
+	mdelay(6);
+
+	/* make sure any received data is returned as fast as possible */
+	/* this is important e.g. when we set the LEDs on the keyboard */
+	gscps2_interrupt(0, NULL);
+
+	return 1;
+}
 
 /*
- * gscps2_write() - send a byte out through the aux interface.
+ * gscps2_enable() - enables or disables the port
  */
+static void gscps2_enable(struct gscps2port *ps2port, bool enable)
+{
+	u8 data;
 
+	/* now enable/disable the port */
+	scoped_guard(spinlock_irqsave, &ps2port->lock) {
+		gscps2_flush(ps2port);
+		data = gscps2_readb_control(ps2port->addr);
+		if (enable)
+			data |= GSC_CTRL_ENBL;
+		else
+			data &= ~GSC_CTRL_ENBL;
+		gscps2_writeb_control(data, ps2port->addr);
+	}
+
+	wait_TBE(ps2port->addr);
+	gscps2_flush(ps2port);
+}
+
+/*
+ * gscps2_reset() - resets the PS/2 port
+ */
+static void gscps2_reset(struct gscps2port *ps2port)
+{
+	/* reset the interface */
+	guard(spinlock_irqsave)(&ps2port->lock);
+	gscps2_flush(ps2port);
+	writeb(0xff, ps2port->addr + GSC_RESET);
+	gscps2_flush(ps2port);
+}
+
+/*
+ * gscps2_write() - send a byte out through the aux interface.
+ */
 static int gscps2_write(struct serio *port, unsigned char data)
 {
 	struct gscps2port *ps2port = port->port_data;
 
 	if (!gscps2_writeb_output(ps2port, data)) {
-		printk(KERN_DEBUG PFX "sending byte %#x failed.\n", data);
+		dev_dbg(&ps2port->padev->dev, "sending byte %#x failed.\n", data);
 		return -1;
 	}
 	return 0;
@@ -412,7 +391,6 @@ static int gscps2_write(struct serio *port, unsigned char data)
  * gscps2_open() is called when a port is opened by the higher layer.
  * It resets and enables the port.
  */
-
 static int gscps2_open(struct serio *port)
 {
 	struct gscps2port *ps2port = port->port_data;
@@ -420,7 +398,7 @@ static int gscps2_open(struct serio *port)
 	gscps2_reset(ps2port);
 
 	/* enable it */
-	gscps2_enable(ps2port, ENABLE);
+	gscps2_enable(ps2port, true);
 
 	gscps2_interrupt(0, NULL);
 
@@ -430,11 +408,11 @@ static int gscps2_open(struct serio *port)
 /*
  * gscps2_close() disables the port
  */
-
 static void gscps2_close(struct serio *port)
 {
 	struct gscps2port *ps2port = port->port_data;
-	gscps2_enable(ps2port, DISABLE);
+
+	gscps2_enable(ps2port, false);
 }
 
 /**
@@ -443,7 +421,6 @@ static void gscps2_close(struct serio *port)
  *
  * @return: success/error report
  */
-
 static int __init gscps2_probe(struct parisc_device *dev)
 {
 	struct gscps2port *ps2port;
@@ -494,8 +471,8 @@ static int __init gscps2_probe(struct parisc_device *dev)
 		goto fail_miserably;
 
 	if (ps2port->id != GSC_ID_KEYBOARD && ps2port->id != GSC_ID_MOUSE) {
-		printk(KERN_WARNING PFX "Unsupported PS/2 port at 0x%08lx (id=%d) ignored\n",
-				hpa, ps2port->id);
+		dev_warn(&dev->dev, "Unsupported PS/2 port at 0x%08lx (id=%d) ignored\n",
+			 hpa, ps2port->id);
 		ret = -ENODEV;
 		goto fail;
 	}
@@ -552,7 +529,6 @@ static int __init gscps2_probe(struct parisc_device *dev)
  *
  * @return: success/error report
  */
-
 static void __exit gscps2_remove(struct parisc_device *dev)
 {
 	struct gscps2port *ps2port = dev_get_drvdata(&dev->dev);
@@ -572,7 +548,6 @@ static void __exit gscps2_remove(struct parisc_device *dev)
 	kfree(ps2port);
 }
 
-
 static const struct parisc_device_id gscps2_device_tbl[] __initconst = {
 	{ HPHW_FIO, HVERSION_REV_ANY_ID, HVERSION_ANY_ID, 0x00084 }, /* LASI PS/2 */
 #ifdef DINO_TESTED
@@ -610,6 +585,5 @@ static void __exit gscps2_exit(void)
 	software_node_unregister(&gscps2_keyboard_node);
 }
 
-
 module_init(gscps2_init);
 module_exit(gscps2_exit);

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 2/7] Input: gscps2 - use RCU for ps2port_list and manage it in open/close

[Dmitry Torokhov]

Managing ps2port_list in gscps2_probe() and gscps2_remove() had two
issues:

- in gscps2_remove(), serio_unregister_port() frees the serio port, but
  because the port remained in ps2port_list until later in remove, a
  shared interrupt firing on another CPU could traverse ps2port_list and
  dereference the freed serio port

- ps2port_list additions and deletions in probe/remove raced locklessly
  against list traversals in gscps2_interrupt().

Convert ps2port_list traversal in gscps2_interrupt() to use RCU, and
move list management to gscps2_open() and gscps2_close(). When
serio_unregister_port() runs during device removal, serio_close() is
invoked, cleanly taking the port out of ps2port_list before the serio
structure is destroyed, while maintaining active hardware communication
during child driver disconnect.

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index b82c56ba8ff7..5b6e311f8a02 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -26,6 +26,7 @@
 #include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/property.h>
+#include <linux/rcupdate.h>
 #include <linux/serio.h>
 
 #include <asm/irq.h>
@@ -195,6 +196,7 @@ struct gscps2port {
 	int id;
 };
 
+static DEFINE_SPINLOCK(ps2port_list_lock);
 static LIST_HEAD(ps2port_list);
 
 /*
@@ -292,14 +294,16 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
 {
 	struct gscps2port *ps2port;
 
-	list_for_each_entry(ps2port, &ps2port_list, node) {
+	guard(rcu)();
+
+	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
 		guard(spinlock_irqsave)(&ps2port->lock);
 
 		gscps2_read_data(ps2port);
-	} /* list_for_each_entry */
+	}
 
 	/* all data was read from the ports - now report the data to upper layer */
-	list_for_each_entry(ps2port, &ps2port_list, node) {
+	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
 		if (gscps2_report_data(ps2port)) {
 			/* More data ready - break early to restart interrupt */
 			break;
@@ -397,6 +401,9 @@ static int gscps2_open(struct serio *port)
 
 	gscps2_reset(ps2port);
 
+	scoped_guard(spinlock_irqsave, &ps2port_list_lock)
+		list_add_tail_rcu(&ps2port->node, &ps2port_list);
+
 	/* enable it */
 	gscps2_enable(ps2port, true);
 
@@ -413,6 +420,11 @@ static void gscps2_close(struct serio *port)
 	struct gscps2port *ps2port = port->port_data;
 
 	gscps2_enable(ps2port, false);
+
+	scoped_guard(spinlock_irqsave, &ps2port_list_lock)
+		list_del_rcu(&ps2port->node);
+
+	synchronize_rcu();
 }
 
 /**
@@ -446,6 +458,7 @@ static int __init gscps2_probe(struct parisc_device *dev)
 
 	ps2port->port = serio;
 	ps2port->padev = dev;
+	INIT_LIST_HEAD(&ps2port->node);
 	ps2port->addr = ioremap(hpa, GSC_STATUS + 4);
 	if (!ps2port->addr) {
 		ret = -ENOMEM;
@@ -501,8 +514,6 @@ static int __init gscps2_probe(struct parisc_device *dev)
 
 	serio_register_port(ps2port->port);
 
-	list_add_tail(&ps2port->node, &ps2port_list);
-
 	return 0;
 
 fail:
@@ -539,12 +550,10 @@ static void __exit gscps2_remove(struct parisc_device *dev)
 	serio_unregister_port(ps2port->port);
 	free_irq(dev->irq, ps2port);
 	gscps2_flush(ps2port);
-	list_del(&ps2port->node);
 	iounmap(ps2port->addr);
 #if 0
 	release_mem_region(dev->hpa, GSC_STATUS + 4);
 #endif
-	dev_set_drvdata(&dev->dev, NULL);
 	kfree(ps2port);
 }
 

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 3/7] Input: gscps2 - protect buffer access in read and report helpers

[Dmitry Torokhov]

In gscps2_report_data(), the ring buffer consumer index ps2port->act was
read and updated locklessly. When gscps2_interrupt() was called from
process context (such as during port write or open) concurrently with a
hardware interrupt running on another CPU, two execution contexts could
execute gscps2_report_data() simultaneously for the same port, racing on
ps2port->act and leading to duplicate, skipped, or out-of-order bytes.

Protect buffer access by taking ps2port->lock inside gscps2_read_data()
and gscps2_report_data(). In gscps2_report_data(), acquire ps2port->lock
only when popping entries from the ring buffer and release it before
calling serio_interrupt() to avoid recursive deadlocks if the input
driver synchronously sends a command back via serio_write().

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index 5b6e311f8a02..fef6fffb6f86 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -238,6 +238,8 @@ static void gscps2_read_data(struct gscps2port *ps2port)
 {
 	u8 status;
 
+	guard(spinlock_irqsave)(&ps2port->lock);
+
 	do {
 		status = gscps2_readb_status(ps2port->addr);
 		if (!(status & GSC_STAT_RBNE))
@@ -255,7 +257,7 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
 	unsigned int rxflags;
 	u8 data, status;
 
-	while (ps2port->act != ps2port->append) {
+	while (true) {
 		/*
 		 * Did new data arrived while we read existing data ?
 		 * If yes, exit now and let the new irq handler start
@@ -264,17 +266,20 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
 		if (gscps2_readb_status(ps2port->addr) & GSC_STAT_CMPINTR)
 			return true;
 
-		status = ps2port->buffer[ps2port->act].str;
-		data   = ps2port->buffer[ps2port->act].data;
+		scoped_guard(spinlock_irqsave, &ps2port->lock) {
+			if (ps2port->act == ps2port->append)
+				return false;
+
+			status = ps2port->buffer[ps2port->act].str;
+			data   = ps2port->buffer[ps2port->act].data;
+			ps2port->act = (ps2port->act + 1) & BUFFER_SIZE;
+		}
 
-		ps2port->act = (ps2port->act + 1) & BUFFER_SIZE;
 		rxflags = ((status & GSC_STAT_TERR) ? SERIO_TIMEOUT : 0) |
 			  ((status & GSC_STAT_PERR) ? SERIO_PARITY  : 0);
 
 		serio_interrupt(ps2port->port, data, rxflags);
 	}
-
-	return false;
 }
 
 /**
@@ -296,11 +301,8 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
 
 	guard(rcu)();
 
-	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
-		guard(spinlock_irqsave)(&ps2port->lock);
-
+	list_for_each_entry_rcu(ps2port, &ps2port_list, node)
 		gscps2_read_data(ps2port);
-	}
 
 	/* all data was read from the ports - now report the data to upper layer */
 	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 4/7] Input: gscps2 - serialize hardware and buffer access in gscps2_flush()

[Dmitry Torokhov]

gscps2_flush() reads from hardware registers and resets the ring buffer
indices ps2port->act and ps2port->append. In gscps2_enable(), the
trailing gscps2_flush() was called without holding ps2port->lock, racing
with concurrent hardware interrupts and buffer access.

Assert that ps2port->lock is held in gscps2_flush() with
lockdep_assert_held(), and ensure all callers acquire ps2port->lock so
that multi-step hardware sequences remain fully serialized without
unprotected windows.

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index fef6fffb6f86..36c25db9ff7f 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -228,6 +228,8 @@ static int wait_TBE(char __iomem *addr)
  */
 static void gscps2_flush(struct gscps2port *ps2port)
 {
+	lockdep_assert_held(&ps2port->lock);
+
 	while (gscps2_readb_status(ps2port->addr) & GSC_STAT_RBNE)
 		gscps2_readb_input(ps2port->addr);
 	ps2port->act = 0;
@@ -364,7 +366,8 @@ static void gscps2_enable(struct gscps2port *ps2port, bool enable)
 	}
 
 	wait_TBE(ps2port->addr);
-	gscps2_flush(ps2port);
+	scoped_guard(spinlock_irqsave, &ps2port->lock)
+		gscps2_flush(ps2port);
 }
 
 /*
@@ -551,7 +554,8 @@ static void __exit gscps2_remove(struct parisc_device *dev)
 
 	serio_unregister_port(ps2port->port);
 	free_irq(dev->irq, ps2port);
-	gscps2_flush(ps2port);
+	scoped_guard(spinlock_irqsave, &ps2port->lock)
+		gscps2_flush(ps2port);
 	iounmap(ps2port->addr);
 #if 0
 	release_mem_region(dev->hpa, GSC_STATUS + 4);

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 5/7] Input: gscps2 - return IRQ_NONE when interrupt is not handled

[Dmitry Torokhov]

gscps2_interrupt() is registered with IRQF_SHARED. Unconditionally
returning IRQ_HANDLED when no data was pending on any GSC PS/2 port
masks unhandled interrupts on the shared interrupt line and prevents the
kernel core spurious interrupt detector from identifying runaway
interrupt storms.

Have gscps2_read_data() return whether any bytes were read, accumulate
the handled status in gscps2_interrupt(), and return IRQ_RETVAL(handled).

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index 36c25db9ff7f..907fb1537595 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -236,8 +236,9 @@ static void gscps2_flush(struct gscps2port *ps2port)
 	ps2port->append = 0;
 }
 
-static void gscps2_read_data(struct gscps2port *ps2port)
+static bool gscps2_read_data(struct gscps2port *ps2port)
 {
+	bool read_any = false;
 	u8 status;
 
 	guard(spinlock_irqsave)(&ps2port->lock);
@@ -247,11 +248,14 @@ static void gscps2_read_data(struct gscps2port *ps2port)
 		if (!(status & GSC_STAT_RBNE))
 			break;
 
+		read_any = true;
 		ps2port->buffer[ps2port->append].str = status;
 		ps2port->buffer[ps2port->append].data =
 				gscps2_readb_input(ps2port->addr);
 		ps2port->append = (ps2port->append + 1) & BUFFER_SIZE;
 	} while (true);
+
+	return read_any;
 }
 
 static bool gscps2_report_data(struct gscps2port *ps2port)
@@ -300,11 +304,14 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
 static irqreturn_t gscps2_interrupt(int irq, void *dev)
 {
 	struct gscps2port *ps2port;
+	bool handled = false;
 
 	guard(rcu)();
 
-	list_for_each_entry_rcu(ps2port, &ps2port_list, node)
-		gscps2_read_data(ps2port);
+	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
+		if (gscps2_read_data(ps2port))
+			handled = true;
+	}
 
 	/* all data was read from the ports - now report the data to upper layer */
 	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
@@ -314,7 +321,7 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
 		}
 	}
 
-	return IRQ_HANDLED;
+	return IRQ_RETVAL(handled);
 }
 
 /*

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 6/7] Input: gscps2 - serialize concurrent interrupt handlers

[Dmitry Torokhov]

gscps2_interrupt() may be invoked concurrently from a hardware interrupt
on one CPU and from process context via gscps2_writeb_output() on another
CPU. In gscps2_report_data(), ps2port->lock is released before calling
serio_interrupt() to avoid recursive deadlocks. However, if two execution
contexts run gscps2_report_data() concurrently for the same port, they
could race to acquire serio->lock inside serio_interrupt(), potentially
delivering multi-byte scancodes out of order.

Serialize execution of gscps2_interrupt() using gscps2_interrupt_lock
with ACQUIRE(spinlock_irqsave_try). Using spin_trylock prevents
overlapping executions and guarantees in-order packet delivery without
risking recursive deadlocks if an input driver synchronously sends a
command back via serio_write(). If the lock cannot be acquired, return
IRQ_NONE to preserve spurious interrupt detection on shared IRQ lines.

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index 907fb1537595..2afd53a163ff 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -288,6 +288,8 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
 	}
 }
 
+static DEFINE_SPINLOCK(gscps2_interrupt_lock);
+
 /**
  * gscps2_interrupt() - Interruption service routine
  * @irq: interrupt number which triggered (unused)
@@ -305,21 +307,31 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
 {
 	struct gscps2port *ps2port;
 	bool handled = false;
+	bool more_data;
+
+	ACQUIRE(spinlock_irqsave_try, lock)(&gscps2_interrupt_lock);
+	if (ACQUIRE_ERR(spinlock_irqsave_try, &lock))
+		return IRQ_NONE;
 
 	guard(rcu)();
 
-	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
-		if (gscps2_read_data(ps2port))
-			handled = true;
-	}
+	do {
+		more_data = false;
 
-	/* all data was read from the ports - now report the data to upper layer */
-	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
-		if (gscps2_report_data(ps2port)) {
-			/* More data ready - break early to restart interrupt */
-			break;
+		list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
+			if (gscps2_read_data(ps2port))
+				handled = true;
 		}
-	}
+
+		/* all data was read from the ports - now report the data to upper layer */
+		list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
+			if (gscps2_report_data(ps2port)) {
+				/* More data ready - restart loop to read new data */
+				more_data = true;
+				break;
+			}
+		}
+	} while (more_data);
 
 	return IRQ_RETVAL(handled);
 }

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 7/7] Input: gscps2 - drop busy-wait and manual interrupt pump on transmit

[Dmitry Torokhov]

In gscps2_writeb_output(), after writing data to GSC_XMTDATA, the driver
explicitly executed mdelay(6) and manually called gscps2_interrupt() as a
polling mechanism to accelerate command responses (such as keyboard ACK
or LED updates).

On PA-RISC, the PS/2 controller asserts a level interrupt to the system
ASIC whenever received data arrives in hardware, and the input/serio
subsystem handles command responses asynchronously via completions.
Busy-waiting for 6 ms on every transmitted byte introduces significant
unnecessary latency during multi-byte command sequences and complicates
interrupt handler locking.

Remove mdelay(6) and the manual invocation of gscps2_interrupt() from
gscps2_writeb_output(), relying on normal hardware interrupt delivery.

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index 2afd53a163ff..985539f1d088 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -356,13 +356,6 @@ static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
 	scoped_guard(spinlock_irqsave, &ps2port->lock)
 		writeb(data, addr + GSC_XMTDATA);
 
-	/* this is ugly, but due to timing of the port it seems to be necessary. */
-	mdelay(6);
-
-	/* make sure any received data is returned as fast as possible */
-	/* this is important e.g. when we set the LEDs on the keyboard */
-	gscps2_interrupt(0, NULL);
-
 	return 1;
 }
 

-- 
2.55.0.897.gb25b4bd76c-goog

Re: [PATCH 4/7] Input: gscps2 - serialize hardware and buffer access in gscps2_flush()

[sashiko-bot]

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

Pre-existing issues:
- [High] gscps2_writeb_output spins infinitely if the hardware receive buffer becomes full while executing in a context where IRQs are disabled.
--

commit 98b3bfa0449cf3b48806ab726d6170adf6e0246e
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

    Input: gscps2 - serialize hardware and buffer access in gscps2_flush()
    
    gscps2_flush() reads from hardware registers and resets the ring buffer
    indices ps2port->act and ps2port->append. In gscps2_enable(), the
    trailing gscps2_flush() was called without holding ps2port->lock, racing
    with concurrent hardware interrupts and buffer access.
    
    Assert that ps2port->lock is held in gscps2_flush() with
    lockdep_assert_held(), and ensure all callers acquire ps2port->lock so
    that multi-step hardware sequences remain fully serialized without
    unprotected windows.

> diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
> index fef6fffb6f865..36c25db9ff7f6 100644
> --- a/drivers/input/serio/gscps2.c
> +++ b/drivers/input/serio/gscps2.c
[ ... ]
> @@ -364,7 +366,8 @@ static void gscps2_enable(struct gscps2port *ps2port, bool enable)

[Severity: High]
This is a pre-existing issue, but can the loop in the preceding function
gscps2_writeb_output() spin infinitely and cause a hard lockup?

Looking at gscps2_writeb_output():

	while (gscps2_readb_status(addr) & GSC_STAT_RBNE)
		cpu_relax();

This lockless loop waits for a status flag to clear without a timeout or
fallback wake mechanism. If a byte arrives from the PS/2 port (setting
GSC_STAT_RBNE) while gscps2_writeb_output() is executing with local
interrupts disabled, the gscps2_interrupt handler cannot run on the same CPU
to drain the buffer.

Could this spin forever if triggered by normal hardware interaction, such as
typing on the keyboard while the system sends a command?

>  	}
>  
>  	wait_TBE(ps2port->addr);
> -	gscps2_flush(ps2port);
> +	scoped_guard(spinlock_irqsave, &ps2port->lock)
> +		gscps2_flush(ps2port);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830-gscps2-v1-0-c733d4cae7f9@gmail.com?part=4

Re: [PATCH 1/7] Input: gscps2 - clean up driver code style and structure

[sashiko-bot]

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

Pre-existing issues:
- [High] The busy-wait loop in `gscps2_writeb_output` polling the hardware status register lacks a timeout, creating a hard lockup (deadlock) risk when invoked with local interrupts disabled.
--

commit edfe57337208b53162446f9db3e046a15ffe59af
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: gscps2 - clean up driver code style and structure

This commit cleans up code style and function ordering in the gscps2 driver.
It updates macro definitions, replaces printk with dev_dbg, and introduces
cpu_relax() into the spin-wait loops.

> diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
> index 43453ec533b22..b82c56ba8ff70 100644
> --- a/drivers/input/serio/gscps2.c
> +++ b/drivers/input/serio/gscps2.c

[ ... ]

> +/*
> + * gscps2_writeb_output() - write a byte to the port
> + *
> + * returns 1 on success, 0 on error
> + */
> +static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
> +{
> +	char __iomem *addr = ps2port->addr;
> +
> +	if (!wait_TBE(addr)) {
> +		dev_dbg(&ps2port->padev->dev, "timeout - could not write byte %#x\n", data);
> +		return 0;
> +	}
> +
> +	while (gscps2_readb_status(addr) & GSC_STAT_RBNE)
> +		cpu_relax();

[Severity: High]
This is a pre-existing issue, but does this busy-wait loop risk a hard CPU
lockup when invoked with local interrupts disabled?

A PS/2 driver like atkbd can call serio_write() from an interrupt context
with local IRQs disabled (for example, in atkbd_interrupt sending
ATKBD_CMD_RESEND in response to a parity error).

If a new byte arrives in the receive buffer concurrently, the GSC_STAT_RBNE
bit is set. Since local IRQs are disabled, the hardware interrupt handler
cannot run to read the pending byte. This means GSC_STAT_RBNE will never
clear and the CPU could hang indefinitely waiting for the buffer to empty.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830-gscps2-v1-0-c733d4cae7f9@gmail.com?part=1

Re: [PATCH 5/7] Input: gscps2 - return IRQ_NONE when interrupt is not handled

[sashiko-bot]

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

Pre-existing issues:
- [High] Unbounded hardware status polling loop can cause a hard CPU lockup
--

commit 9f561e93b317449afad0b8c42cee86d0e78216d9
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: gscps2 - return IRQ_NONE when interrupt is not handled

This commit modifies the GSC PS/2 driver to track whether any bytes were
handled during an interrupt and returns the appropriate IRQ status. This
prevents masking unhandled interrupts on a shared line.

> diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
> index 36c25db9ff7f6..907fb1537595c 100644
> --- a/drivers/input/serio/gscps2.c
> +++ b/drivers/input/serio/gscps2.c
[ ... ]
> @@ -247,11 +248,14 @@ static void gscps2_read_data(struct gscps2port *ps2port)
>  		if (!(status & GSC_STAT_RBNE))
>  			break;
>  
> +		read_any = true;
>  		ps2port->buffer[ps2port->append].str = status;
>  		ps2port->buffer[ps2port->append].data =
>  				gscps2_readb_input(ps2port->addr);
>  		ps2port->append = (ps2port->append + 1) & BUFFER_SIZE;
>  	} while (true);

[Severity: High]
This is a pre-existing issue, but could this unbounded hardware polling loop
cause a hard CPU lockup?

If the PS/2 controller malfunctions and fails to clear the GSC_STAT_RBNE bit
after a read, this loop will never exit. Since gscps2_interrupt runs with
local interrupts disabled, this would freeze the system.

Could we add a bounded iteration guard or retry limit here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830-gscps2-v1-0-c733d4cae7f9@gmail.com?part=5

Re: [PATCH 7/7] Input: gscps2 - drop busy-wait and manual interrupt pump on transmit

[sashiko-bot]

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

Pre-existing issues:
- [High] Infinite spin-loop in `gscps2_writeb_output()` waiting for receive buffer to clear deadlocks the CPU if called from atomic/interrupt context.
- [High] Level-triggered interrupt storm leading to permanent IRQ disable due to `trylock` failure in `gscps2_interrupt()`.
--

commit d741b31bf7785974793010391810ed95e0a48648
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

    Input: gscps2 - drop busy-wait and manual interrupt pump on transmit
    
    In gscps2_writeb_output(), after writing data to GSC_XMTDATA, the driver
    explicitly executed mdelay(6) and manually called gscps2_interrupt() as a
    polling mechanism to accelerate command responses (such as keyboard ACK
    or LED updates).
    
    On PA-RISC, the PS/2 controller asserts a level interrupt to the system
    ASIC whenever received data arrives in hardware, and the input/serio
    subsystem handles command responses asynchronously via completions.

> diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
> index 2afd53a163ffa..985539f1d0886 100644
> --- a/drivers/input/serio/gscps2.c
> +++ b/drivers/input/serio/gscps2.c
> @@ -356,13 +356,6 @@ static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
>  	scoped_guard(spinlock_irqsave, &ps2port->lock)
>  		writeb(data, addr + GSC_XMTDATA);
>  

[Severity: High]
This is a pre-existing issue, but I noticed an infinite spin-loop earlier in
this function:

drivers/input/serio/gscps2.c:gscps2_writeb_output() {
...
	while (gscps2_readb_status(addr) & GSC_STAT_RBNE)
		cpu_relax();
...
}

Could this loop spin indefinitely and deadlock the CPU?

If a hardware parity or frame error triggers atkbd_handle_frame_error(), it
calls serio_write(RESEND) synchronously from within the gscps2_interrupt()
handler's report loop.

If a new PS/2 byte arrives precisely before the write, GSC_STAT_RBNE is
set, and gscps2_writeb_output() will loop here indefinitely waiting for it to
clear.

Because the only code to drain the hardware buffer (gscps2_read_data()) is
blocked on the same thread higher up the call stack, and external interrupts
cannot run because local IRQs are disabled, would this cause a hard deadlock?

> -	/* this is ugly, but due to timing of the port it seems to be necessary. */
> -	mdelay(6);
> -
> -	/* make sure any received data is returned as fast as possible */
> -	/* this is important e.g. when we set the LEDs on the keyboard */
> -	gscps2_interrupt(0, NULL);

[Severity: High]
This is a pre-existing issue, but since we're modifying how we handle the
interrupt pump, I noticed a potential interrupt storm in gscps2_interrupt():

drivers/input/serio/gscps2.c:gscps2_interrupt() {
...
	ACQUIRE(spinlock_irqsave_try, lock)(&gscps2_interrupt_lock);
	if (ACQUIRE_ERR(spinlock_irqsave_try, &lock))
		return IRQ_NONE;
...
}

Could this lead to a level-triggered interrupt storm that permanently disables
the IRQ line?

If CPU B receives a level-triggered hardware interrupt while CPU A holds
gscps2_interrupt_lock (for example, during gscps2_open(), which manually
invokes the ISR outside of standard IRQ serialization without masking the IRQ
controller), CPU B fails the trylock and returns IRQ_NONE.

Because it is a level interrupt, the unhandled IRQ will immediately re-fire
on CPU B upon unmasking from handle_level_irq(), looping rapidly.

Would the generic IRQ subsystem eventually see 99,900 unhandled IRQs out of
100,000, assume nobody cared, and permanently disable the IRQ line?

> -
>  	return 1;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830-gscps2-v1-0-c733d4cae7f9@gmail.com?part=7

Re: [PATCH 3/7] Input: gscps2 - protect buffer access in read and report helpers

[sashiko-bot]

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

Pre-existing issues:
- [High] Hardware polling loop in `gscps2_writeb_output()` deadlocks if called synchronously with interrupts disabled.
- [High] Unbounded IO read loop in `gscps2_read_data()` can cause hard CPU lockup.
--

commit 9b1c3aa5a5184e24692c48f162f22f58a5172f7a
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: gscps2 - protect buffer access in read and report helpers

This commit adds a spinlock to gscps2_read_data() and gscps2_report_data() to
prevent a race condition on the ring buffer consumer index. It fixes an issue
where concurrent execution could lead to duplicate, skipped, or out-of-order
bytes.

> diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
> index 5b6e311f8a02c..fef6fffb6f865 100644
> --- a/drivers/input/serio/gscps2.c
> +++ b/drivers/input/serio/gscps2.c
> @@ -238,6 +238,8 @@ static void gscps2_read_data(struct gscps2port *ps2port)
>  {
>  	u8 status;
>  
> +	guard(spinlock_irqsave)(&ps2port->lock);
> +
>  	do {
>  		status = gscps2_readb_status(ps2port->addr);
>  		if (!(status & GSC_STAT_RBNE))

[Severity: High]
This is a pre-existing issue, but can this unbounded loop risk a hard CPU
lockup?

If the MMIO space becomes unreadable due to a hardware failure, device
disconnection, or bus error, gscps2_readb_status() typically returns 0xFF.

Since 0xFF & 0x01 (GSC_STAT_RBNE) is non-zero, this exit condition never
becomes true:

    if (!(status & GSC_STAT_RBNE))
        break;

The loop body reads the input data, but since the hardware is in a failed
state, it will also return 0xFF, causing the loop to never terminate.

[ ... ]

> @@ -255,7 +257,7 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
>  	unsigned int rxflags;
>  	u8 data, status;
>  
> -	while (ps2port->act != ps2port->append) {
> +	while (true) {
>  		/*
>  		 * Did new data arrived while we read existing data ?
>  		 * If yes, exit now and let the new irq handler start
> @@ -264,17 +266,20 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
>  		if (gscps2_readb_status(ps2port->addr) & GSC_STAT_CMPINTR)
>  			return true;
>  
> -		status = ps2port->buffer[ps2port->act].str;
> -		data   = ps2port->buffer[ps2port->act].data;
> +		scoped_guard(spinlock_irqsave, &ps2port->lock) {
> +			if (ps2port->act == ps2port->append)
> +				return false;
> +
> +			status = ps2port->buffer[ps2port->act].str;
> +			data   = ps2port->buffer[ps2port->act].data;
> +			ps2port->act = (ps2port->act + 1) & BUFFER_SIZE;
> +		}
>  
> -		ps2port->act = (ps2port->act + 1) & BUFFER_SIZE;
>  		rxflags = ((status & GSC_STAT_TERR) ? SERIO_TIMEOUT : 0) |
>  			  ((status & GSC_STAT_PERR) ? SERIO_PARITY  : 0);
>  
>  		serio_interrupt(ps2port->port, data, rxflags);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for the
hardware polling loop in gscps2_writeb_output() to deadlock if called
synchronously from here?

When serio_interrupt() is called with interrupts disabled (since it is invoked
from the gscps2_interrupt() hardware IRQ handler), an input driver like atkbd
can call serio_write() synchronously.

This calls gscps2_writeb_output(), which has this loop:

    while (gscps2_readb_status(addr) & GSC_STAT_RBNE)
        cpu_relax();

If a new byte arrives from the hardware device, GSC_STAT_RBNE (Receive Buffer
Not Empty) becomes set.

The loop waits for RBNE to be cleared, but RBNE is only cleared when the
receive data register is read.

Since we are spinning inside this loop, we never read the data register, and
the IRQ handler cannot run to read it either.

>  	}
> -
> -	return false;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830-gscps2-v1-0-c733d4cae7f9@gmail.com?part=3

Re: [PATCH 6/7] Input: gscps2 - serialize concurrent interrupt handlers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning IRQ_NONE on lock contention in a shared hardware interrupt handler fails to clear the hardware interrupt, causing a spurious interrupt storm that permanently disables the IRQ line.
--

commit f58a76b080709bfb3ea6255a5211466eb9b5af11
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: gscps2 - serialize concurrent interrupt handlers

This commit serializes executions of gscps2_interrupt() using a new
gscps2_interrupt_lock. It uses a trylock and returns IRQ_NONE when the lock
cannot be acquired to prevent overlapping executions and avoid recursive
deadlocks.

> diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
> index 907fb1537595c..2afd53a163ffa 100644
> --- a/drivers/input/serio/gscps2.c
> +++ b/drivers/input/serio/gscps2.c

[ ... ]

> @@ -305,21 +307,31 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
>  {
>  	struct gscps2port *ps2port;
>  	bool handled = false;
> +	bool more_data;
> +
> +	ACQUIRE(spinlock_irqsave_try, lock)(&gscps2_interrupt_lock);
> +	if (ACQUIRE_ERR(spinlock_irqsave_try, &lock))
> +		return IRQ_NONE;

[Severity: High]
Does returning IRQ_NONE on lock contention here fail to clear the hardware
interrupt, causing a spurious interrupt storm?

If CPU A calls gscps2_interrupt() manually (e.g., from gscps2_open()) and
acquires the lock:

CPU A
gscps2_interrupt()
    ACQUIRE(spinlock_irqsave_try, lock)(&gscps2_interrupt_lock); // succeeds

And concurrently, new data arrives causing a hardware interrupt on CPU B:

CPU B (Hardware Interrupt)
gscps2_interrupt()
    ACQUIRE(spinlock_irqsave_try, lock)(&gscps2_interrupt_lock); // fails
    return IRQ_NONE;

Because the interrupt is level-triggered and the source was not cleared, won't
the interrupt controller immediately re-assert the IRQ on CPU B as soon as the
handler returns?

This could trap CPU B in a tight interrupt storm loop. If CPU A takes
significant time in serio_interrupt() (e.g., due to input subsystem lock
contention or console writes), could the storm exceed the kernel's unhandled
interrupt threshold and permanently disable the shared IRQ line?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830-gscps2-v1-0-c733d4cae7f9@gmail.com?part=6