Re: [PATCH v2 17/18] hw/misc: riscv_wgchecker: Check the slot settings in translate

[Daniel Henrique Barboza]

On 4/17/25 7:52 AM, Jim Shu wrote:

The final part of wgChecker we need to implement is actually using the
wgChecker slots programmed by guest to determine whether to block the
transaction or not.

Since this means we now change transaction mappings when
the guest writes to wgChecker slots, we must also call the IOMMU
notifiers at that point.

One tricky part here is that the perm of 'blocked_io_as' is the
condition of deny access. For example, if wgChecker only permits RO
access, the perm of 'downstream_as' will be IOMMU_RO and the perm of
'blocked_io_as' will be IOMMU_WO.

Signed-off-by: Jim Shu <jim....@sifive.com>
---

Reviewed-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>

  hw/misc/riscv_wgchecker.c | 70 ++++++++++++++++++++++++++++++++++++---
  hw/misc/trace-events      |  1 +
  2 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/hw/misc/riscv_wgchecker.c b/hw/misc/riscv_wgchecker.c
index 5d2af7946f..5a70231837 100644
--- a/hw/misc/riscv_wgchecker.c
+++ b/hw/misc/riscv_wgchecker.c
@@ -100,6 +100,52 @@ REG32(SLOT_CFG,             0x010)
  #define P_READ                  (1 << 0)
  #define P_WRITE                 (1 << 1)

+static IOMMUAccessFlags wgc_perm_to_iommu_flags(int wgc_perm)

+{
+    if (wgc_perm == (P_READ | P_WRITE)) {
+        return IOMMU_RW;
+    } else if (wgc_perm & P_WRITE) {
+        return IOMMU_WO;
+    } else if (wgc_perm & P_READ) {
+        return IOMMU_RO;
+    } else {
+        return IOMMU_NONE;
+    }
+}
+
+static void wgchecker_iommu_notify_all(RISCVWgCheckerState *s)
+{
+    /*
+     * Do tlb_flush() to whole address space via memory_region_notify_iommu()
+     * when wgChecker changes it's config.
+     */
+
+    IOMMUTLBEvent event = {
+        .entry = {
+            .addr_mask = -1ULL,
+        }
+    };
+
+    trace_riscv_wgc_iommu_notify_all();
+
+    for (int i=0; i<WGC_NUM_REGIONS; i++) {
+        WgCheckerRegion *region = &s->mem_regions[i];
+        uint32_t nworlds = worldguard_config->nworlds;
+
+        if (!region->downstream) {
+            continue;
+        }
+        event.entry.iova = 0;
+        event.entry.translated_addr = 0;
+        event.type = IOMMU_NOTIFIER_UNMAP;
+        event.entry.perm = IOMMU_NONE;
+
+        for (int wid=0; wid<nworlds; wid++) {
+            memory_region_notify_iommu(&region->upstream, wid, event);
+        }
+    }
+}
+
  static void decode_napot(hwaddr a, hwaddr *sa, hwaddr *ea)
  {
      /*
@@ -309,6 +355,9 @@ static IOMMUTLBEntry riscv_wgc_translate(IOMMUMemoryRegion 
*iommu,
  {
      WgCheckerRegion *region = container_of(iommu, WgCheckerRegion, upstream);
      RISCVWgCheckerState *s = RISCV_WGCHECKER(region->wgchecker);
+    bool is_write;
+    WgAccessResult result;
+    int wgc_perm;
      hwaddr phys_addr;
      uint64_t region_size;

@@ -327,18 +376,25 @@ static IOMMUTLBEntry riscv_wgc_translate(IOMMUMemoryRegion *iommu,

       * Look at the wgChecker configuration for this address, and
       * return a TLB entry directing the transaction at either
       * downstream_as or blocked_io_as, as appropriate.
-     * For the moment, always permit accesses.
       */

/* Use physical address instead of offset */

      phys_addr = addr + region->region_offset;
+    is_write = (flags == IOMMU_WO);

- is_success = true;

+    result = wgc_check_access(s, phys_addr, iommu_idx, is_write);

trace_riscv_wgc_translate(phys_addr, flags,

-        iommu_idx, is_success ? "pass" : "block");
+        iommu_idx, result.is_success ? "pass" : "block");

- ret.target_as = is_success ? &region->downstream_as : &region->blocked_io_as;

+    wgc_perm = result.perm;
+    if (!result.is_success) {
+        /* if target_as is blocked_io_as, the perm is the condition of deny 
access. */
+        wgc_perm ^= (P_READ | P_WRITE);
+    }
+    ret.perm = wgc_perm_to_iommu_flags(wgc_perm);
+
+    ret.target_as = result.is_success ? &region->downstream_as : 
&region->blocked_io_as;
      return ret;
  }

@@ -604,6 +660,9 @@ static void riscv_wgchecker_writeq(void *opaque, hwaddr addr,

              break;
          }

+ /* Flush softmmu TLB when wgChecker changes config. */

+        wgchecker_iommu_notify_all(s);
+
          return;
      }

@@ -699,6 +758,9 @@ static void riscv_wgchecker_writel(void *opaque, hwaddr addr,

              break;
          }

+ /* Flush softmmu TLB when wgChecker changes config. */

+        wgchecker_iommu_notify_all(s);
+
          return;
      }

diff --git a/hw/misc/trace-events b/hw/misc/trace-events

index b1d8538220..54dfcd50a1 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -403,3 +403,4 @@ riscv_wgchecker_mmio_write(uint64_t base, uint64_t offset, 
unsigned int size, ui
  riscv_wgc_mem_blocked_read(uint64_t addr, unsigned size, uint32_t wid) "wgChecker blocked 
read: offset 0x%" PRIx64 " size %u wid %" PRIu32
  riscv_wgc_mem_blocked_write(uint64_t addr, uint64_t data, unsigned size, uint32_t wid) "wgChecker 
blocked write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u wid %" PRIu32
  riscv_wgc_translate(uint64_t addr, int flags, int wid, const char *res) "wgChecker 
translate: addr 0x%016" PRIx64 " flags 0x%x wid %d: %s"
+riscv_wgc_iommu_notify_all(void) "wgChecker iommu: notifying UNMAP for all"