Re: [PATCH 17/24] iommu/amd: Introduce helper function for updating domain ID mapping table

["guanghuifeng@linux.alibaba.com" <guanghuifeng@linux.alibaba.com>]

在 2026/7/27 21:29, Suravee Suthikulpanit 写道:
> AMD vIOMMU hardware uses the Domain ID mapping table to map Guest Domain ID
> (GDomID) to Host Domain ID when it virtualises guest IOMMU commands.
> It uses GID and GDomID to index into the table to look up host domain ID.
>
> Linux IOMMU driver programs the table entry using VFCntlMMIO Guest Domain
> Map Control Register.
>
> Introduce amd_viommu_domain_id_update(), which is used to set the entry
> when attaching the nested device. Clearing the entry is done during VM
> destroy.
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>   drivers/iommu/amd/amd_viommu.h |  2 ++
>   drivers/iommu/amd/nested.c     |  9 ++++++-
>   drivers/iommu/amd/viommu.c     | 45 ++++++++++++++++++++++++++++++++++
>   3 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/amd/amd_viommu.h b/drivers/iommu/amd/amd_viommu.h
> index 8b57717c22a6..b6fd5ffc3b82 100644
> --- a/drivers/iommu/amd/amd_viommu.h
> +++ b/drivers/iommu/amd/amd_viommu.h
> @@ -18,6 +18,8 @@ int amd_viommu_init_one(struct amd_iommu *iommu, struct amd_iommu_viommu *viommu
>   
>   void amd_viommu_uninit_one(struct amd_iommu *iommu, struct amd_iommu_viommu *viommu);
>   
> +int amd_viommu_domain_id_update(struct amd_iommu *iommu, u16 gid,
> +				u16 hdom_id, u16 gdom_id);
>   #else
>   
>   static inline int amd_viommu_init(struct amd_iommu *iommu)
> diff --git a/drivers/iommu/amd/nested.c b/drivers/iommu/amd/nested.c
> index 6f3ae2496160..1365c67168b6 100644
> --- a/drivers/iommu/amd/nested.c
> +++ b/drivers/iommu/amd/nested.c
> @@ -254,6 +254,7 @@ static int nested_attach_device(struct iommu_domain *dom, struct device *dev,
>   				struct iommu_domain *old)
>   {
>   	struct dev_table_entry new = {0};
> +	struct nested_domain *ndom = to_ndomain(dom);
>   	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
>   	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
>   	int ret = 0;
> @@ -267,10 +268,16 @@ static int nested_attach_device(struct iommu_domain *dom, struct device *dev,
>   
>   	mutex_lock(&dev_data->mutex);
>   
> -	set_dte_nested(iommu, dom, dev_data, &new);
> +	ret = set_dte_nested(iommu, dom, dev_data, &new);
> +	if (ret)
> +		goto out_err;
>   
>   	amd_iommu_update_dte(iommu, dev_data, &new);
>   
> +	ret = amd_viommu_domain_id_update(iommu, ndom->viommu->gid,
> +					  ndom->gdom_info->hdom_id, ndom->gdom_id);
> +
> +out_err:
>   	mutex_unlock(&dev_data->mutex);
>   
>   	return ret;
> diff --git a/drivers/iommu/amd/viommu.c b/drivers/iommu/amd/viommu.c
> index 91d0dd3ac912..708f2c7496a4 100644
> --- a/drivers/iommu/amd/viommu.c
> +++ b/drivers/iommu/amd/viommu.c
> @@ -40,6 +40,8 @@
>   #define VIOMMU_DOMID_MAPPING_BASE	0x2000000000ULL
>   #define VIOMMU_DOMID_MAPPING_ENTRY_SIZE	(1 << 19)
>   
> +#define VIOMMU_VFCTRL_GUEST_DID_MAP_CONTROL1_OFFSET	0x08
> +
>   LIST_HEAD(viommu_devid_map);
>   
>   static int viommu_init_pci_vsc(struct amd_iommu *iommu)
> @@ -420,6 +422,22 @@ static void __maybe_unused free_private_vm_region(struct amd_iommu *iommu, u64 *
>   	*entry = NULL;
>   }
>   
> +static void viommu_clear_mapping(struct amd_iommu *iommu,
> +				 struct amd_iommu_viommu *aviommu)
> +{
> +	int i;
> +	u16 gid = aviommu->gid;
> +
> +	/*
> +	 * IOMMU hardware uses the domain ID mapping table to map gdom ID to hdom ID.
> +	 * If the mapping does not exist, the hardware would generate error in the event log.
> +	 * Therefore, initialize all gdom ID entries to map to parent domain ID to prevent
> +	 * unknown mapping scenario.
> +	 */
> +	for (i = 0; i <= VIOMMU_MAX_GDOMID; i++)
> +		amd_viommu_domain_id_update(iommu, gid, aviommu->parent->id, i);
> +}
> +
>   void amd_viommu_uninit_one(struct amd_iommu *iommu, struct amd_iommu_viommu *aviommu)
>   {
>   	pr_debug("%s: gid=%u\n", __func__, aviommu->gid);
> @@ -432,6 +450,7 @@ void amd_viommu_uninit_one(struct amd_iommu *iommu, struct amd_iommu_viommu *avi
>   			       VIOMMU_DOMID_MAPPING_BASE,
>   			       VIOMMU_DOMID_MAPPING_ENTRY_SIZE,
>   			       aviommu->gid);
> +	viommu_clear_mapping(iommu, aviommu);
>   }

Use-after-free in vIOMMU teardown ordering
The teardown path releases the DevID/DomID mapping table memory
BEFORE resetting the vIOMMU state:

      free_private_vm_region(iommu, &aviommu->devid_table, ...);
      free_private_vm_region(iommu, &aviommu->domid_table, ...);
      ...
      viommu_clear_mapping(iommu, aviommu);   /* too late */

    Since the IOMMU hardware may still reference these tables until the
    guest vIOMMU state is reset, freeing the backing memory first can
    result in a use-after-free by hardware.

>   
>   int amd_viommu_init_one(struct amd_iommu *iommu, struct amd_iommu_viommu *viommu)
> @@ -452,8 +471,34 @@ int amd_viommu_init_one(struct amd_iommu *iommu, struct amd_iommu_viommu *viommu
>   	if (ret)
>   		goto err_out;
>   
> +	viommu_clear_mapping(iommu, viommu);
> +
>   	return 0;
>   err_out:
>   	amd_viommu_uninit_one(iommu, viommu);
>   	return -ENOMEM;
>   }
> +
> +/*
> + * Program the DomID via VFCTRL registers
> + * This function will be called during VM init via VFIO.
> + */
> +
> + #define DOMID_ENTRY_GDOMID_MASK	GENMASK_ULL(61, 46)
> + #define DOMID_ENTRY_HDOMID_MASK	GENMASK_ULL(29, 14)
> + #define DOMID_ENTRY_VALID		BIT_ULL(0)
> + #define DOMID_ENTRY_WRITE		BIT_ULL(63)
> +
> +int amd_viommu_domain_id_update(struct amd_iommu *iommu, u16 gid,
> +				u16 hdom_id, u16 gdom_id)
> +{
> +	u64 val;
> +	u8 __iomem *vfctrl = VIOMMU_VFCTRL_MMIO_BASE(iommu, gid);
> +
> +	val = FIELD_PREP(DOMID_ENTRY_GDOMID_MASK, gdom_id) |
> +	      FIELD_PREP(DOMID_ENTRY_HDOMID_MASK, hdom_id) |
> +	      DOMID_ENTRY_WRITE | DOMID_ENTRY_VALID;
> +
> +	writeq(val, vfctrl + VIOMMU_VFCTRL_GUEST_DID_MAP_CONTROL1_OFFSET);
> +	return 0;
> +}