Re: [PATCH 11/23] target/riscv: Rewrite vext_ldff
Richard Henderson <[email protected]>
| Newsgroups | org.nongnu.qemu-riscv,org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
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. I'm not sure I'm understanding your question properly... r~