Re: [PATCH v6 09/24] iommu/amd: Introduce and map vIOMMU private IPA region

[Vasant Hegde <vasant.hegde@amd.com>]

On 9/30/2026 2:53 PM, Suravee Suthikulpanit wrote:
> AMD vIOMMU introduces the IOMMU Private Address (IPA) region for
> guest-side IOMMU virtualization data structures.
> 
> Introduce a per-IOMMU v1 paging domain in viommu_pdom, allocate
> 8MB of backing memory as four 2MB subregions, map them into the
> domain, and add viommu_private_space_init() /
> viommu_private_space_uninit() as a matched pair.
> 
> Register the owning IOMMU in viommu_pdom->iommu_array via
> amd_iommu_pdom_bind_iommu() so unmap uses the standard
> domain_flush_pages_v1() IOTLB invalidation path. On bare metal
> iommu_map() does not flush (amd_iommu_iotlb_sync_map() returns
> early unless amd_iommu_np_cache is set).
> 
> Call viommu_private_space_uninit() from amd_viommu_uninit() so
> boot failure and free_iommu_one() release the 8MB IPA mapping.
> Failed amd_viommu_init() after VF mapping goes through uninit.
> 
> viommu_priv_alloc_map() takes a gfp for backing pages and
> iommu_map() page tables. Boot uses GFP_KERNEL.
> iommu_alloc_pages_node_sz() zeros them. On set_memory_uc()
> or iommu_map() failure, restore WB or leak rather than returning
> UC-mapped memory to the allocator. iommu_map() already unmaps
> any partial IOVA.
> 
> Skip IOTLB invalidation when CONTROL_CMDBUF_EN is clear so boot
> failure after disable_iommus() does not wait on a dead command
> buffer.
> 
> set_memory_uc() on each 2MB-aligned subregion splits the x86
> direct map to 4KB PTEs. set_memory_wb() on free does not
> re-coalesce them, so the direct map stays fragmented (8MB per
> vIOMMU-capable IOMMU) for the life of the machine.
> 
> The private IPA domain skips iommu_domain_init(), so set
> IOMMU_DOMAIN_UNMANAGED. set_dte_entry() only programs a v1 table
> when type has __IOMMU_DOMAIN_PAGING; type 0 would WARN and install
> a cleared DTE when increase_top() rewrites the self DTE.
> > For more info, see section vIOMMU Private Address Space of the
> IOMMU specification [1].
> 
> [1] https://docs.amd.com/v/u/en-US/48882_3.10_PUB
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  drivers/iommu/amd/amd_iommu.h       |   5 +
>  drivers/iommu/amd/amd_iommu_types.h |  11 ++
>  drivers/iommu/amd/init.c            |   7 +-
>  drivers/iommu/amd/iommu.c           |  13 +-
>  drivers/iommu/amd/viommu.c          | 192 ++++++++++++++++++++++++++++
>  5 files changed, 225 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 8c298e090889..a0d5d7e34020 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -53,6 +53,8 @@ extern bool amd_iommu_hatdis;
>  /* Protection domain ops */
>  void amd_iommu_init_identity_domain(void);
>  struct protection_domain *protection_domain_alloc(void);
> +struct iommu_domain *amd_iommu_domain_alloc_paging_v1(struct device *dev,
> +						      u32 flags);
>  struct iommu_domain *amd_iommu_domain_alloc_sva(struct device *dev,
>  						struct mm_struct *mm);
>  void amd_iommu_domain_free(struct iommu_domain *dom);
> @@ -94,6 +96,9 @@ void amd_iommu_domain_flush_pages(struct protection_domain *domain,
>  void amd_iommu_dev_flush_pasid_pages(struct iommu_dev_data *dev_data,
>  				     ioasid_t pasid, u64 address, u64 last);
>  
> +int amd_iommu_pdom_bind_iommu(struct amd_iommu *iommu, struct protection_domain *pdom);
> +void amd_iommu_pdom_unbind_iommu(struct amd_iommu *iommu, struct protection_domain *pdom);
> +
>  #ifdef CONFIG_IRQ_REMAP
>  int amd_iommu_create_irq_domain(struct amd_iommu *iommu);
>  #else
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 9eb9a0c190de..9a90b8fe0fe3 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -417,6 +417,13 @@
>  /* Guest ID bits 0-14; bit 15 is reserved for secure vIOMMU. */
>  #define VIOMMU_MAX_GID		0x7FFF
>  
> +/*
> + * Total IOMMU private region is 8MB (4 x 2MB-subregion)
> + */
> +#define VIOMMU_PRIV_REGION_BASE		(0)
> +#define VIOMMU_PRIV_SUBREGION_CNT	(4)
> +#define VIOMMU_PRIV_SUBREGION_SIZE	(0x200000)  /* 2MB */
> +
>  /* Timeout stuff */
>  #define LOOP_TIMEOUT		100000
>  #define MMIO_STATUS_TIMEOUT	2000000
> @@ -804,6 +811,10 @@ struct amd_iommu {
>  	unsigned char iopfq_name[32];
>  
>  	struct ida gid_ida;		 /* guest IDs for this IOMMU */
> +
> +	/* HW vIOMMU support */
> +	struct protection_domain *viommu_pdom;
> +	void *viommu_priv_region[VIOMMU_PRIV_SUBREGION_CNT];
>  };
>  
>  static inline struct amd_iommu *dev_to_amd_iommu(struct device *dev)
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 5be422591bac..d34f991dee24 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -1805,7 +1805,12 @@ static void __init free_sysfs(struct amd_iommu *iommu)
>  static void __init free_iommu_one(struct amd_iommu *iommu)
>  {
>  	free_sysfs(iommu);
> -	/* Tear down vIOMMU before IOMMU buffers and MMIO are released. */
> +	/*
> +	 * Tear down vIOMMU before buffers and MMIO are released.
> +	 * IOTLB invalidation runs only while CONTROL_CMDBUF_EN is
> +	 * set (amd_viommu_init() unwind). disable_iommus() clears
> +	 * that bit first on the boot-failure path.
> +	 */
>  	amd_viommu_uninit(iommu);
>  	free_iommu_buffers(iommu);
>  	amd_iommu_free_ppr_log(iommu);
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index ae44050fe86a..dfb9268f3c4e 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2509,6 +2509,16 @@ static void pdom_detach_iommu(struct amd_iommu *iommu,
>  	spin_unlock_irqrestore(&pdom->lock, flags);
>  }
>  
> +int amd_iommu_pdom_bind_iommu(struct amd_iommu *iommu, struct protection_domain *pdom)

Why not just rename pdom_attach_iommu / pdom_detach_iommu ?

> +{
> +	return pdom_attach_iommu(iommu, pdom);
> +}
> +
> +void amd_iommu_pdom_unbind_iommu(struct amd_iommu *iommu, struct protection_domain *pdom)
> +{
> +	pdom_detach_iommu(iommu, pdom);
> +}
> +
>  /*
>   * If a device is not yet associated with a domain, this function makes the
>   * device visible in the domain
> @@ -2858,8 +2868,7 @@ static const struct iommu_dirty_ops amdv1_dirty_ops = {
>  	.set_dirty_tracking = amd_iommu_set_dirty_tracking,
>  };
>  
> -static struct iommu_domain *amd_iommu_domain_alloc_paging_v1(struct device *dev,
> -							     u32 flags)
> +struct iommu_domain *amd_iommu_domain_alloc_paging_v1(struct device *dev, u32 flags)
>  {
>  	struct pt_iommu_amdv1_cfg cfg = {};
>  	struct protection_domain *domain;
> diff --git a/drivers/iommu/amd/viommu.c b/drivers/iommu/amd/viommu.c
> index c9207ed47fb6..89d6dc520b2c 100644
> --- a/drivers/iommu/amd/viommu.c
> +++ b/drivers/iommu/amd/viommu.c
> @@ -17,6 +17,7 @@
>  #include "amd_iommu.h"
>  #include "amd_iommu_types.h"
>  #include "amd_viommu.h"
> +#include "../iommu-pages.h"
>  
>  static void __init amd_viommu_vf_vfcntl_unmap(struct amd_iommu *iommu)
>  {
> @@ -39,9 +40,12 @@ static void __init amd_viommu_vf_vfcntl_unmap(struct amd_iommu *iommu)
>  	}
>  }
>  
> +static void viommu_private_space_uninit(struct amd_iommu *iommu);
> +
>  void __init amd_viommu_uninit(struct amd_iommu *iommu)
>  {
>  	iommu->flags &= ~AMD_IOMMU_FLAG_VIOMMU_EN;
> +	viommu_private_space_uninit(iommu);
>  	amd_viommu_vf_vfcntl_unmap(iommu);
>  }
>  
> @@ -110,6 +114,187 @@ static int __init viommu_vf_vfcntl_init(struct amd_iommu *iommu)
>  	return -ENOMEM;
>  }
>  
> +/*
> + * Restore WB and free @va. If WB restore fails, leak the pages so
> + * UC-MINUS direct-map PTEs and the PAT memtype reservation are not
> + * returned to the buddy allocator.
> + */
> +static void viommu_priv_wb_free(void *va, size_t size)

Better viommu_priv_free() so that it matches the alloc function?


> +{
> +	if (WARN_ON_ONCE(set_memory_wb((unsigned long)va,
> +				       size >> PAGE_SHIFT)))
> +		return;
> +	iommu_free_pages(va);
> +}
> +
> +/*
> + * Allocate backing pages, mark UC, and map at @iova in viommu_pdom.
> + * *@out_va is NULL on any failure. @gfp is used for the backing
> + * folio and iommu_map() page tables.
> + */
> +static int viommu_priv_alloc_map(struct amd_iommu *iommu, u64 iova, size_t size,
> +				 gfp_t gfp, void **out_va)
> +{
> +	int ret;
> +	void *va;
> +	int nid = iommu && iommu->dev ? dev_to_node(&iommu->dev->dev) : NUMA_NO_NODE;
> +
> +	*out_va = NULL;
> +
> +	if (!iommu || !iommu->viommu_pdom)
> +		return -EINVAL;

Redundant check as its static function and know we it gets called only if iommu
and viommu_pdom is valid.

> +
> +	va = iommu_alloc_pages_node_sz(nid, gfp, size);
> +	if (!va)
> +		return -ENOMEM;
> +
> +	/*
> +	 * IOMMU spec: backing storage must be UC. set_memory_uc()
> +	 * splits covering 2MB direct-map pages to 4K; set_memory_wb()
> +	 * does not re-coalesce them.
> +	 */
> +	ret = set_memory_uc((unsigned long)va, size >> PAGE_SHIFT);
> +	if (ret)
> +		goto err_uc;
> +
> +	ret = iommu_map(&iommu->viommu_pdom->domain, iova,
> +			iommu_virt_to_phys(va), size,
> +			IOMMU_READ | IOMMU_WRITE, gfp);
> +	if (ret)
> +		goto err_uc;
> +
> +	*out_va = va;
> +	return 0;
> +
> +err_uc:
> +	viommu_priv_wb_free(va, size);
> +	return ret;
> +}
> +
> +static bool viommu_cmdbuf_enabled(struct amd_iommu *iommu)
> +{
> +	if (!iommu->mmio_base)
> +		return false;
> +	return readq(iommu->mmio_base + MMIO_CONTROL_OFFSET) &
> +	       BIT_ULL(CONTROL_CMDBUF_EN);
> +}
> +
> +/*
> + * Unmap @iova if the command buffer is still enabled, restore WB, and
> + * free @cpu_va. Skip IOTLB invalidation after disable_iommus().
> + */
> +static void viommu_priv_unmap_free(struct amd_iommu *iommu, u64 iova, size_t size,
> +				   void *cpu_va)
> +{
> +	size_t unmapped;
> +
> +	if (!cpu_va)
> +		return;
> +	if (!iommu || !iommu->viommu_pdom)
> +		return;
> +
> +	if (viommu_cmdbuf_enabled(iommu)) {

Hmmm. Why do we need this check?  We already have AMD_IOMMU_FLAG_VIOMMU_EN to
track whether vIOMMU is enabled or not. So in amd_viommu_uninit() path, if
required we can check that flag so that we are sure command buffer was enabled.


-Vasant