| Message ID | 20130529190326.24171.86905.stgit@grignak.americas.hpqcorp.net (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
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
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
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
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 :-)
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
> 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
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
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 --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_
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