* [PATCH v3] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs
@ 2023-05-10 11:19 Richard Purdie
2023-05-10 11:42 ` Matheus K. Ferst
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Richard Purdie @ 2023-05-10 11:19 UTC (permalink / raw)
To: qemu-devel
Cc: Víctor Colombo, Matheus Ferst, Daniel Henrique Barboza,
Richard Henderson, Philippe Mathieu-Daudé
The following commits changed the code such that the fallback to MFSS for MFFSCRN,
MFFSCRNI, MFFSCE and MFFSL on pre 3.0 ISAs was removed and became an illegal instruction:
bf8adfd88b547680aa857c46098f3a1e94373160 - target/ppc: Move mffscrn[i] to decodetree
394c2e2fda70da722f20fb60412d6c0ca4bfaa03 - target/ppc: Move mffsce to decodetree
3e5bce70efe6bd1f684efbb21fd2a316cbf0657e - target/ppc: Move mffsl to decodetree
The hardware will handle them as a MFFS instruction as the code did previously.
This means applications that were segfaulting under qemu when encountering these
instructions which is used in glibc libm functions for example.
The fallback for MFFSCDRN and MFFSCDRNI added in a later patch was also missing.
This patch restores the fallback to MFSS for these instructions on pre 3.0s ISAs
as the hardware decoder would, fixing the segfaulting libm code. It doesn't have
the fallback for 3.0 onwards to match hardware behaviour.
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
target/ppc/insn32.decode | 20 +++++++++++++-------
target/ppc/translate/fp-impl.c.inc | 22 ++++++++++++++++------
2 files changed, 29 insertions(+), 13 deletions(-)
v3 - drop fallback to MFFS for 3.0 ISA to match hardware
v2 - switch to use decodetree pattern groups per feedback
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index f8f589e9fd..4fcf3af8d0 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -390,13 +390,19 @@ SETNBCR 011111 ..... ..... ----- 0111100000 - @X_bi
### Move To/From FPSCR
-MFFS 111111 ..... 00000 ----- 1001000111 . @X_t_rc
-MFFSCE 111111 ..... 00001 ----- 1001000111 - @X_t
-MFFSCRN 111111 ..... 10110 ..... 1001000111 - @X_tb
-MFFSCDRN 111111 ..... 10100 ..... 1001000111 - @X_tb
-MFFSCRNI 111111 ..... 10111 ---.. 1001000111 - @X_imm2
-MFFSCDRNI 111111 ..... 10101 --... 1001000111 - @X_imm3
-MFFSL 111111 ..... 11000 ----- 1001000111 - @X_t
+{
+ # Before Power ISA v3.0, MFFS bits 11~15 were reserved and should be ignored
+ MFFS_ISA207 111111 ..... ----- ----- 1001000111 . @X_t_rc
+ [
+ MFFS 111111 ..... 00000 ----- 1001000111 . @X_t_rc
+ MFFSCE 111111 ..... 00001 ----- 1001000111 - @X_t
+ MFFSCRN 111111 ..... 10110 ..... 1001000111 - @X_tb
+ MFFSCDRN 111111 ..... 10100 ..... 1001000111 - @X_tb
+ MFFSCRNI 111111 ..... 10111 ---.. 1001000111 - @X_imm2
+ MFFSCDRNI 111111 ..... 10101 --... 1001000111 - @X_imm3
+ MFFSL 111111 ..... 11000 ----- 1001000111 - @X_t
+ ]
+}
### Decimal Floating-Point Arithmetic Instructions
diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc
index 57d8437851..874774eade 100644
--- a/target/ppc/translate/fp-impl.c.inc
+++ b/target/ppc/translate/fp-impl.c.inc
@@ -568,6 +568,22 @@ static void store_fpscr_masked(TCGv_i64 fpscr, uint64_t clear_mask,
gen_helper_store_fpscr(cpu_env, fpscr_masked, st_mask);
}
+static bool trans_MFFS_ISA207(DisasContext *ctx, arg_X_t_rc *a)
+{
+ if (!(ctx->insns_flags2 & PPC2_ISA300)) {
+ /*
+ * Before Power ISA v3.0, MFFS bits 11~15 were reserved, any instruction
+ * with OPCD=63 and XO=583 should be decoded as MFFS.
+ */
+ return trans_MFFS(ctx, a);
+ }
+ /*
+ * For Power ISA v3.0+, return false and let the pattern group
+ * select the correct instruction.
+ */
+ return false;
+}
+
static bool trans_MFFS(DisasContext *ctx, arg_X_t_rc *a)
{
REQUIRE_FPU(ctx);
@@ -584,7 +600,6 @@ static bool trans_MFFSCE(DisasContext *ctx, arg_X_t *a)
{
TCGv_i64 fpscr;
- REQUIRE_INSNS_FLAGS2(ctx, ISA300);
REQUIRE_FPU(ctx);
gen_reset_fpstatus();
@@ -597,7 +612,6 @@ static bool trans_MFFSCRN(DisasContext *ctx, arg_X_tb *a)
{
TCGv_i64 t1, fpscr;
- REQUIRE_INSNS_FLAGS2(ctx, ISA300);
REQUIRE_FPU(ctx);
t1 = tcg_temp_new_i64();
@@ -614,7 +628,6 @@ static bool trans_MFFSCDRN(DisasContext *ctx, arg_X_tb *a)
{
TCGv_i64 t1, fpscr;
- REQUIRE_INSNS_FLAGS2(ctx, ISA300);
REQUIRE_FPU(ctx);
t1 = tcg_temp_new_i64();
@@ -631,7 +644,6 @@ static bool trans_MFFSCRNI(DisasContext *ctx, arg_X_imm2 *a)
{
TCGv_i64 t1, fpscr;
- REQUIRE_INSNS_FLAGS2(ctx, ISA300);
REQUIRE_FPU(ctx);
t1 = tcg_temp_new_i64();
@@ -647,7 +659,6 @@ static bool trans_MFFSCDRNI(DisasContext *ctx, arg_X_imm3 *a)
{
TCGv_i64 t1, fpscr;
- REQUIRE_INSNS_FLAGS2(ctx, ISA300);
REQUIRE_FPU(ctx);
t1 = tcg_temp_new_i64();
@@ -661,7 +672,6 @@ static bool trans_MFFSCDRNI(DisasContext *ctx, arg_X_imm3 *a)
static bool trans_MFFSL(DisasContext *ctx, arg_X_t *a)
{
- REQUIRE_INSNS_FLAGS2(ctx, ISA300);
REQUIRE_FPU(ctx);
gen_reset_fpstatus();
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs
2023-05-10 11:19 [PATCH v3] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs Richard Purdie
@ 2023-05-10 11:42 ` Matheus K. Ferst
2023-05-10 11:58 ` Richard Henderson
2023-05-15 10:11 ` Daniel Henrique Barboza
2 siblings, 0 replies; 4+ messages in thread
From: Matheus K. Ferst @ 2023-05-10 11:42 UTC (permalink / raw)
To: Richard Purdie, qemu-devel
Cc: Víctor Colombo, Daniel Henrique Barboza, Richard Henderson,
Philippe Mathieu-Daudé
On 10/05/2023 08:19, Richard Purdie wrote:
> The following commits changed the code such that the fallback to MFSS for MFFSCRN,
> MFFSCRNI, MFFSCE and MFFSL on pre 3.0 ISAs was removed and became an illegal instruction:
>
> bf8adfd88b547680aa857c46098f3a1e94373160 - target/ppc: Move mffscrn[i] to decodetree
> 394c2e2fda70da722f20fb60412d6c0ca4bfaa03 - target/ppc: Move mffsce to decodetree
> 3e5bce70efe6bd1f684efbb21fd2a316cbf0657e - target/ppc: Move mffsl to decodetree
>
> The hardware will handle them as a MFFS instruction as the code did previously.
> This means applications that were segfaulting under qemu when encountering these
> instructions which is used in glibc libm functions for example.
>
> The fallback for MFFSCDRN and MFFSCDRNI added in a later patch was also missing.
>
> This patch restores the fallback to MFSS for these instructions on pre 3.0s ISAs
> as the hardware decoder would, fixing the segfaulting libm code. It doesn't have
> the fallback for 3.0 onwards to match hardware behaviour.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
> target/ppc/insn32.decode | 20 +++++++++++++-------
> target/ppc/translate/fp-impl.c.inc | 22 ++++++++++++++++------
> 2 files changed, 29 insertions(+), 13 deletions(-)
>
> v3 - drop fallback to MFFS for 3.0 ISA to match hardware
> v2 - switch to use decodetree pattern groups per feedback
>
Reviewed-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs
2023-05-10 11:19 [PATCH v3] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs Richard Purdie
2023-05-10 11:42 ` Matheus K. Ferst
@ 2023-05-10 11:58 ` Richard Henderson
2023-05-15 10:11 ` Daniel Henrique Barboza
2 siblings, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2023-05-10 11:58 UTC (permalink / raw)
To: Richard Purdie, qemu-devel
Cc: Víctor Colombo, Matheus Ferst, Daniel Henrique Barboza,
Philippe Mathieu-Daudé
On 5/10/23 12:19, Richard Purdie wrote:
> The following commits changed the code such that the fallback to MFSS for MFFSCRN,
> MFFSCRNI, MFFSCE and MFFSL on pre 3.0 ISAs was removed and became an illegal instruction:
>
> bf8adfd88b547680aa857c46098f3a1e94373160 - target/ppc: Move mffscrn[i] to decodetree
> 394c2e2fda70da722f20fb60412d6c0ca4bfaa03 - target/ppc: Move mffsce to decodetree
> 3e5bce70efe6bd1f684efbb21fd2a316cbf0657e - target/ppc: Move mffsl to decodetree
>
> The hardware will handle them as a MFFS instruction as the code did previously.
> This means applications that were segfaulting under qemu when encountering these
> instructions which is used in glibc libm functions for example.
>
> The fallback for MFFSCDRN and MFFSCDRNI added in a later patch was also missing.
>
> This patch restores the fallback to MFSS for these instructions on pre 3.0s ISAs
> as the hardware decoder would, fixing the segfaulting libm code. It doesn't have
> the fallback for 3.0 onwards to match hardware behaviour.
>
> Signed-off-by: Richard Purdie<richard.purdie@linuxfoundation.org>
> ---
> target/ppc/insn32.decode | 20 +++++++++++++-------
> target/ppc/translate/fp-impl.c.inc | 22 ++++++++++++++++------
> 2 files changed, 29 insertions(+), 13 deletions(-)
>
> v3 - drop fallback to MFFS for 3.0 ISA to match hardware
> v2 - switch to use decodetree pattern groups per feedback
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs
2023-05-10 11:19 [PATCH v3] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs Richard Purdie
2023-05-10 11:42 ` Matheus K. Ferst
2023-05-10 11:58 ` Richard Henderson
@ 2023-05-15 10:11 ` Daniel Henrique Barboza
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Henrique Barboza @ 2023-05-15 10:11 UTC (permalink / raw)
To: Richard Purdie, qemu-devel
Cc: Víctor Colombo, Matheus Ferst, Richard Henderson,
Philippe Mathieu-Daudé
Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,
Daniel
On 5/10/23 08:19, Richard Purdie wrote:
> The following commits changed the code such that the fallback to MFSS for MFFSCRN,
> MFFSCRNI, MFFSCE and MFFSL on pre 3.0 ISAs was removed and became an illegal instruction:
>
> bf8adfd88b547680aa857c46098f3a1e94373160 - target/ppc: Move mffscrn[i] to decodetree
> 394c2e2fda70da722f20fb60412d6c0ca4bfaa03 - target/ppc: Move mffsce to decodetree
> 3e5bce70efe6bd1f684efbb21fd2a316cbf0657e - target/ppc: Move mffsl to decodetree
>
> The hardware will handle them as a MFFS instruction as the code did previously.
> This means applications that were segfaulting under qemu when encountering these
> instructions which is used in glibc libm functions for example.
>
> The fallback for MFFSCDRN and MFFSCDRNI added in a later patch was also missing.
>
> This patch restores the fallback to MFSS for these instructions on pre 3.0s ISAs
> as the hardware decoder would, fixing the segfaulting libm code. It doesn't have
> the fallback for 3.0 onwards to match hardware behaviour.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
> target/ppc/insn32.decode | 20 +++++++++++++-------
> target/ppc/translate/fp-impl.c.inc | 22 ++++++++++++++++------
> 2 files changed, 29 insertions(+), 13 deletions(-)
>
> v3 - drop fallback to MFFS for 3.0 ISA to match hardware
> v2 - switch to use decodetree pattern groups per feedback
>
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index f8f589e9fd..4fcf3af8d0 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -390,13 +390,19 @@ SETNBCR 011111 ..... ..... ----- 0111100000 - @X_bi
>
> ### Move To/From FPSCR
>
> -MFFS 111111 ..... 00000 ----- 1001000111 . @X_t_rc
> -MFFSCE 111111 ..... 00001 ----- 1001000111 - @X_t
> -MFFSCRN 111111 ..... 10110 ..... 1001000111 - @X_tb
> -MFFSCDRN 111111 ..... 10100 ..... 1001000111 - @X_tb
> -MFFSCRNI 111111 ..... 10111 ---.. 1001000111 - @X_imm2
> -MFFSCDRNI 111111 ..... 10101 --... 1001000111 - @X_imm3
> -MFFSL 111111 ..... 11000 ----- 1001000111 - @X_t
> +{
> + # Before Power ISA v3.0, MFFS bits 11~15 were reserved and should be ignored
> + MFFS_ISA207 111111 ..... ----- ----- 1001000111 . @X_t_rc
> + [
> + MFFS 111111 ..... 00000 ----- 1001000111 . @X_t_rc
> + MFFSCE 111111 ..... 00001 ----- 1001000111 - @X_t
> + MFFSCRN 111111 ..... 10110 ..... 1001000111 - @X_tb
> + MFFSCDRN 111111 ..... 10100 ..... 1001000111 - @X_tb
> + MFFSCRNI 111111 ..... 10111 ---.. 1001000111 - @X_imm2
> + MFFSCDRNI 111111 ..... 10101 --... 1001000111 - @X_imm3
> + MFFSL 111111 ..... 11000 ----- 1001000111 - @X_t
> + ]
> +}
>
> ### Decimal Floating-Point Arithmetic Instructions
>
> diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc
> index 57d8437851..874774eade 100644
> --- a/target/ppc/translate/fp-impl.c.inc
> +++ b/target/ppc/translate/fp-impl.c.inc
> @@ -568,6 +568,22 @@ static void store_fpscr_masked(TCGv_i64 fpscr, uint64_t clear_mask,
> gen_helper_store_fpscr(cpu_env, fpscr_masked, st_mask);
> }
>
> +static bool trans_MFFS_ISA207(DisasContext *ctx, arg_X_t_rc *a)
> +{
> + if (!(ctx->insns_flags2 & PPC2_ISA300)) {
> + /*
> + * Before Power ISA v3.0, MFFS bits 11~15 were reserved, any instruction
> + * with OPCD=63 and XO=583 should be decoded as MFFS.
> + */
> + return trans_MFFS(ctx, a);
> + }
> + /*
> + * For Power ISA v3.0+, return false and let the pattern group
> + * select the correct instruction.
> + */
> + return false;
> +}
> +
> static bool trans_MFFS(DisasContext *ctx, arg_X_t_rc *a)
> {
> REQUIRE_FPU(ctx);
> @@ -584,7 +600,6 @@ static bool trans_MFFSCE(DisasContext *ctx, arg_X_t *a)
> {
> TCGv_i64 fpscr;
>
> - REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> REQUIRE_FPU(ctx);
>
> gen_reset_fpstatus();
> @@ -597,7 +612,6 @@ static bool trans_MFFSCRN(DisasContext *ctx, arg_X_tb *a)
> {
> TCGv_i64 t1, fpscr;
>
> - REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> REQUIRE_FPU(ctx);
>
> t1 = tcg_temp_new_i64();
> @@ -614,7 +628,6 @@ static bool trans_MFFSCDRN(DisasContext *ctx, arg_X_tb *a)
> {
> TCGv_i64 t1, fpscr;
>
> - REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> REQUIRE_FPU(ctx);
>
> t1 = tcg_temp_new_i64();
> @@ -631,7 +644,6 @@ static bool trans_MFFSCRNI(DisasContext *ctx, arg_X_imm2 *a)
> {
> TCGv_i64 t1, fpscr;
>
> - REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> REQUIRE_FPU(ctx);
>
> t1 = tcg_temp_new_i64();
> @@ -647,7 +659,6 @@ static bool trans_MFFSCDRNI(DisasContext *ctx, arg_X_imm3 *a)
> {
> TCGv_i64 t1, fpscr;
>
> - REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> REQUIRE_FPU(ctx);
>
> t1 = tcg_temp_new_i64();
> @@ -661,7 +672,6 @@ static bool trans_MFFSCDRNI(DisasContext *ctx, arg_X_imm3 *a)
>
> static bool trans_MFFSL(DisasContext *ctx, arg_X_t *a)
> {
> - REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> REQUIRE_FPU(ctx);
>
> gen_reset_fpstatus();
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-05-15 10:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-10 11:19 [PATCH v3] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs Richard Purdie
2023-05-10 11:42 ` Matheus K. Ferst
2023-05-10 11:58 ` Richard Henderson
2023-05-15 10:11 ` Daniel Henrique Barboza
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).