[Qemu-devel] [PATCH] target-mips: add ERETNC instruction and Config5.LLB bit

[Qemu-devel] [PATCH] target-mips: add ERETNC instruction and Config5.LLB bit

[Leon Alrae]

ERETNC is identical to ERET except that an ERETNC will not clear the LLbit
that is set by execution of an LL instruction, and thus when placed between
an LL and SC sequence, will never cause the SC to fail.

Presence of ERETNC is denoted by the Config5.LLB.

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 disas/mips.c                 |  1 +
 target-mips/cpu.h            |  1 +
 target-mips/helper.h         |  1 +
 target-mips/op_helper.c      | 12 +++++++++++-
 target-mips/translate.c      | 20 +++++++++++++++-----
 target-mips/translate_init.c |  4 +++-
 6 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/disas/mips.c b/disas/mips.c
index 8aaa87c..32940fe 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -2409,6 +2409,7 @@ const struct mips_opcode mips_builtin_opcodes[] =
 {"emt",     "",		0x41600be1, 0xffffffff, TRAP,			0,		MT32	},
 {"emt",     "t",	0x41600be1, 0xffe0ffff, TRAP|WR_t,		0,		MT32	},
 {"eret",    "",         0x42000018, 0xffffffff, 0,      		0,		I3|I32	},
+{"eretnc",  "",         0x42000058, 0xffffffff, 0,                    0, I33},
 {"evpe",    "",		0x41600021, 0xffffffff, TRAP,			0,		MT32	},
 {"evpe",    "t",	0x41600021, 0xffe0ffff, TRAP|WR_t,		0,		MT32	},
 {"ext",     "t,r,+A,+C", 0x7c000000, 0xfc00003f, WR_t|RD_s,    		0,		I33	},
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 7b5ef36..474a0e3 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -474,6 +474,7 @@ struct CPUMIPSState {
 #define CP0C5_FRE        8
 #define CP0C5_SBRI       6
 #define CP0C5_MVH        5
+#define CP0C5_LLB        4
 #define CP0C5_UFR        2
 #define CP0C5_NFExists   0
     int32_t CP0_Config6;
diff --git a/target-mips/helper.h b/target-mips/helper.h
index bdd5ba5..8df98c7 100644
--- a/target-mips/helper.h
+++ b/target-mips/helper.h
@@ -348,6 +348,7 @@ DEF_HELPER_1(tlbinvf, void, env)
 DEF_HELPER_1(di, tl, env)
 DEF_HELPER_1(ei, tl, env)
 DEF_HELPER_1(eret, void, env)
+DEF_HELPER_1(eretnc, void, env)
 DEF_HELPER_1(deret, void, env)
 #endif /* !CONFIG_USER_ONLY */
 DEF_HELPER_1(rdhwr_cpunum, tl, env)
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 5ca95c5..90394b0 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -2123,7 +2123,7 @@ static void set_pc(CPUMIPSState *env, target_ulong error_pc)
     }
 }
 
-void helper_eret(CPUMIPSState *env)
+static inline void exception_return(CPUMIPSState *env)
 {
     debug_pre_eret(env);
     if (env->CP0_Status & (1 << CP0St_ERL)) {
@@ -2135,9 +2135,19 @@ void helper_eret(CPUMIPSState *env)
     }
     compute_hflags(env);
     debug_post_eret(env);
+}
+
+void helper_eret(CPUMIPSState *env)
+{
+    exception_return(env);
     env->lladdr = 1;
 }
 
+void helper_eretnc(CPUMIPSState *env)
+{
+    exception_return(env);
+}
+
 void helper_deret(CPUMIPSState *env)
 {
     debug_pre_eret(env);
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 0ca610c..1bdc2e1 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -8142,16 +8142,26 @@ static void gen_cp0 (CPUMIPSState *env, DisasContext *ctx, uint32_t opc, int rt,
             goto die;
         gen_helper_tlbr(cpu_env);
         break;
-    case OPC_ERET:
-        opn = "eret";
-        check_insn(ctx, ISA_MIPS2);
+    case OPC_ERET: /* OPC_ERETNC */
         if ((ctx->insn_flags & ISA_MIPS32R6) &&
             (ctx->hflags & MIPS_HFLAG_BMASK)) {
             MIPS_DEBUG("CTI in delay / forbidden slot");
             goto die;
+        } else {
+            int bit_shift = (ctx->hflags & MIPS_HFLAG_M16) ? 16 : 6;
+            if (ctx->opcode & (1 << bit_shift)) {
+                /* OPC_ERETNC */
+                opn = "eretnc";
+                check_insn(ctx, ISA_MIPS32R5);
+                gen_helper_eretnc(cpu_env);
+            } else {
+                /* OPC_ERET */
+                opn = "eret";
+                check_insn(ctx, ISA_MIPS2);
+                gen_helper_eret(cpu_env);
+            }
+            ctx->bstate = BS_EXCP;
         }
-        gen_helper_eret(cpu_env);
-        ctx->bstate = BS_EXCP;
         break;
     case OPC_DERET:
         opn = "deret";
diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
index 836b7bf..fcf957b 100644
--- a/target-mips/translate_init.c
+++ b/target-mips/translate_init.c
@@ -404,7 +404,8 @@ static const mips_def_t mips_defs[] =
                        (1 << CP0C3_LPA),
         .CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M),
         .CP0_Config4_rw_bitmask = 0,
-        .CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_UFR) | (1 << CP0C5_MVH),
+        .CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_UFR) | (1 << CP0C5_MVH) |
+                       (1 << CP0C5_LLB),
         .CP0_Config5_rw_bitmask = (0 << CP0C5_M) | (1 << CP0C5_K) |
                                   (1 << CP0C5_CV) | (0 << CP0C5_EVA) |
                                   (1 << CP0C5_MSAEn) | (1 << CP0C5_UFR) |
@@ -622,6 +623,7 @@ static const mips_def_t mips_defs[] =
                        (1U << CP0C3_M),
         .CP0_Config4 = MIPS_CONFIG4 | (0xfc << CP0C4_KScrExist) |
                        (3 << CP0C4_IE) | (1 << CP0C4_M),
+        .CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_LLB),
         .CP0_Config5_rw_bitmask = (1 << CP0C5_SBRI) | (1 << CP0C5_FRE) |
                                   (1 << CP0C5_UFE),
         .CP0_LLAddr_rw_bitmask = 0,

Re: [Qemu-devel] [PATCH] target-mips: add ERETNC instruction and Config5.LLB bit

[Aurelien Jarno]

On 2015-06-04 17:00, Leon Alrae wrote:
> ERETNC is identical to ERET except that an ERETNC will not clear the LLbit
> that is set by execution of an LL instruction, and thus when placed between
> an LL and SC sequence, will never cause the SC to fail.
> 
> Presence of ERETNC is denoted by the Config5.LLB.
> 
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> ---
>  disas/mips.c                 |  1 +
>  target-mips/cpu.h            |  1 +
>  target-mips/helper.h         |  1 +
>  target-mips/op_helper.c      | 12 +++++++++++-
>  target-mips/translate.c      | 20 +++++++++++++++-----
>  target-mips/translate_init.c |  4 +++-
>  6 files changed, 32 insertions(+), 7 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

As a side note, I have seen that you have added a check for MIPS2 to the
ERET instruction. This is correct, but given in practice we don't
emulate any MIPS1 CPU, I do wonder if it's not the time to make MIPS2
the basic instruction set and remove all MIPS2 checks.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH] target-mips: add ERETNC instruction and Config5.LLB bit

[Leon Alrae]

On 05/06/15 10:42, Aurelien Jarno wrote:
> On 2015-06-04 17:00, Leon Alrae wrote:
>> ERETNC is identical to ERET except that an ERETNC will not clear the LLbit
>> that is set by execution of an LL instruction, and thus when placed between
>> an LL and SC sequence, will never cause the SC to fail.
>>
>> Presence of ERETNC is denoted by the Config5.LLB.
>>
>> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
>> ---
>>  disas/mips.c                 |  1 +
>>  target-mips/cpu.h            |  1 +
>>  target-mips/helper.h         |  1 +
>>  target-mips/op_helper.c      | 12 +++++++++++-
>>  target-mips/translate.c      | 20 +++++++++++++++-----
>>  target-mips/translate_init.c |  4 +++-
>>  6 files changed, 32 insertions(+), 7 deletions(-)
> 
> Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

Thanks for your review!

> As a side note, I have seen that you have added a check for MIPS2 to the
> ERET instruction. This is correct, but given in practice we don't
> emulate any MIPS1 CPU, I do wonder if it's not the time to make MIPS2
> the basic instruction set and remove all MIPS2 checks.

Yes, in current codebase the MIPS2 checks don't seem to have much value
and the removal makes sense. On the other hand I'm wondering if there
are QEMU users who maintain artificial MIPS1 CPU templates locally to
test if their compiler doesn't emit any non-MIPS1 instructions.

Leon

Re: [Qemu-devel] [PATCH] target-mips: add ERETNC instruction and Config5.LLB bit

[Maciej W. Rozycki]

On Fri, 5 Jun 2015, Leon Alrae wrote:

> > As a side note, I have seen that you have added a check for MIPS2 to the
> > ERET instruction. This is correct, but given in practice we don't
> > emulate any MIPS1 CPU, I do wonder if it's not the time to make MIPS2
> > the basic instruction set and remove all MIPS2 checks.
> 
> Yes, in current codebase the MIPS2 checks don't seem to have much value
> and the removal makes sense. On the other hand I'm wondering if there
> are QEMU users who maintain artificial MIPS1 CPU templates locally to
> test if their compiler doesn't emit any non-MIPS1 instructions.

 Actually ERET is an instruction that has been added with the MIPS III 
ISA.  Sort of, that is, as it was not treated a part of the ISA, but an 
implementation detail specific to the particular exception model, 
introduced with the R4000.  It was only accepted as a part of the 
instruction set with the MIPS32/MIPS64 (r1) architecture that made the 
privilege model a part of the ISA.  Either way real MIPS II hardware (the 
R6000 and some LSI ISA approximates) used RFE instead of ERET, just like 
the R3000.

 As to the removal of MIPS I checks, I think there is some value in 
keeping them for documentation purposes if nothing else.  There's the TLB 
and cache model missing for R3000 class processors, but adding it 
shouldn't be that difficult.  As I keep maintaining support for real MIPS 
I hardware still around I'd be happy to add the missing bits and the right 
templates for some MIPS I implementations sometime, however regrettably I 
can't promise any deadline.  Adding an actual system to emulate along the 
CPU would be another matter.

 It would make sense IMO however to define the minimum ISA level supported 
somehow at the `configure' stage, so that parts of the emulator to support 
older processor models can be optimised away at the compilation stage.  
This would be conceptually similar to what we do in the Linux kernel with 
some optional features, including the ISA level, that are hardcoded to 
specific settings for individual systems where we know from how the system 
has been built that are always present (or absent).

 NB MIPS I user-mode Linux emulation would not be a perfect model as we 
have some additions to the kernel extending the bare MIPS I ISA in the 
Reserved Instruction exception handler to make software's life easier.

  Maciej