Re: [PATCH 1/1] [RISC-V/RVV] Expand the probe_pages helper function to handle probe flags.

[Daniel Henrique Barboza]

On 2/21/25 1:20 PM, Paolo Savini wrote:

This commit expands the probe_pages helper function in
target/riscv/vector_helper.c to handle also the cases in which we need access to
the flags raised while probing the memory and the host address.
This is done in order to provide a unified interface to probe_access and
probe_access_flags.
The new version of probe_pages can now act as a regular call to probe_access as
before and as a call to probe_access_flags. In the latter case the user need to
pass pointers to flags and host address and a boolean value for nonfault.
The flags and host address will be set and made available as for a direct call
to probe_access_flags.

Signed-off-by: Paolo Savini <paolo.sav...@embecosm.com>
---

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

  target/riscv/vector_helper.c | 57 +++++++++++++++++++++++-------------
  1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 772cff8fbe..c0f1b7994e 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -114,25 +114,42 @@ static inline uint32_t vext_max_elems(uint32_t desc, 
uint32_t log2_esz)
   * It will trigger an exception if there is no mapping in TLB
   * and page table walk can't fill the TLB entry. Then the guest
   * software can return here after process the exception or never return.
+ *
+ * This function can also be used when direct access to probe_access_flags is
+ * needed in order to access the flags. If a pointer to a flags operand is
+ * provided the function will call probe_access_flags instead, use nonfault
+ * and update host and flags.
   */
-static void probe_pages(CPURISCVState *env, target_ulong addr,
-                        target_ulong len, uintptr_t ra,
-                        MMUAccessType access_type)
+static void probe_pages(CPURISCVState *env, target_ulong addr, target_ulong 
len,
+                        uintptr_t ra, MMUAccessType access_type, int mmu_index,
+                        void **host, int *flags, bool nonfault)
  {
      target_ulong pagelen = -(addr | TARGET_PAGE_MASK);
      target_ulong curlen = MIN(pagelen, len);
-    int mmu_index = riscv_env_mmu_index(env, false);

- probe_access(env, adjust_addr(env, addr), curlen, access_type,

-                 mmu_index, ra);
+    if (flags != NULL) {
+        *flags = probe_access_flags(env, adjust_addr(env, addr), curlen,
+                                    access_type, mmu_index, nonfault, host, 
ra);
+    } else {
+        probe_access(env, adjust_addr(env, addr), curlen, access_type,
+                     mmu_index, ra);
+    }
+
      if (len > curlen) {
          addr += curlen;
          curlen = len - curlen;
-        probe_access(env, adjust_addr(env, addr), curlen, access_type,
-                     mmu_index, ra);
+        if (flags != NULL) {
+            *flags = probe_access_flags(env, adjust_addr(env, addr), curlen,
+                                        access_type, mmu_index, nonfault,
+                                        host, ra);
+        } else {
+            probe_access(env, adjust_addr(env, addr), curlen, access_type,
+                         mmu_index, ra);
+        }
      }
  }

+

  static inline void vext_set_elem_mask(void *v0, int index,
                                        uint8_t value)
  {
@@ -332,8 +349,8 @@ vext_page_ldst_us(CPURISCVState *env, void *vd, 
target_ulong addr,
      MMUAccessType access_type = is_load ? MMU_DATA_LOAD : MMU_DATA_STORE;

/* Check page permission/pmp/watchpoint/etc. */

-    flags = probe_access_flags(env, adjust_addr(env, addr), size, access_type,
-                               mmu_index, true, &host, ra);
+    probe_pages(env, addr, size, ra, access_type, mmu_index, &host, &flags,
+                true);

if (flags == 0) {

          if (nf == 1) {
@@ -635,7 +652,7 @@ vext_ldff(void *vd, void *v0, target_ulong base, 
CPURISCVState *env,
      uint32_t vma = vext_vma(desc);
      target_ulong addr, addr_probe, addr_i, offset, remain, page_split, elems;
      int mmu_index = riscv_env_mmu_index(env, false);
-    int flags;
+    int flags, probe_flags;
      void *host;

VSTART_CHECK_EARLY_EXIT(env);

@@ -649,15 +666,15 @@ vext_ldff(void *vd, void *v0, target_ulong base, 
CPURISCVState *env,
      }

/* Check page permission/pmp/watchpoint/etc. */

-    flags = probe_access_flags(env, adjust_addr(env, addr), elems * msize,
-                               MMU_DATA_LOAD, mmu_index, true, &host, ra);
+    probe_pages(env, addr, elems * msize, ra, MMU_DATA_LOAD, mmu_index, &host,
+                &flags, true);

/* If we are crossing a page check also the second page. */

      if (env->vl > elems) {
          addr_probe = addr + (elems << log2_esz);
-        flags |= probe_access_flags(env, adjust_addr(env, addr_probe),
-                                    elems * msize, MMU_DATA_LOAD, mmu_index,
-                                    true, &host, ra);
+        probe_pages(env, addr_probe, elems * msize, ra, MMU_DATA_LOAD,
+                    mmu_index, &host, &probe_flags, true);
+        flags |= probe_flags;
      }

if (flags & ~TLB_WATCHPOINT) {

@@ -669,16 +686,16 @@ vext_ldff(void *vd, void *v0, target_ulong base, 
CPURISCVState *env,
              addr_i = adjust_addr(env, base + i * (nf << log2_esz));
              if (i == 0) {
                  /* Allow fault on first element. */
-                probe_pages(env, addr_i, nf << log2_esz, ra, MMU_DATA_LOAD);
+                probe_pages(env, addr_i, nf << log2_esz, ra, MMU_DATA_LOAD,
+                            mmu_index, &host, NULL, false);
              } else {
                  remain = nf << log2_esz;
                  while (remain > 0) {
                      offset = -(addr_i | TARGET_PAGE_MASK);

/* Probe nonfault on subsequent elements. */

-                    flags = probe_access_flags(env, addr_i, offset,
-                                               MMU_DATA_LOAD, mmu_index, true,
-                                               &host, 0);
+                    probe_pages(env, addr_i, offset, 0, MMU_DATA_LOAD,
+                                mmu_index, &host, &flags, true);

/*

                       * Stop if invalid (unmapped) or mmio (transaction may