|
| 1 | +From 74dc54a4de3347f8048866c635fd0c1cc0740ed4 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Peter Maydell < [email protected]> |
| 3 | +Date: Thu, 20 May 2021 14:09:05 +0100 |
| 4 | +Subject: [PATCH 17/17] target/arm: Use correct SP in M-profile exception |
| 5 | + return |
| 6 | + |
| 7 | +When an M-profile CPU is restoring registers from the stack on |
| 8 | +exception return, the stack pointer to use is determined based on |
| 9 | +bits in the magic exception return type value. We were not getting |
| 10 | +this logic entirely correct. |
| 11 | + |
| 12 | +Whether we use one of the Secure stack pointers or one of the |
| 13 | +Non-Secure stack pointers depends on the EXCRET.S bit. However, |
| 14 | +whether we use the MSP or the PSP then depends on the SPSEL bit in |
| 15 | +either the CONTROL_S or CONTROL_NS register. We were incorrectly |
| 16 | +selecting MSP vs PSP based on the EXCRET.SPSEL bit. |
| 17 | + |
| 18 | +(In the pseudocode this is in the PopStack() function, which calls |
| 19 | +LookUpSp_with_security_mode() which in turn looks at the relevant |
| 20 | +CONTROL.SPSEL bit.) |
| 21 | + |
| 22 | +The buggy behaviour wasn't noticeable in most cases, because we write |
| 23 | +EXCRET.SPSEL to the CONTROL.SPSEL bit for the S/NS register selected |
| 24 | +by EXCRET.ES, so we only do the wrong thing when EXCRET.S and |
| 25 | +EXCRET.ES are different. This will happen when secure code takes a |
| 26 | +secure exception, which then tail-chains to a non-secure exception |
| 27 | +which finally returns to the original secure code. |
| 28 | + |
| 29 | +Signed-off-by: Peter Maydell < [email protected]> |
| 30 | +Reviewed-by: Richard Henderson < [email protected]> |
| 31 | + |
| 32 | +--- |
| 33 | + target/arm/m_helper.c | 3 ++- |
| 34 | + 1 file changed, 2 insertions(+), 1 deletion(-) |
| 35 | + |
| 36 | +diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c |
| 37 | +index d63ae465e1..eda74e5545 100644 |
| 38 | +--- a/target/arm/m_helper.c |
| 39 | ++++ b/target/arm/m_helper.c |
| 40 | +@@ -1597,10 +1597,11 @@ static void do_v7m_exception_exit(ARMCPU *cpu) |
| 41 | + * We use this limited C variable scope so we don't accidentally |
| 42 | + * use 'frame_sp_p' after we do something that makes it invalid. |
| 43 | + */ |
| 44 | ++ bool spsel = env->v7m.control[return_to_secure] & R_V7M_CONTROL_SPSEL_MASK; |
| 45 | + uint32_t *frame_sp_p = get_v7m_sp_ptr(env, |
| 46 | + return_to_secure, |
| 47 | + !return_to_handler, |
| 48 | +- return_to_sp_process); |
| 49 | ++ spsel); |
| 50 | + uint32_t frameptr = *frame_sp_p; |
| 51 | + bool pop_ok = true; |
| 52 | + ARMMMUIdx mmu_idx; |
| 53 | +-- |
| 54 | +2.31.1 |
| 55 | + |
0 commit comments