* [PATCH 0/3] linux-user/arm: Improvements for commpage atomics
@ 2022-03-14 4:43 Richard Henderson
2022-03-14 4:43 ` [PATCH 1/3] linux-user/arm: Implement __kernel_memory_barrier Richard Henderson
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Richard Henderson @ 2022-03-14 4:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, laurent
Only the memory_barrer one is a bug; the rest improve system-wide
interaction, and would only affect pre-armv6.
r~
Richard Henderson (3):
linux-user/arm: Implement __kernel_memory_barrier
linux-user/arm: Implement __kernel_cmpxchg with host atomics
linux-user/arm: Implement __kernel_cmpxchg64 with host atomics
linux-user/arm/cpu_loop.c | 166 +++++++++++++++++++++++---------------
1 file changed, 99 insertions(+), 67 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/3] linux-user/arm: Implement __kernel_memory_barrier
2022-03-14 4:43 [PATCH 0/3] linux-user/arm: Improvements for commpage atomics Richard Henderson
@ 2022-03-14 4:43 ` Richard Henderson
2022-03-15 18:11 ` Peter Maydell
2022-03-22 11:46 ` Laurent Vivier
2022-03-14 4:43 ` [PATCH 2/3] linux-user/arm: Implement __kernel_cmpxchg with host atomics Richard Henderson
2022-03-14 4:43 ` [PATCH 3/3] linux-user/arm: Implement __kernel_cmpxchg64 " Richard Henderson
2 siblings, 2 replies; 13+ messages in thread
From: Richard Henderson @ 2022-03-14 4:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, laurent
This fallback syscall was stubbed out.
It would only matter for emulating pre-armv6.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/arm/cpu_loop.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index 032e1ffddf..a0e43b261c 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -158,7 +158,7 @@ do_kernel_trap(CPUARMState *env)
switch (env->regs[15]) {
case 0xffff0fa0: /* __kernel_memory_barrier */
- /* ??? No-op. Will need to do better for SMP. */
+ smp_mb();
break;
case 0xffff0fc0: /* __kernel_cmpxchg */
/* XXX: This only works between threads, not between processes.
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] linux-user/arm: Implement __kernel_cmpxchg with host atomics
2022-03-14 4:43 [PATCH 0/3] linux-user/arm: Improvements for commpage atomics Richard Henderson
2022-03-14 4:43 ` [PATCH 1/3] linux-user/arm: Implement __kernel_memory_barrier Richard Henderson
@ 2022-03-14 4:43 ` Richard Henderson
2022-03-15 18:16 ` Peter Maydell
2022-03-22 11:46 ` Laurent Vivier
2022-03-14 4:43 ` [PATCH 3/3] linux-user/arm: Implement __kernel_cmpxchg64 " Richard Henderson
2 siblings, 2 replies; 13+ messages in thread
From: Richard Henderson @ 2022-03-14 4:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, laurent
The existing implementation using start/end_exclusive
does not provide atomicity across processes.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/arm/cpu_loop.c | 85 +++++++++++++++++++++++++++------------
1 file changed, 60 insertions(+), 25 deletions(-)
diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index a0e43b261c..0122bb34f7 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -75,7 +75,65 @@
put_user_u16(__x, (gaddr)); \
})
-/* Commpage handling -- there is no commpage for AArch64 */
+/*
+ * Similar to code in accel/tcg/user-exec.c, but outside the execution loop.
+ * Must be called with mmap_lock.
+ */
+static void *atomic_mmu_lookup(CPUArchState *env, uint32_t addr, int size)
+{
+ int need_flags = PAGE_READ | PAGE_WRITE_ORG | PAGE_VALID;
+ int page_flags;
+
+ /* Enforce guest required alignment. */
+ if (unlikely(addr & (size - 1))) {
+ force_sig_fault(TARGET_SIGBUS, TARGET_BUS_ADRALN, addr);
+ return NULL;
+ }
+
+ page_flags = page_get_flags(addr);
+ if (unlikely((page_flags & need_flags) != need_flags)) {
+ force_sig_fault(TARGET_SIGSEGV,
+ page_flags & PAGE_VALID ?
+ TARGET_SEGV_ACCERR : TARGET_SEGV_MAPERR, addr);
+ return NULL;
+ }
+
+ return g2h(env_cpu(env), addr);
+}
+
+/*
+ * See the Linux kernel's Documentation/arm/kernel_user_helpers.rst
+ * Input:
+ * r0 = oldval
+ * r1 = newval
+ * r2 = pointer to target value
+ *
+ * Output:
+ * r0 = 0 if *ptr was changed, non-0 if no exchange happened
+ * C set if *ptr was changed, clear if no exchange happened
+ */
+static void arm_kernel_cmpxchg32_helper(CPUARMState *env)
+{
+ uint32_t oldval, newval, val, addr, cpsr, *host_addr;
+
+ oldval = env->regs[0];
+ newval = env->regs[1];
+ addr = env->regs[2];
+
+ mmap_lock();
+ host_addr = atomic_mmu_lookup(env, addr, 8);
+ if (!host_addr) {
+ mmap_unlock();
+ return;
+ }
+
+ val = qatomic_cmpxchg__nocheck(host_addr, oldval, newval);
+ mmap_unlock();
+
+ cpsr = (val == oldval) * CPSR_C;
+ cpsr_write(env, cpsr, CPSR_C, CPSRWriteByInstr);
+ env->regs[0] = cpsr ? 0 : -1;
+}
/*
* See the Linux kernel's Documentation/arm/kernel_user_helpers.txt
@@ -153,36 +211,13 @@ static int
do_kernel_trap(CPUARMState *env)
{
uint32_t addr;
- uint32_t cpsr;
- uint32_t val;
switch (env->regs[15]) {
case 0xffff0fa0: /* __kernel_memory_barrier */
smp_mb();
break;
case 0xffff0fc0: /* __kernel_cmpxchg */
- /* 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[2];
- /* FIXME: This should SEGV if the access fails. */
- if (get_user_u32(val, addr))
- val = ~env->regs[0];
- if (val == env->regs[0]) {
- val = env->regs[1];
- /* FIXME: Check for segfaults. */
- put_user_u32(val, addr);
- env->regs[0] = 0;
- cpsr |= CPSR_C;
- } else {
- env->regs[0] = -1;
- cpsr &= ~CPSR_C;
- }
- cpsr_write(env, cpsr, CPSR_C, CPSRWriteByInstr);
- end_exclusive();
+ arm_kernel_cmpxchg32_helper(env);
break;
case 0xffff0fe0: /* __kernel_get_tls */
env->regs[0] = cpu_get_tls(env);
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] linux-user/arm: Implement __kernel_cmpxchg64 with host atomics
2022-03-14 4:43 [PATCH 0/3] linux-user/arm: Improvements for commpage atomics Richard Henderson
2022-03-14 4:43 ` [PATCH 1/3] linux-user/arm: Implement __kernel_memory_barrier Richard Henderson
2022-03-14 4:43 ` [PATCH 2/3] linux-user/arm: Implement __kernel_cmpxchg with host atomics Richard Henderson
@ 2022-03-14 4:43 ` Richard Henderson
2022-03-15 18:18 ` Peter Maydell
2 siblings, 1 reply; 13+ messages in thread
From: Richard Henderson @ 2022-03-14 4:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, laurent
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(-)
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. */
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] linux-user/arm: Implement __kernel_memory_barrier
2022-03-14 4:43 ` [PATCH 1/3] linux-user/arm: Implement __kernel_memory_barrier Richard Henderson
@ 2022-03-15 18:11 ` Peter Maydell
2022-03-22 11:46 ` Laurent Vivier
1 sibling, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2022-03-15 18:11 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel, Laurent
On Mon, 14 Mar 2022 at 04:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This fallback syscall was stubbed out.
> It would only matter for emulating pre-armv6.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] linux-user/arm: Implement __kernel_cmpxchg with host atomics
2022-03-14 4:43 ` [PATCH 2/3] linux-user/arm: Implement __kernel_cmpxchg with host atomics Richard Henderson
@ 2022-03-15 18:16 ` Peter Maydell
2022-03-22 11:46 ` Laurent Vivier
1 sibling, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2022-03-15 18:16 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel, Laurent
On Mon, 14 Mar 2022 at 04:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The existing implementation using start/end_exclusive
> does not provide atomicity across processes.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/arm/cpu_loop.c | 85 +++++++++++++++++++++++++++------------
> 1 file changed, 60 insertions(+), 25 deletions(-)
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] linux-user/arm: Implement __kernel_cmpxchg64 with host atomics
2022-03-14 4:43 ` [PATCH 3/3] linux-user/arm: Implement __kernel_cmpxchg64 " Richard Henderson
@ 2022-03-15 18:18 ` Peter Maydell
2022-03-15 18:31 ` Richard Henderson
0 siblings, 1 reply; 13+ messages in thread
From: Peter Maydell @ 2022-03-15 18:18 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel, Laurent
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] linux-user/arm: Implement __kernel_cmpxchg64 with host atomics
2022-03-15 18:18 ` Peter Maydell
@ 2022-03-15 18:31 ` Richard Henderson
2022-03-22 11:52 ` Laurent Vivier
0 siblings, 1 reply; 13+ messages in thread
From: Richard Henderson @ 2022-03-15 18:31 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, Laurent
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~
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] linux-user/arm: Implement __kernel_memory_barrier
2022-03-14 4:43 ` [PATCH 1/3] linux-user/arm: Implement __kernel_memory_barrier Richard Henderson
2022-03-15 18:11 ` Peter Maydell
@ 2022-03-22 11:46 ` Laurent Vivier
1 sibling, 0 replies; 13+ messages in thread
From: Laurent Vivier @ 2022-03-22 11:46 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm
Le 14/03/2022 à 05:43, Richard Henderson a écrit :
> This fallback syscall was stubbed out.
> It would only matter for emulating pre-armv6.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/arm/cpu_loop.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
> index 032e1ffddf..a0e43b261c 100644
> --- a/linux-user/arm/cpu_loop.c
> +++ b/linux-user/arm/cpu_loop.c
> @@ -158,7 +158,7 @@ do_kernel_trap(CPUARMState *env)
>
> switch (env->regs[15]) {
> case 0xffff0fa0: /* __kernel_memory_barrier */
> - /* ??? No-op. Will need to do better for SMP. */
> + smp_mb();
> break;
> case 0xffff0fc0: /* __kernel_cmpxchg */
> /* XXX: This only works between threads, not between processes.
Applied to my linux-user-for-7.0 branch.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] linux-user/arm: Implement __kernel_cmpxchg with host atomics
2022-03-14 4:43 ` [PATCH 2/3] linux-user/arm: Implement __kernel_cmpxchg with host atomics Richard Henderson
2022-03-15 18:16 ` Peter Maydell
@ 2022-03-22 11:46 ` Laurent Vivier
2022-03-22 20:08 ` Laurent Vivier
1 sibling, 1 reply; 13+ messages in thread
From: Laurent Vivier @ 2022-03-22 11:46 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm
Le 14/03/2022 à 05:43, Richard Henderson a écrit :
> The existing implementation using start/end_exclusive
> does not provide atomicity across processes.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/arm/cpu_loop.c | 85 +++++++++++++++++++++++++++------------
> 1 file changed, 60 insertions(+), 25 deletions(-)
>
> diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
> index a0e43b261c..0122bb34f7 100644
> --- a/linux-user/arm/cpu_loop.c
> +++ b/linux-user/arm/cpu_loop.c
> @@ -75,7 +75,65 @@
> put_user_u16(__x, (gaddr)); \
> })
>
> -/* Commpage handling -- there is no commpage for AArch64 */
> +/*
> + * Similar to code in accel/tcg/user-exec.c, but outside the execution loop.
> + * Must be called with mmap_lock.
> + */
> +static void *atomic_mmu_lookup(CPUArchState *env, uint32_t addr, int size)
> +{
> + int need_flags = PAGE_READ | PAGE_WRITE_ORG | PAGE_VALID;
> + int page_flags;
> +
> + /* Enforce guest required alignment. */
> + if (unlikely(addr & (size - 1))) {
> + force_sig_fault(TARGET_SIGBUS, TARGET_BUS_ADRALN, addr);
> + return NULL;
> + }
> +
> + page_flags = page_get_flags(addr);
> + if (unlikely((page_flags & need_flags) != need_flags)) {
> + force_sig_fault(TARGET_SIGSEGV,
> + page_flags & PAGE_VALID ?
> + TARGET_SEGV_ACCERR : TARGET_SEGV_MAPERR, addr);
> + return NULL;
> + }
> +
> + return g2h(env_cpu(env), addr);
> +}
> +
> +/*
> + * See the Linux kernel's Documentation/arm/kernel_user_helpers.rst
> + * Input:
> + * r0 = oldval
> + * r1 = newval
> + * r2 = pointer to target value
> + *
> + * Output:
> + * r0 = 0 if *ptr was changed, non-0 if no exchange happened
> + * C set if *ptr was changed, clear if no exchange happened
> + */
> +static void arm_kernel_cmpxchg32_helper(CPUARMState *env)
> +{
> + uint32_t oldval, newval, val, addr, cpsr, *host_addr;
> +
> + oldval = env->regs[0];
> + newval = env->regs[1];
> + addr = env->regs[2];
> +
> + mmap_lock();
> + host_addr = atomic_mmu_lookup(env, addr, 8);
> + if (!host_addr) {
> + mmap_unlock();
> + return;
> + }
> +
> + val = qatomic_cmpxchg__nocheck(host_addr, oldval, newval);
> + mmap_unlock();
> +
> + cpsr = (val == oldval) * CPSR_C;
> + cpsr_write(env, cpsr, CPSR_C, CPSRWriteByInstr);
> + env->regs[0] = cpsr ? 0 : -1;
> +}
>
> /*
> * See the Linux kernel's Documentation/arm/kernel_user_helpers.txt
> @@ -153,36 +211,13 @@ static int
> do_kernel_trap(CPUARMState *env)
> {
> uint32_t addr;
> - uint32_t cpsr;
> - uint32_t val;
>
> switch (env->regs[15]) {
> case 0xffff0fa0: /* __kernel_memory_barrier */
> smp_mb();
> break;
> case 0xffff0fc0: /* __kernel_cmpxchg */
> - /* 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[2];
> - /* FIXME: This should SEGV if the access fails. */
> - if (get_user_u32(val, addr))
> - val = ~env->regs[0];
> - if (val == env->regs[0]) {
> - val = env->regs[1];
> - /* FIXME: Check for segfaults. */
> - put_user_u32(val, addr);
> - env->regs[0] = 0;
> - cpsr |= CPSR_C;
> - } else {
> - env->regs[0] = -1;
> - cpsr &= ~CPSR_C;
> - }
> - cpsr_write(env, cpsr, CPSR_C, CPSRWriteByInstr);
> - end_exclusive();
> + arm_kernel_cmpxchg32_helper(env);
> break;
> case 0xffff0fe0: /* __kernel_get_tls */
> env->regs[0] = cpu_get_tls(env);
Applied to my linux-user-for-7.0 branch.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] linux-user/arm: Implement __kernel_cmpxchg64 with host atomics
2022-03-15 18:31 ` Richard Henderson
@ 2022-03-22 11:52 ` Laurent Vivier
0 siblings, 0 replies; 13+ messages in thread
From: Laurent Vivier @ 2022-03-22 11:52 UTC (permalink / raw)
To: Richard Henderson, Peter Maydell; +Cc: qemu-arm, qemu-devel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] linux-user/arm: Implement __kernel_cmpxchg with host atomics
2022-03-22 11:46 ` Laurent Vivier
@ 2022-03-22 20:08 ` Laurent Vivier
2022-03-23 0:41 ` Richard Henderson
0 siblings, 1 reply; 13+ messages in thread
From: Laurent Vivier @ 2022-03-22 20:08 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm
Le 22/03/2022 à 12:46, Laurent Vivier a écrit :
> Le 14/03/2022 à 05:43, Richard Henderson a écrit :
>> The existing implementation using start/end_exclusive
>> does not provide atomicity across processes.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> linux-user/arm/cpu_loop.c | 85 +++++++++++++++++++++++++++------------
>> 1 file changed, 60 insertions(+), 25 deletions(-)
>>
>> diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
>> index a0e43b261c..0122bb34f7 100644
>> --- a/linux-user/arm/cpu_loop.c
>> +++ b/linux-user/arm/cpu_loop.c
>> @@ -75,7 +75,65 @@
>> put_user_u16(__x, (gaddr)); \
>> })
>> -/* Commpage handling -- there is no commpage for AArch64 */
>> +/*
>> + * Similar to code in accel/tcg/user-exec.c, but outside the execution loop.
>> + * Must be called with mmap_lock.
>> + */
>> +static void *atomic_mmu_lookup(CPUArchState *env, uint32_t addr, int size)
>> +{
>> + int need_flags = PAGE_READ | PAGE_WRITE_ORG | PAGE_VALID;
>> + int page_flags;
>> +
>> + /* Enforce guest required alignment. */
>> + if (unlikely(addr & (size - 1))) {
>> + force_sig_fault(TARGET_SIGBUS, TARGET_BUS_ADRALN, addr);
>> + return NULL;
>> + }
>> +
>> + page_flags = page_get_flags(addr);
>> + if (unlikely((page_flags & need_flags) != need_flags)) {
>> + force_sig_fault(TARGET_SIGSEGV,
>> + page_flags & PAGE_VALID ?
>> + TARGET_SEGV_ACCERR : TARGET_SEGV_MAPERR, addr);
>> + return NULL;
>> + }
>> +
>> + return g2h(env_cpu(env), addr);
>> +}
>> +
>> +/*
>> + * See the Linux kernel's Documentation/arm/kernel_user_helpers.rst
>> + * Input:
>> + * r0 = oldval
>> + * r1 = newval
>> + * r2 = pointer to target value
>> + *
>> + * Output:
>> + * r0 = 0 if *ptr was changed, non-0 if no exchange happened
>> + * C set if *ptr was changed, clear if no exchange happened
>> + */
>> +static void arm_kernel_cmpxchg32_helper(CPUARMState *env)
>> +{
>> + uint32_t oldval, newval, val, addr, cpsr, *host_addr;
>> +
>> + oldval = env->regs[0];
>> + newval = env->regs[1];
>> + addr = env->regs[2];
>> +
>> + mmap_lock();
>> + host_addr = atomic_mmu_lookup(env, addr, 8);
>> + if (!host_addr) {
>> + mmap_unlock();
>> + return;
>> + }
>> +
>> + val = qatomic_cmpxchg__nocheck(host_addr, oldval, newval);
>> + mmap_unlock();
>> +
>> + cpsr = (val == oldval) * CPSR_C;
>> + cpsr_write(env, cpsr, CPSR_C, CPSRWriteByInstr);
>> + env->regs[0] = cpsr ? 0 : -1;
>> +}
>> /*
>> * See the Linux kernel's Documentation/arm/kernel_user_helpers.txt
>> @@ -153,36 +211,13 @@ static int
>> do_kernel_trap(CPUARMState *env)
>> {
>> uint32_t addr;
>> - uint32_t cpsr;
>> - uint32_t val;
>> switch (env->regs[15]) {
>> case 0xffff0fa0: /* __kernel_memory_barrier */
>> smp_mb();
>> break;
>> case 0xffff0fc0: /* __kernel_cmpxchg */
>> - /* 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[2];
>> - /* FIXME: This should SEGV if the access fails. */
>> - if (get_user_u32(val, addr))
>> - val = ~env->regs[0];
>> - if (val == env->regs[0]) {
>> - val = env->regs[1];
>> - /* FIXME: Check for segfaults. */
>> - put_user_u32(val, addr);
>> - env->regs[0] = 0;
>> - cpsr |= CPSR_C;
>> - } else {
>> - env->regs[0] = -1;
>> - cpsr &= ~CPSR_C;
>> - }
>> - cpsr_write(env, cpsr, CPSR_C, CPSRWriteByInstr);
>> - end_exclusive();
>> + arm_kernel_cmpxchg32_helper(env);
>> break;
>> case 0xffff0fe0: /* __kernel_get_tls */
>> env->regs[0] = cpu_get_tls(env);
>
> Applied to my linux-user-for-7.0 branch.
>
>
I have removed this patch and the following one from the branch because it hangs when executed in an
armhf/bionic chroot the following example:
cat > /tmp/hello.go <<EOF
package main
import "fmt"
func main() {
fmt.Println("Hello Google!")
}
EOF
go run /tmp/hello.go
Thanks,
Laurent
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] linux-user/arm: Implement __kernel_cmpxchg with host atomics
2022-03-22 20:08 ` Laurent Vivier
@ 2022-03-23 0:41 ` Richard Henderson
0 siblings, 0 replies; 13+ messages in thread
From: Richard Henderson @ 2022-03-23 0:41 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: qemu-arm
On 3/22/22 13:08, Laurent Vivier wrote:
> I have removed this patch and the following one from the branch because it hangs when
> executed in an armhf/bionic chroot the following example:
>
> cat > /tmp/hello.go <<EOF
> package main
>
> import "fmt"
>
> func main() {
> fmt.Println("Hello Google!")
> }
> EOF
>
> go run /tmp/hello.go
I don't see a hang. I see a SIGBUS, due to a silly typo here:
> +static void arm_kernel_cmpxchg32_helper(CPUARMState *env)
> +{
> + uint32_t oldval, newval, val, addr, cpsr, *host_addr;
> +
> + oldval = env->regs[0];
> + newval = env->regs[1];
> + addr = env->regs[2];
> +
> + mmap_lock();
> + host_addr = atomic_mmu_lookup(env, addr, 8);
s/8/4/.
r~
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-03-23 0:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-14 4:43 [PATCH 0/3] linux-user/arm: Improvements for commpage atomics Richard Henderson
2022-03-14 4:43 ` [PATCH 1/3] linux-user/arm: Implement __kernel_memory_barrier Richard Henderson
2022-03-15 18:11 ` Peter Maydell
2022-03-22 11:46 ` Laurent Vivier
2022-03-14 4:43 ` [PATCH 2/3] linux-user/arm: Implement __kernel_cmpxchg with host atomics Richard Henderson
2022-03-15 18:16 ` Peter Maydell
2022-03-22 11:46 ` Laurent Vivier
2022-03-22 20:08 ` Laurent Vivier
2022-03-23 0:41 ` Richard Henderson
2022-03-14 4:43 ` [PATCH 3/3] linux-user/arm: Implement __kernel_cmpxchg64 " Richard Henderson
2022-03-15 18:18 ` Peter Maydell
2022-03-15 18:31 ` Richard Henderson
2022-03-22 11:52 ` Laurent Vivier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.