On 5/4/23 18:17, Matheus K. Ferst wrote:
On 04/05/2023 08:01, Richard Purdie wrote:
The following commits changed the code such that these instructions became 
invalid
on pre 3.0 ISAs:

   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.
Restore that behaviour. This means applications that were segfaulting under qemu
when encountering these instructions now operate correctly. The instruction
is used in glibc libm functions for example.

Signed-off-by: Richard Purdie <richard.pur...@linuxfoundation.org>
---
  target/ppc/translate/fp-impl.c.inc | 20 ++++++++++++++++----
  1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/target/ppc/translate/fp-impl.c.inc 
b/target/ppc/translate/fp-impl.c.inc
index 57d8437851..cb86381c3f 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 (unlikely(!(ctx->insns_flags2 & PPC2_ISA300))) {
+        return trans_MFFS(ctx, a);
+    }
+

Hi Richard, nice catch!

I believe this may be better addressed by decodetree pattern groups, e.g.:

On insns32.decode:
{
   # 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
   ]
}

And on fp-impl.c.inc:
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;
}

Not quite.  Should be

{
  [
    MFFSCE  111111 ..... 00001 ----- 1001000111 -  @X_t
    ...
    MFFSL   111111 ..... 11000 ----- 1001000111 -  @X_t
  ]
  MFFS      111111 ..... ----- ----- 1001000111 .  @X_t_rc
}

where all of the 3.0 insns do

    if (!(ctx->insns_flags2 & PPC2_ISA300)) {
        return false;
    }

I do not believe that v3.0 rejects bits [11:15] = 00010, for example, which would have been accepted and ignored with v2.07. It should simply treat it as the full MFFS insn.


r~

Reply via email to