On 2015-08-28 17:18, Martin Galvan wrote:
On Thu, Aug 27, 2015 at 3:53 PM, sudarshan.rajagopalan
<sudarshan.rajagopalan at vecna.com> wrote:
The instruction "cmn r2, #3\n" in line 31 basically compares the Link
Register(lr) to value 0xFFFFFFFD (negative #3, because CMN negates the RHS and compares with LHS) and chooses MSP or PSP in the following IT block. This is pretty cool! But, it will not work if you have the floating-point unit (FPU) enabled in your processor, which is enabled in mine. With FPU enabled, the error code returned is either 0xFFFFFFE9 or 0xFFFFFFED, for
which the above assembly instruction will not work out and MSP will be
selected always.

Better way to do is to check the 2nd bit of the error code to determine which stack pointer was used before the exception happened - "tst lr, #4\n" and change the IT block from "itt ne" to "itt eq" and the "mov" and "add"
within this IT block.

Hi Sudarshan! First of all, great catch. It looks like there's indeed a bug
on certain conditions.

While your method is indeed way less convoluted than the clever "compare to -3"
the old code did, I think we can actually make it even simpler
(and save a couple of instructions) by doing the following:

    "tst lr, #4\n" /* Check if bit 2 of LR is zero */
    "itte eq\n" /* IF bit 2 of LR is zero... */
    "mrseq r0, msp\n" /* THEN we were using MSP */
"addeq r0, %[cpufsz]\n" /* THEN revert the previous 'sub sp, %[cpufsz]' */
    "mrsne r0, psp\n" /* ELSE it's not zero; we were using PSP */

What do you think?

Hi Martin,

Yup, we could definitely save 2 cycles and make it look simpler and understandable. Thumps up for the comments explaining LR decoding!

Thanks and Regards,
Sudarshan

-----
Sudarshan Rajagopalan
sudarshan.rajagopalan at vecna.com
www.vecna.com


Also, I must admit it took me quite a while to understand what was
going on there.
If you don't mind, I'd also like to add a brief comment explaining how
to decode LR
after taking an exception. The patch would end up looking like this:

---
cpukit/score/cpu/arm/armv7m-exception-default.c | 27 +++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/cpukit/score/cpu/arm/armv7m-exception-default.c
b/cpukit/score/cpu/arm/armv7m-exception-default.c
index e890cdf..d04dbea 100644
--- a/cpukit/score/cpu/arm/armv7m-exception-default.c
+++ b/cpukit/score/cpu/arm/armv7m-exception-default.c
@@ -22,16 +22,27 @@

 void __attribute__((naked)) _ARMV7M_Exception_default( void )
 {
+ /* On exception entry, ARMv7M saves context state onto a stack pointed to + * by either MSP or PSP. The value stored in LR indicates whether we were + * in Thread or Handler mode, whether we were using the FPU (if any),
+     * and which stack pointer we were using.
+     * In particular, bit 2 of LR will be 0 if we were using MSP.
+     *
+ * For a more detailed explanation, see the Exception Entry Behavior
+     * section of the ARMv7M Architecture Reference Manual.
+     */
+
+    /* As we're in Handler mode here, we'll always operate on MSP.
+ * However, we need to store the right SP in our CPU_Exception_frame.
+     */
   __asm__ volatile (
-    "sub sp, %[cpufsz]\n"
+ "sub sp, %[cpufsz]\n" /* Allocate space for a CPU_Exception_frame. */
     "stm sp, {r0-r12}\n"
-    "mov r2, lr\n"
-    "mrs r1, msp\n"
-    "mrs r0, psp\n"
-    "cmn r2, #3\n"
-    "itt ne\n"
-    "movne r0, r1\n"
-    "addne r0, %[cpufsz]\n"
+ "tst lr, #4\n" /* Check if bit 2 of LR is zero. (If so, PSR.Z = 1) */
+    "itte eq\n" /* IF bit 2 of LR is zero... (PSR.Z == 1) */
+    "mrseq r0, msp\n" /* THEN we were using MSP. */
+    "addeq r0, %[cpufsz]\n" /* THEN, set r0 = old MSP. */
+    "mrsne r0, psp\n" /* ELSE it's not zero; we were using PSP. */
     "add r2, r0, %[v7mlroff]\n"
     "add r1, sp, %[cpulroff]\n"
     "ldm r2, {r3-r5}\n"

_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Reply via email to