From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36800) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjTjI-0007gq-I8 for qemu-devel@nongnu.org; Wed, 29 Oct 2014 09:56:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XjTjE-0002RC-4k for qemu-devel@nongnu.org; Wed, 29 Oct 2014 09:56:04 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:24998) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjTjD-0002Qy-Lk for qemu-devel@nongnu.org; Wed, 29 Oct 2014 09:56:00 -0400 Message-ID: <5450F1EA.3060304@imgtec.com> Date: Wed, 29 Oct 2014 13:55:54 +0000 From: Yongbok Kim MIME-Version: 1.0 References: <1414154549-2102-1-git-send-email-leon.alrae@imgtec.com> <1414154549-2102-9-git-send-email-leon.alrae@imgtec.com> In-Reply-To: <1414154549-2102-9-git-send-email-leon.alrae@imgtec.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 08/15] target-mips: add BadInstr and BadInstrP support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leon Alrae , qemu-devel@nongnu.org Cc: aurelien@aurel32.net On 24/10/2014 13:42, Leon Alrae wrote: > BadInstr Register (CP0 Register 8, Select 1) > The BadInstr register is a read-only register that capture the most recent > instruction which caused an exception. > > BadInstrP Register (CP0 Register 8, Select 2) > The BadInstrP register contains the prior branch instruction, when the > faulting instruction is in a branch delay slot. > > Using error_code to indicate whether AdEL or TLBL was triggered during > instruction fetch, in this case BadInstr is not updated as valid instruction > word is not available. > > Signed-off-by: Leon Alrae > --- > target-mips/cpu.h | 6 ++++ > target-mips/helper.c | 45 +++++++++++++++++++++++++++-- > target-mips/op_helper.c | 17 +++++++++-- > target-mips/translate.c | 76 +++++++++++++++++++++++++++++++++++++++++++++---- > 4 files changed, 133 insertions(+), 11 deletions(-) > > diff --git a/target-mips/cpu.h b/target-mips/cpu.h > index 9c38b4f..4687f4f 100644 > --- a/target-mips/cpu.h > +++ b/target-mips/cpu.h > @@ -283,6 +283,8 @@ struct CPUMIPSState { > #define CP0SRSC4_SRS13 0 > int32_t CP0_HWREna; > target_ulong CP0_BadVAddr; > + uint32_t CP0_BadInstr; > + uint32_t CP0_BadInstrP; > int32_t CP0_Count; > target_ulong CP0_EntryHi; > #define CP0EnHi_EHINV 10 > @@ -383,6 +385,8 @@ struct CPUMIPSState { > #define CP0C2_SA 0 > int32_t CP0_Config3; > #define CP0C3_M 31 > +#define CP0C3_BP 27 > +#define CP0C3_BI 26 > #define CP0C3_ISA_ON_EXC 16 > #define CP0C3_ULRI 13 > #define CP0C3_RXI 12 > @@ -453,6 +457,8 @@ struct CPUMIPSState { > CPUMIPSFPUContext fpus[MIPS_FPU_MAX]; > /* QEMU */ > int error_code; > +#define EXCP_TLB_NOMATCH 0x1 > +#define EXCP_INST_NOTAVAIL 0x2 /* No valid instruction word for BadInstr */ > uint32_t hflags; /* CPU State */ > /* TMASK defines different execution modes */ > #define MIPS_HFLAG_TMASK 0x1807FF > diff --git a/target-mips/helper.c b/target-mips/helper.c > index 05325d9..c92b25c 100644 > --- a/target-mips/helper.c > +++ b/target-mips/helper.c > @@ -25,6 +25,7 @@ > > #include "cpu.h" > #include "sysemu/kvm.h" > +#include "exec/cpu_ldst.h" > > enum { > TLBRET_XI = -6, > @@ -241,6 +242,10 @@ static void raise_mmu_exception(CPUMIPSState *env, target_ulong address, > CPUState *cs = CPU(mips_env_get_cpu(env)); > int exception = 0, error_code = 0; > > + if (rw == MMU_INST_FETCH) { > + error_code |= EXCP_INST_NOTAVAIL; > + } > + > switch (tlb_error) { > default: > case TLBRET_BADADDR: > @@ -259,7 +264,7 @@ static void raise_mmu_exception(CPUMIPSState *env, target_ulong address, > } else { > exception = EXCP_TLBL; > } > - error_code = 1; > + error_code |= EXCP_TLB_NOMATCH; > break; > case TLBRET_INVALID: > /* TLB match with no valid bit */ > @@ -451,6 +456,21 @@ static void set_hflags_for_handler (CPUMIPSState *env) > << MIPS_HFLAG_M16_SHIFT); > } > } > + > +static inline void set_badinstr_registers(CPUMIPSState *env) > +{ > + if (env->hflags & MIPS_HFLAG_M16) { > + /* TODO: add BadInstr support for microMIPS */ > + return; > + } > + if (env->CP0_Config3 & (1 << CP0C3_BI)) { > + env->CP0_BadInstr = cpu_ldl_code(env, env->active_tc.PC); > + } > + if ((env->CP0_Config3 & (1 << CP0C3_BP)) && > + (env->hflags & MIPS_HFLAG_BMASK)) { > + env->CP0_BadInstrP = cpu_ldl_code(env, env->active_tc.PC - 4); > + } > +} > #endif > > void mips_cpu_do_interrupt(CPUState *cs) > @@ -458,6 +478,7 @@ void mips_cpu_do_interrupt(CPUState *cs) > #if !defined(CONFIG_USER_ONLY) > MIPSCPU *cpu = MIPS_CPU(cs); > CPUMIPSState *env = &cpu->env; > + bool update_badinstr = 0; > target_ulong offset; > int cause = -1; > const char *name; > @@ -566,10 +587,13 @@ void mips_cpu_do_interrupt(CPUState *cs) > goto set_EPC; > case EXCP_LTLBL: > cause = 1; > + update_badinstr = !(env->error_code & EXCP_INST_NOTAVAIL); > goto set_EPC; > case EXCP_TLBL: > cause = 2; > - if (env->error_code == 1 && !(env->CP0_Status & (1 << CP0St_EXL))) { > + update_badinstr = !(env->error_code & EXCP_INST_NOTAVAIL); > + if ((env->error_code & EXCP_TLB_NOMATCH) && > + !(env->CP0_Status & (1 << CP0St_EXL))) { > #if defined(TARGET_MIPS64) > int R = env->CP0_BadVAddr >> 62; > int UX = (env->CP0_Status & (1 << CP0St_UX)) != 0; > @@ -586,7 +610,9 @@ void mips_cpu_do_interrupt(CPUState *cs) > goto set_EPC; > case EXCP_TLBS: > cause = 3; > - if (env->error_code == 1 && !(env->CP0_Status & (1 << CP0St_EXL))) { > + update_badinstr = 1; > + if ((env->error_code & EXCP_TLB_NOMATCH) && > + !(env->CP0_Status & (1 << CP0St_EXL))) { > #if defined(TARGET_MIPS64) > int R = env->CP0_BadVAddr >> 62; > int UX = (env->CP0_Status & (1 << CP0St_UX)) != 0; > @@ -603,9 +629,11 @@ void mips_cpu_do_interrupt(CPUState *cs) > goto set_EPC; > case EXCP_AdEL: > cause = 4; > + update_badinstr = !(env->error_code & EXCP_INST_NOTAVAIL); > goto set_EPC; > case EXCP_AdES: > cause = 5; > + update_badinstr = 1; > goto set_EPC; > case EXCP_IBE: > cause = 6; > @@ -615,32 +643,40 @@ void mips_cpu_do_interrupt(CPUState *cs) > goto set_EPC; > case EXCP_SYSCALL: > cause = 8; > + update_badinstr = 1; > goto set_EPC; > case EXCP_BREAK: > cause = 9; > + update_badinstr = 1; > goto set_EPC; > case EXCP_RI: > cause = 10; > + update_badinstr = 1; > goto set_EPC; > case EXCP_CpU: > cause = 11; > + update_badinstr = 1; > env->CP0_Cause = (env->CP0_Cause & ~(0x3 << CP0Ca_CE)) | > (env->error_code << CP0Ca_CE); > goto set_EPC; > case EXCP_OVERFLOW: > cause = 12; > + update_badinstr = 1; > goto set_EPC; > case EXCP_TRAP: > cause = 13; > + update_badinstr = 1; > goto set_EPC; > case EXCP_FPE: > cause = 15; > + update_badinstr = 1; > goto set_EPC; > case EXCP_C2E: > cause = 18; > goto set_EPC; > case EXCP_TLBRI: > cause = 19; > + update_badinstr = 1; > goto set_EPC; > case EXCP_TLBXI: > cause = 20; TLBXI requires updating the register. DSPDIS might also. > @@ -671,6 +707,9 @@ void mips_cpu_do_interrupt(CPUState *cs) > set_EPC: > if (!(env->CP0_Status & (1 << CP0St_EXL))) { > env->CP0_EPC = exception_resume_pc(env); > + if (update_badinstr) { > + set_badinstr_registers(env); > + } > if (env->hflags & MIPS_HFLAG_BMASK) { > env->CP0_Cause |= (1U << CP0Ca_BD); > } else { > diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c > index 43a441a..6c87a1f 100644 > --- a/target-mips/op_helper.c > +++ b/target-mips/op_helper.c > @@ -2238,13 +2238,26 @@ void helper_wait(CPUMIPSState *env) > #if !defined(CONFIG_USER_ONLY) > > void mips_cpu_do_unaligned_access(CPUState *cs, vaddr addr, > - int is_write, int is_user, uintptr_t retaddr) > + int access_type, int is_user, > + uintptr_t retaddr) > { > MIPSCPU *cpu = MIPS_CPU(cs); > CPUMIPSState *env = &cpu->env; > + int error_code = 0; > + int excp; > > env->CP0_BadVAddr = addr; > - do_raise_exception(env, (is_write == 1) ? EXCP_AdES : EXCP_AdEL, retaddr); > + > + if (access_type == MMU_DATA_STORE) { > + excp = EXCP_AdES; > + } else { > + excp = EXCP_AdEL; > + if (access_type == MMU_INST_FETCH) { > + error_code |= EXCP_INST_NOTAVAIL; > + } > + } > + > + do_raise_exception_err(env, excp, error_code, retaddr); > } > > void tlb_fill(CPUState *cs, target_ulong addr, int is_write, int mmu_idx, > diff --git a/target-mips/translate.c b/target-mips/translate.c > index fdb61be..3e6e990 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -1175,6 +1175,8 @@ typedef struct DisasContext { > int kscrexist; > bool rxi; > int ie; > + bool bi; > + bool bp; > } DisasContext; > > enum { > @@ -4828,9 +4830,25 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel) > tcg_gen_ext32s_tl(arg, arg); > rn = "BadVAddr"; > break; > + case 1: > + if (ctx->bi) { > + gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstr)); > + rn = "BadInstr"; > + } else { > + gen_mfc0_unimplemented(ctx, arg); > + } > + break; > + case 2: > + if (ctx->bp) { > + gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrP)); > + rn = "BadInstrP"; > + } else { > + gen_mfc0_unimplemented(ctx, arg); > + } > + break; > default: > goto die; > - } > + } > break; > case 9: > switch (sel) { > @@ -5428,8 +5446,22 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel) > } > break; > case 8: > - /* ignored */ > - rn = "BadVAddr"; > + switch (sel) { > + case 0: > + /* ignored */ > + rn = "BadVAddr"; > + break; > + case 1: > + /* ignored */ > + rn = "BadInstr"; > + break; > + case 2: > + /* ignored */ > + rn = "BadInstrP"; > + break; > + default: > + goto die; > + } > break; > case 9: > switch (sel) { > @@ -6054,6 +6086,22 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel) > tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_BadVAddr)); > rn = "BadVAddr"; > break; > + case 1: > + if (ctx->bi) { > + gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstr)); > + rn = "BadInstr"; > + } else { > + gen_mfc0_unimplemented(ctx, arg); > + } > + break; > + case 2: > + if (ctx->bp) { > + gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrP)); > + rn = "BadInstrP"; > + } else { > + gen_mfc0_unimplemented(ctx, arg); > + } > + break; > default: > goto die; > } > @@ -6639,8 +6687,22 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel) > } > break; > case 8: > - /* ignored */ > - rn = "BadVAddr"; > + switch (sel) { > + case 0: > + /* ignored */ > + rn = "BadVAddr"; > + break; > + case 1: > + /* ignored */ > + rn = "BadInstr"; > + break; > + case 2: > + /* ignored */ > + rn = "BadInstrP"; > + break; > + default: > + goto die; > + } > break; > case 9: > switch (sel) { > @@ -16862,7 +16924,7 @@ static void decode_opc (CPUMIPSState *env, DisasContext *ctx) > /* make sure instructions are on a word boundary */ > if (ctx->pc & 0x3) { > env->CP0_BadVAddr = ctx->pc; > - generate_exception(ctx, EXCP_AdEL); > + generate_exception_err(ctx, EXCP_AdEL, EXCP_INST_NOTAVAIL); > return; > } > > @@ -17500,6 +17562,8 @@ gen_intermediate_code_internal(MIPSCPU *cpu, TranslationBlock *tb, > ctx.kscrexist = (env->CP0_Config4 >> CP0C4_KScrExist) & 0xff; > ctx.rxi = (env->CP0_Config3 >> CP0C3_RXI) & 1; > ctx.ie = (env->CP0_Config4 >> CP0C4_IE) & 3; > + ctx.bi = (env->CP0_Config3 >> CP0C3_BI) & 1; > + ctx.bp = (env->CP0_Config3 >> CP0C3_BP) & 1; > /* Restore delay slot state from the tb context. */ > ctx.hflags = (uint32_t)tb->flags; /* FIXME: maybe use 64 bits here? */ > ctx.ulri = env->CP0_Config3 & (1 << CP0C3_ULRI); otherwise, Reviewed-by: Yongbok Kim Regards, Yongbok