Re: [Qemu-devel] [PATCH] target-sparc: fix register corruption in ldstub if there is no write permission
On 24/06/16 17:01, Artyom Tarasenko wrote: On Fri, Jun 24, 2016 at 5:51 PM, Mark Cave-Ayland wrote: On 24/06/16 13:34, Artyom Tarasenko wrote: Signed-off-by: Artyom Tarasenko --- target-sparc/translate.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/target-sparc/translate.c b/target-sparc/translate.c index 5111cf0..065326c 100644 --- a/target-sparc/translate.c +++ b/target-sparc/translate.c @@ -5187,12 +5187,15 @@ printf("ops, illegal rdhpr\n"); case 0xd: /* ldstub -- XXX: should be atomically */ { TCGv r_const; +TCGv tmp = tcg_temp_new(); gen_address_mask(dc, cpu_addr); -tcg_gen_qemu_ld8u(cpu_val, cpu_addr, dc->mem_idx); +tcg_gen_qemu_ld8u(tmp, cpu_addr, dc->mem_idx); r_const = tcg_const_tl(0xff); tcg_gen_qemu_st8(r_const, cpu_addr, dc->mem_idx); +tcg_gen_mov_tl(cpu_val, tmp); tcg_temp_free(r_const); +tcg_temp_free(tmp); } break; case 0x0f: Looks like you beat me to it - I can confirm that this fixes the issue here for me. Whilst testing I noticed another regression under qemu-system-sparc, however bisection reveals that this isn't caused by a SPARC-specific patch (and can be followed up separately) so: Tested-by: Mark Cave-Ayland Good. Then we can route it via your tree. (With Richard's Reviewed-by) I'm still worried why it didn't hit us before. Oops, looks like our mails overlapped. In that case I'll send a pull request ASAP. ATB, Mark.
Re: [Qemu-devel] [PATCH] target-sparc: fix register corruption in ldstub if there is no write permission
On 24/06/16 16:58, Richard Henderson wrote: On 06/24/2016 05:34 AM, Artyom Tarasenko wrote: Signed-off-by: Artyom Tarasenko --- target-sparc/translate.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/target-sparc/translate.c b/target-sparc/translate.c index 5111cf0..065326c 100644 --- a/target-sparc/translate.c +++ b/target-sparc/translate.c @@ -5187,12 +5187,15 @@ printf("ops, illegal rdhpr\n"); case 0xd: /* ldstub -- XXX: should be atomically */ { TCGv r_const; +TCGv tmp = tcg_temp_new(); gen_address_mask(dc, cpu_addr); -tcg_gen_qemu_ld8u(cpu_val, cpu_addr, dc->mem_idx); +tcg_gen_qemu_ld8u(tmp, cpu_addr, dc->mem_idx); r_const = tcg_const_tl(0xff); tcg_gen_qemu_st8(r_const, cpu_addr, dc->mem_idx); +tcg_gen_mov_tl(cpu_val, tmp); tcg_temp_free(r_const); +tcg_temp_free(tmp); ldstub_asi has the same problem on mainline. It looks like I fixed that one on my sparc branch though. r~ In that case would it make sense to prepend this patch to a v4 respin of your latest SPARC patchset (as tested by Artyom)? ATB, Mark.
Re: [Qemu-devel] [PATCH] target-sparc: fix register corruption in ldstub if there is no write permission
On Fri, Jun 24, 2016 at 5:51 PM, Mark Cave-Ayland wrote: > On 24/06/16 13:34, Artyom Tarasenko wrote: > >> Signed-off-by: Artyom Tarasenko >> --- >> target-sparc/translate.c | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/target-sparc/translate.c b/target-sparc/translate.c >> index 5111cf0..065326c 100644 >> --- a/target-sparc/translate.c >> +++ b/target-sparc/translate.c >> @@ -5187,12 +5187,15 @@ printf("ops, illegal rdhpr\n"); >> case 0xd: /* ldstub -- XXX: should be atomically */ >> { >> TCGv r_const; >> +TCGv tmp = tcg_temp_new(); >> >> gen_address_mask(dc, cpu_addr); >> -tcg_gen_qemu_ld8u(cpu_val, cpu_addr, >> dc->mem_idx); >> +tcg_gen_qemu_ld8u(tmp, cpu_addr, dc->mem_idx); >> r_const = tcg_const_tl(0xff); >> tcg_gen_qemu_st8(r_const, cpu_addr, dc->mem_idx); >> +tcg_gen_mov_tl(cpu_val, tmp); >> tcg_temp_free(r_const); >> +tcg_temp_free(tmp); >> } >> break; >> case 0x0f: >> > > Looks like you beat me to it - I can confirm that this fixes the issue here > for me. Whilst testing I noticed another regression under qemu-system-sparc, > however bisection reveals that this isn't caused by a SPARC-specific patch > (and can be followed up separately) so: > > Tested-by: Mark Cave-Ayland > Good. Then we can route it via your tree. (With Richard's Reviewed-by) I'm still worried why it didn't hit us before. Artyom -- Regards, Artyom Tarasenko SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu
Re: [Qemu-devel] [PATCH] target-sparc: fix register corruption in ldstub if there is no write permission
On 06/24/2016 05:34 AM, Artyom Tarasenko wrote: Signed-off-by: Artyom Tarasenko --- target-sparc/translate.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/target-sparc/translate.c b/target-sparc/translate.c index 5111cf0..065326c 100644 --- a/target-sparc/translate.c +++ b/target-sparc/translate.c @@ -5187,12 +5187,15 @@ printf("ops, illegal rdhpr\n"); case 0xd: /* ldstub -- XXX: should be atomically */ { TCGv r_const; +TCGv tmp = tcg_temp_new(); gen_address_mask(dc, cpu_addr); -tcg_gen_qemu_ld8u(cpu_val, cpu_addr, dc->mem_idx); +tcg_gen_qemu_ld8u(tmp, cpu_addr, dc->mem_idx); r_const = tcg_const_tl(0xff); tcg_gen_qemu_st8(r_const, cpu_addr, dc->mem_idx); +tcg_gen_mov_tl(cpu_val, tmp); tcg_temp_free(r_const); +tcg_temp_free(tmp); ldstub_asi has the same problem on mainline. It looks like I fixed that one on my sparc branch though. r~
Re: [Qemu-devel] [PATCH] target-sparc: fix register corruption in ldstub if there is no write permission
On 24/06/16 13:34, Artyom Tarasenko wrote: Signed-off-by: Artyom Tarasenko --- target-sparc/translate.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/target-sparc/translate.c b/target-sparc/translate.c index 5111cf0..065326c 100644 --- a/target-sparc/translate.c +++ b/target-sparc/translate.c @@ -5187,12 +5187,15 @@ printf("ops, illegal rdhpr\n"); case 0xd: /* ldstub -- XXX: should be atomically */ { TCGv r_const; +TCGv tmp = tcg_temp_new(); gen_address_mask(dc, cpu_addr); -tcg_gen_qemu_ld8u(cpu_val, cpu_addr, dc->mem_idx); +tcg_gen_qemu_ld8u(tmp, cpu_addr, dc->mem_idx); r_const = tcg_const_tl(0xff); tcg_gen_qemu_st8(r_const, cpu_addr, dc->mem_idx); +tcg_gen_mov_tl(cpu_val, tmp); tcg_temp_free(r_const); +tcg_temp_free(tmp); } break; case 0x0f: Looks like you beat me to it - I can confirm that this fixes the issue here for me. Whilst testing I noticed another regression under qemu-system-sparc, however bisection reveals that this isn't caused by a SPARC-specific patch (and can be followed up separately) so: Tested-by: Mark Cave-Ayland ATB, Mark.
Re: [Qemu-devel] [PATCH] target-sparc: fix register corruption in ldstub if there is no write permission
On 06/24/2016 05:34 AM, Artyom Tarasenko wrote: Signed-off-by: Artyom Tarasenko --- target-sparc/translate.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson r~