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 <a...@ozlabs.ru>
> Cc: Reza Arbab <ar...@linux.ibm.com>
> Cc: Alistair Popple <alist...@popple.id.au>
> Signed-off-by: Oliver O'Halloran <ooh...@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 <a...@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);
> }
>
--
Alexey
