1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH for-4.0] target/ppc: Fix QEMU crash with stxsdx

Greg Kurz posted 1 patch 5 years ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/155371035249.2038502.12364252604337688538.stgit@bahia.lan
Maintainers: David Gibson <david@gibson.dropbear.id.au>
target/ppc/translate/vsx-impl.inc.c |    2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[Qemu-devel] [PATCH for-4.0] target/ppc: Fix QEMU crash with stxsdx
Posted by Greg Kurz 5 years ago 
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);                                    \
 }
Re: [Qemu-devel] [PATCH for-4.0] target/ppc: Fix QEMU crash with stxsdx
Posted by Mark Cave-Ayland 5 years ago 
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.
Re: [Qemu-devel] [PATCH for-4.0] target/ppc: Fix QEMU crash with stxsdx
Posted by David Gibson 5 years ago 
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.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.