[PATCH kernel] powerpc/powernv/ioda2: Add devices only from buses which belong to PE

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE4078d

6 messages, 4 authors, 2015-06-15 · open the first message on its own page

[PATCH kernel] powerpc/powernv/ioda2: Add devices only from buses which belong to PE

From: Alexey Kardashevskiy <hidden>
Date: 2015-06-12 06:21:40

The existing code puts all devices from a root PE to the same IOMMU group.
However it is a possible situation when subordinate buses belong to
separate PEs, in this case devices from these subordinate buses
should be added to lower level PE rather to the root PE.

This limits pnv_ioda_setup_bus_dma() invocation to only PEs which own
all subordinate buses.

Suggested-by: Gavin Shan <redacted>
Signed-off-by: Alexey Kardashevskiy <redacted>
---

This would be nice to have together with the DDW patchset.
This does not fix anything DDW patchset did, it fixes IOMMU
groups management which is essential for the whole feature to work.

---
 arch/powerpc/platforms/powernv/pci-ioda.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index a0b00c1..3b1dc79 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1702,7 +1702,7 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
 		set_iommu_table_base(&dev->dev, pe->table_group.tables[0]);
 		iommu_add_device(&dev->dev);
 
-		if (dev->subordinate)
+		if ((pe->flags & PNV_IODA_PE_BUS_ALL) && dev->subordinate)
 			pnv_ioda_setup_bus_dma(pe, dev->subordinate);
 	}
 }
-- 
2.4.0.rc3.8.gfb3e7d5

Re: [PATCH kernel] powerpc/powernv/ioda2: Add devices only from buses which belong to PE

From: Gavin Shan <hidden>
Date: 2015-06-12 06:48:11

On Fri, Jun 12, 2015 at 04:19:17PM +1000, Alexey Kardashevskiy wrote:
The existing code puts all devices from a root PE to the same IOMMU group.
However it is a possible situation when subordinate buses belong to
separate PEs, in this case devices from these subordinate buses
should be added to lower level PE rather to the root PE.

This limits pnv_ioda_setup_bus_dma() invocation to only PEs which own
all subordinate buses.

Suggested-by: Gavin Shan <redacted>
Signed-off-by: Alexey Kardashevskiy <redacted>
Alexey, I think it's good candidate for stable.

Thanks,
Gavin
quoted hunk
---

This would be nice to have together with the DDW patchset.
This does not fix anything DDW patchset did, it fixes IOMMU
groups management which is essential for the whole feature to work.

---
arch/powerpc/platforms/powernv/pci-ioda.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index a0b00c1..3b1dc79 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1702,7 +1702,7 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
		set_iommu_table_base(&dev->dev, pe->table_group.tables[0]);
		iommu_add_device(&dev->dev);

-		if (dev->subordinate)
+		if ((pe->flags & PNV_IODA_PE_BUS_ALL) && dev->subordinate)
			pnv_ioda_setup_bus_dma(pe, dev->subordinate);
	}
}
-- 
2.4.0.rc3.8.gfb3e7d5

Re: [PATCH kernel] powerpc/powernv/ioda2: Add devices only from buses which belong to PE

From: Nishanth Aravamudan <hidden>
Date: 2015-06-12 15:06:51

On 12.06.2015 [16:47:03 +1000], Gavin Shan wrote:
On Fri, Jun 12, 2015 at 04:19:17PM +1000, Alexey Kardashevskiy wrote:
quoted
The existing code puts all devices from a root PE to the same IOMMU group.
However it is a possible situation when subordinate buses belong to
separate PEs, in this case devices from these subordinate buses
should be added to lower level PE rather to the root PE.

This limits pnv_ioda_setup_bus_dma() invocation to only PEs which own
all subordinate buses.

Suggested-by: Gavin Shan <redacted>
Signed-off-by: Alexey Kardashevskiy <redacted>
Alexey, I think it's good candidate for stable.
Why? This commit message doesn't imply there is a serious bug just a
"possible" (does that mean theoretical?) situtation and the following:
quoted
This would be nice to have together with the DDW patchset.
This does not fix anything DDW patchset did, it fixes IOMMU
groups management which is essential for the whole feature to work.
implies it is both related and independent of DDW?

-Nish

Re: [PATCH kernel] powerpc/powernv/ioda2: Add devices only from buses which belong to PE

From: Alexey Kardashevskiy <hidden>
Date: 2015-06-12 16:48:08

On 06/13/2015 01:06 AM, Nishanth Aravamudan wrote:
On 12.06.2015 [16:47:03 +1000], Gavin Shan wrote:
quoted
On Fri, Jun 12, 2015 at 04:19:17PM +1000, Alexey Kardashevskiy wrote:
quoted
The existing code puts all devices from a root PE to the same IOMMU group.
However it is a possible situation when subordinate buses belong to
separate PEs, in this case devices from these subordinate buses
should be added to lower level PE rather to the root PE.

This limits pnv_ioda_setup_bus_dma() invocation to only PEs which own
all subordinate buses.

Suggested-by: Gavin Shan <redacted>
Signed-off-by: Alexey Kardashevskiy <redacted>
Alexey, I think it's good candidate for stable.
Why? This commit message doesn't imply there is a serious bug just a
"possible" (does that mean theoretical?) situtation and the following:
quoted
quoted
This would be nice to have together with the DDW patchset.
This does not fix anything DDW patchset did, it fixes IOMMU
groups management which is essential for the whole feature to work.
implies it is both related and independent of DDW?

DDW is for fast devices and I just got my hands on a "firestone" machine 
(p8 + nvidia tesla = very fast machine) where this bug appeared first time.


-- 
Alexey

Re: [PATCH kernel] powerpc/powernv/ioda2: Add devices only from buses which belong to PE

From: Gavin Shan <hidden>
Date: 2015-06-15 00:14:07

On Fri, Jun 12, 2015 at 08:06:43AM -0700, Nishanth Aravamudan wrote:
On 12.06.2015 [16:47:03 +1000], Gavin Shan wrote:
quoted
On Fri, Jun 12, 2015 at 04:19:17PM +1000, Alexey Kardashevskiy wrote:
quoted
The existing code puts all devices from a root PE to the same IOMMU group.
However it is a possible situation when subordinate buses belong to
separate PEs, in this case devices from these subordinate buses
should be added to lower level PE rather to the root PE.

This limits pnv_ioda_setup_bus_dma() invocation to only PEs which own
all subordinate buses.

Suggested-by: Gavin Shan <redacted>
Signed-off-by: Alexey Kardashevskiy <redacted>
Alexey, I think it's good candidate for stable.
Why? This commit message doesn't imply there is a serious bug just a
"possible" (does that mean theoretical?) situtation and the following:
The problem depends on the PCI topology. Below topology, existing on
firestone machine, is one of the cases that wrong IOMMU group is used
for devices behind the upstream port of the PCIe switch: The PE for
Bus#2 has same IOMMU group as that one of the PE for "Root Bus" wrongly
here.

                        |
                        +---------------+       (Root Bus)
                        |               |
                  [ Root Port ]    [ System Peripheral ]
                        +
                        |			(Bus#1)
                        +
                  [ Up Port ]
			+
			|
           +------------+-----------+           (Bus#2)
           |                        |
     [ Dn port]                [ Dn Port]
quoted
quoted
This would be nice to have together with the DDW patchset.
This does not fix anything DDW patchset did, it fixes IOMMU
groups management which is essential for the whole feature to work.
implies it is both related and independent of DDW?
The problem is independent of DDW.

Thanks,
Gavin
-Nish

Re: [kernel] powerpc/powernv/ioda2: Add devices only from buses which belong to PE

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2015-06-15 06:49:52

On Fri, 2015-12-06 at 06:19:17 UTC, Alexey Kardashevskiy wrote:
The existing code puts all devices from a root PE to the same IOMMU group.
However it is a possible situation when subordinate buses belong to
separate PEs, in this case devices from these subordinate buses
should be added to lower level PE rather to the root PE.

This limits pnv_ioda_setup_bus_dma() invocation to only PEs which own
all subordinate buses.

Suggested-by: Gavin Shan <redacted>
Signed-off-by: Alexey Kardashevskiy <redacted>
Alexey/Gavin, can one of you write a better changelog for this. Explaining
what exactly is broken without it, how it fixes the problem, and why that is
the right fix.

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help