Re: [PATCH 03/24] iommu/amd: Detect and initialize AMD vIOMMU feature

[Vasant Hegde]

Hi Suravee,

On 7/27/2026 6:58 PM, Suravee Suthikulpanit wrote:
> The feature is advertised w/ EFR[VIOMMUSup]. Please see the AMD IOMMU
> specification[1] for more detail.
> 
> Introduce a new global variable amd_iommu_viommu, which is used to
> control the feature enablement in the driver. Currently, the feature
> is default to disabled. Once the feature is fully supported, it will be
> changed to enabled by default.
> 
> [1] https://docs.amd.com/v/u/en-US/48882_3.11_IOMMU_PUB
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> ---
>  drivers/iommu/amd/Makefile          |  2 +-
>  drivers/iommu/amd/amd_iommu.h       |  2 ++
>  drivers/iommu/amd/amd_iommu_types.h |  1 +
>  drivers/iommu/amd/amd_viommu.h      | 22 ++++++++++++++++++++++
>  drivers/iommu/amd/init.c            | 15 +++++++++++++++
>  drivers/iommu/amd/viommu.c          | 29 +++++++++++++++++++++++++++++
>  6 files changed, 70 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/iommu/amd/amd_viommu.h
>  create mode 100644 drivers/iommu/amd/viommu.c
> 
> diff --git a/drivers/iommu/amd/Makefile b/drivers/iommu/amd/Makefile


.../...

> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -34,6 +34,7 @@
>  #include <linux/crash_dump.h>
>  
>  #include "amd_iommu.h"
> +#include "amd_viommu.h"
>  #include "../irq_remapping.h"
>  #include "../iommu-pages.h"
>  
> @@ -196,6 +197,9 @@ bool amdr_ivrs_remap_support __read_mostly;
>  
>  bool amd_iommu_force_isolation __read_mostly;
>  
> +/* VIOMMU enabling flag */
> +bool amd_iommu_viommu;

may be add "__ro_after_init" ?

> +
>  unsigned long amd_iommu_pgsize_bitmap __ro_after_init = AMD_IOMMU_PGSIZES;
>  
>  enum iommu_init_state {
> @@ -2188,6 +2192,12 @@ static int __init iommu_init_pci(struct amd_iommu *iommu)
>  	if (check_feature(FEATURE_PPR) && amd_iommu_alloc_ppr_log(iommu))
>  		return -ENOMEM;
>  
> +	ret = amd_viommu_init(iommu);
> +	if (ret) {
> +		pr_err("Failed to initialize vIOMMU.\n");
> +		amd_iommu_viommu = false;
> +	}
> +
>  	if (iommu->cap & (1UL << IOMMU_CAP_NPCACHE)) {
>  		pr_info("Using strict mode due to virtualization\n");
>  		iommu_set_dma_strict();
> @@ -2281,6 +2291,9 @@ static void print_iommu_info(void)
>  		if (check_feature2(FEATURE_SEVSNPIO_SUP))
>  			pr_cont(" SEV-TIO");
>  
> +		if (check_feature(FEATURE_VIOMMU))
> +			pr_cont(" vIOMMU");
> +
>  		pr_cont("\n");
>  	}
>  
> @@ -2293,6 +2306,8 @@ static void print_iommu_info(void)
>  		pr_info("V2 page table enabled (Paging mode : %d level)\n",
>  			amd_iommu_gpt_level);
>  	}
> +	if (amd_iommu_viommu)
> +		pr_info("AMD-Vi: vIOMMU enabled\n");

pr_fmt already prepends "AMD-Vi:". Please remove it from above pr_info.




>  }
>  
>  static int __init amd_iommu_init_pci(void)
> diff --git a/drivers/iommu/amd/viommu.c b/drivers/iommu/amd/viommu.c
> new file mode 100644
> index 000000000000..f4b5f96d4785
> --- /dev/null
> +++ b/drivers/iommu/amd/viommu.c
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2023 Advanced Micro Devices, Inc.
> + */
> +
> +#define pr_fmt(fmt)     "AMD-Vi: " fmt
> +#define dev_fmt(fmt)    pr_fmt(fmt)
> +
> +#include <linux/iommu.h>
> +#include <linux/iommufd.h>
> +#include <linux/amd-iommu.h>
> +#include <uapi/linux/iommufd.h>
> +
> +#include <asm/iommu.h>
> +#include <asm/set_memory.h>
> +
> +#include "iommufd.h"
> +#include "amd_iommu.h"
> +#include "amd_iommu_types.h"
> +#include "amd_viommu.h"
> +
> +int __init amd_viommu_init(struct amd_iommu *iommu)
> +{
> +	if (!amd_iommu_viommu ||
> +	    !check_feature(FEATURE_VIOMMU))

If check_feature returns false, then return -ENOSUPPORT so that called sets
amd_iommu_viommu to false.

Also how about renaming it as "amd_iommu_viommu_present" ? Because following
patches defines another structure with same name. It works, but add confusion.

-Vasant