[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH for-4.0] target/ppc: Fix QEMU crash w
From: |
Mark Cave-Ayland |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH for-4.0] target/ppc: Fix QEMU crash with stxsdx |
Date: |
Wed, 27 Mar 2019 19:59:12 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 |
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 <address@hidden>
> ---
> 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 <address@hidden>
ATB,
Mark.