Re: [PATCH 18/18] amd_iommu: Do not emit I/O page fault events during replay()

[Sairaj Kodilkar]

On 4/14/2025 7:32 AM, Alejandro Jimenez wrote:

Do not emit an I/O page fault on amdvi_page_walk() when a valid mapping
is not found. The current role of amdvi_page_walk() is to be a helper
for the translate() method and ultimately the IOMMU replay()
functionality. These operations might be executed while the guest is
running, but do not necessarily correspond 1:1 with guest-initiated page
walks.

One example of such scenario is when VFIO code calls
memory_region_iommu_replay() (which ends up calling amdvi_walk_page())

I dont think memory_region_iommu_replay() will call amdvi_walk_page()
after your changes. Since you have added a replay callback
memory_region_iommu_replay should return after calling
amdvi_iommu_replay() because of following lines --->

if (imrc->replay) {
        imrc->replay(iommu_mr,n);
        return;
}

Having said that, I am not sure if there are paths (other than emulated
device write) that calls translate(). So lets keep this patch but remove
or change the example.

Regards
Sairaj Kodilkar

to sync the dirty bitmap, or after registering a new notifier. The guest
would get IO_PAGE_FAULT events for all the regions where a mapping
doesn't currently exist, which is not correct.

Note that after this change there are no users of amdvi_page_fault(),
but since the IO page fault handling will be addressed in upcoming work,
I am choosing to mark it as unused rather than deleting it.

Signed-off-by: Alejandro Jimenez <alejandro.j.jime...@oracle.com>
---
  hw/i386/amd_iommu.c | 7 ++-----
  1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index dc29a52bd845..ac7000dbafc7 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -263,8 +263,8 @@ static void amdvi_encode_event(uint64_t *evt, uint16_t 
devid, uint64_t addr,
   *
   * @addr: virtual address in translation request
   */
-static void amdvi_page_fault(AMDVIState *s, uint16_t devid,
-                             hwaddr addr, uint16_t info)
+static void __attribute__((unused))
+amdvi_page_fault(AMDVIState *s, uint16_t devid, hwaddr addr, uint16_t info)
  {
      uint64_t evt[2];

@@ -1254,9 +1254,6 @@ static void amdvi_page_walk(AMDVIAddressSpace *as, uint64_t *dte,

       */
      if ((pte == (uint64_t)-1) || (pte == (uint64_t)-2) ||
          !IOMMU_PTE_PRESENT(pte) || perms != (perms & amdvi_get_perms(pte))) {
-
-        amdvi_page_fault(as->iommu_state, as->devfn, addr, perms);
-        trace_amdvi_page_fault(addr);
          return;
      }