* [PATCH] target/ppc: Fix SPE unavailable exception triggering
@ 2020-07-25 19:14 Matthieu Bucchianeri
2020-07-25 21:04 ` no-reply
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Matthieu Bucchianeri @ 2020-07-25 19:14 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Matthieu Bucchianeri, david
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;
+ }
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.
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] target/ppc: Fix SPE unavailable exception triggering
2020-07-25 19:14 [PATCH] target/ppc: Fix SPE unavailable exception triggering Matthieu Bucchianeri
@ 2020-07-25 21:04 ` no-reply
2020-07-25 21:04 ` no-reply
2020-07-26 10:04 ` BALATON Zoltan
2 siblings, 0 replies; 7+ messages in thread
From: no-reply @ 2020-07-25 21:04 UTC (permalink / raw)
To: matthieu.bucchianeri; +Cc: qemu-ppc, qemu-devel, matthieu.bucchianeri, david
Patchew URL: https://patchew.org/QEMU/20200725191436.31828-1-matthieu.bucchianeri@leostella.com/
Hi,
This series failed build test on FreeBSD host. Please find the details below.
The full log is available at
http://patchew.org/logs/20200725191436.31828-1-matthieu.bucchianeri@leostella.com/testing.FreeBSD/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/ppc: Fix SPE unavailable exception triggering
2020-07-25 19:14 [PATCH] target/ppc: Fix SPE unavailable exception triggering Matthieu Bucchianeri
2020-07-25 21:04 ` no-reply
@ 2020-07-25 21:04 ` no-reply
2020-07-26 4:07 ` Matthieu Bucchianeri
2020-07-26 10:04 ` BALATON Zoltan
2 siblings, 1 reply; 7+ messages in thread
From: no-reply @ 2020-07-25 21:04 UTC (permalink / raw)
To: matthieu.bucchianeri; +Cc: qemu-ppc, qemu-devel, matthieu.bucchianeri, david
Patchew URL: https://patchew.org/QEMU/20200725191436.31828-1-matthieu.bucchianeri@leostella.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20200725191436.31828-1-matthieu.bucchianeri@leostella.com
Subject: [PATCH] target/ppc: Fix SPE unavailable exception triggering
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
From https://github.com/patchew-project/qemu
7adfbea..b0ce3f0 master -> master
From https://github.com/patchew-project/qemu
* [new tag] patchew/20200725191436.31828-1-matthieu.bucchianeri@leostella.com -> patchew/20200725191436.31828-1-matthieu.bucchianeri@leostella.com
fatal: failed to write ref-pack file
fatal: The remote end hung up unexpectedly
Traceback (most recent call last):
File "patchew-tester2/src/patchew-cli", line 531, in test_one
git_clone_repo(clone, r["repo"], r["head"], logf, True)
File "patchew-tester2/src/patchew-cli", line 62, in git_clone_repo
subprocess.check_call(clone_cmd, stderr=logf, stdout=logf)
File "/opt/rh/rh-python36/root/usr/lib64/python3.6/subprocess.py", line 291, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'clone', '-q', '/home/patchew2/.cache/patchew-git-cache/httpsgithubcompatchewprojectqemu-3c8cf5a9c21ff8782164d1def7f44bd888713384', '/var/tmp/patchew-tester-tmp-xmt7q0s9/src']' returned non-zero exit status 128.
The full log is available at
http://patchew.org/logs/20200725191436.31828-1-matthieu.bucchianeri@leostella.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] target/ppc: Fix SPE unavailable exception triggering
2020-07-25 21:04 ` no-reply
@ 2020-07-26 4:07 ` Matthieu Bucchianeri
0 siblings, 0 replies; 7+ messages in thread
From: Matthieu Bucchianeri @ 2020-07-26 4:07 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: david
This looks like an infrastructure issue. Nothing in the log seems to indicate an issue.
checkpatch was run locally, and passed.
# ./scripts/checkpatch.pl 0001-target-ppc-Fix-SPE-unavailable-exception-triggering.patch
total: 0 errors, 0 warnings, 192 lines checked
0001-target-ppc-Fix-SPE-unavailable-exception-triggering.patch has no obvious style problems and is ready for submission.
Please let me know if any other action is required.
-----Original Message-----
From: no-reply@patchew.org <no-reply@patchew.org>
Sent: Saturday, July 25, 2020 2:04 PM
To: Matthieu Bucchianeri <matthieu.bucchianeri@leostella.com>
Cc: qemu-devel@nongnu.org; qemu-ppc@nongnu.org; Matthieu Bucchianeri <matthieu.bucchianeri@leostella.com>; david@gibson.dropbear.id.au
Subject: Re: [PATCH] target/ppc: Fix SPE unavailable exception triggering
External Message
Patchew URL: https://usg02.safelinks.protection.office365.us/?url=https%3A%2F%2Fpatchew.org%2FQEMU%2F20200725191436.31828-1-matthieu.bucchianeri%40leostella.com%2F&data=02%7C01%7Cmatthieu.bucchianeri%40leostella.com%7C0e0c3fd9fbfb4349331c08d830de600c%7Cdbc51146ab26404b9c4bcce4f3331cc5%7C1%7C0%7C637313078942748821&sdata=VsssvL40%2BF6ALCN1ifpeTLzRQBpF4rq4N%2BY1zCMsCOg%3D&reserved=0
Hi,
This series seems to have some coding style problems. See output below for more information:
Type: series
Message-id: 20200725191436.31828-1-matthieu.bucchianeri@leostella.com
Subject: [PATCH] target/ppc: Fix SPE unavailable exception triggering
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
From https://usg02.safelinks.protection.office365.us/?url=https%3A%2F%2Fgithub.com%2Fpatchew-project%2Fqemu&data=02%7C01%7Cmatthieu.bucchianeri%40leostella.com%7C0e0c3fd9fbfb4349331c08d830de600c%7Cdbc51146ab26404b9c4bcce4f3331cc5%7C1%7C0%7C637313078942748821&sdata=3K5lhzhLRHqKcVhazaFA7L7IbNLiJgrBkli68vsRvE4%3D&reserved=0
7adfbea..b0ce3f0 master -> master
From https://usg02.safelinks.protection.office365.us/?url=https%3A%2F%2Fgithub.com%2Fpatchew-project%2Fqemu&data=02%7C01%7Cmatthieu.bucchianeri%40leostella.com%7C0e0c3fd9fbfb4349331c08d830de600c%7Cdbc51146ab26404b9c4bcce4f3331cc5%7C1%7C0%7C637313078942748821&sdata=3K5lhzhLRHqKcVhazaFA7L7IbNLiJgrBkli68vsRvE4%3D&reserved=0
* [new tag] patchew/20200725191436.31828-1-matthieu.bucchianeri@leostella.com -> patchew/20200725191436.31828-1-matthieu.bucchianeri@leostella.com
fatal: failed to write ref-pack file
fatal: The remote end hung up unexpectedly Traceback (most recent call last):
File "patchew-tester2/src/patchew-cli", line 531, in test_one
git_clone_repo(clone, r["repo"], r["head"], logf, True)
File "patchew-tester2/src/patchew-cli", line 62, in git_clone_repo
subprocess.check_call(clone_cmd, stderr=logf, stdout=logf)
File "/opt/rh/rh-python36/root/usr/lib64/python3.6/subprocess.py", line 291, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'clone', '-q', '/home/patchew2/.cache/patchew-git-cache/httpsgithubcompatchewprojectqemu-3c8cf5a9c21ff8782164d1def7f44bd888713384', '/var/tmp/patchew-tester-tmp-xmt7q0s9/src']' returned non-zero exit status 128.
The full log is available at
https://usg02.safelinks.protection.office365.us/?url=http%3A%2F%2Fpatchew.org%2Flogs%2F20200725191436.31828-1-matthieu.bucchianeri%40leostella.com%2Ftesting.checkpatch%2F%3Ftype%3Dmessage&data=02%7C01%7Cmatthieu.bucchianeri%40leostella.com%7C0e0c3fd9fbfb4349331c08d830de600c%7Cdbc51146ab26404b9c4bcce4f3331cc5%7C1%7C0%7C637313078942758818&sdata=YUr7tMKudnWYc71za3pXFP%2FLCpUwxNS9oXb3yVvyHZ0%3D&reserved=0.
---
Email generated automatically by Patchew [https://usg02.safelinks.protection.office365.us/?url=https%3A%2F%2Fpatchew.org%2F&data=02%7C01%7Cmatthieu.bucchianeri%40leostella.com%7C0e0c3fd9fbfb4349331c08d830de600c%7Cdbc51146ab26404b9c4bcce4f3331cc5%7C1%7C0%7C637313078942758818&sdata=wlEWpkyPvgFsy%2FTAs497SWyG0%2B%2FoBS3OuCKOvvSvNdA%3D&reserved=0].
Please send your feedback to patchew-devel@redhat.com
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/ppc: Fix SPE unavailable exception triggering
2020-07-25 19:14 [PATCH] target/ppc: Fix SPE unavailable exception triggering Matthieu Bucchianeri
2020-07-25 21:04 ` no-reply
2020-07-25 21:04 ` no-reply
@ 2020-07-26 10:04 ` BALATON Zoltan
2020-07-26 16:59 ` Matthieu Bucchianeri
2 siblings, 1 reply; 7+ messages in thread
From: BALATON Zoltan @ 2020-07-26 10:04 UTC (permalink / raw)
To: Matthieu Bucchianeri; +Cc: qemu-ppc, qemu-devel, david
[-- Attachment #1: Type: text/plain, Size: 12834 bytes --]
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.
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/ppc: Fix SPE unavailable exception triggering
2020-07-26 10:04 ` BALATON Zoltan
@ 2020-07-26 16:59 ` Matthieu Bucchianeri
2020-07-27 3:29 ` david
0 siblings, 1 reply; 7+ messages in thread
From: Matthieu Bucchianeri @ 2020-07-26 16:59 UTC (permalink / raw)
To: qemu-devel, qemu-ppc, BALATON Zoltan; +Cc: david
Hello Balaton,
Thank you for your thorough review! See my response below.
> > 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.
I should have probably mentioned somewhere that this patch also
fixes a resource leak in that function. It is not very obvious
when looking at it as a patch, but if you take a look at the
original code, you will see that I removed these allocations on
purpose:
https://github.com/qemu/qemu/blob/d577dbaac7553767232faabb6a3e291aebd348ae/target/ppc/translate/spe-impl.inc.c#L532
> static inline void gen_evmwsmiaa(DisasContext *ctx)
> {
> TCGv_i64 acc = tcg_temp_new_i64();
> TCGv_i64 tmp = tcg_temp_new_i64();
>
> gen_evmwsmi(ctx); /* rD := rA * rB */
>
> acc = tcg_temp_new_i64();
> tmp = tcg_temp_new_i64();
I apologize for not making this any more clear in my description.
Let me know if this looks correct now, with the full context in mind.
Thanks.
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/ppc: Fix SPE unavailable exception triggering
2020-07-26 16:59 ` Matthieu Bucchianeri
@ 2020-07-27 3:29 ` david
0 siblings, 0 replies; 7+ messages in thread
From: david @ 2020-07-27 3:29 UTC (permalink / raw)
To: Matthieu Bucchianeri; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2884 bytes --]
On Sun, Jul 26, 2020 at 04:59:16PM +0000, Matthieu Bucchianeri wrote:
> Hello Balaton,
>
> Thank you for your thorough review! See my response below.
>
> > > 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.
>
> I should have probably mentioned somewhere that this patch also
> fixes a resource leak in that function. It is not very obvious
> when looking at it as a patch, but if you take a look at the
> original code, you will see that I removed these allocations on
> purpose:
>
>
>https://github.com/qemu/qemu/blob/d577dbaac7553767232faabb6a3e291aebd348ae/target/ppc/translate/spe-impl.inc.c#L532
Ok, can you please split the memory leak fix into a separate patch to
make this easier to review.
>
> > static inline void gen_evmwsmiaa(DisasContext *ctx)
> > {
> > TCGv_i64 acc = tcg_temp_new_i64();
> > TCGv_i64 tmp = tcg_temp_new_i64();
> >
> > gen_evmwsmi(ctx); /* rD := rA * rB */
> >
> > acc = tcg_temp_new_i64();
> > tmp = tcg_temp_new_i64();
>
> I apologize for not making this any more clear in my description.
>
> Let me know if this looks correct now, with the full context in mind.
>
> Thanks.
>
> 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.
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-27 3:33 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-25 19:14 [PATCH] target/ppc: Fix SPE unavailable exception triggering Matthieu Bucchianeri
2020-07-25 21:04 ` no-reply
2020-07-25 21:04 ` no-reply
2020-07-26 4:07 ` Matthieu Bucchianeri
2020-07-26 10:04 ` BALATON Zoltan
2020-07-26 16:59 ` Matthieu Bucchianeri
2020-07-27 3:29 ` david
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.