| Message ID | 20220314044305.138794-4-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | linux-user/arm: Improvements for commpage atomics | expand |
On Mon, 14 Mar 2022 at 04:46, Richard Henderson <richard.henderson@linaro.org> wrote: > > If CONFIG_ATOMIC64, we can use a host cmpxchg and provide > atomicity across processes; otherwise we have no choice but > to continue using start/end_exclusive. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > -segv: > - end_exclusive(); > - /* We get the PC of the entry address - which is as good as anything, > - on a real kernel what you get depends on which mode it uses. */ This comment about the PC the guest signal handler is going to see when we take the SEGV is still valid, I think ? > - /* XXX: check env->error_code */ > - force_sig_fault(TARGET_SIGSEGV, TARGET_SEGV_MAPERR, > - env->exception.vaddress); > + segv: > + force_sig_fault(TARGET_SIGSEGV, > + page_get_flags(addr) & PAGE_VALID ? > + TARGET_SEGV_ACCERR : TARGET_SEGV_MAPERR, addr); > } Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 3/15/22 11:18, Peter Maydell wrote: >> -segv: >> - end_exclusive(); >> - /* We get the PC of the entry address - which is as good as anything, >> - on a real kernel what you get depends on which mode it uses. */ > > This comment about the PC the guest signal handler is going > to see when we take the SEGV is still valid, I think ? Yes. I guess I could move it to the block comment in front of atomic_mmu_lookup, because it would apply to both the SEGV and the BUS raised there. r~
Le 15/03/2022 à 19:31, Richard Henderson a écrit : > On 3/15/22 11:18, Peter Maydell wrote: >>> -segv: >>> - end_exclusive(); >>> - /* We get the PC of the entry address - which is as good as anything, >>> - on a real kernel what you get depends on which mode it uses. */ >> >> This comment about the PC the guest signal handler is going >> to see when we take the SEGV is still valid, I think ? > > Yes. I guess I could move it to the block comment in front of atomic_mmu_lookup, because it would > apply to both the SEGV and the BUS raised there. Applied to my linux-user-for-7.0 branch with the following change: diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c index d9651f199f97..0172a7aae7e8 100644 --- a/linux-user/arm/cpu_loop.c +++ b/linux-user/arm/cpu_loop.c @@ -78,6 +78,8 @@ /* * Similar to code in accel/tcg/user-exec.c, but outside the execution loop. * Must be called with mmap_lock. + * We get the PC of the entry address - which is as good as anything, + * on a real kernel what you get depends on which mode it uses. */ static void *atomic_mmu_lookup(CPUArchState *env, uint32_t addr, int size) { Let me know if you prefer something else. Thanks, Laurent
diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c index 0122bb34f7..d9651f199f 100644 --- a/linux-user/arm/cpu_loop.c +++ b/linux-user/arm/cpu_loop.c @@ -136,7 +136,7 @@ static void arm_kernel_cmpxchg32_helper(CPUARMState *env) } /* - * See the Linux kernel's Documentation/arm/kernel_user_helpers.txt + * See the Linux kernel's Documentation/arm/kernel_user_helpers.rst * Input: * r0 = pointer to oldval * r1 = pointer to newval @@ -153,57 +153,54 @@ static void arm_kernel_cmpxchg64_helper(CPUARMState *env) { uint64_t oldval, newval, val; uint32_t addr, cpsr; + uint64_t *host_addr; - /* Based on the 32 bit code in do_kernel_trap */ + addr = env->regs[0]; + if (get_user_u64(oldval, addr)) { + goto segv; + } - /* XXX: This only works between threads, not between processes. - It's probably possible to implement this with native host - operations. However things like ldrex/strex are much harder so - there's not much point trying. */ - start_exclusive(); - cpsr = cpsr_read(env); + addr = env->regs[1]; + if (get_user_u64(newval, addr)) { + goto segv; + } + + mmap_lock(); addr = env->regs[2]; - - if (get_user_u64(oldval, env->regs[0])) { - env->exception.vaddress = env->regs[0]; - goto segv; - }; - - if (get_user_u64(newval, env->regs[1])) { - env->exception.vaddress = env->regs[1]; - goto segv; - }; - - if (get_user_u64(val, addr)) { - env->exception.vaddress = addr; - goto segv; + host_addr = atomic_mmu_lookup(env, addr, 8); + if (!host_addr) { + mmap_unlock(); + return; } +#ifdef CONFIG_ATOMIC64 + val = qatomic_cmpxchg__nocheck(host_addr, oldval, newval); + cpsr = (val == oldval) * CPSR_C; +#else + /* + * This only works between threads, not between processes, but since + * the host has no 64-bit cmpxchg, it is the best that we can do. + */ + start_exclusive(); + val = *host_addr; if (val == oldval) { - val = newval; - - if (put_user_u64(val, addr)) { - env->exception.vaddress = addr; - goto segv; - }; - - env->regs[0] = 0; - cpsr |= CPSR_C; + *host_addr = newval; + cpsr = CPSR_C; } else { - env->regs[0] = -1; - cpsr &= ~CPSR_C; + cpsr = 0; } - cpsr_write(env, cpsr, CPSR_C, CPSRWriteByInstr); end_exclusive(); +#endif + mmap_unlock(); + + cpsr_write(env, cpsr, CPSR_C, CPSRWriteByInstr); + env->regs[0] = cpsr ? 0 : -1; return; -segv: - end_exclusive(); - /* We get the PC of the entry address - which is as good as anything, - on a real kernel what you get depends on which mode it uses. */ - /* XXX: check env->error_code */ - force_sig_fault(TARGET_SIGSEGV, TARGET_SEGV_MAPERR, - env->exception.vaddress); + segv: + force_sig_fault(TARGET_SIGSEGV, + page_get_flags(addr) & PAGE_VALID ? + TARGET_SEGV_ACCERR : TARGET_SEGV_MAPERR, addr); } /* Handle a jump to the kernel code page. */
If CONFIG_ATOMIC64, we can use a host cmpxchg and provide atomicity across processes; otherwise we have no choice but to continue using start/end_exclusive. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/arm/cpu_loop.c | 79 +++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 41 deletions(-)