Re: [PATCH v5 05/14] iommu/amd: Introduce helper function amd_iommu_update_dte()

[Nicolin Chen <nicolinc@nvidia.com>]

On Wed, Nov 12, 2025 at 06:24:57PM +0000, Suravee Suthikulpanit wrote:
> +void amd_iommu_update_dte(struct amd_iommu *iommu,
> +			     struct iommu_dev_data *dev_data,
> +			     struct dev_table_entry *new)
> +{
> +	update_dte256(iommu, dev_data, new);
> +	clone_aliases(iommu, dev_data->dev);
> +	device_flush_dte(dev_data);
> +	iommu_completion_wait(iommu);
> +}
> +
>  static void get_dte256(struct amd_iommu *iommu, struct iommu_dev_data *dev_data,
>  		      struct dev_table_entry *dte)
>  {
> @@ -2088,7 +2104,7 @@ static void set_dte_entry(struct amd_iommu *iommu,
>  
>  	set_dte_gcr3_table(iommu, dev_data, &new);
>  
> -	update_dte256(iommu, dev_data, &new);
> +	amd_iommu_update_dte(iommu, dev_data, &new);
>  
>  	/*
>  	 * A kdump kernel might be replacing a domain ID that was copied from
> @@ -2108,7 +2124,7 @@ static void clear_dte_entry(struct amd_iommu *iommu, struct iommu_dev_data *dev_
>  	struct dev_table_entry new = {};
>  
>  	amd_iommu_make_clear_dte(dev_data, &new);
> -	update_dte256(iommu, dev_data, &new);
> +	amd_iommu_update_dte(iommu, dev_data, &new);
>  }
>  
>  /* Update and flush DTE for the given device */
> @@ -2120,10 +2136,6 @@ static void dev_update_dte(struct iommu_dev_data *dev_data, bool set)
>  		set_dte_entry(iommu, dev_data, 0, 0);
>  	else
>  		clear_dte_entry(iommu, dev_data);

I found these two are somewhat unnecessary.

set_dte_entry()
{
	u32 old_domid;

	make_clear_dte(dev_data, dte, &new);
	....
	amd_iommu_update_dte(iommu, dev_data, &new);
	if (old_domid)
		amd_iommu_flush_tlb_domid(iommu, old_domid);
}

clear_dte_entry()
{
	make_clear_dte(dev_data, dte, &new);
	amd_iommu_update_dte(iommu, dev_data, &new);
}

And given that dev_update_dte() now are just calling these them
without any other thing to do. Why not just unwrap them:

dev_update_dte(struct iommu_dev_data *dev_data, bool set)
{
	u32 old_domid = 0;

	make_clear_dte(dev_data, dte, &new);
	if (!set)
		goto update_dte;
	....
update_dte:
	amd_iommu_update_dte(iommu, dev_data, &new);
	if (old_domid)
		amd_iommu_flush_tlb_domid(iommu, old_domid);
}

?

Nicolin