[PATCH] powerpc/powernv: Exclude MSI region in extended bridge window

[PATCH] powerpc/powernv: Exclude MSI region in extended bridge window

[Gavin Shan]

The windows of root port and bridge behind that are extended to
the PHB's windows to accomodate the PCI hotplug happening in
future. The PHB's 64KB 32-bits MSI region is included in bridge's
M32 windows (in hardware) though it's excluded in the corresponding
resource, as the bridge's M32 windows have 1MB as their minimal
alignment. We observed EEH error during system boot when the MSI
region is included in bridge's M32 window.

This excludes top 1MB (including 64KB 32-bits MSI region) region
from bridge's M32 windows when extending them.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index bde7f76..e0a8a92 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -3239,6 +3239,7 @@ static void pnv_pci_fixup_bridge_resources(struct pci_bus *bus,
 	struct pnv_phb *phb = hose->private_data;
 	struct pci_dev *bridge = bus->self;
 	struct resource *r, *w;
+	bool msi_region = false;
 	int i;
 
 	/* Check if we need apply fixup to the bridge's windows */
@@ -3259,11 +3260,25 @@ static void pnv_pci_fixup_bridge_resources(struct pci_bus *bus,
 			 (type & IORESOURCE_PREFETCH) &&
 			 phb->ioda.m64_segsize)
 			w = &hose->mem_resources[1];
-		else if (r->flags & type & IORESOURCE_MEM)
+		else if (r->flags & type & IORESOURCE_MEM) {
 			w = &hose->mem_resources[0];
+			msi_region = true;
+		}
 
 		r->start = w->start;
 		r->end = w->end;
+
+		/* The 64KB 32-bits MSI region shouldn't be included in
+		 * the 32-bits bridge window. Otherwise, we can see strange
+		 * issues. One of them is EEH error observed on Garrison.
+		 *
+		 * Exclude top 1MB region which is the minimal alignment of
+		 * 32-bits bridge window.
+		 */
+		if (msi_region) {
+			r->end += 0x10000;
+			r->end -= 0x100000;
+		}
 	}
 }
 
-- 
2.1.0

Re: [PATCH] powerpc/powernv: Exclude MSI region in extended bridge window

[Gavin Shan]

On Tue, Jun 21, 2016 at 12:41:05PM +1000, Gavin Shan wrote:
>The windows of root port and bridge behind that are extended to
>the PHB's windows to accomodate the PCI hotplug happening in
>future. The PHB's 64KB 32-bits MSI region is included in bridge's
>M32 windows (in hardware) though it's excluded in the corresponding
>resource, as the bridge's M32 windows have 1MB as their minimal
>alignment. We observed EEH error during system boot when the MSI
>region is included in bridge's M32 window.
>
>This excludes top 1MB (including 64KB 32-bits MSI region) region
>from bridge's M32 windows when extending them.
>
>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>---
> arch/powerpc/platforms/powernv/pci-ioda.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>

Michael, I saw the PCI hotplug patches have been merged to your "test"
branch. This one is the fix for EEH error found on Garrison platform.
Please apply it on top of that series.

Thanks,
Gavin

>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>index bde7f76..e0a8a92 100644
>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>@@ -3239,6 +3239,7 @@ static void pnv_pci_fixup_bridge_resources(struct pci_bus *bus,
> 	struct pnv_phb *phb = hose->private_data;
> 	struct pci_dev *bridge = bus->self;
> 	struct resource *r, *w;
>+	bool msi_region = false;
> 	int i;
>
> 	/* Check if we need apply fixup to the bridge's windows */
>@@ -3259,11 +3260,25 @@ static void pnv_pci_fixup_bridge_resources(struct pci_bus *bus,
> 			 (type & IORESOURCE_PREFETCH) &&
> 			 phb->ioda.m64_segsize)
> 			w = &hose->mem_resources[1];
>-		else if (r->flags & type & IORESOURCE_MEM)
>+		else if (r->flags & type & IORESOURCE_MEM) {
> 			w = &hose->mem_resources[0];
>+			msi_region = true;
>+		}
>
> 		r->start = w->start;
> 		r->end = w->end;
>+
>+		/* The 64KB 32-bits MSI region shouldn't be included in
>+		 * the 32-bits bridge window. Otherwise, we can see strange
>+		 * issues. One of them is EEH error observed on Garrison.
>+		 *
>+		 * Exclude top 1MB region which is the minimal alignment of
>+		 * 32-bits bridge window.
>+		 */
>+		if (msi_region) {
>+			r->end += 0x10000;
>+			r->end -= 0x100000;
>+		}
> 	}
> }
>
>-- 
>2.1.0
>

Re: powerpc/powernv: Exclude MSI region in extended bridge window

[Michael Ellerman]

On Tue, 2016-21-06 at 02:41:05 UTC, Gavin Shan wrote:
> The windows of root port and bridge behind that are extended to
> the PHB's windows to accomodate the PCI hotplug happening in
> future. The PHB's 64KB 32-bits MSI region is included in bridge's
> M32 windows (in hardware) though it's excluded in the corresponding
> resource, as the bridge's M32 windows have 1MB as their minimal
> alignment. We observed EEH error during system boot when the MSI
> region is included in bridge's M32 window.
> 
> This excludes top 1MB (including 64KB 32-bits MSI region) region
> from bridge's M32 windows when extending them.

AFAICS you added that code in "powerpc/powernv: Extend PCI bridge resources", so
I'll squash it into that. That way there is no window of breakage.

cheers

Re: powerpc/powernv: Exclude MSI region in extended bridge window

[Gavin Shan]

On Tue, Jun 21, 2016 at 02:30:48PM +1000, Michael Ellerman wrote:
>On Tue, 2016-21-06 at 02:41:05 UTC, Gavin Shan wrote:
>> The windows of root port and bridge behind that are extended to
>> the PHB's windows to accomodate the PCI hotplug happening in
>> future. The PHB's 64KB 32-bits MSI region is included in bridge's
>> M32 windows (in hardware) though it's excluded in the corresponding
>> resource, as the bridge's M32 windows have 1MB as their minimal
>> alignment. We observed EEH error during system boot when the MSI
>> region is included in bridge's M32 window.
>> 
>> This excludes top 1MB (including 64KB 32-bits MSI region) region
>> from bridge's M32 windows when extending them.
>
>AFAICS you added that code in "powerpc/powernv: Extend PCI bridge resources", so
>I'll squash it into that. That way there is no window of breakage.
>

Yeah, I guess it's the best way to go. Thanks a lot, Michael.

Thanks,
Gavin