| Message ID | 1392983613-10090-4-git-send-email-shangw@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, 2014-02-21 at 19:53 +0800, Gavin Shan wrote: > EEH core tries to recover from fenced PHB or frozen PE. Unfortunately, > the log isn't consistent with the site because enabling IO path for > the frozen PE before collecting log would ruin the site. The patch > solves the problem to cache the PHB diag-data in advance with the > help of additional flag PNV_PHB_FLAG_DIAG to pnv_phb::flags. Ok, so correct me if I'm wrong, but you are - Collecting the diag data in get_state, as a sort of side effect (what happens if get_state is called multiple times ?) - Dumping it later on Any reason why we can't instead dump it immediately ? Also do we have a clean trigger for when we detect an error condition ? It can either be the result of an interrupt or a driver called get_state following an ffffffff. Are both path eventually going into the same function to handle a "new" error condition ? That's where I would both collect and dump the EEH state.. Cheers, Ben. > Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com> > --- > arch/powerpc/platforms/powernv/eeh-ioda.c | 65 ++++++++++++++++++----------- > arch/powerpc/platforms/powernv/pci.c | 21 ++++++---- > arch/powerpc/platforms/powernv/pci.h | 1 + > 3 files changed, 55 insertions(+), 32 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c > index 04b4710..3ed8d22 100644 > --- a/arch/powerpc/platforms/powernv/eeh-ioda.c > +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c > @@ -114,6 +114,27 @@ DEFINE_SIMPLE_ATTRIBUTE(ioda_eeh_inbB_dbgfs_ops, ioda_eeh_inbB_dbgfs_get, > ioda_eeh_inbB_dbgfs_set, "0x%llx\n"); > #endif /* CONFIG_DEBUG_FS */ > > +static void ioda_eeh_phb_diag(struct pci_controller *hose) > +{ > + struct pnv_phb *phb = hose->private_data; > + unsigned long flags; > + long rc; > + > + spin_lock_irqsave(&phb->lock, flags); > + > + rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob, > + PNV_PCI_DIAG_BUF_SIZE); > + if (rc == OPAL_SUCCESS) { > + phb->flags |= PNV_PHB_FLAG_DIAG; > + } else { > + pr_warn("%s: Can't get diag-data for PHB#%x (%ld)\n", > + __func__, hose->global_number, rc); > + phb->flags &= ~PNV_PHB_FLAG_DIAG; > + } > + > + spin_unlock_irqrestore(&phb->lock, flags); > +} > + > /** > * ioda_eeh_post_init - Chip dependent post initialization > * @hose: PCI controller > @@ -272,6 +293,8 @@ static int ioda_eeh_get_state(struct eeh_pe *pe) > result |= EEH_STATE_DMA_ACTIVE; > result |= EEH_STATE_MMIO_ENABLED; > result |= EEH_STATE_DMA_ENABLED; > + } else { > + ioda_eeh_phb_diag(hose); > } > > return result; > @@ -541,24 +564,13 @@ static int ioda_eeh_reset(struct eeh_pe *pe, int option) > static int ioda_eeh_get_log(struct eeh_pe *pe, int severity, > char *drv_log, unsigned long len) > { > - s64 ret; > + struct pnv_phb *phb = pe->phb->private_data; > unsigned long flags; > - struct pci_controller *hose = pe->phb; > - struct pnv_phb *phb = hose->private_data; > > spin_lock_irqsave(&phb->lock, flags); > > - ret = opal_pci_get_phb_diag_data2(phb->opal_id, > - phb->diag.blob, PNV_PCI_DIAG_BUF_SIZE); > - if (ret) { > - spin_unlock_irqrestore(&phb->lock, flags); > - pr_warning("%s: Can't get log for PHB#%x-PE#%x (%lld)\n", > - __func__, hose->global_number, pe->addr, ret); > - return -EIO; > - } > - > - /* The PHB diag-data is always indicative */ > - pnv_pci_dump_phb_diag_data(hose, phb->diag.blob); > + pnv_pci_dump_phb_diag_data(pe->phb, phb->diag.blob); > + phb->flags &= ~PNV_PHB_FLAG_DIAG; > > spin_unlock_irqrestore(&phb->lock, flags); > > @@ -646,19 +658,11 @@ static void ioda_eeh_hub_diag(struct pci_controller *hose) > } > } > > -static void ioda_eeh_phb_diag(struct pci_controller *hose) > +static void ioda_eeh_phb_diag_dump(struct pci_controller *hose) > { > struct pnv_phb *phb = hose->private_data; > - long rc; > - > - rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob, > - PNV_PCI_DIAG_BUF_SIZE); > - if (rc != OPAL_SUCCESS) { > - pr_warning("%s: Failed to get diag-data for PHB#%x (%ld)\n", > - __func__, hose->global_number, rc); > - return; > - } > > + ioda_eeh_phb_diag(hose); > pnv_pci_dump_phb_diag_data(hose, phb->diag.blob); > } > > @@ -778,7 +782,7 @@ static int ioda_eeh_next_error(struct eeh_pe **pe) > pr_info("EEH: PHB#%x informative error " > "detected\n", > hose->global_number); > - ioda_eeh_phb_diag(hose); > + ioda_eeh_phb_diag_dump(hose); > ret = EEH_NEXT_ERR_NONE; > } > > @@ -809,6 +813,17 @@ static int ioda_eeh_next_error(struct eeh_pe **pe) > } > > /* > + * EEH core will try recover from fenced PHB or > + * frozen PE. In the time for frozen PE, EEH core > + * enable IO path for that before collecting logs, > + * but it ruins the site. So we have to cache the > + * log in advance here. > + */ > + if (ret == EEH_NEXT_ERR_FROZEN_PE || > + ret == EEH_NEXT_ERR_FENCED_PHB) > + ioda_eeh_phb_diag(hose); > + > + /* > * If we have no errors on the specific PHB or only > * informative error there, we continue poking it. > * Otherwise, we need actions to be taken by upper > diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c > index 437c37d..67b2254 100644 > --- a/arch/powerpc/platforms/powernv/pci.c > +++ b/arch/powerpc/platforms/powernv/pci.c > @@ -259,11 +259,15 @@ static void pnv_pci_dump_phb3_diag_data(struct pci_controller *hose, > void pnv_pci_dump_phb_diag_data(struct pci_controller *hose, > unsigned char *log_buff) > { > + struct pnv_phb *phb = hose->private_data; > struct OpalIoPhbErrorCommon *common; > > if (!hose || !log_buff) > return; > > + if (!(phb->flags & PNV_PHB_FLAG_DIAG)) > + return; > + > common = (struct OpalIoPhbErrorCommon *)log_buff; > switch (common->ioType) { > case OPAL_PHB_ERROR_DATA_TYPE_P7IOC: > @@ -281,13 +285,19 @@ void pnv_pci_dump_phb_diag_data(struct pci_controller *hose, > static void pnv_pci_handle_eeh_config(struct pnv_phb *phb, u32 pe_no) > { > unsigned long flags, rc; > - int has_diag; > + bool has_diag = false; > > spin_lock_irqsave(&phb->lock, flags); > > - rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob, > - PNV_PCI_DIAG_BUF_SIZE); > - has_diag = (rc == OPAL_SUCCESS); > + if (!(phb->flags & PNV_PHB_FLAG_DIAG)) { > + rc = opal_pci_get_phb_diag_data2(phb->opal_id, > + phb->diag.blob, > + PNV_PCI_DIAG_BUF_SIZE); > + if (rc == OPAL_SUCCESS) { > + phb->flags |= PNV_PHB_FLAG_DIAG; > + has_diag = true; > + } > + } > > rc = opal_pci_eeh_freeze_clear(phb->opal_id, pe_no, > OPAL_EEH_ACTION_CLEAR_FREEZE_ALL); > @@ -303,9 +313,6 @@ static void pnv_pci_handle_eeh_config(struct pnv_phb *phb, u32 pe_no) > */ > if (has_diag) > pnv_pci_dump_phb_diag_data(phb->hose, phb->diag.blob); > - else > - pr_warning("PCI %d: No diag data available\n", > - phb->hose->global_number); > } > > spin_unlock_irqrestore(&phb->lock, flags); > diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h > index adeb3c4..153af9a 100644 > --- a/arch/powerpc/platforms/powernv/pci.h > +++ b/arch/powerpc/platforms/powernv/pci.h > @@ -82,6 +82,7 @@ struct pnv_eeh_ops { > #endif /* CONFIG_EEH */ > > #define PNV_PHB_FLAG_EEH (1 << 0) > +#define PNV_PHB_FLAG_DIAG (1 << 1) > > struct pnv_phb { > struct pci_controller *hose;
On Sat, Feb 22, 2014 at 07:01:27AM +1100, Benjamin Herrenschmidt wrote: >On Fri, 2014-02-21 at 19:53 +0800, Gavin Shan wrote: >> EEH core tries to recover from fenced PHB or frozen PE. Unfortunately, >> the log isn't consistent with the site because enabling IO path for >> the frozen PE before collecting log would ruin the site. The patch >> solves the problem to cache the PHB diag-data in advance with the >> help of additional flag PNV_PHB_FLAG_DIAG to pnv_phb::flags. > >Ok, so correct me if I'm wrong, but you are > > - Collecting the diag data in get_state, as a sort of side effect >(what happens if get_state is called multiple times ?) > > - Dumping it later on > Yeah, the patch would have some problems when get_state gets called for multiple times: the log could be much more than what we expected in case that frozen PE#1 detected and in progress to handle it (we don't dump it yet), frozen PE#2 detected. The log would include all information for frozen PE#1 and PE#2 and it's not expected. Another case is get_state called for multiple times on frozen PE#1 and we can check EEH_PE_ISOLATED to avoid diag-data over-writting. I'm thinking of a new mechanism (please refer to the reply below). >Any reason why we can't instead dump it immediately ? Also do we have a >clean trigger for when we detect an error condition ? It can either be >the result of an interrupt or a driver called get_state following an >ffffffff. Are both path eventually going into the same function to >handle a "new" error condition ? That's where I would both collect and >dump the EEH state.. > The reason I don't want dump it immediately is that I would keep struct pnv_eeh_ops::get_log() to dump diag-data to guest in the future. The problem is that we have only one PHB diag-data instance in struct pnv_phb::diag.blob[]. I'm thinking of to have each PE to have diag-data for itself and the things would look like followings. Ben, please comment :-) - Extend "struct eeh_pe" to have a platform pointer (void *data). And we still collect diag-data in get_state() or next_error(), which will be dumped in pnv_eeh_ops->get_log(). The disadvantage could be lots of memory (extra 8KB usually) consumed by each PE instance. - For PCI config accessors and informative (also dead PHB, dead P7IOC), we still use pnv_phb::diag.blob[]. Thanks, Gavin >> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com> >> --- >> arch/powerpc/platforms/powernv/eeh-ioda.c | 65 ++++++++++++++++++----------- >> arch/powerpc/platforms/powernv/pci.c | 21 ++++++---- >> arch/powerpc/platforms/powernv/pci.h | 1 + >> 3 files changed, 55 insertions(+), 32 deletions(-) >> >> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c >> index 04b4710..3ed8d22 100644 >> --- a/arch/powerpc/platforms/powernv/eeh-ioda.c >> +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c >> @@ -114,6 +114,27 @@ DEFINE_SIMPLE_ATTRIBUTE(ioda_eeh_inbB_dbgfs_ops, ioda_eeh_inbB_dbgfs_get, >> ioda_eeh_inbB_dbgfs_set, "0x%llx\n"); >> #endif /* CONFIG_DEBUG_FS */ >> >> +static void ioda_eeh_phb_diag(struct pci_controller *hose) >> +{ >> + struct pnv_phb *phb = hose->private_data; >> + unsigned long flags; >> + long rc; >> + >> + spin_lock_irqsave(&phb->lock, flags); >> + >> + rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob, >> + PNV_PCI_DIAG_BUF_SIZE); >> + if (rc == OPAL_SUCCESS) { >> + phb->flags |= PNV_PHB_FLAG_DIAG; >> + } else { >> + pr_warn("%s: Can't get diag-data for PHB#%x (%ld)\n", >> + __func__, hose->global_number, rc); >> + phb->flags &= ~PNV_PHB_FLAG_DIAG; >> + } >> + >> + spin_unlock_irqrestore(&phb->lock, flags); >> +} >> + >> /** >> * ioda_eeh_post_init - Chip dependent post initialization >> * @hose: PCI controller >> @@ -272,6 +293,8 @@ static int ioda_eeh_get_state(struct eeh_pe *pe) >> result |= EEH_STATE_DMA_ACTIVE; >> result |= EEH_STATE_MMIO_ENABLED; >> result |= EEH_STATE_DMA_ENABLED; >> + } else { >> + ioda_eeh_phb_diag(hose); >> } >> >> return result; >> @@ -541,24 +564,13 @@ static int ioda_eeh_reset(struct eeh_pe *pe, int option) >> static int ioda_eeh_get_log(struct eeh_pe *pe, int severity, >> char *drv_log, unsigned long len) >> { >> - s64 ret; >> + struct pnv_phb *phb = pe->phb->private_data; >> unsigned long flags; >> - struct pci_controller *hose = pe->phb; >> - struct pnv_phb *phb = hose->private_data; >> >> spin_lock_irqsave(&phb->lock, flags); >> >> - ret = opal_pci_get_phb_diag_data2(phb->opal_id, >> - phb->diag.blob, PNV_PCI_DIAG_BUF_SIZE); >> - if (ret) { >> - spin_unlock_irqrestore(&phb->lock, flags); >> - pr_warning("%s: Can't get log for PHB#%x-PE#%x (%lld)\n", >> - __func__, hose->global_number, pe->addr, ret); >> - return -EIO; >> - } >> - >> - /* The PHB diag-data is always indicative */ >> - pnv_pci_dump_phb_diag_data(hose, phb->diag.blob); >> + pnv_pci_dump_phb_diag_data(pe->phb, phb->diag.blob); >> + phb->flags &= ~PNV_PHB_FLAG_DIAG; >> >> spin_unlock_irqrestore(&phb->lock, flags); >> >> @@ -646,19 +658,11 @@ static void ioda_eeh_hub_diag(struct pci_controller *hose) >> } >> } >> >> -static void ioda_eeh_phb_diag(struct pci_controller *hose) >> +static void ioda_eeh_phb_diag_dump(struct pci_controller *hose) >> { >> struct pnv_phb *phb = hose->private_data; >> - long rc; >> - >> - rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob, >> - PNV_PCI_DIAG_BUF_SIZE); >> - if (rc != OPAL_SUCCESS) { >> - pr_warning("%s: Failed to get diag-data for PHB#%x (%ld)\n", >> - __func__, hose->global_number, rc); >> - return; >> - } >> >> + ioda_eeh_phb_diag(hose); >> pnv_pci_dump_phb_diag_data(hose, phb->diag.blob); >> } >> >> @@ -778,7 +782,7 @@ static int ioda_eeh_next_error(struct eeh_pe **pe) >> pr_info("EEH: PHB#%x informative error " >> "detected\n", >> hose->global_number); >> - ioda_eeh_phb_diag(hose); >> + ioda_eeh_phb_diag_dump(hose); >> ret = EEH_NEXT_ERR_NONE; >> } >> >> @@ -809,6 +813,17 @@ static int ioda_eeh_next_error(struct eeh_pe **pe) >> } >> >> /* >> + * EEH core will try recover from fenced PHB or >> + * frozen PE. In the time for frozen PE, EEH core >> + * enable IO path for that before collecting logs, >> + * but it ruins the site. So we have to cache the >> + * log in advance here. >> + */ >> + if (ret == EEH_NEXT_ERR_FROZEN_PE || >> + ret == EEH_NEXT_ERR_FENCED_PHB) >> + ioda_eeh_phb_diag(hose); >> + >> + /* >> * If we have no errors on the specific PHB or only >> * informative error there, we continue poking it. >> * Otherwise, we need actions to be taken by upper >> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c >> index 437c37d..67b2254 100644 >> --- a/arch/powerpc/platforms/powernv/pci.c >> +++ b/arch/powerpc/platforms/powernv/pci.c >> @@ -259,11 +259,15 @@ static void pnv_pci_dump_phb3_diag_data(struct pci_controller *hose, >> void pnv_pci_dump_phb_diag_data(struct pci_controller *hose, >> unsigned char *log_buff) >> { >> + struct pnv_phb *phb = hose->private_data; >> struct OpalIoPhbErrorCommon *common; >> >> if (!hose || !log_buff) >> return; >> >> + if (!(phb->flags & PNV_PHB_FLAG_DIAG)) >> + return; >> + >> common = (struct OpalIoPhbErrorCommon *)log_buff; >> switch (common->ioType) { >> case OPAL_PHB_ERROR_DATA_TYPE_P7IOC: >> @@ -281,13 +285,19 @@ void pnv_pci_dump_phb_diag_data(struct pci_controller *hose, >> static void pnv_pci_handle_eeh_config(struct pnv_phb *phb, u32 pe_no) >> { >> unsigned long flags, rc; >> - int has_diag; >> + bool has_diag = false; >> >> spin_lock_irqsave(&phb->lock, flags); >> >> - rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob, >> - PNV_PCI_DIAG_BUF_SIZE); >> - has_diag = (rc == OPAL_SUCCESS); >> + if (!(phb->flags & PNV_PHB_FLAG_DIAG)) { >> + rc = opal_pci_get_phb_diag_data2(phb->opal_id, >> + phb->diag.blob, >> + PNV_PCI_DIAG_BUF_SIZE); >> + if (rc == OPAL_SUCCESS) { >> + phb->flags |= PNV_PHB_FLAG_DIAG; >> + has_diag = true; >> + } >> + } >> >> rc = opal_pci_eeh_freeze_clear(phb->opal_id, pe_no, >> OPAL_EEH_ACTION_CLEAR_FREEZE_ALL); >> @@ -303,9 +313,6 @@ static void pnv_pci_handle_eeh_config(struct pnv_phb *phb, u32 pe_no) >> */ >> if (has_diag) >> pnv_pci_dump_phb_diag_data(phb->hose, phb->diag.blob); >> - else >> - pr_warning("PCI %d: No diag data available\n", >> - phb->hose->global_number); >> } >> >> spin_unlock_irqrestore(&phb->lock, flags); >> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h >> index adeb3c4..153af9a 100644 >> --- a/arch/powerpc/platforms/powernv/pci.h >> +++ b/arch/powerpc/platforms/powernv/pci.h >> @@ -82,6 +82,7 @@ struct pnv_eeh_ops { >> #endif /* CONFIG_EEH */ >> >> #define PNV_PHB_FLAG_EEH (1 << 0) >> +#define PNV_PHB_FLAG_DIAG (1 << 1) >> >> struct pnv_phb { >> struct pci_controller *hose; > >
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c index 04b4710..3ed8d22 100644 --- a/arch/powerpc/platforms/powernv/eeh-ioda.c +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c @@ -114,6 +114,27 @@ DEFINE_SIMPLE_ATTRIBUTE(ioda_eeh_inbB_dbgfs_ops, ioda_eeh_inbB_dbgfs_get, ioda_eeh_inbB_dbgfs_set, "0x%llx\n"); #endif /* CONFIG_DEBUG_FS */ +static void ioda_eeh_phb_diag(struct pci_controller *hose) +{ + struct pnv_phb *phb = hose->private_data; + unsigned long flags; + long rc; + + spin_lock_irqsave(&phb->lock, flags); + + rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob, + PNV_PCI_DIAG_BUF_SIZE); + if (rc == OPAL_SUCCESS) { + phb->flags |= PNV_PHB_FLAG_DIAG; + } else { + pr_warn("%s: Can't get diag-data for PHB#%x (%ld)\n", + __func__, hose->global_number, rc); + phb->flags &= ~PNV_PHB_FLAG_DIAG; + } + + spin_unlock_irqrestore(&phb->lock, flags); +} + /** * ioda_eeh_post_init - Chip dependent post initialization * @hose: PCI controller @@ -272,6 +293,8 @@ static int ioda_eeh_get_state(struct eeh_pe *pe) result |= EEH_STATE_DMA_ACTIVE; result |= EEH_STATE_MMIO_ENABLED; result |= EEH_STATE_DMA_ENABLED; + } else { + ioda_eeh_phb_diag(hose); } return result; @@ -541,24 +564,13 @@ static int ioda_eeh_reset(struct eeh_pe *pe, int option) static int ioda_eeh_get_log(struct eeh_pe *pe, int severity, char *drv_log, unsigned long len) { - s64 ret; + struct pnv_phb *phb = pe->phb->private_data; unsigned long flags; - struct pci_controller *hose = pe->phb; - struct pnv_phb *phb = hose->private_data; spin_lock_irqsave(&phb->lock, flags); - ret = opal_pci_get_phb_diag_data2(phb->opal_id, - phb->diag.blob, PNV_PCI_DIAG_BUF_SIZE); - if (ret) { - spin_unlock_irqrestore(&phb->lock, flags); - pr_warning("%s: Can't get log for PHB#%x-PE#%x (%lld)\n", - __func__, hose->global_number, pe->addr, ret); - return -EIO; - } - - /* The PHB diag-data is always indicative */ - pnv_pci_dump_phb_diag_data(hose, phb->diag.blob); + pnv_pci_dump_phb_diag_data(pe->phb, phb->diag.blob); + phb->flags &= ~PNV_PHB_FLAG_DIAG; spin_unlock_irqrestore(&phb->lock, flags); @@ -646,19 +658,11 @@ static void ioda_eeh_hub_diag(struct pci_controller *hose) } } -static void ioda_eeh_phb_diag(struct pci_controller *hose) +static void ioda_eeh_phb_diag_dump(struct pci_controller *hose) { struct pnv_phb *phb = hose->private_data; - long rc; - - rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob, - PNV_PCI_DIAG_BUF_SIZE); - if (rc != OPAL_SUCCESS) { - pr_warning("%s: Failed to get diag-data for PHB#%x (%ld)\n", - __func__, hose->global_number, rc); - return; - } + ioda_eeh_phb_diag(hose); pnv_pci_dump_phb_diag_data(hose, phb->diag.blob); } @@ -778,7 +782,7 @@ static int ioda_eeh_next_error(struct eeh_pe **pe) pr_info("EEH: PHB#%x informative error " "detected\n", hose->global_number); - ioda_eeh_phb_diag(hose); + ioda_eeh_phb_diag_dump(hose); ret = EEH_NEXT_ERR_NONE; } @@ -809,6 +813,17 @@ static int ioda_eeh_next_error(struct eeh_pe **pe) } /* + * EEH core will try recover from fenced PHB or + * frozen PE. In the time for frozen PE, EEH core + * enable IO path for that before collecting logs, + * but it ruins the site. So we have to cache the + * log in advance here. + */ + if (ret == EEH_NEXT_ERR_FROZEN_PE || + ret == EEH_NEXT_ERR_FENCED_PHB) + ioda_eeh_phb_diag(hose); + + /* * If we have no errors on the specific PHB or only * informative error there, we continue poking it. * Otherwise, we need actions to be taken by upper diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c index 437c37d..67b2254 100644 --- a/arch/powerpc/platforms/powernv/pci.c +++ b/arch/powerpc/platforms/powernv/pci.c @@ -259,11 +259,15 @@ static void pnv_pci_dump_phb3_diag_data(struct pci_controller *hose, void pnv_pci_dump_phb_diag_data(struct pci_controller *hose, unsigned char *log_buff) { + struct pnv_phb *phb = hose->private_data; struct OpalIoPhbErrorCommon *common; if (!hose || !log_buff) return; + if (!(phb->flags & PNV_PHB_FLAG_DIAG)) + return; + common = (struct OpalIoPhbErrorCommon *)log_buff; switch (common->ioType) { case OPAL_PHB_ERROR_DATA_TYPE_P7IOC: @@ -281,13 +285,19 @@ void pnv_pci_dump_phb_diag_data(struct pci_controller *hose, static void pnv_pci_handle_eeh_config(struct pnv_phb *phb, u32 pe_no) { unsigned long flags, rc; - int has_diag; + bool has_diag = false; spin_lock_irqsave(&phb->lock, flags); - rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob, - PNV_PCI_DIAG_BUF_SIZE); - has_diag = (rc == OPAL_SUCCESS); + if (!(phb->flags & PNV_PHB_FLAG_DIAG)) { + rc = opal_pci_get_phb_diag_data2(phb->opal_id, + phb->diag.blob, + PNV_PCI_DIAG_BUF_SIZE); + if (rc == OPAL_SUCCESS) { + phb->flags |= PNV_PHB_FLAG_DIAG; + has_diag = true; + } + } rc = opal_pci_eeh_freeze_clear(phb->opal_id, pe_no, OPAL_EEH_ACTION_CLEAR_FREEZE_ALL); @@ -303,9 +313,6 @@ static void pnv_pci_handle_eeh_config(struct pnv_phb *phb, u32 pe_no) */ if (has_diag) pnv_pci_dump_phb_diag_data(phb->hose, phb->diag.blob); - else - pr_warning("PCI %d: No diag data available\n", - phb->hose->global_number); } spin_unlock_irqrestore(&phb->lock, flags); diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h index adeb3c4..153af9a 100644 --- a/arch/powerpc/platforms/powernv/pci.h +++ b/arch/powerpc/platforms/powernv/pci.h @@ -82,6 +82,7 @@ struct pnv_eeh_ops { #endif /* CONFIG_EEH */ #define PNV_PHB_FLAG_EEH (1 << 0) +#define PNV_PHB_FLAG_DIAG (1 << 1) struct pnv_phb { struct pci_controller *hose;
EEH core tries to recover from fenced PHB or frozen PE. Unfortunately, the log isn't consistent with the site because enabling IO path for the frozen PE before collecting log would ruin the site. The patch solves the problem to cache the PHB diag-data in advance with the help of additional flag PNV_PHB_FLAG_DIAG to pnv_phb::flags. Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com> --- arch/powerpc/platforms/powernv/eeh-ioda.c | 65 ++++++++++++++++++----------- arch/powerpc/platforms/powernv/pci.c | 21 ++++++---- arch/powerpc/platforms/powernv/pci.h | 1 + 3 files changed, 55 insertions(+), 32 deletions(-)