Re: [PATCH v10 4/7] PCI/DPC: Unify and plumb error handling into DPC
On 2018-02-23 00:09, Christoph Hellwig wrote:
On Thu, Feb 22, 2018 at 09:32:09PM +0530, Oza Pawandeep wrote:
Current DPC driver does not do recovery, e.g. calling end-point's
driver's
callbacks, which sanitize the sw.
DPC driver implements link_reset callback, and calls pci_do_recovery.
Signed-off-by: Oza Pawandeep
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index a5a79f0..124f42e 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -343,6 +343,8 @@ static inline resource_size_t
pci_resource_alignment(struct pci_dev *dev,
void pci_enable_acs(struct pci_dev *dev);
/* PCI error reporting and recovery */
+#define DPC_FATAL 4
+
void pci_do_recovery(struct pci_dev *dev, int severity);
#ifdef CONFIG_PCIEASPM
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 38e40c6..208b427 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -13,6 +13,7 @@
#include
#include "../pci.h"
#include "aer/aerdrv.h"
+#include "portdrv.h"
struct dpc_dev {
struct pcie_device *dev;
@@ -45,6 +46,60 @@ struct dpc_dev {
"Memory Request Completion Timeout", /* Bit Position 18 */
};
+static int find_dpc_dev_iter(struct device *device, void *data)
+{
+ struct pcie_port_service_driver *service_driver;
+ struct device **dev;
+
+ dev = (struct device **) data;
+
+ if (device->bus == &pcie_port_bus_type && device->driver) {
+ service_driver = to_service_driver(device->driver);
Please move the initial assignment to the declaration line to
clean this up a bit:
struct device **dev = (struct device **)data;
Same thing happens a couple more times in this patch.
+EXPORT_SYMBOL(pci_find_dpc_service);
EXPORT_SYMBOL_GPL for PCI layer internals again - and ditto for
probably about everything in this series.
will take care of all these comments.
#if IS_ENABLED(CONFIG_PCIEAER)
- /* Use the aer driver of the component firstly */
- driver = pci_find_aer_service(udev);
+ if ((severity == AER_FATAL) ||
+ (severity == AER_NONFATAL) ||
+ (severity == AER_CORRECTABLE))
+ driver = pci_find_aer_service(udev);
#endif
No need for the inner braces here.
- if (severity == AER_FATAL)
+ if ((severity == AER_FATAL) ||
+ (severity == DPC_FATAL))
state = pci_channel_io_frozen;
else
state = pci_channel_io_normal;
Same here and a few more times below.
will take care
Thanks,
Oza.
Re: [PATCH v10 4/7] PCI/DPC: Unify and plumb error handling into DPC
On Thu, Feb 22, 2018 at 09:32:09PM +0530, Oza Pawandeep wrote:
> Current DPC driver does not do recovery, e.g. calling end-point's driver's
> callbacks, which sanitize the sw.
>
> DPC driver implements link_reset callback, and calls pci_do_recovery.
>
> Signed-off-by: Oza Pawandeep
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index a5a79f0..124f42e 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -343,6 +343,8 @@ static inline resource_size_t
> pci_resource_alignment(struct pci_dev *dev,
> void pci_enable_acs(struct pci_dev *dev);
>
> /* PCI error reporting and recovery */
> +#define DPC_FATAL4
> +
> void pci_do_recovery(struct pci_dev *dev, int severity);
>
> #ifdef CONFIG_PCIEASPM
> diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
> index 38e40c6..208b427 100644
> --- a/drivers/pci/pcie/pcie-dpc.c
> +++ b/drivers/pci/pcie/pcie-dpc.c
> @@ -13,6 +13,7 @@
> #include
> #include "../pci.h"
> #include "aer/aerdrv.h"
> +#include "portdrv.h"
>
> struct dpc_dev {
> struct pcie_device *dev;
> @@ -45,6 +46,60 @@ struct dpc_dev {
> "Memory Request Completion Timeout", /* Bit Position 18 */
> };
>
> +static int find_dpc_dev_iter(struct device *device, void *data)
> +{
> + struct pcie_port_service_driver *service_driver;
> + struct device **dev;
> +
> + dev = (struct device **) data;
> +
> + if (device->bus == &pcie_port_bus_type && device->driver) {
> + service_driver = to_service_driver(device->driver);
Please move the initial assignment to the declaration line to
clean this up a bit:
struct device **dev = (struct device **)data;
Same thing happens a couple more times in this patch.
> +EXPORT_SYMBOL(pci_find_dpc_service);
EXPORT_SYMBOL_GPL for PCI layer internals again - and ditto for
probably about everything in this series.
> #if IS_ENABLED(CONFIG_PCIEAER)
> - /* Use the aer driver of the component firstly */
> - driver = pci_find_aer_service(udev);
> + if ((severity == AER_FATAL) ||
> + (severity == AER_NONFATAL) ||
> + (severity == AER_CORRECTABLE))
> + driver = pci_find_aer_service(udev);
> #endif
No need for the inner braces here.
> - if (severity == AER_FATAL)
> + if ((severity == AER_FATAL) ||
> + (severity == DPC_FATAL))
> state = pci_channel_io_frozen;
> else
> state = pci_channel_io_normal;
Same here and a few more times below.
[PATCH v10 4/7] PCI/DPC: Unify and plumb error handling into DPC
Current DPC driver does not do recovery, e.g. calling end-point's driver's
callbacks, which sanitize the sw.
DPC driver implements link_reset callback, and calls pci_do_recovery.
Signed-off-by: Oza Pawandeep
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index a5a79f0..124f42e 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -343,6 +343,8 @@ static inline resource_size_t pci_resource_alignment(struct
pci_dev *dev,
void pci_enable_acs(struct pci_dev *dev);
/* PCI error reporting and recovery */
+#define DPC_FATAL 4
+
void pci_do_recovery(struct pci_dev *dev, int severity);
#ifdef CONFIG_PCIEASPM
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 38e40c6..208b427 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -13,6 +13,7 @@
#include
#include "../pci.h"
#include "aer/aerdrv.h"
+#include "portdrv.h"
struct dpc_dev {
struct pcie_device *dev;
@@ -45,6 +46,60 @@ struct dpc_dev {
"Memory Request Completion Timeout", /* Bit Position 18 */
};
+static int find_dpc_dev_iter(struct device *device, void *data)
+{
+ struct pcie_port_service_driver *service_driver;
+ struct device **dev;
+
+ dev = (struct device **) data;
+
+ if (device->bus == &pcie_port_bus_type && device->driver) {
+ service_driver = to_service_driver(device->driver);
+ if (service_driver->service == PCIE_PORT_SERVICE_DPC) {
+ *dev = device;
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+static struct device *pci_find_dpc_dev(struct pci_dev *pdev)
+{
+ struct device *dev = NULL;
+
+ device_for_each_child(&pdev->dev, &dev, find_dpc_dev_iter);
+
+ return dev;
+}
+
+static int find_dpc_service_iter(struct device *device, void *data)
+{
+ struct pcie_port_service_driver *service_driver, **drv;
+
+ drv = (struct pcie_port_service_driver **) data;
+
+ if (device->bus == &pcie_port_bus_type && device->driver) {
+ service_driver = to_service_driver(device->driver);
+ if (service_driver->service == PCIE_PORT_SERVICE_DPC) {
+ *drv = service_driver;
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+struct pcie_port_service_driver *pci_find_dpc_service(struct pci_dev *dev)
+{
+ struct pcie_port_service_driver *drv = NULL;
+
+ device_for_each_child(&dev->dev, &drv, find_dpc_service_iter);
+
+ return drv;
+}
+EXPORT_SYMBOL(pci_find_dpc_service);
+
static int dpc_wait_rp_inactive(struct dpc_dev *dpc)
{
unsigned long timeout = jiffies + HZ;
@@ -82,12 +137,25 @@ 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)
+/**
+ * dpc_reset_link - reset link DPC routine
+ * @dev: pointer to Root Port's pci_dev data structure
+ *
+ * Invoked by Port Bus driver when performing link reset at Root Port.
+ */
+static pci_ers_result_t dpc_reset_link(struct pci_dev *pdev)
{
- 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;
+ struct pci_dev *dev, *temp;
+ struct dpc_dev *dpc;
+ struct pcie_device *pciedev;
+ struct device *devdpc;
+ u16 cap, ctl;
+
+ devdpc = pci_find_dpc_dev(pdev);
+ pciedev = to_pcie_device(devdpc);
+ dpc = get_service_data(pciedev);
+ cap = dpc->cap_pos;
pci_lock_rescan_remove();
list_for_each_entry_safe_reverse(dev, temp, &parent->devices,
@@ -104,21 +172,31 @@ static void dpc_work(struct work_struct *work)
dpc_wait_link_inactive(dpc);
if (dpc->rp_extensions && dpc_wait_rp_inactive(dpc))
- return;
+ 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_write_config_word(pdev, dpc->cap_pos + 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);
+ ctl | PCI_EXP_DPC_CTL_INT_EN);
+
+ 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;
+
+ /* From DPC p
