Skip to contentSkip to conversation

PCI/DPC: Defer all event handling to work queue

PATCH In review Keith Busch 5 participants 14 messages Jan 29, 2018 – Jan 31, 2018 PCI

lore
Share & tools
lore

Keith Busch

author
#0 lore raw

PATCH 1/6 PCI/DPC: Defer all event handling to work queue

This moves all event handling to the existing work queue, which will make passing event information easier for the actual handler.

Signed-off-by Keith Busch <[email protected]>
---
 drivers/pci/pcie/pcie-dpc.c | 47 +++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)
drivers/pci/pcie/pcie-dpc.c +24 -23
index e53e1fa82e9d..ecdb76bc7b56 100644
@@ -45,6 +45,8 @@ static const char * const rp_pio_error_string[] = {
 	"Memory Request Completion Timeout",		 /* Bit Position 18 */
 };
 
+static void dpc_process_rp_pio_error(struct dpc_dev *dpc);
+
 static int dpc_wait_rp_inactive(struct dpc_dev *dpc)
 {
 	unsigned long timeout = jiffies + HZ;

@@ -87,7 +89,27 @@ static void dpc_work(struct work_struct *work)
 	struct dpc_dev *dpc = container_of(work, struct dpc_dev, work);
 	struct pci_dev *dev, *temp, *pdev = dpc->dev->port;
 	struct pci_bus *parent = pdev->subordinate;
-	u16 cap = dpc->cap_pos, ctl;
+	u16 cap = dpc->cap_pos, ctl, status, source, reason, ext_reason;
+
+	pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
+	pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID, &source);
+
+	dev_info(&pdev->dev, "DPC containment event, status:%#06x source:%#06x\n",
+		status, source);
+
+	reason = (status & PCI_EXP_DPC_STATUS_TRIGGER_RSN) >> 1;
+	ext_reason = (status & PCI_EXP_DPC_STATUS_TRIGGER_RSN_EXT) >> 5;
+
+	dev_warn(&pdev->dev, "DPC %s detected, remove downstream devices\n",
+		 (reason == 0) ? "unmasked uncorrectable error" :
+		 (reason == 1) ? "ERR_NONFATAL" :
+		 (reason == 2) ? "ERR_FATAL" :
+		 (ext_reason == 0) ? "RP PIO error" :
+		 (ext_reason == 1) ? "software trigger" :
+				     "reserved error");
+
+	if (dpc->rp_extensions && reason == 3 && ext_reason == 0)
+		dpc_process_rp_pio_error(dpc);
 
 	pci_lock_rescan_remove();
 	list_for_each_entry_safe_reverse(dev, temp, &parent->devices,

@@ -185,8 +207,7 @@ static irqreturn_t dpc_irq(int irq, void *context)
 {
 	struct dpc_dev *dpc = (struct dpc_dev *)context;
 	struct pci_dev *pdev = dpc->dev->port;
-	struct device *dev = &dpc->dev->device;
-	u16 cap = dpc->cap_pos, ctl, status, source, reason, ext_reason;
+	u16 cap = dpc->cap_pos, ctl, status;
 
 	pci_read_config_word(pdev, cap + PCI_EXP_DPC_CTL, &ctl);
 

@@ -207,26 +228,6 @@ static irqreturn_t dpc_irq(int irq, void *context)
 	pci_write_config_word(pdev, cap + PCI_EXP_DPC_CTL,
 			      ctl & ~PCI_EXP_DPC_CTL_INT_EN);
 
-	pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID,
-			     &source);
-
-	dev_info(dev, "DPC containment event, status:%#06x source:%#06x\n",
-		status, source);
-
-	reason = (status & PCI_EXP_DPC_STATUS_TRIGGER_RSN) >> 1;
-	ext_reason = (status & PCI_EXP_DPC_STATUS_TRIGGER_RSN_EXT) >> 5;
-
-	dev_warn(dev, "DPC %s detected, remove downstream devices\n",
-		 (reason == 0) ? "unmasked uncorrectable error" :
-		 (reason == 1) ? "ERR_NONFATAL" :
-		 (reason == 2) ? "ERR_FATAL" :
-		 (ext_reason == 0) ? "RP PIO error" :
-		 (ext_reason == 1) ? "software trigger" :
-				     "reserved error");
-	/* show RP PIO error detail information */
-	if (dpc->rp_extensions && reason == 3 && ext_reason == 0)
-		dpc_process_rp_pio_error(dpc);
-
 	schedule_work(&dpc->work);
 
 	return IRQ_HANDLED;
Signature · 1 line
2.14.3
author
#1 lore raw

PATCH 2/6 PCI/DPC: Leave interrupts enabled while handling event

The control register was being abused as a way to know if a shared interrupt is notifying the driver of a new DPC event. A DPC capable port can not trigger a second interrupt until the host acknowledges the first, and since DPC handles events in a deferred work queue, we don't need to use the config register to know if the DPC driver needs to handle the interrupt. We just need to make sure we don't schedule the same work multiple times.

Signed-off-by Keith Busch <[email protected]>
---
 drivers/pci/pcie/pcie-dpc.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)
drivers/pci/pcie/pcie-dpc.c +5 -18
index ecdb76bc7b56..cf0398ccaeb6 100644
@@ -89,7 +89,7 @@ static void dpc_work(struct work_struct *work)
 	struct dpc_dev *dpc = container_of(work, struct dpc_dev, work);
 	struct pci_dev *dev, *temp, *pdev = dpc->dev->port;
 	struct pci_bus *parent = pdev->subordinate;
-	u16 cap = dpc->cap_pos, ctl, status, source, reason, ext_reason;
+	u16 cap = dpc->cap_pos, status, source, reason, ext_reason;
 
 	pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
 	pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID, &source);

@@ -135,10 +135,6 @@ static void dpc_work(struct work_struct *work)
 
 	pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS,
 		PCI_EXP_DPC_STATUS_TRIGGER | PCI_EXP_DPC_STATUS_INTERRUPT);
-
-	pci_read_config_word(pdev, cap + PCI_EXP_DPC_CTL, &ctl);
-	pci_write_config_word(pdev, cap + PCI_EXP_DPC_CTL,
-			      ctl | PCI_EXP_DPC_CTL_INT_EN);
 }
 
 static void dpc_process_rp_pio_error(struct dpc_dev *dpc)

@@ -207,16 +203,10 @@ static irqreturn_t dpc_irq(int irq, void *context)
 {
 	struct dpc_dev *dpc = (struct dpc_dev *)context;
 	struct pci_dev *pdev = dpc->dev->port;
-	u16 cap = dpc->cap_pos, ctl, status;
-
-	pci_read_config_word(pdev, cap + PCI_EXP_DPC_CTL, &ctl);
-
-	if (!(ctl & PCI_EXP_DPC_CTL_INT_EN) || ctl == (u16)(~0))
-		return IRQ_NONE;
+	u16 cap = dpc->cap_pos, status;
 
 	pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
-
-	if (!(status & PCI_EXP_DPC_STATUS_INTERRUPT))
+	if (!(status & PCI_EXP_DPC_STATUS_INTERRUPT) || status == (u16)(~0))
 		return IRQ_NONE;
 
 	if (!(status & PCI_EXP_DPC_STATUS_TRIGGER)) {

@@ -225,11 +215,8 @@ static irqreturn_t dpc_irq(int irq, void *context)
 		return IRQ_HANDLED;
 	}
 
-	pci_write_config_word(pdev, cap + PCI_EXP_DPC_CTL,
-			      ctl & ~PCI_EXP_DPC_CTL_INT_EN);
-
-	schedule_work(&dpc->work);
-
+	if (!work_busy(&dpc->work))
+		schedule_work(&dpc->work);
 	return IRQ_HANDLED;
 }
Signature · 1 line
2.14.3
#2 lore raw

Re: [PATCH 2/6] PCI/DPC: Leave interrupts enabled while handling event

drivers/pci/pcie/pcie-dpc.c static irqreturn_t dpc_irq(int irq, void *context) · line 219

Quoted 2 lines · Keith Busch
drivers/pci/pcie/pcie-dpc.c +2 -0
+	if (!work_busy(&dpc->work))
+		schedule_work(&dpc->work);

Isn't there a race condition between the time that dpc_work() clears PCI_EXP_DPC_STATUS register and when work actually completes by returning from dpc_work()?

If the interrupt arrives just in this window, this code will not schedule the work.

Signature · 3 lines
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
#3 lore raw

Re: [PATCH 2/6] PCI/DPC: Leave interrupts enabled while handling event

+ Oza

Quoted 13 lines · Sinan Kaya

drivers/pci/pcie/pcie-dpc.c static irqreturn_t dpc_irq(int irq, void *context) · line 219

Quoted 2 lines · Keith Busch
drivers/pci/pcie/pcie-dpc.c +2 -0
+     if (!work_busy(&dpc->work))
+             schedule_work(&dpc->work);

Isn't there a race condition between the time that dpc_work() clears PCI_EXP_DPC_STATUS register and when work actually completes by returning from dpc_work()?

If the interrupt arrives just in this window, this code will not schedule the work.

--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

#4 lore raw

Re: [PATCH 2/6] PCI/DPC: Leave interrupts enabled while handling event

Quoted 16 lines · Oza Pawandeep

+ Oza

Quoted 13 lines · Sinan Kaya

drivers/pci/pcie/pcie-dpc.c static irqreturn_t dpc_irq(int irq, void *context) · line 219

Quoted 2 lines · Keith Busch
drivers/pci/pcie/pcie-dpc.c +2 -0
+     if (!work_busy(&dpc->work))
+             schedule_work(&dpc->work);

Isn't there a race condition between the time that dpc_work() clears PCI_EXP_DPC_STATUS register and when work actually completes by returning from dpc_work()?

If the interrupt arrives just in this window, this code will not schedule the work.

--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

besides, there is one more problem which I was debugging:
if RP doesnt support MSI, then legacy interrupts are not well handled and we get interrupt storm. this is because;
PCI_EXP_DPC_STATUS_INTERRUPT is being cleared in deferred work.
which should get cleared in dpc_irq interrupt handler.
I will post a patch for this, but I see there are lot of changes being made in dpc and aer lately.

Bjorn: Can you please help to review my AER/DPC series of patches so that I do not have to keep rebasing and do manual merge ?

Regards,
Oza.

#5 lore raw

Re: [PATCH 2/6] PCI/DPC: Leave interrupts enabled while handling event

Quoted 27 lines · On 2018-01-30 12:04, [email protected] wrote:
Quoted 16 lines · Oza Pawandeep

+ Oza

Quoted 13 lines · Sinan Kaya

drivers/pci/pcie/pcie-dpc.c static irqreturn_t dpc_irq(int irq, void *context) · line 219

Quoted 2 lines · Keith Busch
drivers/pci/pcie/pcie-dpc.c +2 -0
+     if (!work_busy(&dpc->work))
+             schedule_work(&dpc->work);

Isn't there a race condition between the time that dpc_work() clears PCI_EXP_DPC_STATUS register and when work actually completes by returning from dpc_work()?

If the interrupt arrives just in this window, this code will not schedule the work.

--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

besides, there is one more problem which I was debugging:
if RP doesnt support MSI, then legacy interrupts are not well handled and we get interrupt storm.
this is because;
PCI_EXP_DPC_STATUS_INTERRUPT is being cleared in deferred work. which should get cleared in dpc_irq interrupt handler.
I will post a patch for this, but I see there are lot of changes being made in dpc and aer lately.

Quoted 5 lines

Bjorn: Can you please help to review my AER/DPC series of patches so that I do not have to keep rebasing and do manual merge ?

Regards,
Oza.

the DPC code assumes that the interrupt will be edge triggered (MSI), and clears interrupt in deferred work. but for SPI (level triggered) it should be cleared in dpc_irq.

Regards,
Oza.

author
#6 lore raw

Re: [PATCH 2/6] PCI/DPC: Leave interrupts enabled while handling event

In reply to [email protected]
Quoted 24 lines · On Mon, Jan 29, 2018 at 10:34:44PM -0800, [email protected] wrote:
Quoted 22 lines · Oza Pawandeep

+ Oza

Quoted 18 lines · Sinan Kaya

drivers/pci/pcie/pcie-dpc.c static irqreturn_t dpc_irq(int irq, void *context) · line 219

Quoted 2 lines · Keith Busch
drivers/pci/pcie/pcie-dpc.c +2 -0
+     if (!work_busy(&dpc->work))
+             schedule_work(&dpc->work);

Isn't there a race condition between the time that dpc_work() clears PCI_EXP_DPC_STATUS
register and when work actually completes by returning from dpc_work()?

If the interrupt arrives just in this window, this code will not schedule the work.

--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Quoted 6 lines

besides, there is one more problem which I was debugging:
if RP doesnt support MSI, then legacy interrupts are not well handled and we get interrupt storm.
this is because;
PCI_EXP_DPC_STATUS_INTERRUPT is being cleared in deferred work. which should get cleared in dpc_irq interrupt handler.

Okay, thanks for that information. I haven't got a DPC capable device that supports INTx, so that's a gap in my current testing capabilities.

It looks like as you're suggesting, we should clear DPC Interrupt Status in the top-half IRQ handler, and we should not see another DPC interrupt raised until after we clear DPC Trigger Status in the bottom-half.

#7 lore raw

Re: [PATCH 2/6] PCI/DPC: Leave interrupts enabled while handling event

Quoted 32 lines · Keith Busch
Quoted 24 lines · On Mon, Jan 29, 2018 at 10:34:44PM -0800, [email protected] wrote:
Quoted 22 lines · Oza Pawandeep

+ Oza

Quoted 18 lines · Sinan Kaya

drivers/pci/pcie/pcie-dpc.c static irqreturn_t dpc_irq(int irq, void *context) · line 219

Quoted 2 lines · Keith Busch
drivers/pci/pcie/pcie-dpc.c +2 -0
+     if (!work_busy(&dpc->work))
+             schedule_work(&dpc->work);

Isn't there a race condition between the time that dpc_work() clears PCI_EXP_DPC_STATUS
register and when work actually completes by returning from dpc_work()?

If the interrupt arrives just in this window, this code will not schedule the work.

--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Quoted 6 lines

besides, there is one more problem which I was debugging:
if RP doesnt support MSI, then legacy interrupts are not well handled and we get interrupt storm.
this is because;
PCI_EXP_DPC_STATUS_INTERRUPT is being cleared in deferred work. which should get cleared in dpc_irq interrupt handler.

Quoted 6 lines

Okay, thanks for that information. I haven't got a DPC capable device that supports INTx, so that's a gap in my current testing capabilities.

It looks like as you're suggesting, we should clear DPC Interrupt Status in the top-half IRQ handler, and we should not see another DPC interrupt raised until after we clear DPC Trigger Status in the bottom-half.

yes that's right. I am preparing a patch with that, and testing it on our platform. please let me know on what branch should I post it ?
because other mail from Bjorn suggests me to post on pci/master. since he will take care of merging it.

Regards,
Oza.

#8 lore raw

Re: [PATCH 2/6] PCI/DPC: Leave interrupts enabled while handling event

In reply to [email protected]
Quoted 2 lines · On Tue, Jan 30, 2018 at 12:04:44PM +0530, [email protected] wrote:

Bjorn: Can you please help to review my AER/DPC series of patches so that I do not have to keep rebasing and do manual merge ?

They're in the queue and I'll get to them this cycle.

You don't need to rebase them unless you change your patches to actually require something from another branch. If there are conflicts with other patches that are in-flight, I'll resolve those myself.

Since your patches will probably land in the v4.17 merge, it *will* be nice if you rebase them when I update the pci/master branch, which will probably be to v4.16-rc1. But unless you *depend* on features in another branch, it's easiest if you base them on pci/master, which generally stays the same throughout the cycle.

Bjorn

#9 lore raw

Re: [PATCH 2/6] PCI/DPC: Leave interrupts enabled while handling event

Quoted 11 lines · Bjorn Helgaas
Quoted 2 lines · On Tue, Jan 30, 2018 at 12:04:44PM +0530, [email protected] wrote:

Bjorn: Can you please help to review my AER/DPC series of patches so that I do not have to keep rebasing and do manual merge ?

They're in the queue and I'll get to them this cycle.

You don't need to rebase them unless you change your patches to actually require something from another branch. If there are conflicts with other patches that are in-flight, I'll resolve those myself.

Quoted 7 lines

Since your patches will probably land in the v4.17 merge, it *will* be nice if you rebase them when I update the pci/master branch, which will probably be to v4.16-rc1. But unless you *depend* on features in another branch, it's easiest if you base them on pci/master, which generally stays the same throughout the cycle.

Bjorn

sure, will rebase them on top of pci/master.

Regards,
Oza.

author
#10 lore raw

PATCH 3/6 PCI/DPC: Remove rp_pio_status from dpc struct

In reply to Keith Busch

we don't need to save the rp pio status across multiple contexts anymore as all DPC event handling occurs in a single work queue context.

Signed-off-by Keith Busch <[email protected]>
---
 drivers/pci/pcie/pcie-dpc.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)
drivers/pci/pcie/pcie-dpc.c +11 -12
index cf0398ccaeb6..f0f5f5ba71ff 100644
@@ -19,7 +19,6 @@ struct dpc_dev {
 	struct work_struct	work;
 	u16			cap_pos;
 	bool			rp_extensions;
-	u32			rp_pio_status;
 	u8			rp_log_size;
 };
 

@@ -45,7 +44,7 @@ static const char * const rp_pio_error_string[] = {
 	"Memory Request Completion Timeout",		 /* Bit Position 18 */
 };
 
-static void dpc_process_rp_pio_error(struct dpc_dev *dpc);
+static u32 dpc_process_rp_pio_error(struct dpc_dev *dpc);
 
 static int dpc_wait_rp_inactive(struct dpc_dev *dpc)
 {

@@ -90,6 +89,7 @@ static void dpc_work(struct work_struct *work)
 	struct pci_dev *dev, *temp, *pdev = dpc->dev->port;
 	struct pci_bus *parent = pdev->subordinate;
 	u16 cap = dpc->cap_pos, status, source, reason, ext_reason;
+	u32 pio_status = 0;
 
 	pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
 	pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID, &source);

@@ -109,7 +109,7 @@ static void dpc_work(struct work_struct *work)
 				     "reserved error");
 
 	if (dpc->rp_extensions && reason == 3 && ext_reason == 0)
-		dpc_process_rp_pio_error(dpc);
+		pio_status = dpc_process_rp_pio_error(dpc);
 
 	pci_lock_rescan_remove();
 	list_for_each_entry_safe_reverse(dev, temp, &parent->devices,

@@ -127,17 +127,15 @@ static void dpc_work(struct work_struct *work)
 	dpc_wait_link_inactive(dpc);
 	if (dpc->rp_extensions && dpc_wait_rp_inactive(dpc))
 		return;
-	if (dpc->rp_extensions && dpc->rp_pio_status) {
+	if (dpc->rp_extensions && pio_status)
 		pci_write_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS,
-				       dpc->rp_pio_status);
-		dpc->rp_pio_status = 0;
-	}
+				       pio_status);
 
 	pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS,
 		PCI_EXP_DPC_STATUS_TRIGGER | PCI_EXP_DPC_STATUS_INTERRUPT);
 }
 
-static void dpc_process_rp_pio_error(struct dpc_dev *dpc)
+static u32 dpc_process_rp_pio_error(struct dpc_dev *dpc)
 {
 	struct device *dev = &dpc->dev->device;
 	struct pci_dev *pdev = dpc->dev->port;

@@ -148,14 +146,14 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc)
 	int i;
 	u32 dw0, dw1, dw2, dw3;
 	u32 log;
-	u32 prefix;
+	u32 prefix, pio_status;
 
 	pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS, &status);
 	pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_MASK, &mask);
 	dev_err(dev, "rp_pio_status: %#010x, rp_pio_mask: %#010x\n",
 		status, mask);
 
-	dpc->rp_pio_status = status;
+	pio_status = status;
 
 	pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SEVERITY, &sev);
 	pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SYSERROR, &syserr);

@@ -175,7 +173,7 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc)
 	}
 
 	if (dpc->rp_log_size < 4)
-		return;
+		return pio_status;
 	pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG,
 			      &dw0);
 	pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 4,

@@ -188,7 +186,7 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc)
 		dw0, dw1, dw2, dw3);
 
 	if (dpc->rp_log_size < 5)
-		return;
+		return pio_status;
 	pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_IMPSPEC_LOG, &log);
 	dev_err(dev, "RP PIO ImpSpec Log %#010x\n", log);
 

@@ -197,6 +195,7 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc)
 			cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG, &prefix);
 		dev_err(dev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix);
 	}
+	return pio_status;
 }
 
 static irqreturn_t dpc_irq(int irq, void *context)
Signature · 1 line
2.14.3
author
#11 lore raw

PATCH 4/6 PCI/DPC: Cleanup declarations

In reply to Keith Busch

Combining declarations of the same type and removing function forward declaration. No functional change.

Signed-off-by Keith Busch <[email protected]>
---
 drivers/pci/pcie/pcie-dpc.c | 116 +++++++++++++++++++++-----------------------
 1 file changed, 55 insertions(+), 61 deletions(-)
drivers/pci/pcie/pcie-dpc.c +55 -61
index f0f5f5ba71ff..8c28840b6b91 100644
@@ -44,8 +44,6 @@ static const char * const rp_pio_error_string[] = {
 	"Memory Request Completion Timeout",		 /* Bit Position 18 */
 };
 
-static u32 dpc_process_rp_pio_error(struct dpc_dev *dpc);
-
 static int dpc_wait_rp_inactive(struct dpc_dev *dpc)
 {
 	unsigned long timeout = jiffies + HZ;

@@ -83,70 +81,14 @@ static void dpc_wait_link_inactive(struct dpc_dev *dpc)
 		dev_warn(dev, "Link state not disabled for DPC event\n");
 }
 
-static void dpc_work(struct work_struct *work)
-{
-	struct dpc_dev *dpc = container_of(work, struct dpc_dev, work);
-	struct pci_dev *dev, *temp, *pdev = dpc->dev->port;
-	struct pci_bus *parent = pdev->subordinate;
-	u16 cap = dpc->cap_pos, status, source, reason, ext_reason;
-	u32 pio_status = 0;
-
-	pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
-	pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID, &source);
-
-	dev_info(&pdev->dev, "DPC containment event, status:%#06x source:%#06x\n",
-		status, source);
-
-	reason = (status & PCI_EXP_DPC_STATUS_TRIGGER_RSN) >> 1;
-	ext_reason = (status & PCI_EXP_DPC_STATUS_TRIGGER_RSN_EXT) >> 5;
-
-	dev_warn(&pdev->dev, "DPC %s detected, remove downstream devices\n",
-		 (reason == 0) ? "unmasked uncorrectable error" :
-		 (reason == 1) ? "ERR_NONFATAL" :
-		 (reason == 2) ? "ERR_FATAL" :
-		 (ext_reason == 0) ? "RP PIO error" :
-		 (ext_reason == 1) ? "software trigger" :
-				     "reserved error");
-
-	if (dpc->rp_extensions && reason == 3 && ext_reason == 0)
-		pio_status = dpc_process_rp_pio_error(dpc);
-
-	pci_lock_rescan_remove();
-	list_for_each_entry_safe_reverse(dev, temp, &parent->devices,
-					 bus_list) {
-		pci_dev_get(dev);
-		pci_dev_set_disconnected(dev, NULL);
-		if (pci_has_subordinate(dev))
-			pci_walk_bus(dev->subordinate,
-				     pci_dev_set_disconnected, NULL);
-		pci_stop_and_remove_bus_device(dev);
-		pci_dev_put(dev);
-	}
-	pci_unlock_rescan_remove();
-
-	dpc_wait_link_inactive(dpc);
-	if (dpc->rp_extensions && dpc_wait_rp_inactive(dpc))
-		return;
-	if (dpc->rp_extensions && pio_status)
-		pci_write_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS,
-				       pio_status);
-
-	pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS,
-		PCI_EXP_DPC_STATUS_TRIGGER | PCI_EXP_DPC_STATUS_INTERRUPT);
-}
-
 static u32 dpc_process_rp_pio_error(struct dpc_dev *dpc)
 {
 	struct device *dev = &dpc->dev->device;
 	struct pci_dev *pdev = dpc->dev->port;
-	u16 cap = dpc->cap_pos;
-	u32 status, mask;
-	u32 sev, syserr, exc;
-	u16 dpc_status, first_error;
+	u16 cap = dpc->cap_pos, dpc_status, first_error;
+	u32 status, mask, sev, syserr, exc, dw0, dw1, dw2, dw3, log, prefix,
+		pio_status;
 	int i;
-	u32 dw0, dw1, dw2, dw3;
-	u32 log;
-	u32 prefix, pio_status;
 
 	pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS, &status);
 	pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_MASK, &mask);

@@ -198,6 +140,58 @@ static u32 dpc_process_rp_pio_error(struct dpc_dev *dpc)
 	return pio_status;
 }
 
+static void dpc_work(struct work_struct *work)
+{
+	struct dpc_dev *dpc = container_of(work, struct dpc_dev, work);
+	struct pci_dev *dev, *temp, *pdev = dpc->dev->port;
+	struct pci_bus *parent = pdev->subordinate;
+	u16 cap = dpc->cap_pos, status, source, reason, ext_reason;
+	u32 pio_status = 0;
+
+	pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
+	pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID, &source);
+
+	dev_info(&pdev->dev, "DPC containment event, status:%#06x source:%#06x\n",
+		status, source);
+
+	reason = (status & PCI_EXP_DPC_STATUS_TRIGGER_RSN) >> 1;
+	ext_reason = (status & PCI_EXP_DPC_STATUS_TRIGGER_RSN_EXT) >> 5;
+
+	dev_warn(&pdev->dev, "DPC %s detected, remove downstream devices\n",
+		 (reason == 0) ? "unmasked uncorrectable error" :
+		 (reason == 1) ? "ERR_NONFATAL" :
+		 (reason == 2) ? "ERR_FATAL" :
+		 (ext_reason == 0) ? "RP PIO error" :
+		 (ext_reason == 1) ? "software trigger" :
+				     "reserved error");
+
+	if (dpc->rp_extensions && reason == 3 && ext_reason == 0)
+		pio_status = dpc_process_rp_pio_error(dpc);
+
+	pci_lock_rescan_remove();
+	list_for_each_entry_safe_reverse(dev, temp, &parent->devices,
+					 bus_list) {
+		pci_dev_get(dev);
+		pci_dev_set_disconnected(dev, NULL);
+		if (pci_has_subordinate(dev))
+			pci_walk_bus(dev->subordinate,
+				     pci_dev_set_disconnected, NULL);
+		pci_stop_and_remove_bus_device(dev);
+		pci_dev_put(dev);
+	}
+	pci_unlock_rescan_remove();
+
+	dpc_wait_link_inactive(dpc);
+	if (dpc->rp_extensions && dpc_wait_rp_inactive(dpc))
+		return;
+	if (dpc->rp_extensions && pio_status)
+		pci_write_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS,
+				       pio_status);
+
+	pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS,
+		PCI_EXP_DPC_STATUS_TRIGGER | PCI_EXP_DPC_STATUS_INTERRUPT);
+}
+
 static irqreturn_t dpc_irq(int irq, void *context)
 {
 	struct dpc_dev *dpc = (struct dpc_dev *)context;
Signature · 1 line
2.14.3
author
#12 lore raw

PATCH 5/6 PCI/DPC: Enable ERR_COR

In reply to Keith Busch

A DPC port may be configured to send ERR_COR message when the triggered. This patch enables this feature so additional notification of the event is possible.

Signed-off-by Keith Busch <[email protected]>
---
 drivers/pci/pcie/pcie-dpc.c   | 5 ++++-
 include/uapi/linux/pci_regs.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)
drivers/pci/pcie/pcie-dpc.c +4 -1
index 8c28840b6b91..bf9f7e0f3f90 100644
@@ -255,7 +255,10 @@ static int dpc_probe(struct pcie_device *dev)
 		}
 	}
 
-	ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_NONFATAL | PCI_EXP_DPC_CTL_INT_EN;
+	ctl = (ctl & 0xfff4) |
+		PCI_EXP_DPC_CTL_EN_NONFATAL |
+		PCI_EXP_DPC_CTL_INT_EN |
+		PCI_EXP_DPC_CTL_ERR_COR;
 	pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, ctl);
 
 	dev_info(device, "DPC error containment capabilities: Int Msg #%d, RPExt%c PoisonedTLP%c SwTrigger%c RP PIO Log %d, DL_ActiveErr%c\n",
include/uapi/linux/pci_regs.h +1 -0
index 0c79eac5e9b8..9cfcecdc3ec7 100644
@@ -980,6 +980,7 @@
 #define PCI_EXP_DPC_CTL			6	/* DPC control */
 #define  PCI_EXP_DPC_CTL_EN_NONFATAL 	0x0002	/* Enable trigger on ERR_NONFATAL message */
 #define  PCI_EXP_DPC_CTL_INT_EN 	0x0008	/* DPC Interrupt Enable */
+#define  PCI_EXP_DPC_CTL_ERR_COR 	0x0010	/* DPC ERR_COR Enable */
 
 #define PCI_EXP_DPC_STATUS		8	/* DPC Status */
 #define  PCI_EXP_DPC_STATUS_TRIGGER	    0x0001 /* Trigger Status */
Signature · 1 line
2.14.3
author
#13 lore raw

PATCH 6/6 PCI/DPC: Print AER status in DPC event handling

In reply to Keith Busch

A DPC enabled device suppresses ERR_(NON)FATAL messages, preventing the AER handler from reporting error details. If the DPC trigger reason says the downstream port detected the error, this patch has the DPC driver collect the AER uncorrectable status for logging, then clears the status.

Signed-off-by Keith Busch <[email protected]>

---

This is dependent on this patch from pci/aer branch, but I don't see this as being merged in pci/next.

https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/aer&id=39a6fa4a6cb35e2d93fc2374f9e33e055826d06f

 drivers/pci/pcie/pcie-dpc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/pci/pcie/pcie-dpc.c +6 -1
index bf9f7e0f3f90..e124701050f4 100644
@@ -142,6 +142,7 @@ static u32 dpc_process_rp_pio_error(struct dpc_dev *dpc)
 
 static void dpc_work(struct work_struct *work)
 {
+	struct aer_err_info info;
 	struct dpc_dev *dpc = container_of(work, struct dpc_dev, work);
 	struct pci_dev *dev, *temp, *pdev = dpc->dev->port;
 	struct pci_bus *parent = pdev->subordinate;

@@ -165,8 +166,12 @@ static void dpc_work(struct work_struct *work)
 		 (ext_reason == 1) ? "software trigger" :
 				     "reserved error");
 
-	if (dpc->rp_extensions && reason == 3 && ext_reason == 0)
+	if (dpc->rp_extensions && reason == 3 && ext_reason == 0) {
 		pio_status = dpc_process_rp_pio_error(dpc);
+	} else if (reason == 0 && aer_get_device_error_info(pdev, &info)) {
+		aer_print_error(pdev, &info);
+		pci_cleanup_aer_uncorrect_error_status(pdev);
+	}
 
 	pci_lock_rescan_remove();
 	list_for_each_entry_safe_reverse(dev, temp, &parent->devices,
Signature · 1 line
2.14.3