[PATCH RFC 0/3] PCI: pciehp: Report surprise removal during safe removal

[PATCH RFC 0/3] PCI: pciehp: Report surprise removal during safe removal

[Abhin Parekadan Jose]

Bjorn asked for this to be pulled out of the dormant virtio thread and
posted separately as a purely PCI series [1]. This is that repost. It
carries one patch from Michael's RFC v5 as a dependency and drops the
virtio side entirely.

The problem, as identified by Lukas [2]: if a safe removal is already in
progress when the device is surprise removed, pciehp cannot report the
disconnect.  The removal blocks waiting on a device interrupt or status
read, and the IRQ thread is single-threaded and is itself executing that
removal, so it never runs again to report the device gone. The removal
hangs indefinitely.

Lukas noted that pciehp_isr() does run while the IRQ thread is blocked,
but argued this was not viable either, because pciehp_ist() must ignore
link and presence changes caused by SBR or DPC, and telling those apart
takes seconds which cannot be spent in hardirq.

Patch 2 sidesteps that by not doing the work in hardirq. pciehp_isr()
only checks PDS, and defers everything else to a work item running in
process context, where it is free to sleep and to repeat the spurious
link change test.

Patches:

  1/3 Michael's "PCI: Report surprise removal event" from RFC v5,
      unchanged apart from the fixing commit subject. Needed for
      disconnect_work_enable and the disconnect_work.

  2/3 The pciehp change. Adds disconnect_work to struct controller,
      scheduled from pciehp_isr() on PDC or DLLSC when
      !pciehp_card_present().  pciehp_disconnect_work() then runs in
      process context, where it re-tests for spurious link changes and
      confirms the card is still absent before scheduling the driver's
      disconnect work.

  3/3 A POC driver for the QEMU edu device that blocks in remove()
      waiting for an interrupt, standing in for del_gendisk() stuck in
      blk_mq_freeze_queue_wait().  Not for merge -- included so the
      hang can be reproduced.

Testing

Reproducing this needs QEMU changes, since neither device_del nor the
attention button produces a true surprise removal.  A branch with both
is here [3]:

  - a delayed-IRQ register on the edu device (BAR0 0x30, write N ms)
  - a pcie_surprise_del monitor command that drops the device and
    generates PDC=1, DLLSC=1, PDS=0

Test 1 (Hang in remove() on the user thread, then suprise remove):

  ./qemu-system-aarch64 -machine virt,gic-version=3   -cpu cortex-a57 \
    -m 512 -smp 2   -kernel Image  -initrd initramfs.cpio.gz  \
    -device pcie-root-port,id=rp1,chassis=1,slot=1 \
    -device edu,bus=rp1,id=edu0 -append "console=ttyAMA0 rdinit=/init" \
    -nographic  -monitor unix:/tmp/qemu-mon.sock,server,nowait

  guest# echo 1 > /sys/bus/pci/devices/0000:01:00.0/remove

  host$  echo "pcie_surprise_del edu0" | socat - unix-connect:/tmp/qemu-mon.sock

  This is the test that MST had solved.

Test 2 (Hang in remove() on the IRQ thread, then suprise remove):

  ./qemu-system-aarch64 -machine virt,gic-version=3   -cpu cortex-a57 \
    -m 512 -smp 2   -kernel Image  -initrd initramfs.cpio.gz  \
    -device pcie-root-port,id=rp1,chassis=1,slot=1 \
    -device edu,bus=rp1,id=edu0 -append "console=ttyAMA0 rdinit=/init" \
    -nographic  -monitor unix:/tmp/qemu-mon.sock,server,nowait

  guest# echo 0 > /sys/bus/pci/slots/1/power

  host$  echo "pcie_surprise_del edu0" | socat - unix-connect:/tmp/qemu-mon.sock

  This is the test I am trying to solve.

Without patch 2 the safe removal never returns. With it, pciehp_isr()
schedules ctrl->disconnect_work, which walks the bus and schedules
pdev->disconnect_work; the wait in the POC driver completes and
remove() proceeds.

Open questions

  - Is this a viable approach?

[1] https://lore.kernel.org/all/20260826194815.GA1552818@bhelgaas/
[2] https://lore.kernel.org/all/aHlZE18kPuHuDtTT@wunner.de/
[3] https://gitlab.com/abhinkop/qemu/-/commits/suprise-removal

Assisted-by: LLM

Abhin Parekadan Jose (2):
  PCI: pciehp: Report surprise removal from pciehp_isr()
  misc: Add edu_srpoc surprise removal POC driver

Michael S. Tsirkin (1):
  PCI: Report surprise removal event

 drivers/misc/Makefile            |   1 +
 drivers/misc/edu_srpoc.c         | 169 +++++++++++++++++++++++++++++++
 drivers/pci/hotplug/pciehp.h     |   1 +
 drivers/pci/hotplug/pciehp_hpc.c |  56 ++++++++--
 drivers/pci/pci.h                |  12 +++
 include/linux/pci.h              |  45 ++++++++
 6 files changed, 276 insertions(+), 8 deletions(-)
 create mode 100644 drivers/misc/edu_srpoc.c

--
2.51.1

[PATCH RFC 1/3] PCI: Report surprise removal event

[Abhin Parekadan Jose]

From: "Michael S. Tsirkin" <mst@redhat.com>

At the moment, in case of a surprise removal, the regular remove
callback is invoked, exclusively.  This works well, because mostly,
the cleanup would be the same.

However, there's a race: imagine device removal was initiated by a user
action, such as driver unbind, and it in turn initiated some cleanup
and is now waiting for an interrupt from the device. If the device is
now surprise-removed, that never arrives and the remove callback hangs
forever.

For example, this was reported for virtio-blk:

	1. the graceful removal is ongoing in the remove() callback,
	    where disk deletion del_gendisk() is ongoing, which waits
	    for the requests to complete,

	2. Now few requests are yet to complete, and surprise removal
	    started.

	    At this point, virtio block driver will not get notified by
	   the driver core layer, because it is likely serializing
	   remove() happening by +user/driver unload and PCI hotplug
	   driver-initiated device removal.  So vblk driver doesn't
	   know that device is removed, block layer is waiting for
	   requests completions to arrive which it never gets.
	   So del_gendisk() gets stuck.

Drivers can artificially add timeouts to handle that, but it can be
flaky.

Instead, let's add a way for the driver to be notified about the
disconnect. It can then do any necessary cleanup, knowing that
the device is inactive.

Since cleanups can take a long time, this takes an approach of a work
struct that the driver initiates and enables on probe, and tears down on
remove.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Link: https://lore.kernel.org/all/fba3d235e38c1c6fcef2a30ed083ad9e25b20fa3.1752094439.git.mst@redhat.com/
[Abhin: adapted commit message subject]
Signed-off-by: Abhin Parekadan Jose <abhinjoses@gmail.com>
---
 drivers/pci/pci.h   |  6 ++++++
 include/linux/pci.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index ba3c3fddddc2..23b1605e783a 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -805,6 +805,12 @@ static inline int pci_dev_set_disconnected(struct pci_dev *dev, void *unused)
 	pci_dev_set_io_state(dev, pci_channel_io_perm_failure);
 	pci_doe_disconnected(dev);
 
+	if (READ_ONCE(dev->disconnect_work_enable)) {
+		/* Make sure work is up to date. */
+		smp_rmb();
+		schedule_work(&dev->disconnect_work);
+	}
+
 	return 0;
 }
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d31a8d107b1e..06d43f57f509 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -592,6 +592,9 @@ struct pci_dev {
 	u8 reset_methods[PCI_NUM_RESET_METHODS]; /* In priority order */
 
 	struct gpio_desc *wake;		/* WAKE# GPIO */
+	/* Report disconnect events. 0x0 - disable, 0x1 - enable */
+	u8 disconnect_work_enable;
+	struct work_struct disconnect_work;
 
 #ifdef CONFIG_PCIE_TPH
 	u16		tph_cap;	/* TPH capability offset */
@@ -2123,6 +2126,48 @@ pci_release_mem_regions(struct pci_dev *pdev)
 			    pci_select_bars(pdev, IORESOURCE_MEM));
 }
 
+/*
+ * Run this first thing after getting a disconnect work, to prevent it from
+ * running multiple times.
+ * Returns: true if disconnect was enabled, proceed. false if disabled, abort.
+ */
+static inline bool pci_test_and_clear_disconnect_enable(struct pci_dev *pdev)
+{
+	u8 enable = 0x1;
+	u8 disable = 0x0;
+
+	return try_cmpxchg(&pdev->disconnect_work_enable, &enable, disable);
+}
+
+/*
+ * Caller must initialize @pdev->disconnect_work before invoking this.
+ * The work function must run and check pci_test_and_clear_disconnect_enable.
+ * Note that device can go away right after this call.
+ */
+static inline void pci_set_disconnect_work(struct pci_dev *pdev)
+{
+	/* Make sure WQ has been initialized already */
+	smp_wmb();
+
+	WRITE_ONCE(pdev->disconnect_work_enable, 0x1);
+
+	/* check the device did not go away meanwhile. */
+	mb();
+
+	if (!pci_device_is_present(pdev))
+		schedule_work(&pdev->disconnect_work);
+}
+
+static inline void pci_clear_disconnect_work(struct pci_dev *pdev)
+{
+	WRITE_ONCE(pdev->disconnect_work_enable, 0x0);
+
+	/* Make sure to stop using work from now on. */
+	smp_wmb();
+
+	cancel_work_sync(&pdev->disconnect_work);
+}
+
 bool pci_suspend_retains_context(struct pci_dev *pdev);
 
 #else /* CONFIG_PCI is not enabled */
-- 
2.51.1

[PATCH RFC 2/3] PCI: pciehp: Report surprise removal from pciehp_isr()

[Abhin Parekadan Jose]

A surprise removal during a safe removal cannot be reported: the
removal blocks waiting on a device interrupt or status read, and the
single-threaded IRQ thread is itself executing that removal, so it
cannot report that the device is gone. The removal hangs.

The hardirq handler pciehp_isr() still runs while the IRQ thread is
blocked, so it can report the disconnect. However, pciehp_ist()
deliberately ignores link and presence changes caused by a Secondary
Bus Reset or Downstream Port Containment, where the device is only
temporarily inaccessible. Distinguishing those normally requires
waiting for the SBR or DPC to conclude, which takes seconds and is not
possible in hardirq context.

Schedule a work item from pciehp_isr() when a PDC or DLLSC event
arrives and PDS indicates if the device is connected/disconnected.
This provides us a pathway to wait/block/sleep as we will not be
in pciehp_isr().

Move the scheduling of the driver's disconnect notification out of
pci_dev_set_disconnected() into schedule_notification_work() so it can
be invoked from the new work item. Factor the spurious link change
test out of pciehp_ist() into pciehp_is_spurious_link_change() so both
pciehp_ist() and pciehp_disconnect_work() can use it.

Link: https://lore.kernel.org/all/aHlZE18kPuHuDtTT@wunner.de/
Signed-off-by: Abhin Parekadan Jose <abhinjoses@gmail.com>
---
 drivers/pci/hotplug/pciehp.h     |  1 +
 drivers/pci/hotplug/pciehp_hpc.c | 56 +++++++++++++++++++++++++++-----
 drivers/pci/pci.h                |  6 ++++
 3 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index debc79b0adfb..c8ceb9320e2e 100644
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -116,6 +116,7 @@ struct controller {
 	unsigned int ist_running;
 	int request_result;
 	wait_queue_head_t requester;
+	struct work_struct disconnect_work;
 };
 
 /**
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 4c62140a3cb4..235ca8a176f1 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -620,6 +620,45 @@ static void pciehp_ignore_link_change(struct controller *ctrl,
 	up_read(&ctrl->reset_lock);
 }
 
+/*
+ * Link Down/Up events caused by Downstream Port Containment if recovery
+ * succeeded, or caused by Secondary Bus Reset, suspend to D3cold, firmware
+ * update, FPGA reconfiguration, etc. are spurious and should be ignored.
+ */
+static bool pciehp_is_spurious_link_change(struct controller *ctrl,
+						  struct pci_dev *pdev,
+						  u32 events)
+{
+	return (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) &&
+		(pci_dpc_recovered(pdev) || pci_hp_spurious_link_change(pdev)) &&
+		ctrl->state == ON_STATE;
+}
+
+/*
+ * Workaround to not wait in the isr.
+ */
+static void pciehp_disconnect_work(struct work_struct *work)
+{
+	struct pci_bus *bus;
+	struct controller *ctrl = container_of(work, struct controller,
+					       disconnect_work);
+	struct pci_dev *pdev = ctrl_dev(ctrl);
+	u32 events;
+
+	events = atomic_read(&ctrl->pending_events);
+
+	if (pciehp_is_spurious_link_change(ctrl, pdev, events))
+		return;
+
+	bus = ctrl->pcie->port->subordinate;
+
+	/* The card may have returned */
+	if (!bus || pciehp_card_present(ctrl) != 0)
+		return;
+
+	pci_walk_bus(bus, schedule_notification_work, NULL);
+}
+
 static irqreturn_t pciehp_isr(int irq, void *dev_id)
 {
 	struct controller *ctrl = (struct controller *)dev_id;
@@ -722,6 +761,12 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id)
 
 	/* Save pending events for consumption by IRQ thread. */
 	atomic_or(events, &ctrl->pending_events);
+
+	/* presence change events */
+	if ((events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) &&
+	    !pciehp_card_present(ctrl))
+		schedule_work(&ctrl->disconnect_work);
+
 	return IRQ_WAKE_THREAD;
 }
 
@@ -761,14 +806,7 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id)
 				      PCI_EXP_SLTCTL_ATTN_IND_ON);
 	}
 
-	/*
-	 * Ignore Link Down/Up events caused by Downstream Port Containment
-	 * if recovery succeeded, or caused by Secondary Bus Reset,
-	 * suspend to D3cold, firmware update, FPGA reconfiguration, etc.
-	 */
-	if ((events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) &&
-	    (pci_dpc_recovered(pdev) || pci_hp_spurious_link_change(pdev)) &&
-	    ctrl->state == ON_STATE) {
+	if (pciehp_is_spurious_link_change(ctrl, pdev, events)) {
 		u16 ignored_events = PCI_EXP_SLTSTA_DLLSC;
 
 		if (!ctrl->inband_presence_disabled)
@@ -1036,6 +1074,7 @@ struct controller *pcie_init(struct pcie_device *dev)
 	init_waitqueue_head(&ctrl->requester);
 	init_waitqueue_head(&ctrl->queue);
 	INIT_DELAYED_WORK(&ctrl->button_work, pciehp_queue_pushbutton_work);
+	INIT_WORK(&ctrl->disconnect_work, pciehp_disconnect_work);
 	dbg_ctrl(ctrl);
 
 	down_read(&pci_bus_sem);
@@ -1096,6 +1135,7 @@ struct controller *pcie_init(struct pcie_device *dev)
 void pciehp_release_ctrl(struct controller *ctrl)
 {
 	cancel_delayed_work_sync(&ctrl->button_work);
+	cancel_work_sync(&ctrl->disconnect_work);
 	kfree(ctrl);
 }
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 23b1605e783a..4e17878edeab 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -805,6 +805,12 @@ static inline int pci_dev_set_disconnected(struct pci_dev *dev, void *unused)
 	pci_dev_set_io_state(dev, pci_channel_io_perm_failure);
 	pci_doe_disconnected(dev);
 
+	return 0;
+}
+
+static inline int schedule_notification_work(struct pci_dev *dev, void *unused)
+{
+	pci_dev_set_disconnected(dev, NULL);
 	if (READ_ONCE(dev->disconnect_work_enable)) {
 		/* Make sure work is up to date. */
 		smp_rmb();
-- 
2.51.1

[PATCH RFC 3/3] misc: Add edu_srpoc surprise removal POC driver

[Abhin Parekadan Jose]

A test driver for the QEMU edu device that reproduces the surprise
removal hang described in MST's RFC v5 thread.

- hacked in a reg to the edu device on qemu to raise a delayed irq
- This driver writes to that reg in remove and waits for the irq to be
  handled. This simulates del_gendisk() blocked in
  blk_mq_freeze_queue_wait()

Assisted-by: LLM
Signed-off-by: Abhin Parekadan Jose <abhinjoses@gmail.com>
---
 drivers/misc/Makefile    |   1 +
 drivers/misc/edu_srpoc.c | 169 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 170 insertions(+)
 create mode 100644 drivers/misc/edu_srpoc.c

diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index e8d8d5d88c0d..1479bf19c646 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_AD525X_DPOT_I2C)	+= ad525x_dpot-i2c.o
 obj-$(CONFIG_AD525X_DPOT_SPI)	+= ad525x_dpot-spi.o
 obj-$(CONFIG_ATMEL_SSC)		+= atmel-ssc.o
 obj-$(CONFIG_DUMMY_IRQ)		+= dummy-irq.o
+obj-y				+= edu_srpoc.o
 obj-$(CONFIG_ICS932S401)	+= ics932s401.o
 obj-$(CONFIG_LKDTM)		+= lkdtm/
 obj-$(CONFIG_TI_FPC202)		+= ti_fpc202.o
diff --git a/drivers/misc/edu_srpoc.c b/drivers/misc/edu_srpoc.c
new file mode 100644
index 000000000000..f536bc4aa253
--- /dev/null
+++ b/drivers/misc/edu_srpoc.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * edu_srpoc.c Surprise Removal POC driver for the QEMU edu device
+ *
+ * In remove(), schedules a delayed interrupt on the edu device and
+ * blocks waiting for it to complete. This simulates del_gendisk()
+ * blocked in blk_mq_freeze_queue_wait() on slow in-flight I/O.
+ *
+ * Surprise-remove the device during this window to reproduce the hang.
+ *
+ * edu BAR 0 registers used:
+ *   0x08  Factorial: write N to compute N! asynchronously
+ *   0x20  Status: write EDU_STATUS_IRQFACT to enable IRQ on completion
+ *   0x24  IRQ status: bit 0 = FACT_IRQ, bit 9 = DELAY_IRQ
+ *   0x30  Delayed IRQ: write N (ms). Hacked in this functionality(not upstream).
+ *   0x64  IRQ lower: write bitmask to ack
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/interrupt.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+
+#define PCI_VENDOR_ID_EDU	0x1234
+#define PCI_DEVICE_ID_EDU	0x11e8
+
+#define EDU_REG_FACT		0x08
+#define EDU_REG_STATUS		0x20
+#define EDU_REG_DELAYED_IRQ	0x30
+#define EDU_REG_IRQ_STATUS	0x24
+#define EDU_REG_IRQ_LOWER	0x64
+
+#define EDU_STATUS_IRQFACT	0x80
+#define EDU_FACT_IRQ		BIT(0)
+#define EDU_DELAY_IRQ		BIT(9)
+
+struct edu_dev {
+	struct pci_dev		*pdev;
+	void __iomem		*regs;
+	struct completion	irq_done;
+};
+
+static irqreturn_t edu_irq_handler(int irq, void *data)
+{
+	struct edu_dev *edu = data;
+	u32 status;
+
+	status = ioread32(edu->regs + EDU_REG_IRQ_STATUS);
+	if (!status)
+		return IRQ_NONE;
+
+	iowrite32(status, edu->regs + EDU_REG_IRQ_LOWER);
+
+	if (status & (EDU_FACT_IRQ | EDU_DELAY_IRQ)) {
+		complete(&edu->irq_done);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void edu_disconnect(struct work_struct *work)
+{
+	struct pci_dev *pdev = container_of(work, struct pci_dev,
+					    disconnect_work);
+	struct edu_dev *edu = pci_get_drvdata(pdev);
+
+	if (!pci_test_and_clear_disconnect_enable(pdev))
+		return;
+
+	if (!edu)
+		return;
+
+	dev_info(&pdev->dev, "disconnect_work fired — unblocking remove()\n");
+	complete(&edu->irq_done);
+}
+
+static int edu_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct edu_dev *edu;
+	int err;
+
+	edu = devm_kzalloc(&pdev->dev, sizeof(*edu), GFP_KERNEL);
+	if (!edu)
+		return -ENOMEM;
+
+	edu->pdev = pdev;
+	init_completion(&edu->irq_done);
+
+	err = pci_enable_device(pdev);
+	if (err)
+		return err;
+
+	err = pci_request_regions(pdev, "edu_srpoc");
+	if (err)
+		goto err_disable;
+
+	edu->regs = pci_iomap(pdev, 0, 0);
+	if (!edu->regs) {
+		err = -ENOMEM;
+		goto err_release;
+	}
+
+	pci_set_master(pdev);
+
+	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI | PCI_IRQ_INTX);
+	if (err < 0)
+		goto err_iounmap;
+
+	err = request_irq(pci_irq_vector(pdev, 0), edu_irq_handler,
+			  IRQF_SHARED, "edu_srpoc", edu);
+	if (err)
+		goto err_free_vectors;
+
+	pci_set_drvdata(pdev, edu);
+
+	INIT_WORK(&pdev->disconnect_work, edu_disconnect);
+	pci_set_disconnect_work(pdev);
+
+	dev_info(&pdev->dev, "edu_srpoc probed\n");
+	return 0;
+
+err_free_vectors:
+	pci_free_irq_vectors(pdev);
+err_iounmap:
+	pci_iounmap(pdev, edu->regs);
+err_release:
+	pci_release_regions(pdev);
+err_disable:
+	pci_disable_device(pdev);
+	return err;
+}
+
+static void edu_remove(struct pci_dev *pdev)
+{
+	struct edu_dev *edu = pci_get_drvdata(pdev);
+
+	iowrite32(EDU_STATUS_IRQFACT, edu->regs + EDU_REG_STATUS);
+	iowrite32(600000, edu->regs + EDU_REG_DELAYED_IRQ);
+
+	dev_info(&pdev->dev, "Waiting for IRQ in remove()\n");
+	wait_for_completion(&edu->irq_done);
+	dev_info(&pdev->dev, "Unblocked, cleaning up\n");
+
+	pci_clear_disconnect_work(pdev);
+	free_irq(pci_irq_vector(pdev, 0), edu);
+	pci_free_irq_vectors(pdev);
+	pci_iounmap(pdev, edu->regs);
+	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+}
+
+static const struct pci_device_id edu_ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_EDU, PCI_DEVICE_ID_EDU) },
+	{ 0 }
+};
+MODULE_DEVICE_TABLE(pci, edu_ids);
+
+static struct pci_driver edu_driver = {
+	.name		= "edu_srpoc",
+	.id_table	= edu_ids,
+	.probe		= edu_probe,
+	.remove		= edu_remove,
+};
+
+module_pci_driver(edu_driver);
+MODULE_AUTHOR("Abhin Parekadan Jose");
+MODULE_DESCRIPTION("edu surprise removal POC driver");
+MODULE_LICENSE("GPL");
-- 
2.51.1

Re: [PATCH RFC 1/3] PCI: Report surprise removal event

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition between `pci_dev_set_disconnected()` and `pci_clear_disconnect_work()` allows a `work_struct` to be queued after the driver teardown completes, leading to use-after-free or execution of unloaded module code.
--

commit 889ac5c66b7f1749706d4c7c756b37926ccf5245
Author: Michael S. Tsirkin <mst@redhat.com>

PCI: Report surprise removal event

This commit introduces a mechanism to report surprise removal events to
drivers via a work_struct. It allows drivers to initiate disconnect cleanup
without waiting on device interrupts that will never arrive.

Link: https://lore.kernel.org/all/fba3d235e38c1c6fcef2a30ed083ad9e25b20fa3.1752094439.git.mst@redhat.com/

> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index ba3c3fddddc23..23b1605e783a3 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -805,6 +805,12 @@ static inline int pci_dev_set_disconnected(struct pci_dev *dev, void *unused)
>  	pci_dev_set_io_state(dev, pci_channel_io_perm_failure);
>  	pci_doe_disconnected(dev);
>  
> +	if (READ_ONCE(dev->disconnect_work_enable)) {
> +		/* Make sure work is up to date. */
> +		smp_rmb();
> +		schedule_work(&dev->disconnect_work);

[Severity: High]
Does this lockless check introduce a Time-of-Check to Time-of-Use (TOCTOU)
race window?

If a thread executing pci_dev_set_disconnected() is preempted right after
checking READ_ONCE(dev->disconnect_work_enable) but before calling
schedule_work(), the work might get queued after driver teardown completes.

If a concurrent driver unbind operation calls pci_clear_disconnect_work(),
it will set the flag to 0 and call cancel_work_sync(). Since the work is
not yet queued, cancel_work_sync() would return immediately. The device is
unbound and can be freed, or the module unloaded. When the preempted thread
resumes, it enqueues the work_struct which now resides in freed memory,
potentially causing a Use-After-Free or a crash from executing unmapped
memory.

Note: This issue is not resolved by follow-up patches in this series; the
vulnerable logic is merely moved to schedule_notification_work().

> +	}
> +
>  	return 0;
>  }

[ ... ]

> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d31a8d107b1ef..06d43f57f5094 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h

[ ... ]

> +static inline void pci_clear_disconnect_work(struct pci_dev *pdev)
> +{
> +	WRITE_ONCE(pdev->disconnect_work_enable, 0x0);
> +
> +	/* Make sure to stop using work from now on. */
> +	smp_wmb();
> +
> +	cancel_work_sync(&pdev->disconnect_work);

[Severity: High]
This teardown function assumes cancel_work_sync() will catch all pending
work, but can it synchronize with threads that were preempted just before
calling schedule_work() in pci_dev_set_disconnected()?

> +}

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

Re: [PATCH RFC 3/3] misc: Add edu_srpoc surprise removal POC driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Unconditional compilation of test driver causes kernel bloat and hangs on normal unbind
- [High] Missing surprise removal check (0xFFFFFFFF) in shared IRQ handler causes interrupt storms
- [Medium] Missing pci_clear_master() in cleanup and error paths violates PCI resource management rules
--

commit bf44d9ecb886ad9012988c5c1b317cd2fd7cfe83
Author: Abhin Parekadan Jose <abhinjoses@gmail.com>

misc: Add edu_srpoc surprise removal POC driver
    
This commit introduces a test driver for the QEMU edu device that reproduces
a surprise removal hang. It writes to a hacked register in remove() to trigger
a delayed IRQ and blocks waiting for it to simulate del_gendisk() behavior.

> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index e8d8d5d88c0df..1479bf19c6467 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
[ ... ]
> @@ -9,6 +9,7 @@ obj-$(CONFIG_AD525X_DPOT_I2C)	+= ad525x_dpot-i2c.o
>  obj-$(CONFIG_AD525X_DPOT_SPI)	+= ad525x_dpot-spi.o
>  obj-$(CONFIG_ATMEL_SSC)		+= atmel-ssc.o
>  obj-$(CONFIG_DUMMY_IRQ)		+= dummy-irq.o
> +obj-y				+= edu_srpoc.o

[Severity: High]
Is it intentional to unconditionally compile this test driver into the kernel
image? Using obj-y forces the module to be built-in for any configuration with
misc devices enabled, which causes unnecessary kernel bloat.

>  obj-$(CONFIG_ICS932S401)	+= ics932s401.o
>  obj-$(CONFIG_LKDTM)		+= lkdtm/
>  obj-$(CONFIG_TI_FPC202)		+= ti_fpc202.o
> diff --git a/drivers/misc/edu_srpoc.c b/drivers/misc/edu_srpoc.c
> new file mode 100644
> index 0000000000000..f536bc4aa253d
> --- /dev/null
> +++ b/drivers/misc/edu_srpoc.c
[ ... ]
> +static irqreturn_t edu_irq_handler(int irq, void *data)
> +{
> +	struct edu_dev *edu = data;
> +	u32 status;
> +
> +	status = ioread32(edu->regs + EDU_REG_IRQ_STATUS);
> +	if (!status)
> +		return IRQ_NONE;

[Severity: High]
What happens to this check during a surprise removal where MMIO reads typically
return all 1s (0xFFFFFFFF)?

Since 0xFFFFFFFF evaluates as non-zero, it appears this will bypass the
return IRQ_NONE. The handler would then incorrectly complete edu->irq_done
and return IRQ_HANDLED. Because the IRQ is registered as IRQF_SHARED, falsely
claiming the interrupt without clearing it on the actual hardware could defeat
the kernel's spurious interrupt detector, leading to an infinite interrupt
storm.

> +
> +	iowrite32(status, edu->regs + EDU_REG_IRQ_LOWER);
> +
> +	if (status & (EDU_FACT_IRQ | EDU_DELAY_IRQ)) {
> +		complete(&edu->irq_done);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
[ ... ]
> +static int edu_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
[ ... ]
> +	pci_set_master(pdev);
> +
> +	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI | PCI_IRQ_INTX);
> +	if (err < 0)
> +		goto err_iounmap;
> +
> +	err = request_irq(pci_irq_vector(pdev, 0), edu_irq_handler,
> +			  IRQF_SHARED, "edu_srpoc", edu);
> +	if (err)
> +		goto err_free_vectors;
> +
> +	pci_set_drvdata(pdev, edu);
> +
> +	INIT_WORK(&pdev->disconnect_work, edu_disconnect);
> +	pci_set_disconnect_work(pdev);
> +
> +	dev_info(&pdev->dev, "edu_srpoc probed\n");
> +	return 0;
> +
> +err_free_vectors:
> +	pci_free_irq_vectors(pdev);
> +err_iounmap:
> +	pci_iounmap(pdev, edu->regs);

[Severity: Medium]
Should pci_clear_master() be called in the error paths here to match the
earlier pci_set_master() call? Failing to clear the bus master bit before
releasing resources and disabling the device could theoretically allow rogue
DMA operations if the hardware acts unexpectedly.

> +err_release:
> +	pci_release_regions(pdev);
> +err_disable:
> +	pci_disable_device(pdev);
> +	return err;
> +}
> +
> +static void edu_remove(struct pci_dev *pdev)
> +{
> +	struct edu_dev *edu = pci_get_drvdata(pdev);
> +
> +	iowrite32(EDU_STATUS_IRQFACT, edu->regs + EDU_REG_STATUS);
> +	iowrite32(600000, edu->regs + EDU_REG_DELAYED_IRQ);
> +
> +	dev_info(&pdev->dev, "Waiting for IRQ in remove()\n");
> +	wait_for_completion(&edu->irq_done);

[Severity: High]
If this driver is bound to a standard QEMU edu device (which lacks the custom
EDU_REG_DELAYED_IRQ functionality), will this wait forever? A normal device
unbind via sysfs or system shutdown seems like it would hang indefinitely here
since the delayed hardware interrupt will never fire and disconnect_work is
only triggered on surprise removal.

> +	dev_info(&pdev->dev, "Unblocked, cleaning up\n");
> +
> +	pci_clear_disconnect_work(pdev);
> +	free_irq(pci_irq_vector(pdev, 0), edu);
> +	pci_free_irq_vectors(pdev);
> +	pci_iounmap(pdev, edu->regs);
> +	pci_release_regions(pdev);
> +	pci_disable_device(pdev);

[Severity: Medium]
Does this cleanup path also need a call to pci_clear_master() before
pci_disable_device() to ensure bus mastering is properly disabled upon
removal?

> +}

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

Re: [PATCH RFC 2/3] PCI: pciehp: Report surprise removal from pciehp_isr()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Time-of-Check to Time-of-Use (TOCTOU) race condition in `schedule_notification_work()` allows queueing work on a device that is concurrently being removed, leading to a Use-After-Free.
- [High] Concurrent execution of `pciehp_ist` and `pciehp_disconnect_work` introduces multiple race conditions resulting in destructive double-consumption of event state and spurious link change flags, leading to erroneous device disconnection or teardown.
- [Medium] `pciehp_card_present()` returns `-ENODEV` on read errors, but the new logic incorrectly evaluates this negative error code differently in different contexts, failing to handle unresponsive controllers.
--

commit 25ff2fa1b2bf05d104ab00dedddfbc0139ac2747
Author: Abhin Parekadan Jose <abhinjoses@gmail.com>
Subject: PCI: pciehp: Report surprise removal from pciehp_isr()

This commit attempts to prevent surprise removals from hanging when the
single-threaded IRQ thread is blocked waiting on a device read. It schedules
a work item from the hardirq handler on presence changes to notify drivers
of the disconnect without blocking.

Link: https://lore.kernel.org/all/aHlZE18kPuHuDtTT@wunner.de/

> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index 4c62140a3cb44..235ca8a176f1e 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -620,6 +620,45 @@ static void pciehp_ignore_link_change(struct controller *ctrl,
>  	up_read(&ctrl->reset_lock);
>  }
>  
> +/*
> + * Link Down/Up events caused by Downstream Port Containment if recovery
> + * succeeded, or caused by Secondary Bus Reset, suspend to D3cold, firmware
> + * update, FPGA reconfiguration, etc. are spurious and should be ignored.
> + */
> +static bool pciehp_is_spurious_link_change(struct controller *ctrl,
> +						  struct pci_dev *pdev,
> +						  u32 events)
> +{
> +	return (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) &&
> +		(pci_dpc_recovered(pdev) || pci_hp_spurious_link_change(pdev)) &&
> +		ctrl->state == ON_STATE;
> +}

[Severity: High]
Does concurrent execution of pciehp_ist() and pciehp_disconnect_work()
cause destructive double-consumption of event state here?

A spurious link flap (like DPC or SBR) will schedule both the pciehp_ist()
IRQ thread and the new pciehp_disconnect_work() item.

When this function calls pci_hp_spurious_link_change(pdev), it evaluates:

drivers/pci/hotplug/pci_hotplug_core.c:pci_hp_spurious_link_change() {
    ...
    return test_and_clear_bit(PCI_LINK_CHANGED, &pdev->priv_flags);
}

If both threads evaluate this concurrently, the single-shot bit is cleared
upon the first read. One thread will receive false, miss the spurious
check, and erroneously trigger a device teardown or disconnect.

> +
> +/*
> + * Workaround to not wait in the isr.
> + */
> +static void pciehp_disconnect_work(struct work_struct *work)
> +{
> +	struct pci_bus *bus;
> +	struct controller *ctrl = container_of(work, struct controller,
> +					       disconnect_work);
> +	struct pci_dev *pdev = ctrl_dev(ctrl);
> +	u32 events;
> +
> +	events = atomic_read(&ctrl->pending_events);
> +
> +	if (pciehp_is_spurious_link_change(ctrl, pdev, events))
> +		return;

[Severity: High]
Can this atomic_read() result in a missed spurious link check if
pciehp_ist() executes first?

pciehp_ist() destructively clears the pending events:

drivers/pci/hotplug/pciehp_hpc.c:pciehp_ist() {
    ...
    events = atomic_xchg(&ctrl->pending_events, 0);
    ...
}

If pciehp_ist() exchanges the events before this work item reads them,
events will be 0. This causes pciehp_is_spurious_link_change() to return
false, which erroneously allows the work item to proceed and disconnect
the device during a transient link flap.

> +
> +	bus = ctrl->pcie->port->subordinate;
> +
> +	/* The card may have returned */
> +	if (!bus || pciehp_card_present(ctrl) != 0)
> +		return;

[Severity: Medium]
Does this incorrectly evaluate negative error codes on slot status read
failures?

If pciehp_card_present() encounters a read error (e.g. from an abrupt
removal or ungraceful D3cold transition), it returns -ENODEV (-19).

Since -ENODEV != 0 evaluates to true, this function will incorrectly
return early instead of notifying drivers of the unreachable controller.

> +
> +	pci_walk_bus(bus, schedule_notification_work, NULL);
> +}
> +
>  static irqreturn_t pciehp_isr(int irq, void *dev_id)
>  {
>  	struct controller *ctrl = (struct controller *)dev_id;
[ ... ]
> @@ -722,6 +761,12 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id)
>  
>  	/* Save pending events for consumption by IRQ thread. */
>  	atomic_or(events, &ctrl->pending_events);
> +
> +	/* presence change events */
> +	if ((events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) &&
> +	    !pciehp_card_present(ctrl))
> +		schedule_work(&ctrl->disconnect_work);

[Severity: Medium]
Will this fail to schedule the disconnect work if the controller is
unresponsive?

As noted above, pciehp_card_present() returns -ENODEV on a read failure.
!(-ENODEV) evaluates to false, causing this condition to fail and
leaving the disconnect work unscheduled when the controller is entirely
unreachable.

> +
>  	return IRQ_WAKE_THREAD;
>  }
>  
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 23b1605e783a3..4e17878edeab6 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -805,6 +805,12 @@ static inline int pci_dev_set_disconnected(struct pci_dev *dev, void *unused)
>  	pci_dev_set_io_state(dev, pci_channel_io_perm_failure);
>  	pci_doe_disconnected(dev);
>  
> +	return 0;
> +}
> +
> +static inline int schedule_notification_work(struct pci_dev *dev, void *unused)
> +{
> +	pci_dev_set_disconnected(dev, NULL);
>  	if (READ_ONCE(dev->disconnect_work_enable)) {
>  		/* Make sure work is up to date. */
>  		smp_rmb();
> +		schedule_work(&dev->disconnect_work);
> +	}

[Severity: Critical]
Is there a Time-of-Check to Time-of-Use race condition here that could lead to
a use-after-free on the pci_dev object?

If the pci_walk_bus thread reads disconnect_work_enable as true but is preempted
before calling schedule_work, device removal can concurrently detach the driver
and execute pci_clear_disconnect_work().

Because the work is not yet queued, cancel_work_sync() will return immediately.
The removal thread will then block at down_write(&pci_bus_sem) in
pci_destroy_dev().

When the preempted walk thread resumes, it queues the work and releases the
read lock. The removal thread then unblocks, deletes the device, and frees
the pci_dev structure, but the workqueue will still attempt to execute the
queued work on the freed memory.

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

Re: [PATCH RFC 2/3] PCI: pciehp: Report surprise removal from pciehp_isr()

[Michael S. Tsirkin]

On Sat, Sep 05, 2026 at 06:38:59PM +0000, Abhin Parekadan Jose wrote:
> A surprise removal during a safe removal cannot be reported: the
> removal blocks waiting on a device interrupt or status read, and the
> single-threaded IRQ thread is itself executing that removal, so it
> cannot report that the device is gone. The removal hangs.
> 
> The hardirq handler pciehp_isr() still runs while the IRQ thread is
> blocked, so it can report the disconnect. However, pciehp_ist()
> deliberately ignores link and presence changes caused by a Secondary
> Bus Reset or Downstream Port Containment, where the device is only
> temporarily inaccessible. Distinguishing those normally requires
> waiting for the SBR or DPC to conclude, which takes seconds and is not
> possible in hardirq context.
> 
> Schedule a work item from pciehp_isr() when a PDC or DLLSC event
> arrives and PDS indicates if the device is connected/disconnected.
> This provides us a pathway to wait/block/sleep as we will not be
> in pciehp_isr().
> 
> Move the scheduling of the driver's disconnect notification out of
> pci_dev_set_disconnected() into schedule_notification_work() so it can
> be invoked from the new work item. Factor the spurious link change
> test out of pciehp_ist() into pciehp_is_spurious_link_change() so both
> pciehp_ist() and pciehp_disconnect_work() can use it.
> 
> Link: https://lore.kernel.org/all/aHlZE18kPuHuDtTT@wunner.de/
> Signed-off-by: Abhin Parekadan Jose <abhinjoses@gmail.com>
> ---
>  drivers/pci/hotplug/pciehp.h     |  1 +
>  drivers/pci/hotplug/pciehp_hpc.c | 56 +++++++++++++++++++++++++++-----
>  drivers/pci/pci.h                |  6 ++++
>  3 files changed, 55 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
> index debc79b0adfb..c8ceb9320e2e 100644
> --- a/drivers/pci/hotplug/pciehp.h
> +++ b/drivers/pci/hotplug/pciehp.h
> @@ -116,6 +116,7 @@ struct controller {
>  	unsigned int ist_running;
>  	int request_result;
>  	wait_queue_head_t requester;
> +	struct work_struct disconnect_work;
>  };
>  
>  /**
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index 4c62140a3cb4..235ca8a176f1 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -620,6 +620,45 @@ static void pciehp_ignore_link_change(struct controller *ctrl,
>  	up_read(&ctrl->reset_lock);
>  }
>  
> +/*
> + * Link Down/Up events caused by Downstream Port Containment if recovery
> + * succeeded, or caused by Secondary Bus Reset, suspend to D3cold, firmware
> + * update, FPGA reconfiguration, etc. are spurious and should be ignored.
> + */
> +static bool pciehp_is_spurious_link_change(struct controller *ctrl,
> +						  struct pci_dev *pdev,
> +						  u32 events)
> +{
> +	return (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) &&
> +		(pci_dpc_recovered(pdev) || pci_hp_spurious_link_change(pdev)) &&
> +		ctrl->state == ON_STATE;
> +}
> +
> +/*
> + * Workaround to not wait in the isr.
> + */
> +static void pciehp_disconnect_work(struct work_struct *work)
> +{
> +	struct pci_bus *bus;
> +	struct controller *ctrl = container_of(work, struct controller,
> +					       disconnect_work);
> +	struct pci_dev *pdev = ctrl_dev(ctrl);
> +	u32 events;
> +
> +	events = atomic_read(&ctrl->pending_events);
> +
> +	if (pciehp_is_spurious_link_change(ctrl, pdev, events))
> +		return;
> +
> +	bus = ctrl->pcie->port->subordinate;
> +
> +	/* The card may have returned */
> +	if (!bus || pciehp_card_present(ctrl) != 0)
> +		return;
> +
> +	pci_walk_bus(bus, schedule_notification_work, NULL);
> +}
> +
>  static irqreturn_t pciehp_isr(int irq, void *dev_id)
>  {
>  	struct controller *ctrl = (struct controller *)dev_id;
> @@ -722,6 +761,12 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id)
>  
>  	/* Save pending events for consumption by IRQ thread. */
>  	atomic_or(events, &ctrl->pending_events);
> +
> +	/* presence change events */
> +	if ((events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) &&
> +	    !pciehp_card_present(ctrl))
> +		schedule_work(&ctrl->disconnect_work);
> +
>  	return IRQ_WAKE_THREAD;
>  }
>  
> @@ -761,14 +806,7 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id)
>  				      PCI_EXP_SLTCTL_ATTN_IND_ON);
>  	}
>  
> -	/*
> -	 * Ignore Link Down/Up events caused by Downstream Port Containment
> -	 * if recovery succeeded, or caused by Secondary Bus Reset,
> -	 * suspend to D3cold, firmware update, FPGA reconfiguration, etc.
> -	 */
> -	if ((events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) &&
> -	    (pci_dpc_recovered(pdev) || pci_hp_spurious_link_change(pdev)) &&
> -	    ctrl->state == ON_STATE) {
> +	if (pciehp_is_spurious_link_change(ctrl, pdev, events)) {
>  		u16 ignored_events = PCI_EXP_SLTSTA_DLLSC;
>  
>  		if (!ctrl->inband_presence_disabled)
> @@ -1036,6 +1074,7 @@ struct controller *pcie_init(struct pcie_device *dev)
>  	init_waitqueue_head(&ctrl->requester);
>  	init_waitqueue_head(&ctrl->queue);
>  	INIT_DELAYED_WORK(&ctrl->button_work, pciehp_queue_pushbutton_work);
> +	INIT_WORK(&ctrl->disconnect_work, pciehp_disconnect_work);
>  	dbg_ctrl(ctrl);
>  
>  	down_read(&pci_bus_sem);
> @@ -1096,6 +1135,7 @@ struct controller *pcie_init(struct pcie_device *dev)
>  void pciehp_release_ctrl(struct controller *ctrl)
>  {
>  	cancel_delayed_work_sync(&ctrl->button_work);
> +	cancel_work_sync(&ctrl->disconnect_work);
>  	kfree(ctrl);
>  }
>  
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 23b1605e783a..4e17878edeab 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -805,6 +805,12 @@ static inline int pci_dev_set_disconnected(struct pci_dev *dev, void *unused)
>  	pci_dev_set_io_state(dev, pci_channel_io_perm_failure);
>  	pci_doe_disconnected(dev);
>  
> +	return 0;
> +}
> +
> +static inline int schedule_notification_work(struct pci_dev *dev, void *unused)
> +{
> +	pci_dev_set_disconnected(dev, NULL);


Does this not break what patch 1 was trying to do,
for everyone who does not call schedule_notification_work,
that is, everyone except pciehp?

I'd say do the reverse: make schedule_notification_work
schedule the work, and have both pcieh and 
pci_dev_set_disconnected call that.


>  	if (READ_ONCE(dev->disconnect_work_enable)) {
>  		/* Make sure work is up to date. */
>  		smp_rmb();
> -- 
> 2.51.1
>