Address error and recovery for AER and DPC
cover Address error and recovery for AER and DPC
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
Signature · 2 lines
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.
PATCH 1/4 PCI/AER: factor out error reporting from AER
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.
drivers/acpi/apei/ghes.c +1 -1
@@ -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,
drivers/pci/pcie/Makefile +1 -1
@@ -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
drivers/pci/pcie/aer/aerdrv.h +0 -30
@@ -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);
drivers/pci/pcie/aer/aerdrv_core.c +11 -295
373 lines not on this page · raw message
drivers/pci/pcie/aer/aerdrv_errprint.c +16 -11
@@ -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);
drivers/pci/pcie/pcie-err.c new file +333 -0
333 lines not on this page · raw message
drivers/pci/pcie/portdrv.h +1 -0
@@ -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_ */
include/linux/aer.h +0 -4
@@ -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 {
include/linux/pci.h +21 -0
@@ -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 resvoid 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
Signature · 2 lines
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.
kbuild test robot
build botRe: [PATCH 1/4] PCI/AER: factor out error reporting from AER
Thank you for the patch! Yet something to improve:
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 <[email protected]>
:::::: CC: Tony Luck <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
kbuild test robot
build botRe: [PATCH 1/4] PCI/AER: factor out error reporting from AER
In reply to Oza PawandeepThank you for the patch! Yet something to improve:
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
PATCH 2/4 PCI/DPC/AER: Address Concurrency between AER and DPC
In reply to Oza PawandeepThis 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.
drivers/pci/pcie/pcie-dpc.c +85 -4
@@ -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 )
drivers/pci/pcie/pcie-err.c +10 -5
@@ -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; }
drivers/pci/pcie/portdrv.h +1 -0
@@ -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_ */
include/linux/pci.h +2 -1
@@ -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);
Signature · 2 lines
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.
kbuild test robot
build botRe: [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC
Thank you for the patch! Perhaps something to improve:
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
kbuild test robot
build bot[RFC PATCH] PCI/DPC/AER: pci_find_dpc_dev() can be static
In reply to Oza PawandeepFixes: fc0212127f27 ("PCI/DPC/AER: Address Concurrency between AER and DPC")
--- pcie-dpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pci/pcie/pcie-dpc.c +1 -1
@@ -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 ;
kbuild test robot
build botRe: [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC
In reply to Oza PawandeepThank you for the patch! Yet something to improve:
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
kbuild test robot
build botRe: [PATCH 2/4] PCI/DPC/AER: Address Concurrency between AER and DPC
In reply to Oza PawandeepThank you for the patch! Yet something to improve:
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
PATCH 3/4 PCI/ERR: Do not do recovery if DPC service is active
In reply to Oza PawandeepIf 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
drivers/pci/pcie/pcie-err.c +36 -0
@@ -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;
Signature · 2 lines
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.
PATCH 4/4 PCI/DPC: Enumerate the devices after DPC trigger event
In reply to Oza PawandeepImplement error_resume callback in DPC, which, after DPC trigger event enumerates the devices beneath.
drivers/pci/pcie/pcie-dpc.c +38 -0
@@ -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, };
drivers/pci/pcie/pcie-err.c +23 -5
@@ -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);
include/linux/pci.h +2 -1
@@ -2010,7 +2010,8 @@ static inline resource_size_t pci_iov_resource_size(struct pci_dev *dev, int respci_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);
Signature · 2 lines
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.
Quoted 2 lines · Oza Pawandeep
DPC should enumerate the devices after recovering the link, which is achieved by implementing
error_resumecallback.
Wouldn't that race with the link-up event that pciehp currently handles?
Quoted 5 lines · Keith Busch
Quoted 2 lines · Oza Pawandeep
DPC should enumerate the devices after recovering the link, which is achieved by implementing
error_resumecallback.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.