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

[Suthikulpanit, Suravee]

On 8/19/2026 3:34 PM, guanghuifeng@xxxxxxxxxxxxxxxxx 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@xxxxxxxxxx>
> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> > ---
> >   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