Re: [PATCH 06/24] iommu/amd: Map vIOMMU VF and VF Control MMIO BARs

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

Hi Suravee,


On 7/27/2026 6:58 PM, Suravee Suthikulpanit wrote:
> Enable hardware vIOMMU on an IOMMU by locating its PCI vendor-specific
> capability (VSC), reading the VF and VF Control BAR addresses, and
> mapping them for host access (256MB VF, 4MB VF Control).
> 
> VF Control covers the first 4K of guest IOMMU MMIO (control registers,
> trapped by QEMU). VF MMIO covers the third 4K (virtualized by the
> IOMMU). Per-guest bases use the Guest ID from the previous patch.
> 
> Initialize the per-amd_iommu gid_ida here so amd_iommu_gid_alloc() can
> run when IOMMUFD creates a vIOMMU instance. Export MMIO map helpers and
> call amd_viommu_uninit() from IOMMU teardown.
> 
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  drivers/iommu/amd/amd_iommu.h       |   2 +
>  drivers/iommu/amd/amd_iommu_types.h |  34 ++++++++
>  drivers/iommu/amd/amd_viommu.h      |   6 ++
>  drivers/iommu/amd/init.c            |   5 +-
>  drivers/iommu/amd/viommu.c          | 124 ++++++++++++++++++++++++++++
>  5 files changed, 169 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 9f2a1a8a6d3c..044bc9a634a1 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -28,6 +28,8 @@ void amd_iommu_set_rlookup_table(struct amd_iommu *iommu, u16 devid);
>  void iommu_feature_enable(struct amd_iommu *iommu, u8 bit);
>  void *__init iommu_alloc_4k_pages(struct amd_iommu *iommu,
>  				  gfp_t gfp, size_t size);
> +u8 __iomem * __init iommu_map_mmio_space(u64 address, u64 end);
> +void __init iommu_unmap_mmio_space(struct amd_iommu *iommu);
>  
>  #ifdef CONFIG_AMD_IOMMU_DEBUGFS
>  void amd_iommu_debugfs_setup(void);
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 00f964d5b149..e88e0bacd1a9 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -40,6 +40,12 @@
>  #define MMIO_RANGE_OFFSET	0x0c
>  #define MMIO_MISC_OFFSET	0x10
>  
> +/* vIOMMU Capability offsets (from IOMMU Capability Header) */
> +#define MMIO_VSC_VF_BAR_LO_OFFSET	0x08
> +#define MMIO_VSC_VF_BAR_HI_OFFSET	0x0c
> +#define MMIO_VSC_VF_CNTL_BAR_LO_OFFSET	0x10
> +#define MMIO_VSC_VF_CNTL_BAR_HI_OFFSET	0x14
> +
>  /* Masks, shifts and macros to parse the device range capability */
>  #define MMIO_RANGE_LD_MASK	0xff000000
>  #define MMIO_RANGE_FD_MASK	0x00ff0000
> @@ -473,6 +479,20 @@ extern bool amdr_ivrs_remap_support;
>  #define for_each_ivhd_dte_flags(entry) \
>  	list_for_each_entry((entry), &amd_ivhd_dev_flags_list, list)
>  
> +/* VIOMMU stuff */
> +#define VIOMMU_VF_MMIO_ENTRY_SIZE		4096
> +#define VIOMMU_VFCTRL_MMIO_ENTRY_SIZE		64
> +
> +/* Host ioremap/request_mem_region sizes for VF / VF_CNTL BARs */
> +#define VIOMMU_VF_MMIO_MAP_SIZE		0x10000000UL
> +#define VIOMMU_VF_CNTL_MMIO_MAP_SIZE	0x400000UL
> +
> +#define VIOMMU_VF_MMIO_BASE(iommu, guestId) \
> +	(iommu->vf_base + (guestId * VIOMMU_VF_MMIO_ENTRY_SIZE))
> +
> +#define VIOMMU_VFCTRL_MMIO_BASE(iommu, guestId) \
> +	(iommu->vfctrl_base + (guestId * VIOMMU_VFCTRL_MMIO_ENTRY_SIZE))
> +
>  struct amd_iommu;
>  struct iommu_domain;
>  struct irq_domain;
> @@ -686,6 +706,20 @@ struct amd_iommu {
>  	 */
>  	u16 cap_ptr;
>  
> +	/* Vendor-Specific Capability (VSC) pointer. */
> +	u16 vsc_offset;

I think we can remove tracking vsc_offset (see below).

> +
> +	/*
> +	 * VF MMIO base physical address. This is needed to calculate/pass
> +	 * per guest VF MMIO address (3rd 4K of IOMMU MMIO space)
> +	 */
> +	u64 vf_base_phys;
> +	u64 vf_cntl_phys;
> +
> +	/* virtual addresses of vIOMMU VF/VF_CNTL BAR */
> +	u8 __iomem *vf_base;
> +	u8 __iomem *vfctrl_base;
> +
>  	/* pci domain of this IOMMU */
>  	struct amd_iommu_pci_seg *pci_seg;
>  
> diff --git a/drivers/iommu/amd/amd_viommu.h b/drivers/iommu/amd/amd_viommu.h
> index f08ab9ef23a9..d0c4fdd00809 100644


.../...


> diff --git a/drivers/iommu/amd/viommu.c b/drivers/iommu/amd/viommu.c
> index f4b5f96d4785..014ae16bf58b 100644
> --- a/drivers/iommu/amd/viommu.c
> +++ b/drivers/iommu/amd/viommu.c
> @@ -7,9 +7,15 @@
>  #define dev_fmt(fmt)    pr_fmt(fmt)
>  
>  #include <linux/iommu.h>
> +#include <linux/amd-iommu.h>
> +
> +#include <linux/fs.h>
> +#include <linux/cdev.h>
> +#include <linux/ioctl.h>
>  #include <linux/iommufd.h>
>  #include <linux/amd-iommu.h>
>  #include <uapi/linux/iommufd.h>
> +#include <linux/mem_encrypt.h>
>  
>  #include <asm/iommu.h>
>  #include <asm/set_memory.h>
> @@ -18,12 +24,130 @@
>  #include "amd_iommu.h"
>  #include "amd_iommu_types.h"
>  #include "amd_viommu.h"
> +#include "../iommu-pages.h"
> +
> +LIST_HEAD(viommu_devid_map);
> +
> +static int viommu_init_pci_vsc(struct amd_iommu *iommu)

May be viommu_find_pci_vsc as its not initializing, instead find the offset.

Also if we pass vsc_offset to viommu_vf_vfcntl_init() -OR- call this function
from viommu_vf_vfcntl_init() then we can skip tracking `vsc_offset` in amd_iommu
structure.


> +{
> +	iommu->vsc_offset = pci_find_capability(iommu->dev, PCI_CAP_ID_VNDR);
> +	if (!iommu->vsc_offset)
> +		return -ENODEV;
> +
> +	DUMP_printk("device:%s, vsc offset:%04x\n",
> +		    pci_name(iommu->dev), iommu->vsc_offset);
> +	return 0;
> +}
> +
> +static void amd_viommu_gid_ida_init(struct amd_iommu *iommu)
> +{
> +	ida_init(&iommu->gid_ida);
> +	iommu->gid_ida_inited = true;

Once we remove `gid_ida_inited` may be we can call `ida_init` inside
amd_viommu_init() itself.

> +}
> +
> +static void amd_viommu_gid_ida_fini(struct amd_iommu *iommu)
> +{
> +	if (!iommu->gid_ida_inited)
> +		return;
> +
> +	ida_destroy(&iommu->gid_ida);
> +	iommu->gid_ida_inited = false;
> +}
> +
> +static void __init amd_viommu_vf_vfcntl_unmap(struct amd_iommu *iommu)
> +{
> +	if (iommu->vfctrl_base) {
> +		iounmap(iommu->vfctrl_base);
> +		iommu->vfctrl_base = NULL;
> +	}
> +	if (iommu->vf_cntl_phys)
> +		release_mem_region(iommu->vf_cntl_phys, VIOMMU_VF_CNTL_MMIO_MAP_SIZE);
> +
> +	if (iommu->vf_base) {
> +		iounmap(iommu->vf_base);
> +		iommu->vf_base = NULL;
> +	}
> +	if (iommu->vf_base_phys)
> +		release_mem_region(iommu->vf_base_phys, VIOMMU_VF_MMIO_MAP_SIZE);
> +}
> +
> +void __init amd_viommu_uninit(struct amd_iommu *iommu)
> +{
> +	amd_viommu_gid_ida_fini(iommu);
> +	amd_viommu_vf_vfcntl_unmap(iommu);
> +}
> +
> +static int __init viommu_vf_vfcntl_init(struct amd_iommu *iommu)
> +{
> +	u32 lo, hi;
> +	u64 vf_phys, vf_cntl_phys;
> +
> +	/* Setting up VF and VF_CNTL MMIOs */
> +	pci_read_config_dword(iommu->dev, iommu->vsc_offset + MMIO_VSC_VF_BAR_LO_OFFSET, &lo);
> +	pci_read_config_dword(iommu->dev, iommu->vsc_offset + MMIO_VSC_VF_BAR_HI_OFFSET, &hi);
> +	vf_phys = hi;
> +	vf_phys = (vf_phys << 32) | lo;
> +	if (!(vf_phys & 1)) {
> +		pr_err(FW_BUG "vf_phys disabled\n");
> +		return -EINVAL;
> +	}
> +
> +	pci_read_config_dword(iommu->dev, iommu->vsc_offset + MMIO_VSC_VF_CNTL_BAR_LO_OFFSET, &lo);
> +	pci_read_config_dword(iommu->dev, iommu->vsc_offset + MMIO_VSC_VF_CNTL_BAR_HI_OFFSET, &hi);
> +	vf_cntl_phys = hi;
> +	vf_cntl_phys = (vf_cntl_phys << 32) | lo;
> +	if (!(vf_cntl_phys & 1)) {
> +		pr_err(FW_BUG "vf_cntl_phys disabled\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!vf_phys || !vf_cntl_phys) {
> +		pr_err(FW_BUG "AMD-Vi: Unassigned VF resources.\n");
> +		return -ENOMEM;
> +	}

Redundant check as previous check confirms both vf_phys and vf_cntl_phys is
enabled. If we want to check non-zero address then we have to remove bit zero
and add check


> +
> +	/* Mapping 256MB of VF and 4MB of VF_CNTL BARs */
> +	vf_phys &= ~1ULL;
> +	iommu->vf_base = iommu_map_mmio_space(vf_phys, VIOMMU_VF_MMIO_MAP_SIZE);
> +	if (!iommu->vf_base) {
> +		pr_err("Can't reserve vf_base\n");
> +		return -ENOMEM;
> +	}
> +	iommu->vf_base_phys = vf_phys;
> +
> +	vf_cntl_phys &= ~1ULL;
> +	iommu->vfctrl_base = iommu_map_mmio_space(vf_cntl_phys, VIOMMU_VF_CNTL_MMIO_MAP_SIZE);
> +	if (!iommu->vfctrl_base) {
> +		pr_err("Can't reserve vfctrl_base\n");
> +		goto err_out;
> +	}
> +	iommu->vf_cntl_phys = vf_cntl_phys;
> +
> +	pr_debug("%s: IOMMU device:%s, vf_base:%#llx, vfctrl_base:%#llx\n",

better DUMP_printk ?

> +		 __func__, pci_name(iommu->dev), vf_phys, vf_cntl_phys);
> +	return 0;
> +err_out:
> +	amd_viommu_uninit(iommu);

Its odd. We shouldn't call amd_viommu_uninit from here. May be call
amd_viommu_vf_vfcntl_unmap() ?


-Vasant