| Message ID | 576786e68176302163d09977c6c8c3eb6d7a8ee5.1466771151.git.atar4qemu@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 24/06/16 13:34, Artyom Tarasenko wrote: > Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com> > --- > 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 <mark.cave-ayland@ilande.co.uk> ATB, Mark.
On 06/24/2016 05:34 AM, Artyom Tarasenko wrote: > Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com> > --- > target-sparc/translate.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson <rth@twiddle.net> r~
On 06/24/2016 05:34 AM, Artyom Tarasenko wrote: > Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com> > --- > 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~
On Fri, Jun 24, 2016 at 5:51 PM, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > On 24/06/16 13:34, Artyom Tarasenko wrote: > >> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com> >> --- >> 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 <mark.cave-ayland@ilande.co.uk> > 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
On 24/06/16 16:58, Richard Henderson wrote: > On 06/24/2016 05:34 AM, Artyom Tarasenko wrote: >> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com> >> --- >> 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.
On 24/06/16 17:01, Artyom Tarasenko wrote: > On Fri, Jun 24, 2016 at 5:51 PM, Mark Cave-Ayland > <mark.cave-ayland@ilande.co.uk> wrote: >> On 24/06/16 13:34, Artyom Tarasenko wrote: >> >>> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com> >>> --- >>> 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 <mark.cave-ayland@ilande.co.uk> >> > > 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.
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:
Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com> --- target-sparc/translate.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)