Re: [PATCH] powernv: eeh: add buffer for P7IOC hub error data

[Gavin Shan <shangw@linux.vnet.ibm.com>]

On Thu, Dec 19, 2013 at 05:18:53PM -0600, Brian W Hart wrote:
>Prevent ioda_eeh_hub_diag() from clobbering itself when called by supplying
>a buffer for P7IOC hub diagnostic data.  Take care to inform OPAL of the
>correct size for the buffer.
>
>Signed-off-by: Brian W Hart <hartb@linux.vnet.ibm.com>
>---
>
>I hope I've understood this correctly.  It looks to me like
>ioda_eeh_hub_data is effectively asking OPAL to clobber its own
>text (via 'data') when it makes the call to retrieve the hub data.
>

Yeah, we should have used following variable as HUB diag-data instead.

static char *hub_diag = NULL;

However, it's not safe to allocate page-sized buffer for "hub_diag".

>Added a hub diagnostic structure per-phb.  Perhaps the diagnostic
>structure better belongs in the phb->diag union, but I wasn't sure whether
>we'd need to carry the hub and PHB diag data at the same time.
>

Please put hub diag-data to struct pnv_phb::diag since we don't need
carry hub and PHB diag-data at same time. With it, please remove
variable "hub_diag" as well.

> arch/powerpc/platforms/powernv/eeh-ioda.c | 4 ++--
> arch/powerpc/platforms/powernv/pci.h      | 2 ++
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>index 8184ef5..dfd9134 100644
>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>@@ -636,8 +636,8 @@ static void ioda_eeh_hub_diag(struct pci_controller *hose)
> 	struct OpalIoP7IOCErrorData *data;
> 	long rc;
>
>-	data = (struct OpalIoP7IOCErrorData *)ioda_eeh_hub_diag;
>-	rc = opal_pci_get_hub_diag_data(phb->hub_id, data, PAGE_SIZE);
>+	data = (struct OpalIoP7IOCErrorData *)&phb->p7ioc_err;
>+	rc = opal_pci_get_hub_diag_data(phb->hub_id, data, sizeof *data);
> 	if (rc != OPAL_SUCCESS) {
> 		pr_warning("%s: Failed to get HUB#%llx diag-data (%ld)\n",
> 			   __func__, phb->hub_id, rc);
>diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
>index 911c24e..d1b6d8c 100644
>--- a/arch/powerpc/platforms/powernv/pci.h
>+++ b/arch/powerpc/platforms/powernv/pci.h
>@@ -177,6 +177,8 @@ struct pnv_phb {
> 		unsigned char			blob[PNV_PCI_DIAG_BUF_SIZE];
> 		struct OpalIoP7IOCPhbErrorData	p7ioc;
> 	} diag;
>+
>+	struct OpalIoP7IOCErrorData p7ioc_err;
> };
>
> extern struct pci_ops pnv_pci_ops;

Thanks,
Gavin