Re: [PATCH v6 13/24] iommu/amd: Program IOMMU DTE with the private IPA domain

[Vasant Hegde]

On 9/30/2026 2:53 PM, Suravee Suthikulpanit wrote:
> The IOMMU PCI function does not go through the normal device attach
> path, so it has no iommu_dev_data. Add amd_iommu_alloc_dev_data()
> and amd_iommu_free_dev_data() for a synthetic object that is not
> interned in pci_seg->dev_data_xa. Program that DTE with the private
> IPA domain's v1 page table so the IOMMU can DMA to its own vIOMMU
> backing store.
> 
> Leave the synthetic DTE off pdom->dev_list and with no struct
> device so clone_aliases() is skipped; it is only iommu->devid.
> Rewrite it from amd_iommu_change_top() when the IPA table grows
> so later high maps remain reachable.
> 
> Do this after viommu_private_space_init() so viommu_pdom exists.
> On teardown, clear the DTE only while CONTROL_CMDBUF_EN is set,
> then free the object. After disable_iommus() the INV_DTE flush
> would time out, so skip it and still free the object. Failed
> self DTE allocation unwinds through amd_viommu_uninit().
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> ---
>  drivers/iommu/amd/amd_iommu.h       |   4 ++
>  drivers/iommu/amd/amd_iommu_types.h |   2 +
>  drivers/iommu/amd/init.c            |   7 +-
>  drivers/iommu/amd/iommu.c           | 102 +++++++++++++++++++++++++---
>  drivers/iommu/amd/viommu.c          |  57 ++++++++++++++++
>  5 files changed, 161 insertions(+), 11 deletions(-)
> 


.../...

> +
> +static bool viommu_cmdbuf_enabled(struct amd_iommu *iommu);
> +
> +static void viommu_free_self_dev_data(struct amd_iommu *iommu)
> +{
> +	struct iommu_dev_data *dev_data = iommu->viommu_dev_data;
> +	struct dev_table_entry new = {};
> +
> +	if (!dev_data)
> +		return;
> +
> +	/*
> +	 * INV_DTE needs the command buffer. Skip it after
> +	 * disable_iommus() clears CONTROL_CMDBUF_EN. The init
> +	 * unwind still flushes while the buffer is enabled.
> +	 */
> +	if (viommu_cmdbuf_enabled(iommu)) {

As mentioned in previous patch, do we really need this check?

> +		amd_iommu_make_clear_dte(iommu, dev_data->devid, &new);
> +		amd_iommu_update_dte(iommu, dev_data, &new);
> +	}
> +	amd_iommu_free_dev_data(iommu, dev_data);
> +	iommu->viommu_dev_data = NULL;
> +}
> +
> +static int viommu_alloc_self_dev_data(struct amd_iommu *iommu)
> +{
> +	struct protection_domain *pdom = iommu->viommu_pdom;
> +	struct pt_iommu_amdv1_hw_info pt_info;
> +	struct iommu_dev_data *dev_data;
> +	struct dev_table_entry new = {};
> +
> +	dev_data = amd_iommu_alloc_dev_data(iommu->devid);

I am not getting why we can't call allc_dev_data? So that we can track these
devid (including translation devid) in dev_data_xa. So that we can catch
duplicate allocations?



> +	if (!dev_data) {
> +		pr_err("%s: Failed to allocate dev_data\n", __func__);
> +		return -ENOMEM;
> +	}
> +	/*
> +	 * Synthetic DTE for iommu->devid only: no struct device, so
> +	 * amd_iommu_update_dte() skips clone_aliases(), and not on
> +	 * pdom->dev_list (not an IOMMU-API attach).
> +	 */
> +	dev_data->dev = NULL;

I am not able to figureout why we cannot track iommu device structure here. Does
it mess up anything? if we hook this to pdom->dev_list then we don't need
special update_viommu_self_dte().


-Vasant