Re: [RFC PATCH 0/1] RISC-V vector fault-only-first vl truncation under plugin memory callbacks

On Tue, 2026-08-11 at 18:43 +0800, Liang Li wrote:
> Hi,
> 
> This is an RFC for a RISC-V vector fault-only-first interaction with
> plugin
> memory callbacks. I'd like feedback on whether this is considered a
> bug and
> whether the fix is at the right layer before I invest in tests/etc.
> 
> The fault-only-first load (vle*ff, e.g. vle8ff.v) decides vl by
> probing each
> element and truncating at the first fault. That probe goes through
> probe_access_flags(), which -- when a plugin registers a vcpu_mem
> callback --
> returns a non-zero "not plain RAM" flag for ordinary RAM: TLB_MMIO
> via
> force_mmio in system mode (commit 6d03226b42), or TLB_FORCE_SLOW in
> user mode.
> 
> For a normal load these flags only select the fast/slow path and the
> data is
> still read correctly. But vext_ldff() treats any flag other than
> TLB_WATCHPOINT
> as "this element faults", so the first non-first element is reported
> faulting
> and vl is truncated to 1. The loaded data stays correct, but a
> vectorized
> strlen/scan degenerates from N bytes/iter to 1 byte/iter, inflating
> the
> dynamic instruction count whenever a memory-observing plugin is
> attached.
> 
> We hit this in practice while generating SimPoint BBVs for
> SPECcpu2017
> 500.perlbench_r under a memory-observing plugin: the instruction
> stream
> diverged from the plugin-disabled baseline at perl_parse's vectorized
> strlen.
> 
> Patch 1 routes vext_ldff()'s probes through probe_access_full_mmu()
> (check_mem_cbs=false), leaving the plain-load path unchanged so
> plugin
> observation of real loads/stores is preserved. A user-mode
> counterpart of
> probe_access_full_mmu() is added (it previously only existed in
> system mode).
> 
> Reproduced on master (v11.1.0-rc3) in riscv64-linux-user. With the
> minimal
> reproducer below: enabling the plugin inflates the vle8ff execution
> count
> from 32M to 512M (~16x) and total insns from ~234M to ~3594M; with
> this
> patch applied, both counts are identical to the no-plugin baseline
> (32M).
> 
> Questions:
> 
>   1. Is the vl truncation under a memory-observing plugin a bug, or
> is
>      reflecting force_mmio/TLB_FORCE_SLOW into the FOF probe
> intentional?
>   2. Is probe_access_full_mmu() at the FOF call sites the preferred
> layer,
>      or should probe_access_flags() itself not fold plugin-induced
> force-slow
>      into its returned flags?
>   3. I noticed target/i386/tcg/access.c already works around the same
>      force-slow (the "No haddr means probe_access wants to force slow
> path"
>      comment). Would a more general fix at the probe_access_flags()
> layer be
>      preferred so each target doesn't paper over it independently?
> 
> Patch is compile-tested on riscv64 system and user modes. A proper
> tests/tcg
> case will follow once the approach is agreed.
> 
> Reproducer (two files, linux-user)
> ==================================
> 
> Below are two self-contained files verified against this tree. Build
> the
> plugin with gcc, the guest with a riscv64 cross-gcc (needs rv64gcv),
> then
> run with qemu-riscv64 built from this tree.
> 
> --- vle8ff_repro.c (plugin) ---
> /*
>  * Registers a vcpu_mem callback (the trigger) and counts vle8ff
> executions.
>  * Load with ,nocb for the baseline, ,memcb=true to enable the
> callback.
>  */
> #include <inttypes.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> #include <qemu-plugin.h>
> 
> QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> 
> static bool memcb_enable = false;
> static uint64_t vle8ff_count = 0;
> static uint64_t total_insn = 0;
> 
> static inline bool is_vle8ff(uint32_t insn)
> {
>     return (insn & 0x0007FFFF) == 0x00070087;
> }
> 
> static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t
> info,
>                      uint64_t vaddr, void *udata)
> {
>     /* empty: registering this callback is what triggers force_mmio
> */
> }
> 
> static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
> {
>     uint32_t insn = (uintptr_t)udata;
>     total_insn++;
>     if (is_vle8ff(insn)) {
>         vle8ff_count++;
>     }
> }
> 
> static void vcpu_tb_trans(struct qemu_plugin_tb *tb, void *udata)
> {
>     size_t n = qemu_plugin_tb_n_insns(tb);
>     for (size_t i = 0; i < n; i++) {
>         struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb,
> i);
>         uint32_t code = 0;
>         qemu_plugin_insn_data(insn, &code, sizeof(code));
>         qemu_plugin_register_vcpu_insn_exec_cb(insn, vcpu_insn_exec,
>                                               
> QEMU_PLUGIN_CB_NO_REGS,
>                                                (void
> *)(uintptr_t)code);
>         if (memcb_enable) {
>             qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem,
>                                              QEMU_PLUGIN_CB_NO_REGS,
>                                              QEMU_PLUGIN_MEM_RW,
> NULL);
>         }
>     }
> }
> 
> static void plugin_exit(void *p)
> {
>     fprintf(stderr, "memcb=%d total_insn=%" PRIu64 " vle8ff=%" PRIu64
> "\n",
>             memcb_enable, total_insn, vle8ff_count);
> }
> 
> QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>                                            const qemu_info_t *info,
>                                            int argc, char **argv)
> {
>     for (int i = 0; i < argc; i++) {
>         if (strstr(argv[i], "memcb") && !strstr(argv[i], "off"))
> memcb_enable = true;
>         else if (strstr(argv[i], "nocb")) memcb_enable = false;
>     }
>     qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans, NULL);
>     qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
>     return 0;
> }
> 
> --- vle8ff_asm.c (guest) ---
> #include <string.h>
> #include <stdint.h>
> volatile uint64_t sink;
> 
> static size_t my_strlen(const char *s) {
>     register long a4 asm("a4") = (long)s;
>     register long a2 asm("a2");
>     for (;;) {
>         long vl;
>         asm volatile(".option push\n.option arch,+v\n"
>                      "vsetvli %0, zero, e8, m1, ta, ma\n"
>                      "vle8ff.v v1, (%1)\n"
>                      "vmseq.vi v1, v1, 0\n"
>                      "csrr %0, vl\n"
>                      "vfirst.m %2, v1\n"
>                      ".option pop\n"
>                      : "=r"(vl), "+r"(a4), "=r"(a2) ::
> "v1","memory");
>         if (a2 < 0) { a4 += vl; continue; }
>         return (size_t)(a4 - (long)s) + a2;
>     }
> }
> 
> int main(void) {
>     char buf[256];
>     memset(buf, 'A', 255); buf[255] = 0;
>     for (int i = 0; i < 2000000; i++) sink = my_strlen(buf);
>     return (int)(sink & 1);
> }
> 
> --- Build and run ---
> # plugin (host gcc, needs glib)
> gcc -shared -fPIC -O2 -I include/plugins \
>     $(pkg-config --cflags glib-2.0) -o vle8ff_repro.so vle8ff_repro.c
> # guest (riscv64 cross-gcc, needs V extension)
> riscv64-unknown-linux-gnu-gcc -O2 -march=rv64gcv -mabi=lp64d -static
> \
>     vle8ff_asm.c -o vle8ff_asm
> # baseline vs with-callback
> qemu-riscv64 -plugin vle8ff_repro.so,nocb     vle8ff_asm
> qemu-riscv64 -plugin vle8ff_repro.so,memcb=on vle8ff_asm
> 
> On unpatched master the memcb=on run reports ~16x more vle8ff
> executions
> than nocb; with this patch both runs report the same count.
> 
> Thanks,
> Liang Li
> 
> Liang Li (1):
>   target/riscv: fix vector fault-only-first vl truncation under
> plugin
>     memory callbacks

+ Anton

Is this what you were talking about?

Alistair

> 
>  accel/tcg/user-exec.c            | 35 ++++++++++++++++++++++----
>  include/accel/tcg/probe.h        |  8 ++++--
>  target/riscv/tcg/vector_helper.c | 42 ++++++++++++++++++++++++------
> --
>  3 files changed, 68 insertions(+), 17 deletions(-)
> 
> --
> 2.51.1
>