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

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

On 8/19/2026 3:34 PM, guanghuifeng@linux.alibaba.com wrote:
> 
> 在 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 <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 |  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);

Actually, this is a typo in the spec. AMD will update the spec in the 
next revision. The correct bit is 28.

>> +
>>   /*
>>    * 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.

I'll handle the return of iommu_queue_command() in iommu_reset_vmmio(). 
The amd_iommu_completion_wait() does not normally handle error in this 
driver. This needs a separate clean up.

Thanks,
Suravee