From 001b3cab51ebfcb13e8dd03ea25bfa3bd0c517a3 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 20 Mar 2017 12:41:44 +0000 Subject: [PATCH 1/4] arm: HVC and SMC encodings don't exist for M profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M profile doesn't have the HVC or SMC encodings, so make them always UNDEF rather than generating calls to helper functions that assume A/R profile. Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Message-id: 1487616072-9226-2-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/target/arm/translate.c b/target/arm/translate.c index b859f10755..216852b673 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -10377,6 +10377,9 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw goto illegal_op; if (insn & (1 << 26)) { + if (arm_dc_feature(s, ARM_FEATURE_M)) { + goto illegal_op; + } if (!(insn & (1 << 20))) { /* Hypervisor call (v7) */ int imm16 = extract32(insn, 16, 4) << 12 From 43ac65742319ef5ac4461daf43316b189cd21e89 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 20 Mar 2017 12:41:44 +0000 Subject: [PATCH 2/4] arm: Don't decode MRS(banked) or MSR(banked) for M profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M profile doesn't have the MSR(banked) and MRS(banked) instructions and uses the encodings for different kinds of M-profile MRS/MSR. Guard the relevant bits of the decode logic to make sure we don't accidentally fall into them by accident on M-profile. (The bit being checked for this (bit 5) is part of the SYSm field on M-profile, but since no currently allocated system registers have encodings with bit 5 of SYSm set, this hasn't been a problem in practice.) Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Message-id: 1487616072-9226-3-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index 216852b673..a5f5a28ba4 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -10500,7 +10500,8 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw gen_exception_return(s, tmp); break; case 6: /* MRS */ - if (extract32(insn, 5, 1)) { + if (extract32(insn, 5, 1) && + !arm_dc_feature(s, ARM_FEATURE_M)) { /* MRS (banked) */ int sysm = extract32(insn, 16, 4) | (extract32(insn, 4, 1) << 4); @@ -10521,7 +10522,8 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw store_reg(s, rd, tmp); break; case 7: /* MRS */ - if (extract32(insn, 5, 1)) { + if (extract32(insn, 5, 1) && + !arm_dc_feature(s, ARM_FEATURE_M)) { /* MRS (banked) */ int sysm = extract32(insn, 16, 4) | (extract32(insn, 4, 1) << 4); From 3d54026fb06d1aea7ebb4e9825970b06bebcacac Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 20 Mar 2017 12:41:44 +0000 Subject: [PATCH 3/4] arm: Enforce should-be-1 bits in MRS decoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MRS instruction requires that bits [19..16] are all 1s, and for A/R profile also that bits [7..0] are all 0s. At this point in the decode tree we have checked all of the rest of the instruction but were allowing these to be any value. If these bits are not set then the result is architecturally UNPREDICTABLE, but choosing to UNDEF is more helpful to the user and avoids unexpected odd behaviour if the encodings are used for some purpose in future architecture versions. Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Message-id: 1487616072-9226-4-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/target/arm/translate.c b/target/arm/translate.c index a5f5a28ba4..c4acff5f99 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -10510,6 +10510,14 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw break; } + if (extract32(insn, 16, 4) != 0xf) { + goto illegal_op; + } + if (!arm_dc_feature(s, ARM_FEATURE_M) && + extract32(insn, 0, 8) != 0) { + goto illegal_op; + } + /* mrs cpsr */ tmp = tcg_temp_new_i32(); if (arm_dc_feature(s, ARM_FEATURE_M)) { @@ -10537,6 +10545,12 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw if (IS_USER(s) || arm_dc_feature(s, ARM_FEATURE_M)) { goto illegal_op; } + + if (extract32(insn, 16, 4) != 0xf || + extract32(insn, 0, 8) != 0) { + goto illegal_op; + } + tmp = load_cpu_field(spsr); store_reg(s, rd, tmp); break; From b28b3377d7e9ba35611d454d5a63ef50cab1f8c5 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 20 Mar 2017 12:41:44 +0000 Subject: [PATCH 4/4] arm: Fix APSR writes via M profile MSR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our implementation of writes to the APSR for M-profile via the MSR instruction was badly broken. First and worst, we had the sense wrong on the test of bit 2 of the SYSm field -- this is supposed to request an APSR write if bit 2 is 0 but we were doing it if bit 2 was 1. This bug was introduced in commit 58117c9bb429cd, so hasn't been in a QEMU release. Secondly, the choice of exactly which parts of APSR should be written is defined by bits in the 'mask' field. We were not passing these through from instruction decode, making it impossible to check them in the helper. Pass the mask bits through from the instruction decode to the helper function and process them appropriately; fix the wrong sense of the SYSm bit 2 check. Invalid mask values and invalid combinations of mask and register number are UNPREDICTABLE; we choose to treat them as if the mask values were valid. Signed-off-by: Peter Maydell Message-id: 1487616072-9226-5-git-send-email-peter.maydell@linaro.org Reviewed-by: Alex Bennée --- target/arm/helper.c | 24 +++++++++++++++++++++--- target/arm/translate.c | 3 ++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index 8646a7a119..8cb7a9451c 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -8548,8 +8548,18 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) } } -void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val) +void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) { + /* We're passed bits [11..0] of the instruction; extract + * SYSm and the mask bits. + * Invalid combinations of SYSm and mask are UNPREDICTABLE; + * we choose to treat them as if the mask bits were valid. + * NB that the pseudocode 'mask' variable is bits [11..10], + * whereas ours is [11..8]. + */ + uint32_t mask = extract32(maskreg, 8, 4); + uint32_t reg = extract32(maskreg, 0, 8); + if (arm_current_el(env) == 0 && reg > 7) { /* only xPSR sub-fields may be written by unprivileged */ return; @@ -8558,8 +8568,16 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val) switch (reg) { case 0 ... 7: /* xPSR sub-fields */ /* only APSR is actually writable */ - if (reg & 4) { - xpsr_write(env, val, 0xf8000000); /* APSR */ + if (!(reg & 4)) { + uint32_t apsrmask = 0; + + if (mask & 8) { + apsrmask |= 0xf8000000; /* APSR NZCVQ */ + } + if ((mask & 4) && arm_feature(env, ARM_FEATURE_THUMB_DSP)) { + apsrmask |= 0x000f0000; /* APSR GE[3:0] */ + } + xpsr_write(env, val, apsrmask); } break; case 8: /* MSP */ diff --git a/target/arm/translate.c b/target/arm/translate.c index c4acff5f99..e32e38cadd 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -10403,7 +10403,8 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw case 0: /* msr cpsr. */ if (arm_dc_feature(s, ARM_FEATURE_M)) { tmp = load_reg(s, rn); - addr = tcg_const_i32(insn & 0xff); + /* the constant is the mask and SYSm fields */ + addr = tcg_const_i32(insn & 0xfff); gen_helper_v7m_msr(cpu_env, addr, tmp); tcg_temp_free_i32(addr); tcg_temp_free_i32(tmp);