| Message ID | 20231026001542.1141412-111-richard.henderson@linaro.org |
|---|---|
| State | Accepted |
| Commit | 5e3b17bbe9cc49c67d68f4a676113361944c8709 |
| Headers | show |
| Series | [PULL,01/94] target/sparc: Clear may_lookup for npc == DYNAMIC_PC | expand |
On 26/10/2023 01:15, Richard Henderson wrote: > Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > Acked-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/sparc/insns.decode | 1 + > target/sparc/translate.c | 41 +++++++++++++++++++++------------------ > 2 files changed, 23 insertions(+), 19 deletions(-) > > diff --git a/target/sparc/insns.decode b/target/sparc/insns.decode > index 6817d52ca2..a19d191603 100644 > --- a/target/sparc/insns.decode > +++ b/target/sparc/insns.decode > @@ -261,6 +261,7 @@ FDIVs 10 ..... 110100 ..... 0 0100 1101 ..... @r_r_r > FDIVd 10 ..... 110100 ..... 0 0100 1110 ..... @r_r_r > FDIVq 10 ..... 110100 ..... 0 0100 1111 ..... @r_r_r > FsMULd 10 ..... 110100 ..... 0 0110 1001 ..... @r_r_r > +FdMULq 10 ..... 110100 ..... 0 0110 1110 ..... @r_r_r > FdTOx 10 ..... 110100 00000 0 1000 0010 ..... @r_r2 > FxTOd 10 ..... 110100 00000 0 1000 1000 ..... @r_r2 > FiTOs 10 ..... 110100 00000 0 1100 0100 ..... @r_r2 > diff --git a/target/sparc/translate.c b/target/sparc/translate.c > index 6626042776..29643768e1 100644 > --- a/target/sparc/translate.c > +++ b/target/sparc/translate.c > @@ -1669,21 +1669,6 @@ static void gen_ne_fop_QQ(DisasContext *dc, int rd, int rs, > } > #endif > > -static void gen_fop_QDD(DisasContext *dc, int rd, int rs1, int rs2, > - void (*gen)(TCGv_ptr, TCGv_i64, TCGv_i64)) > -{ > - TCGv_i64 src1, src2; > - > - src1 = gen_load_fpr_D(dc, rs1); > - src2 = gen_load_fpr_D(dc, rs2); > - > - gen(tcg_env, src1, src2); > - gen_helper_check_ieee_exceptions(cpu_fsr, tcg_env); > - > - gen_op_store_QT0_fpr(QFPREG(rd)); > - gen_update_fprs_dirty(dc, QFPREG(rd)); > -} > - > #ifdef TARGET_SPARC64 > static void gen_fop_DF(DisasContext *dc, int rd, int rs, > void (*gen)(TCGv_i64, TCGv_ptr, TCGv_i32)) > @@ -4982,6 +4967,27 @@ TRANS(FSUBq, ALL, do_env_qqq, a, gen_helper_fsubq) > TRANS(FMULq, ALL, do_env_qqq, a, gen_helper_fmulq) > TRANS(FDIVq, ALL, do_env_qqq, a, gen_helper_fdivq) > > +static bool trans_FdMULq(DisasContext *dc, arg_r_r_r *a) > +{ > + TCGv_i64 src1, src2; > + > + if (gen_trap_ifnofpu(dc)) { > + return true; > + } > + if (gen_trap_float128(dc)) { > + return true; > + } > + > + gen_op_clear_ieee_excp_and_FTT(); > + src1 = gen_load_fpr_D(dc, a->rs1); > + src2 = gen_load_fpr_D(dc, a->rs2); > + gen_helper_fdmulq(tcg_env, src1, src2); > + gen_helper_check_ieee_exceptions(cpu_fsr, tcg_env); > + gen_op_store_QT0_fpr(QFPREG(a->rd)); > + gen_update_fprs_dirty(dc, QFPREG(a->rd)); > + return advance_pc(dc); > +} > + > #define CHECK_IU_FEATURE(dc, FEATURE) \ > if (!((dc)->def->features & CPU_FEATURE_ ## FEATURE)) \ > goto illegal_insn; > @@ -5048,11 +5054,8 @@ static void disas_sparc_legacy(DisasContext *dc, unsigned int insn) > case 0x4b: /* fmulq */ > case 0x4f: /* fdivq */ > case 0x69: /* fsmuld */ > - g_assert_not_reached(); /* in decodetree */ > case 0x6e: /* fdmulq */ > - CHECK_FPU_FEATURE(dc, FLOAT128); > - gen_fop_QDD(dc, rd, rs1, rs2, gen_helper_fdmulq); > - break; > + g_assert_not_reached(); /* in decodetree */ > case 0xc6: /* fdtos */ > gen_fop_FD(dc, rd, rs2, gen_helper_fdtos); > break; Hi Richard, I was working through my SPARC boot tests for your latest target/sparc series when I spotted a segfault on my FreeBSD SPARC64 image. A git bisect indicated that this was the patch that originally introduced the error, something I must have missed when testing the original decodetree conversion series. The reproducer is: ./qemu-system-sparc64 -m 256 -cdrom FreeBSD-10.3-RELEASE-sparc64-bootonly.iso \ -boot d -nographic and the error is a segfault in devd: ... ... Trying to mount root from cd9660:/dev/iso9660/10_3_RELEASE_SPARC64_BO [ro]... Entropy harvesting: interrupts ethernet point_to_point swi. Starting file system checks: Mounting local file systems:. Writing entropy file:. /etc/rc: WARNING: $hostname is not set -- see rc.conf(5). Starting Network: lo0 hme0. lo0: flags=8049<UP,LOOPBACK,RUNNING,MULTICAST> metric 0 mtu 16384 options=600003<RXCSUM,TXCSUM,RXCSUM_IPV6,TXCSUM_IPV6> inet6 ::1 prefixlen 128 inet6 fe80::1%lo0 prefixlen 64 scopeid 0x2 inet 127.0.0.1 netmask 0xff000000 nd6 options=21<PERFORMNUD,AUTO_LINKLOCAL> hme0: flags=8802<BROADCAST,SIMPLEX,MULTICAST> metric 0 mtu 1500 options=8000b<RXCSUM,TXCSUM,VLAN_MTU,LINKSTATE> ether 52:54:00:12:34:56 nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL> media: Ethernet autoselect Starting devd. pid 246 (ps), uid 0: exited on signal 11 Segmentation fault ^^^^^^^^^^^^^^^^^^ Starting Network: hme0. hme0: flags=8802<BROADCAST,SIMPLEX,MULTICAST> metric 0 mtu 1500 options=8000b<RXCSUM,TXCSUM,VLAN_MTU,LINKSTATE> ether 52:54:00:12:34:56 nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL> media: Ethernet autoselect ... ... ATB, Mark.
On 11/6/23 14:02, Mark Cave-Ayland wrote: > I was working through my SPARC boot tests for your latest target/sparc series when I > spotted a segfault on my FreeBSD SPARC64 image. A git bisect indicated that this was the > patch that originally introduced the error, something I must have missed when testing the > original decodetree conversion series. > > The reproducer is: > > ./qemu-system-sparc64 -m 256 -cdrom FreeBSD-10.3-RELEASE-sparc64-bootonly.iso \ > -boot d -nographic > > and the error is a segfault in devd: > > ... > ... > Trying to mount root from cd9660:/dev/iso9660/10_3_RELEASE_SPARC64_BO [ro]... > Entropy harvesting: interrupts ethernet point_to_point swi. > Starting file system checks: > Mounting local file systems:. > Writing entropy file:. > /etc/rc: WARNING: $hostname is not set -- see rc.conf(5). > Starting Network: lo0 hme0. > lo0: flags=8049<UP,LOOPBACK,RUNNING,MULTICAST> metric 0 mtu 16384 > options=600003<RXCSUM,TXCSUM,RXCSUM_IPV6,TXCSUM_IPV6> > inet6 ::1 prefixlen 128 > inet6 fe80::1%lo0 prefixlen 64 scopeid 0x2 > inet 127.0.0.1 netmask 0xff000000 > nd6 options=21<PERFORMNUD,AUTO_LINKLOCAL> > hme0: flags=8802<BROADCAST,SIMPLEX,MULTICAST> metric 0 mtu 1500 > options=8000b<RXCSUM,TXCSUM,VLAN_MTU,LINKSTATE> > ether 52:54:00:12:34:56 > nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL> > media: Ethernet autoselect > Starting devd. > pid 246 (ps), uid 0: exited on signal 11 > Segmentation fault > ^^^^^^^^^^^^^^^^^^ I certainly can't imagine that FdMULq is really at fault, because it's not implemented on real hardware (and thus I really doubt FreeBSD attempted to use it), and CPU_FEATURE_FLOAT128 is not enabled by your command-line. The only thing that I can imagine is that this is some sort of timing related issue and bisect behaved randomly. All that said, I can't replicate this with master. Can you, now? r~
On 07/11/2023 04:49, Richard Henderson wrote: > On 11/6/23 14:02, Mark Cave-Ayland wrote: >> I was working through my SPARC boot tests for your latest target/sparc series when >> I spotted a segfault on my FreeBSD SPARC64 image. A git bisect indicated that this >> was the patch that originally introduced the error, something I must have missed >> when testing the original decodetree conversion series. >> >> The reproducer is: >> >> ./qemu-system-sparc64 -m 256 -cdrom FreeBSD-10.3-RELEASE-sparc64-bootonly.iso \ >> -boot d -nographic >> >> and the error is a segfault in devd: >> >> ... >> ... >> Trying to mount root from cd9660:/dev/iso9660/10_3_RELEASE_SPARC64_BO [ro]... >> Entropy harvesting: interrupts ethernet point_to_point swi. >> Starting file system checks: >> Mounting local file systems:. >> Writing entropy file:. >> /etc/rc: WARNING: $hostname is not set -- see rc.conf(5). >> Starting Network: lo0 hme0. >> lo0: flags=8049<UP,LOOPBACK,RUNNING,MULTICAST> metric 0 mtu 16384 >> options=600003<RXCSUM,TXCSUM,RXCSUM_IPV6,TXCSUM_IPV6> >> inet6 ::1 prefixlen 128 >> inet6 fe80::1%lo0 prefixlen 64 scopeid 0x2 >> inet 127.0.0.1 netmask 0xff000000 >> nd6 options=21<PERFORMNUD,AUTO_LINKLOCAL> >> hme0: flags=8802<BROADCAST,SIMPLEX,MULTICAST> metric 0 mtu 1500 >> options=8000b<RXCSUM,TXCSUM,VLAN_MTU,LINKSTATE> >> ether 52:54:00:12:34:56 >> nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL> >> media: Ethernet autoselect >> Starting devd. >> pid 246 (ps), uid 0: exited on signal 11 >> Segmentation fault >> ^^^^^^^^^^^^^^^^^^ > > > I certainly can't imagine that FdMULq is really at fault, because it's not > implemented on real hardware (and thus I really doubt FreeBSD attempted to use it), > and CPU_FEATURE_FLOAT128 is not enabled by your command-line. > > The only thing that I can imagine is that this is some sort of timing related issue > and bisect behaved randomly. Hmmm you're right, it seems that there is a semi-random aspect to the issue which is why the bisection didn't give a good result. In order to mitigate this, I repeated the bisection again but this time booting the FreeBSD image 5 times in a row, only marking the commit as good if all 5 boot tests passed [1] without displaying a segfault message [2]. That bisection led me to this commit: 86b82fe021f46ed4501b16132f7e3fccd0a1ad5d is the first bad commit commit 86b82fe021f46ed4501b16132f7e3fccd0a1ad5d Author: Richard Henderson <richard.henderson@linaro.org> Date: Wed Oct 4 17:51:37 2023 -0700 target/sparc: Move JMPL, RETT, RETURN to decodetree Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Acked-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> target/sparc/insns.decode | 7 +++ target/sparc/translate.c | 126 +++++++++++++++++++++++++++++----------------- 2 files changed, 88 insertions(+), 45 deletions(-) I tried repeating the boot 5 times in a row test on both this commit and the commit before it, and that appeared to confirm the bisection result with the failures only appearing with commit 86b82fe021. > All that said, I can't replicate this with master. > Can you, now? Yes, the issue is still present in master for me. My host is Debian bookworm (stable, x86_64) configured as './configure' '--target-list=sparc64-softmmu' '--enable-slirp' '--enable-debug' and the command line is: ./qemu-system-sparc64 -m 256 -cdrom FreeBSD-10.3-RELEASE-sparc64-bootonly.iso \ -boot d -nographic ATB, Mark. [1] Sometimes the image hangs just after "Trying to mount root .." but that appears to be a timing issue and not directly related to this series. In this cases where this happened, I simply quit QEMU and rebooted the image again. [2] Normally the segfault message comes from devd, but during the bisection I did occasionally see it coming from other processes.
diff --git a/target/sparc/insns.decode b/target/sparc/insns.decode index 6817d52ca2..a19d191603 100644 --- a/target/sparc/insns.decode +++ b/target/sparc/insns.decode @@ -261,6 +261,7 @@ FDIVs 10 ..... 110100 ..... 0 0100 1101 ..... @r_r_r FDIVd 10 ..... 110100 ..... 0 0100 1110 ..... @r_r_r FDIVq 10 ..... 110100 ..... 0 0100 1111 ..... @r_r_r FsMULd 10 ..... 110100 ..... 0 0110 1001 ..... @r_r_r +FdMULq 10 ..... 110100 ..... 0 0110 1110 ..... @r_r_r FdTOx 10 ..... 110100 00000 0 1000 0010 ..... @r_r2 FxTOd 10 ..... 110100 00000 0 1000 1000 ..... @r_r2 FiTOs 10 ..... 110100 00000 0 1100 0100 ..... @r_r2 diff --git a/target/sparc/translate.c b/target/sparc/translate.c index 6626042776..29643768e1 100644 --- a/target/sparc/translate.c +++ b/target/sparc/translate.c @@ -1669,21 +1669,6 @@ static void gen_ne_fop_QQ(DisasContext *dc, int rd, int rs, } #endif -static void gen_fop_QDD(DisasContext *dc, int rd, int rs1, int rs2, - void (*gen)(TCGv_ptr, TCGv_i64, TCGv_i64)) -{ - TCGv_i64 src1, src2; - - src1 = gen_load_fpr_D(dc, rs1); - src2 = gen_load_fpr_D(dc, rs2); - - gen(tcg_env, src1, src2); - gen_helper_check_ieee_exceptions(cpu_fsr, tcg_env); - - gen_op_store_QT0_fpr(QFPREG(rd)); - gen_update_fprs_dirty(dc, QFPREG(rd)); -} - #ifdef TARGET_SPARC64 static void gen_fop_DF(DisasContext *dc, int rd, int rs, void (*gen)(TCGv_i64, TCGv_ptr, TCGv_i32)) @@ -4982,6 +4967,27 @@ TRANS(FSUBq, ALL, do_env_qqq, a, gen_helper_fsubq) TRANS(FMULq, ALL, do_env_qqq, a, gen_helper_fmulq) TRANS(FDIVq, ALL, do_env_qqq, a, gen_helper_fdivq) +static bool trans_FdMULq(DisasContext *dc, arg_r_r_r *a) +{ + TCGv_i64 src1, src2; + + if (gen_trap_ifnofpu(dc)) { + return true; + } + if (gen_trap_float128(dc)) { + return true; + } + + gen_op_clear_ieee_excp_and_FTT(); + src1 = gen_load_fpr_D(dc, a->rs1); + src2 = gen_load_fpr_D(dc, a->rs2); + gen_helper_fdmulq(tcg_env, src1, src2); + gen_helper_check_ieee_exceptions(cpu_fsr, tcg_env); + gen_op_store_QT0_fpr(QFPREG(a->rd)); + gen_update_fprs_dirty(dc, QFPREG(a->rd)); + return advance_pc(dc); +} + #define CHECK_IU_FEATURE(dc, FEATURE) \ if (!((dc)->def->features & CPU_FEATURE_ ## FEATURE)) \ goto illegal_insn; @@ -5048,11 +5054,8 @@ static void disas_sparc_legacy(DisasContext *dc, unsigned int insn) case 0x4b: /* fmulq */ case 0x4f: /* fdivq */ case 0x69: /* fsmuld */ - g_assert_not_reached(); /* in decodetree */ case 0x6e: /* fdmulq */ - CHECK_FPU_FEATURE(dc, FLOAT128); - gen_fop_QDD(dc, rd, rs1, rs2, gen_helper_fdmulq); - break; + g_assert_not_reached(); /* in decodetree */ case 0xc6: /* fdtos */ gen_fop_FD(dc, rd, rs2, gen_helper_fdtos); break;