| Message ID | 20131219231407.GA22418@oc3347516403.ibm.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Thu, Dec 19, 2013 at 05:14:07PM -0600, Brian W Hart wrote: >PHB diagnostic buffer may be smaller than PAGE_SIZE, especially when >PAGE_SIZE > 4KB. > I think you're talking about that PAGE_SIZE could be configured to have variable size (e.g. 4KB). So it's not safe to pass PAGE_SIZE to OPAL API opal_pci_get_phb_diag_data2(). Instead, we should pass PNV_PCI_DIAG_BUF_SIZE and it makes sense to me :-) Also, it needs to be backported to stable kernel as well. >Signed-off-by: Brian W Hart <hartb@linux.vnet.ibm.com> Acked-by: Gavin Shan <shangw@linux.vnet.ibm.com> >--- > arch/powerpc/platforms/powernv/eeh-ioda.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > >diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c >index 02245ce..8184ef5 100644 >--- a/arch/powerpc/platforms/powernv/eeh-ioda.c >+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c >@@ -820,14 +820,15 @@ static void ioda_eeh_phb_diag(struct pci_controller *hose) > struct OpalIoPhbErrorCommon *common; > long rc; > >- common = (struct OpalIoPhbErrorCommon *)phb->diag.blob; >- rc = opal_pci_get_phb_diag_data2(phb->opal_id, common, PAGE_SIZE); >+ 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; > } > >+ common = (struct OpalIoPhbErrorCommon *)phb->diag.blob; > switch (common->ioType) { > case OPAL_PHB_ERROR_DATA_TYPE_P7IOC: > ioda_eeh_p7ioc_phb_diag(hose, common); Thanks, Gavin
On Fri, Dec 20, 2013 at 09:35:39AM +0800, Gavin Shan wrote: >On Thu, Dec 19, 2013 at 05:14:07PM -0600, Brian W Hart wrote: >>PHB diagnostic buffer may be smaller than PAGE_SIZE, especially when >>PAGE_SIZE > 4KB. >> > >I think you're talking about that PAGE_SIZE could be configured >to have variable size (e.g. 4KB). So it's not safe to pass PAGE_SIZE >to OPAL API opal_pci_get_phb_diag_data2(). Instead, we should pass >PNV_PCI_DIAG_BUF_SIZE and it makes sense to me :-) > >Also, it needs to be backported to stable kernel as well. > >>Signed-off-by: Brian W Hart <hartb@linux.vnet.ibm.com> > >Acked-by: Gavin Shan <shangw@linux.vnet.ibm.com> > Sorry, Brian. It has been fixed as part of the following commit, which has been put into Ben's powerpc-next branch :-) commit 93aef2a789778e7ec787179fc9b34ca4885a5ef3 161 static void ioda_eeh_phb_diag(struct pci_controller *hose) 162 { 163 struct pnv_phb *phb = hose->private_data; 164 - struct OpalIoPhbErrorCommon *common; 165 long rc; 166 167 - common = (struct OpalIoPhbErrorCommon *)phb->diag.blob; 168 - rc = opal_pci_get_phb_diag_data2(phb->opal_id, common, PAGE_SIZE); 169 + rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob, 170 + PNV_PCI_DIAG_BUF_SIZE); >>--- >> arch/powerpc/platforms/powernv/eeh-ioda.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c >>index 02245ce..8184ef5 100644 >>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c >>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c >>@@ -820,14 +820,15 @@ static void ioda_eeh_phb_diag(struct pci_controller *hose) >> struct OpalIoPhbErrorCommon *common; >> long rc; >> >>- common = (struct OpalIoPhbErrorCommon *)phb->diag.blob; >>- rc = opal_pci_get_phb_diag_data2(phb->opal_id, common, PAGE_SIZE); >>+ 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; >> } >> >>+ common = (struct OpalIoPhbErrorCommon *)phb->diag.blob; >> switch (common->ioType) { >> case OPAL_PHB_ERROR_DATA_TYPE_P7IOC: >> ioda_eeh_p7ioc_phb_diag(hose, common); > Thanks, Gavin
On Fri, Dec 20, 2013 at 09:59:37AM +0800, Gavin Shan wrote: > On Fri, Dec 20, 2013 at 09:35:39AM +0800, Gavin Shan wrote: > >On Thu, Dec 19, 2013 at 05:14:07PM -0600, Brian W Hart wrote: > >>PHB diagnostic buffer may be smaller than PAGE_SIZE, especially when > >>PAGE_SIZE > 4KB. > >> > > > >I think you're talking about that PAGE_SIZE could be configured > >to have variable size (e.g. 4KB). So it's not safe to pass PAGE_SIZE > >to OPAL API opal_pci_get_phb_diag_data2(). Instead, we should pass > >PNV_PCI_DIAG_BUF_SIZE and it makes sense to me :-) Yeah, I noticed the problem because our test machine has PAGE_SIZE of 64K with the buffer only being 8K. [...] > Sorry, Brian. It has been fixed as part of the following commit, which > has been put into Ben's powerpc-next branch :-) Thank you!
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c index 02245ce..8184ef5 100644 --- a/arch/powerpc/platforms/powernv/eeh-ioda.c +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c @@ -820,14 +820,15 @@ static void ioda_eeh_phb_diag(struct pci_controller *hose) struct OpalIoPhbErrorCommon *common; long rc; - common = (struct OpalIoPhbErrorCommon *)phb->diag.blob; - rc = opal_pci_get_phb_diag_data2(phb->opal_id, common, PAGE_SIZE); + 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; } + common = (struct OpalIoPhbErrorCommon *)phb->diag.blob; switch (common->ioType) { case OPAL_PHB_ERROR_DATA_TYPE_P7IOC: ioda_eeh_p7ioc_phb_diag(hose, common);
PHB diagnostic buffer may be smaller than PAGE_SIZE, especially when PAGE_SIZE > 4KB. Signed-off-by: Brian W Hart <hartb@linux.vnet.ibm.com> --- arch/powerpc/platforms/powernv/eeh-ioda.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)