| Message ID | 1487616072-9226-4-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm: Fix M profile MSR/MRS | expand |
Peter Maydell <peter.maydell@linaro.org> writes: > 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 <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@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 0f8548f..9090356 100644 > --- a/target/arm/translate.c > +++ b/target/arm/translate.c > @@ -10498,6 +10498,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)) { > @@ -10525,6 +10533,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; -- Alex Bennée
diff --git a/target/arm/translate.c b/target/arm/translate.c index 0f8548f..9090356 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -10498,6 +10498,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)) { @@ -10525,6 +10533,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;
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 <peter.maydell@linaro.org> --- target/arm/translate.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) -- 2.7.4