Re: [PATCH v6 09/18] iommu/vt-d: Combine disable_dmar_iommu() and free_dmar_iommu()

[Baolu Lu <baolu.lu@linux.intel.com>]

On 10/25/26 10:19, Samiullah Khawaja wrote:
> Every caller of disable_dmar_iommu() immediately follows it with a call
> to free_dmar_iommu(). Combine the two helpers into release_dmar_iommu()
> and simplify the teardown and error paths in init_dmars(),
> intel_iommu_add(), and dmar_iommu_hotplug().
> 
> Also drop the early return when
> WARN_ON(!ida_is_empty(&iommu->domain_ida)) triggers so translation is
> still disabled before freeing the IOMMU resources.
> 
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> ---
>   drivers/iommu/intel/iommu.c | 30 ++++++++++--------------------
>   1 file changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 23a1da351ddf..35ec2ae0c8d5 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -985,21 +985,17 @@ static void iommu_disable_translation(struct intel_iommu *iommu)
>   	raw_spin_unlock_irqrestore(&iommu->register_lock, flag);
>   }
>   
> -static void disable_dmar_iommu(struct intel_iommu *iommu)
> +static void release_dmar_iommu(struct intel_iommu *iommu)
>   {
>   	/*
>   	 * All iommu domains must have been detached from the devices,
>   	 * hence there should be no domain IDs in use.
>   	 */
> -	if (WARN_ON(!ida_is_empty(&iommu->domain_ida)))
> -		return;
> +	WARN_ON(!ida_is_empty(&iommu->domain_ida));
>   
>   	if (iommu->gcmd & DMA_GCMD_TE)
>   		iommu_disable_translation(iommu);
> -}
>   
> -static void free_dmar_iommu(struct intel_iommu *iommu)
> -{
>   	if (iommu->copied_tables) {
>   		bitmap_free(iommu->copied_tables);
>   		iommu->copied_tables = NULL;
> @@ -1867,10 +1863,8 @@ static int __init init_dmars(void)
>   	return 0;
>   
>   free_iommu:
> -	for_each_active_iommu(iommu, drhd) {
> -		disable_dmar_iommu(iommu);
> -		free_dmar_iommu(iommu);
> -	}
> +	for_each_active_iommu(iommu, drhd)
> +		release_dmar_iommu(iommu);
>   
>   	return ret;
>   }
> @@ -2280,12 +2274,12 @@ static int intel_iommu_add(struct dmar_drhd_unit *dmaru)
>   	if (ecap_prs(iommu->ecap)) {
>   		ret = intel_iommu_enable_prq(iommu);
>   		if (ret)
> -			goto disable_iommu;
> +			goto out;
>   	}
>   
>   	ret = dmar_set_interrupt(iommu);
>   	if (ret)
> -		goto disable_iommu;
> +		goto out;
>   
>   	iommu_set_root_entry(iommu);
>   	iommu_enable_translation(iommu);
> @@ -2293,10 +2287,8 @@ static int intel_iommu_add(struct dmar_drhd_unit *dmaru)
>   	iommu_disable_protect_mem_regions(iommu);
>   	return 0;
>   
> -disable_iommu:
> -	disable_dmar_iommu(iommu);
>   out:
> -	free_dmar_iommu(iommu);
> +	release_dmar_iommu(iommu);
>   	return ret;
>   }
>   
> @@ -2310,12 +2302,10 @@ int dmar_iommu_hotplug(struct dmar_drhd_unit *dmaru, bool insert)
>   	if (iommu == NULL)
>   		return -EINVAL;
>   
> -	if (insert) {
> +	if (insert)
>   		ret = intel_iommu_add(dmaru);
> -	} else {
> -		disable_dmar_iommu(iommu);
> -		free_dmar_iommu(iommu);
> -	}
> +	else
> +		release_dmar_iommu(iommu);
>   
>   	return ret;
>   }

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>