On Wed, Nov 12, 2025 at 06:25:06PM +0000, Suravee Suthikulpanit wrote:
> Introduce set_dte_nested() to program guest translation settings in
> the host DTE when attaches the nested domain to a device.
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
> drivers/iommu/amd/nested.c | 69 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 69 insertions(+)
>
> diff --git a/drivers/iommu/amd/nested.c b/drivers/iommu/amd/nested.c
> index 1bbcb16abecc..eeb5d9b3a58f 100644
> --- a/drivers/iommu/amd/nested.c
> +++ b/drivers/iommu/amd/nested.c
> @@ -153,6 +153,74 @@ amd_iommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
> return ERR_PTR(ret);
> }
>
> +static void set_dte_nested(struct amd_iommu *iommu,
> + struct iommu_domain *dom,
> + struct iommu_dev_data *dev_data)
> +{
> + struct protection_domain *parent;
> + struct dev_table_entry new = {0};
> + struct nested_domain *ndom = to_ndomain(dom);
> + struct iommu_hwpt_amd_guest *gdte = &ndom->gdte;
> + struct pt_iommu_amdv1_hw_info pt_info;
> +
> + /*
> + * The nest parent domain is attached during the call to the
> + * struct iommu_ops.viommu_init(), which will be stored as part
> + * of the struct amd_iommu_viommu.parent.
> + */
> + if (WARN_ON(!ndom->viommu || !ndom->viommu->parent))
> + return;
> +
> + parent = ndom->viommu->parent;
> + amd_iommu_make_clear_dte(dev_data, &new);
> +
> + /* Retrieve the current pagetable info via the IOMMU PT API. */
> + pt_iommu_amdv1_hw_info(&parent->amdv1, &pt_info);
> +
> + /*
> + * Use nested domain ID to program DTE.
> + * See amd_iommu_alloc_domain_nested().
> + */
> + amd_iommu_set_dte_v1(dev_data, parent, ndom->gdom_info->hdom_id, &pt_info, &new);
> +
> + /* Guest PPR */
> + new.data[0] |= gdte->dte[0] & DTE_FLAG_PPR;
> +
> + /* Guest translation stuff */
> + new.data[0] |= gdte->dte[0] & (DTE_GLX | DTE_FLAG_GV | DTE_FLAG_GIOV);
> +
> + /* GCR3 table */
> + new.data[0] |= gdte->dte[0] & DTE_GCR3_14_12;
> + new.data[1] |= gdte->dte[1] & (DTE_GCR3_30_15 | DTE_GCR3_51_31);
> +
> + /* Guest paging mode */
> + new.data[2] |= gdte->dte[2] & DTE_GPT_LEVEL_MASK;
> +
> + amd_iommu_update_dte(iommu, dev_data, &new);
The functions should be consistent a "set" function should just set a
struct dev_table_entry. A set function should not call
amd_iommu_update_dte().
So either lift the amd_iommu_update_dte() (I prefer) or change the
function name?
> +}
> +
> +static int nested_attach_device(struct iommu_domain *dom, struct device *dev,
> + struct iommu_domain *old)
> +{
> + 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;
> +
> + if (WARN_ON(dom->type != IOMMU_DOMAIN_NESTED))
> + return -EINVAL;
This is not needed, the ops are for nesting they are only called by
nesting domain types.
> + mutex_lock(&dev_data->mutex);
> +
> + /* Setup DTE for nested translation and
> + * update the device table
> + */
> + set_dte_nested(iommu, dom, dev_data);
> +
> + mutex_unlock(&dev_data->mutex);
This needs to make sure there are not PASIDs enabled.
And similarly the PASID attach path needs to to check that a v1 or
blocking domain is on the rid not identiy, not nesting.
But overall this looks OK and I think the series a whole is looking
pretty good. If you fix these little things it can possibly make this
cycle?
Jason