From f8d46c89c86fa11c60c05e101a3a4c7458623103 Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Wed, 20 Jun 2018 15:38:27 -0600 Subject: [PATCH 1/6] PCI/DPC: Leave interrupts enabled while handling event Now that the DPC driver clears the interrupt status before exiting the IRQ handler, we don't need to abuse the DPC control register to know if a shared interrupt is for a new DPC event: a DPC port can not trigger a second interrupt until the host clears the trigger status later in the work queue handler. Signed-off-by: Keith Busch Signed-off-by: Bjorn Helgaas Reviewed-by: Sinan Kaya Reviewed-by: Oza Pawandeep --- drivers/pci/pcie/dpc.c | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 921ed979109d..972aac892846 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -77,7 +77,7 @@ static pci_ers_result_t dpc_reset_link(struct pci_dev *pdev) struct dpc_dev *dpc; struct pcie_device *pciedev; struct device *devdpc; - u16 cap, ctl; + u16 cap; /* * DPC disables the Link automatically in hardware, so it has @@ -105,10 +105,6 @@ static pci_ers_result_t dpc_reset_link(struct pci_dev *pdev) pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS, PCI_EXP_DPC_STATUS_TRIGGER); - 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); - return PCI_ERS_RESULT_RECOVERED; } @@ -183,16 +179,11 @@ 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; - - 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, source, reason, ext_reason; 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)) { @@ -201,9 +192,6 @@ 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); - pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID, &source); @@ -226,9 +214,8 @@ static irqreturn_t dpc_irq(int irq, void *context) pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS, PCI_EXP_DPC_STATUS_INTERRUPT); - - schedule_work(&dpc->work); - + if (status & PCI_EXP_DPC_STATUS_TRIGGER) + schedule_work(&dpc->work); return IRQ_HANDLED; } From 0c27e28f77179548aa3d25a8f7a3dda22fd3e080 Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Mon, 16 Jul 2018 17:05:02 -0500 Subject: [PATCH 2/6] PCI/DPC: Defer event handling to work queue Move all event handling to the existing work queue, which will make it simpler to pass event information to the handler. Signed-off-by: Keith Busch Signed-off-by: Bjorn Helgaas Reviewed-by: Sinan Kaya Reviewed-by: Oza Pawandeep --- drivers/pci/pcie/dpc.c | 44 +++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 972aac892846..5f5fac279c75 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -108,14 +108,6 @@ static pci_ers_result_t dpc_reset_link(struct pci_dev *pdev) return PCI_ERS_RESULT_RECOVERED; } -static void dpc_work(struct work_struct *work) -{ - struct dpc_dev *dpc = container_of(work, struct dpc_dev, work); - struct pci_dev *pdev = dpc->dev->port; - - /* We configure DPC so it only triggers on ERR_FATAL */ - pcie_do_fatal_recovery(pdev, PCIE_PORT_SERVICE_DPC); -} static void dpc_process_rp_pio_error(struct dpc_dev *dpc) { @@ -174,33 +166,21 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc) } } -static irqreturn_t dpc_irq(int irq, void *context) +static void dpc_work(struct work_struct *work) { - struct dpc_dev *dpc = (struct dpc_dev *)context; + struct dpc_dev *dpc = container_of(work, struct dpc_dev, work); struct pci_dev *pdev = dpc->dev->port; struct device *dev = &dpc->dev->device; u16 cap = dpc->cap_pos, status, source, reason, ext_reason; pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status); - - if (!(status & PCI_EXP_DPC_STATUS_INTERRUPT) || status == (u16)(~0)) - return IRQ_NONE; - - if (!(status & PCI_EXP_DPC_STATUS_TRIGGER)) { - pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS, - PCI_EXP_DPC_STATUS_INTERRUPT); - return IRQ_HANDLED; - } - - pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID, - &source); + 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); + 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" : @@ -208,10 +188,26 @@ static irqreturn_t dpc_irq(int irq, void *context) (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); + /* We configure DPC so it only triggers on ERR_FATAL */ + pcie_do_fatal_recovery(pdev, PCIE_PORT_SERVICE_DPC); +} + +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, status; + + pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status); + + if (!(status & PCI_EXP_DPC_STATUS_INTERRUPT) || status == (u16)(~0)) + return IRQ_NONE; + pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS, PCI_EXP_DPC_STATUS_INTERRUPT); if (status & PCI_EXP_DPC_STATUS_TRIGGER) From f1d16b17568e46f61cc59e2d0725b0fca856a34f Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Mon, 16 Jul 2018 17:05:03 -0500 Subject: [PATCH 3/6] PCI/DPC: Remove rp_pio_status from dpc struct We don't need to save the rp pio status across multiple contexts as all DPC event handling occurs in a single work queue context. Signed-off-by: Keith Busch Signed-off-by: Bjorn Helgaas Reviewed-by: Sinan Kaya Reviewed-by: Oza Pawandeep --- drivers/pci/pcie/dpc.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 5f5fac279c75..1b0b25ba947c 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/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; }; @@ -96,11 +95,6 @@ static pci_ers_result_t dpc_reset_link(struct pci_dev *pdev) if (dpc->rp_extensions && dpc_wait_rp_inactive(dpc)) return PCI_ERS_RESULT_DISCONNECT; - if (dpc->rp_extensions && dpc->rp_pio_status) { - pci_write_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS, - dpc->rp_pio_status); - dpc->rp_pio_status = 0; - } pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS, PCI_EXP_DPC_STATUS_TRIGGER); @@ -122,8 +116,6 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc) dev_err(dev, "rp_pio_status: %#010x, rp_pio_mask: %#010x\n", status, mask); - dpc->rp_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); pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_EXCEPTION, &exc); @@ -134,15 +126,14 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc) pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &dpc_status); first_error = (dpc_status & 0x1f00) >> 8; - status &= ~mask; for (i = 0; i < ARRAY_SIZE(rp_pio_error_string); i++) { - if (status & (1 << i)) + if ((status & ~mask) & (1 << i)) dev_err(dev, "[%2d] %s%s\n", i, rp_pio_error_string[i], first_error == i ? " (First)" : ""); } if (dpc->rp_log_size < 4) - return; + goto clear_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, @@ -155,7 +146,7 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc) dw0, dw1, dw2, dw3); if (dpc->rp_log_size < 5) - return; + goto clear_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); @@ -164,6 +155,8 @@ 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); } + clear_status: + pci_write_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS, status); } static void dpc_work(struct work_struct *work) From 8aefa9b0d9105d70eb8799d2b4feffd784e22bae Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Mon, 16 Jul 2018 17:05:05 -0500 Subject: [PATCH 4/6] PCI/DPC: Print AER status in DPC event handling 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, collect the AER uncorrectable status for logging, then clear the status. Signed-off-by: Keith Busch Signed-off-by: Bjorn Helgaas Reviewed-by: Sinan Kaya Reviewed-by: Oza Pawandeep --- drivers/pci/pcie/dpc.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 1b0b25ba947c..f6098dd171f3 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -6,6 +6,7 @@ * Copyright (C) 2016 Intel Corp. */ +#include #include #include #include @@ -161,6 +162,7 @@ static void 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 *pdev = dpc->dev->port; struct device *dev = &dpc->dev->device; @@ -185,6 +187,10 @@ static void dpc_work(struct work_struct *work) /* show RP PIO error detail information */ if (dpc->rp_extensions && reason == 3 && ext_reason == 0) 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); + } /* We configure DPC so it only triggers on ERR_FATAL */ pcie_do_fatal_recovery(pdev, PCIE_PORT_SERVICE_DPC); From 738c4e411dadbe341f80f30d56518b68ae1e605f Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Mon, 16 Jul 2018 17:05:06 -0500 Subject: [PATCH 5/6] PCI/DPC: Use threaded IRQ for bottom half handling Remove the work struct that was being used to handle a DPC event and use a threaded IRQ instead. Signed-off-by: Keith Busch Signed-off-by: Bjorn Helgaas Reviewed-by: Sinan Kaya Reviewed-by: Oza Pawandeep --- drivers/pci/pcie/dpc.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index f6098dd171f3..fd6b96ca1c2d 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -17,7 +17,6 @@ struct dpc_dev { struct pcie_device *dev; - struct work_struct work; u16 cap_pos; bool rp_extensions; u8 rp_log_size; @@ -160,10 +159,10 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc) pci_write_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS, status); } -static void dpc_work(struct work_struct *work) +static irqreturn_t dpc_handler(int irq, void *context) { struct aer_err_info info; - struct dpc_dev *dpc = container_of(work, struct dpc_dev, work); + struct dpc_dev *dpc = context; struct pci_dev *pdev = dpc->dev->port; struct device *dev = &dpc->dev->device; u16 cap = dpc->cap_pos, status, source, reason, ext_reason; @@ -194,6 +193,8 @@ static void dpc_work(struct work_struct *work) /* We configure DPC so it only triggers on ERR_FATAL */ pcie_do_fatal_recovery(pdev, PCIE_PORT_SERVICE_DPC); + + return IRQ_HANDLED; } static irqreturn_t dpc_irq(int irq, void *context) @@ -210,7 +211,7 @@ static irqreturn_t dpc_irq(int irq, void *context) pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS, PCI_EXP_DPC_STATUS_INTERRUPT); if (status & PCI_EXP_DPC_STATUS_TRIGGER) - schedule_work(&dpc->work); + return IRQ_WAKE_THREAD; return IRQ_HANDLED; } @@ -232,11 +233,11 @@ static int dpc_probe(struct pcie_device *dev) dpc->cap_pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_DPC); dpc->dev = dev; - INIT_WORK(&dpc->work, dpc_work); set_service_data(dev, dpc); - status = devm_request_irq(device, dev->irq, dpc_irq, IRQF_SHARED, - "pcie-dpc", dpc); + status = devm_request_threaded_irq(device, dev->irq, dpc_irq, + dpc_handler, IRQF_SHARED, + "pcie-dpc", dpc); if (status) { dev_warn(device, "request IRQ%d failed: %d\n", dev->irq, status); From e77b8216a2f9bec430ec46c34763567329ddafd9 Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Mon, 16 Jul 2018 17:05:07 -0500 Subject: [PATCH 6/6] PCI/DPC: Remove indirection waiting for inactive link Simplify waiting for the contained link to become inactive, removing the indirection to a unnecessary DPC-specific handler. Signed-off-by: Keith Busch Signed-off-by: Bjorn Helgaas Reviewed-by: Sinan Kaya Reviewed-by: Oza Pawandeep --- drivers/pci/pcie/dpc.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index fd6b96ca1c2d..f03279fc87cd 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -64,18 +64,12 @@ static int dpc_wait_rp_inactive(struct dpc_dev *dpc) return 0; } -static void dpc_wait_link_inactive(struct dpc_dev *dpc) -{ - struct pci_dev *pdev = dpc->dev->port; - - pcie_wait_for_link(pdev, false); -} - static pci_ers_result_t dpc_reset_link(struct pci_dev *pdev) { struct dpc_dev *dpc; struct pcie_device *pciedev; struct device *devdpc; + u16 cap; /* @@ -91,7 +85,7 @@ static pci_ers_result_t dpc_reset_link(struct pci_dev *pdev) * Wait until the Link is inactive, then clear DPC Trigger Status * to allow the Port to leave DPC. */ - dpc_wait_link_inactive(dpc); + pcie_wait_for_link(pdev, false); if (dpc->rp_extensions && dpc_wait_rp_inactive(dpc)) return PCI_ERS_RESULT_DISCONNECT;