Re: [PATCH 11/23] target/riscv: Rewrite vext_ldff

On 2026-08-25 13:15, Richard Henderson wrote:
> On 8/25/26 10:58, Max Chou wrote:
> 
> > On 2026-08-15 12:45, Richard Henderson wrote:
> > > Do not call probe_pages for every active element.
> > > We can make do with no more than 2 such calls for
> > > the two pages the insn might reference.
> > > 
> > > Signed-off-by: Richard Henderson <[email protected]>
> > > +    /*
> > > +     * Test whether the first page is accessible.
> > > +     * If the first element is active, it must succeed.
> > > +     */
> > > +    flags = probe_access_flags(env, adjust_addr(env, addr),
> > > +                               MIN(last, last_in_page) - addr + 1,
> > > +                               MMU_DATA_LOAD, mmu_index, !first_active,
> > > +                               &host, ra);
> > Hi Richard,
> > 
> > This probe traverses every byte from the initial active element to the
> > end of the page, not just the bytes of active elements. In the masked
> > case, the range may encompass a masked-off element, and masked-off body
> > elements do not perform memory accesses.
> > I think it may causes unexpected vl.
> > 
> > > +    /* Get number of complete elements in the first page. */
> > > +    elems = MIN(page_split / msize, vl - i);
> > > +
> > > +    /* Load complete elements from the first page. */
> > > +    if (likely(elems)) {
> > > +        uint32_t page_evl = i + elems;
> > > +
> > > +        if (flags == 0) {
> > ...
> > > +        } else {
> > > +            /*
> > > +             * If the first element is active, it must succeed.
> > > +             * This will load from MMIO or fault from INVALID.
> > > +             */
> > > +            if (first_active) {
> > > +                vext_ldst_nf_tlb(env, vd, addr, 0, nf, esz,
> > > +                                 max_elems, ldst_tlb, ra);
> > > +                i = 1;
> > > +                addr += msize;
> > > +            }
> > > +
> > > +            /* Stop if invalid (unmapped) or mmio (transaction may fail). */
> > > +            if (flags & (TLB_INVALID_MASK | TLB_MMIO)) {
> > > +                env->vl = i;
> > > +                goto tail;
> > > +            }
> > > +
> > For an example, assume
> > - vl = 3
> > - vstart = 0
> > - the mask be [1, 0, 1]
> > - assume element 0 and element 2 be readable, but deny the byte at element 1
> > - all three elements are in the same target page.
> > 
> > In theory, the element 1 is masked off and doesn’t perform any memory
> > access, so the value of vl remains 3.
> > 
> > But the previous probe covers element 1 to 2 and the flags will be non
> > zero due to the denied masked-off element 2. Then the vl will set to 1
> > here.
> > 
> > Maybe we could switch to per element prob when vm is 0 and flags is not 0?
> 
> How are you going to deny the byte at element 1 to be unreadable?  Are you
> expecting this to be some PMP thing, with a 1 byte range?
> 
> How I expect things to work is that:
> 
>  (1) We scan forward for the first active element, set vstart.
> 
>  (2) Probe the page for that element.  If that faults, vstart is visible to
> the trap handler.
> 
>  (3) Discounting MMIO, all following elements in the same page cannot fault,
> and we can process them immediately.  Obviously inactive elements get vma
> handling not loads.
> 
Hi Richard,

Sorry, my previous example was unclear. It does not need a 1-byte PMP
region: RISC-V PMP definition allows the minimum 4-byte PMP region,
which exactly matches one e32 element.

For example, with vl=3, vstart=0, e32, and v0.mask = [1, 0, 1],
configure locked PMP entries as follows:

  PMP0: NA4   [base + 4, base + 7], L, ---    # element 1, no read
  PMP1: NAPOT target page,          L, R      # lower-priority page allow

  bytes:       base       base + 4     base + 8
               +-----+    +-----+      +-----+
  element:       0          1            2
  mask:          1          0            1
  access:     active      inactive      active
  PMP:        PMP1 R      PMP0 ---      PMP1 R

The RISC-V spec defines that masked vector loads access memory and raise
exceptions only for active elements.
Therefore this situation has reads only for elements 0 and 2; PMP0 must
not create an access-fault condition for this instruction.

My concern is that the page probe may be unsafe in some situations
because PMP permissions can change at 4-byte granularity inside a
target page, while the probe includes bytes which are not part of
active vector memory operations.

Maybe we could use the page probe only when it returns flags == 0,
and otherwise fall back to checking only active elements or active
runs?

I'm trying to create masked fauly-only-first + PMP test for this.

Thanks,

rnax

> I'm not sure I'm understanding your question properly...
> 
> 
> r~
>