diff mbox

[v3] aerdrv: Move cper_print_aer() call out of interrupt context

Message ID 20130529190326.24171.86905.stgit@grignak.americas.hpqcorp.net
State Not Applicable
Headers show

Commit Message

Lance Ortiz May 29, 2013, 7:03 p.m. UTC 
The following warning was seen on 3.9 when a corrected PCIe error was being
handled by the AER subsystem.

WARNING: at .../drivers/pci/search.c:214 pci_get_dev_by_id+0x8a/0x90()

This occurred because a call to pci_get_domain_bus_and_slot() was added to
cper_print_pcie() to setup for the call to cper_print_aer().  The warning
showed up because cper_print_pcie() is called in an interrupt context and
pci_get* functions are not supposed to be called in that context.

The solution is to move the cper_print_aer() call out of the interrupt
context and into aer_recover_work_func() to avoid any warnings when calling
pci_get* functions.

v2 - Re-worded header text.  Removed prefix arg from cper_print_aer().
     Added TODO message in cper_print_aer().
v3 - Changed type of u8* to struct aer_capability_regs* in the code
     to avoid too much casting based on comment from Bjorn Helgaas.

Signed-off-by: Lance Ortiz <lance.ortiz@hp.com>
Acked-by: Borislav Petkov <bp@suse.de>
---

 drivers/acpi/apei/cper.c               |   18 ------------------
 drivers/acpi/apei/ghes.c               |    4 +++-
 drivers/pci/pcie/aer/aerdrv_core.c     |    5 ++++-
 drivers/pci/pcie/aer/aerdrv_errprint.c |   10 ++++++++--
 include/linux/aer.h                    |    5 +++--
 5 files changed, 18 insertions(+), 24 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Rafael J. Wysocki May 29, 2013, 10:48 p.m. UTC | #1 
On Wednesday, May 29, 2013 01:03:26 PM Lance Ortiz wrote:
> The following warning was seen on 3.9 when a corrected PCIe error was being
> handled by the AER subsystem.
> 
> WARNING: at .../drivers/pci/search.c:214 pci_get_dev_by_id+0x8a/0x90()
> 
> This occurred because a call to pci_get_domain_bus_and_slot() was added to
> cper_print_pcie() to setup for the call to cper_print_aer().  The warning
> showed up because cper_print_pcie() is called in an interrupt context and
> pci_get* functions are not supposed to be called in that context.
> 
> The solution is to move the cper_print_aer() call out of the interrupt
> context and into aer_recover_work_func() to avoid any warnings when calling
> pci_get* functions.
> 
> v2 - Re-worded header text.  Removed prefix arg from cper_print_aer().
>      Added TODO message in cper_print_aer().
> v3 - Changed type of u8* to struct aer_capability_regs* in the code
>      to avoid too much casting based on comment from Bjorn Helgaas.
> 
> Signed-off-by: Lance Ortiz <lance.ortiz@hp.com>
> Acked-by: Borislav Petkov <bp@suse.de>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
> 
>  drivers/acpi/apei/cper.c               |   18 ------------------
>  drivers/acpi/apei/ghes.c               |    4 +++-
>  drivers/pci/pcie/aer/aerdrv_core.c     |    5 ++++-
>  drivers/pci/pcie/aer/aerdrv_errprint.c |   10 ++++++++--
>  include/linux/aer.h                    |    5 +++--
>  5 files changed, 18 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
> index 1e5d8a4..8713229 100644
> --- a/drivers/acpi/apei/cper.c
> +++ b/drivers/acpi/apei/cper.c
> @@ -250,10 +250,6 @@ static const char *cper_pcie_port_type_strs[] = {
>  static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
>  			    const struct acpi_hest_generic_data *gdata)
>  {
> -#ifdef CONFIG_ACPI_APEI_PCIEAER
> -	struct pci_dev *dev;
> -#endif
> -
>  	if (pcie->validation_bits & CPER_PCIE_VALID_PORT_TYPE)
>  		printk("%s""port_type: %d, %s\n", pfx, pcie->port_type,
>  		       pcie->port_type < ARRAY_SIZE(cper_pcie_port_type_strs) ?
> @@ -285,20 +281,6 @@ static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
>  		printk(
>  	"%s""bridge: secondary_status: 0x%04x, control: 0x%04x\n",
>  	pfx, pcie->bridge.secondary_status, pcie->bridge.control);
> -#ifdef CONFIG_ACPI_APEI_PCIEAER
> -	dev = pci_get_domain_bus_and_slot(pcie->device_id.segment,
> -			pcie->device_id.bus, pcie->device_id.function);
> -	if (!dev) {
> -		pr_err("PCI AER Cannot get PCI device %04x:%02x:%02x.%d\n",
> -			pcie->device_id.segment, pcie->device_id.bus,
> -			pcie->device_id.slot, pcie->device_id.function);
> -		return;
> -	}
> -	if (pcie->validation_bits & CPER_PCIE_VALID_AER_INFO)
> -		cper_print_aer(pfx, dev, gdata->error_severity,
> -				(struct aer_capability_regs *) pcie->aer_info);
> -	pci_dev_put(dev);
> -#endif
>  }
>  
>  static const char *apei_estatus_section_flag_strs[] = {
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index d668a8a..403baf4 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -454,7 +454,9 @@ static void ghes_do_proc(struct ghes *ghes,
>  				aer_severity = cper_severity_to_aer(sev);
>  				aer_recover_queue(pcie_err->device_id.segment,
>  						  pcie_err->device_id.bus,
> -						  devfn, aer_severity);
> +						  devfn, aer_severity,
> +						  (struct aer_capability_regs *)
> +						  pcie_err->aer_info);
>  			}
>  
>  		}
> diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c
> index 564d97f..120549a 100644
> --- a/drivers/pci/pcie/aer/aerdrv_core.c
> +++ b/drivers/pci/pcie/aer/aerdrv_core.c
> @@ -582,6 +582,7 @@ struct aer_recover_entry
>  	u8	devfn;
>  	u16	domain;
>  	int	severity;
> +	struct aer_capability_regs *regs;
>  };
>  
>  static DEFINE_KFIFO(aer_recover_ring, struct aer_recover_entry,
> @@ -595,7 +596,7 @@ static DEFINE_SPINLOCK(aer_recover_ring_lock);
>  static DECLARE_WORK(aer_recover_work, aer_recover_work_func);
>  
>  void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
> -		       int severity)
> +		       int severity, struct aer_capability_regs *aer_regs)
>  {
>  	unsigned long flags;
>  	struct aer_recover_entry entry = {
> @@ -603,6 +604,7 @@ void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
>  		.devfn		= devfn,
>  		.domain		= domain,
>  		.severity	= severity,
> +		.regs		= aer_regs,
>  	};
>  
>  	spin_lock_irqsave(&aer_recover_ring_lock, flags);
> @@ -629,6 +631,7 @@ static void aer_recover_work_func(struct work_struct *work)
>  			       PCI_SLOT(entry.devfn), PCI_FUNC(entry.devfn));
>  			continue;
>  		}
> +		cper_print_aer(pdev, entry.severity, entry.regs);
>  		do_recovery(pdev, entry.severity);
>  		pci_dev_put(pdev);
>  	}
> diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
> index 5ab1425..f277dc3 100644
> --- a/drivers/pci/pcie/aer/aerdrv_errprint.c
> +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
> @@ -220,13 +220,19 @@ int cper_severity_to_aer(int cper_severity)
>  }
>  EXPORT_SYMBOL_GPL(cper_severity_to_aer);
>  
> -void cper_print_aer(const char *prefix, struct pci_dev *dev, int cper_severity,
> +void cper_print_aer(struct pci_dev *dev, int cper_severity,
>  		    struct aer_capability_regs *aer)
>  {
>  	int aer_severity, layer, agent, status_strs_size, tlp_header_valid = 0;
>  	u32 status, mask;
>  	const char **status_strs;
>  
> +	/*
> +	 * TODO: This function needs to be re-written so that it's output
> +	 * matches the output of aer_print_error().  Right now, the output
> +	 * is formatted very differently.
> +	 */
> +
>  	aer_severity = cper_severity_to_aer(cper_severity);
>  	if (aer_severity == AER_CORRECTABLE) {
>  		status = aer->cor_status;
> @@ -244,7 +250,7 @@ void cper_print_aer(const char *prefix, struct pci_dev *dev, int cper_severity,
>  	agent = AER_GET_AGENT(aer_severity, status);
>  	dev_err(&dev->dev, "aer_status: 0x%08x, aer_mask: 0x%08x\n",
>  	       status, mask);
> -	cper_print_bits(prefix, status, status_strs, status_strs_size);
> +	cper_print_bits("", status, status_strs, status_strs_size);
>  	dev_err(&dev->dev, "aer_layer=%s, aer_agent=%s\n",
>  	       aer_error_layer[layer], aer_agent_string[agent]);
>  	if (aer_severity != AER_CORRECTABLE)
> diff --git a/include/linux/aer.h b/include/linux/aer.h
> index ec10e1b..737f90a 100644
> --- a/include/linux/aer.h
> +++ b/include/linux/aer.h
> @@ -49,10 +49,11 @@ static inline int pci_cleanup_aer_uncorrect_error_status(struct pci_dev *dev)
>  }
>  #endif
>  
> -extern void cper_print_aer(const char *prefix, struct pci_dev *dev,
> +extern void cper_print_aer(struct pci_dev *dev,
>  			   int cper_severity, struct aer_capability_regs *aer);
>  extern int cper_severity_to_aer(int cper_severity);
>  extern void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
> -			      int severity);
> +			      int severity,
> +			      struct aer_capability_regs *aer_regs);
>  #endif //_AER_H_
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Luck May 29, 2013, 11:07 p.m. UTC | #2 
PiArCS8qDQo+ICsJICogVE9ETzogVGhpcyBmdW5jdGlvbiBuZWVkcyB0byBiZSByZS13cml0dGVu
IHNvIHRoYXQgaXQncyBvdXRwdXQNCj4gKwkgKiBtYXRjaGVzIHRoZSBvdXRwdXQgb2YgYWVyX3By
aW50X2Vycm9yKCkuICBSaWdodCBub3csIHRoZSBvdXRwdXQNCj4gKwkgKiBpcyBmb3JtYXR0ZWQg
dmVyeSBkaWZmZXJlbnRseS4NCj4gKwkgKi8NCg0KU28gd2UgaGF2ZSB0aGlzIGJpZyAiVE9ETyIg
Y29tbWVudCBzaXR0aW5nIHRoZXJlIHZlcnkgcHJvbWluZW50bHkgLi4uIHdoaWNoIExpbnVzDQpp
cyBib3VuZCB0byBhc2sgYWJvdXQgaWYgSSBhc2sgaGltIHRvIHB1bGwgdGhpcyBpbnRvIDMuMTAt
cmNYIC4uLiB3aGF0J3MgdGhlIGltcGFjdCBvZg0KdGhpcz8gIFdoYXQgc2hvdWxkIEkgc2F5IHdo
ZW4gaGUgYXNrcyB3aHkgc2hvdWxkIGhlIHB1bGwgdGhpcyBmaXggaW50byAzLjEwIHdoZW4NCnRo
ZXJlIGlzIHN0aWxsIHNvbWUgd29yayB0byBkbz8gIElzIG1hdGNoaW5nIHRoZSBvdXRwdXQgbm8g
YmlnIGRlYWwgYW5kIGNhbiB3YWl0IGZvcg0Kc29tZSBmdXR1cmUsIHdoaWxlIG1vdmluZyB0aGUg
cGNpIGJpdHMgdG8gdGhlIHdvcmsgZnVuY3Rpb24gbmVlZHMgdG8gZ28gaW4gbm93Pw0KDQotVG9u
eQ0K
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ortiz, Lance E May 30, 2013, 4:55 a.m. UTC | #3 
PiA+ICsJLyoNCj4gPiArCSAqIFRPRE86IFRoaXMgZnVuY3Rpb24gbmVlZHMgdG8gYmUgcmUtd3Jp
dHRlbiBzbyB0aGF0IGl0J3Mgb3V0cHV0DQo+ID4gKwkgKiBtYXRjaGVzIHRoZSBvdXRwdXQgb2Yg
YWVyX3ByaW50X2Vycm9yKCkuICBSaWdodCBub3csIHRoZQ0KPiBvdXRwdXQNCj4gPiArCSAqIGlz
IGZvcm1hdHRlZCB2ZXJ5IGRpZmZlcmVudGx5Lg0KPiA+ICsJICovDQo+IA0KPiBTbyB3ZSBoYXZl
IHRoaXMgYmlnICJUT0RPIiBjb21tZW50IHNpdHRpbmcgdGhlcmUgdmVyeSBwcm9taW5lbnRseSAu
Li4NCj4gd2hpY2ggTGludXMNCj4gaXMgYm91bmQgdG8gYXNrIGFib3V0IGlmIEkgYXNrIGhpbSB0
byBwdWxsIHRoaXMgaW50byAzLjEwLXJjWCAuLi4NCj4gd2hhdCdzIHRoZSBpbXBhY3Qgb2YNCj4g
dGhpcz8gIFdoYXQgc2hvdWxkIEkgc2F5IHdoZW4gaGUgYXNrcyB3aHkgc2hvdWxkIGhlIHB1bGwg
dGhpcyBmaXggaW50bw0KPiAzLjEwIHdoZW4NCj4gdGhlcmUgaXMgc3RpbGwgc29tZSB3b3JrIHRv
IGRvPyAgSXMgbWF0Y2hpbmcgdGhlIG91dHB1dCBubyBiaWcgZGVhbCBhbmQNCj4gY2FuIHdhaXQg
Zm9yDQo+IHNvbWUgZnV0dXJlLCB3aGlsZSBtb3ZpbmcgdGhlIHBjaSBiaXRzIHRvIHRoZSB3b3Jr
IGZ1bmN0aW9uIG5lZWRzIHRvIGdvDQo+IGluIG5vdz8NCg0KVG9ueSwgDQoNCllvdSBoYXZlIGEg
Z29vZCBwb2ludC4gIElkZWFsbHkgdGhlIGNvbnNvbGUgb3V0cHV0IHNob3VsZCBiZSB0aGUgc2Ft
ZSBpbiBib3RoIHRoZSBhZXIgYW5kIHRoZSBjcGVyIGNhc2UuICBUaGUgb3V0cHV0IGluIGNwZXJf
cHJpbnRfZXJyb3IoKSBkb2VzIGdpdmUgdXMgYSByZWFzb25hYmxlIGFtb3VudCBvZiBpbmZvcm1h
dGlvbiwganVzdCBub3QgYXMgZGV0YWlsZWQgYXMgSSB0aGUgYWVyIGNhc2UuIEFsc28gbm93IHdo
YXQgd2UgaGF2ZSB0aGUgdHJhY2UgZXZlbnQgZm9yIGFlciwgdGhlIGNvbnNvbGUgb3V0cHV0IG1p
Z2h0IGJlIGxlc3MgaW1wb3J0YW50LiAgVGhpcyBUT0RPIGlzIGEgbm90ZSBmb3IgZnV0dXJlIGNs
ZWFuLXVwIGFuZCBpcyBub3QgZGlyZWN0bHkgcmVsYXRlZCB0byB0aGUgYnVnIGJlaW5nIGZpeGVk
IHdpdGggdGhpcyBwYXRjaC4gIFdoaWNoIGxlbmRzIHRvIHRoZSBhcmd1bWVudCBvZiB3aHkgcHV0
IHRoZSBUT0RPIGluIHRoaXMgcGF0Y2g/ICBPcHBvcnR1bmlzdGljLiAgSSBkb27igJl0IHRoaW5r
IHdlIHdhbnQgdG8gY3JlYXRlIGEgc2VwYXJhdGUgcGF0Y2gganVzdCBmb3IgYSBUT0RPIG5vdGUu
ICANCg0KU28sIHdoeSBwdWxsIHRoaXMgcGF0Y2ggaW4gZXZlbiB0aG91Z2ggdGhlcmUgaXMgd29y
ayB0byBkbz8gIFRoZSBwYXRjaCBmaXhlcyBhIHdhcm5pbmcgdGhhdCBtaWdodCBjYXVzZSBjdXN0
b21lcnMgdW4tZHVlIGNvbmNlcm4gYW5kIHJlbW92ZXMgYSBjYWxsIGluIGludGVycnVwdCBjb250
ZXh0IHRoYXQgc2hvdWxkIG5vdCBiZSB0aGVyZS4gIA0KDQpMYW5jZQ0K
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Borislav Petkov May 30, 2013, 7:28 a.m. UTC | #4 
On Thu, May 30, 2013 at 04:55:20AM +0000, Ortiz, Lance E wrote:
> This TODO is a note for future clean-up and is not directly related to
>the bug being fixed with this patch. Which lends to the argument of why
>put the TODO in this patch? Opportunistic. I don’t think we want to
>create a separate patch just for a TODO note.

Sounds to me, this TODO item should be on your TODO list - not in kernel
sources :-)
Steven Rostedt May 30, 2013, 1:37 p.m. UTC | #5 
On Thu, 2013-05-30 at 09:28 +0200, Borislav Petkov wrote:
> On Thu, May 30, 2013 at 04:55:20AM +0000, Ortiz, Lance E wrote:
> > This TODO is a note for future clean-up and is not directly related to
> >the bug being fixed with this patch. Which lends to the argument of why
> >put the TODO in this patch? Opportunistic. I don’t think we want to
> >create a separate patch just for a TODO note.
> 
> Sounds to me, this TODO item should be on your TODO list - not in kernel
> sources :-)
> 

Also, that TODO sounds like there's output to userspace that can be
parsed by a userspace tool. If a tool expects the current format, it may
not be acceptable to change it later.

If the contents of this patch has nothing to do with the TODO, then
leave it out. It just confuses things.

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ortiz, Lance E May 30, 2013, 1:58 p.m. UTC | #6 
> On Thu, May 30, 2013 at 04:55:20AM +0000, Ortiz, Lance E wrote:

> > This TODO is a note for future clean-up and is not directly related

> to

> >the bug being fixed with this patch. Which lends to the argument of

> why

> >put the TODO in this patch? Opportunistic. I don’t think we want to

> >create a separate patch just for a TODO note.

> 

> Sounds to me, this TODO item should be on your TODO list - not in

> kernel

> sources :-)

> --

Makes sense.  I will yank the TODO and resubmit the patch.  

Lance
Ortiz, Lance E May 30, 2013, 2:04 p.m. UTC | #7 
PiA+DQo+ID4gU291bmRzIHRvIG1lLCB0aGlzIFRPRE8gaXRlbSBzaG91bGQgYmUgb24geW91ciBU
T0RPIGxpc3QgLSBub3QgaW4NCj4ga2VybmVsDQo+ID4gc291cmNlcyA6LSkNCj4gPg0KPiANCj4g
QWxzbywgdGhhdCBUT0RPIHNvdW5kcyBsaWtlIHRoZXJlJ3Mgb3V0cHV0IHRvIHVzZXJzcGFjZSB0
aGF0IGNhbiBiZQ0KPiBwYXJzZWQgYnkgYSB1c2Vyc3BhY2UgdG9vbC4gSWYgYSB0b29sIGV4cGVj
dHMgdGhlIGN1cnJlbnQgZm9ybWF0LCBpdA0KPiBtYXkNCj4gbm90IGJlIGFjY2VwdGFibGUgdG8g
Y2hhbmdlIGl0IGxhdGVyLg0KPiANCj4gSWYgdGhlIGNvbnRlbnRzIG9mIHRoaXMgcGF0Y2ggaGFz
IG5vdGhpbmcgdG8gZG8gd2l0aCB0aGUgVE9ETywgdGhlbg0KPiBsZWF2ZSBpdCBvdXQuIEl0IGp1
c3QgY29uZnVzZXMgdGhpbmdzLg0KDQpTdGV2ZSwgeW91IGRvIGhhdmUgYSBnb29kIHBvaW50IGhl
cmUuICBJIGFtIHdvbmRlcmluZyBpZiB0aGF0IGlzIHdoeSB3ZSBzaG91bGQgY29uc2lkZXIgY2hh
bmdpbmcgdGhlIG91dHB1dCB0byBtYXRjaCBhZXJfcHJpbnRfZXJyb3IoKS4gIFRoZSBjb2RlIHBh
dGggdG8gYWVyX3ByaW50X2Vycm9yKCkgaXMgdGhlIG1vcmUgY29tbW9uIHBhdGggd2hlcmUgbm90
IGFzIG1hbnkgcGxhdGZvcm1zIHN1cHBvcnQgdGhlIGNwZXJfcHJpbnRfZXJyb3IoKSBwYXRoIChm
aXJtd2FyZSBmaXJzdCBBRVIpLiAgU28gaXQgaXMgbW9yZSBsaWtlbHkgdGhhdCBhbnkgdG9vbHMg
d3JpdHRlbiB3b3VsZCBrbm93IGhvdyB0byBwYXJzZSB0aGUgb3V0cHV0IGZyb20gYWVyX3ByaW50
X2Vycm9yKCkuICBJdCB3b3VsZCBiZSBnb29kIGZvciB0aG9zZSB0b29scyB0byBzdXBwb3J0IGZp
cm13YXJlIGZpcnN0IEFFUiB3aGVuIGl0IGJlY29tZXMgbW9yZSBjb21tb24uICBPZiBjb3Vyc2Ug
dGhpcyBpcyBwdXJlbHkgY29uamVjdHVyZS4gIEkgaGF2ZSBubyBpZGVhIGlmIHRoZXJlIGFyZSBh
bnkgdG9vbHMgdGhhdCBwYXJzZSB0aGlzIHRleHQgb3V0cHV0Lg0KDQpMYW5jZQ0K
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Borislav Petkov May 31, 2013, 7:41 a.m. UTC | #8 
On Thu, May 30, 2013 at 02:04:42PM +0000, Ortiz, Lance E wrote:
> Steve, you do have a good point here. I am wondering if that is why
> we should consider changing the output to match aer_print_error().
> The code path to aer_print_error() is the more common path where not
> as many platforms support the cper_print_error() path (firmware first
> AER). So it is more likely that any tools written would know how to
> parse the output from aer_print_error(). It would be good for those
> tools to support firmware first AER when it becomes more common. Of
> course this is purely conjecture. I have no idea if there are any
> tools that parse this text output.

Whatever you end up doing, just make sure you've hammered out the
information going out to userspace to be clear, succinct and complete
(as far as possible, of course). Because once you cast it in stone and
tools start using it, changing its format is a huge PITA, if at all
possible.

And if the error formats are compatible, then sharing the format is
obviously advantageous.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
index 1e5d8a4..8713229 100644
--- a/drivers/acpi/apei/cper.c
+++ b/drivers/acpi/apei/cper.c
@@ -250,10 +250,6 @@  static const char *cper_pcie_port_type_strs[] = {
 static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
 			    const struct acpi_hest_generic_data *gdata)
 {
-#ifdef CONFIG_ACPI_APEI_PCIEAER
-	struct pci_dev *dev;
-#endif
-
 	if (pcie->validation_bits & CPER_PCIE_VALID_PORT_TYPE)
 		printk("%s""port_type: %d, %s\n", pfx, pcie->port_type,
 		       pcie->port_type < ARRAY_SIZE(cper_pcie_port_type_strs) ?
@@ -285,20 +281,6 @@  static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
 		printk(
 	"%s""bridge: secondary_status: 0x%04x, control: 0x%04x\n",
 	pfx, pcie->bridge.secondary_status, pcie->bridge.control);
-#ifdef CONFIG_ACPI_APEI_PCIEAER
-	dev = pci_get_domain_bus_and_slot(pcie->device_id.segment,
-			pcie->device_id.bus, pcie->device_id.function);
-	if (!dev) {
-		pr_err("PCI AER Cannot get PCI device %04x:%02x:%02x.%d\n",
-			pcie->device_id.segment, pcie->device_id.bus,
-			pcie->device_id.slot, pcie->device_id.function);
-		return;
-	}
-	if (pcie->validation_bits & CPER_PCIE_VALID_AER_INFO)
-		cper_print_aer(pfx, dev, gdata->error_severity,
-				(struct aer_capability_regs *) pcie->aer_info);
-	pci_dev_put(dev);
-#endif
 }
 
 static const char *apei_estatus_section_flag_strs[] = {
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index d668a8a..403baf4 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -454,7 +454,9 @@  static void ghes_do_proc(struct ghes *ghes,
 				aer_severity = cper_severity_to_aer(sev);
 				aer_recover_queue(pcie_err->device_id.segment,
 						  pcie_err->device_id.bus,
-						  devfn, aer_severity);
+						  devfn, aer_severity,
+						  (struct aer_capability_regs *)
+						  pcie_err->aer_info);
 			}
 
 		}
diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c
index 564d97f..120549a 100644
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -582,6 +582,7 @@  struct aer_recover_entry
 	u8	devfn;
 	u16	domain;
 	int	severity;
+	struct aer_capability_regs *regs;
 };
 
 static DEFINE_KFIFO(aer_recover_ring, struct aer_recover_entry,
@@ -595,7 +596,7 @@  static DEFINE_SPINLOCK(aer_recover_ring_lock);
 static DECLARE_WORK(aer_recover_work, aer_recover_work_func);
 
 void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
-		       int severity)
+		       int severity, struct aer_capability_regs *aer_regs)
 {
 	unsigned long flags;
 	struct aer_recover_entry entry = {
@@ -603,6 +604,7 @@  void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
 		.devfn		= devfn,
 		.domain		= domain,
 		.severity	= severity,
+		.regs		= aer_regs,
 	};
 
 	spin_lock_irqsave(&aer_recover_ring_lock, flags);
@@ -629,6 +631,7 @@  static void aer_recover_work_func(struct work_struct *work)
 			       PCI_SLOT(entry.devfn), PCI_FUNC(entry.devfn));
 			continue;
 		}
+		cper_print_aer(pdev, entry.severity, entry.regs);
 		do_recovery(pdev, entry.severity);
 		pci_dev_put(pdev);
 	}
diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
index 5ab1425..f277dc3 100644
--- a/drivers/pci/pcie/aer/aerdrv_errprint.c
+++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
@@ -220,13 +220,19 @@  int cper_severity_to_aer(int cper_severity)
 }
 EXPORT_SYMBOL_GPL(cper_severity_to_aer);
 
-void cper_print_aer(const char *prefix, struct pci_dev *dev, int cper_severity,
+void cper_print_aer(struct pci_dev *dev, int cper_severity,
 		    struct aer_capability_regs *aer)
 {
 	int aer_severity, layer, agent, status_strs_size, tlp_header_valid = 0;
 	u32 status, mask;
 	const char **status_strs;
 
+	/*
+	 * TODO: This function needs to be re-written so that it's output
+	 * matches the output of aer_print_error().  Right now, the output
+	 * is formatted very differently.
+	 */
+
 	aer_severity = cper_severity_to_aer(cper_severity);
 	if (aer_severity == AER_CORRECTABLE) {
 		status = aer->cor_status;
@@ -244,7 +250,7 @@  void cper_print_aer(const char *prefix, struct pci_dev *dev, int cper_severity,
 	agent = AER_GET_AGENT(aer_severity, status);
 	dev_err(&dev->dev, "aer_status: 0x%08x, aer_mask: 0x%08x\n",
 	       status, mask);
-	cper_print_bits(prefix, status, status_strs, status_strs_size);
+	cper_print_bits("", status, status_strs, status_strs_size);
 	dev_err(&dev->dev, "aer_layer=%s, aer_agent=%s\n",
 	       aer_error_layer[layer], aer_agent_string[agent]);
 	if (aer_severity != AER_CORRECTABLE)
diff --git a/include/linux/aer.h b/include/linux/aer.h
index ec10e1b..737f90a 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -49,10 +49,11 @@  static inline int pci_cleanup_aer_uncorrect_error_status(struct pci_dev *dev)
 }
 #endif
 
-extern void cper_print_aer(const char *prefix, struct pci_dev *dev,
+extern void cper_print_aer(struct pci_dev *dev,
 			   int cper_severity, struct aer_capability_regs *aer);
 extern int cper_severity_to_aer(int cper_severity);
 extern void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
-			      int severity);
+			      int severity,
+			      struct aer_capability_regs *aer_regs);
 #endif //_AER_H_