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

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

[Keith Busch]

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 <keith.busch@intel.com>
---
 drivers/pci/pcie/pcie-dpc.c | 47 +++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index e53e1fa82e9d..ecdb76bc7b56 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -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;
-- 
2.14.3

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

[Keith Busch]

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 <keith.busch@intel.com>
---
 drivers/pci/pcie/pcie-dpc.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index ecdb76bc7b56..cf0398ccaeb6 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -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;
 }
 
-- 
2.14.3

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

[Sinan Kaya]

On 1/29/2018 4:31 PM, Keith Busch wrote:
> +	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.

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

[Oza Pawandeep]

+ Oza

On Tue, Jan 30, 2018 at 7:41 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
> On 1/29/2018 4:31 PM, Keith Busch wrote:
>> +     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.

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

[poza]

On 2018-01-30 11:59, Oza Pawandeep wrote:
> + Oza
> 
> On Tue, Jan 30, 2018 at 7:41 AM, Sinan Kaya <okaya@codeaurora.org> 
> wrote:
>> On 1/29/2018 4:31 PM, Keith Busch wrote:
>>> +     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.

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

[poza]

On 2018-01-30 12:04, poza@codeaurora.org wrote:
> On 2018-01-30 11:59, Oza Pawandeep wrote:
>> + Oza
>> 
>> On Tue, Jan 30, 2018 at 7:41 AM, Sinan Kaya <okaya@codeaurora.org> 
>> wrote:
>>> On 1/29/2018 4:31 PM, Keith Busch wrote:
>>>> +     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.

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.

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

[Keith Busch]

On Mon, Jan 29, 2018 at 10:34:44PM -0800, poza@codeaurora.org wrote:
> On 2018-01-30 11:59, Oza Pawandeep wrote:
> > + Oza
> > 
> > On Tue, Jan 30, 2018 at 7:41 AM, Sinan Kaya <okaya@codeaurora.org> 
> > wrote:
> >> On 1/29/2018 4:31 PM, Keith Busch wrote:
> >>> +     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.

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.

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

[poza]

On 2018-01-30 23:47, Keith Busch wrote:
> On Mon, Jan 29, 2018 at 10:34:44PM -0800, poza@codeaurora.org wrote:
>> On 2018-01-30 11:59, Oza Pawandeep wrote:
>> > + Oza
>> >
>> > On Tue, Jan 30, 2018 at 7:41 AM, Sinan Kaya <okaya@codeaurora.org>
>> > wrote:
>> >> On 1/29/2018 4:31 PM, Keith Busch wrote:
>> >>> +     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.
> 
> 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.

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

[Bjorn Helgaas]

On Tue, Jan 30, 2018 at 12:04:44PM +0530, poza@codeaurora.org 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

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

[poza]

On 2018-01-31 00:00, Bjorn Helgaas wrote:
> On Tue, Jan 30, 2018 at 12:04:44PM +0530, poza@codeaurora.org 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

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

Regards,
Oza.

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

[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 <keith.busch@intel.com>
---
 drivers/pci/pcie/pcie-dpc.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index cf0398ccaeb6..f0f5f5ba71ff 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -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)
-- 
2.14.3

[PATCH 4/6] PCI/DPC: Cleanup declarations

[Keith Busch]

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

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 drivers/pci/pcie/pcie-dpc.c | 116 +++++++++++++++++++++-----------------------
 1 file changed, 55 insertions(+), 61 deletions(-)

diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index f0f5f5ba71ff..8c28840b6b91 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -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;
-- 
2.14.3

[PATCH 5/6] PCI/DPC: Enable ERR_COR

[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 <keith.busch@intel.com>
---
 drivers/pci/pcie/pcie-dpc.c   | 5 ++++-
 include/uapi/linux/pci_regs.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 8c28840b6b91..bf9f7e0f3f90 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -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",
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 0c79eac5e9b8..9cfcecdc3ec7 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -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 */
-- 
2.14.3

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

[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 <keith.busch@intel.com>
---

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(-)

diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index bf9f7e0f3f90..e124701050f4 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -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,
-- 
2.14.3