linux-kernel - Re: [PATCH v5 05/14] iommu/amd: Introduce helper function amd_iommu_update

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aRYvFmHdUImiU0zA@Asurada-Nvidia>
Date: Thu, 13 Nov 2025 11:18:46 -0800
From: Nicolin Chen <nicolinc@...dia.com>
To: Suravee Suthikulpanit <suravee.suthikulpanit@....com>
CC: <jgg@...dia.com>, <linux-kernel@...r.kernel.org>, <robin.murphy@....com>,
	<will@...nel.org>, <joro@...tes.org>, <kevin.tian@...el.com>,
	<jsnitsel@...hat.com>, <vasant.hegde@....com>, <iommu@...ts.linux.dev>,
	<santosh.shukla@....com>, <sairaj.arunkodilkar@....com>, <jon.grimm@....com>,
	<prashanthpra@...gle.com>, <wvw@...gle.com>, <wnliu@...gle.com>,
	<gptran@...gle.com>, <kpsingh@...gle.com>, <joao.m.martins@...cle.com>,
	<alejandro.j.jimenez@...cle.com>
Subject: Re: [PATCH v5 05/14] iommu/amd: Introduce helper function
 amd_iommu_update_dte()

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