Blob Blame History Raw
From 34fb6bf9b13aef4ca14224f2175ecd189e98160b Mon Sep 17 00:00:00 2001
From: Keith Busch <keith.busch@intel.com>
Date: Wed, 5 Sep 2018 14:35:41 -0600
Subject: [PATCH] PCI: pciehp: Fix hot-add vs powerfault detection order
Git-commit: 34fb6bf9b13aef4ca14224f2175ecd189e98160b
Patch-mainline: v4.19
References: FATE#326303

If both hot-add and power fault were observed in a single interrupt, we
handled the hot-add first, then the power fault, in this path:

  pciehp_ist
    if (events & (PDC | DLLSC))
      pciehp_handle_presence_or_link_change
        case OFF_STATE:
          pciehp_enable_slot
            __pciehp_enable_slot
              board_added
                pciehp_power_on_slot
                  ctrl->power_fault_detected = 0
                  pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_ON, PCI_EXP_SLTCTL_PCC)
                pciehp_green_led_on(p_slot)             # power LED on
		pciehp_set_attention_status(p_slot, 0)  # attention LED off
    if ((events & PFD) && !ctrl->power_fault_detected)
      ctrl->power_fault_detected = 1
      pciehp_set_attention_status(1)                    # attention LED on
      pciehp_green_led_off(slot)                        # power LED off

This left the attention indicator on (even though the hot-add succeeded)
and the power indicator off (even though the slot power was on).

Fix this by checking for power faults before checking for new devices.

Prior to 0e94916e6091, this was successful because everything was chained
through work queues and the order was:

  INT_PRESENCE_ON -> INT_POWER_FAULT -> ENABLE_REQ

The ENABLE_REQ cleared the power fault at the end, but now everything is
handled inline with the interrupt thread, such that the work ENABLE_REQ was
doing happens before power fault handling now.

Fixes: 0e94916e6091 ("PCI: pciehp: Handle events synchronously")
Signed-off-by: Keith Busch <keith.busch@intel.com>
[bhelgaas: changelog]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/pci/hotplug/pciehp_hpc.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -510,7 +510,7 @@ int pciehp_power_on_slot(struct slot *sl
 	u16 slot_status;
 	int retval;
 
-	/* Clear sticky power-fault bit from previous power failures */
+	/* Clear power-fault bit from previous power failures */
 	pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
 	if (slot_status & PCI_EXP_SLTSTA_PFD)
 		pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
@@ -621,6 +621,14 @@ static irqreturn_t pciehp_ist(int irq, v
 		pciehp_handle_button_press(slot);
 	}
 
+	/* Check Power Fault Detected */
+	if ((events & PCI_EXP_SLTSTA_PFD) && !ctrl->power_fault_detected) {
+		ctrl->power_fault_detected = 1;
+		ctrl_err(ctrl, "Slot(%s): Power fault\n", slot_name(slot));
+		pciehp_set_attention_status(slot, 1);
+		pciehp_green_led_off(slot);
+	}
+
 	/*
 	 * Disable requests have higher priority than Presence Detect Changed
 	 * or Data Link Layer State Changed events.
@@ -632,14 +640,6 @@ static irqreturn_t pciehp_ist(int irq, v
 		pciehp_handle_presence_or_link_change(slot, events);
 	up_read(&ctrl->reset_lock);
 
-	/* Check Power Fault Detected */
-	if ((events & PCI_EXP_SLTSTA_PFD) && !ctrl->power_fault_detected) {
-		ctrl->power_fault_detected = 1;
-		ctrl_err(ctrl, "Slot(%s): Power fault\n", slot_name(slot));
-		pciehp_set_attention_status(slot, 1);
-		pciehp_green_led_off(slot);
-	}
-
 	wake_up(&ctrl->requester);
 	return IRQ_HANDLED;
 }