This patch set brings in support for DPC and AER to co-exist and not to
race for recovery.
The current implementation of AER and error message broadcasting to the
EP driver is tightly coupled and limited to AER service driver.
It is important to factor out broadcasting and other link handling
callbacks. So that not only when AER gets triggered, but also when DPC get
triggered, or both get triggered simultaneously (for e.g. ERR_FATAL),
callbacks are handled appropriately.
having modularized the code, the race between AER and DPC is handled
gracefully.
for e.g. when DPC is active and kicked in, AER should not attempt to do
recovery, because DPC takes care of it.
DPC should enumerate the devices after recovering the link, which is
achieved by implementing error_resume callback.
Changes since v1:
Kbuild errors fixed:
> pci_find_dpc_dev made static
> ras_event.h updated
> pci_find_aer_service call with CONFIG check
> pci_find_dpc_service call with CONFIG check
Oza Pawandeep (4):
PCI/AER: factor out error reporting from AER
PCI/DPC/AER: Address Concurrency between AER and DPC
PCI/ERR: Do not do recovery if DPC service is active
PCI/DPC: Enumerate the devices after DPC trigger event
drivers/acpi/apei/ghes.c | 2 +-
drivers/pci/pcie/Makefile | 2 +-
drivers/pci/pcie/aer/aerdrv.h | 30 ---
drivers/pci/pcie/aer/aerdrv_core.c | 306 +------------------------
drivers/pci/pcie/aer/aerdrv_errprint.c | 27 ++-
drivers/pci/pcie/pcie-dpc.c | 127 ++++++++++-
drivers/pci/pcie/pcie-err.c | 399 +++++++++++++++++++++++++++++++++
drivers/pci/pcie/portdrv.h | 2 +
include/linux/aer.h | 4 -
include/linux/pci.h | 23 ++
include/ras/ras_event.h | 6 +-
11 files changed, 579 insertions(+), 349 deletions(-)
create mode 100644 drivers/pci/pcie/pcie-err.c
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.,
a Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
This patch addresses the race condition between AER and DPC for recovery.
Current DPC driver does not do recovery, e.g. calling end-point's driver's
callbacks, which sanitize the device.
DPC driver implements link_reset callback, and calls pci_do_recovery.
Signed-off-by: Oza Pawandeep <[email protected]>
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 2d976a6..68296ec 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -15,6 +15,9 @@
#include <linux/pci.h>
#include <linux/pcieport_if.h>
#include "../pci.h"
+#include "portdrv.h"
+
+static pci_ers_result_t dpc_reset_link(struct pci_dev *pdev);
struct rp_pio_header_log_regs {
u32 dw0;
@@ -67,6 +70,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;
@@ -104,11 +161,23 @@ static void dpc_wait_link_inactive(struct dpc_dev *dpc)
dev_warn(dev, "Link state not disabled for DPC event\n");
}
-static void interrupt_event_handler(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;
+ struct pci_dev *dev, *temp;
+ struct dpc_dev *dpc;
+ struct pcie_device *pciedev;
+ struct device *devdpc;
+
+ devdpc = pci_find_dpc_dev(pdev);
+ pciedev = to_pcie_device(devdpc);
+ dpc = get_service_data(pciedev);
pci_lock_rescan_remove();
list_for_each_entry_safe_reverse(dev, temp, &parent->devices,
@@ -125,7 +194,7 @@ static void interrupt_event_handler(struct work_struct *work)
dpc_wait_link_inactive(dpc);
if (dpc->rp && dpc_wait_rp_inactive(dpc))
- return;
+ return PCI_ERS_RESULT_DISCONNECT;
if (dpc->rp && dpc->rp_pio_status) {
pci_write_config_dword(pdev,
dpc->cap_pos + PCI_EXP_DPC_RP_PIO_STATUS,
@@ -135,6 +204,17 @@ static void interrupt_event_handler(struct work_struct *work)
pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_STATUS,
PCI_EXP_DPC_STATUS_TRIGGER | PCI_EXP_DPC_STATUS_INTERRUPT);
+
+ return PCI_ERS_RESULT_RECOVERED;
+}
+
+static void interrupt_event_handler(struct work_struct *work)
+{
+ struct dpc_dev *dpc = container_of(work, struct dpc_dev, work);
+ struct pci_dev *pdev = dpc->dev->port;
+
+ /* From DPC point of view error is always FATAL. */
+ pci_do_recovery(pdev, PCI_ERR_DPC_FATAL);
}
static void dpc_rp_pio_print_tlp_header(struct device *dev,
@@ -339,6 +419,7 @@ static void dpc_remove(struct pcie_device *dev)
.service = PCIE_PORT_SERVICE_DPC,
.probe = dpc_probe,
.remove = dpc_remove,
+ .reset_link = dpc_reset_link,
};
static int __init dpc_service_init(void)
diff --git a/drivers/pci/pcie/pcie-err.c b/drivers/pci/pcie/pcie-err.c
index a76a8bf..858c94c 100644
--- a/drivers/pci/pcie/pcie-err.c
+++ b/drivers/pci/pcie/pcie-err.c
@@ -176,7 +176,7 @@ static pci_ers_result_t pci_default_reset_link(struct pci_dev *dev)
return PCI_ERS_RESULT_RECOVERED;
}
-pci_ers_result_t pci_reset_link(struct pci_dev *dev)
+pci_ers_result_t pci_reset_link(struct pci_dev *dev, int severity)
{
struct pci_dev *udev;
pci_ers_result_t status;
@@ -190,9 +190,17 @@ pci_ers_result_t pci_reset_link(struct pci_dev *dev)
udev = dev->bus->self;
}
+
+ /* Use the service driver of the component firstly */
+#if IS_ENABLED(CONFIG_PCIEDPC)
+ if (severity == PCI_ERR_DPC_FATAL)
+ driver = pci_find_dpc_service(udev);
+#endif
#if IS_ENABLED(CONFIG_PCIEAER)
- /* Use the aer driver of the component firstly */
- driver = pci_find_aer_service(udev);
+ if ((severity == PCI_ERR_AER_FATAL) ||
+ (severity == PCI_ERR_AER_NONFATAL) ||
+ (severity == PCI_ERR_AER_CORRECTABLE))
+ driver = pci_find_aer_service(udev);
#endif
if (driver && driver->reset_link) {
@@ -282,7 +290,8 @@ void pci_do_recovery(struct pci_dev *dev, int severity)
mutex_lock(&pci_err_recovery_lock);
- if (severity == PCI_ERR_AER_FATAL)
+ if ((severity == PCI_ERR_AER_FATAL) ||
+ (severity == PCI_ERR_DPC_FATAL))
state = pci_channel_io_frozen;
else
state = pci_channel_io_normal;
@@ -292,8 +301,9 @@ void pci_do_recovery(struct pci_dev *dev, int severity)
"error_detected",
pci_report_error_detected);
- if (severity == PCI_ERR_AER_FATAL) {
- result = pci_reset_link(dev);
+ if ((severity == PCI_ERR_AER_FATAL) ||
+ (severity == PCI_ERR_DPC_FATAL)) {
+ result = pci_reset_link(dev, severity);
if (result != PCI_ERS_RESULT_RECOVERED)
goto failed;
}
diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index 4f1992d..b013e24 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -80,4 +80,5 @@ static inline void pcie_port_platform_notify(struct pci_dev *port, int *mask){}
#endif /* !CONFIG_ACPI */
struct pcie_port_service_driver *pci_find_aer_service(struct pci_dev *dev);
+struct pcie_port_service_driver *pci_find_dpc_service(struct pci_dev *dev);
#endif /* _PORTDRV_H_ */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 083408e..123ee15 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2005,6 +2005,7 @@ static inline resource_size_t pci_iov_resource_size(struct pci_dev *dev, int res
#define PCI_ERR_AER_NONFATAL 0
#define PCI_ERR_AER_FATAL 1
#define PCI_ERR_AER_CORRECTABLE 2
+#define PCI_ERR_DPC_FATAL 4
pci_ers_result_t pci_broadcast_error_message(struct pci_dev *dev,
enum pci_channel_state state,
@@ -2014,7 +2015,7 @@ pci_ers_result_t pci_broadcast_error_message(struct pci_dev *dev,
int pci_report_slot_reset(struct pci_dev *dev, void *data);
int pci_report_resume(struct pci_dev *dev, void *data);
int pci_report_error_detected(struct pci_dev *dev, void *data);
-pci_ers_result_t pci_reset_link(struct pci_dev *dev);
+pci_ers_result_t pci_reset_link(struct pci_dev *dev, int severity);
pci_ers_result_t pci_merge_result(enum pci_ers_result orig,
enum pci_ers_result new);
void pci_do_recovery(struct pci_dev *dev, int severity);
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.,
a Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Implement error_resume callback in DPC, which, after DPC trigger event
enumerates the devices beneath.
Signed-off-by: Oza Pawandeep <[email protected]>
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 68296ec..4c6bef3 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -161,6 +161,43 @@ static void dpc_wait_link_inactive(struct dpc_dev *dpc)
dev_warn(dev, "Link state not disabled for DPC event\n");
}
+static bool dpc_wait_link_active(struct pci_dev *pdev)
+{
+ unsigned long timeout = jiffies + HZ;
+ u16 lnk_status;
+ bool ret = true;
+
+ pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
+
+ while (!(lnk_status & PCI_EXP_LNKSTA_DLLLA) &&
+ !time_after(jiffies, timeout)) {
+ msleep(10);
+ pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
+ }
+
+ if (!(lnk_status & PCI_EXP_LNKSTA_DLLLA)) {
+ dev_warn(&pdev->dev, "Link state not enabled after DPC event\n");
+ ret = false;
+ }
+
+ return ret;
+}
+
+/**
+ * dpc_error_resume - enumerate the devices beneath
+ * @dev: pointer to Root Port's pci_dev data structure
+ *
+ * Invoked by Port Bus driver during nonfatal recovery.
+ */
+static void dpc_error_resume(struct pci_dev *pdev)
+{
+ if (dpc_wait_link_active(pdev)) {
+ pci_lock_rescan_remove();
+ pci_rescan_bus(pdev->bus);
+ pci_unlock_rescan_remove();
+ }
+}
+
/**
* dpc_reset_link - reset link DPC routine
* @dev: pointer to Root Port's pci_dev data structure
@@ -419,6 +456,7 @@ static void dpc_remove(struct pcie_device *dev)
.service = PCIE_PORT_SERVICE_DPC,
.probe = dpc_probe,
.remove = dpc_remove,
+ .error_resume = dpc_error_resume,
.reset_link = dpc_reset_link,
};
diff --git a/drivers/pci/pcie/pcie-err.c b/drivers/pci/pcie/pcie-err.c
index 1991cc8..aadf0eb 100644
--- a/drivers/pci/pcie/pcie-err.c
+++ b/drivers/pci/pcie/pcie-err.c
@@ -238,7 +238,8 @@ pci_ers_result_t pci_reset_link(struct pci_dev *dev, int severity)
pci_ers_result_t pci_broadcast_error_message(struct pci_dev *dev,
enum pci_channel_state state,
char *error_mesg,
- int (*cb)(struct pci_dev *, void *))
+ int (*cb)(struct pci_dev *, void *),
+ int severity)
{
struct pci_err_broadcast_data result_data;
@@ -250,6 +251,15 @@ pci_ers_result_t pci_broadcast_error_message(struct pci_dev *dev,
result_data.result = PCI_ERS_RESULT_RECOVERED;
if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
+ /* If DPC is triggered, call resume error hanlder
+ * because, at this point we can safely assume that
+ * link recovery has happened.
+ */
+ if ((severity == PCI_ERR_DPC_FATAL) &&
+ (cb == pci_report_resume)) {
+ cb(dev, NULL);
+ return PCI_ERS_RESULT_RECOVERED;
+ }
/*
* If the error is reported by a bridge, we think this error
* is related to the downstream link of the bridge, so we
@@ -335,7 +345,8 @@ void pci_do_recovery(struct pci_dev *dev, int severity)
status = pci_broadcast_error_message(dev,
state,
"error_detected",
- pci_report_error_detected);
+ pci_report_error_detected,
+ severity);
if ((severity == PCI_ERR_AER_FATAL) ||
(severity == PCI_ERR_DPC_FATAL)) {
@@ -344,11 +355,15 @@ void pci_do_recovery(struct pci_dev *dev, int severity)
goto failed;
}
+ if (severity == PCI_ERR_DPC_FATAL)
+ goto resume;
+
if (status == PCI_ERS_RESULT_CAN_RECOVER)
status = pci_broadcast_error_message(dev,
state,
"mmio_enabled",
- pci_report_mmio_enabled);
+ pci_report_mmio_enabled,
+ severity);
if (status == PCI_ERS_RESULT_NEED_RESET) {
/*
@@ -359,16 +374,19 @@ void pci_do_recovery(struct pci_dev *dev, int severity)
status = pci_broadcast_error_message(dev,
state,
"slot_reset",
- pci_report_slot_reset);
+ pci_report_slot_reset,
+ severity);
}
if (status != PCI_ERS_RESULT_RECOVERED)
goto failed;
+resume:
pci_broadcast_error_message(dev,
state,
"resume",
- pci_report_resume);
+ pci_report_resume,
+ severity);
dev_info(&dev->dev, "Device recovery successful\n");
mutex_unlock(&pci_err_recovery_lock);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 123ee15..46e2526 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2010,7 +2010,8 @@ static inline resource_size_t pci_iov_resource_size(struct pci_dev *dev, int res
pci_ers_result_t pci_broadcast_error_message(struct pci_dev *dev,
enum pci_channel_state state,
char *error_mesg,
- int (*cb)(struct pci_dev *, void *));
+ int (*cb)(struct pci_dev *, void *),
+ int severity);
int pci_report_mmio_enabled(struct pci_dev *dev, void *data);
int pci_report_slot_reset(struct pci_dev *dev, void *data);
int pci_report_resume(struct pci_dev *dev, void *data);
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.,
a Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
If AER attempts to do recovery for any device, and DPC is active on
any upstream port, AER should not do recovery, since it will be handled
by DPC
Change-Id: Ida507ce9145f420e35302db34e967f1b421e15c9
Signed-off-by: Oza Pawandeep <[email protected]>
diff --git a/drivers/pci/pcie/pcie-err.c b/drivers/pci/pcie/pcie-err.c
index 858c94c..1991cc8 100644
--- a/drivers/pci/pcie/pcie-err.c
+++ b/drivers/pci/pcie/pcie-err.c
@@ -274,6 +274,22 @@ pci_ers_result_t pci_broadcast_error_message(struct pci_dev *dev,
return result_data.result;
}
+/*
+ * pcie_port_upstream_bridge - returns immediate upstream bridge.
+ * dev: pcie device
+ */
+static struct pci_dev *pcie_port_upstream_bridge(struct pci_dev *dev)
+{
+ struct pci_dev *parent;
+
+ parent = pci_upstream_bridge(dev);
+
+ if (parent && pci_is_pcie(parent))
+ return parent;
+
+ return NULL;
+}
+
/**
* pci_do_recovery - handle nonfatal/fatal error recovery process
* @dev: pointer to a pci_dev data structure of agent detecting an error
@@ -287,9 +303,29 @@ void pci_do_recovery(struct pci_dev *dev, int severity)
{
pci_ers_result_t status, result = PCI_ERS_RESULT_RECOVERED;
enum pci_channel_state state;
+ struct pcie_port_service_driver *driver;
+ struct pci_dev *pdev = dev;
mutex_lock(&pci_err_recovery_lock);
+ if (severity != PCI_ERR_DPC_FATAL) {
+ /*
+ * DPC service could be running in RP
+ * or any upstream switch.
+ */
+ do {
+ driver = pci_find_dpc_service(pdev);
+ if (driver) {
+ dev_printk(KERN_NOTICE, &dev->dev,
+ "AER: Recovery to be done by DPC %s\n",
+ pci_name(dev));
+ mutex_unlock(&pci_err_recovery_lock);
+ return;
+ }
+ pdev = pcie_port_upstream_bridge(dev);
+ } while (pdev);
+ }
+
if ((severity == PCI_ERR_AER_FATAL) ||
(severity == PCI_ERR_DPC_FATAL))
state = pci_channel_io_frozen;
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.,
a Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
This patch factors out error reporting callbacks, which are currently
tightly coupled with AER.
DPC should be able to call these callbacks when DPC trigger event occurs.
Signed-off-by: Oza Pawandeep <[email protected]>
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 6402f7f..fd053e5 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -462,7 +462,7 @@ static void ghes_do_proc(struct ghes *ghes,
* use, so treat it as a fatal AER error.
*/
if (gdata->flags & CPER_SEC_RESET)
- aer_severity = AER_FATAL;
+ aer_severity = PCI_ERR_AER_FATAL;
aer_recover_queue(pcie_err->device_id.segment,
pcie_err->device_id.bus,
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index 223e4c3..d669497 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -6,7 +6,7 @@
# Build PCI Express ASPM if needed
obj-$(CONFIG_PCIEASPM) += aspm.o
-pcieportdrv-y := portdrv_core.o portdrv_pci.o portdrv_bus.o
+pcieportdrv-y := portdrv_core.o portdrv_pci.o portdrv_bus.o pcie-err.o
pcieportdrv-$(CONFIG_ACPI) += portdrv_acpi.o
obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o
diff --git a/drivers/pci/pcie/aer/aerdrv.h b/drivers/pci/pcie/aer/aerdrv.h
index 5449e5c..bc9db53 100644
--- a/drivers/pci/pcie/aer/aerdrv.h
+++ b/drivers/pci/pcie/aer/aerdrv.h
@@ -76,36 +76,6 @@ struct aer_rpc {
*/
};
-struct aer_broadcast_data {
- enum pci_channel_state state;
- enum pci_ers_result result;
-};
-
-static inline pci_ers_result_t merge_result(enum pci_ers_result orig,
- enum pci_ers_result new)
-{
- if (new == PCI_ERS_RESULT_NO_AER_DRIVER)
- return PCI_ERS_RESULT_NO_AER_DRIVER;
-
- if (new == PCI_ERS_RESULT_NONE)
- return orig;
-
- switch (orig) {
- case PCI_ERS_RESULT_CAN_RECOVER:
- case PCI_ERS_RESULT_RECOVERED:
- orig = new;
- break;
- case PCI_ERS_RESULT_DISCONNECT:
- if (new == PCI_ERS_RESULT_NEED_RESET)
- orig = PCI_ERS_RESULT_NEED_RESET;
- break;
- default:
- break;
- }
-
- return orig;
-}
-
extern struct bus_type pcie_port_bus_type;
void aer_isr(struct work_struct *work);
void aer_print_error(struct pci_dev *dev, struct aer_err_info *info);
diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c
index 7448052..758e744 100644
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -165,7 +165,7 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
return false;
/* Check if error is recorded */
- if (e_info->severity == AER_CORRECTABLE) {
+ if (e_info->severity == PCI_ERR_AER_CORRECTABLE) {
pci_read_config_dword(dev, pos + PCI_ERR_COR_STATUS, &status);
pci_read_config_dword(dev, pos + PCI_ERR_COR_MASK, &mask);
} else {
@@ -234,189 +234,6 @@ static bool find_source_device(struct pci_dev *parent,
return true;
}
-static int report_error_detected(struct pci_dev *dev, void *data)
-{
- pci_ers_result_t vote;
- const struct pci_error_handlers *err_handler;
- struct aer_broadcast_data *result_data;
- result_data = (struct aer_broadcast_data *) data;
-
- device_lock(&dev->dev);
- dev->error_state = result_data->state;
-
- if (!dev->driver ||
- !dev->driver->err_handler ||
- !dev->driver->err_handler->error_detected) {
- if (result_data->state == pci_channel_io_frozen &&
- dev->hdr_type != PCI_HEADER_TYPE_BRIDGE) {
- /*
- * In case of fatal recovery, if one of down-
- * stream device has no driver. We might be
- * unable to recover because a later insmod
- * of a driver for this device is unaware of
- * its hw state.
- */
- dev_printk(KERN_DEBUG, &dev->dev, "device has %s\n",
- dev->driver ?
- "no AER-aware driver" : "no driver");
- }
-
- /*
- * If there's any device in the subtree that does not
- * have an error_detected callback, returning
- * PCI_ERS_RESULT_NO_AER_DRIVER prevents calling of
- * the subsequent mmio_enabled/slot_reset/resume
- * callbacks of "any" device in the subtree. All the
- * devices in the subtree are left in the error state
- * without recovery.
- */
-
- if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
- vote = PCI_ERS_RESULT_NO_AER_DRIVER;
- else
- vote = PCI_ERS_RESULT_NONE;
- } else {
- err_handler = dev->driver->err_handler;
- vote = err_handler->error_detected(dev, result_data->state);
- }
-
- result_data->result = merge_result(result_data->result, vote);
- device_unlock(&dev->dev);
- return 0;
-}
-
-static int report_mmio_enabled(struct pci_dev *dev, void *data)
-{
- pci_ers_result_t vote;
- const struct pci_error_handlers *err_handler;
- struct aer_broadcast_data *result_data;
- result_data = (struct aer_broadcast_data *) data;
-
- device_lock(&dev->dev);
- if (!dev->driver ||
- !dev->driver->err_handler ||
- !dev->driver->err_handler->mmio_enabled)
- goto out;
-
- err_handler = dev->driver->err_handler;
- vote = err_handler->mmio_enabled(dev);
- result_data->result = merge_result(result_data->result, vote);
-out:
- device_unlock(&dev->dev);
- return 0;
-}
-
-static int report_slot_reset(struct pci_dev *dev, void *data)
-{
- pci_ers_result_t vote;
- const struct pci_error_handlers *err_handler;
- struct aer_broadcast_data *result_data;
- result_data = (struct aer_broadcast_data *) data;
-
- device_lock(&dev->dev);
- if (!dev->driver ||
- !dev->driver->err_handler ||
- !dev->driver->err_handler->slot_reset)
- goto out;
-
- err_handler = dev->driver->err_handler;
- vote = err_handler->slot_reset(dev);
- result_data->result = merge_result(result_data->result, vote);
-out:
- device_unlock(&dev->dev);
- return 0;
-}
-
-static int report_resume(struct pci_dev *dev, void *data)
-{
- const struct pci_error_handlers *err_handler;
-
- device_lock(&dev->dev);
- dev->error_state = pci_channel_io_normal;
-
- if (!dev->driver ||
- !dev->driver->err_handler ||
- !dev->driver->err_handler->resume)
- goto out;
-
- err_handler = dev->driver->err_handler;
- err_handler->resume(dev);
-out:
- device_unlock(&dev->dev);
- return 0;
-}
-
-/**
- * broadcast_error_message - handle message broadcast to downstream drivers
- * @dev: pointer to from where in a hierarchy message is broadcasted down
- * @state: error state
- * @error_mesg: message to print
- * @cb: callback to be broadcasted
- *
- * Invoked during error recovery process. Once being invoked, the content
- * of error severity will be broadcasted to all downstream drivers in a
- * hierarchy in question.
- */
-static pci_ers_result_t broadcast_error_message(struct pci_dev *dev,
- enum pci_channel_state state,
- char *error_mesg,
- int (*cb)(struct pci_dev *, void *))
-{
- struct aer_broadcast_data result_data;
-
- dev_printk(KERN_DEBUG, &dev->dev, "broadcast %s message\n", error_mesg);
- result_data.state = state;
- if (cb == report_error_detected)
- result_data.result = PCI_ERS_RESULT_CAN_RECOVER;
- else
- result_data.result = PCI_ERS_RESULT_RECOVERED;
-
- if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
- /*
- * If the error is reported by a bridge, we think this error
- * is related to the downstream link of the bridge, so we
- * do error recovery on all subordinates of the bridge instead
- * of the bridge and clear the error status of the bridge.
- */
- if (cb == report_error_detected)
- dev->error_state = state;
- pci_walk_bus(dev->subordinate, cb, &result_data);
- if (cb == report_resume) {
- pci_cleanup_aer_uncorrect_error_status(dev);
- dev->error_state = pci_channel_io_normal;
- }
- } else {
- /*
- * If the error is reported by an end point, we think this
- * error is related to the upstream link of the end point.
- */
- if (state == pci_channel_io_normal)
- /*
- * the error is non fatal so the bus is ok, just invoke
- * the callback for the function that logged the error.
- */
- cb(dev, &result_data);
- else
- pci_walk_bus(dev->bus, cb, &result_data);
- }
-
- return result_data.result;
-}
-
-/**
- * default_reset_link - default reset function
- * @dev: pointer to pci_dev data structure
- *
- * Invoked when performing link reset on a Downstream Port or a
- * Root Port with no aer driver.
- */
-static pci_ers_result_t default_reset_link(struct pci_dev *dev)
-{
- pci_reset_bridge_secondary_bus(dev);
- dev_printk(KERN_DEBUG, &dev->dev, "downstream link has been reset\n");
- return PCI_ERS_RESULT_RECOVERED;
-}
-
static int find_aer_service_iter(struct device *device, void *data)
{
struct pcie_port_service_driver *service_driver, **drv;
@@ -434,7 +251,7 @@ static int find_aer_service_iter(struct device *device, void *data)
return 0;
}
-static struct pcie_port_service_driver *find_aer_service(struct pci_dev *dev)
+struct pcie_port_service_driver *pci_find_aer_service(struct pci_dev *dev)
{
struct pcie_port_service_driver *drv = NULL;
@@ -442,108 +259,7 @@ static struct pcie_port_service_driver *find_aer_service(struct pci_dev *dev)
return drv;
}
-
-static pci_ers_result_t reset_link(struct pci_dev *dev)
-{
- struct pci_dev *udev;
- pci_ers_result_t status;
- struct pcie_port_service_driver *driver;
-
- if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
- /* Reset this port for all subordinates */
- udev = dev;
- } else {
- /* Reset the upstream component (likely downstream port) */
- udev = dev->bus->self;
- }
-
- /* Use the aer driver of the component firstly */
- driver = find_aer_service(udev);
-
- if (driver && driver->reset_link) {
- status = driver->reset_link(udev);
- } else if (udev->has_secondary_link) {
- status = default_reset_link(udev);
- } else {
- dev_printk(KERN_DEBUG, &dev->dev,
- "no link-reset support at upstream device %s\n",
- pci_name(udev));
- return PCI_ERS_RESULT_DISCONNECT;
- }
-
- if (status != PCI_ERS_RESULT_RECOVERED) {
- dev_printk(KERN_DEBUG, &dev->dev,
- "link reset at upstream device %s failed\n",
- pci_name(udev));
- return PCI_ERS_RESULT_DISCONNECT;
- }
-
- return status;
-}
-
-/**
- * do_recovery - handle nonfatal/fatal error recovery process
- * @dev: pointer to a pci_dev data structure of agent detecting an error
- * @severity: error severity type
- *
- * Invoked when an error is nonfatal/fatal. Once being invoked, broadcast
- * error detected message to all downstream drivers within a hierarchy in
- * question and return the returned code.
- */
-static void do_recovery(struct pci_dev *dev, int severity)
-{
- pci_ers_result_t status, result = PCI_ERS_RESULT_RECOVERED;
- enum pci_channel_state state;
-
- if (severity == AER_FATAL)
- state = pci_channel_io_frozen;
- else
- state = pci_channel_io_normal;
-
- status = broadcast_error_message(dev,
- state,
- "error_detected",
- report_error_detected);
-
- if (severity == AER_FATAL) {
- result = reset_link(dev);
- if (result != PCI_ERS_RESULT_RECOVERED)
- goto failed;
- }
-
- if (status == PCI_ERS_RESULT_CAN_RECOVER)
- status = broadcast_error_message(dev,
- state,
- "mmio_enabled",
- report_mmio_enabled);
-
- if (status == PCI_ERS_RESULT_NEED_RESET) {
- /*
- * TODO: Should call platform-specific
- * functions to reset slot before calling
- * drivers' slot_reset callbacks?
- */
- status = broadcast_error_message(dev,
- state,
- "slot_reset",
- report_slot_reset);
- }
-
- if (status != PCI_ERS_RESULT_RECOVERED)
- goto failed;
-
- broadcast_error_message(dev,
- state,
- "resume",
- report_resume);
-
- dev_info(&dev->dev, "AER: Device recovery successful\n");
- return;
-
-failed:
- /* TODO: Should kernel panic here? */
- dev_info(&dev->dev, "AER: Device recovery failed\n");
-}
+EXPORT_SYMBOL(pci_find_aer_service);
/**
* handle_error_source - handle logging error into an event log
@@ -559,7 +275,7 @@ static void handle_error_source(struct pcie_device *aerdev,
{
int pos;
- if (info->severity == AER_CORRECTABLE) {
+ if (info->severity == PCI_ERR_AER_CORRECTABLE) {
/*
* Correctable error does not need software intervention.
* No need to go through error recovery process.
@@ -569,7 +285,7 @@ static void handle_error_source(struct pcie_device *aerdev,
pci_write_config_dword(dev, pos + PCI_ERR_COR_STATUS,
info->status);
} else
- do_recovery(dev, info->severity);
+ pci_do_recovery(dev, info->severity);
}
#ifdef CONFIG_ACPI_APEI_PCIEAER
@@ -633,7 +349,7 @@ static void aer_recover_work_func(struct work_struct *work)
continue;
}
cper_print_aer(pdev, entry.severity, entry.regs);
- do_recovery(pdev, entry.severity);
+ pci_do_recovery(pdev, entry.severity);
pci_dev_put(pdev);
}
}
@@ -662,7 +378,7 @@ static int get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
if (!pos)
return 1;
- if (info->severity == AER_CORRECTABLE) {
+ if (info->severity == PCI_ERR_AER_CORRECTABLE) {
pci_read_config_dword(dev, pos + PCI_ERR_COR_STATUS,
&info->status);
pci_read_config_dword(dev, pos + PCI_ERR_COR_MASK,
@@ -670,7 +386,7 @@ static int get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
if (!(info->status & ~info->mask))
return 0;
} else if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
- info->severity == AER_NONFATAL) {
+ info->severity == PCI_ERR_AER_NONFATAL) {
/* Link is still healthy for IO reads */
pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS,
@@ -733,7 +449,7 @@ static void aer_isr_one_error(struct pcie_device *p_device,
*/
if (e_src->status & PCI_ERR_ROOT_COR_RCV) {
e_info->id = ERR_COR_ID(e_src->id);
- e_info->severity = AER_CORRECTABLE;
+ e_info->severity = PCI_ERR_AER_CORRECTABLE;
if (e_src->status & PCI_ERR_ROOT_MULTI_COR_RCV)
e_info->multi_error_valid = 1;
@@ -750,9 +466,9 @@ static void aer_isr_one_error(struct pcie_device *p_device,
e_info->id = ERR_UNCOR_ID(e_src->id);
if (e_src->status & PCI_ERR_ROOT_FATAL_RCV)
- e_info->severity = AER_FATAL;
+ e_info->severity = PCI_ERR_AER_FATAL;
else
- e_info->severity = AER_NONFATAL;
+ e_info->severity = PCI_ERR_AER_NONFATAL;
if (e_src->status & PCI_ERR_ROOT_MULTI_UNCOR_RCV)
e_info->multi_error_valid = 1;
diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
index 54c4b69..222c56c 100644
--- a/drivers/pci/pcie/aer/aerdrv_errprint.c
+++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
@@ -29,11 +29,14 @@
#define AER_AGENT_COMPLETER 2
#define AER_AGENT_TRANSMITTER 3
-#define AER_AGENT_REQUESTER_MASK(t) ((t == AER_CORRECTABLE) ? \
+#define AER_AGENT_REQUESTER_MASK(t) \
+ ((t == PCI_ERR_AER_CORRECTABLE) ? \
0 : (PCI_ERR_UNC_COMP_TIME|PCI_ERR_UNC_UNSUP))
-#define AER_AGENT_COMPLETER_MASK(t) ((t == AER_CORRECTABLE) ? \
+#define AER_AGENT_COMPLETER_MASK(t) \
+ ((t == PCI_ERR_AER_CORRECTABLE) ? \
0 : PCI_ERR_UNC_COMP_ABORT)
-#define AER_AGENT_TRANSMITTER_MASK(t) ((t == AER_CORRECTABLE) ? \
+#define AER_AGENT_TRANSMITTER_MASK(t) \
+ ((t == PCI_ERR_AER_CORRECTABLE) ? \
(PCI_ERR_COR_REP_ROLL|PCI_ERR_COR_REP_TIMER) : 0)
#define AER_GET_AGENT(t, e) \
@@ -46,9 +49,11 @@
#define AER_DATA_LINK_LAYER_ERROR 1
#define AER_TRANSACTION_LAYER_ERROR 2
-#define AER_PHYSICAL_LAYER_ERROR_MASK(t) ((t == AER_CORRECTABLE) ? \
+#define AER_PHYSICAL_LAYER_ERROR_MASK(t) \
+ ((t == PCI_ERR_AER_CORRECTABLE) ? \
PCI_ERR_COR_RCVR : 0)
-#define AER_DATA_LINK_LAYER_ERROR_MASK(t) ((t == AER_CORRECTABLE) ? \
+#define AER_DATA_LINK_LAYER_ERROR_MASK(t) \
+ ((t == PCI_ERR_AER_CORRECTABLE) ? \
(PCI_ERR_COR_BAD_TLP| \
PCI_ERR_COR_BAD_DLLP| \
PCI_ERR_COR_REP_ROLL| \
@@ -147,7 +152,7 @@ static void __aer_print_error(struct pci_dev *dev,
if (!(status & (1 << i)))
continue;
- if (info->severity == AER_CORRECTABLE)
+ if (info->severity == PCI_ERR_AER_CORRECTABLE)
errmsg = i < ARRAY_SIZE(aer_correctable_error_string) ?
aer_correctable_error_string[i] : NULL;
else
@@ -210,11 +215,11 @@ int cper_severity_to_aer(int cper_severity)
{
switch (cper_severity) {
case CPER_SEV_RECOVERABLE:
- return AER_NONFATAL;
+ return PCI_ERR_AER_NONFATAL;
case CPER_SEV_FATAL:
- return AER_FATAL;
+ return PCI_ERR_AER_FATAL;
default:
- return AER_CORRECTABLE;
+ return PCI_ERR_AER_CORRECTABLE;
}
}
EXPORT_SYMBOL_GPL(cper_severity_to_aer);
@@ -226,7 +231,7 @@ void cper_print_aer(struct pci_dev *dev, int aer_severity,
u32 status, mask;
const char **status_strs;
- if (aer_severity == AER_CORRECTABLE) {
+ if (aer_severity == PCI_ERR_AER_CORRECTABLE) {
status = aer->cor_status;
mask = aer->cor_mask;
status_strs = aer_correctable_error_string;
@@ -247,7 +252,7 @@ void cper_print_aer(struct pci_dev *dev, int aer_severity,
dev_err(&dev->dev, "aer_layer=%s, aer_agent=%s\n",
aer_error_layer[layer], aer_agent_string[agent]);
- if (aer_severity != AER_CORRECTABLE)
+ if (aer_severity != PCI_ERR_AER_CORRECTABLE)
dev_err(&dev->dev, "aer_uncor_severity: 0x%08x\n",
aer->uncor_severity);
diff --git a/drivers/pci/pcie/pcie-err.c b/drivers/pci/pcie/pcie-err.c
new file mode 100644
index 0000000..a76a8bf
--- /dev/null
+++ b/drivers/pci/pcie/pcie-err.c
@@ -0,0 +1,335 @@
+/*
+ * Copyright (c) 2017, The Linux Foundation. All rights reserved.
+
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/pci.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/aer.h>
+#include <linux/pcieport_if.h>
+#include "portdrv.h"
+
+static DEFINE_MUTEX(pci_err_recovery_lock);
+
+pci_ers_result_t pci_merge_result(enum pci_ers_result orig,
+ enum pci_ers_result new)
+{
+ if (new == PCI_ERS_RESULT_NO_AER_DRIVER)
+ return PCI_ERS_RESULT_NO_AER_DRIVER;
+
+ if (new == PCI_ERS_RESULT_NONE)
+ return orig;
+
+ switch (orig) {
+ case PCI_ERS_RESULT_CAN_RECOVER:
+ case PCI_ERS_RESULT_RECOVERED:
+ orig = new;
+ break;
+ case PCI_ERS_RESULT_DISCONNECT:
+ if (new == PCI_ERS_RESULT_NEED_RESET)
+ orig = PCI_ERS_RESULT_NEED_RESET;
+ break;
+ default:
+ break;
+ }
+
+ return orig;
+}
+
+int pci_report_mmio_enabled(struct pci_dev *dev, void *data)
+{
+ pci_ers_result_t vote;
+ const struct pci_error_handlers *err_handler;
+ struct pci_err_broadcast_data *result_data;
+
+ result_data = (struct pci_err_broadcast_data *) data;
+
+ device_lock(&dev->dev);
+ if (!dev->driver ||
+ !dev->driver->err_handler ||
+ !dev->driver->err_handler->mmio_enabled)
+ goto out;
+
+ err_handler = dev->driver->err_handler;
+ vote = err_handler->mmio_enabled(dev);
+ result_data->result = pci_merge_result(result_data->result, vote);
+out:
+ device_unlock(&dev->dev);
+ return 0;
+}
+
+int pci_report_slot_reset(struct pci_dev *dev, void *data)
+{
+ pci_ers_result_t vote;
+ const struct pci_error_handlers *err_handler;
+ struct pci_err_broadcast_data *result_data;
+
+ result_data = (struct pci_err_broadcast_data *) data;
+
+ device_lock(&dev->dev);
+ if (!dev->driver ||
+ !dev->driver->err_handler ||
+ !dev->driver->err_handler->slot_reset)
+ goto out;
+
+ err_handler = dev->driver->err_handler;
+ vote = err_handler->slot_reset(dev);
+ result_data->result = pci_merge_result(result_data->result, vote);
+out:
+ device_unlock(&dev->dev);
+ return 0;
+}
+
+int pci_report_resume(struct pci_dev *dev, void *data)
+{
+ const struct pci_error_handlers *err_handler;
+
+ device_lock(&dev->dev);
+ dev->error_state = pci_channel_io_normal;
+
+ if (!dev->driver ||
+ !dev->driver->err_handler ||
+ !dev->driver->err_handler->resume)
+ goto out;
+
+ err_handler = dev->driver->err_handler;
+ err_handler->resume(dev);
+out:
+ device_unlock(&dev->dev);
+ return 0;
+}
+
+int pci_report_error_detected(struct pci_dev *dev, void *data)
+{
+ pci_ers_result_t vote;
+ const struct pci_error_handlers *err_handler;
+ struct pci_err_broadcast_data *result_data;
+
+ result_data = (struct pci_err_broadcast_data *) data;
+
+ device_lock(&dev->dev);
+ dev->error_state = result_data->state;
+
+ if (!dev->driver ||
+ !dev->driver->err_handler ||
+ !dev->driver->err_handler->error_detected) {
+ if (result_data->state == pci_channel_io_frozen &&
+ dev->hdr_type != PCI_HEADER_TYPE_BRIDGE) {
+ /*
+ * In case of fatal recovery, if one of down-
+ * stream device has no driver. We might be
+ * unable to recover because a later insmod
+ * of a driver for this device is unaware of
+ * its hw state.
+ */
+ dev_printk(KERN_DEBUG, &dev->dev, "device has %s\n",
+ dev->driver ?
+ "no error-aware driver" : "no driver");
+ }
+
+ /*
+ * If there's any device in the subtree that does not
+ * have an error_detected callback, returning
+ * PCI_ERS_RESULT_NO_AER_DRIVER prevents calling of
+ * the subsequent mmio_enabled/slot_reset/resume
+ * callbacks of "any" device in the subtree. All the
+ * devices in the subtree are left in the error state
+ * without recovery.
+ */
+
+ if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
+ vote = PCI_ERS_RESULT_NO_AER_DRIVER;
+ else
+ vote = PCI_ERS_RESULT_NONE;
+ } else {
+ err_handler = dev->driver->err_handler;
+ vote = err_handler->error_detected(dev, result_data->state);
+ }
+
+ result_data->result = pci_merge_result(result_data->result, vote);
+ device_unlock(&dev->dev);
+ return 0;
+}
+
+/**
+ * pci_default_reset_link - default reset function
+ * @dev: pointer to pci_dev data structure
+ *
+ * Invoked when performing link reset on a Downstream Port or a
+ * Root Port with no aer driver.
+ */
+static pci_ers_result_t pci_default_reset_link(struct pci_dev *dev)
+{
+ pci_reset_bridge_secondary_bus(dev);
+ dev_printk(KERN_DEBUG, &dev->dev, "downstream link has been reset\n");
+ return PCI_ERS_RESULT_RECOVERED;
+}
+
+pci_ers_result_t pci_reset_link(struct pci_dev *dev)
+{
+ struct pci_dev *udev;
+ pci_ers_result_t status;
+ struct pcie_port_service_driver *driver = NULL;
+
+ if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
+ /* Reset this port for all subordinates */
+ udev = dev;
+ } else {
+ /* Reset the upstream component (likely downstream port) */
+ udev = dev->bus->self;
+ }
+
+#if IS_ENABLED(CONFIG_PCIEAER)
+ /* Use the aer driver of the component firstly */
+ driver = pci_find_aer_service(udev);
+#endif
+
+ if (driver && driver->reset_link) {
+ status = driver->reset_link(udev);
+ } else if (udev->has_secondary_link) {
+ status = pci_default_reset_link(udev);
+ } else {
+ dev_printk(KERN_DEBUG, &dev->dev,
+ "no link-reset support at upstream device %s\n",
+ pci_name(udev));
+ return PCI_ERS_RESULT_DISCONNECT;
+ }
+
+ if (status != PCI_ERS_RESULT_RECOVERED) {
+ dev_printk(KERN_DEBUG, &dev->dev,
+ "link reset at upstream device %s failed\n",
+ pci_name(udev));
+ return PCI_ERS_RESULT_DISCONNECT;
+ }
+
+ return status;
+}
+
+/**
+ * pci_broadcast_error_message - handle message broadcast to downstream drivers
+ * @dev: pointer to from where in a hierarchy message is broadcasted down
+ * @state: error state
+ * @error_mesg: message to print
+ * @cb: callback to be broadcasted
+ *
+ * Invoked during error recovery process. Once being invoked, the content
+ * of error severity will be broadcasted to all downstream drivers in a
+ * hierarchy in question.
+ */
+pci_ers_result_t pci_broadcast_error_message(struct pci_dev *dev,
+ enum pci_channel_state state,
+ char *error_mesg,
+ int (*cb)(struct pci_dev *, void *))
+{
+ struct pci_err_broadcast_data result_data;
+
+ dev_printk(KERN_DEBUG, &dev->dev, "broadcast %s message\n", error_mesg);
+ result_data.state = state;
+ if (cb == pci_report_error_detected)
+ result_data.result = PCI_ERS_RESULT_CAN_RECOVER;
+ else
+ result_data.result = PCI_ERS_RESULT_RECOVERED;
+
+ if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
+ /*
+ * If the error is reported by a bridge, we think this error
+ * is related to the downstream link of the bridge, so we
+ * do error recovery on all subordinates of the bridge instead
+ * of the bridge and clear the error status of the bridge.
+ */
+ if (cb == pci_report_error_detected)
+ dev->error_state = state;
+ pci_walk_bus(dev->subordinate, cb, &result_data);
+ if (cb == pci_report_resume) {
+ pci_cleanup_aer_uncorrect_error_status(dev);
+ dev->error_state = pci_channel_io_normal;
+ }
+ } else {
+ /*
+ * If the error is reported by an end point, we think this
+ * error is related to the upstream link of the end point.
+ */
+ pci_walk_bus(dev->bus, cb, &result_data);
+ }
+
+ return result_data.result;
+}
+
+/**
+ * pci_do_recovery - handle nonfatal/fatal error recovery process
+ * @dev: pointer to a pci_dev data structure of agent detecting an error
+ * @severity: error severity type
+ *
+ * Invoked when an error is nonfatal/fatal. Once being invoked, broadcast
+ * error detected message to all downstream drivers within a hierarchy in
+ * question and return the returned code.
+ */
+void pci_do_recovery(struct pci_dev *dev, int severity)
+{
+ pci_ers_result_t status, result = PCI_ERS_RESULT_RECOVERED;
+ enum pci_channel_state state;
+
+ mutex_lock(&pci_err_recovery_lock);
+
+ if (severity == PCI_ERR_AER_FATAL)
+ state = pci_channel_io_frozen;
+ else
+ state = pci_channel_io_normal;
+
+ status = pci_broadcast_error_message(dev,
+ state,
+ "error_detected",
+ pci_report_error_detected);
+
+ if (severity == PCI_ERR_AER_FATAL) {
+ result = pci_reset_link(dev);
+ if (result != PCI_ERS_RESULT_RECOVERED)
+ goto failed;
+ }
+
+ if (status == PCI_ERS_RESULT_CAN_RECOVER)
+ status = pci_broadcast_error_message(dev,
+ state,
+ "mmio_enabled",
+ pci_report_mmio_enabled);
+
+ if (status == PCI_ERS_RESULT_NEED_RESET) {
+ /*
+ * TODO: Should call platform-specific
+ * functions to reset slot before calling
+ * drivers' slot_reset callbacks?
+ */
+ status = pci_broadcast_error_message(dev,
+ state,
+ "slot_reset",
+ pci_report_slot_reset);
+ }
+
+ if (status != PCI_ERS_RESULT_RECOVERED)
+ goto failed;
+
+ pci_broadcast_error_message(dev,
+ state,
+ "resume",
+ pci_report_resume);
+
+ dev_info(&dev->dev, "Device recovery successful\n");
+ mutex_unlock(&pci_err_recovery_lock);
+ return;
+
+failed:
+ /* TODO: Should kernel panic here? */
+ mutex_unlock(&pci_err_recovery_lock);
+ dev_info(&dev->dev, "Device recovery failed\n");
+}
diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index a854bc5..4f1992d 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -79,4 +79,5 @@ static inline void pcie_port_platform_notify(struct pci_dev *port, int *mask)
static inline void pcie_port_platform_notify(struct pci_dev *port, int *mask){}
#endif /* !CONFIG_ACPI */
+struct pcie_port_service_driver *pci_find_aer_service(struct pci_dev *dev);
#endif /* _PORTDRV_H_ */
diff --git a/include/linux/aer.h b/include/linux/aer.h
index 8f87bbe..3eac8ed 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -11,10 +11,6 @@
#include <linux/errno.h>
#include <linux/types.h>
-#define AER_NONFATAL 0
-#define AER_FATAL 1
-#define AER_CORRECTABLE 2
-
struct pci_dev;
struct aer_header_log_regs {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c170c92..083408e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -739,6 +739,10 @@ struct pci_error_handlers {
void (*resume)(struct pci_dev *dev);
};
+struct pci_err_broadcast_data {
+ enum pci_channel_state state;
+ enum pci_ers_result result;
+};
struct module;
struct pci_driver {
@@ -1998,6 +2002,23 @@ static inline resource_size_t pci_iov_resource_size(struct pci_dev *dev, int res
void pci_hp_remove_module_link(struct pci_slot *pci_slot);
#endif
+#define PCI_ERR_AER_NONFATAL 0
+#define PCI_ERR_AER_FATAL 1
+#define PCI_ERR_AER_CORRECTABLE 2
+
+pci_ers_result_t pci_broadcast_error_message(struct pci_dev *dev,
+ enum pci_channel_state state,
+ char *error_mesg,
+ int (*cb)(struct pci_dev *, void *));
+int pci_report_mmio_enabled(struct pci_dev *dev, void *data);
+int pci_report_slot_reset(struct pci_dev *dev, void *data);
+int pci_report_resume(struct pci_dev *dev, void *data);
+int pci_report_error_detected(struct pci_dev *dev, void *data);
+pci_ers_result_t pci_reset_link(struct pci_dev *dev);
+pci_ers_result_t pci_merge_result(enum pci_ers_result orig,
+ enum pci_ers_result new);
+void pci_do_recovery(struct pci_dev *dev, int severity);
+
/**
* pci_pcie_cap - get the saved PCIe capability offset
* @dev: PCI device
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index 9c68986..6176e90 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -316,10 +316,10 @@
TP_printk("%s PCIe Bus Error: severity=%s, %s\n",
__get_str(dev_name),
- __entry->severity == AER_CORRECTABLE ? "Corrected" :
- __entry->severity == AER_FATAL ?
+ __entry->severity == PCI_ERR_AER_CORRECTABLE ? "Corrected" :
+ __entry->severity == PCI_ERR_AER_FATAL ?
"Fatal" : "Uncorrected, non-fatal",
- __entry->severity == AER_CORRECTABLE ?
+ __entry->severity == PCI_ERR_AER_CORRECTABLE ?
__print_flags(__entry->status, "|", aer_correctable_errors) :
__print_flags(__entry->status, "|", aer_uncorrectable_errors))
);
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.,
a Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
On Fri, Dec 29, 2017 at 12:54:17PM +0530, Oza Pawandeep wrote:
> This patch addresses the race condition between AER and DPC for recovery.
>
> Current DPC driver does not do recovery, e.g. calling end-point's driver's
> callbacks, which sanitize the device.
> DPC driver implements link_reset callback, and calls pci_do_recovery.
I'm not sure I see why any of this is necessary for two reasons:
1. A downstream port containment event disables the link. How can a driver
sanitize an end device when all the end devices below the containment are
physically inaccessible? Any attempt to access such devices will just
end with either CA or UR (depending on DPC control settings). Since we
already know the failed outcome from attempting to access such devices,
why do you want the drivers to do anything?
2. A DPC event suppresses the error message required for the Linux
AER driver to run. How can AER and DPC run concurrently?
On 2017-12-29 22:53, Keith Busch wrote:
> On Fri, Dec 29, 2017 at 12:54:17PM +0530, Oza Pawandeep wrote:
>> This patch addresses the race condition between AER and DPC for
>> recovery.
>>
>> Current DPC driver does not do recovery, e.g. calling end-point's
>> driver's
>> callbacks, which sanitize the device.
>> DPC driver implements link_reset callback, and calls pci_do_recovery.
>
> I'm not sure I see why any of this is necessary for two reasons:
>
> 1. A downstream port containment event disables the link. How can a
> driver
> sanitize an end device when all the end devices below the containment
> are
> physically inaccessible? Any attempt to access such devices will just
> end with either CA or UR (depending on DPC control settings). Since we
> already know the failed outcome from attempting to access such devices,
> why do you want the drivers to do anything?
>
Ok I think my statement was misleading, not device sanitation, but the
device driver making
SW sanitize.
for e.g. have a look at e1000_io_error_detected which is called say in
case of AER ERR_FATAL msg.
which sanitizes sw stack, interrupts management (synchronize_irq),
delete timers etc..
yes, DPC would have made the link state disabled, and HW would have
reset the internal logic with
quiescence activities so yes, any transaction on will end with CA or UR.
well but device driver
has to handle rest of the possible things as I mentioned (error
callbacks)
> 2. A DPC event suppresses the error message required for the Linux
> AER driver to run. How can AER and DPC run concurrently?
I afraid I could not grasp the first line completely.
but they way it is triggering AER and DPC on our platform concurrently
is, we have same MSIx registered
for both AER and DPC, and linux calls the shared handlers to handle both
the triggers anyway.
otherwise also if ERR_FATAL msg occurs, the Root port should trigger
both AER and DPC
(assuming both are enabled, and no FW first for AER/DPC)
the problem with the current framework of AER and DPC in Linux is:
both try to act independently, while we know that (for e.g. ERR_FATAL
msg) is responsible for triggering
both AER and DPC depending on the configuration. (currently DPC is
configured for both FATAL and NONFATAL in linux anyway)
It does not make sense that AER goes ahead and attempts to sanitize with
the device driver's callbacks as I mentioned.
and DPC being unaware, asynchronously disables the link (although this
is all HW)
but DPC service driver should adapt to some kind of error handling and
error resume which AER has adapted.
Hence this whole design changes proposed with respect to error handling.
Let me give you another problem statement on the same line:
when DPC is active, AER does not need to act at all...because it doesnt
make sense for AER to act independently.,
without knowing what DPC service driver is upto!
which is handled in one of the patches.
the point I am trying to make is: DPC should not rely on AER to call
error callbacks, and AER should not be doing it without knowing that
DPC is active and it is also going to some course of action (be in HW or
SW)
Regards,
Oza.
On Fri, Dec 29, 2017 at 11:30:02PM +0530, [email protected] wrote:
> On 2017-12-29 22:53, Keith Busch wrote:
>
> > 2. A DPC event suppresses the error message required for the Linux
> > AER driver to run. How can AER and DPC run concurrently?
>
> I afraid I could not grasp the first line completely.
A DPC capable and enabled port discards error messages; the ERR_FATAL
or ERR_NONFATAL message required to trigger AER handling won't exist in
such a setup.
This behavior is defined in the specification 6.2.10 for Downstream
Port Containment:
When DPC is triggered due to receipt of an uncorrectable error Message,
the Requester ID from the Message is recorded in the DPC Error
Source ID register and that Message is discarded and not forwarded
Upstream. When DPC is triggered by an unmasked uncorrectable error,
that error will not be signaled with an uncorrectable error Message,
even if otherwise enabled.
On 2017-12-29 23:43, Keith Busch wrote:
> On Fri, Dec 29, 2017 at 11:30:02PM +0530, [email protected] wrote:
>> On 2017-12-29 22:53, Keith Busch wrote:
>>
>> > 2. A DPC event suppresses the error message required for the Linux
>> > AER driver to run. How can AER and DPC run concurrently?
>>
>> I afraid I could not grasp the first line completely.
>
> A DPC capable and enabled port discards error messages; the ERR_FATAL
> or ERR_NONFATAL message required to trigger AER handling won't exist in
> such a setup.
>
> This behavior is defined in the specification 6.2.10 for Downstream
> Port Containment:
>
> When DPC is triggered due to receipt of an uncorrectable error
> Message,
> the Requester ID from the Message is recorded in the DPC Error
> Source ID register and that Message is discarded and not forwarded
> Upstream. When DPC is triggered by an unmasked uncorrectable error,
> that error will not be signaled with an uncorrectable error Message,
> even if otherwise enabled.
In my understanding, thiis talks about DPC enabled switch. this case is
taken care as well.
if you look at patchset-3, when AER is triggered, AER's pci_dev is of
endpoint
will traverse all the way up until it finds associated DPC service
enabled.
pdev = pcie_port_upstream_bridge(dev);
if AER is not triggered, then at switch level DPC will take
care/suppress the msg
and entire SW will not come into picture then.
But specifically the patches attempts to bring in some sort of
coordination and understanding between AER and DPC.
as I mentioned in my previous mail.
Regards,
Oza.
Hi Keith,
On 12/29/2017 12:23 PM, Keith Busch wrote:
> On Fri, Dec 29, 2017 at 12:54:17PM +0530, Oza Pawandeep wrote:
>> This patch addresses the race condition between AER and DPC for recovery.
>>
>> Current DPC driver does not do recovery, e.g. calling end-point's driver's
>> callbacks, which sanitize the device.
>> DPC driver implements link_reset callback, and calls pci_do_recovery.
>
> I'm not sure I see why any of this is necessary for two reasons:
>
> 1. A downstream port containment event disables the link. How can a driver
> sanitize an end device when all the end devices below the containment are
> physically inaccessible? Any attempt to access such devices will just
> end with either CA or UR (depending on DPC control settings). Since we
> already know the failed outcome from attempting to access such devices,
> why do you want the drivers to do anything?
The reset callback to the endpoint driver has a status field indicating
whether the IO is frozen or not. If IO is not frozen, an endpoint driver
can potentially recover from the error by reissuing the failed request.
If IO is frozen, then the endpoint driver needs to clean up outstanding
resources. It is not safe to just shutdown the driver while there are
transactions in flight. This is the reason for the status field and a
chance for driver to clean up any state machines and resources.
Also note that the error callback has a result return value. An endpoint
driver indicates whether it was successful on recovering or not.
>
> 2. A DPC event suppresses the error message required for the Linux
> AER driver to run. How can AER and DPC run concurrently?
>
As we briefly discussed in previous email exchanges, I think you are
looking at a use case with a switch that supports DPC functionality.
Oza and I are looking at a root port functionality with DPC feature.
As you already know, AER errors are logged to AER capability register
independent of the DPC driver presence.
A root port is also allowed to share the MSI interrupts across DPC and
AER.
Therefore, when a DPC interrupt fires; both AER driver and DPC driver
starts recovery work. This is the issue we are trying to deal with.
In the end, the driver needs to work for both root port and switches.
I think you verified it against a switch. We are doing the same for a
root port and submitting the plumbing code.
--
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.
On Tue, Jan 02, 2018 at 08:25:08AM -0500, Sinan Kaya wrote:
> > 2. A DPC event suppresses the error message required for the Linux
> > AER driver to run. How can AER and DPC run concurrently?
> >
>
> As we briefly discussed in previous email exchanges, I think you are
> looking at a use case with a switch that supports DPC functionality.
No, I'm interested in DPC in a general.
> Oza and I are looking at a root port functionality with DPC feature.
>
> As you already know, AER errors are logged to AER capability register
> independent of the DPC driver presence.
The error is noted in the Uncorrectable Error Status Register if that's
what triggered the DPC event. This register has nothing to do with the
Root Error Status Register, which is required to have received an error
Message in order to have a status for the AER driver.
> A root port is also allowed to share the MSI interrupts across DPC and
> AER.
>
> Therefore, when a DPC interrupt fires; both AER driver and DPC driver
> starts recovery work. This is the issue we are trying to deal with.
If DPC is implemented correctly, the AER Root Status can't have an
uncorrectable status for the driver to deal with. The only thing the AER
driver could possibly see is a correctable error if DPC ERR_COR Enable
is set.
> In the end, the driver needs to work for both root port and switches.
> I think you verified it against a switch. We are doing the same for a
> root port and submitting the plumbing code.
I think we need to consider the possibility you are enabling a platform
that implemented DPC incorrectly. There's nothing in the specification
that says that DPC enabled root ports are not to discard the error message
if it came from downstream, or skip signalling the message for root port
detected errors.
On 1/2/2018 12:12 PM, Keith Busch wrote:
> On Tue, Jan 02, 2018 at 08:25:08AM -0500, Sinan Kaya wrote:
>>> 2. A DPC event suppresses the error message required for the Linux
>>> AER driver to run. How can AER and DPC run concurrently?
>>>
>>
>> As we briefly discussed in previous email exchanges, I think you are
>> looking at a use case with a switch that supports DPC functionality.
>
> No, I'm interested in DPC in a general.
>
>> Oza and I are looking at a root port functionality with DPC feature.
>>
>> As you already know, AER errors are logged to AER capability register
>> independent of the DPC driver presence.
>
> The error is noted in the Uncorrectable Error Status Register if that's
> what triggered the DPC event. This register has nothing to do with the
> Root Error Status Register, which is required to have received an error
> Message in order to have a status for the AER driver.
>
>> A root port is also allowed to share the MSI interrupts across DPC and
>> AER.
>>
>> Therefore, when a DPC interrupt fires; both AER driver and DPC driver
>> starts recovery work. This is the issue we are trying to deal with.
>
> If DPC is implemented correctly, the AER Root Status can't have an
> uncorrectable status for the driver to deal with. The only thing the AER
> driver could possibly see is a correctable error if DPC ERR_COR Enable
> is set.
>
>> In the end, the driver needs to work for both root port and switches.
>> I think you verified it against a switch. We are doing the same for a
>> root port and submitting the plumbing code.
>
> I think we need to consider the possibility you are enabling a platform
> that implemented DPC incorrectly. There's nothing in the specification
> that says that DPC enabled root ports are not to discard the error message
> if it came from downstream, or skip signalling the message for root port
> detected errors.
>
I'll circle this with the HW team.
The current code still doesn't handle outstanding transactions properly.
We can probably split the patch into two and deal with this aspect later.
--
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.
On Fri, Dec 29, 2017 at 12:54:15PM +0530, Oza Pawandeep wrote:
> This patch set brings in support for DPC and AER to co-exist and not to
> race for recovery.
>
> The current implementation of AER and error message broadcasting to the
> EP driver is tightly coupled and limited to AER service driver.
> It is important to factor out broadcasting and other link handling
> callbacks. So that not only when AER gets triggered, but also when DPC get
> triggered, or both get triggered simultaneously (for e.g. ERR_FATAL),
> callbacks are handled appropriately.
> having modularized the code, the race between AER and DPC is handled
> gracefully.
> for e.g. when DPC is active and kicked in, AER should not attempt to do
> recovery, because DPC takes care of it.
High-level question:
We have some convoluted code in negotiate_os_control() and
aer_service_init() that (I think) essentially disables AER unless the
platform firmware grants us permission to use it.
The last implementation note in PCIe r3.1, sec 6.2.10 says
DPC may be controlled in some configurations by platform firmware
and in other configurations by the operating system. DPC
functionality is strongly linked with the functionality in Advanced
Error Reporting. To avoid conflicts over whether platform firmware
or the operating system have control of DPC, it is recommended that
platform firmware and operating systems always link the control of
DPC to the control of Advanced Error Reporting.
I read that as suggesting that we should enable DPC support in Linux
if and only if we also enable AER. But I don't see anything in DPC
that looks like that. Should there be something there? Should DPC be
restructured so it's enabled and handled inside the AER driver instead
of being a separate driver?
Bjorn
On Tue, Jan 02, 2018 at 01:02:15PM -0600, Bjorn Helgaas wrote:
> On Fri, Dec 29, 2017 at 12:54:15PM +0530, Oza Pawandeep wrote:
> > This patch set brings in support for DPC and AER to co-exist and not to
> > race for recovery.
> >
> > The current implementation of AER and error message broadcasting to the
> > EP driver is tightly coupled and limited to AER service driver.
> > It is important to factor out broadcasting and other link handling
> > callbacks. So that not only when AER gets triggered, but also when DPC get
> > triggered, or both get triggered simultaneously (for e.g. ERR_FATAL),
> > callbacks are handled appropriately.
> > having modularized the code, the race between AER and DPC is handled
> > gracefully.
> > for e.g. when DPC is active and kicked in, AER should not attempt to do
> > recovery, because DPC takes care of it.
>
> High-level question:
>
> We have some convoluted code in negotiate_os_control() and
> aer_service_init() that (I think) essentially disables AER unless the
> platform firmware grants us permission to use it.
>
> The last implementation note in PCIe r3.1, sec 6.2.10 says
>
> DPC may be controlled in some configurations by platform firmware
> and in other configurations by the operating system. DPC
> functionality is strongly linked with the functionality in Advanced
> Error Reporting. To avoid conflicts over whether platform firmware
> or the operating system have control of DPC, it is recommended that
> platform firmware and operating systems always link the control of
> DPC to the control of Advanced Error Reporting.
>
> I read that as suggesting that we should enable DPC support in Linux
> if and only if we also enable AER. But I don't see anything in DPC
> that looks like that. Should there be something there? Should DPC be
> restructured so it's enabled and handled inside the AER driver instead
> of being a separate driver?
Yes, I agree the two should be linked. I submitted a patch for that here,
though driver responsibilities are still separate in this series:
https://marc.info/?l=linux-pci&m=151371742225111&w=2
On 1/2/2018 2:02 PM, Bjorn Helgaas wrote:
> I read that as suggesting that we should enable DPC support in Linux
> if and only if we also enable AER. But I don't see anything in DPC
> that looks like that. Should there be something there? Should DPC be
> restructured so it's enabled and handled inside the AER driver instead
> of being a separate driver?
I think Keith posted a patch to do this. If firmware first is enabled, DPC
init is skipped after his patch.
Oza was able to plumb the DPC handling into error recovery callbacks of
the portdrv since the portdrv layer already provides this facilities such
as reset_link and resume.
The way DPC and AER works is almost identical from AER portdrv perspective.
I really like his plumbing. Putting DPC code into AER makes it more
convoluted in my opinion.
--
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.
On 2018-01-03 00:32, Bjorn Helgaas wrote:
> On Fri, Dec 29, 2017 at 12:54:15PM +0530, Oza Pawandeep wrote:
>> This patch set brings in support for DPC and AER to co-exist and not
>> to
>> race for recovery.
>>
>> The current implementation of AER and error message broadcasting to
>> the
>> EP driver is tightly coupled and limited to AER service driver.
>> It is important to factor out broadcasting and other link handling
>> callbacks. So that not only when AER gets triggered, but also when DPC
>> get
>> triggered, or both get triggered simultaneously (for e.g. ERR_FATAL),
>> callbacks are handled appropriately.
>> having modularized the code, the race between AER and DPC is handled
>> gracefully.
>> for e.g. when DPC is active and kicked in, AER should not attempt to
>> do
>> recovery, because DPC takes care of it.
>
> High-level question:
>
> We have some convoluted code in negotiate_os_control() and
> aer_service_init() that (I think) essentially disables AER unless the
> platform firmware grants us permission to use it.
>
> The last implementation note in PCIe r3.1, sec 6.2.10 says
>
> DPC may be controlled in some configurations by platform firmware
> and in other configurations by the operating system. DPC
> functionality is strongly linked with the functionality in Advanced
> Error Reporting. To avoid conflicts over whether platform firmware
> or the operating system have control of DPC, it is recommended that
> platform firmware and operating systems always link the control of
> DPC to the control of Advanced Error Reporting.
>
> I read that as suggesting that we should enable DPC support in Linux
> if and only if we also enable AER. But I don't see anything in DPC
> that looks like that. Should there be something there? Should DPC be
> restructured so it's enabled and handled inside the AER driver instead
> of being a separate driver?
>
> Bjorn
The whole idea of factoring out error handing and plug it back to DPC is
to
enable DPC is participate synchronously in pcie_port_service_driver
hooks.
AER and DPC both being port service driver, it makes more sense, for DPC
to be able
to do with those callbacks as much as AER is able to do with those
callbacks currently.
but those callbacks are tightly coupled with AER driver.
that way DPC and AER can act independently in their own space, by
gaining more control.
and if needed, both can synchronize the callbacks.
Regards,
Oza.