Re: [PATCH 08/24] iommu/amd: Introduce Reset vMMIO Command

在 2026/7/27 21:28, Suravee Suthikulpanit 写道:
> Introduce new IOMMU commands for vIOMMU to reset
> virtualized MMIO registers of a particular guest.
>
> Reviewed-by: Weinan Liu <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
>   drivers/iommu/amd/amd_iommu.h       |  1 +
>   drivers/iommu/amd/amd_iommu_types.h |  1 +
>   drivers/iommu/amd/iommu.c           | 22 ++++++++++++++++++++++
>   drivers/iommu/amd/iommufd.c         |  3 +++
>   4 files changed, 27 insertions(+)
>
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 044bc9a634a1..2ce207529ea0 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -11,6 +11,7 @@
>   
>   #include "amd_iommu_types.h"
>   
> +void iommu_reset_vmmio(struct amd_iommu *iommu, u16 gid);
>   extern int amd_iommu_evtlog_size;
>   extern int amd_iommu_pprlog_size;
>   
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index cc7049bbfa14..44fa1d6c64d6 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -218,6 +218,7 @@
>   #define CMD_INV_IRT		0x05
>   #define CMD_COMPLETE_PPR	0x07
>   #define CMD_INV_ALL		0x08
> +#define CMD_RESET_VMMIO		0x0A
>   
>   #define CMD_COMPL_WAIT_STORE_MASK	0x01
>   #define CMD_COMPL_WAIT_INT_MASK		0x02
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 73fba8be40d1..6f5ecc48f4ad 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -1428,6 +1428,18 @@ static void build_inv_irt(struct iommu_cmd *cmd, u16 devid)
>   	CMD_SET_TYPE(cmd, CMD_INV_IRT);
>   }
>   
> +static void build_reset_vmmio(struct iommu_cmd *cmd, u16 gid,
> +			      bool vcmd, bool all)
> +{
> +	memset(cmd, 0, sizeof(*cmd));
> +	cmd->data[0] = gid;
> +	if (all)
> +		cmd->data[0] |= (1 << 28);
> +	if (vcmd)
> +		cmd->data[0] |= (1 << 31);
> +	CMD_SET_TYPE(cmd, CMD_RESET_VMMIO);
> +}

This looks like a bug. Per the AMD IOMMU Specification (Rev 3.10,
Section 2.4.10), the 'All' bit of the RESET_VMMIO command is at bit
position 27, not bit 28. Writing bit 28 targets a reserved field and
may either trigger an ILLEGAL_COMMAND_ERROR or silently fail to reset
the full vIOMMU context. This should be:

     if (all)
         cmd->data[0] |= (1 << 27);


> +
>   /*
>    * Writes the command to the IOMMUs command buffer and informs the
>    * hardware about the new command.
> @@ -1668,6 +1680,16 @@ void amd_iommu_flush_all_caches(struct amd_iommu *iommu)
>   	}
>   }
>   
> +void iommu_reset_vmmio(struct amd_iommu *iommu, u16 gid)
> +{
> +	struct iommu_cmd cmd;
> +
> +	build_reset_vmmio(&cmd, gid, 1, 1);
> +
> +	iommu_queue_command(iommu, &cmd);
> +	amd_iommu_completion_wait(iommu);
> +}
> +

Both iommu_queue_command() and amd_iommu_completion_wait() return
status, but iommu_reset_vmmio() returns void and ignores them. A
failed reset during vIOMMU init would go unnoticed. Consider
propagating the error to the caller.


>   /*
>    * Command send function for flushing on-device TLB
>    */
> diff --git a/drivers/iommu/amd/iommufd.c b/drivers/iommu/amd/iommufd.c
> index 9f76bfa1d6ea..81e0244348a2 100644
> --- a/drivers/iommu/amd/iommufd.c
> +++ b/drivers/iommu/amd/iommufd.c
> @@ -80,6 +80,9 @@ int amd_iommufd_viommu_init(struct iommufd_viommu *viommu, struct iommu_domain *
>   
>   	data.out_vfmmio_mmap_offset = aviommu->vfmmio_mmap_offset;
>   
> +	/* Reset vIOMMU MMIOs to initialize the vIOMMU */
> +	iommu_reset_vmmio(iommu, aviommu->gid);
> +
>   	ret = iommu_copy_struct_to_user(user_data, &data,
>   					IOMMU_VIOMMU_TYPE_AMD,
>   					out_vfmmio_mmap_offset);