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

[guanghuifeng@xxxxxxxxxxxxxxxxx]

在 2026/9/15 2:47, Suravee Suthikulpanit 写道:
> Introduce new IOMMU commands for vIOMMU to reset
> virtualized MMIO registers of a particular guest.
>
> Always program RESET_MMIO_ALL_FLAG and RESET_MMIO_VCMD_FLAG.
> Propagate iommu_completion_wait() errors to the caller.
>
> Reviewed-by: Weinan Liu <wnliu@xxxxxxxxxx>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> ---
>   drivers/iommu/amd/amd_iommu.h       |  1 +
>   drivers/iommu/amd/amd_iommu_types.h |  3 +++
>   drivers/iommu/amd/iommu.c           | 21 +++++++++++++++++++++
>   drivers/iommu/amd/iommufd.c         |  5 +++++
>   4 files changed, 30 insertions(+)
>
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 51e4364e8564..8c298e090889 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -233,4 +233,5 @@ amd_iommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
>   /* Guest ID for vIOMMU */
>   int amd_iommu_gid_alloc(struct amd_iommu *iommu);
>   void amd_iommu_gid_free(struct amd_iommu *iommu, int gid);
> +int amd_iommu_reset_vmmio(struct amd_iommu *iommu, u16 gid);
>   #endif /* AMD_IOMMU_H */
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 3aaaf69a1e7d..9eb9a0c190de 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -209,6 +209,9 @@
>   #define CMD_INV_IRT		0x05
>   #define CMD_COMPLETE_PPR	0x07
>   #define CMD_INV_ALL		0x08
> +#define CMD_RESET_VMMIO		0x0A
> +#define RESET_MMIO_ALL_FLAG	BIT(28)

RESET_MMIO_ALL_FLAG is defined at the wrong bit position?

Per spec Section 2.4.10 (RESET_VMMIO Command), the data[0] dword layout is:
Bit 31     : VCmd  - Reset virtual command buffer for the specific Guest
Bits 30:28 : Reserved
Bit 27     : All (IAll) - Reset the whole virtual IOMMU for the specific 
Guest
Bits 26:16 : Reserved
Bits 15:0  : GuestID

The "All" bit is at position 27, not 28. Bit 28 falls within the 
Reserved field (30:28).As currently coded, build_reset_vmmio() sets bit 
28 (Reserved) and leaves bit 27 (All) clear. This means:
The intended "reset the whole virtual IOMMU" semantics (including 
clearing VMVld in VF Control MMIO Offset 10h) is never actually triggered.
Writing 1 to a Reserved bit has undefined behavior per the spec - it may 
be silently ignored or trigger an ILLEGAL_COMMAND_ERROR event depending 
on the implementation.

-#define RESET_MMIO_ALL_FLAG    BIT(28)
+#define RESET_MMIO_ALL_FLAG    BIT(27)


> +#define RESET_MMIO_VCMD_FLAG	BIT(31)
>   
>   #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 fa9708af258e..94c59f503662 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -1517,6 +1517,13 @@ 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)
> +{
> +	memset(cmd, 0, sizeof(*cmd));
> +	cmd->data[0] = gid | RESET_MMIO_ALL_FLAG | RESET_MMIO_VCMD_FLAG;
> +	CMD_SET_TYPE(cmd, CMD_RESET_VMMIO);
> +}
> +
>   /*
>    * Writes the command to the IOMMUs command buffer and informs the
>    * hardware about the new command.
> @@ -1770,6 +1777,20 @@ void amd_iommu_flush_all_caches(struct amd_iommu *iommu)
>   	}
>   }
>   
> +int amd_iommu_reset_vmmio(struct amd_iommu *iommu, u16 gid)
> +{
> +	int ret;
> +	struct iommu_cmd cmd;
> +
> +	build_reset_vmmio(&cmd, gid);
> +
> +	ret = iommu_queue_command(iommu, &cmd);
> +	if (ret)
> +		return ret;
> +
> +	return iommu_completion_wait(iommu);
> +}
> +
>   /*
>    * Command send function for flushing on-device TLB
>    */
> diff --git a/drivers/iommu/amd/iommufd.c b/drivers/iommu/amd/iommufd.c
> index 7f358e57efde..2e34491e3cf7 100644
> --- a/drivers/iommu/amd/iommufd.c
> +++ b/drivers/iommu/amd/iommufd.c
> @@ -92,6 +92,11 @@ 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 */
> +	ret = amd_iommu_reset_vmmio(iommu, aviommu->gid);
> +	if (ret)
> +		goto err_init;
> +
>   	ret = iommu_copy_struct_to_user(user_data, &data,
>   					IOMMU_VIOMMU_TYPE_AMD,
>   					out_vfmmio_mmap_offset);