| Message ID | 1506092407-26985-9-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ARM v8M: exception entry, exit and security | expand |
On 09/22/2017 11:59 AM, Peter Maydell wrote: > In the v8M architecture, return from an exception to a PC which > has bit 0 set is not UNPREDICTABLE; it is defined that bit 0 > is discarded [R_HRJH]. Restrict our complaint about this to v7M. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > target/arm/helper.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 979129e..59a07d2 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -6421,16 +6421,22 @@ static void do_v7m_exception_exit(ARMCPU *cpu) > env->regs[12] = ldl_phys(cs->as, frameptr + 0x10); > env->regs[14] = ldl_phys(cs->as, frameptr + 0x14); > env->regs[15] = ldl_phys(cs->as, frameptr + 0x18); > - if (env->regs[15] & 1) { > + > + /* Returning from an exception with a PC with bit 0 set is defined > + * behaviour on v8M (bit 0 is ignored), but for v7M it was specified > + * to be UNPREDICTABLE. In practice actual v7M hardware seems to ignore > + * the lsbit, and there are several RTOSes out there which incorrectly > + * assume the r15 in the stack frame should be a Thumb-style "lsbit > + * indicates ARM/Thumb" value, so ignore the bit on v7M as well, but > + * complain about the badly behaved guest. > + */ > + if ((env->regs[15] & 1) && !arm_feature(env, ARM_FEATURE_V8)) { > qemu_log_mask(LOG_GUEST_ERROR, > "M profile return from interrupt with misaligned " > - "PC is UNPREDICTABLE\n"); > - /* Actual hardware seems to ignore the lsbit, and there are several > - * RTOSes out there which incorrectly assume the r15 in the stack > - * frame should be a Thumb-style "lsbit indicates ARM/Thumb" value. > - */ > - env->regs[15] &= ~1U; > + "PC is UNPREDICTABLE on v7M\n"); > } > + env->regs[15] &= ~1U; > + > xpsr = ldl_phys(cs->as, frameptr + 0x1c); > > if (arm_feature(env, ARM_FEATURE_V8)) { >
On 09/22/2017 10:59 AM, Peter Maydell wrote: > + if ((env->regs[15] & 1) && !arm_feature(env, ARM_FEATURE_V8)) { > qemu_log_mask(LOG_GUEST_ERROR, > "M profile return from interrupt with misaligned " > - "PC is UNPREDICTABLE\n"); > - /* Actual hardware seems to ignore the lsbit, and there are several > - * RTOSes out there which incorrectly assume the r15 in the stack > - * frame should be a Thumb-style "lsbit indicates ARM/Thumb" value. > - */ > - env->regs[15] &= ~1U; > + "PC is UNPREDICTABLE on v7M\n"); > } > + env->regs[15] &= ~1U; If you're going to always test regs[15] & 1, you might as well use that and avoid an unlikely(?) writeback. if (env->regs[15] & 1) { if (!arm_feature(env, ARM_FEATURE_V8)) { qemu_log_mask(...); } env->regs[15] &= ~1U; } Otherwise, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 10/05/2017 12:32 PM, Richard Henderson wrote: > if (env->regs[15] & 1) { > if (!arm_feature(env, ARM_FEATURE_V8)) { > qemu_log_mask(...); > } > env->regs[15] &= ~1U; > } Bah. Even better to move the bit clear statement before the feature check. The two loads from env->regs[15] will no longer be separated by a function call and therefore CSEd by the compiler. r~
diff --git a/target/arm/helper.c b/target/arm/helper.c index 979129e..59a07d2 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6421,16 +6421,22 @@ static void do_v7m_exception_exit(ARMCPU *cpu) env->regs[12] = ldl_phys(cs->as, frameptr + 0x10); env->regs[14] = ldl_phys(cs->as, frameptr + 0x14); env->regs[15] = ldl_phys(cs->as, frameptr + 0x18); - if (env->regs[15] & 1) { + + /* Returning from an exception with a PC with bit 0 set is defined + * behaviour on v8M (bit 0 is ignored), but for v7M it was specified + * to be UNPREDICTABLE. In practice actual v7M hardware seems to ignore + * the lsbit, and there are several RTOSes out there which incorrectly + * assume the r15 in the stack frame should be a Thumb-style "lsbit + * indicates ARM/Thumb" value, so ignore the bit on v7M as well, but + * complain about the badly behaved guest. + */ + if ((env->regs[15] & 1) && !arm_feature(env, ARM_FEATURE_V8)) { qemu_log_mask(LOG_GUEST_ERROR, "M profile return from interrupt with misaligned " - "PC is UNPREDICTABLE\n"); - /* Actual hardware seems to ignore the lsbit, and there are several - * RTOSes out there which incorrectly assume the r15 in the stack - * frame should be a Thumb-style "lsbit indicates ARM/Thumb" value. - */ - env->regs[15] &= ~1U; + "PC is UNPREDICTABLE on v7M\n"); } + env->regs[15] &= ~1U; + xpsr = ldl_phys(cs->as, frameptr + 0x1c); if (arm_feature(env, ARM_FEATURE_V8)) {
In the v8M architecture, return from an exception to a PC which has bit 0 set is not UNPREDICTABLE; it is defined that bit 0 is discarded [R_HRJH]. Restrict our complaint about this to v7M. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) -- 2.7.4