* [Qemu-devel] [PATCH 1/4] linux-user/s390x: Mask si_addr for SIGSEGV
2017-11-06 18:33 [Qemu-devel] [PATCH 0/4] linux-user: fix various SIGSEGV delivery bugs Peter Maydell
@ 2017-11-06 18:33 ` Peter Maydell
2017-11-07 8:06 ` Laurent Vivier
` (2 more replies)
2017-11-06 18:33 ` [Qemu-devel] [PATCH 2/4] linux-user/ppc: Report correct fault address for data faults Peter Maydell
` (3 subsequent siblings)
4 siblings, 3 replies; 19+ messages in thread
From: Peter Maydell @ 2017-11-06 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Riku Voipio, Laurent Vivier
For s390x, the address passed to a signal handler in the
siginfo_t si_addr field is masked (in the kernel this is done in
do_sigbus() and do_sigsegv() in arch/s390/mm/fault.c). Implement
this architecture-specific oddity in linux-user.
This is one of the issues described in
https://bugs.launchpad.net/qemu/+bug/1705118
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
linux-user/main.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/linux-user/main.c b/linux-user/main.c
index aa02f25..b6dd9ef 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3238,6 +3238,10 @@ void cpu_loop(CPUAlphaState *env)
#endif /* TARGET_ALPHA */
#ifdef TARGET_S390X
+
+/* s390x masks the fault address it reports in si_addr for SIGSEGV and SIGBUS */
+#define S390X_FAIL_ADDR_MASK -4096LL
+
void cpu_loop(CPUS390XState *env)
{
CPUState *cs = CPU(s390_env_get_cpu(env));
@@ -3294,7 +3298,7 @@ void cpu_loop(CPUS390XState *env)
sig = TARGET_SIGSEGV;
/* XXX: check env->error_code */
n = TARGET_SEGV_MAPERR;
- addr = env->__excp_addr;
+ addr = env->__excp_addr & S390X_FAIL_ADDR_MASK;
goto do_signal;
case PGM_EXECUTE:
case PGM_SPECIFICATION:
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] linux-user/s390x: Mask si_addr for SIGSEGV
2017-11-06 18:33 ` [Qemu-devel] [PATCH 1/4] linux-user/s390x: Mask si_addr for SIGSEGV Peter Maydell
@ 2017-11-07 8:06 ` Laurent Vivier
2017-11-07 15:34 ` Philippe Mathieu-Daudé
2017-11-08 21:18 ` Richard Henderson
2 siblings, 0 replies; 19+ messages in thread
From: Laurent Vivier @ 2017-11-07 8:06 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Riku Voipio
Le 06/11/2017 à 19:33, Peter Maydell a écrit :
> For s390x, the address passed to a signal handler in the
> siginfo_t si_addr field is masked (in the kernel this is done in
> do_sigbus() and do_sigsegv() in arch/s390/mm/fault.c). Implement
> this architecture-specific oddity in linux-user.
>
> This is one of the issues described in
> https://bugs.launchpad.net/qemu/+bug/1705118
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> linux-user/main.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index aa02f25..b6dd9ef 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -3238,6 +3238,10 @@ void cpu_loop(CPUAlphaState *env)
> #endif /* TARGET_ALPHA */
>
> #ifdef TARGET_S390X
> +
> +/* s390x masks the fault address it reports in si_addr for SIGSEGV and SIGBUS */
> +#define S390X_FAIL_ADDR_MASK -4096LL
> +
> void cpu_loop(CPUS390XState *env)
> {
> CPUState *cs = CPU(s390_env_get_cpu(env));
> @@ -3294,7 +3298,7 @@ void cpu_loop(CPUS390XState *env)
> sig = TARGET_SIGSEGV;
> /* XXX: check env->error_code */
> n = TARGET_SEGV_MAPERR;
> - addr = env->__excp_addr;
> + addr = env->__excp_addr & S390X_FAIL_ADDR_MASK;
> goto do_signal;
> case PGM_EXECUTE:
> case PGM_SPECIFICATION:
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] linux-user/s390x: Mask si_addr for SIGSEGV
2017-11-06 18:33 ` [Qemu-devel] [PATCH 1/4] linux-user/s390x: Mask si_addr for SIGSEGV Peter Maydell
2017-11-07 8:06 ` Laurent Vivier
@ 2017-11-07 15:34 ` Philippe Mathieu-Daudé
2017-11-08 21:18 ` Richard Henderson
2 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-11-07 15:34 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Riku Voipio, Laurent Vivier, patches
On 11/06/2017 03:33 PM, Peter Maydell wrote:
> For s390x, the address passed to a signal handler in the
> siginfo_t si_addr field is masked (in the kernel this is done in
> do_sigbus() and do_sigsegv() in arch/s390/mm/fault.c). Implement
> this architecture-specific oddity in linux-user.
>
> This is one of the issues described in
> https://bugs.launchpad.net/qemu/+bug/1705118
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> linux-user/main.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index aa02f25..b6dd9ef 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -3238,6 +3238,10 @@ void cpu_loop(CPUAlphaState *env)
> #endif /* TARGET_ALPHA */
>
> #ifdef TARGET_S390X
> +
> +/* s390x masks the fault address it reports in si_addr for SIGSEGV and SIGBUS */
> +#define S390X_FAIL_ADDR_MASK -4096LL
> +
> void cpu_loop(CPUS390XState *env)
> {
> CPUState *cs = CPU(s390_env_get_cpu(env));
> @@ -3294,7 +3298,7 @@ void cpu_loop(CPUS390XState *env)
> sig = TARGET_SIGSEGV;
> /* XXX: check env->error_code */
> n = TARGET_SEGV_MAPERR;
> - addr = env->__excp_addr;
> + addr = env->__excp_addr & S390X_FAIL_ADDR_MASK;
> goto do_signal;
> case PGM_EXECUTE:
> case PGM_SPECIFICATION:
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] linux-user/s390x: Mask si_addr for SIGSEGV
2017-11-06 18:33 ` [Qemu-devel] [PATCH 1/4] linux-user/s390x: Mask si_addr for SIGSEGV Peter Maydell
2017-11-07 8:06 ` Laurent Vivier
2017-11-07 15:34 ` Philippe Mathieu-Daudé
@ 2017-11-08 21:18 ` Richard Henderson
2017-11-09 11:12 ` Peter Maydell
2 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2017-11-08 21:18 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Riku Voipio, Laurent Vivier, patches
On 11/06/2017 07:33 PM, Peter Maydell wrote:
> For s390x, the address passed to a signal handler in the
> siginfo_t si_addr field is masked (in the kernel this is done in
> do_sigbus() and do_sigsegv() in arch/s390/mm/fault.c). Implement
> this architecture-specific oddity in linux-user.
>
> This is one of the issues described in
> https://bugs.launchpad.net/qemu/+bug/1705118
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> linux-user/main.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
Accurate, but really seems like a s390x kernel bug.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] linux-user/s390x: Mask si_addr for SIGSEGV
2017-11-08 21:18 ` Richard Henderson
@ 2017-11-09 11:12 ` Peter Maydell
0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2017-11-09 11:12 UTC (permalink / raw)
To: Richard Henderson; +Cc: QEMU Developers, Riku Voipio, Laurent Vivier, patches
On 8 November 2017 at 21:18, Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 11/06/2017 07:33 PM, Peter Maydell wrote:
>> For s390x, the address passed to a signal handler in the
>> siginfo_t si_addr field is masked (in the kernel this is done in
>> do_sigbus() and do_sigsegv() in arch/s390/mm/fault.c). Implement
>> this architecture-specific oddity in linux-user.
>>
>> This is one of the issues described in
>> https://bugs.launchpad.net/qemu/+bug/1705118
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> linux-user/main.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> Accurate, but really seems like a s390x kernel bug.
The kernel code goes out of its way to do the masking, so if it's
a bug presumably it's a "retain back compat with some older
bug" thing...
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 2/4] linux-user/ppc: Report correct fault address for data faults
2017-11-06 18:33 [Qemu-devel] [PATCH 0/4] linux-user: fix various SIGSEGV delivery bugs Peter Maydell
2017-11-06 18:33 ` [Qemu-devel] [PATCH 1/4] linux-user/s390x: Mask si_addr for SIGSEGV Peter Maydell
@ 2017-11-06 18:33 ` Peter Maydell
2017-11-07 8:17 ` Laurent Vivier
2017-11-08 21:19 ` Richard Henderson
2017-11-06 18:33 ` [Qemu-devel] [PATCH 3/4] linux-user/sparc: Put address for data faults where linux-user expects it Peter Maydell
` (2 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Peter Maydell @ 2017-11-06 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Riku Voipio, Laurent Vivier
For faults on loads and stores, ppc_cpu_handle_mmu_fault() in
target/ppc/user_only_helper.c stores the offending address
in env->spr[SPR_DAR]. Report this correctly to the guest
in si_addr, rather than incorrectly using the address of the
instruction that caused the fault.
This fixes the test case in
https://bugs.launchpad.net/qemu/+bug/1077116
for ppc, ppc64 and ppc64le.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
linux-user/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/main.c b/linux-user/main.c
index b6dd9ef..6286661 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1420,7 +1420,7 @@ void cpu_loop(CPUPPCState *env)
info.si_code = TARGET_SEGV_MAPERR;
break;
}
- info._sifields._sigfault._addr = env->nip;
+ info._sifields._sigfault._addr = env->spr[SPR_DAR];
queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
break;
case POWERPC_EXCP_ISI: /* Instruction storage exception */
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] linux-user/ppc: Report correct fault address for data faults
2017-11-06 18:33 ` [Qemu-devel] [PATCH 2/4] linux-user/ppc: Report correct fault address for data faults Peter Maydell
@ 2017-11-07 8:17 ` Laurent Vivier
2017-11-08 21:19 ` Richard Henderson
1 sibling, 0 replies; 19+ messages in thread
From: Laurent Vivier @ 2017-11-07 8:17 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Riku Voipio
Le 06/11/2017 à 19:33, Peter Maydell a écrit :
> For faults on loads and stores, ppc_cpu_handle_mmu_fault() in
> target/ppc/user_only_helper.c stores the offending address
> in env->spr[SPR_DAR]. Report this correctly to the guest
> in si_addr, rather than incorrectly using the address of the
> instruction that caused the fault.
>
> This fixes the test case in
> https://bugs.launchpad.net/qemu/+bug/1077116
> for ppc, ppc64 and ppc64le.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> linux-user/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index b6dd9ef..6286661 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -1420,7 +1420,7 @@ void cpu_loop(CPUPPCState *env)
> info.si_code = TARGET_SEGV_MAPERR;
> break;
> }
> - info._sifields._sigfault._addr = env->nip;
> + info._sifields._sigfault._addr = env->spr[SPR_DAR];
> queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> break;
> case POWERPC_EXCP_ISI: /* Instruction storage exception */
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] linux-user/ppc: Report correct fault address for data faults
2017-11-06 18:33 ` [Qemu-devel] [PATCH 2/4] linux-user/ppc: Report correct fault address for data faults Peter Maydell
2017-11-07 8:17 ` Laurent Vivier
@ 2017-11-08 21:19 ` Richard Henderson
1 sibling, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2017-11-08 21:19 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Riku Voipio, Laurent Vivier, patches
On 11/06/2017 07:33 PM, Peter Maydell wrote:
> For faults on loads and stores, ppc_cpu_handle_mmu_fault() in
> target/ppc/user_only_helper.c stores the offending address
> in env->spr[SPR_DAR]. Report this correctly to the guest
> in si_addr, rather than incorrectly using the address of the
> instruction that caused the fault.
>
> This fixes the test case in
> https://bugs.launchpad.net/qemu/+bug/1077116
> for ppc, ppc64 and ppc64le.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> linux-user/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 3/4] linux-user/sparc: Put address for data faults where linux-user expects it
2017-11-06 18:33 [Qemu-devel] [PATCH 0/4] linux-user: fix various SIGSEGV delivery bugs Peter Maydell
2017-11-06 18:33 ` [Qemu-devel] [PATCH 1/4] linux-user/s390x: Mask si_addr for SIGSEGV Peter Maydell
2017-11-06 18:33 ` [Qemu-devel] [PATCH 2/4] linux-user/ppc: Report correct fault address for data faults Peter Maydell
@ 2017-11-06 18:33 ` Peter Maydell
2017-11-07 8:28 ` Laurent Vivier
` (3 more replies)
2017-11-06 18:33 ` [Qemu-devel] [PATCH 4/4] linux-user: Handle rt_sigaction correctly for SPARC Peter Maydell
2017-11-07 20:02 ` [Qemu-devel] [PATCH 0/4] linux-user: fix various SIGSEGV delivery bugs Riku Voipio
4 siblings, 4 replies; 19+ messages in thread
From: Peter Maydell @ 2017-11-06 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Riku Voipio, Laurent Vivier
In the user-mode-only version of sparc_cpu_handle_mmu_fault(),
we must save the fault address for a data fault into the CPU
state's mmu registers, because the code in linux-user/main.c
expects to find it there in order to populate the si_addr
field of the guest siginfo.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/sparc/mmu_helper.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
index 126ea5e..d5b6c1e 100644
--- a/target/sparc/mmu_helper.c
+++ b/target/sparc/mmu_helper.c
@@ -30,10 +30,18 @@
int sparc_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw,
int mmu_idx)
{
+ SPARCCPU *cpu = SPARC_CPU(cs);
+ CPUSPARCState *env = &cpu->env;
+
if (rw & 2) {
cs->exception_index = TT_TFAULT;
} else {
cs->exception_index = TT_DFAULT;
+#ifdef TARGET_SPARC64
+ env->dmmu.mmuregs[4] = address;
+#else
+ env->mmuregs[4] = address;
+#endif
}
return 1;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] linux-user/sparc: Put address for data faults where linux-user expects it
2017-11-06 18:33 ` [Qemu-devel] [PATCH 3/4] linux-user/sparc: Put address for data faults where linux-user expects it Peter Maydell
@ 2017-11-07 8:28 ` Laurent Vivier
2017-11-07 9:20 ` Peter Maydell
2017-11-07 9:25 ` Laurent Vivier
` (2 subsequent siblings)
3 siblings, 1 reply; 19+ messages in thread
From: Laurent Vivier @ 2017-11-07 8:28 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Riku Voipio
Le 06/11/2017 à 19:33, Peter Maydell a écrit :
> In the user-mode-only version of sparc_cpu_handle_mmu_fault(),
> we must save the fault address for a data fault into the CPU
> state's mmu registers, because the code in linux-user/main.c
> expects to find it there in order to populate the si_addr
> field of the guest siginfo.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target/sparc/mmu_helper.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
> index 126ea5e..d5b6c1e 100644
> --- a/target/sparc/mmu_helper.c
> +++ b/target/sparc/mmu_helper.c
> @@ -30,10 +30,18 @@
> int sparc_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw,
> int mmu_idx)
> {
> + SPARCCPU *cpu = SPARC_CPU(cs);
> + CPUSPARCState *env = &cpu->env;
> +
> if (rw & 2) {
> cs->exception_index = TT_TFAULT;
> } else {
> cs->exception_index = TT_DFAULT;
> +#ifdef TARGET_SPARC64
> + env->dmmu.mmuregs[4] = address;
> +#else
> + env->mmuregs[4] = address;
> +#endif
> }
> return 1;
> }
>
The softmmu version of sparc_cpu_handle_mmu_fault() also updates
mmuregs[3]. Is it needed for this one (for ucontext)?
Thanks,
Laurent
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] linux-user/sparc: Put address for data faults where linux-user expects it
2017-11-07 8:28 ` Laurent Vivier
@ 2017-11-07 9:20 ` Peter Maydell
0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2017-11-07 9:20 UTC (permalink / raw)
To: Laurent Vivier; +Cc: QEMU Developers, patches, Riku Voipio
On 7 November 2017 at 08:28, Laurent Vivier <laurent@vivier.eu> wrote:
> Le 06/11/2017 à 19:33, Peter Maydell a écrit :
>> In the user-mode-only version of sparc_cpu_handle_mmu_fault(),
>> we must save the fault address for a data fault into the CPU
>> state's mmu registers, because the code in linux-user/main.c
>> expects to find it there in order to populate the si_addr
>> field of the guest siginfo.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> target/sparc/mmu_helper.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
>> index 126ea5e..d5b6c1e 100644
>> --- a/target/sparc/mmu_helper.c
>> +++ b/target/sparc/mmu_helper.c
>> @@ -30,10 +30,18 @@
>> int sparc_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw,
>> int mmu_idx)
>> {
>> + SPARCCPU *cpu = SPARC_CPU(cs);
>> + CPUSPARCState *env = &cpu->env;
>> +
>> if (rw & 2) {
>> cs->exception_index = TT_TFAULT;
>> } else {
>> cs->exception_index = TT_DFAULT;
>> +#ifdef TARGET_SPARC64
>> + env->dmmu.mmuregs[4] = address;
>> +#else
>> + env->mmuregs[4] = address;
>> +#endif
>> }
>> return 1;
>> }
>>
>
> The softmmu version of sparc_cpu_handle_mmu_fault() also updates
> mmuregs[3]. Is it needed for this one (for ucontext)?
Nothing in linux-user/ reads mmuregs[3], so I assume not.
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] linux-user/sparc: Put address for data faults where linux-user expects it
2017-11-06 18:33 ` [Qemu-devel] [PATCH 3/4] linux-user/sparc: Put address for data faults where linux-user expects it Peter Maydell
2017-11-07 8:28 ` Laurent Vivier
@ 2017-11-07 9:25 ` Laurent Vivier
2017-11-07 15:26 ` Philippe Mathieu-Daudé
2017-11-08 21:21 ` Richard Henderson
3 siblings, 0 replies; 19+ messages in thread
From: Laurent Vivier @ 2017-11-07 9:25 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Riku Voipio
Le 06/11/2017 à 19:33, Peter Maydell a écrit :
> In the user-mode-only version of sparc_cpu_handle_mmu_fault(),
> we must save the fault address for a data fault into the CPU
> state's mmu registers, because the code in linux-user/main.c
> expects to find it there in order to populate the si_addr
> field of the guest siginfo.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target/sparc/mmu_helper.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
> index 126ea5e..d5b6c1e 100644
> --- a/target/sparc/mmu_helper.c
> +++ b/target/sparc/mmu_helper.c
> @@ -30,10 +30,18 @@
> int sparc_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw,
> int mmu_idx)
> {
> + SPARCCPU *cpu = SPARC_CPU(cs);
> + CPUSPARCState *env = &cpu->env;
> +
> if (rw & 2) {
> cs->exception_index = TT_TFAULT;
> } else {
> cs->exception_index = TT_DFAULT;
> +#ifdef TARGET_SPARC64
> + env->dmmu.mmuregs[4] = address;
> +#else
> + env->mmuregs[4] = address;
> +#endif
> }
> return 1;
> }
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] linux-user/sparc: Put address for data faults where linux-user expects it
2017-11-06 18:33 ` [Qemu-devel] [PATCH 3/4] linux-user/sparc: Put address for data faults where linux-user expects it Peter Maydell
2017-11-07 8:28 ` Laurent Vivier
2017-11-07 9:25 ` Laurent Vivier
@ 2017-11-07 15:26 ` Philippe Mathieu-Daudé
2017-11-08 21:21 ` Richard Henderson
3 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-11-07 15:26 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Riku Voipio, Laurent Vivier, patches
On 11/06/2017 03:33 PM, Peter Maydell wrote:
> In the user-mode-only version of sparc_cpu_handle_mmu_fault(),
> we must save the fault address for a data fault into the CPU
> state's mmu registers, because the code in linux-user/main.c
> expects to find it there in order to populate the si_addr
> field of the guest siginfo.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> target/sparc/mmu_helper.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
> index 126ea5e..d5b6c1e 100644
> --- a/target/sparc/mmu_helper.c
> +++ b/target/sparc/mmu_helper.c
> @@ -30,10 +30,18 @@
> int sparc_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw,
> int mmu_idx)
> {
> + SPARCCPU *cpu = SPARC_CPU(cs);
> + CPUSPARCState *env = &cpu->env;
> +
> if (rw & 2) {
> cs->exception_index = TT_TFAULT;
> } else {
> cs->exception_index = TT_DFAULT;
> +#ifdef TARGET_SPARC64
> + env->dmmu.mmuregs[4] = address;
> +#else
> + env->mmuregs[4] = address;
> +#endif
> }
> return 1;
> }
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] linux-user/sparc: Put address for data faults where linux-user expects it
2017-11-06 18:33 ` [Qemu-devel] [PATCH 3/4] linux-user/sparc: Put address for data faults where linux-user expects it Peter Maydell
` (2 preceding siblings ...)
2017-11-07 15:26 ` Philippe Mathieu-Daudé
@ 2017-11-08 21:21 ` Richard Henderson
3 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2017-11-08 21:21 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Riku Voipio, Laurent Vivier, patches
On 11/06/2017 07:33 PM, Peter Maydell wrote:
> In the user-mode-only version of sparc_cpu_handle_mmu_fault(),
> we must save the fault address for a data fault into the CPU
> state's mmu registers, because the code in linux-user/main.c
> expects to find it there in order to populate the si_addr
> field of the guest siginfo.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target/sparc/mmu_helper.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 4/4] linux-user: Handle rt_sigaction correctly for SPARC
2017-11-06 18:33 [Qemu-devel] [PATCH 0/4] linux-user: fix various SIGSEGV delivery bugs Peter Maydell
` (2 preceding siblings ...)
2017-11-06 18:33 ` [Qemu-devel] [PATCH 3/4] linux-user/sparc: Put address for data faults where linux-user expects it Peter Maydell
@ 2017-11-06 18:33 ` Peter Maydell
2017-11-07 14:11 ` Laurent Vivier
2017-11-07 15:44 ` Philippe Mathieu-Daudé
2017-11-07 20:02 ` [Qemu-devel] [PATCH 0/4] linux-user: fix various SIGSEGV delivery bugs Riku Voipio
4 siblings, 2 replies; 19+ messages in thread
From: Peter Maydell @ 2017-11-06 18:33 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Riku Voipio, Laurent Vivier
SPARC is like Alpha in its handling of the rt_sigaction syscall:
it takes an extra parameter 'restorer' which needs to be copied
into the sa_restorer field of the sigaction struct. The order
of the arguments differs slightly between SPARC and Alpha but
the implementation is otherwise the same. (Compare the
rt_sigaction() functions in arch/sparc/kernel/sys_sparc_64.c
and arch/alpha/kernel/signal.c.)
Note that this change is somewhat moot until SPARC acquires
support for actually delivering RT signals.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
linux-user/syscall.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d4497de..8beab51 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8556,8 +8556,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
case TARGET_NR_rt_sigaction:
{
#if defined(TARGET_ALPHA)
- struct target_sigaction act, oact, *pact = 0;
+ /* For Alpha and SPARC this is a 5 argument syscall, with
+ * a 'restorer' parameter which must be copied into the
+ * sa_restorer field of the sigaction struct.
+ * For Alpha that 'restorer' is arg5; for SPARC it is arg4,
+ * and arg5 is the sigsetsize.
+ * Alpha also has a separate rt_sigaction struct that it uses
+ * here; SPARC uses the usual sigaction struct.
+ */
struct target_rt_sigaction *rt_act;
+ struct target_sigaction act, oact, *pact = 0;
if (arg4 != sizeof(target_sigset_t)) {
ret = -TARGET_EINVAL;
@@ -8583,18 +8591,29 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
unlock_user_struct(rt_act, arg3, 1);
}
#else
+#ifdef TARGET_SPARC
+ target_ulong restorer = arg4;
+ target_ulong sigsetsize = arg5;
+#else
+ target_ulong sigsetsize = arg4;
+#endif
struct target_sigaction *act;
struct target_sigaction *oact;
- if (arg4 != sizeof(target_sigset_t)) {
+ if (sigsetsize != sizeof(target_sigset_t)) {
ret = -TARGET_EINVAL;
break;
}
if (arg2) {
- if (!lock_user_struct(VERIFY_READ, act, arg2, 1))
+ if (!lock_user_struct(VERIFY_READ, act, arg2, 1)) {
goto efault;
- } else
+ }
+#ifdef TARGET_SPARC
+ act->sa_restorer = restorer;
+#endif
+ } else {
act = NULL;
+ }
if (arg3) {
if (!lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) {
ret = -TARGET_EFAULT;
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] linux-user: Handle rt_sigaction correctly for SPARC
2017-11-06 18:33 ` [Qemu-devel] [PATCH 4/4] linux-user: Handle rt_sigaction correctly for SPARC Peter Maydell
@ 2017-11-07 14:11 ` Laurent Vivier
2017-11-07 15:44 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 19+ messages in thread
From: Laurent Vivier @ 2017-11-07 14:11 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Riku Voipio
Le 06/11/2017 à 19:33, Peter Maydell a écrit :
> SPARC is like Alpha in its handling of the rt_sigaction syscall:
> it takes an extra parameter 'restorer' which needs to be copied
> into the sa_restorer field of the sigaction struct. The order
> of the arguments differs slightly between SPARC and Alpha but
> the implementation is otherwise the same. (Compare the
> rt_sigaction() functions in arch/sparc/kernel/sys_sparc_64.c
> and arch/alpha/kernel/signal.c.)
>
> Note that this change is somewhat moot until SPARC acquires
> support for actually delivering RT signals.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> linux-user/syscall.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index d4497de..8beab51 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8556,8 +8556,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> case TARGET_NR_rt_sigaction:
> {
> #if defined(TARGET_ALPHA)
> - struct target_sigaction act, oact, *pact = 0;
> + /* For Alpha and SPARC this is a 5 argument syscall, with
> + * a 'restorer' parameter which must be copied into the
> + * sa_restorer field of the sigaction struct.
> + * For Alpha that 'restorer' is arg5; for SPARC it is arg4,
> + * and arg5 is the sigsetsize.
> + * Alpha also has a separate rt_sigaction struct that it uses
> + * here; SPARC uses the usual sigaction struct.
> + */
> struct target_rt_sigaction *rt_act;
> + struct target_sigaction act, oact, *pact = 0;
>
> if (arg4 != sizeof(target_sigset_t)) {
> ret = -TARGET_EINVAL;
> @@ -8583,18 +8591,29 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> unlock_user_struct(rt_act, arg3, 1);
> }
> #else
> +#ifdef TARGET_SPARC
> + target_ulong restorer = arg4;
> + target_ulong sigsetsize = arg5;
> +#else
> + target_ulong sigsetsize = arg4;
> +#endif
> struct target_sigaction *act;
> struct target_sigaction *oact;
>
> - if (arg4 != sizeof(target_sigset_t)) {
> + if (sigsetsize != sizeof(target_sigset_t)) {
> ret = -TARGET_EINVAL;
> break;
> }
> if (arg2) {
> - if (!lock_user_struct(VERIFY_READ, act, arg2, 1))
> + if (!lock_user_struct(VERIFY_READ, act, arg2, 1)) {
> goto efault;
> - } else
> + }
> +#ifdef TARGET_SPARC
> + act->sa_restorer = restorer;
> +#endif
> + } else {
> act = NULL;
> + }
> if (arg3) {
> if (!lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) {
> ret = -TARGET_EFAULT;
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] linux-user: Handle rt_sigaction correctly for SPARC
2017-11-06 18:33 ` [Qemu-devel] [PATCH 4/4] linux-user: Handle rt_sigaction correctly for SPARC Peter Maydell
2017-11-07 14:11 ` Laurent Vivier
@ 2017-11-07 15:44 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-11-07 15:44 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Riku Voipio, Laurent Vivier, patches
Hi Peter,
On 11/06/2017 03:33 PM, Peter Maydell wrote:
> SPARC is like Alpha in its handling of the rt_sigaction syscall:
> it takes an extra parameter 'restorer' which needs to be copied
> into the sa_restorer field of the sigaction struct. The order
> of the arguments differs slightly between SPARC and Alpha but
> the implementation is otherwise the same. (Compare the
> rt_sigaction() functions in arch/sparc/kernel/sys_sparc_64.c
> and arch/alpha/kernel/signal.c.)
>
> Note that this change is somewhat moot until SPARC acquires
> support for actually delivering RT signals.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> linux-user/syscall.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index d4497de..8beab51 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8556,8 +8556,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> case TARGET_NR_rt_sigaction:
> {
> - struct target_sigaction act, oact, *pact = 0;
> + /* For Alpha and SPARC this is a 5 argument syscall, with
> + * a 'restorer' parameter which must be copied into the
> + * sa_restorer field of the sigaction struct.
> + * For Alpha that 'restorer' is arg5; for SPARC it is arg4,
> + * and arg5 is the sigsetsize.
> + * Alpha also has a separate rt_sigaction struct that it uses
> + * here; SPARC uses the usual sigaction struct.
> + */
I find it easier to read moving the #if after the comment.
Since using restorer for sparc improve readability we can use it for
alpha as well (code is a bit more consistent):
#if defined(TARGET_ALPHA)
target_ulong restorer = arg5;
> struct target_rt_sigaction *rt_act;
> + struct target_sigaction act, oact, *pact = 0;
>
> if (arg4 != sizeof(target_sigset_t)) {
> ret = -TARGET_EINVAL;
...
act.sa_restorer = restorer;
Whichever way:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> @@ -8583,18 +8591,29 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> unlock_user_struct(rt_act, arg3, 1);
> }
> #else
> +#ifdef TARGET_SPARC
> + target_ulong restorer = arg4;
> + target_ulong sigsetsize = arg5;
> +#else
> + target_ulong sigsetsize = arg4;
> +#endif
> struct target_sigaction *act;
> struct target_sigaction *oact;
>
> - if (arg4 != sizeof(target_sigset_t)) {
> + if (sigsetsize != sizeof(target_sigset_t)) {
> ret = -TARGET_EINVAL;
> break;
> }
> if (arg2) {
> - if (!lock_user_struct(VERIFY_READ, act, arg2, 1))
> + if (!lock_user_struct(VERIFY_READ, act, arg2, 1)) {
> goto efault;
> - } else
> + }
> +#ifdef TARGET_SPARC
> + act->sa_restorer = restorer;
> +#endif
> + } else {
> act = NULL;
> + }
> if (arg3) {
> if (!lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) {
> ret = -TARGET_EFAULT;
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 0/4] linux-user: fix various SIGSEGV delivery bugs
2017-11-06 18:33 [Qemu-devel] [PATCH 0/4] linux-user: fix various SIGSEGV delivery bugs Peter Maydell
` (3 preceding siblings ...)
2017-11-06 18:33 ` [Qemu-devel] [PATCH 4/4] linux-user: Handle rt_sigaction correctly for SPARC Peter Maydell
@ 2017-11-07 20:02 ` Riku Voipio
4 siblings, 0 replies; 19+ messages in thread
From: Riku Voipio @ 2017-11-07 20:02 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, patches, Laurent Vivier
On Mon, Nov 06, 2017 at 06:33:22PM +0000, Peter Maydell wrote:
> This patchset fixes most of the problems with delivering SIGSEGV
> noted in https://bugs.launchpad.net/qemu/+bug/1705118:
> * s390x is missing masking operations on the fault address
> passed to the guest in si_addr
> * ppc is passing the PC of the offending insn, not the data
> address, into si_addr
> * sparc fails to record the address of a data fault and so
> passes 0 into si_addr
> * sparc isn't implementing rt_sigaction correctly (it has an
> extra 'restorer' argument, similar to alpha)
>
> It doesn't actually get the sparc testcase from that bug working,
> though, because SPARC is entirely missing support for setup_rt_frame()
> and do_rt_sigreturn(), so it can't deliver RT signals.
>
> Implementing RT signal delivery is too big a bugfix for an afternoon,
> so here are the simpler parts. I'll leave the signal delivery part
> to somebody who cares about SPARC guests...
>
> thanks
> -- PMM
Thanks!
series applied to linux-user tree,
Riku
> Peter Maydell (4):
> linux-user/s390x: Mask si_addr for SIGSEGV
> linux-user/ppc: Report correct fault address for data faults
> linux-user/sparc: Put address for data faults where linux-user expects
> it
> linux-user: Handle rt_sigaction correctly for SPARC
>
> linux-user/main.c | 8 ++++++--
> linux-user/syscall.c | 27 +++++++++++++++++++++++----
> target/sparc/mmu_helper.c | 8 ++++++++
> 3 files changed, 37 insertions(+), 6 deletions(-)
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 19+ messages in thread