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

["Suthikulpanit, Suravee" <suravee.suthikulpanit@amd.com>]

On 9/19/2026 10:11 PM, guanghuifeng@linux.alibaba.com wrote:
> 
> 
> 在 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@google.com>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> ---
>>   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)

As I mentioned in the v4 here 
(https://lore.kernel.org/linux-iommu/20260727132913.22475-1-suravee.suthikulpanit@amd.com/T/#m6d0d1670600e1217b6138fce7d936805326d9bb6), 
there is a typo in the spec, which will be corrected in the next 
revision. The correct position for ALL bit is 28.

Thanks,
Suravee.