[PATCH 2/2] target/riscv: rvv: Fix page probe issues in vext_ldff

Max Chou posted 2 patches 2 weeks, 5 days ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
[PATCH 2/2] target/riscv: rvv: Fix page probe issues in vext_ldff
Posted by Max Chou 2 weeks, 5 days ago
Commit 17288e38bebf ("optimize the memory probing for vector
fault-only-first loads") introduced an optimization that moved from
per-element probing to a fast-path broad probe. Unfortunately it
introduced following bugs in cross-page handling:

- Wrong condition for second page probing: checked "env->vl > elems"
  instead of "env->vl > elems + env->vstart", failing to account for
  the vstart offset.

- Incorrect second page address calculation: used
  "addr + (elems << log2_esz)" instead of "addr + page_split".
  For segment loads (nf > 1), this would probe the wrong address,not
  at the page boundary.

- Wrong second page probe size: used "elems * msize" (the first page
  size) instead of calculating the remaining size as
  "(env->vl - env->vstart) * msize - page_split". This would probe
  too little memory and could miss faults.

This commit fixes these bugs by leveraging the probe_pages helper
which automatically handles cross-page memory accesses correctly.

Fixes: 17288e38bebf ("optimize the memory probing for vector fault-only-first loads.")

Signed-off-by: Max Chou <max.chou@sifive.com>
---
 target/riscv/vector_helper.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index a74ce70943..f038997095 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -772,9 +772,9 @@ vext_ldff(void *vd, void *v0, target_ulong base, CPURISCVState *env,
     uint32_t esz = 1 << log2_esz;
     uint32_t msize = nf * esz;
     uint32_t vma = vext_vma(desc);
-    target_ulong addr, addr_probe, addr_i, offset, remain, page_split, elems;
+    target_ulong addr, addr_i, offset, remain, page_split, elems;
     int mmu_index = riscv_env_mmu_index(env, false);
-    int flags, probe_flags;
+    int flags;
     void *host;
 
     VSTART_CHECK_EARLY_EXIT(env, env->vl);
@@ -788,16 +788,8 @@ vext_ldff(void *vd, void *v0, target_ulong base, CPURISCVState *env,
     }
 
     /* Check page permission/pmp/watchpoint/etc. */
-    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);
-        probe_pages(env, addr_probe, elems * msize, ra, MMU_DATA_LOAD,
-                    mmu_index, &host, &probe_flags, true);
-        flags |= probe_flags;
-    }
+    probe_pages(env, addr, (env->vl - env->vstart) * msize, ra, MMU_DATA_LOAD,
+                mmu_index, &host, &flags, true);
 
     if (flags & ~TLB_WATCHPOINT) {
         /* probe every access */
-- 
2.43.7
Re: [PATCH 2/2] target/riscv: rvv: Fix page probe issues in vext_ldff
Posted by Alistair Francis 2 weeks, 4 days ago
On Wed, Mar 18, 2026 at 11:38 AM Max Chou <max.chou@sifive.com> wrote:
>
> Commit 17288e38bebf ("optimize the memory probing for vector
> fault-only-first loads") introduced an optimization that moved from
> per-element probing to a fast-path broad probe. Unfortunately it
> introduced following bugs in cross-page handling:
>
> - Wrong condition for second page probing: checked "env->vl > elems"
>   instead of "env->vl > elems + env->vstart", failing to account for
>   the vstart offset.
>
> - Incorrect second page address calculation: used
>   "addr + (elems << log2_esz)" instead of "addr + page_split".
>   For segment loads (nf > 1), this would probe the wrong address,not
>   at the page boundary.
>
> - Wrong second page probe size: used "elems * msize" (the first page
>   size) instead of calculating the remaining size as
>   "(env->vl - env->vstart) * msize - page_split". This would probe
>   too little memory and could miss faults.
>
> This commit fixes these bugs by leveraging the probe_pages helper
> which automatically handles cross-page memory accesses correctly.
>
> Fixes: 17288e38bebf ("optimize the memory probing for vector fault-only-first loads.")
>
> Signed-off-by: Max Chou <max.chou@sifive.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/vector_helper.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index a74ce70943..f038997095 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -772,9 +772,9 @@ vext_ldff(void *vd, void *v0, target_ulong base, CPURISCVState *env,
>      uint32_t esz = 1 << log2_esz;
>      uint32_t msize = nf * esz;
>      uint32_t vma = vext_vma(desc);
> -    target_ulong addr, addr_probe, addr_i, offset, remain, page_split, elems;
> +    target_ulong addr, addr_i, offset, remain, page_split, elems;
>      int mmu_index = riscv_env_mmu_index(env, false);
> -    int flags, probe_flags;
> +    int flags;
>      void *host;
>
>      VSTART_CHECK_EARLY_EXIT(env, env->vl);
> @@ -788,16 +788,8 @@ vext_ldff(void *vd, void *v0, target_ulong base, CPURISCVState *env,
>      }
>
>      /* Check page permission/pmp/watchpoint/etc. */
> -    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);
> -        probe_pages(env, addr_probe, elems * msize, ra, MMU_DATA_LOAD,
> -                    mmu_index, &host, &probe_flags, true);
> -        flags |= probe_flags;
> -    }
> +    probe_pages(env, addr, (env->vl - env->vstart) * msize, ra, MMU_DATA_LOAD,
> +                mmu_index, &host, &flags, true);
>
>      if (flags & ~TLB_WATCHPOINT) {
>          /* probe every access */
> --
> 2.43.7
>
>