Which includes DTE update, clone_aliases, DTE flush and completion-wait
commands to avoid code duplication when reuse to setup DTE for nested
translation.

Also, make amd_iommu_update_dte() non-static to reuse in
in a new nested.c file for nested translation.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 drivers/iommu/amd/amd_iommu.h |  4 ++++
 drivers/iommu/amd/iommu.c     | 24 ++++++++++++++++++------
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index a6ffb1ece2f9..3ad8b5e65a82 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -187,6 +187,10 @@ void amd_iommu_domain_set_pgtable(struct protection_domain *domain,
 struct dev_table_entry *get_dev_table(struct amd_iommu *iommu);
 struct iommu_dev_data *search_dev_data(struct amd_iommu *iommu, u16 devid);
 
+void amd_iommu_update_dte(struct amd_iommu *iommu,
+			  struct iommu_dev_data *dev_data,
+			  struct dev_table_entry *new);
+
 static inline void
 amd_iommu_make_clear_dte(struct iommu_dev_data *dev_data, struct dev_table_entry *new)
 {
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 0be2c818504c..a36426f096d2 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -76,6 +76,8 @@ static void set_dte_entry(struct amd_iommu *iommu,
 			  struct iommu_dev_data *dev_data,
 			  phys_addr_t top_paddr, unsigned int top_level);
 
+static int device_flush_dte(struct iommu_dev_data *dev_data);
+
 static void amd_iommu_change_top(struct pt_iommu *iommu_table,
 				 phys_addr_t top_paddr, unsigned int top_level);
 
@@ -86,6 +88,10 @@ static bool amd_iommu_enforce_cache_coherency(struct iommu_domain *domain);
 static int amd_iommu_set_dirty_tracking(struct iommu_domain *domain,
 					bool enable);
 
+static void clone_aliases(struct amd_iommu *iommu, struct device *dev);
+
+static int iommu_completion_wait(struct amd_iommu *iommu);
+
 /****************************************************************************
  *
  * Helper functions
@@ -203,6 +209,16 @@ static void update_dte256(struct amd_iommu *iommu, struct iommu_dev_data *dev_da
 	spin_unlock_irqrestore(&dev_data->dte_lock, flags);
 }
 
+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);
-
-	clone_aliases(iommu, dev_data->dev);
-	device_flush_dte(dev_data);
-	iommu_completion_wait(iommu);
 }
 
 /*
-- 
2.34.1

On Wed, Nov 12, 2025 at 06:24:57PM +0000, Suravee Suthikulpanit wrote:
> Which includes DTE update, clone_aliases, DTE flush and completion-wait
> commands to avoid code duplication when reuse to setup DTE for nested
> translation.
> 
> Also, make amd_iommu_update_dte() non-static to reuse in
> in a new nested.c file for nested translation.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  drivers/iommu/amd/amd_iommu.h |  4 ++++
>  drivers/iommu/amd/iommu.c     | 24 ++++++++++++++++++------
>  2 files changed, 22 insertions(+), 6 deletions(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

> +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);
> +}

Something to think about later, but clone_aliases is not optimal
now.. Since we have a "struct dev_table_entry *new" right here we can
pass it directly into clone_aliases() which can avoid doing
get_dte256() entirely.

And I don't think we need setup_aliases() to call clone_alises() these
days as the core code is now always immediately attaching a domain of
some kind after probe_device which will write a known DTE anyhow.

Finally, it would be an improvement to this alias code if it worked
like arm and just kept an allocated array of the all the DTE table
indexes setup during device probe time and just ran over the list
instead of repeatedly using PCI functions and callbacks..

Jason

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