From 6b7f0b61f080b886c9b4bba8240379ce90e20b12 Mon Sep 17 00:00:00 2001 From: Peter Maydell <peter.maydell@linaro.org> Date: Thu, 11 Feb 2016 11:17:30 +0000 Subject: [PATCH 01/15] target-arm: Fix typo in comment in arm_is_secure_below_el3() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a typo where "EL2" was written but "EL3" intended. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com> Message-id: 1454506721-11843-2-git-send-email-peter.maydell@linaro.org --- target-arm/cpu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-arm/cpu.h b/target-arm/cpu.h index b8b3364615..52284e9aaf 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -931,7 +931,7 @@ static inline bool arm_is_secure_below_el3(CPUARMState *env) if (arm_feature(env, ARM_FEATURE_EL3)) { return !(env->cp15.scr_el3 & SCR_NS); } else { - /* If EL2 is not supported then the secure state is implementation + /* If EL3 is not supported then the secure state is implementation * defined, in which case QEMU defaults to non-secure. */ return false; From 5513c3abed8e5fabe116830c63f0d3fe1f94bd21 Mon Sep 17 00:00:00 2001 From: Peter Maydell <peter.maydell@linaro.org> Date: Thu, 11 Feb 2016 11:17:30 +0000 Subject: [PATCH 02/15] target-arm: Implement MDCR_EL3 and SDCR Implement the MDCR_EL3 register (which is SDCR for AArch32). For the moment we implement it as reads-as-written. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Message-id: 1454506721-11843-3-git-send-email-peter.maydell@linaro.org --- target-arm/cpu.h | 1 + target-arm/helper.c | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 52284e9aaf..cf2df5032c 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -382,6 +382,7 @@ typedef struct CPUARMState { uint64_t mdscr_el1; uint64_t oslsr_el1; /* OS Lock Status */ uint64_t mdcr_el2; + uint64_t mdcr_el3; /* If the counter is enabled, this stores the last time the counter * was reset. Otherwise it stores the counter value */ diff --git a/target-arm/helper.c b/target-arm/helper.c index 5ea507f5bc..1efe304532 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -364,6 +364,24 @@ static CPAccessResult access_el3_aa32ns_aa64any(CPUARMState *env, return CP_ACCESS_OK; } +/* Some secure-only AArch32 registers trap to EL3 if used from + * Secure EL1 (but are just ordinary UNDEF in other non-EL3 contexts). + * Note that an access from Secure EL1 can only happen if EL3 is AArch64. + * We assume that the .access field is set to PL1_RW. + */ +static CPAccessResult access_trap_aa32s_el1(CPUARMState *env, + const ARMCPRegInfo *ri) +{ + if (arm_current_el(env) == 3) { + return CP_ACCESS_OK; + } + if (arm_is_secure_below_el3(env)) { + return CP_ACCESS_TRAP_EL3; + } + /* This will be EL1 NS and EL2 NS, which just UNDEF */ + return CP_ACCESS_TRAP_UNCATEGORIZED; +} + static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { ARMCPU *cpu = arm_env_get_cpu(env); @@ -3532,6 +3550,14 @@ static const ARMCPRegInfo el3_cp_reginfo[] = { .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0, .access = PL3_RW, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3), .writefn = scr_write }, + { .name = "MDCR_EL3", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 3, .opc2 = 1, + .resetvalue = 0, + .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el3) }, + { .name = "SDCR", .type = ARM_CP_ALIAS, + .cp = 15, .opc1 = 0, .crn = 1, .crm = 3, .opc2 = 1, + .access = PL1_RW, .accessfn = access_trap_aa32s_el1, + .fieldoffset = offsetoflow32(CPUARMState, cp15.mdcr_el3) }, { .name = "SDER32_EL3", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 1, .access = PL3_RW, .resetvalue = 0, From efe4a274083f61484a8f1478d93f229d43aa8095 Mon Sep 17 00:00:00 2001 From: Peter Maydell <peter.maydell@linaro.org> Date: Thu, 11 Feb 2016 11:17:30 +0000 Subject: [PATCH 03/15] target-arm: Use access_trap_aa32s_el1() for SCR and MVBAR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The registers MVBAR and SCR should have the behaviour of trapping to EL3 if accessed from Secure EL1, but we were incorrectly implementing them to UNDEF (which would trap to EL1). Fix this by using the new access_trap_aa32s_el1() access function. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Message-id: 1454506721-11843-4-git-send-email-peter.maydell@linaro.org --- target-arm/helper.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 1efe304532..98eccd621c 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -3548,7 +3548,8 @@ static const ARMCPRegInfo el3_cp_reginfo[] = { .resetvalue = 0, .writefn = scr_write }, { .name = "SCR", .type = ARM_CP_ALIAS, .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0, - .access = PL3_RW, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3), + .access = PL1_RW, .accessfn = access_trap_aa32s_el1, + .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3), .writefn = scr_write }, { .name = "MDCR_EL3", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 3, .opc2 = 1, @@ -3571,7 +3572,8 @@ static const ARMCPRegInfo el3_cp_reginfo[] = { .access = PL3_W | PL1_R, .resetvalue = 0, .fieldoffset = offsetof(CPUARMState, cp15.nsacr) }, { .name = "MVBAR", .cp = 15, .opc1 = 0, .crn = 12, .crm = 0, .opc2 = 1, - .access = PL3_RW, .writefn = vbar_write, .resetvalue = 0, + .access = PL1_RW, .accessfn = access_trap_aa32s_el1, + .writefn = vbar_write, .resetvalue = 0, .fieldoffset = offsetof(CPUARMState, cp15.mvbar) }, { .name = "SCTLR_EL3", .state = ARM_CP_STATE_AA64, .type = ARM_CP_ALIAS, /* reset handled by AArch32 view */ From 533e93f1cf12c570aab45f14663dab6fb8ea3ffc Mon Sep 17 00:00:00 2001 From: Peter Maydell <peter.maydell@linaro.org> Date: Thu, 11 Feb 2016 11:17:30 +0000 Subject: [PATCH 04/15] target-arm: Update arm_generate_debug_exceptions() to handle EL2/EL3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The arm_generate_debug_exceptions() function as originally implemented assumes no EL2 or EL3. Since we now have much more of an implementation of those now, fix this assumption. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com> Message-id: 1454506721-11843-5-git-send-email-peter.maydell@linaro.org --- target-arm/cpu.h | 48 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/target-arm/cpu.h b/target-arm/cpu.h index cf2df5032c..0fb79d0879 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -1742,9 +1742,7 @@ typedef enum ARMASIdx { ARMASIdx_S = 1, } ARMASIdx; -/* Return the Exception Level targeted by debug exceptions; - * currently always EL1 since we don't implement EL2 or EL3. - */ +/* Return the Exception Level targeted by debug exceptions. */ static inline int arm_debug_target_el(CPUARMState *env) { bool secure = arm_is_secure(env); @@ -1767,6 +1765,14 @@ static inline int arm_debug_target_el(CPUARMState *env) static inline bool aa64_generate_debug_exceptions(CPUARMState *env) { + if (arm_is_secure(env)) { + /* MDCR_EL3.SDD disables debug events from Secure state */ + if (extract32(env->cp15.mdcr_el3, 16, 1) != 0 + || arm_current_el(env) == 3) { + return false; + } + } + if (arm_current_el(env) == arm_debug_target_el(env)) { if ((extract32(env->cp15.mdscr_el1, 13, 1) == 0) || (env->daif & PSTATE_D)) { @@ -1778,10 +1784,42 @@ static inline bool aa64_generate_debug_exceptions(CPUARMState *env) static inline bool aa32_generate_debug_exceptions(CPUARMState *env) { - if (arm_current_el(env) == 0 && arm_el_is_aa64(env, 1)) { + int el = arm_current_el(env); + + if (el == 0 && arm_el_is_aa64(env, 1)) { return aa64_generate_debug_exceptions(env); } - return arm_current_el(env) != 2; + + if (arm_is_secure(env)) { + int spd; + + if (el == 0 && (env->cp15.sder & 1)) { + /* SDER.SUIDEN means debug exceptions from Secure EL0 + * are always enabled. Otherwise they are controlled by + * SDCR.SPD like those from other Secure ELs. + */ + return true; + } + + spd = extract32(env->cp15.mdcr_el3, 14, 2); + switch (spd) { + case 1: + /* SPD == 0b01 is reserved, but behaves as 0b00. */ + case 0: + /* For 0b00 we return true if external secure invasive debug + * is enabled. On real hardware this is controlled by external + * signals to the core. QEMU always permits debug, and behaves + * as if DBGEN, SPIDEN, NIDEN and SPNIDEN are all tied high. + */ + return true; + case 2: + return false; + case 3: + return true; + } + } + + return el != 2; } /* Return true if debugging exceptions are currently enabled. From 3f208fd76bcc91a8506681bb8472f2398fe6f487 Mon Sep 17 00:00:00 2001 From: Peter Maydell <peter.maydell@linaro.org> Date: Thu, 11 Feb 2016 11:17:31 +0000 Subject: [PATCH 05/15] target-arm: Add isread parameter to CPAccessFns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit System registers might have access requirements which need to be described via a CPAccessFn and which differ for reads and writes. For this to be possible we need to pass the access function a parameter to tell it whether the access being checked is a read or a write. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com> Message-id: 1454506721-11843-6-git-send-email-peter.maydell@linaro.org --- target-arm/cpu.h | 4 +- target-arm/helper.c | 81 ++++++++++++++++++++++++-------------- target-arm/helper.h | 2 +- target-arm/op_helper.c | 5 ++- target-arm/translate-a64.c | 6 ++- target-arm/translate.c | 7 +++- 6 files changed, 68 insertions(+), 37 deletions(-) diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 0fb79d0879..5137632ccc 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -1319,7 +1319,9 @@ typedef uint64_t CPReadFn(CPUARMState *env, const ARMCPRegInfo *opaque); typedef void CPWriteFn(CPUARMState *env, const ARMCPRegInfo *opaque, uint64_t value); /* Access permission check functions for coprocessor registers. */ -typedef CPAccessResult CPAccessFn(CPUARMState *env, const ARMCPRegInfo *opaque); +typedef CPAccessResult CPAccessFn(CPUARMState *env, + const ARMCPRegInfo *opaque, + bool isread); /* Hook function for register reset */ typedef void CPResetFn(CPUARMState *env, const ARMCPRegInfo *opaque); diff --git a/target-arm/helper.c b/target-arm/helper.c index 98eccd621c..1db8de3ad3 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -344,7 +344,8 @@ void init_cpreg_list(ARMCPU *cpu) * access_el3_aa32ns_aa64any: Used to check both AArch32/64 register views. */ static CPAccessResult access_el3_aa32ns(CPUARMState *env, - const ARMCPRegInfo *ri) + const ARMCPRegInfo *ri, + bool isread) { bool secure = arm_is_secure_below_el3(env); @@ -356,10 +357,11 @@ static CPAccessResult access_el3_aa32ns(CPUARMState *env, } static CPAccessResult access_el3_aa32ns_aa64any(CPUARMState *env, - const ARMCPRegInfo *ri) + const ARMCPRegInfo *ri, + bool isread) { if (!arm_el_is_aa64(env, 3)) { - return access_el3_aa32ns(env, ri); + return access_el3_aa32ns(env, ri, isread); } return CP_ACCESS_OK; } @@ -370,7 +372,8 @@ static CPAccessResult access_el3_aa32ns_aa64any(CPUARMState *env, * We assume that the .access field is set to PL1_RW. */ static CPAccessResult access_trap_aa32s_el1(CPUARMState *env, - const ARMCPRegInfo *ri) + const ARMCPRegInfo *ri, + bool isread) { if (arm_current_el(env) == 3) { return CP_ACCESS_OK; @@ -652,7 +655,8 @@ static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri, env->cp15.cpacr_el1 = value; } -static CPAccessResult cpacr_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult cpacr_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { if (arm_feature(env, ARM_FEATURE_V8)) { /* Check if CPACR accesses are to be trapped to EL2 */ @@ -669,7 +673,8 @@ static CPAccessResult cpacr_access(CPUARMState *env, const ARMCPRegInfo *ri) return CP_ACCESS_OK; } -static CPAccessResult cptr_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult cptr_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { /* Check if CPTR accesses are set to trap to EL3 */ if (arm_current_el(env) == 2 && (env->cp15.cptr_el[3] & CPTR_TCPAC)) { @@ -711,7 +716,8 @@ static const ARMCPRegInfo v6_cp_reginfo[] = { REGINFO_SENTINEL }; -static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { /* Performance monitor registers user accessibility is controlled * by PMUSERENR. @@ -1155,7 +1161,8 @@ static void teecr_write(CPUARMState *env, const ARMCPRegInfo *ri, env->teecr = value; } -static CPAccessResult teehbr_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult teehbr_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { if (arm_current_el(env) == 0 && (env->teecr & 1)) { return CP_ACCESS_TRAP; @@ -1208,7 +1215,8 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = { #ifndef CONFIG_USER_ONLY -static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { /* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero */ if (arm_current_el(env) == 0 && !extract32(env->cp15.c14_cntkctl, 0, 2)) { @@ -1217,7 +1225,8 @@ static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo *ri) return CP_ACCESS_OK; } -static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx) +static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx, + bool isread) { unsigned int cur_el = arm_current_el(env); bool secure = arm_is_secure(env); @@ -1236,7 +1245,8 @@ static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx) return CP_ACCESS_OK; } -static CPAccessResult gt_timer_access(CPUARMState *env, int timeridx) +static CPAccessResult gt_timer_access(CPUARMState *env, int timeridx, + bool isread) { unsigned int cur_el = arm_current_el(env); bool secure = arm_is_secure(env); @@ -1258,29 +1268,34 @@ static CPAccessResult gt_timer_access(CPUARMState *env, int timeridx) } static CPAccessResult gt_pct_access(CPUARMState *env, - const ARMCPRegInfo *ri) + const ARMCPRegInfo *ri, + bool isread) { - return gt_counter_access(env, GTIMER_PHYS); + return gt_counter_access(env, GTIMER_PHYS, isread); } static CPAccessResult gt_vct_access(CPUARMState *env, - const ARMCPRegInfo *ri) + const ARMCPRegInfo *ri, + bool isread) { - return gt_counter_access(env, GTIMER_VIRT); + return gt_counter_access(env, GTIMER_VIRT, isread); } -static CPAccessResult gt_ptimer_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult gt_ptimer_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { - return gt_timer_access(env, GTIMER_PHYS); + return gt_timer_access(env, GTIMER_PHYS, isread); } -static CPAccessResult gt_vtimer_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult gt_vtimer_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { - return gt_timer_access(env, GTIMER_VIRT); + return gt_timer_access(env, GTIMER_VIRT, isread); } static CPAccessResult gt_stimer_access(CPUARMState *env, - const ARMCPRegInfo *ri) + const ARMCPRegInfo *ri, + bool isread) { /* The AArch64 register view of the secure physical timer is * always accessible from EL3, and configurably accessible from @@ -1777,7 +1792,8 @@ static void par_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) #ifndef CONFIG_USER_ONLY /* get_phys_addr() isn't present for user-mode-only targets */ -static CPAccessResult ats_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult ats_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { if (ri->opc2 & 4) { /* The ATS12NSO* operations must trap to EL3 if executed in @@ -1922,7 +1938,8 @@ static void ats1h_write(CPUARMState *env, const ARMCPRegInfo *ri, A32_BANKED_CURRENT_REG_SET(env, par, par64); } -static CPAccessResult at_s1e2_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult at_s1e2_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { if (arm_current_el(env) == 3 && !(env->cp15.scr_el3 & SCR_NS)) { return CP_ACCESS_TRAP; @@ -2576,7 +2593,8 @@ static void aa64_fpsr_write(CPUARMState *env, const ARMCPRegInfo *ri, vfp_set_fpsr(env, value); } -static CPAccessResult aa64_daif_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult aa64_daif_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UMA)) { return CP_ACCESS_TRAP; @@ -2591,7 +2609,8 @@ static void aa64_daif_write(CPUARMState *env, const ARMCPRegInfo *ri, } static CPAccessResult aa64_cacheop_access(CPUARMState *env, - const ARMCPRegInfo *ri) + const ARMCPRegInfo *ri, + bool isread) { /* Cache invalidate/clean: NOP, but EL0 must UNDEF unless * SCTLR_EL1.UCI is set. @@ -2847,7 +2866,8 @@ static void tlbi_aa64_ipas2e1is_write(CPUARMState *env, const ARMCPRegInfo *ri, } } -static CPAccessResult aa64_zva_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult aa64_zva_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { /* We don't implement EL2, so the only control on DC ZVA is the * bit in the SCTLR which can prohibit access for EL0. @@ -2864,13 +2884,14 @@ static uint64_t aa64_dczid_read(CPUARMState *env, const ARMCPRegInfo *ri) int dzp_bit = 1 << 4; /* DZP indicates whether DC ZVA access is allowed */ - if (aa64_zva_access(env, NULL) == CP_ACCESS_OK) { + if (aa64_zva_access(env, NULL, false) == CP_ACCESS_OK) { dzp_bit = 0; } return cpu->dcz_blocksize | dzp_bit; } -static CPAccessResult sp_el0_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult sp_el0_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { if (!(env->pstate & PSTATE_SP)) { /* Access to SP_EL0 is undefined if it's being used as @@ -2909,7 +2930,8 @@ static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri, tlb_flush(CPU(cpu), 1); } -static CPAccessResult fpexc32_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult fpexc32_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { if ((env->cp15.cptr_el[2] & CPTR_TFP) && arm_current_el(env) == 2) { return CP_ACCESS_TRAP_EL2; @@ -3658,7 +3680,8 @@ static const ARMCPRegInfo el3_cp_reginfo[] = { REGINFO_SENTINEL }; -static CPAccessResult ctr_el0_access(CPUARMState *env, const ARMCPRegInfo *ri) +static CPAccessResult ctr_el0_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) { /* Only accessible in EL0 if SCTLR.UCT is set (and only in AArch64, * but the AArch32 CTR has its own reginfo struct) diff --git a/target-arm/helper.h b/target-arm/helper.h index c2a85c722a..c98e9cea3d 100644 --- a/target-arm/helper.h +++ b/target-arm/helper.h @@ -62,7 +62,7 @@ DEF_HELPER_1(cpsr_read, i32, env) DEF_HELPER_3(v7m_msr, void, env, i32, i32) DEF_HELPER_2(v7m_mrs, i32, env, i32) -DEF_HELPER_3(access_check_cp_reg, void, env, ptr, i32) +DEF_HELPER_4(access_check_cp_reg, void, env, ptr, i32, i32) DEF_HELPER_3(set_cp_reg, void, env, ptr, i32) DEF_HELPER_2(get_cp_reg, i32, env, ptr) DEF_HELPER_3(set_cp_reg64, void, env, ptr, i64) diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index a5ee65fe2f..313c0f8bd7 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -457,7 +457,8 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t regno, uint32_t val) } } -void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome) +void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome, + uint32_t isread) { const ARMCPRegInfo *ri = rip; int target_el; @@ -471,7 +472,7 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome) return; } - switch (ri->accessfn(env, ri)) { + switch (ri->accessfn(env, ri, isread)) { case CP_ACCESS_OK: return; case CP_ACCESS_TRAP: diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c index d780e09900..7f65aeab64 100644 --- a/target-arm/translate-a64.c +++ b/target-arm/translate-a64.c @@ -1367,16 +1367,18 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, * runtime; this may result in an exception. */ TCGv_ptr tmpptr; - TCGv_i32 tcg_syn; + TCGv_i32 tcg_syn, tcg_isread; uint32_t syndrome; gen_a64_set_pc_im(s->pc - 4); tmpptr = tcg_const_ptr(ri); syndrome = syn_aa64_sysregtrap(op0, op1, op2, crn, crm, rt, isread); tcg_syn = tcg_const_i32(syndrome); - gen_helper_access_check_cp_reg(cpu_env, tmpptr, tcg_syn); + tcg_isread = tcg_const_i32(isread); + gen_helper_access_check_cp_reg(cpu_env, tmpptr, tcg_syn, tcg_isread); tcg_temp_free_ptr(tmpptr); tcg_temp_free_i32(tcg_syn); + tcg_temp_free_i32(tcg_isread); } /* Handle special cases first */ diff --git a/target-arm/translate.c b/target-arm/translate.c index f6a38bcc09..2c8213b07a 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -7169,7 +7169,7 @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) * call in order to handle c15_cpar. */ TCGv_ptr tmpptr; - TCGv_i32 tcg_syn; + TCGv_i32 tcg_syn, tcg_isread; uint32_t syndrome; /* Note that since we are an implementation which takes an @@ -7214,9 +7214,12 @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) gen_set_pc_im(s, s->pc - 4); tmpptr = tcg_const_ptr(ri); tcg_syn = tcg_const_i32(syndrome); - gen_helper_access_check_cp_reg(cpu_env, tmpptr, tcg_syn); + tcg_isread = tcg_const_i32(isread); + gen_helper_access_check_cp_reg(cpu_env, tmpptr, tcg_syn, + tcg_isread); tcg_temp_free_ptr(tmpptr); tcg_temp_free_i32(tcg_syn); + tcg_temp_free_i32(tcg_isread); } /* Handle special cases first */ From 2f027fc52d4b444a47cb05a9c96697372a6b57d2 Mon Sep 17 00:00:00 2001 From: Peter Maydell <peter.maydell@linaro.org> Date: Thu, 11 Feb 2016 11:17:31 +0000 Subject: [PATCH 06/15] target-arm: Implement NSACR trapping behaviour Implement some corner cases of the behaviour of the NSACR register on ARMv8: * if EL3 is AArch64 then accessing the NSACR from Secure EL1 with AArch32 should trap to EL3 * if EL3 is not present or is AArch64 then reads from NS EL1 and NS EL2 return constant 0xc00 It would in theory be possible to implement all these with a single reginfo definition, but for clarity we use three separate definitions for the three cases and install the right one based on the CPU feature flags. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Message-id: 1454506721-11843-7-git-send-email-peter.maydell@linaro.org --- target-arm/helper.c | 62 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 1db8de3ad3..2f9db72806 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -3563,6 +3563,25 @@ static const ARMCPRegInfo el2_cp_reginfo[] = { REGINFO_SENTINEL }; +static CPAccessResult nsacr_access(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) +{ + /* The NSACR is RW at EL3, and RO for NS EL1 and NS EL2. + * At Secure EL1 it traps to EL3. + */ + if (arm_current_el(env) == 3) { + return CP_ACCESS_OK; + } + if (arm_is_secure_below_el3(env)) { + return CP_ACCESS_TRAP_EL3; + } + /* Accesses from EL1 NS and EL2 NS are UNDEF for write but allow reads. */ + if (isread) { + return CP_ACCESS_OK; + } + return CP_ACCESS_TRAP_UNCATEGORIZED; +} + static const ARMCPRegInfo el3_cp_reginfo[] = { { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0, @@ -3589,10 +3608,6 @@ static const ARMCPRegInfo el3_cp_reginfo[] = { .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 1, .access = PL3_RW, .resetvalue = 0, .fieldoffset = offsetoflow32(CPUARMState, cp15.sder) }, - /* TODO: Implement NSACR trapping of secure EL1 accesses to EL3 */ - { .name = "NSACR", .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2, - .access = PL3_W | PL1_R, .resetvalue = 0, - .fieldoffset = offsetof(CPUARMState, cp15.nsacr) }, { .name = "MVBAR", .cp = 15, .opc1 = 0, .crn = 12, .crm = 0, .opc2 = 1, .access = PL1_RW, .accessfn = access_trap_aa32s_el1, .writefn = vbar_write, .resetvalue = 0, @@ -4363,6 +4378,45 @@ void register_cp_regs_for_features(ARMCPU *cpu) }; define_one_arm_cp_reg(cpu, &rvbar); } + /* The behaviour of NSACR is sufficiently various that we don't + * try to describe it in a single reginfo: + * if EL3 is 64 bit, then trap to EL3 from S EL1, + * reads as constant 0xc00 from NS EL1 and NS EL2 + * if EL3 is 32 bit, then RW at EL3, RO at NS EL1 and NS EL2 + * if v7 without EL3, register doesn't exist + * if v8 without EL3, reads as constant 0xc00 from NS EL1 and NS EL2 + */ + if (arm_feature(env, ARM_FEATURE_EL3)) { + if (arm_feature(env, ARM_FEATURE_AARCH64)) { + ARMCPRegInfo nsacr = { + .name = "NSACR", .type = ARM_CP_CONST, + .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2, + .access = PL1_RW, .accessfn = nsacr_access, + .resetvalue = 0xc00 + }; + define_one_arm_cp_reg(cpu, &nsacr); + } else { + ARMCPRegInfo nsacr = { + .name = "NSACR", + .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2, + .access = PL3_RW | PL1_R, + .resetvalue = 0, + .fieldoffset = offsetof(CPUARMState, cp15.nsacr) + }; + define_one_arm_cp_reg(cpu, &nsacr); + } + } else { + if (arm_feature(env, ARM_FEATURE_V8)) { + ARMCPRegInfo nsacr = { + .name = "NSACR", .type = ARM_CP_CONST, + .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2, + .access = PL1_R, + .resetvalue = 0xc00 + }; + define_one_arm_cp_reg(cpu, &nsacr); + } + } + if (arm_feature(env, ARM_FEATURE_MPU)) { if (arm_feature(env, ARM_FEATURE_V6)) { /* PMSAv6 not implemented */ From 3ad901bc2b98f5539af9a7d4aef140a6d8fa6442 Mon Sep 17 00:00:00 2001 From: Peter Maydell <peter.maydell@linaro.org> Date: Thu, 11 Feb 2016 11:17:31 +0000 Subject: [PATCH 07/15] target-arm: Enable EL3 for Cortex-A53 and Cortex-A57 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enable EL3 support for our Cortex-A53 and Cortex-A57 CPU models. We have enough implemented now to be able to run real world code at least to some extent (I can boot ARM Trusted Firmware to the point where it pulls in OP-TEE and then falls over because it doesn't have a UEFI image it can chain to). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com> Message-id: 1454506721-11843-8-git-send-email-peter.maydell@linaro.org --- target-arm/cpu64.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c index c847513b25..c5bc19a405 100644 --- a/target-arm/cpu64.c +++ b/target-arm/cpu64.c @@ -109,6 +109,7 @@ static void aarch64_a57_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_V8_SHA256); set_feature(&cpu->env, ARM_FEATURE_V8_PMULL); set_feature(&cpu->env, ARM_FEATURE_CRC); + set_feature(&cpu->env, ARM_FEATURE_EL3); cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A57; cpu->midr = 0x411fd070; cpu->revidr = 0x00000000; @@ -161,6 +162,7 @@ static void aarch64_a53_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_V8_SHA256); set_feature(&cpu->env, ARM_FEATURE_V8_PMULL); set_feature(&cpu->env, ARM_FEATURE_CRC); + set_feature(&cpu->env, ARM_FEATURE_EL3); cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A53; cpu->midr = 0x410fd034; cpu->revidr = 0x00000000; From fc05f4a62c568b607ec3fe428a419bb38205b570 Mon Sep 17 00:00:00 2001 From: Peter Maydell <peter.maydell@linaro.org> Date: Thu, 11 Feb 2016 11:17:31 +0000 Subject: [PATCH 08/15] target-arm: Correct misleading 'is_thumb' syn_* parameter names In syndrome register values, the IL bit indicates the instruction length, and is 1 for 4-byte instructions and 0 for 2-byte instructions. All A64 and A32 instructions are 4-byte, but Thumb instructions may be either 2 or 4 bytes long. Unfortunately we named the parameter to the syn_* functions for constructing syndromes "is_thumb", which falsely implies that it should be set for all Thumb instructions, rather than only the 16-bit ones. Fix the functions to name the parameter 'is_16bit' instead. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com> Message-id: 1454683067-16001-2-git-send-email-peter.maydell@linaro.org --- target-arm/internals.h | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/target-arm/internals.h b/target-arm/internals.h index d226bbe857..a648c1e7d5 100644 --- a/target-arm/internals.h +++ b/target-arm/internals.h @@ -270,10 +270,10 @@ static inline uint32_t syn_aa64_smc(uint32_t imm16) return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff); } -static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb) +static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_16bit) { return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff) - | (is_thumb ? 0 : ARM_EL_IL); + | (is_16bit ? 0 : ARM_EL_IL); } static inline uint32_t syn_aa32_hvc(uint32_t imm16) @@ -291,10 +291,10 @@ static inline uint32_t syn_aa64_bkpt(uint32_t imm16) return (EC_AA64_BKPT << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff); } -static inline uint32_t syn_aa32_bkpt(uint32_t imm16, bool is_thumb) +static inline uint32_t syn_aa32_bkpt(uint32_t imm16, bool is_16bit) { return (EC_AA32_BKPT << ARM_EL_EC_SHIFT) | (imm16 & 0xffff) - | (is_thumb ? 0 : ARM_EL_IL); + | (is_16bit ? 0 : ARM_EL_IL); } static inline uint32_t syn_aa64_sysregtrap(int op0, int op1, int op2, @@ -308,48 +308,48 @@ static inline uint32_t syn_aa64_sysregtrap(int op0, int op1, int op2, static inline uint32_t syn_cp14_rt_trap(int cv, int cond, int opc1, int opc2, int crn, int crm, int rt, int isread, - bool is_thumb) + bool is_16bit) { return (EC_CP14RTTRAP << ARM_EL_EC_SHIFT) - | (is_thumb ? 0 : ARM_EL_IL) + | (is_16bit ? 0 : ARM_EL_IL) | (cv << 24) | (cond << 20) | (opc2 << 17) | (opc1 << 14) | (crn << 10) | (rt << 5) | (crm << 1) | isread; } static inline uint32_t syn_cp15_rt_trap(int cv, int cond, int opc1, int opc2, int crn, int crm, int rt, int isread, - bool is_thumb) + bool is_16bit) { return (EC_CP15RTTRAP << ARM_EL_EC_SHIFT) - | (is_thumb ? 0 : ARM_EL_IL) + | (is_16bit ? 0 : ARM_EL_IL) | (cv << 24) | (cond << 20) | (opc2 << 17) | (opc1 << 14) | (crn << 10) | (rt << 5) | (crm << 1) | isread; } static inline uint32_t syn_cp14_rrt_trap(int cv, int cond, int opc1, int crm, int rt, int rt2, int isread, - bool is_thumb) + bool is_16bit) { return (EC_CP14RRTTRAP << ARM_EL_EC_SHIFT) - | (is_thumb ? 0 : ARM_EL_IL) + | (is_16bit ? 0 : ARM_EL_IL) | (cv << 24) | (cond << 20) | (opc1 << 16) | (rt2 << 10) | (rt << 5) | (crm << 1) | isread; } static inline uint32_t syn_cp15_rrt_trap(int cv, int cond, int opc1, int crm, int rt, int rt2, int isread, - bool is_thumb) + bool is_16bit) { return (EC_CP15RRTTRAP << ARM_EL_EC_SHIFT) - | (is_thumb ? 0 : ARM_EL_IL) + | (is_16bit ? 0 : ARM_EL_IL) | (cv << 24) | (cond << 20) | (opc1 << 16) | (rt2 << 10) | (rt << 5) | (crm << 1) | isread; } -static inline uint32_t syn_fp_access_trap(int cv, int cond, bool is_thumb) +static inline uint32_t syn_fp_access_trap(int cv, int cond, bool is_16bit) { return (EC_ADVSIMDFPACCESSTRAP << ARM_EL_EC_SHIFT) - | (is_thumb ? 0 : ARM_EL_IL) + | (is_16bit ? 0 : ARM_EL_IL) | (cv << 24) | (cond << 20); } From 4df322593037d2700f72dfdfb967300b7ad2e696 Mon Sep 17 00:00:00 2001 From: Peter Maydell <peter.maydell@linaro.org> Date: Thu, 11 Feb 2016 11:17:31 +0000 Subject: [PATCH 09/15] target-arm: Fix IL bit reported for Thumb coprocessor traps All Thumb coprocessor instructions are 32 bits, so the IL bit in the syndrome register should be set. Pass false to the syn_* function's is_16bit argument rather than s->thumb so we report the correct IL bit. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com> Message-id: 1454683067-16001-3-git-send-email-peter.maydell@linaro.org --- target-arm/translate.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 2c8213b07a..8e8ffee9c6 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -7184,19 +7184,19 @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) case 14: if (is64) { syndrome = syn_cp14_rrt_trap(1, 0xe, opc1, crm, rt, rt2, - isread, s->thumb); + isread, false); } else { syndrome = syn_cp14_rt_trap(1, 0xe, opc1, opc2, crn, crm, - rt, isread, s->thumb); + rt, isread, false); } break; case 15: if (is64) { syndrome = syn_cp15_rrt_trap(1, 0xe, opc1, crm, rt, rt2, - isread, s->thumb); + isread, false); } else { syndrome = syn_cp15_rt_trap(1, 0xe, opc1, opc2, crn, crm, - rt, isread, s->thumb); + rt, isread, false); } break; default: From 7d197d2db5e99e4c8b20f6771ddc7303acaa1c89 Mon Sep 17 00:00:00 2001 From: Peter Maydell <peter.maydell@linaro.org> Date: Thu, 11 Feb 2016 11:17:32 +0000 Subject: [PATCH 10/15] target-arm: Fix IL bit reported for Thumb VFP and Neon traps All Thumb Neon and VFP instructions are 32 bits, so the IL bit in the syndrome register should be set. Pass false to the syn_* function's is_16bit argument rather than s->thumb so we report the correct IL bit. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com> Message-id: 1454683067-16001-4-git-send-email-peter.maydell@linaro.org --- target-arm/translate.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 8e8ffee9c6..cf3dc33774 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -3077,7 +3077,7 @@ static int disas_vfp_insn(DisasContext *s, uint32_t insn) */ if (s->fp_excp_el) { gen_exception_insn(s, 4, EXCP_UDEF, - syn_fp_access_trap(1, 0xe, s->thumb), s->fp_excp_el); + syn_fp_access_trap(1, 0xe, false), s->fp_excp_el); return 0; } @@ -4399,7 +4399,7 @@ static int disas_neon_ls_insn(DisasContext *s, uint32_t insn) */ if (s->fp_excp_el) { gen_exception_insn(s, 4, EXCP_UDEF, - syn_fp_access_trap(1, 0xe, s->thumb), s->fp_excp_el); + syn_fp_access_trap(1, 0xe, false), s->fp_excp_el); return 0; } @@ -5137,7 +5137,7 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn) */ if (s->fp_excp_el) { gen_exception_insn(s, 4, EXCP_UDEF, - syn_fp_access_trap(1, 0xe, s->thumb), s->fp_excp_el); + syn_fp_access_trap(1, 0xe, false), s->fp_excp_el); return 0; } From 568496c0c0f1863a4bc18539962cd8d81baa4e30 Mon Sep 17 00:00:00 2001 From: Sergey Fedorov <serge.fdrv@gmail.com> Date: Thu, 11 Feb 2016 11:17:32 +0000 Subject: [PATCH 11/15] cpu: Add callback to check architectural watchpoint match When QEMU watchpoint matches, that is not definitely an architectural watchpoint match yet. If it is a stop-before-access watchpoint then that is hardly possible to ignore it after throwing a TCG exception. A special callback is introduced to check for architectural watchpoint match before raising a TCG exception. Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com> Message-id: 1454256948-10485-2-git-send-email-serge.fdrv@gmail.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- exec.c | 6 ++++++ include/qom/cpu.h | 8 ++++++-- qom/cpu.c | 9 +++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/exec.c b/exec.c index 7d67c11601..07598d0acd 100644 --- a/exec.c +++ b/exec.c @@ -2070,6 +2070,7 @@ static const MemoryRegionOps notdirty_mem_ops = { static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags) { CPUState *cpu = current_cpu; + CPUClass *cc = CPU_GET_CLASS(cpu); CPUArchState *env = cpu->env_ptr; target_ulong pc, cs_base; target_ulong vaddr; @@ -2095,6 +2096,11 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags) wp->hitaddr = vaddr; wp->hitattrs = attrs; if (!cpu->watchpoint_hit) { + if (wp->flags & BP_CPU && + !cc->debug_check_watchpoint(cpu, wp)) { + wp->flags &= ~BP_WATCHPOINT_HIT; + continue; + } cpu->watchpoint_hit = wp; tb_check_watchpoint(cpu); if (wp->flags & BP_STOP_BEFORE_ACCESS) { diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 035179c09c..ff54600e71 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -64,6 +64,7 @@ typedef uint64_t vaddr; #define CPU_GET_CLASS(obj) OBJECT_GET_CLASS(CPUClass, (obj), TYPE_CPU) typedef struct CPUState CPUState; +typedef struct CPUWatchpoint CPUWatchpoint; typedef void (*CPUUnassignedAccess)(CPUState *cpu, hwaddr addr, bool is_write, bool is_exec, int opaque, @@ -106,6 +107,8 @@ struct TranslationBlock; * a memory access with the specified memory transaction attributes. * @gdb_read_register: Callback for letting GDB read a register. * @gdb_write_register: Callback for letting GDB write a register. + * @debug_check_watchpoint: Callback: return true if the architectural + * watchpoint whose address has matched should really fire. * @debug_excp_handler: Callback for handling debug exceptions. * @write_elf64_note: Callback for writing a CPU-specific ELF note to a * 64-bit VM coredump. @@ -165,6 +168,7 @@ typedef struct CPUClass { int (*asidx_from_attrs)(CPUState *cpu, MemTxAttrs attrs); int (*gdb_read_register)(CPUState *cpu, uint8_t *buf, int reg); int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg); + bool (*debug_check_watchpoint)(CPUState *cpu, CPUWatchpoint *wp); void (*debug_excp_handler)(CPUState *cpu); int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu, @@ -207,14 +211,14 @@ typedef struct CPUBreakpoint { QTAILQ_ENTRY(CPUBreakpoint) entry; } CPUBreakpoint; -typedef struct CPUWatchpoint { +struct CPUWatchpoint { vaddr vaddr; vaddr len; vaddr hitaddr; MemTxAttrs hitattrs; int flags; /* BP_* */ QTAILQ_ENTRY(CPUWatchpoint) entry; -} CPUWatchpoint; +}; struct KVMState; struct kvm_run; diff --git a/qom/cpu.c b/qom/cpu.c index 38dc713ce2..aeb32f1b6e 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -189,6 +189,14 @@ static int cpu_common_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg) return 0; } +static bool cpu_common_debug_check_watchpoint(CPUState *cpu, CPUWatchpoint *wp) +{ + /* If no extra check is required, QEMU watchpoint match can be considered + * as an architectural match. + */ + return true; +} + bool target_words_bigendian(void); static bool cpu_common_virtio_is_big_endian(CPUState *cpu) { @@ -353,6 +361,7 @@ static void cpu_class_init(ObjectClass *klass, void *data) k->gdb_write_register = cpu_common_gdb_write_register; k->virtio_is_big_endian = cpu_common_virtio_is_big_endian; k->debug_excp_handler = cpu_common_noop; + k->debug_check_watchpoint = cpu_common_debug_check_watchpoint; k->cpu_exec_enter = cpu_common_noop; k->cpu_exec_exit = cpu_common_noop; k->cpu_exec_interrupt = cpu_common_exec_interrupt; From 3826121d9298cde1d29ead05910e1f40125ee9b0 Mon Sep 17 00:00:00 2001 From: Sergey Fedorov <serge.fdrv@gmail.com> Date: Thu, 11 Feb 2016 11:17:32 +0000 Subject: [PATCH 12/15] target-arm: Implement checking of fired watchpoint ARM stops before access to a location covered by watchpoint. Also, QEMU watchpoint fire is not necessarily an architectural watchpoint match. Unfortunately, that is hardly possible to ignore a fired watchpoint in debug exception handler. So move watchpoint check from debug exception handler to the dedicated watchpoint checking callback. Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1454256948-10485-3-git-send-email-serge.fdrv@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target-arm/cpu.c | 1 + target-arm/internals.h | 3 +++ target-arm/op_helper.c | 35 +++++++++++++++++++++-------------- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 7ddbf3d19b..f2393cd9f2 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -1483,6 +1483,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data) cc->gdb_arch_name = arm_gdb_arch_name; cc->gdb_stop_before_watchpoint = true; cc->debug_excp_handler = arm_debug_excp_handler; + cc->debug_check_watchpoint = arm_debug_check_watchpoint; cc->disas_set_info = arm_disas_set_info; diff --git a/target-arm/internals.h b/target-arm/internals.h index a648c1e7d5..70bec4a4d2 100644 --- a/target-arm/internals.h +++ b/target-arm/internals.h @@ -409,6 +409,9 @@ void hw_breakpoint_update(ARMCPU *cpu, int n); */ void hw_breakpoint_update_all(ARMCPU *cpu); +/* Callback function for checking if a watchpoint should trigger. */ +bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp); + /* Callback function for when a watchpoint or breakpoint triggers. */ void arm_debug_excp_handler(CPUState *cs); diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index 313c0f8bd7..bd48549826 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -976,6 +976,16 @@ void HELPER(check_breakpoints)(CPUARMState *env) } } +bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp) +{ + /* Called by core code when a CPU watchpoint fires; need to check if this + * is also an architectural watchpoint match. + */ + ARMCPU *cpu = ARM_CPU(cs); + + return check_watchpoints(cpu); +} + void arm_debug_excp_handler(CPUState *cs) { /* Called by core code when a watchpoint or breakpoint fires; @@ -987,23 +997,20 @@ void arm_debug_excp_handler(CPUState *cs) if (wp_hit) { if (wp_hit->flags & BP_CPU) { - cs->watchpoint_hit = NULL; - if (check_watchpoints(cpu)) { - bool wnr = (wp_hit->flags & BP_WATCHPOINT_HIT_WRITE) != 0; - bool same_el = arm_debug_target_el(env) == arm_current_el(env); + bool wnr = (wp_hit->flags & BP_WATCHPOINT_HIT_WRITE) != 0; + bool same_el = arm_debug_target_el(env) == arm_current_el(env); - if (extended_addresses_enabled(env)) { - env->exception.fsr = (1 << 9) | 0x22; - } else { - env->exception.fsr = 0x2; - } - env->exception.vaddress = wp_hit->hitaddr; - raise_exception(env, EXCP_DATA_ABORT, - syn_watchpoint(same_el, 0, wnr), - arm_debug_target_el(env)); + cs->watchpoint_hit = NULL; + + if (extended_addresses_enabled(env)) { + env->exception.fsr = (1 << 9) | 0x22; } else { - cpu_resume_from_signal(cs, NULL); + env->exception.fsr = 0x2; } + env->exception.vaddress = wp_hit->hitaddr; + raise_exception(env, EXCP_DATA_ABORT, + syn_watchpoint(same_el, 0, wnr), + arm_debug_target_el(env)); } } else { uint64_t pc = is_a64(env) ? env->pc : env->regs[15]; From 97f4ed3b71cca015e82bb601a17cd816b83fff05 Mon Sep 17 00:00:00 2001 From: Prasad J Pandit <pjp@fedoraproject.org> Date: Thu, 11 Feb 2016 11:17:32 +0000 Subject: [PATCH 13/15] sd: limit 'req.cmd' while using as an array index While processing standard SD commands, the 'req.cmd' value could lead to OOB read when used as an index into 'sd_cmd_type' or 'sd_cmd_class' arrays. Limit 'req.cmd' value to avoid such an access. Reported-by: Qinghao Tang <luodalongde@gmail.com> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1453315857-1352-1-git-send-email-ppandit@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/sd/sd.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 7580449f97..dd614b0890 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -669,8 +669,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, /* Not interpreting this as an app command */ sd->card_status &= ~APP_CMD; - if (sd_cmd_type[req.cmd] == sd_ac || sd_cmd_type[req.cmd] == sd_adtc) + if (sd_cmd_type[req.cmd & 0x3F] == sd_ac + || sd_cmd_type[req.cmd & 0x3F] == sd_adtc) { rca = req.arg >> 16; + } DPRINTF("CMD%d 0x%08x state %d\n", req.cmd, req.arg, sd->state); switch (req.cmd) { @@ -1341,7 +1343,8 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req) if (req->cmd == 16 || req->cmd == 55) { return 1; } - return sd_cmd_class[req->cmd] == 0 || sd_cmd_class[req->cmd] == 7; + return sd_cmd_class[req->cmd & 0x3F] == 0 + || sd_cmd_class[req->cmd & 0x3F] == 7; } int sd_do_command(SDState *sd, SDRequest *req, From 7ea686f5dda7cb9a5425fab716ce41260eeecf15 Mon Sep 17 00:00:00 2001 From: Andrew Jones <drjones@redhat.com> Date: Thu, 11 Feb 2016 11:17:32 +0000 Subject: [PATCH 14/15] hw/arm/virt: fix max-cpus check mach-virt doesn't yet support hotplug, but command lines specifying -smp <num>,maxcpus=<bigger-num> don't fail. Of course specifying bigger-num as something bigger than the machine supports, e.g. > 8 on a gicv2 machine, should fail though. This fix also makes mach- virt's max-cpus check truly consistent with the one in vl.c:main, as the one there was already correctly checking max-cpus instead of smp-cpus. Reported-by: Shannon Zhao <shannon.zhao@linaro.org> Signed-off-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Shannon Zhao <shannon.zhao@linaro.org> Message-id: 1454511578-24863-1-git-send-email-drjones@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 15658f49c4..44bbbea92b 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1013,7 +1013,7 @@ static void machvirt_init(MachineState *machine) MemoryRegion *sysmem = get_system_memory(); MemoryRegion *secure_sysmem = NULL; int gic_version = vms->gic_version; - int n, max_cpus; + int n, virt_max_cpus; MemoryRegion *ram = g_new(MemoryRegion, 1); const char *cpu_model = machine->cpu_model; VirtBoardInfo *vbi; @@ -1051,15 +1051,15 @@ static void machvirt_init(MachineState *machine) * many redistributors we can fit into the memory map. */ if (gic_version == 3) { - max_cpus = vbi->memmap[VIRT_GIC_REDIST].size / 0x20000; + virt_max_cpus = vbi->memmap[VIRT_GIC_REDIST].size / 0x20000; } else { - max_cpus = GIC_NCPU; + virt_max_cpus = GIC_NCPU; } - if (smp_cpus > max_cpus) { + if (max_cpus > virt_max_cpus) { error_report("Number of SMP CPUs requested (%d) exceeds max CPUs " "supported by machine 'mach-virt' (%d)", - smp_cpus, max_cpus); + max_cpus, virt_max_cpus); exit(1); } From f0afa73164778570083504a185d7498884c68d65 Mon Sep 17 00:00:00 2001 From: Stephen Warren <swarren@wwwdotorg.org> Date: Thu, 11 Feb 2016 11:17:32 +0000 Subject: [PATCH 15/15] bcm2835_property: implement "get board revision" query Return a valid value from the BCM2835 property mailbox query "get board revision". This query is used by U-Boot. Implementing it fixes the first obvious difference between qemu and real HW. The value returned is currently hard-coded to match the RPi2 I own. Other values are legal, e.g. different board manufacturer field values are likely to exist in the wild. Cc: Andrew Baumann <Andrew.Baumann@microsoft.com> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org> Reviewed-by: Andrew Baumann <Andrew.Baumann@microsoft.com> Message-id: 1454993910-24077-1-git-send-email-swarren@wwwdotorg.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/bcm2835_peripherals.c | 2 ++ hw/arm/bcm2836.c | 2 ++ hw/arm/raspi.c | 2 ++ hw/misc/bcm2835_property.c | 4 ++-- include/hw/misc/bcm2835_property.h | 1 + 5 files changed, 9 insertions(+), 2 deletions(-) diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c index 18b72ecb69..e4fb48b803 100644 --- a/hw/arm/bcm2835_peripherals.c +++ b/hw/arm/bcm2835_peripherals.c @@ -58,6 +58,8 @@ static void bcm2835_peripherals_init(Object *obj) /* Property channel */ object_initialize(&s->property, sizeof(s->property), TYPE_BCM2835_PROPERTY); object_property_add_child(obj, "property", OBJECT(&s->property), NULL); + object_property_add_alias(obj, "board-rev", OBJECT(&s->property), + "board-rev", &error_abort); qdev_set_parent_bus(DEVICE(&s->property), sysbus_get_default()); object_property_add_const_link(OBJECT(&s->property), "dma-mr", diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c index 69c7438317..8a4d13c7d9 100644 --- a/hw/arm/bcm2836.c +++ b/hw/arm/bcm2836.c @@ -39,6 +39,8 @@ static void bcm2836_init(Object *obj) TYPE_BCM2835_PERIPHERALS); object_property_add_child(obj, "peripherals", OBJECT(&s->peripherals), &error_abort); + object_property_add_alias(obj, "board-rev", OBJECT(&s->peripherals), + "board-rev", &error_abort); qdev_set_parent_bus(DEVICE(&s->peripherals), sysbus_get_default()); } diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c index 0c9427c40e..7d3d21ab32 100644 --- a/hw/arm/raspi.c +++ b/hw/arm/raspi.c @@ -128,6 +128,8 @@ static void raspi2_init(MachineState *machine) &error_abort); object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus", &error_abort); + object_property_set_int(OBJECT(&s->soc), 0xa21041, "board-rev", + &error_abort); object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_abort); setup_boot(machine, 2, machine->ram_size); diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c index e42b43e72d..45bd6c18ce 100644 --- a/hw/misc/bcm2835_property.c +++ b/hw/misc/bcm2835_property.c @@ -43,8 +43,7 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) resplen = 4; break; case 0x00010002: /* Get board revision */ - qemu_log_mask(LOG_UNIMP, - "bcm2835_property: %x get board revision NYI\n", tag); + stl_phys(&s->dma_as, value + 12, s->board_rev); resplen = 4; break; case 0x00010003: /* Get board MAC address */ @@ -258,6 +257,7 @@ static void bcm2835_property_realize(DeviceState *dev, Error **errp) } static Property bcm2835_property_props[] = { + DEFINE_PROP_UINT32("board-rev", BCM2835PropertyState, board_rev, 0), DEFINE_PROP_UINT32("ram-size", BCM2835PropertyState, ram_size, 0), DEFINE_PROP_END_OF_LIST() }; diff --git a/include/hw/misc/bcm2835_property.h b/include/hw/misc/bcm2835_property.h index fcf5f3deca..df889eaa08 100644 --- a/include/hw/misc/bcm2835_property.h +++ b/include/hw/misc/bcm2835_property.h @@ -23,6 +23,7 @@ typedef struct { MemoryRegion iomem; qemu_irq mbox_irq; MACAddr macaddr; + uint32_t board_rev; uint32_t ram_size; uint32_t addr; bool pending;