[PATCH 3/7] powerpc/powernv/pci: Register iommu group at PE DMA setup

[Alexey Kardashevskiy]

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 at ozlabs.ru>
> Cc: Reza Arbab <arbab at linux.ibm.com>
> Cc: Alistair Popple <alistair at popple.id.au>
> Signed-off-by: Oliver O'Halloran <oohall at 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 at 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