qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] target/ppc: Fix SPE unavailable exception triggering


From: BALATON Zoltan
Subject: Re: [PATCH] target/ppc: Fix SPE unavailable exception triggering
Date: Sun, 26 Jul 2020 12:04:58 +0200 (CEST)
User-agent: Alpine 2.22 (BSF 395 2020-01-19)

On Sat, 25 Jul 2020, Matthieu Bucchianeri wrote:
When emulating certain floating point instructions or vector instructions on
PowerPC machines, QEMU did not properly generate the SPE/Embedded Floating-
Point Unavailable interrupt. See the buglink further below for references to
the relevant NXP documentation.

This patch fixes the behavior of some evfs* instructions that were
incorrectly emitting the interrupt.

More importantly, this patch fixes the behavior of several efd* and ev*
instructions that were not generating the interrupt. Triggering the
interrupt for these instructions fixes lazy FPU/vector context switching on
some operating systems like Linux.

Without this patch, the result of some double-precision arithmetic could be
corrupted due to the lack of proper saving and restoring of the upper
32-bit part of the general-purpose registers.

Buglink: https://bugs.launchpad.net/qemu/+bug/1888918
Buglink: https://bugs.launchpad.net/qemu/+bug/1611394
Signed-off-by: Matthieu Bucchianeri <matthieu.bucchianeri@leostella.com>
---
target/ppc/translate/spe-impl.inc.c | 101 ++++++++++++++++++----------
1 file changed, 66 insertions(+), 35 deletions(-)

diff --git a/target/ppc/translate/spe-impl.inc.c 
b/target/ppc/translate/spe-impl.inc.c
index 36b4d5654d..2e6e799a25 100644
--- a/target/ppc/translate/spe-impl.inc.c
+++ b/target/ppc/translate/spe-impl.inc.c
@@ -349,14 +349,24 @@ static inline void gen_evmergelohi(DisasContext *ctx)
}
static inline void gen_evsplati(DisasContext *ctx)
{
-    uint64_t imm = ((int32_t)(rA(ctx->opcode) << 27)) >> 27;
+    uint64_t imm;
+    if (unlikely(!ctx->spe_enabled)) {
+        gen_exception(ctx, POWERPC_EXCP_SPEU);
+        return;
+    }
+    imm = ((int32_t)(rA(ctx->opcode) << 27)) >> 27;

    tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], imm);
    tcg_gen_movi_tl(cpu_gprh[rD(ctx->opcode)], imm);
}
static inline void gen_evsplatfi(DisasContext *ctx)
{
-    uint64_t imm = rA(ctx->opcode) << 27;
+    uint64_t imm;
+    if (unlikely(!ctx->spe_enabled)) {
+        gen_exception(ctx, POWERPC_EXCP_SPEU);
+        return;
+    }
+    imm = rA(ctx->opcode) << 27;

    tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], imm);
    tcg_gen_movi_tl(cpu_gprh[rD(ctx->opcode)], imm);
@@ -389,21 +399,37 @@ static inline void gen_evsel(DisasContext *ctx)

static void gen_evsel0(DisasContext *ctx)
{
+    if (unlikely(!ctx->spe_enabled)) {
+        gen_exception(ctx, POWERPC_EXCP_SPEU);
+        return;
+    }
    gen_evsel(ctx);
}

static void gen_evsel1(DisasContext *ctx)
{
+    if (unlikely(!ctx->spe_enabled)) {
+        gen_exception(ctx, POWERPC_EXCP_SPEU);
+        return;
+    }
    gen_evsel(ctx);
}

static void gen_evsel2(DisasContext *ctx)
{
+    if (unlikely(!ctx->spe_enabled)) {
+        gen_exception(ctx, POWERPC_EXCP_SPEU);
+        return;
+    }
    gen_evsel(ctx);
}

static void gen_evsel3(DisasContext *ctx)
{
+    if (unlikely(!ctx->spe_enabled)) {
+        gen_exception(ctx, POWERPC_EXCP_SPEU);
+        return;
+    }
    gen_evsel(ctx);
}

@@ -518,6 +544,11 @@ static inline void gen_evmwsmia(DisasContext *ctx)
{
    TCGv_i64 tmp;

+    if (unlikely(!ctx->spe_enabled)) {
+        gen_exception(ctx, POWERPC_EXCP_SPEU);
+        return;
+    }
+
    gen_evmwsmi(ctx);            /* rD := rA * rB */

    tmp = tcg_temp_new_i64();
@@ -531,8 +562,13 @@ static inline void gen_evmwsmia(DisasContext *ctx)

static inline void gen_evmwsmiaa(DisasContext *ctx)
{
-    TCGv_i64 acc = tcg_temp_new_i64();
-    TCGv_i64 tmp = tcg_temp_new_i64();
+    TCGv_i64 acc;
+    TCGv_i64 tmp;
+
+    if (unlikely(!ctx->spe_enabled)) {
+        gen_exception(ctx, POWERPC_EXCP_SPEU);
+        return;
+    }

Isn't this missing here:

acc = tcg_temp_new_i64();
tmp = tcg_temp_new_i64();

You've removed allocating temps but this hunk does not add it back after the exception unlike others in this patch.

Regards,
BALATON Zoltan


    gen_evmwsmi(ctx);           /* rD := rA * rB */

@@ -892,8 +928,14 @@ static inline void gen_##name(DisasContext *ctx)           
                   \
#define GEN_SPEFPUOP_CONV_32_64(name)                                         \
static inline void gen_##name(DisasContext *ctx)                              \
{                                                                             \
-    TCGv_i64 t0 = tcg_temp_new_i64();                                         \
-    TCGv_i32 t1 = tcg_temp_new_i32();                                         \
+    TCGv_i64 t0;                                                              \
+    TCGv_i32 t1;                                                              \
+    if (unlikely(!ctx->spe_enabled)) {                                        \
+        gen_exception(ctx, POWERPC_EXCP_SPEU);                                \
+        return;                                                               \
+    }                                                                         \
+    t0 = tcg_temp_new_i64();                                                  \
+    t1 = tcg_temp_new_i32();                                                  \
    gen_load_gpr64(t0, rB(ctx->opcode));                                      \
    gen_helper_##name(t1, cpu_env, t0);                                       \
    tcg_gen_extu_i32_tl(cpu_gpr[rD(ctx->opcode)], t1);                        \
@@ -903,8 +945,14 @@ static inline void gen_##name(DisasContext *ctx)           
                   \
#define GEN_SPEFPUOP_CONV_64_32(name)                                         \
static inline void gen_##name(DisasContext *ctx)                              \
{                                                                             \
-    TCGv_i64 t0 = tcg_temp_new_i64();                                         \
-    TCGv_i32 t1 = tcg_temp_new_i32();                                         \
+    TCGv_i64 t0;                                                              \
+    TCGv_i32 t1;                                                              \
+    if (unlikely(!ctx->spe_enabled)) {                                        \
+        gen_exception(ctx, POWERPC_EXCP_SPEU);                                \
+        return;                                                               \
+    }                                                                         \
+    t0 = tcg_temp_new_i64();                                                  \
+    t1 = tcg_temp_new_i32();                                                  \
    tcg_gen_trunc_tl_i32(t1, cpu_gpr[rB(ctx->opcode)]);                       \
    gen_helper_##name(t0, cpu_env, t1);                                       \
    gen_store_gpr64(rD(ctx->opcode), t0);                                     \
@@ -914,7 +962,12 @@ static inline void gen_##name(DisasContext *ctx)           
                   \
#define GEN_SPEFPUOP_CONV_64_64(name)                                         \
static inline void gen_##name(DisasContext *ctx)                              \
{                                                                             \
-    TCGv_i64 t0 = tcg_temp_new_i64();                                         \
+    TCGv_i64 t0;                                                              \
+    if (unlikely(!ctx->spe_enabled)) {                                        \
+        gen_exception(ctx, POWERPC_EXCP_SPEU);                                \
+        return;                                                               \
+    }                                                                         \
+    t0 = tcg_temp_new_i64();                                                  \
    gen_load_gpr64(t0, rB(ctx->opcode));                                      \
    gen_helper_##name(t0, cpu_env, t0);                                       \
    gen_store_gpr64(rD(ctx->opcode), t0);                                     \
@@ -923,13 +976,8 @@ static inline void gen_##name(DisasContext *ctx)           
                   \
#define GEN_SPEFPUOP_ARITH2_32_32(name)                                       \
static inline void gen_##name(DisasContext *ctx)                              \
{                                                                             \
-    TCGv_i32 t0, t1;                                                          \
-    if (unlikely(!ctx->spe_enabled)) {                                        \
-        gen_exception(ctx, POWERPC_EXCP_SPEU);                                \
-        return;                                                               \
-    }                                                                         \
-    t0 = tcg_temp_new_i32();                                                  \
-    t1 = tcg_temp_new_i32();                                                  \
+    TCGv_i32 t0 = tcg_temp_new_i32();                                         \
+    TCGv_i32 t1 = tcg_temp_new_i32();                                         \
    tcg_gen_trunc_tl_i32(t0, cpu_gpr[rA(ctx->opcode)]);                       \
    tcg_gen_trunc_tl_i32(t1, cpu_gpr[rB(ctx->opcode)]);                       \
    gen_helper_##name(t0, cpu_env, t0, t1);                                   \
@@ -958,13 +1006,8 @@ static inline void gen_##name(DisasContext *ctx)          
                    \
#define GEN_SPEFPUOP_COMP_32(name)                                            \
static inline void gen_##name(DisasContext *ctx)                              \
{                                                                             \
-    TCGv_i32 t0, t1;                                                          \
-    if (unlikely(!ctx->spe_enabled)) {                                        \
-        gen_exception(ctx, POWERPC_EXCP_SPEU);                                \
-        return;                                                               \
-    }                                                                         \
-    t0 = tcg_temp_new_i32();                                                  \
-    t1 = tcg_temp_new_i32();                                                  \
+    TCGv_i32 t0 = tcg_temp_new_i32();                                         \
+    TCGv_i32 t1 = tcg_temp_new_i32();                                         \
                                                                              \
    tcg_gen_trunc_tl_i32(t0, cpu_gpr[rA(ctx->opcode)]);                       \
    tcg_gen_trunc_tl_i32(t1, cpu_gpr[rB(ctx->opcode)]);                       \
@@ -1074,28 +1117,16 @@ GEN_SPEFPUOP_ARITH2_32_32(efsmul);
GEN_SPEFPUOP_ARITH2_32_32(efsdiv);
static inline void gen_efsabs(DisasContext *ctx)
{
-    if (unlikely(!ctx->spe_enabled)) {
-        gen_exception(ctx, POWERPC_EXCP_SPEU);
-        return;
-    }
    tcg_gen_andi_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
                    (target_long)~0x80000000LL);
}
static inline void gen_efsnabs(DisasContext *ctx)
{
-    if (unlikely(!ctx->spe_enabled)) {
-        gen_exception(ctx, POWERPC_EXCP_SPEU);
-        return;
-    }
    tcg_gen_ori_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
                   0x80000000);
}
static inline void gen_efsneg(DisasContext *ctx)
{
-    if (unlikely(!ctx->spe_enabled)) {
-        gen_exception(ctx, POWERPC_EXCP_SPEU);
-        return;
-    }
    tcg_gen_xori_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
                    0x80000000);
}
--
2.17.1

LeoStella, LLC
A joint venture of Thales Alenia Space and Spaceflight Industries

12501 E Marginal Way S • Tukwila, WA 98168

Proprietary Document: This document may contain trade secrets or otherwise 
proprietary and confidential information owned by LeoStella LLC. It is intended 
only for the individual addressee and may not be copied, distributed, or 
otherwise disclosed without LeoStella LLC express prior written authorization.
Export Controlled: This document may contain technical data whose export is 
restricted by the Arms Export Control Act (Title 22, U.S.C., Sec 2751 et seq.) 
or the Export Administration Act of 1979, as amended, Title 50,U.S.C., app 2401 
et seq. Violation of these export laws are subject to severe criminal 
penalties. Recipient shall not export, re-export, or otherwise transfer or 
share this document to any foreign person (as defined by U.S. export laws) 
without advance written authorization from LeoStella LLC.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]