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 for Android: free password hash cracker in your pocket

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

Message-ID: <8ab60419-5fbc-418e-a059-e28e6c0682b0@amd.com>
Date: Thu, 15 Jan 2026 16:20:08 +0700
From: "Suthikulpanit, Suravee" <suravee.suthikulpanit@....com>
To: Nicolin Chen <nicolinc@...dia.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 11/14/2025 2:18 AM, Nicolin Chen wrote:
> 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);
> }
> 
> ?

Currently, set_dte_entry() is called from multiple call-path. Therefore, 
I would like to keep this way to simplify the code reuse.  The 
clear_dte_entry() is called only from one place, but I feel that having 
a separate function is simpler to read.

Thanks,
Suravee