Re: [PATCH 1/2] amd_iommu: Honor DTE[IR] and DTE[IW] when DTE[Mode] is 0

Hello Alejandro,

Thank you for the detailed review!

On Thu, 6 Aug 2026 at 15:57, Alejandro Jimenez
<[email protected]> wrote:

> Thank you for the patches and the detailed description in the commit
> message. While the problem you are describing is valid, I think the
> solution in this series only addresses it partially. If my understanding is
> correct, your proposed Mode=0 handling covers a scenario like the following:
>
> - an earlier INVALIDATE_DEVTAB_ENTRY read DTE[Mode]>0 and set
> addr_translation = true
> - guest changes DTE in RAM to Mode=0
> - The corresponding DTE invalidation has not yet been received/processed by
> QEMU, so addr_translation still reflects the older DTE with Mode>0
> - a DMA request misses the IOTLB and triggers a page walk
> - amdvi_do_translate() reads the new Mode=0 DTE, which conflicts with the
> older DTE state cached in addr_translation = true
>
> I think the reverse scenario can also happen where a previous
> addr_translation = false can select the passthrough code path while the
> guest memory already has a DTE[Mode]>0.

You're right. The approach that you suggest:

> The approach I am considering is to cache the relevant DTE state (i.e. only
> the address translation related state) for each address space, and update
> it when processing INVALIDATE_DEVTAB_ENTRY or a global invalidation
> (INVALIDATE_IOMMU_ALL). THis way we avoid reading the DTE from guest memory
> at various paths, and instead use the cached version, so the emulation code
> makes decisions based on a consistent state.

would solve this problem for the part of the code that I've touched, and other
parts too.

> Based on my reading of the AMDVi documentation, the OS must issue
> INVALIDATE_DEVTAB_ENTRY after modifying a DTE before it can rely on any the
> updated field(s) being used. A guest can change the Device Table in its
> RAM, but the synchronization point where the change starts affecting
> behavior in the HW (or emulated HW in this case) is after the next
> INVALIDATE_DEVTAB_ENTRY command.

By the way, as I understand per the AMDVi specification, it would not be
required to invalidate DTEs if address translation or interrupt translation
information is changed, as long as V=0 and IV=0 before the change. However,
I feel that emulating this will be complex, since the Device Table writes will
have to be intercepted. Moreover, Linux (and probably many other OSes) seem
to always flush DTEs, so implementing this wouldn't be necessary.

> There are other issues that the patch doesn't address e.g. for a VFIO
> device, the replay() callback does not handle Mode=0. It calls
> amdvi_sync_shadow_page_table_range()->fetch_pte() which rejects Mode=0, so
> no MAP notifications would be generated in case of DTEs with RO or WO
> permissions.

Oh, okay. I was not aware of this.

> I think what I describe above should be a prerequisite for your patch to
> then implement consistent handling of IR/IW permissions with Mode=0. It is
> a much larger change, so I am still working through it, but you are welcome
> to contribute now that I have hopefully explained the idea :).

Sure! I am willing to make a V2 of this series, fixing the replay callback and
taking advantage of the cached DTE, once you have implemented it.

Cheers,
Daniel