| Message ID | 4A8E1921.5090500@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Fri, 2009-08-21 at 12:48 +0900, Hidetoshi Seto wrote: > Multiple bits might be set in the Uncorrectable Error Status > register. But aer_print_error_source() only report a error of > the lowest bit set in the error status register. > > So it is better to print strings for all bits unmasked and set. > > And check First Error Pointer to mark the error occurred first. > This FEP is not valid when the corresponding bit of the Uncorrectable > Error Status register is not set, or unimplemented or undefined. > > Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com> > --- > drivers/pci/pcie/aer/aerdrv.h | 1 + > drivers/pci/pcie/aer/aerdrv_core.c | 4 ++++ > drivers/pci/pcie/aer/aerdrv_errprint.c | 8 ++++---- > 3 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/pcie/aer/aerdrv.h b/drivers/pci/pcie/aer/aerdrv.h > index 0db530d..436bf79 100644 > --- a/drivers/pci/pcie/aer/aerdrv.h > +++ b/drivers/pci/pcie/aer/aerdrv.h > @@ -61,6 +61,7 @@ struct aer_err_info { > u16 id; > int severity; /* 0:NONFATAL | 1:FATAL | 2:COR */ > int flags; > + int first; > unsigned int status; /* COR/UNCOR Error Status */ > unsigned int mask; /* COR/UNCOR Error Mask */ > struct header_log_regs tlp; /* TLP Header */ > diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c > index 38b3933..995644e 100644 > --- a/drivers/pci/pcie/aer/aerdrv_core.c > +++ b/drivers/pci/pcie/aer/aerdrv_core.c > @@ -727,6 +727,10 @@ static int get_device_error_info(struct pci_dev *dev, struct aer_err_info *info) > if (!(info->status & ~info->mask)) > return AER_UNSUCCESS; > > + /* Get First Error Pointer */ > + pci_read_config_dword(dev, pos + PCI_ERR_CAP, &info->first); Perhaps use a temporary variable here? > + info->first = PCI_ERR_CAP_FEP(info->first); > + > if (info->status & AER_LOG_TLP_MASKS) { > info->flags |= AER_TLP_HEADER_VALID_FLAG; > pci_read_config_dword(dev, > diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c > index 919c2c0..b6303fc 100644 > --- a/drivers/pci/pcie/aer/aerdrv_errprint.c > +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c > @@ -169,11 +169,11 @@ static void aer_print_error_source(struct aer_err_info *info) > errmsg = aer_uncorrectable_error_string[i]; > > if (errmsg) > - AER_PR(info, "%s\t:\n", errmsg); > + AER_PR(info, "%s\t: %s\n", errmsg, > + info->first == i ? "First" : ""); > else > - AER_PR(info, "Unknown Error Bit %2d \t:\n", i); > - > - break; > + AER_PR(info, "Unknown Error Bit %2d \t: %s\n", > + i, info->first == i ? "First" : ""); > } > } > Reviewed-by: Andrew Patterson <andrew.patterson@hp.com>
Andrew Patterson wrote: > > Reviewed-by: Andrew Patterson <andrew.patterson@hp.com> Thank you very much for your review, and sorry to my late reply. I'll post v2 reflecting your comment soon. (with your Reviewed-by to some of them unchanged) Thanks, H.Seto -- 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/pci/pcie/aer/aerdrv.h b/drivers/pci/pcie/aer/aerdrv.h index 0db530d..436bf79 100644 --- a/drivers/pci/pcie/aer/aerdrv.h +++ b/drivers/pci/pcie/aer/aerdrv.h @@ -61,6 +61,7 @@ struct aer_err_info { u16 id; int severity; /* 0:NONFATAL | 1:FATAL | 2:COR */ int flags; + int first; unsigned int status; /* COR/UNCOR Error Status */ unsigned int mask; /* COR/UNCOR Error Mask */ struct header_log_regs tlp; /* TLP Header */ diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c index 38b3933..995644e 100644 --- a/drivers/pci/pcie/aer/aerdrv_core.c +++ b/drivers/pci/pcie/aer/aerdrv_core.c @@ -727,6 +727,10 @@ static int get_device_error_info(struct pci_dev *dev, struct aer_err_info *info) if (!(info->status & ~info->mask)) return AER_UNSUCCESS; + /* Get First Error Pointer */ + pci_read_config_dword(dev, pos + PCI_ERR_CAP, &info->first); + info->first = PCI_ERR_CAP_FEP(info->first); + if (info->status & AER_LOG_TLP_MASKS) { info->flags |= AER_TLP_HEADER_VALID_FLAG; pci_read_config_dword(dev, diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c index 919c2c0..b6303fc 100644 --- a/drivers/pci/pcie/aer/aerdrv_errprint.c +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c @@ -169,11 +169,11 @@ static void aer_print_error_source(struct aer_err_info *info) errmsg = aer_uncorrectable_error_string[i]; if (errmsg) - AER_PR(info, "%s\t:\n", errmsg); + AER_PR(info, "%s\t: %s\n", errmsg, + info->first == i ? "First" : ""); else - AER_PR(info, "Unknown Error Bit %2d \t:\n", i); - - break; + AER_PR(info, "Unknown Error Bit %2d \t: %s\n", + i, info->first == i ? "First" : ""); } }
Multiple bits might be set in the Uncorrectable Error Status register. But aer_print_error_source() only report a error of the lowest bit set in the error status register. So it is better to print strings for all bits unmasked and set. And check First Error Pointer to mark the error occurred first. This FEP is not valid when the corresponding bit of the Uncorrectable Error Status register is not set, or unimplemented or undefined. Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com> --- drivers/pci/pcie/aer/aerdrv.h | 1 + drivers/pci/pcie/aer/aerdrv_core.c | 4 ++++ drivers/pci/pcie/aer/aerdrv_errprint.c | 8 ++++---- 3 files changed, 9 insertions(+), 4 deletions(-)