[PATCH] drivers/pci/hotplug: Handle presence detection change properly

Subsystems: linux for powerpc (32-bit and 64-bit), pci subsystem, the rest

STALE3530d REVIEWED: 1 (0M)

1 review trailer.

4 messages, 2 authors, 2016-12-22 · open the first message on its own page

[PATCH] drivers/pci/hotplug: Handle presence detection change properly

From: Gavin Shan <hidden>
Date: 2016-12-20 06:49:45

The surprise hotplug is driven by interrupt in PowerNV PCI hotplug
driver. In the interrupt handler, pnv_php_interrupt(), we bail when
pnv_pci_get_presence_state() returns zero wrongly. It causes the
presence change event is always ignored incorrectly.

This fixes the issue by bailing on error (non-zero value) returned
from pnv_pci_get_presence_state().

Fixes: 360aebd85a4 ("drivers/pci/hotplug: Support surprise hotplug in powernv driver")
Cc: stable@vger.kernel.org #v4.9+
Reported-by: Hank Chang <redacted>
Signed-off-by: Gavin Shan <redacted>
Tested-by: Willie Liauw <redacted>
---
 drivers/pci/hotplug/pnv_php.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 56efaf7..38a2309 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -707,8 +707,12 @@ static irqreturn_t pnv_php_interrupt(int irq, void *data)
 		added = !!(lsts & PCI_EXP_LNKSTA_DLLLA);
 	} else if (sts & PCI_EXP_SLTSTA_PDC) {
 		ret = pnv_pci_get_presence_state(php_slot->id, &presence);
-		if (!ret)
+		if (ret) {
+			dev_warn(&pdev->dev, "PCI slot [%s] error %d handling PDC event (0x%04x)\n",
+				php_slot->name, ret, sts);
 			return IRQ_HANDLED;
+		}
+
 		added = !!(presence == OPAL_PCI_SLOT_PRESENT);
 	} else {
 		return IRQ_NONE;
-- 
2.7.4

Re: [PATCH] drivers/pci/hotplug: Handle presence detection change properly

From: Greg KH <gregkh@linuxfoundation.org>
Date: 2016-12-20 09:36:58

On Tue, Dec 20, 2016 at 05:49:33PM +1100, Gavin Shan wrote:
quoted hunk
The surprise hotplug is driven by interrupt in PowerNV PCI hotplug
driver. In the interrupt handler, pnv_php_interrupt(), we bail when
pnv_pci_get_presence_state() returns zero wrongly. It causes the
presence change event is always ignored incorrectly.

This fixes the issue by bailing on error (non-zero value) returned
from pnv_pci_get_presence_state().

Fixes: 360aebd85a4 ("drivers/pci/hotplug: Support surprise hotplug in powernv driver")
Cc: stable@vger.kernel.org #v4.9+
Reported-by: Hank Chang <redacted>
Signed-off-by: Gavin Shan <redacted>
Tested-by: Willie Liauw <redacted>
---
 drivers/pci/hotplug/pnv_php.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 56efaf7..38a2309 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -707,8 +707,12 @@ static irqreturn_t pnv_php_interrupt(int irq, void *data)
 		added = !!(lsts & PCI_EXP_LNKSTA_DLLLA);
 	} else if (sts & PCI_EXP_SLTSTA_PDC) {
 		ret = pnv_pci_get_presence_state(php_slot->id, &presence);
-		if (!ret)
+		if (ret) {
+			dev_warn(&pdev->dev, "PCI slot [%s] error %d handling PDC event (0x%04x)\n",
+				php_slot->name, ret, sts);
What can a user do with this warning?

Re: [PATCH] drivers/pci/hotplug: Handle presence detection change properly

From: Gavin Shan <hidden>
Date: 2016-12-20 23:07:04

On Tue, Dec 20, 2016 at 10:37:09AM +0100, Greg KH wrote:
On Tue, Dec 20, 2016 at 05:49:33PM +1100, Gavin Shan wrote:
quoted
The surprise hotplug is driven by interrupt in PowerNV PCI hotplug
driver. In the interrupt handler, pnv_php_interrupt(), we bail when
pnv_pci_get_presence_state() returns zero wrongly. It causes the
presence change event is always ignored incorrectly.

This fixes the issue by bailing on error (non-zero value) returned
from pnv_pci_get_presence_state().

Fixes: 360aebd85a4 ("drivers/pci/hotplug: Support surprise hotplug in powernv driver")
Cc: stable@vger.kernel.org #v4.9+
Reported-by: Hank Chang <redacted>
Signed-off-by: Gavin Shan <redacted>
Tested-by: Willie Liauw <redacted>
---
 drivers/pci/hotplug/pnv_php.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 56efaf7..38a2309 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -707,8 +707,12 @@ static irqreturn_t pnv_php_interrupt(int irq, void *data)
 		added = !!(lsts & PCI_EXP_LNKSTA_DLLLA);
 	} else if (sts & PCI_EXP_SLTSTA_PDC) {
 		ret = pnv_pci_get_presence_state(php_slot->id, &presence);
-		if (!ret)
+		if (ret) {
+			dev_warn(&pdev->dev, "PCI slot [%s] error %d handling PDC event (0x%04x)\n",
+				php_slot->name, ret, sts);
What can a user do with this warning?
This warning is to catch developer's attention only. It's very
rare for pnv_pci_get_presence_state() to fail as it simply checks
hardware regiter bit (from Slot Status regsiter in PCIe capability
or PHB's Hotplug Override register) in the skiboot firmware.

User needs to retry the operation (hot-add or hot-remove) when
seeing this warning. Greg, please let me know if below message
is better?

dev_warn(&pdev->dev, "PCI slot [%s] error %d getting presence status on event 0x%04x, to retry the operation.\n",
         php_slot->name, ret, sts);

Thanks,
Gavin

Re: [PATCH] drivers/pci/hotplug: Handle presence detection change properly

From: Gavin Shan <hidden>
Date: 2016-12-22 05:42:36

On Tue, Dec 20, 2016 at 05:49:33PM +1100, Gavin Shan wrote:
The surprise hotplug is driven by interrupt in PowerNV PCI hotplug
driver. In the interrupt handler, pnv_php_interrupt(), we bail when
pnv_pci_get_presence_state() returns zero wrongly. It causes the
presence change event is always ignored incorrectly.

This fixes the issue by bailing on error (non-zero value) returned
from pnv_pci_get_presence_state().

Fixes: 360aebd85a4 ("drivers/pci/hotplug: Support surprise hotplug in powernv driver")
Cc: stable@vger.kernel.org #v4.9+
Reported-by: Hank Chang <redacted>
Signed-off-by: Gavin Shan <redacted>
Tested-by: Willie Liauw <redacted>
---
Please ignore this one for now. I will post a revised series to fix
more issues found in recent testing. Also, the warning message will
be modified accordingly.

Thanks,
Gavin
quoted hunk
drivers/pci/hotplug/pnv_php.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 56efaf7..38a2309 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -707,8 +707,12 @@ static irqreturn_t pnv_php_interrupt(int irq, void *data)
		added = !!(lsts & PCI_EXP_LNKSTA_DLLLA);
	} else if (sts & PCI_EXP_SLTSTA_PDC) {
		ret = pnv_pci_get_presence_state(php_slot->id, &presence);
-		if (!ret)
+		if (ret) {
+			dev_warn(&pdev->dev, "PCI slot [%s] error %d handling PDC event (0x%04x)\n",
+				php_slot->name, ret, sts);
			return IRQ_HANDLED;
+		}
+
		added = !!(presence == OPAL_PCI_SLOT_PRESENT);
	} else {
		return IRQ_NONE;
-- 
2.7.4
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help