refactor(embed/core): replace enable/disable_irq by irq_lock/unlock

[no changelog]

core/embed/bootloader/main.c

@@ -345,7 +345,7 @@ void real_jump_to_firmware(void) {
 __attribute__((noreturn)) void jump_to_fw_through_reset(void) {
   display_fade(display_backlight(-1), 0, 200);
 
-  disable_irq();
+  __disable_irq();
   delete_secrets();
   NVIC_SystemReset();
   for (;;)

core/embed/firmware/mpconfigport.h

@@ -194,8 +194,8 @@ typedef long mp_off_t;
 
 #include "irq.h"
 
-#define MICROPY_BEGIN_ATOMIC_SECTION()     disable_irq()
-#define MICROPY_END_ATOMIC_SECTION(state)  enable_irq(state)
+#define MICROPY_BEGIN_ATOMIC_SECTION()     irq_lock()
+#define MICROPY_END_ATOMIC_SECTION(state)  irq_unlock(state)
 #define MICROPY_EVENT_POLL_HOOK \
     do { \
         extern void mp_handle_pending(bool); \

core/embed/trezorhal/stm32f4/bootutils.c

@@ -72,7 +72,7 @@ void reboot_to_bootloader(void) {
 #ifdef STM32U5
   // extern uint32_t g_boot_command;
   g_boot_command = boot_command;
-  disable_irq();
+  __disable_irq();
   delete_secrets();
   NVIC_SystemReset();
 #else

core/embed/trezorhal/stm32f4/i2c_bus.c

@@ -328,9 +328,9 @@ void i2c_bus_close(i2c_bus_t* bus) {
 }
 
 i2c_status_t i2c_packet_status(i2c_packet_t* packet) {
-  uint32_t irq_state = disable_irq();
+  irq_key_t irq_key = irq_lock();
   i2c_status_t status = packet->status;
-  enable_irq(irq_state);
+  irq_unlock(irq_key);
   return status;
 }
 
@@ -421,14 +421,14 @@ i2c_status_t i2c_bus_submit(i2c_bus_t* bus, i2c_packet_t* packet) {
   packet->status = I2C_STATUS_PENDING;
 
   // Insert packet into the queue
-  uint32_t irq_state = disable_irq();
+  irq_key_t irq_key = irq_lock();
   if (i2c_bus_add_packet(bus, packet)) {
     // The queue was empty, start the operation
     if (!bus->callback_executed && !bus->abort_pending) {
       i2c_bus_head_continue(bus);
     }
   }
-  enable_irq(irq_state);
+  irq_unlock(irq_key);
 
   return I2C_STATUS_OK;
 }
@@ -439,7 +439,7 @@ void i2c_bus_abort(i2c_bus_t* bus, i2c_packet_t* packet) {
     return;
   }
 
-  uint32_t irq_state = disable_irq();
+  irq_key_t irq_key = irq_lock();
 
   if (packet->status == I2C_STATUS_PENDING) {
     if (i2c_bus_remove_packet(bus, packet) && bus->next_op > 0) {
@@ -463,7 +463,7 @@ void i2c_bus_abort(i2c_bus_t* bus, i2c_packet_t* packet) {
     packet->status = I2C_STATUS_ABORTED;
   }
 
-  enable_irq(irq_state);
+  irq_unlock(irq_key);
 }
 
 // Completes the current packet by removing it from the queue

core/embed/trezorhal/stm32f4/irq.h

@@ -36,22 +36,56 @@ extern uint32_t irq_stats[IRQ_STATS_MAX];
 #define IRQ_EXIT(irq)
 #endif
 
+typedef uint32_t irq_key_t;
+
 // Checks if interrupts are enabled
-#define IS_IRQ_ENABLED(state) (((state) & 1) == 0)
+#define IS_IRQ_ENABLED(key) (((key) & 1) == 0)
 
 // Get the current value of the CPU's exception mask register.
 // The least significant bit indicates if interrupts are enabled or disabled.
-static inline uint32_t query_irq(void) { return __get_PRIMASK(); }
+static inline irq_key_t query_irq(void) { return __get_PRIMASK(); }
 
-// Restore the CPU's exception mask register to a previous state
-static inline void enable_irq(uint32_t state) { __set_PRIMASK(state); }
+// Disables interrupts and returns the previous interrupt state.
+//
+// This function is used to create critical sections by disabling interrupts
+// on a Cortex-M platform. It returns the current state of the PRIMASK register,
+// which controls the global interrupt enable/disable state.
+//
+// Important:
+// - The `"memory"` clobber is included to prevent the compiler from reordering
+//   memory operations across this function, ensuring that all memory accesses
+//   efore `irq_lock()` are completed before interrupts are disabled.
+// - The order of operations on non-volatile variables relative to this
+//   function is not guaranteed without memory barriers or other
+//   synchronization mechanisms.
+// - When using Link-Time Optimization (LTO), ensure that the behavior of these
+//   functions is thoroughly tested, as LTO can lead to more aggressive
+//   optimizations. While GCC typically respects the order of `volatile`
+//   operations, this is not guaranteed by the C standard.
+static inline irq_key_t irq_lock(void) {
+  uint32_t key;
+  __asm volatile(
+      "MRS %0, PRIMASK\n"
+      "CPSID i"
+      : "=r"(key)
+      :
+      : "memory"  // Clobber memory to ensure correct memory operations
+  );
+  return key;
+}
 
-// Disable all interrupts and return the current state of the
-// CPU's exception mask register
-static inline uint32_t disable_irq(void) {
-  uint32_t state = __get_PRIMASK();
-  __disable_irq();
-  return state;
+// Restores the interrupt state to what it was before `irq_lock`.
+//
+// This function re-enables interrupts based on the PRIMASK state passed to it.
+// It should be used in conjunction with `irq_lock` to restore the previous
+// interrupt state after a critical section.
+static inline void irq_unlock(irq_key_t key) {
+  __asm volatile(
+      "MSR PRIMASK, %0\n"
+      :
+      : "r"(key)
+      : "memory"  // Clobber memory to ensure correct memory operations
+  );
 }
 
 // IRQ priority levels used throughout the system

core/embed/trezorhal/stm32f4/sdcard.c

@@ -253,13 +253,13 @@ static HAL_StatusTypeDef sdcard_wait_finished(SD_HandleTypeDef *sd,
   uint32_t start = HAL_GetTick();
   for (;;) {
     // Do an atomic check of the state; WFI will exit even if IRQs are disabled
-    uint32_t irq_state = disable_irq();
+    irq_key_t irq_key = irq_lock();
     if (sd->State != HAL_SD_STATE_BUSY) {
-      enable_irq(irq_state);
+      irq_unlock(irq_key);
       break;
     }
     __WFI();
-    enable_irq(irq_state);
+    irq_unlock(irq_key);
     if (HAL_GetTick() - start >= timeout) {
       return HAL_TIMEOUT;
     }

core/embed/trezorhal/stm32f4/systick.c

@@ -106,7 +106,7 @@ void systick_update_freq(void) {
 uint64_t systick_cycles(void) {
   systick_driver_t* drv = &g_systick_driver;
 
-  uint32_t irq_state = disable_irq();
+  irq_key_t irq_key = irq_lock();
 
   // Current value of the SysTick counter
   uint32_t val = SysTick->VAL;
@@ -125,7 +125,7 @@ uint64_t systick_cycles(void) {
 
   uint64_t cycles = drv->cycles + ((val > 0) ? (drv->cycles_per_ms - val) : 0);
 
-  enable_irq(irq_state);
+  irq_unlock(irq_key);
 
   return cycles;
 }

core/embed/trezorhal/stm32f4/systimer.c

@@ -96,7 +96,7 @@ systimer_t* systimer_create(systimer_callback_t callback, void* context) {
     return NULL;
   }
 
-  uint32_t irq_state = disable_irq();
+  irq_key_t irq_key = irq_lock();
 
   // Find a free timer entry
   for (int i = 0; i < MAX_SYSTIMERS; i++) {
@@ -108,13 +108,13 @@ systimer_t* systimer_create(systimer_callback_t callback, void* context) {
       timer->context = context;
       timer->callback = callback;
 
-      enable_irq(irq_state);
+      irq_unlock(irq_key);
       return timer;
     }
   }
 
   // No free timer entry found
-  enable_irq(irq_state);
+  irq_unlock(irq_key);
   return NULL;
 }
 
@@ -137,11 +137,11 @@ void systimer_set(systimer_t* timer, uint32_t delay_ms) {
   uint64_t delay = systick_us_to_cycles((uint64_t)delay_ms * 1000);
   uint64_t expiration = systick_cycles() + delay;
 
-  uint32_t irq_state = disable_irq();
+  irq_key_t irq_key = irq_lock();
   timer->expiration = expiration;
   timer->period = 0;
   timer->scheduled = true;
-  enable_irq(irq_state);
+  irq_unlock(irq_key);
 }
 
 void systimer_set_periodic(systimer_t* timer, uint32_t period_ms) {
@@ -154,11 +154,11 @@ void systimer_set_periodic(systimer_t* timer, uint32_t period_ms) {
   uint64_t period = systick_us_to_cycles((uint64_t)period_ms * 1000);
   uint64_t expiration = systick_cycles() + period;
 
-  uint32_t irq_state = disable_irq();
+  irq_key_t irq_key = irq_lock();
   timer->expiration = expiration;
   timer->period = period;
   timer->scheduled = true;
-  enable_irq(irq_state);
+  irq_unlock(irq_key);
 }
 
 bool systimer_unset(systimer_t* timer) {
@@ -168,10 +168,10 @@ bool systimer_unset(systimer_t* timer) {
     return false;
   }
 
-  uint32_t irq_state = disable_irq();
+  irq_key_t irq_key = irq_lock();
   bool was_scheduled = timer->scheduled;
   timer->scheduled = false;
-  enable_irq(irq_state);
+  irq_unlock(irq_key);
   return was_scheduled;
 }
 
@@ -182,10 +182,10 @@ systimer_key_t systimer_suspend(systimer_t* timer) {
     return false;
   }
 
-  uint32_t irq_state = disable_irq();
+  irq_key_t irq_key = irq_lock();
   bool was_suspended = timer->suspended;
   timer->suspended = true;
-  enable_irq(irq_state);
+  irq_unlock(irq_key);
   return was_suspended;
 }
 

core/embed/trezorhal/stm32u5/i2c_bus.c

@@ -360,9 +360,9 @@ void i2c_bus_close(i2c_bus_t* bus) {
 }
 
 i2c_status_t i2c_packet_status(i2c_packet_t* packet) {
-  uint32_t irq_state = disable_irq();
+  irq_key_t irq_key = irq_lock();
   i2c_status_t status = packet->status;
-  enable_irq(irq_state);
+  irq_unlock(irq_key);
   return status;
 }
 
@@ -499,14 +499,14 @@ i2c_status_t i2c_bus_submit(i2c_bus_t* bus, i2c_packet_t* packet) {
   packet->status = I2C_STATUS_PENDING;
 
   // Insert packet into the queue
-  uint32_t irq_state = disable_irq();
+  irq_key_t irq_key = irq_lock();
   if (i2c_bus_add_packet(bus, packet)) {
     // The queue was empty, start the operation
     if (!bus->callback_executed && !bus->abort_pending) {
       i2c_bus_head_continue(bus);
     }
   }
-  enable_irq(irq_state);
+  irq_unlock(irq_key);
 
   return I2C_STATUS_OK;
 }
@@ -517,7 +517,7 @@ void i2c_bus_abort(i2c_bus_t* bus, i2c_packet_t* packet) {
     return;
   }
 
-  uint32_t irq_state = disable_irq();
+  irq_key_t irq_key = irq_lock();
 
   if (packet->status == I2C_STATUS_PENDING) {
     if (i2c_bus_remove_packet(bus, packet) && bus->next_op > 0) {
@@ -546,7 +546,7 @@ void i2c_bus_abort(i2c_bus_t* bus, i2c_packet_t* packet) {
     packet->status = I2C_STATUS_ABORTED;
   }
 
-  enable_irq(irq_state);
+  irq_unlock(irq_key);
 }
 
 // Completes the current packet by removing it from the queue

core/embed/trezorhal/stm32u5/sdcard.c

@@ -256,13 +256,13 @@ static HAL_StatusTypeDef sdcard_wait_finished(SD_HandleTypeDef *sd,
   uint32_t start = HAL_GetTick();
   for (;;) {
     // Do an atomic check of the state; WFI will exit even if IRQs are disabled
-    uint32_t irq_state = disable_irq();
+    irq_key_t irq_key = irq_lock();
     if (sd->State != HAL_SD_STATE_BUSY) {
-      enable_irq(irq_state);
+      irq_unlock(irq_key);
       break;
     }
     __WFI();
-    enable_irq(irq_state);
+    irq_unlock(irq_key);
     if (HAL_GetTick() - start >= timeout) {
       return HAL_TIMEOUT;
     }