linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Address error and recovery for AER and DPC
@ 2017-12-27 10:20 Oza Pawandeep
  2017-12-27 10:20 ` [PATCH 1/4] PCI/AER: factor out error reporting from AER Oza Pawandeep
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Oza Pawandeep @ 2017-12-27 10:20 UTC (permalink / raw)
  To: Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci, linux-kernel,
	Dongdong Liu, Gabriele Paoloni, Keith Busch, Wei Zhang,
	Sinan Kaya, Timur Tabi
  Cc: Oza Pawandeep

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.

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            | 392 +++++++++++++++++++++++++++++++++
 drivers/pci/pcie/portdrv.h             |   2 +
 include/linux/aer.h                    |   4 -
 include/linux/pci.h                    |  23 ++
 10 files changed, 569 insertions(+), 346 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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] PCI/AER: factor out error reporting from AER
  2017-12-27 10:20 [PATCH 0/4] Address error and recovery for AER and DPC Oza Pawandeep
@ 2017-12-27 10:20 ` Oza Pawandeep
  2017-12-28 15:36   ` kbuild test robot
  2017-12-28 16:07   ` kbuild test robot
  2017-12-27 10:20 ` [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC Oza Pawandeep
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: Oza Pawandeep @ 2017-12-27 10:20 UTC (permalink / raw)
  To: Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci, linux-kernel,
	Dongdong Liu, Gabriele Paoloni, Keith Busch, Wei Zhang,
	Sinan Kaya, Timur Tabi
  Cc: Oza Pawandeep

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 <poza@codeaurora.org>

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..d59866c
--- /dev/null
+++ b/drivers/pci/pcie/pcie-err.c
@@ -0,0 +1,333 @@
+/*
+ * 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;
+
+	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 = pci_find_aer_service(udev);
+
+	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
-- 
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.

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC
  2017-12-27 10:20 [PATCH 0/4] Address error and recovery for AER and DPC Oza Pawandeep
  2017-12-27 10:20 ` [PATCH 1/4] PCI/AER: factor out error reporting from AER Oza Pawandeep
@ 2017-12-27 10:20 ` Oza Pawandeep
  2017-12-28 16:27   ` kbuild test robot
                     ` (3 more replies)
  2017-12-27 10:20 ` [PATCH 3/4] PCI/ERR: Do not do recovery if DPC service is active Oza Pawandeep
                   ` (2 subsequent siblings)
  4 siblings, 4 replies; 13+ messages in thread
From: Oza Pawandeep @ 2017-12-27 10:20 UTC (permalink / raw)
  To: Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci, linux-kernel,
	Dongdong Liu, Gabriele Paoloni, Keith Busch, Wei Zhang,
	Sinan Kaya, Timur Tabi
  Cc: Oza Pawandeep

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 <poza@codeaurora.org>

diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 2d976a6..e7ced58 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;
+}
+
+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 d59866c..8bac584 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;
@@ -191,7 +191,10 @@ pci_ers_result_t pci_reset_link(struct pci_dev *dev)
 	}
 
 	/* Use the aer driver of the component firstly */
-	driver = pci_find_aer_service(udev);
+	if (severity == PCI_ERR_DPC_FATAL)
+		driver = pci_find_dpc_service(udev);
+	else
+		driver = pci_find_aer_service(udev);
 
 	if (driver && driver->reset_link) {
 		status = driver->reset_link(udev);
@@ -280,7 +283,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;
@@ -290,8 +294,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.

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] PCI/ERR: Do not do recovery if DPC service is active
  2017-12-27 10:20 [PATCH 0/4] Address error and recovery for AER and DPC Oza Pawandeep
  2017-12-27 10:20 ` [PATCH 1/4] PCI/AER: factor out error reporting from AER Oza Pawandeep
  2017-12-27 10:20 ` [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC Oza Pawandeep
@ 2017-12-27 10:20 ` Oza Pawandeep
  2017-12-27 10:20 ` [PATCH 4/4] PCI/DPC: Enumerate the devices after DPC trigger event Oza Pawandeep
  2017-12-28 17:34 ` [PATCH 0/4] Address error and recovery for AER and DPC Keith Busch
  4 siblings, 0 replies; 13+ messages in thread
From: Oza Pawandeep @ 2017-12-27 10:20 UTC (permalink / raw)
  To: Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci, linux-kernel,
	Dongdong Liu, Gabriele Paoloni, Keith Busch, Wei Zhang,
	Sinan Kaya, Timur Tabi
  Cc: Oza Pawandeep

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 <poza@codeaurora.org>

diff --git a/drivers/pci/pcie/pcie-err.c b/drivers/pci/pcie/pcie-err.c
index 8bac584..1f01e76 100644
--- a/drivers/pci/pcie/pcie-err.c
+++ b/drivers/pci/pcie/pcie-err.c
@@ -267,6 +267,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
@@ -280,9 +296,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.

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] PCI/DPC: Enumerate the devices after DPC trigger event
  2017-12-27 10:20 [PATCH 0/4] Address error and recovery for AER and DPC Oza Pawandeep
                   ` (2 preceding siblings ...)
  2017-12-27 10:20 ` [PATCH 3/4] PCI/ERR: Do not do recovery if DPC service is active Oza Pawandeep
@ 2017-12-27 10:20 ` Oza Pawandeep
  2017-12-28 17:34 ` [PATCH 0/4] Address error and recovery for AER and DPC Keith Busch
  4 siblings, 0 replies; 13+ messages in thread
From: Oza Pawandeep @ 2017-12-27 10:20 UTC (permalink / raw)
  To: Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci, linux-kernel,
	Dongdong Liu, Gabriele Paoloni, Keith Busch, Wei Zhang,
	Sinan Kaya, Timur Tabi
  Cc: Oza Pawandeep

Implement error_resume callback in DPC, which, after DPC trigger event
enumerates the devices beneath.

Signed-off-by: Oza Pawandeep <poza@codeaurora.org>

diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index e7ced58..78e557f 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 1f01e76..9c4377c 100644
--- a/drivers/pci/pcie/pcie-err.c
+++ b/drivers/pci/pcie/pcie-err.c
@@ -231,7 +231,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;
 
@@ -243,6 +244,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
@@ -328,7 +338,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)) {
@@ -337,11 +348,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) {
 		/*
@@ -352,16 +367,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.

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] PCI/AER: factor out error reporting from AER
  2017-12-27 10:20 ` [PATCH 1/4] PCI/AER: factor out error reporting from AER Oza Pawandeep
@ 2017-12-28 15:36   ` kbuild test robot
  2017-12-28 16:07   ` kbuild test robot
  1 sibling, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2017-12-28 15:36 UTC (permalink / raw)
  To: Oza Pawandeep
  Cc: kbuild-all, Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci, linux-kernel,
	Dongdong Liu, Gabriele Paoloni, Keith Busch, Wei Zhang,
	Sinan Kaya, Timur Tabi, Oza Pawandeep

[-- Attachment #1: Type: text/plain, Size: 9135 bytes --]

Hi Oza,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.15-rc5]
[cannot apply to pci/next pm/linux-next next-20171222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Oza-Pawandeep/Address-error-and-recovery-for-AER-and-DPC/20171228-222058
config: i386-randconfig-s0-201752 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from include/trace/define_trace.h:96:0,
                    from include/ras/ras_event.h:414,
                    from drivers/ras/ras.c:15:
   include/trace/../../include/ras/ras_event.h: In function 'trace_raw_output_aer_event':
>> include/trace/../../include/ras/ras_event.h:319:24: error: 'AER_CORRECTABLE' undeclared (first use in this function)
      __entry->severity == AER_CORRECTABLE ? "Corrected" :
                           ^
   include/trace/trace_events.h:360:22: note: in definition of macro 'DECLARE_EVENT_CLASS'
     trace_seq_printf(s, print);     \
                         ^~~~~
   include/trace/trace_events.h:79:9: note: in expansion of macro 'PARAMS'
            PARAMS(print));         \
            ^~~~~~
   include/trace/../../include/ras/ras_event.h:298:1: note: in expansion of macro 'TRACE_EVENT'
    TRACE_EVENT(aer_event,
    ^~~~~~~~~~~
   include/trace/../../include/ras/ras_event.h:317:2: note: in expansion of macro 'TP_printk'
     TP_printk("%s PCIe Bus Error: severity=%s, %s\n",
     ^~~~~~~~~
   include/trace/../../include/ras/ras_event.h:319:24: note: each undeclared identifier is reported only once for each function it appears in
      __entry->severity == AER_CORRECTABLE ? "Corrected" :
                           ^
   include/trace/trace_events.h:360:22: note: in definition of macro 'DECLARE_EVENT_CLASS'
     trace_seq_printf(s, print);     \
                         ^~~~~
   include/trace/trace_events.h:79:9: note: in expansion of macro 'PARAMS'
            PARAMS(print));         \
            ^~~~~~
   include/trace/../../include/ras/ras_event.h:298:1: note: in expansion of macro 'TRACE_EVENT'
    TRACE_EVENT(aer_event,
    ^~~~~~~~~~~
   include/trace/../../include/ras/ras_event.h:317:2: note: in expansion of macro 'TP_printk'
     TP_printk("%s PCIe Bus Error: severity=%s, %s\n",
     ^~~~~~~~~
>> include/trace/../../include/ras/ras_event.h:320:25: error: 'AER_FATAL' undeclared (first use in this function)
       __entry->severity == AER_FATAL ?
                            ^
   include/trace/trace_events.h:360:22: note: in definition of macro 'DECLARE_EVENT_CLASS'
     trace_seq_printf(s, print);     \
                         ^~~~~
   include/trace/trace_events.h:79:9: note: in expansion of macro 'PARAMS'
            PARAMS(print));         \
            ^~~~~~
   include/trace/../../include/ras/ras_event.h:298:1: note: in expansion of macro 'TRACE_EVENT'
    TRACE_EVENT(aer_event,
    ^~~~~~~~~~~
   include/trace/../../include/ras/ras_event.h:317:2: note: in expansion of macro 'TP_printk'
     TP_printk("%s PCIe Bus Error: severity=%s, %s\n",
     ^~~~~~~~~

vim +/AER_CORRECTABLE +319 include/trace/../../include/ras/ras_event.h

297b64c7 Tyler Baicar 2017-06-21  254  
297b64c7 Tyler Baicar 2017-06-21  255  /*
0a2409aa Chen, Gong   2014-06-11  256   * PCIe AER Trace event
0a2409aa Chen, Gong   2014-06-11  257   *
0a2409aa Chen, Gong   2014-06-11  258   * These events are generated when hardware detects a corrected or
0a2409aa Chen, Gong   2014-06-11  259   * uncorrected event on a PCIe device. The event report has
0a2409aa Chen, Gong   2014-06-11  260   * the following structure:
0a2409aa Chen, Gong   2014-06-11  261   *
0a2409aa Chen, Gong   2014-06-11  262   * char * dev_name -	The name of the slot where the device resides
0a2409aa Chen, Gong   2014-06-11  263   *			([domain:]bus:device.function).
0a2409aa Chen, Gong   2014-06-11  264   * u32 status -		Either the correctable or uncorrectable register
0a2409aa Chen, Gong   2014-06-11  265   *			indicating what error or errors have been seen
0a2409aa Chen, Gong   2014-06-11  266   * u8 severity -	error severity 0:NONFATAL 1:FATAL 2:CORRECTED
0a2409aa Chen, Gong   2014-06-11  267   */
0a2409aa Chen, Gong   2014-06-11  268  
0a2409aa Chen, Gong   2014-06-11  269  #define aer_correctable_errors					\
99d44024 Chen, Gong   2014-08-13  270  	{PCI_ERR_COR_RCVR,	"Receiver Error"},		\
99d44024 Chen, Gong   2014-08-13  271  	{PCI_ERR_COR_BAD_TLP,	"Bad TLP"},			\
99d44024 Chen, Gong   2014-08-13  272  	{PCI_ERR_COR_BAD_DLLP,	"Bad DLLP"},			\
99d44024 Chen, Gong   2014-08-13  273  	{PCI_ERR_COR_REP_ROLL,	"RELAY_NUM Rollover"},		\
99d44024 Chen, Gong   2014-08-13  274  	{PCI_ERR_COR_REP_TIMER,	"Replay Timer Timeout"},	\
cb9a684a Chen, Gong   2014-08-13  275  	{PCI_ERR_COR_ADV_NFAT,	"Advisory Non-Fatal Error"},	\
cb9a684a Chen, Gong   2014-08-13  276  	{PCI_ERR_COR_INTERNAL,	"Corrected Internal Error"},	\
cb9a684a Chen, Gong   2014-08-13  277  	{PCI_ERR_COR_LOG_OVER,	"Header Log Overflow"}
0a2409aa Chen, Gong   2014-06-11  278  
0a2409aa Chen, Gong   2014-06-11  279  #define aer_uncorrectable_errors				\
846fc709 Chen, Gong   2014-08-13  280  	{PCI_ERR_UNC_UND,	"Undefined"},			\
cb9a684a Chen, Gong   2014-08-13  281  	{PCI_ERR_UNC_DLP,	"Data Link Protocol Error"},	\
cb9a684a Chen, Gong   2014-08-13  282  	{PCI_ERR_UNC_SURPDN,	"Surprise Down Error"},		\
99d44024 Chen, Gong   2014-08-13  283  	{PCI_ERR_UNC_POISON_TLP,"Poisoned TLP"},		\
cb9a684a Chen, Gong   2014-08-13  284  	{PCI_ERR_UNC_FCP,	"Flow Control Protocol Error"},	\
99d44024 Chen, Gong   2014-08-13  285  	{PCI_ERR_UNC_COMP_TIME,	"Completion Timeout"},		\
99d44024 Chen, Gong   2014-08-13  286  	{PCI_ERR_UNC_COMP_ABORT,"Completer Abort"},		\
99d44024 Chen, Gong   2014-08-13  287  	{PCI_ERR_UNC_UNX_COMP,	"Unexpected Completion"},	\
99d44024 Chen, Gong   2014-08-13  288  	{PCI_ERR_UNC_RX_OVER,	"Receiver Overflow"},		\
99d44024 Chen, Gong   2014-08-13  289  	{PCI_ERR_UNC_MALF_TLP,	"Malformed TLP"},		\
cb9a684a Chen, Gong   2014-08-13  290  	{PCI_ERR_UNC_ECRC,	"ECRC Error"},			\
cb9a684a Chen, Gong   2014-08-13  291  	{PCI_ERR_UNC_UNSUP,	"Unsupported Request Error"},	\
cb9a684a Chen, Gong   2014-08-13  292  	{PCI_ERR_UNC_ACSV,	"ACS Violation"},		\
cb9a684a Chen, Gong   2014-08-13  293  	{PCI_ERR_UNC_INTN,	"Uncorrectable Internal Error"},\
cb9a684a Chen, Gong   2014-08-13  294  	{PCI_ERR_UNC_MCBTLP,	"MC Blocked TLP"},		\
cb9a684a Chen, Gong   2014-08-13  295  	{PCI_ERR_UNC_ATOMEG,	"AtomicOp Egress Blocked"},	\
cb9a684a Chen, Gong   2014-08-13  296  	{PCI_ERR_UNC_TLPPRE,	"TLP Prefix Blocked Error"}
0a2409aa Chen, Gong   2014-06-11  297  
0a2409aa Chen, Gong   2014-06-11 @298  TRACE_EVENT(aer_event,
0a2409aa Chen, Gong   2014-06-11  299  	TP_PROTO(const char *dev_name,
0a2409aa Chen, Gong   2014-06-11  300  		 const u32 status,
0a2409aa Chen, Gong   2014-06-11  301  		 const u8 severity),
0a2409aa Chen, Gong   2014-06-11  302  
0a2409aa Chen, Gong   2014-06-11  303  	TP_ARGS(dev_name, status, severity),
0a2409aa Chen, Gong   2014-06-11  304  
0a2409aa Chen, Gong   2014-06-11  305  	TP_STRUCT__entry(
0a2409aa Chen, Gong   2014-06-11  306  		__string(	dev_name,	dev_name	)
0a2409aa Chen, Gong   2014-06-11  307  		__field(	u32,		status		)
0a2409aa Chen, Gong   2014-06-11  308  		__field(	u8,		severity	)
0a2409aa Chen, Gong   2014-06-11  309  	),
0a2409aa Chen, Gong   2014-06-11  310  
0a2409aa Chen, Gong   2014-06-11  311  	TP_fast_assign(
0a2409aa Chen, Gong   2014-06-11  312  		__assign_str(dev_name, dev_name);
0a2409aa Chen, Gong   2014-06-11  313  		__entry->status		= status;
0a2409aa Chen, Gong   2014-06-11  314  		__entry->severity	= severity;
0a2409aa Chen, Gong   2014-06-11  315  	),
0a2409aa Chen, Gong   2014-06-11  316  
0a2409aa Chen, Gong   2014-06-11  317  	TP_printk("%s PCIe Bus Error: severity=%s, %s\n",
0a2409aa Chen, Gong   2014-06-11  318  		__get_str(dev_name),
0a2409aa Chen, Gong   2014-06-11 @319  		__entry->severity == AER_CORRECTABLE ? "Corrected" :
0a2409aa Chen, Gong   2014-06-11 @320  			__entry->severity == AER_FATAL ?
0a2409aa Chen, Gong   2014-06-11  321  			"Fatal" : "Uncorrected, non-fatal",
0a2409aa Chen, Gong   2014-06-11  322  		__entry->severity == AER_CORRECTABLE ?
0a2409aa Chen, Gong   2014-06-11  323  		__print_flags(__entry->status, "|", aer_correctable_errors) :
0a2409aa Chen, Gong   2014-06-11  324  		__print_flags(__entry->status, "|", aer_uncorrectable_errors))
0a2409aa Chen, Gong   2014-06-11  325  );
0a2409aa Chen, Gong   2014-06-11  326  

:::::: The code at line 319 was first introduced by commit
:::::: 0a2409aad38e97b1db55e6515b990be7b17060f6 trace, AER: Move trace into unified interface

:::::: TO: Chen, Gong <gong.chen@linux.intel.com>
:::::: CC: Tony Luck <tony.luck@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24415 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] PCI/AER: factor out error reporting from AER
  2017-12-27 10:20 ` [PATCH 1/4] PCI/AER: factor out error reporting from AER Oza Pawandeep
  2017-12-28 15:36   ` kbuild test robot
@ 2017-12-28 16:07   ` kbuild test robot
  1 sibling, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2017-12-28 16:07 UTC (permalink / raw)
  To: Oza Pawandeep
  Cc: kbuild-all, Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci, linux-kernel,
	Dongdong Liu, Gabriele Paoloni, Keith Busch, Wei Zhang,
	Sinan Kaya, Timur Tabi, Oza Pawandeep

[-- Attachment #1: Type: text/plain, Size: 2233 bytes --]

Hi Oza,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.15-rc5]
[cannot apply to pci/next pm/linux-next next-20171222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Oza-Pawandeep/Address-error-and-recovery-for-AER-and-DPC/20171228-222058
config: i386-randconfig-i0-201752 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/pci/pcie/pcie-err.o: In function `pci_reset_link':
>> drivers/pci/pcie/pcie-err.c:194: undefined reference to `pci_find_aer_service'

vim +194 drivers/pci/pcie/pcie-err.c

   178	
   179	pci_ers_result_t pci_reset_link(struct pci_dev *dev)
   180	{
   181		struct pci_dev *udev;
   182		pci_ers_result_t status;
   183		struct pcie_port_service_driver *driver;
   184	
   185		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
   186			/* Reset this port for all subordinates */
   187			udev = dev;
   188		} else {
   189			/* Reset the upstream component (likely downstream port) */
   190			udev = dev->bus->self;
   191		}
   192	
   193		/* Use the aer driver of the component firstly */
 > 194		driver = pci_find_aer_service(udev);
   195	
   196		if (driver && driver->reset_link) {
   197			status = driver->reset_link(udev);
   198		} else if (udev->has_secondary_link) {
   199			status = pci_default_reset_link(udev);
   200		} else {
   201			dev_printk(KERN_DEBUG, &dev->dev,
   202				"no link-reset support at upstream device %s\n",
   203				pci_name(udev));
   204			return PCI_ERS_RESULT_DISCONNECT;
   205		}
   206	
   207		if (status != PCI_ERS_RESULT_RECOVERED) {
   208			dev_printk(KERN_DEBUG, &dev->dev,
   209				"link reset at upstream device %s failed\n",
   210				pci_name(udev));
   211			return PCI_ERS_RESULT_DISCONNECT;
   212		}
   213	
   214		return status;
   215	}
   216	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25669 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH] PCI/DPC/AER: pci_find_dpc_dev() can be static
  2017-12-27 10:20 ` [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC Oza Pawandeep
  2017-12-28 16:27   ` kbuild test robot
@ 2017-12-28 16:27   ` kbuild test robot
  2017-12-28 17:37   ` [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC kbuild test robot
  2017-12-28 18:07   ` kbuild test robot
  3 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2017-12-28 16:27 UTC (permalink / raw)
  To: Oza Pawandeep
  Cc: kbuild-all, Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci, linux-kernel,
	Dongdong Liu, Gabriele Paoloni, Keith Busch, Wei Zhang,
	Sinan Kaya, Timur Tabi, Oza Pawandeep


Fixes: fc0212127f27 ("PCI/DPC/AER: Address Concurrency between AER and DPC")
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
 pcie-dpc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index e7ced58..68296ec 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -88,7 +88,7 @@ static int find_dpc_dev_iter(struct device *device, void *data)
 	return 0;
 }
 
-struct device *pci_find_dpc_dev(struct pci_dev *pdev)
+static struct device *pci_find_dpc_dev(struct pci_dev *pdev)
 {
 	struct device *dev = NULL;
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC
  2017-12-27 10:20 ` [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC Oza Pawandeep
@ 2017-12-28 16:27   ` kbuild test robot
  2017-12-28 16:27   ` [RFC PATCH] PCI/DPC/AER: pci_find_dpc_dev() can be static kbuild test robot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2017-12-28 16:27 UTC (permalink / raw)
  To: Oza Pawandeep
  Cc: kbuild-all, Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci, linux-kernel,
	Dongdong Liu, Gabriele Paoloni, Keith Busch, Wei Zhang,
	Sinan Kaya, Timur Tabi, Oza Pawandeep

Hi Oza,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.15-rc5]
[cannot apply to pci/next pm/linux-next next-20171222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Oza-Pawandeep/Address-error-and-recovery-for-AER-and-DPC/20171228-222058
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)


Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/4] Address error and recovery for AER and DPC
  2017-12-27 10:20 [PATCH 0/4] Address error and recovery for AER and DPC Oza Pawandeep
                   ` (3 preceding siblings ...)
  2017-12-27 10:20 ` [PATCH 4/4] PCI/DPC: Enumerate the devices after DPC trigger event Oza Pawandeep
@ 2017-12-28 17:34 ` Keith Busch
  2017-12-29  5:15   ` poza
  4 siblings, 1 reply; 13+ messages in thread
From: Keith Busch @ 2017-12-28 17:34 UTC (permalink / raw)
  To: Oza Pawandeep
  Cc: Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, Dongdong Liu, Gabriele Paoloni,
	Wei Zhang, Sinan Kaya, Timur Tabi

On Wed, Dec 27, 2017 at 02:20:18AM -0800, Oza Pawandeep wrote:
> DPC should enumerate the devices after recovering the link, which is
> achieved by implementing error_resume callback.

Wouldn't that race with the link-up event that pciehp currently handles?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC
  2017-12-27 10:20 ` [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC Oza Pawandeep
  2017-12-28 16:27   ` kbuild test robot
  2017-12-28 16:27   ` [RFC PATCH] PCI/DPC/AER: pci_find_dpc_dev() can be static kbuild test robot
@ 2017-12-28 17:37   ` kbuild test robot
  2017-12-28 18:07   ` kbuild test robot
  3 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2017-12-28 17:37 UTC (permalink / raw)
  To: Oza Pawandeep
  Cc: kbuild-all, Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci, linux-kernel,
	Dongdong Liu, Gabriele Paoloni, Keith Busch, Wei Zhang,
	Sinan Kaya, Timur Tabi, Oza Pawandeep

[-- Attachment #1: Type: text/plain, Size: 1216 bytes --]

Hi Oza,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.15-rc5]
[cannot apply to pci/next pm/linux-next next-20171222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Oza-Pawandeep/Address-error-and-recovery-for-AER-and-DPC/20171228-222058
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers/pci/pcie/pcie-err.o: In function `pci_reset_link':
>> pcie-err.c:(.text+0x3f8): undefined reference to `pci_find_dpc_service'
   pcie-err.c:(.text+0x3f8): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `pci_find_dpc_service'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37442 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC
  2017-12-27 10:20 ` [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC Oza Pawandeep
                     ` (2 preceding siblings ...)
  2017-12-28 17:37   ` [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC kbuild test robot
@ 2017-12-28 18:07   ` kbuild test robot
  3 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2017-12-28 18:07 UTC (permalink / raw)
  To: Oza Pawandeep
  Cc: kbuild-all, Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci, linux-kernel,
	Dongdong Liu, Gabriele Paoloni, Keith Busch, Wei Zhang,
	Sinan Kaya, Timur Tabi, Oza Pawandeep

[-- Attachment #1: Type: text/plain, Size: 2355 bytes --]

Hi Oza,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.15-rc5]
[cannot apply to pci/next pm/linux-next next-20171222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Oza-Pawandeep/Address-error-and-recovery-for-AER-and-DPC/20171228-222058
config: x86_64-randconfig-r0-12282251 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/pci/pcie/pcie-err.o: In function `pci_reset_link':
>> drivers/pci/pcie/pcie-err.c:195: undefined reference to `pci_find_dpc_service'

vim +195 drivers/pci/pcie/pcie-err.c

   178	
   179	pci_ers_result_t pci_reset_link(struct pci_dev *dev, int severity)
   180	{
   181		struct pci_dev *udev;
   182		pci_ers_result_t status;
   183		struct pcie_port_service_driver *driver;
   184	
   185		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
   186			/* Reset this port for all subordinates */
   187			udev = dev;
   188		} else {
   189			/* Reset the upstream component (likely downstream port) */
   190			udev = dev->bus->self;
   191		}
   192	
   193		/* Use the aer driver of the component firstly */
   194		if (severity == PCI_ERR_DPC_FATAL)
 > 195			driver = pci_find_dpc_service(udev);
   196		else
   197			driver = pci_find_aer_service(udev);
   198	
   199		if (driver && driver->reset_link) {
   200			status = driver->reset_link(udev);
   201		} else if (udev->has_secondary_link) {
   202			status = pci_default_reset_link(udev);
   203		} else {
   204			dev_printk(KERN_DEBUG, &dev->dev,
   205				"no link-reset support at upstream device %s\n",
   206				pci_name(udev));
   207			return PCI_ERS_RESULT_DISCONNECT;
   208		}
   209	
   210		if (status != PCI_ERS_RESULT_RECOVERED) {
   211			dev_printk(KERN_DEBUG, &dev->dev,
   212				"link reset at upstream device %s failed\n",
   213				pci_name(udev));
   214			return PCI_ERS_RESULT_DISCONNECT;
   215		}
   216	
   217		return status;
   218	}
   219	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31810 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/4] Address error and recovery for AER and DPC
  2017-12-28 17:34 ` [PATCH 0/4] Address error and recovery for AER and DPC Keith Busch
@ 2017-12-29  5:15   ` poza
  0 siblings, 0 replies; 13+ messages in thread
From: poza @ 2017-12-29  5:15 UTC (permalink / raw)
  To: Keith Busch
  Cc: Bjorn Helgaas, Philippe Ombredanne, Thomas Gleixner,
	Greg Kroah-Hartman, Kate Stewart, linux-pci, linux-kernel,
	Dongdong Liu, Gabriele Paoloni, Wei Zhang, Sinan Kaya, Timur Tabi

On 2017-12-28 23:04, Keith Busch wrote:
> On Wed, Dec 27, 2017 at 02:20:18AM -0800, Oza Pawandeep wrote:
>> DPC should enumerate the devices after recovering the link, which is
>> achieved by implementing error_resume callback.
> 
> Wouldn't that race with the link-up event that pciehp currently 
> handles?

It is with pci_lock_rescan_remove().
I was able to test with and without pciehp on our platform, and things 
seemed to be okay.

Regards,
Oza.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-12-29  5:15 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-27 10:20 [PATCH 0/4] Address error and recovery for AER and DPC Oza Pawandeep
2017-12-27 10:20 ` [PATCH 1/4] PCI/AER: factor out error reporting from AER Oza Pawandeep
2017-12-28 15:36   ` kbuild test robot
2017-12-28 16:07   ` kbuild test robot
2017-12-27 10:20 ` [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC Oza Pawandeep
2017-12-28 16:27   ` kbuild test robot
2017-12-28 16:27   ` [RFC PATCH] PCI/DPC/AER: pci_find_dpc_dev() can be static kbuild test robot
2017-12-28 17:37   ` [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC kbuild test robot
2017-12-28 18:07   ` kbuild test robot
2017-12-27 10:20 ` [PATCH 3/4] PCI/ERR: Do not do recovery if DPC service is active Oza Pawandeep
2017-12-27 10:20 ` [PATCH 4/4] PCI/DPC: Enumerate the devices after DPC trigger event Oza Pawandeep
2017-12-28 17:34 ` [PATCH 0/4] Address error and recovery for AER and DPC Keith Busch
2017-12-29  5:15   ` poza

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).