arm: fix Cortex-M IRQ masking

commit 5442622d883c231e7ce264b1d04018d43d9be7ef
Author: Aidan MacDonald <[email protected]>
Date:   Tue Jan 20 22:24:40 2026 +0000

    arm: fix Cortex-M IRQ masking
    
    The following inline assembly in set_irq_level() turned out
    to have incorrect constraints:
    
        int newvalue = /* input parameter */;
        int oldvalue;
    
        asm volatile ("mrs %0, primask\n"
                      "msr primask, %1\n"
                      : "=r"(oldvalue) : "r"(newvalue));
    
    leading to incorrect code generation for common cases like
    disable_irq_save(), which compiles to:
    
        mov r5, #1
        mrs r5, primask
        msr primask, r5
    
    ...which doesn't disable IRQs at all, since both of the
    operands got assigned to the same register; the write of
    'oldvalue' clobbers the 'newvalue' input before it's used.
    
    Apparently GCC assumes that input operands are read before
    output operands are written. One way to fix this is adding
    the '&' constraint: "=&r"(oldvalue), but it's better to
    break things down into separate, simpler asm statements
    which GCC can figure out itself.
    
    Also add compiler memory barriers where primask is modified
    to ensure loads/stores aren't incorrectly moved outside of
    critical sections.
    
    While here, optimize disable_irq_save() a bit by using the
    cpsid instruction, which avoids the extra "mov" and register
    allocation needed by "msr primask".
    
    Change-Id: Iac94a76db5bac399a1cf028da4241a0473259a46

diff --git a/firmware/target/arm/system-arm-micro.h b/firmware/target/arm/system-arm-micro.h
index a414a30a3e..a364a882fe 100644
--- a/firmware/target/arm/system-arm-micro.h
+++ b/firmware/target/arm/system-arm-micro.h
@@ -26,9 +26,6 @@
 #define IRQ_STATUS          0x01
 #define HIGHEST_IRQ_LEVEL   IRQ_DISABLED
 
-#define disable_irq_save() \
-    set_irq_level(IRQ_DISABLED)
-
 /* For compatibility with ARM classic */
 #define CPU_MODE_THREAD_CONTEXT 0
 
@@ -47,39 +44,51 @@
                   __func__, __mproc, __massert); })
 
 /* Core-level interrupt masking */
-
-static inline int set_irq_level(int primask)
+static inline void enable_irq(void)
 {
-    int oldvalue;
-
-    asm volatile ("mrs %0, primask\n"
-                  "msr primask, %1\n"
-                  : "=r"(oldvalue) : "r"(primask));
+    asm volatile ("cpsie i" ::: "memory");
+}
 
-    return oldvalue;
+static inline void disable_irq(void)
+{
+    asm volatile ("cpsid i" ::: "memory");
 }
 
 static inline void restore_irq(int primask)
 {
-    asm volatile ("msr primask, %0" :: "r"(primask));
+    asm volatile ("msr primask, %0" :: "r"(primask) : "memory");
 }
 
-static inline void enable_irq(void)
+static inline int get_irq_level(void)
 {
-    asm volatile ("cpsie i");
+    int primask;
+
+    asm volatile("mrs %0, primask" : "=r"(primask));
+
+    return primask;
 }
 
-static inline void disable_irq(void)
+static inline int disable_irq_save(void)
 {
-    asm volatile ("cpsid i");
+    int oldlevel = get_irq_level();
+
+    disable_irq();
+
+    return oldlevel;
 }
 
-static inline bool irq_enabled(void)
+static inline int set_irq_level(int primask)
 {
-    int primask;
-    asm volatile ("mrs %0, primask" : "=r"(primask));
+    int oldvalue = get_irq_level();
 
-    return !(primask & 1);
+    restore_irq(primask);
+
+    return oldvalue;
+}
+
+static inline bool irq_enabled(void)
+{
+    return get_irq_level() == IRQ_ENABLED;
 }
 
 static inline unsigned long get_interrupt_number(void)
-- 
rockbox-cvs mailing list
[email protected]
https://lists.haxx.se/mailman/listinfo/rockbox-cvs