Re: [PATCH v1 11/13] iommu/amd/pgtbl_v2: Invalidate updated page ranges only

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Oct 06, 2023 at 10:16:22AM +0000, Vasant Hegde wrote:
> amd_iommu_domain_flush_pages() uses protection domain page table mode
> to detect/get default PASID. Hence we can use this function to
> invalidate v2 page table as well.
> 
> Also in __set_gcr3()/__clear_gcr3() path call domain_flush_pages()
> directly. So that we can remove __amd_iommu_flush_tlb() and related
> functions. This works fine as we have GCR3 table per domain.
> 
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>  drivers/iommu/amd/io_pgtable_v2.c | 10 ++--------
>  drivers/iommu/amd/iommu.c         |  6 ++++--
>  2 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iommu/amd/io_pgtable_v2.c b/drivers/iommu/amd/io_pgtable_v2.c
> index f818a7e254d4..6d69ba60744f 100644
> --- a/drivers/iommu/amd/io_pgtable_v2.c
> +++ b/drivers/iommu/amd/io_pgtable_v2.c
> @@ -244,7 +244,6 @@ static int iommu_v2_map_pages(struct io_pgtable_ops *ops, unsigned long iova,
>  	unsigned long mapped_size = 0;
>  	unsigned long o_iova = iova;
>  	size_t size = pgcount << __ffs(pgsize);
> -	int count = 0;
>  	int ret = 0;
>  	bool updated = false;
>  
> @@ -265,19 +264,14 @@ static int iommu_v2_map_pages(struct io_pgtable_ops *ops, unsigned long iova,
>  
>  		*pte = set_pte_attr(paddr, map_size, prot);
>  
> -		count++;
>  		iova += map_size;
>  		paddr += map_size;
>  		mapped_size += map_size;
>  	}
>  
>  out:
> -	if (updated) {
> -		if (count > 1)
> -			amd_iommu_flush_tlb(&pdom->domain, 0);
> -		else
> -			amd_iommu_flush_page(&pdom->domain, 0, o_iova);
> -	}
> +	if (updated)
> +		amd_iommu_domain_flush_pages(pdom, o_iova, size);
>  
>  	if (mapped)
>  		*mapped += mapped_size;
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 8da445e51251..764a5656b56b 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2698,7 +2698,8 @@ static int __set_gcr3(struct protection_domain *domain, u32 pasid,
>  
>  	*pte = (cr3 & PAGE_MASK) | GCR3_VALID;
>  
> -	return __amd_iommu_flush_tlb(domain, pasid);
> +	domain_flush_pages(domain, pasid, 0, CMD_INV_IOMMU_ALL_PAGES_ADDRESS);
> +	return 0;
>  }
>  
>  static int __clear_gcr3(struct protection_domain *domain, u32 pasid)
> @@ -2714,7 +2715,8 @@ static int __clear_gcr3(struct protection_domain *domain, u32 pasid)
>  
>  	*pte = 0;
>  
> -	return __amd_iommu_flush_tlb(domain, pasid);
> +	domain_flush_pages(domain, pasid, 0, CMD_INV_IOMMU_ALL_PAGES_ADDRESS);
> +	return 0;
>  }

You should make the other "flush all" wrapper accept a PASID and call
it here as well instead of open coding it, or do away with the flush
all wraper completely.

Jason