| Message ID | 155371035249.2038502.12364252604337688538.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
| Series | [for-4.0] target/ppc: Fix QEMU crash with stxsdx | expand |
On 27/03/2019 18:12, Greg Kurz wrote: > I've been hitting several QEMU crashes while running a fedora29 ppc64le > guest under TCG. Each time, this would occur several minutes after the > guest reached login: > > Fedora 29 (Twenty Nine) > Kernel 4.20.6-200.fc29.ppc64le on an ppc64le (hvc0) > > Web console: https://localhost:9090/ > > localhost login: > tcg/tcg.c:3211: tcg fatal error > > This happens because a bug crept up in the gen_stxsdx() helper when it > was converted to use VSR register accessors by commit 8b3b2d75c7c04 > "target/ppc: introduce get_cpu_vsr{l,h}() and set_cpu_vsr{l,h}() helpers > for VSR register access". > > The code creates a temporary, passes it directly to gen_qemu_st64_i64() > and then to set_cpu_vrsh()... which looks like this was mistakenly > coded as a load instead of a store. > > Reverse the logic: read the VSR to the temporary first and then store > it to memory. > > Fixes: 8b3b2d75c7c0481544e277dad226223245e058eb > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > target/ppc/translate/vsx-impl.inc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/ppc/translate/vsx-impl.inc.c b/target/ppc/translate/vsx-impl.inc.c > index 508e9199c8df..489b2436e49f 100644 > --- a/target/ppc/translate/vsx-impl.inc.c > +++ b/target/ppc/translate/vsx-impl.inc.c > @@ -356,8 +356,8 @@ static void gen_##name(DisasContext *ctx) \ > gen_set_access_type(ctx, ACCESS_INT); \ > EA = tcg_temp_new(); \ > gen_addr_reg_index(ctx, EA); \ > + get_cpu_vsrh(t0, xS(ctx->opcode)); \ > gen_qemu_##operation(ctx, t0, EA); \ > - set_cpu_vsrh(xS(ctx->opcode), t0); \ > tcg_temp_free(EA); \ > tcg_temp_free_i64(t0); \ > } Gah, yes - I remember at one point having to double check the get/set direction but obviously my brain and fingers got the better of me here. Apologies for the inconvenience. Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> ATB, Mark.
On Wed, Mar 27, 2019 at 07:59:12PM +0000, Mark Cave-Ayland wrote: > On 27/03/2019 18:12, Greg Kurz wrote: > > > I've been hitting several QEMU crashes while running a fedora29 ppc64le > > guest under TCG. Each time, this would occur several minutes after the > > guest reached login: > > > > Fedora 29 (Twenty Nine) > > Kernel 4.20.6-200.fc29.ppc64le on an ppc64le (hvc0) > > > > Web console: https://localhost:9090/ > > > > localhost login: > > tcg/tcg.c:3211: tcg fatal error > > > > This happens because a bug crept up in the gen_stxsdx() helper when it > > was converted to use VSR register accessors by commit 8b3b2d75c7c04 > > "target/ppc: introduce get_cpu_vsr{l,h}() and set_cpu_vsr{l,h}() helpers > > for VSR register access". > > > > The code creates a temporary, passes it directly to gen_qemu_st64_i64() > > and then to set_cpu_vrsh()... which looks like this was mistakenly > > coded as a load instead of a store. > > > > Reverse the logic: read the VSR to the temporary first and then store > > it to memory. > > > > Fixes: 8b3b2d75c7c0481544e277dad226223245e058eb > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > target/ppc/translate/vsx-impl.inc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/target/ppc/translate/vsx-impl.inc.c b/target/ppc/translate/vsx-impl.inc.c > > index 508e9199c8df..489b2436e49f 100644 > > --- a/target/ppc/translate/vsx-impl.inc.c > > +++ b/target/ppc/translate/vsx-impl.inc.c > > @@ -356,8 +356,8 @@ static void gen_##name(DisasContext *ctx) \ > > gen_set_access_type(ctx, ACCESS_INT); \ > > EA = tcg_temp_new(); \ > > gen_addr_reg_index(ctx, EA); \ > > + get_cpu_vsrh(t0, xS(ctx->opcode)); \ > > gen_qemu_##operation(ctx, t0, EA); \ > > - set_cpu_vsrh(xS(ctx->opcode), t0); \ > > tcg_temp_free(EA); \ > > tcg_temp_free_i64(t0); \ > > } > > Gah, yes - I remember at one point having to double check the get/set direction but > obviously my brain and fingers got the better of me here. Apologies for the > inconvenience. > > Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Applied, thanks.
diff --git a/target/ppc/translate/vsx-impl.inc.c b/target/ppc/translate/vsx-impl.inc.c index 508e9199c8df..489b2436e49f 100644 --- a/target/ppc/translate/vsx-impl.inc.c +++ b/target/ppc/translate/vsx-impl.inc.c @@ -356,8 +356,8 @@ static void gen_##name(DisasContext *ctx) \ gen_set_access_type(ctx, ACCESS_INT); \ EA = tcg_temp_new(); \ gen_addr_reg_index(ctx, EA); \ + get_cpu_vsrh(t0, xS(ctx->opcode)); \ gen_qemu_##operation(ctx, t0, EA); \ - set_cpu_vsrh(xS(ctx->opcode), t0); \ tcg_temp_free(EA); \ tcg_temp_free_i64(t0); \ }
I've been hitting several QEMU crashes while running a fedora29 ppc64le guest under TCG. Each time, this would occur several minutes after the guest reached login: Fedora 29 (Twenty Nine) Kernel 4.20.6-200.fc29.ppc64le on an ppc64le (hvc0) Web console: https://localhost:9090/ localhost login: tcg/tcg.c:3211: tcg fatal error This happens because a bug crept up in the gen_stxsdx() helper when it was converted to use VSR register accessors by commit 8b3b2d75c7c04 "target/ppc: introduce get_cpu_vsr{l,h}() and set_cpu_vsr{l,h}() helpers for VSR register access". The code creates a temporary, passes it directly to gen_qemu_st64_i64() and then to set_cpu_vrsh()... which looks like this was mistakenly coded as a load instead of a store. Reverse the logic: read the VSR to the temporary first and then store it to memory. Fixes: 8b3b2d75c7c0481544e277dad226223245e058eb Signed-off-by: Greg Kurz <groug@kaod.org> --- target/ppc/translate/vsx-impl.inc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)