| Message ID | 20200406030745.24595-4-oohall@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9b9408c55935ecc3b1c27b3eeb5a507394113cbb |
| Headers | show |
| Series | [1/7] powerpc/powernv/npu: Clean up compound table group initialisation | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (2c0ce4ff35994a7b12cc9879ced52c9e7c2e6667) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 54 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On 06/04/2020 13:07, Oliver O'Halloran wrote: > Move the registration of IOMMU groups out of the post-phb init fixup and > into when we configure DMA for a PE. For most devices this doesn't > result in any functional changes, but for NVLink attached GPUs it > requires a bit of care. When the GPU is probed an IOMMU group would be > created for the PE that contains it. We need to ensure that group is > removed before we add the PE to the compound group that's used to keep > the translations see by the PCIe and NVLink buses the same. > > No functional changes. Probably. > > Cc: Alexey Kardashevskiy <aik@ozlabs.ru> > Cc: Reza Arbab <arbab@linux.ibm.com> > Cc: Alistair Popple <alistair@popple.id.au> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> > --- > arch/powerpc/platforms/powernv/npu-dma.c | 9 +++++++++ > arch/powerpc/platforms/powernv/pci-ioda.c | 16 ++++++---------- > 2 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c > index de617549c9a3..4fbbdfa8b327 100644 > --- a/arch/powerpc/platforms/powernv/npu-dma.c > +++ b/arch/powerpc/platforms/powernv/npu-dma.c > @@ -469,6 +469,15 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe) > compound_group->pgsizes = pe->table_group.pgsizes; > } > > + /* > + * I'm not sure this is strictly required, but it's probably a good idea > + * since the table_group for the PE is going to be attached to the This seems just a right thing to do so I suggest changing the comment from "not sure" to "we are going to put GPU to a compound group and won't need the individual group". Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > + * compound table group. If we leave the PE's iommu group active then > + * we might have the same table_group being modifiable via two sepeate > + * iommu groups. > + */ > + iommu_group_put(pe->table_group.group); > + > pnv_comp_attach_table_group(npucomp, pe); > > return compound_group; > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c > index 2c340504fa77..cf0aaef1b8fa 100644 > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > @@ -1619,10 +1619,6 @@ static void pnv_ioda_setup_vf_PE(struct pci_dev *pdev, u16 num_vfs) > } > > pnv_pci_ioda2_setup_dma_pe(phb, pe); > -#ifdef CONFIG_IOMMU_API > - iommu_register_group(&pe->table_group, > - pe->phb->hose->global_number, pe->pe_number); > -#endif > } > } > > @@ -2661,9 +2657,6 @@ static void pnv_pci_ioda_setup_iommu_api(void) > continue; > > table_group = &pe->table_group; > - iommu_register_group(&pe->table_group, > - pe->phb->hose->global_number, > - pe->pe_number); > } > pnv_ioda_setup_bus_iommu_group(pe, table_group, > pe->pbus); > @@ -2748,14 +2741,17 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb, > IOMMU_TABLE_GROUP_MAX_TABLES; > pe->table_group.max_levels = POWERNV_IOMMU_MAX_LEVELS; > pe->table_group.pgsizes = pnv_ioda_parse_tce_sizes(phb); > -#ifdef CONFIG_IOMMU_API > - pe->table_group.ops = &pnv_pci_ioda2_ops; > -#endif > > rc = pnv_pci_ioda2_setup_default_config(pe); > if (rc) > return; > > +#ifdef CONFIG_IOMMU_API > + pe->table_group.ops = &pnv_pci_ioda2_ops; > + iommu_register_group(&pe->table_group, phb->hose->global_number, > + pe->pe_number); > +#endif > + > if (pe->flags & (PNV_IODA_PE_BUS | PNV_IODA_PE_BUS_ALL)) > pnv_ioda_setup_bus_dma(pe, pe->pbus); > } >
diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c index de617549c9a3..4fbbdfa8b327 100644 --- a/arch/powerpc/platforms/powernv/npu-dma.c +++ b/arch/powerpc/platforms/powernv/npu-dma.c @@ -469,6 +469,15 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe) compound_group->pgsizes = pe->table_group.pgsizes; } + /* + * I'm not sure this is strictly required, but it's probably a good idea + * since the table_group for the PE is going to be attached to the + * compound table group. If we leave the PE's iommu group active then + * we might have the same table_group being modifiable via two sepeate + * iommu groups. + */ + iommu_group_put(pe->table_group.group); + pnv_comp_attach_table_group(npucomp, pe); return compound_group; diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c index 2c340504fa77..cf0aaef1b8fa 100644 --- a/arch/powerpc/platforms/powernv/pci-ioda.c +++ b/arch/powerpc/platforms/powernv/pci-ioda.c @@ -1619,10 +1619,6 @@ static void pnv_ioda_setup_vf_PE(struct pci_dev *pdev, u16 num_vfs) } pnv_pci_ioda2_setup_dma_pe(phb, pe); -#ifdef CONFIG_IOMMU_API - iommu_register_group(&pe->table_group, - pe->phb->hose->global_number, pe->pe_number); -#endif } } @@ -2661,9 +2657,6 @@ static void pnv_pci_ioda_setup_iommu_api(void) continue; table_group = &pe->table_group; - iommu_register_group(&pe->table_group, - pe->phb->hose->global_number, - pe->pe_number); } pnv_ioda_setup_bus_iommu_group(pe, table_group, pe->pbus); @@ -2748,14 +2741,17 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb, IOMMU_TABLE_GROUP_MAX_TABLES; pe->table_group.max_levels = POWERNV_IOMMU_MAX_LEVELS; pe->table_group.pgsizes = pnv_ioda_parse_tce_sizes(phb); -#ifdef CONFIG_IOMMU_API - pe->table_group.ops = &pnv_pci_ioda2_ops; -#endif rc = pnv_pci_ioda2_setup_default_config(pe); if (rc) return; +#ifdef CONFIG_IOMMU_API + pe->table_group.ops = &pnv_pci_ioda2_ops; + iommu_register_group(&pe->table_group, phb->hose->global_number, + pe->pe_number); +#endif + if (pe->flags & (PNV_IODA_PE_BUS | PNV_IODA_PE_BUS_ALL)) pnv_ioda_setup_bus_dma(pe, pe->pbus); }
Move the registration of IOMMU groups out of the post-phb init fixup and into when we configure DMA for a PE. For most devices this doesn't result in any functional changes, but for NVLink attached GPUs it requires a bit of care. When the GPU is probed an IOMMU group would be created for the PE that contains it. We need to ensure that group is removed before we add the PE to the compound group that's used to keep the translations see by the PCIe and NVLink buses the same. No functional changes. Probably. Cc: Alexey Kardashevskiy <aik@ozlabs.ru> Cc: Reza Arbab <arbab@linux.ibm.com> Cc: Alistair Popple <alistair@popple.id.au> Signed-off-by: Oliver O'Halloran <oohall@gmail.com> --- arch/powerpc/platforms/powernv/npu-dma.c | 9 +++++++++ arch/powerpc/platforms/powernv/pci-ioda.c | 16 ++++++---------- 2 files changed, 15 insertions(+), 10 deletions(-)