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

[Suthikulpanit, Suravee]

On 9/19/2026 10:11 PM, guanghuifeng@xxxxxxxxxxxxxxxxx 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@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)

As I mentioned in the v4 here 
(https://lore.kernel.org/linux-iommu/20260727132913.22475-1-suravee.suthikulpanit@xxxxxxx/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.