All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs
@ 2023-05-06  6:52 Richard Purdie
  2023-05-06  7:27 ` Richard Henderson
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Richard Purdie @ 2023-05-06  6:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Víctor Colombo, Matheus Ferst, Daniel Henrique Barboza,
	Richard Henderson

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 and also ensures
the MFSS instruction is used for currently reserved bits to handle other potential
ISA additions more correctly.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 target/ppc/insn32.decode           | 19 ++++++++++++-------
 target/ppc/translate/fp-impl.c.inc | 30 ++++++++++++++++++++++++------
 2 files changed, 36 insertions(+), 13 deletions(-)

v2 - switch to use decodetree pattern groups per feedback

diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index f8f589e9fd..3c4e2c2fc2 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -390,13 +390,18 @@ 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
+  [
+    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
+  ]
+  MFFS            111111 ..... ----- ----- 1001000111 .   @X_t_rc
+}
 
 ### 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..10dfd91aa4 100644
--- a/target/ppc/translate/fp-impl.c.inc
+++ b/target/ppc/translate/fp-impl.c.inc
@@ -584,7 +584,10 @@ static bool trans_MFFSCE(DisasContext *ctx, arg_X_t *a)
 {
     TCGv_i64 fpscr;
 
-    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
+    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
+        return false;
+    }
+
     REQUIRE_FPU(ctx);
 
     gen_reset_fpstatus();
@@ -597,7 +600,10 @@ static bool trans_MFFSCRN(DisasContext *ctx, arg_X_tb *a)
 {
     TCGv_i64 t1, fpscr;
 
-    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
+    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
+        return false;
+    }
+
     REQUIRE_FPU(ctx);
 
     t1 = tcg_temp_new_i64();
@@ -614,7 +620,10 @@ static bool trans_MFFSCDRN(DisasContext *ctx, arg_X_tb *a)
 {
     TCGv_i64 t1, fpscr;
 
-    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
+    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
+        return false;
+    }
+
     REQUIRE_FPU(ctx);
 
     t1 = tcg_temp_new_i64();
@@ -631,7 +640,10 @@ static bool trans_MFFSCRNI(DisasContext *ctx, arg_X_imm2 *a)
 {
     TCGv_i64 t1, fpscr;
 
-    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
+    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
+        return false;
+    }
+
     REQUIRE_FPU(ctx);
 
     t1 = tcg_temp_new_i64();
@@ -647,7 +659,10 @@ static bool trans_MFFSCDRNI(DisasContext *ctx, arg_X_imm3 *a)
 {
     TCGv_i64 t1, fpscr;
 
-    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
+    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
+        return false;
+    }
+
     REQUIRE_FPU(ctx);
 
     t1 = tcg_temp_new_i64();
@@ -661,7 +676,10 @@ 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);
+    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
+        return false;
+    }
+
     REQUIRE_FPU(ctx);
 
     gen_reset_fpstatus();
-- 
2.39.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs
  2023-05-06  6:52 [PATCH v2] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs Richard Purdie
@ 2023-05-06  7:27 ` Richard Henderson
  2023-05-08 12:11 ` Matheus K. Ferst
  2023-05-10  7:28 ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2023-05-06  7:27 UTC (permalink / raw)
  To: Richard Purdie, qemu-devel
  Cc: Víctor Colombo, Matheus Ferst, Daniel Henrique Barboza

On 5/6/23 07:52, 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 and also ensures
> the MFSS instruction is used for currently reserved bits to handle other potential
> ISA additions more correctly.
> 
> Signed-off-by: Richard Purdie<richard.purdie@linuxfoundation.org>
> ---
>   target/ppc/insn32.decode           | 19 ++++++++++++-------
>   target/ppc/translate/fp-impl.c.inc | 30 ++++++++++++++++++++++++------
>   2 files changed, 36 insertions(+), 13 deletions(-)
> 
> v2 - switch to use decodetree pattern groups per feedback

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs
  2023-05-06  6:52 [PATCH v2] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs Richard Purdie
  2023-05-06  7:27 ` Richard Henderson
@ 2023-05-08 12:11 ` Matheus K. Ferst
  2023-05-10  8:05   ` Richard Henderson
  2023-05-10  7:28 ` Philippe Mathieu-Daudé
  2 siblings, 1 reply; 5+ messages in thread
From: Matheus K. Ferst @ 2023-05-08 12:11 UTC (permalink / raw)
  To: Richard Purdie, qemu-devel
  Cc: Víctor Colombo, Daniel Henrique Barboza, Richard Henderson

On 06/05/2023 03:52, 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 and also ensures
> the MFSS instruction is used for currently reserved bits to handle other potential
> ISA additions more correctly.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>   target/ppc/insn32.decode           | 19 ++++++++++++-------
>   target/ppc/translate/fp-impl.c.inc | 30 ++++++++++++++++++++++++------
>   2 files changed, 36 insertions(+), 13 deletions(-)
> 
> v2 - switch to use decodetree pattern groups per feedback
> 
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index f8f589e9fd..3c4e2c2fc2 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -390,13 +390,18 @@ 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
> +  [
> +    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
> +  ]
> +  MFFS            111111 ..... ----- ----- 1001000111 .   @X_t_rc
> +}
> 
>   ### 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..10dfd91aa4 100644
> --- a/target/ppc/translate/fp-impl.c.inc
> +++ b/target/ppc/translate/fp-impl.c.inc
> @@ -584,7 +584,10 @@ static bool trans_MFFSCE(DisasContext *ctx, arg_X_t *a)
>   {
>       TCGv_i64 fpscr;
> 
> -    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> +    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
> +        return false;
> +    }
> +
>       REQUIRE_FPU(ctx);
> 
>       gen_reset_fpstatus();
> @@ -597,7 +600,10 @@ static bool trans_MFFSCRN(DisasContext *ctx, arg_X_tb *a)
>   {
>       TCGv_i64 t1, fpscr;
> 
> -    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> +    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
> +        return false;
> +    }
> +
>       REQUIRE_FPU(ctx);
> 
>       t1 = tcg_temp_new_i64();
> @@ -614,7 +620,10 @@ static bool trans_MFFSCDRN(DisasContext *ctx, arg_X_tb *a)
>   {
>       TCGv_i64 t1, fpscr;
> 
> -    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> +    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
> +        return false;
> +    }
> +
>       REQUIRE_FPU(ctx);
> 
>       t1 = tcg_temp_new_i64();
> @@ -631,7 +640,10 @@ static bool trans_MFFSCRNI(DisasContext *ctx, arg_X_imm2 *a)
>   {
>       TCGv_i64 t1, fpscr;
> 
> -    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> +    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
> +        return false;
> +    }
> +
>       REQUIRE_FPU(ctx);
> 
>       t1 = tcg_temp_new_i64();
> @@ -647,7 +659,10 @@ static bool trans_MFFSCDRNI(DisasContext *ctx, arg_X_imm3 *a)
>   {
>       TCGv_i64 t1, fpscr;
> 
> -    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> +    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
> +        return false;
> +    }
> +
>       REQUIRE_FPU(ctx);
> 
>       t1 = tcg_temp_new_i64();
> @@ -661,7 +676,10 @@ 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);
> +    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
> +        return false;
> +    }
> +
>       REQUIRE_FPU(ctx);
> 
>       gen_reset_fpstatus();
> --
> 2.39.2
> 

As mention in the v1 thread, we should validate bits 11~15 on Power ISA 
v3.0+ to follow what the ISA says and keep the same behavior as the 
hardware. Again, sorry for the delayed response.

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] 5+ messages in thread

* Re: [PATCH v2] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs
  2023-05-06  6:52 [PATCH v2] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs Richard Purdie
  2023-05-06  7:27 ` Richard Henderson
  2023-05-08 12:11 ` Matheus K. Ferst
@ 2023-05-10  7:28 ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-05-10  7:28 UTC (permalink / raw)
  To: Richard Purdie, qemu-devel
  Cc: Víctor Colombo, Matheus Ferst, Daniel Henrique Barboza,
	Richard Henderson

On 6/5/23 08:52, 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 and also ensures
> the MFSS instruction is used for currently reserved bits to handle other potential
> ISA additions more correctly.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>   target/ppc/insn32.decode           | 19 ++++++++++++-------
>   target/ppc/translate/fp-impl.c.inc | 30 ++++++++++++++++++++++++------
>   2 files changed, 36 insertions(+), 13 deletions(-)
> 
> v2 - switch to use decodetree pattern groups per feedback
> 
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index f8f589e9fd..3c4e2c2fc2 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -390,13 +390,18 @@ 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
> +  [
> +    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
> +  ]
> +  MFFS            111111 ..... ----- ----- 1001000111 .   @X_t_rc
> +}

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Fixes: bf8adfd88b ("target/ppc: Move mffscrn[i] to decodetree")
Fixes: 394c2e2fda ("target/ppc: Move mffsce to decodetree")
Fixes: 3e5bce70ef ("target/ppc: Move mffsl to decodetree")



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs
  2023-05-08 12:11 ` Matheus K. Ferst
@ 2023-05-10  8:05   ` Richard Henderson
  0 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2023-05-10  8:05 UTC (permalink / raw)
  To: Matheus K. Ferst, Richard Purdie, qemu-devel
  Cc: Víctor Colombo, Daniel Henrique Barboza

On 5/8/23 13:11, Matheus K. Ferst wrote:
> On 06/05/2023 03:52, 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 and also ensures
>> the MFSS instruction is used for currently reserved bits to handle other potential
>> ISA additions more correctly.
>>
>> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>> ---
>>   target/ppc/insn32.decode           | 19 ++++++++++++-------
>>   target/ppc/translate/fp-impl.c.inc | 30 ++++++++++++++++++++++++------
>>   2 files changed, 36 insertions(+), 13 deletions(-)
>>
>> v2 - switch to use decodetree pattern groups per feedback
>>
>> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
>> index f8f589e9fd..3c4e2c2fc2 100644
>> --- a/target/ppc/insn32.decode
>> +++ b/target/ppc/insn32.decode
>> @@ -390,13 +390,18 @@ 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
>> +  [
>> +    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
>> +  ]
>> +  MFFS            111111 ..... ----- ----- 1001000111 .   @X_t_rc
>> +}
>>
>>   ### 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..10dfd91aa4 100644
>> --- a/target/ppc/translate/fp-impl.c.inc
>> +++ b/target/ppc/translate/fp-impl.c.inc
>> @@ -584,7 +584,10 @@ static bool trans_MFFSCE(DisasContext *ctx, arg_X_t *a)
>>   {
>>       TCGv_i64 fpscr;
>>
>> -    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
>> +    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
>> +        return false;
>> +    }
>> +
>>       REQUIRE_FPU(ctx);
>>
>>       gen_reset_fpstatus();
>> @@ -597,7 +600,10 @@ static bool trans_MFFSCRN(DisasContext *ctx, arg_X_tb *a)
>>   {
>>       TCGv_i64 t1, fpscr;
>>
>> -    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
>> +    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
>> +        return false;
>> +    }
>> +
>>       REQUIRE_FPU(ctx);
>>
>>       t1 = tcg_temp_new_i64();
>> @@ -614,7 +620,10 @@ static bool trans_MFFSCDRN(DisasContext *ctx, arg_X_tb *a)
>>   {
>>       TCGv_i64 t1, fpscr;
>>
>> -    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
>> +    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
>> +        return false;
>> +    }
>> +
>>       REQUIRE_FPU(ctx);
>>
>>       t1 = tcg_temp_new_i64();
>> @@ -631,7 +640,10 @@ static bool trans_MFFSCRNI(DisasContext *ctx, arg_X_imm2 *a)
>>   {
>>       TCGv_i64 t1, fpscr;
>>
>> -    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
>> +    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
>> +        return false;
>> +    }
>> +
>>       REQUIRE_FPU(ctx);
>>
>>       t1 = tcg_temp_new_i64();
>> @@ -647,7 +659,10 @@ static bool trans_MFFSCDRNI(DisasContext *ctx, arg_X_imm3 *a)
>>   {
>>       TCGv_i64 t1, fpscr;
>>
>> -    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
>> +    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
>> +        return false;
>> +    }
>> +
>>       REQUIRE_FPU(ctx);
>>
>>       t1 = tcg_temp_new_i64();
>> @@ -661,7 +676,10 @@ 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);
>> +    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
>> +        return false;
>> +    }
>> +
>>       REQUIRE_FPU(ctx);
>>
>>       gen_reset_fpstatus();
>> -- 
>> 2.39.2
>>
> 
> As mention in the v1 thread, we should validate bits 11~15 on Power ISA v3.0+ to follow 
> what the ISA says and keep the same behavior as the hardware. Again, sorry for the delayed 
> response.

Thanks for double-checking.  In that case the v1 patch is indeed better.


r~


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-05-10  8:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-06  6:52 [PATCH v2] target/ppc: Fix fallback to MFSS for MFFS* instructions on pre 3.0 ISAs Richard Purdie
2023-05-06  7:27 ` Richard Henderson
2023-05-08 12:11 ` Matheus K. Ferst
2023-05-10  8:05   ` Richard Henderson
2023-05-10  7:28 ` Philippe Mathieu-Daudé

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.