Re: [PATCH v6 16/24] iommu/amd: Add IOMMUFD vDevice and DevID mapping

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

On 9/30/2026 2:53 PM, Suravee Suthikulpanit wrote:
> IOMMUFD vDevice allocation supplies the guest device ID (virt_id).
> The AMD vIOMMU Device ID mapping table translates that gdev_id to the
> host device ID for virtualised IOMMU commands.
> 
> Add vdevice_init, which validates a PCI device and virt_id, then
> programs the mapping through VFCTRL via amd_viommu_set_device_mapping().
> Get the host struct device via iommufd_vdevice_to_device(). Set
> vdev->destroy to restore an idle mapping when the vDevice is removed
> while the vIOMMU is still alive.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  drivers/iommu/amd/amd_iommu_types.h | 12 +++++
>  drivers/iommu/amd/amd_viommu.h      | 10 ++++
>  drivers/iommu/amd/iommufd.c         | 52 +++++++++++++++++++
>  drivers/iommu/amd/viommu.c          | 79 ++++++++++++++++++++++++++++-
>  4 files changed, 152 insertions(+), 1 deletion(-)
> 


.../...

> +}
> +
> +/*
> + * Called from drivers/iommu/iommufd/viommu.c: iommufd_vdevice_abort()
> + *
> + * Restore an idle DevID map entry for this virt_id. Every DevID mapping
> + * table slot must have V=1 for both active and non-active devices. A
> + * non-valid entry raises INVALID_DEVICE_REQUEST when the IOMMU processes
> + * INVALIDATE_IOMMU_ALL. Use host device ID 0: that RID is the IOMMU PCI

We don't support  INVALIDATE_IOMMU_ALL command inside guest. But with current
kernel guest will endup walking entire devid/dom Id range and hitting
INVALID_DEVICE_REQUEST.

> + * function and does not normally host an endpoint.
> + */
> +static void _amd_viommu_vdevice_destroy(struct iommufd_vdevice *vdev)
> +{
> +	struct iommufd_viommu *viommu = vdev->viommu;
> +	struct amd_iommu_viommu *aviommu =
> +		container_of(viommu, struct amd_iommu_viommu, core);
> +
> +	amd_viommu_set_device_mapping(aviommu, 0, (u16)vdev->virt_id);
> +}
> +
> +/*
> + * Called from drivers/iommu/iommufd/viommu.c: iommufd_vdevice_alloc_ioctl()
> + */
> +static int _amd_viommu_vdevice_init(struct iommufd_vdevice *vdev)
> +{
> +	struct device *dev = iommufd_vdevice_to_device(vdev);
> +	struct pci_dev *pdev;
> +	struct iommufd_viommu *viommu = vdev->viommu;
> +	struct amd_iommu_viommu *aviommu = container_of(viommu, struct amd_iommu_viommu, core);
> +
> +	if (!dev_is_pci(dev))
> +		return -EINVAL;
> +	pdev = to_pci_dev(dev);
> +
> +	if (vdev->virt_id > VIOMMU_MAX_GDEVID) {
> +		pr_err("%s: Invalid virtual ID (virt_id=%#llx)\n", __func__, vdev->virt_id);
> +		return -EINVAL;
> +	}
> +
> +	pr_debug("%s: gid=%#x, hdev_id=%#x, gdev_id=%#llx\n", __func__,
> +		 aviommu->gid, pci_dev_id(pdev), vdev->virt_id);
> +
> +	amd_viommu_set_device_mapping(aviommu, pci_dev_id(pdev),
> +				      (u16)vdev->virt_id);
> +	vdev->destroy = _amd_viommu_vdevice_destroy;
> +
> +	return 0;
>  }
>  
>  /*
> @@ -149,4 +199,6 @@ static void amd_iommufd_viommu_destroy(struct iommufd_viommu *viommu)
>  static const struct iommufd_viommu_ops amd_viommu_ops = {
>  	.alloc_domain_nested = amd_iommu_alloc_domain_nested,
>  	.destroy = amd_iommufd_viommu_destroy,
> +	.vdevice_size = VDEVICE_STRUCT_SIZE(struct amd_iommu_vdevice, core),
> +	.vdevice_init = _amd_viommu_vdevice_init,
>  };
> diff --git a/drivers/iommu/amd/viommu.c b/drivers/iommu/amd/viommu.c
> index b160c70f3804..e263d63afe3d 100644
> --- a/drivers/iommu/amd/viommu.c
> +++ b/drivers/iommu/amd/viommu.c
> @@ -6,6 +6,7 @@
>  #define pr_fmt(fmt)     "AMD-Vi: " fmt
>  #define dev_fmt(fmt)    pr_fmt(fmt)
>  
> +#include <linux/delay.h>
>  #include <linux/iommu.h>
>  #include <linux/iommufd.h>
>  #include <uapi/linux/iommufd.h>
> @@ -29,7 +30,6 @@
>   * VIOMMU_MAX_GDEVID / VIOMMU_MAX_GDOMID are 16-bit guest DevID
>   * and DomID table indices (hardware mapping-table width), not GID.
>   */
> -#define VIOMMU_MAX_GDEVID		0xFFFF
>  #define VIOMMU_DEVID_MAPPING_BASE	0x1000000000ULL
>  #define VIOMMU_DEVID_MAPPING_ENTRY_SIZE	(1 << 20)
>  
> @@ -45,6 +45,8 @@ static_assert(VIOMMU_DEVID_MAPPING_ENTRY_SIZE >=
>  static_assert(VIOMMU_DOMID_MAPPING_ENTRY_SIZE >=
>  	      (VIOMMU_MAX_GDOMID + 1ULL) * sizeof(u64));
>  
> +#define VIOMMU_VFCTRL_GUEST_DID_MAP_CONTROL0_OFFSET	0x00
> +
>  static void __init amd_viommu_vf_vfcntl_unmap(struct amd_iommu *iommu)
>  {
>  	if (iommu->vfctrl_base) {
> @@ -448,6 +450,81 @@ static void __maybe_unused free_private_vm_region(struct amd_iommu *iommu, u64 *
>  	*entry = NULL;
>  }
>  
> +#define DEVID_ENTRY_GDEVID_MASK		GENMASK_ULL(61, 46)
> +#define DEVID_ENTRY_QUEUEID_MASK	GENMASK_ULL(45, 30)
> +#define DEVID_ENTRY_HDEVID_MASK		GENMASK_ULL(29, 14)
> +#define DEVID_ENTRY_WRITE		BIT_ULL(63)
> +#define DEVID_ENTRY_VALID		BIT_ULL(0)
> +
> +/*
> + * CONTROL0/CONTROL1 bit 63 (WRITE) is a doorbell: hardware copies one
> + * entry into the IPA-backed table and clears WRITE when that DMA write
> + * retires. This is not a command-buffer operation;
> + * iommu_completion_wait() does not drain it. A readq() of CONTROL_EN
> + * is also unrelated. Poll WRITE=0 before ringing a new doorbell and
> + * after each write so the table update is retired before the caller
> + * proceeds (DTE commit, unmap/free).
> + */
> +static void vfctrl_wait_write_clear(struct amd_iommu *iommu, u16 gid,
> +				    unsigned int offset)
> +{
> +	u8 __iomem *reg = VIOMMU_VFCTRL_MMIO_BASE(iommu, gid) + offset;
> +	int i;
> +
> +	for (i = 0; i < LOOP_TIMEOUT; i++) {
> +		if (!(readq(reg) & DEVID_ENTRY_WRITE))
> +			return;
> +		udelay(1);
> +	}
> +	pr_err("%s: WRITE timeout gid=%#x offset=%#x val=%#llx\n",
> +	       __func__, gid, offset, readq(reg));

Shouldn't -ETIMEDOUT propagate so that vDevice allocation fails when
the mapping was never committed?


> +}
> +
> +/*
> + * Guest DevID table update via VFCTRL CONTROL0.
> + *
> + * TODO: The driver does not support QueueID yet,
> + *       so current callers pass 0. This will be
> + *       enhanced in the future.
> + */
> +static void set_device_mapping_unlocked(struct amd_iommu *iommu, u16 gid,

You mean mapping_locked?

-Vasant