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

[Abhin Parekadan Jose <abhinjoses@gmail.com>]

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