Re: [PATCH v2 10/14] iommu/amd: Remove conditions from domain free paths

[Vasant Hegde <vasant.hegde@amd.com>]

Jason,

On 8/30/2024 5:36 AM, Jason Gunthorpe wrote:
> Don't use tlb as some flag to indicate if protection_domain_alloc()
> completed. Have protection_domain_alloc() unwind itself in the normal
> kernel style and require protection_domain_free() only be called on
> successful results of protection_domain_alloc().
> 
> Also, the amd_iommu_domain_free() op is never called by the core code with
> a NULL argument, so remove all the NULL tests as well.
> 
> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/amd/iommu.c | 29 ++++++++++-------------------
>  1 file changed, 10 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 806156632bf70a..e23b7f7bf5b65d 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2258,17 +2258,9 @@ static void cleanup_domain(struct protection_domain *domain)
>  
>  void protection_domain_free(struct protection_domain *domain)
>  {
> -	if (!domain)
> -		return;
> -
>  	WARN_ON(!list_empty(&domain->dev_list));
> -
> -	if (domain->iop.pgtbl.cfg.tlb)
> -		free_io_pgtable_ops(&domain->iop.pgtbl.ops);
> -
> -	if (domain->id)
> -		domain_id_free(domain->id);
> -
> +	free_io_pgtable_ops(&domain->iop.pgtbl.ops);

This is common path gets for all domain type including passthrough, SVA.
Hence its not yet ready for unconditional call. This is causing NULL pointer
dereference when I try to change domain type.

For now, can you keep abvoe check -OR- add PAGING_DOMAIN check? I will revisit
after implementing global static identity domain and fixing SVA code.

-Vasant