| Message ID | 1433433631-5322-1-git-send-email-leon.alrae@imgtec.com |
|---|---|
| State | New |
| Headers | show |
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.
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
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
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,
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(-)