| Message ID | 20240719010707.1319675-13-richard.henderson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes for user-only munmap races | expand |
Reviewed-by: Max Chou <max.chou@sifive.com> On 2024/7/19 9:07 AM, Richard Henderson wrote: > The current pairing of tlb_vaddr_to_host with extra is either > inefficient (user-only, with page_check_range) or incorrect > (system, with probe_pages). > > For proper non-fault behaviour, use probe_access_flags with > its nonfault parameter set to true. > > Acked-by: Alistair Francis <alistair.francis@wdc.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/riscv/vector_helper.c | 31 +++++++++++++++++-------------- > 1 file changed, 17 insertions(+), 14 deletions(-) > > diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c > index 1b4d5a8e37..10a52ceb5b 100644 > --- a/target/riscv/vector_helper.c > +++ b/target/riscv/vector_helper.c > @@ -474,7 +474,6 @@ vext_ldff(void *vd, void *v0, target_ulong base, > vext_ldst_elem_fn *ldst_elem, > uint32_t log2_esz, uintptr_t ra) > { > - void *host; > uint32_t i, k, vl = 0; > uint32_t nf = vext_nf(desc); > uint32_t vm = vext_vm(desc); > @@ -493,27 +492,31 @@ vext_ldff(void *vd, void *v0, target_ulong base, > } > addr = adjust_addr(env, base + i * (nf << log2_esz)); > if (i == 0) { > + /* Allow fault on first element. */ > probe_pages(env, addr, nf << log2_esz, ra, MMU_DATA_LOAD); > } else { > - /* if it triggers an exception, no need to check watchpoint */ > remain = nf << log2_esz; > while (remain > 0) { > + void *host; > + int flags; > + > offset = -(addr | TARGET_PAGE_MASK); > - host = tlb_vaddr_to_host(env, addr, MMU_DATA_LOAD, mmu_index); > - if (host) { > -#ifdef CONFIG_USER_ONLY > - if (!page_check_range(addr, offset, PAGE_READ)) { > - vl = i; > - goto ProbeSuccess; > - } > -#else > - probe_pages(env, addr, offset, ra, MMU_DATA_LOAD); > -#endif > - } else { > + > + /* Probe nonfault on subsequent elements. */ > + flags = probe_access_flags(env, addr, offset, MMU_DATA_LOAD, > + mmu_index, true, &host, 0); > + > + /* > + * Stop if invalid (unmapped) or mmio (transaction may fail). > + * Do not stop if watchpoint, as the spec says that > + * first-fault should continue to access the same > + * elements regardless of any watchpoint. > + */ > + if (flags & ~TLB_WATCHPOINT) { > vl = i; > goto ProbeSuccess; > } > - if (remain <= offset) { > + if (remain <= offset) { > break; > } > remain -= offset;
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c index 1b4d5a8e37..10a52ceb5b 100644 --- a/target/riscv/vector_helper.c +++ b/target/riscv/vector_helper.c @@ -474,7 +474,6 @@ vext_ldff(void *vd, void *v0, target_ulong base, vext_ldst_elem_fn *ldst_elem, uint32_t log2_esz, uintptr_t ra) { - void *host; uint32_t i, k, vl = 0; uint32_t nf = vext_nf(desc); uint32_t vm = vext_vm(desc); @@ -493,27 +492,31 @@ vext_ldff(void *vd, void *v0, target_ulong base, } addr = adjust_addr(env, base + i * (nf << log2_esz)); if (i == 0) { + /* Allow fault on first element. */ probe_pages(env, addr, nf << log2_esz, ra, MMU_DATA_LOAD); } else { - /* if it triggers an exception, no need to check watchpoint */ remain = nf << log2_esz; while (remain > 0) { + void *host; + int flags; + offset = -(addr | TARGET_PAGE_MASK); - host = tlb_vaddr_to_host(env, addr, MMU_DATA_LOAD, mmu_index); - if (host) { -#ifdef CONFIG_USER_ONLY - if (!page_check_range(addr, offset, PAGE_READ)) { - vl = i; - goto ProbeSuccess; - } -#else - probe_pages(env, addr, offset, ra, MMU_DATA_LOAD); -#endif - } else { + + /* Probe nonfault on subsequent elements. */ + flags = probe_access_flags(env, addr, offset, MMU_DATA_LOAD, + mmu_index, true, &host, 0); + + /* + * Stop if invalid (unmapped) or mmio (transaction may fail). + * Do not stop if watchpoint, as the spec says that + * first-fault should continue to access the same + * elements regardless of any watchpoint. + */ + if (flags & ~TLB_WATCHPOINT) { vl = i; goto ProbeSuccess; } - if (remain <= offset) { + if (remain <= offset) { break; } remain -= offset;