[nrf fromtree] drivers: nrf_qspi_nor: Fix EXTXIP + Flash write race condition

When parts of code are running from EXTXIP and at the same
time QSPI external flash writes/erases are happenning there
is a high risk of a race condition where a thread running
EXTXIP code starts executing while an external flash operation
is in progress; This can lead to execution stalling or
bus faults/usage faults.

The issue was observed when parts of MCUMGR were relocated to
EXTXIP and DFU was performed.

To fix the issue added the CONFIG_NORDIC_QSPI_NOR_XIP_FLASH_SCHED_LOCK
option which should be used when executing parts of code from EXTXIP.
It ensures the blocking variant of nrfx_qspi_ is used and that
no preemption happens while an external flash write is in progress
(by making qspi_wait_while_writing blocking)

Signed-off-by: Artur Hadasz <artur.hadasz@nordicsemi.no>
Signed-off-by: Andrzej Puzdrowski <andrzej.puzdrowski@nordicsemi.no>

(cherry picked from commit 318948b)

Author: ahasztag

drivers/flash/Kconfig.nordic_qspi_nor

@@ -52,6 +52,18 @@ config NORDIC_QSPI_NOR_XIP
 	  QSPI NOR flash chip is executed until the driver has been setup.
 	  This will also disable power management for the QSPI NOR flash chip.
 
+config NORDIC_QSPI_NOR_XIP_FLASH_SCHED_LOCK
+	bool "Hold scheduler lock during QSPI NOR program/erase when XIP"
+	depends on NORDIC_QSPI_NOR_XIP
+	depends on MULTITHREADING
+	help
+	  When executing code from external QSPI flash (XIP), other threads must
+	  not fetch instructions from the same device while it is programming or
+	  erasing. Taking the scheduler lock for the duration of each flash
+	  operation prevents other threads from running XIP code concurrently.
+	  WIP polling uses busy-wait instead of sleeping so the lock is not
+	  violated. Enable for split XIP + DFU workloads.
+
 config NORDIC_QSPI_NOR_TIMEOUT_MS
 	int "Timeout for QSPI operations (ms)"
 	default 500

drivers/flash/nrf_qspi_nor.c

@@ -350,6 +350,11 @@ static void qspi_release(const struct device *dev)
 
 static inline void qspi_wait_for_completion(const struct device *dev, int res)
 {
+#if defined(CONFIG_NORDIC_QSPI_NOR_XIP_FLASH_SCHED_LOCK)
+	/* Do nothing, as the QSPI transfers are blocking */
+	ARG_UNUSED(res);
+	ARG_UNUSED(dev);
+#else
 	struct qspi_nor_data *dev_data = dev->data;
 
 	if (res == 0) {
@@ -366,6 +371,7 @@ static inline void qspi_wait_for_completion(const struct device *dev, int res)
 		irq_unlock(key);
 #endif /* CONFIG_MULTITHREADING */
 	}
+#endif /* CONFIG_NORDIC_QSPI_NOR_XIP_FLASH_SCHED_LOCK */
 }
 
 static inline void qspi_complete(struct qspi_nor_data *dev_data)
@@ -377,6 +383,20 @@ static inline void qspi_complete(struct qspi_nor_data *dev_data)
 #endif /* CONFIG_MULTITHREADING */
 }
 
+#if defined(CONFIG_NORDIC_QSPI_NOR_XIP_FLASH_SCHED_LOCK)
+
+/** In case of XIP execution from external flash, the QSPI transfers must be blocking
+ *  in order to avoid race conditions where at the same time the QSPI external flash
+ *  is being programmed and the QSPI is being used to execute code from the external flash.
+ *  Passing NULL to nrfx_qspi_init() will make the QSPI transfers blocking.
+ *  Note - after the QSPI transfer is complete (the READY event is received) a
+ *  nrfx_qspi_... blocking function will exit, but the external flash write operation
+ *  will be still in progress. Thus, qspi_wait_while_writing must be blocking as well.
+ *  Actually, most of the race conditions would in fact occur by preempting qspi_wait_while_writing
+ */
+#define qspi_handler NULL
+
+#else
 /**
  * @brief QSPI handler
  *
@@ -392,6 +412,7 @@ static void qspi_handler(nrfx_qspi_evt_t event, void *p_context)
 		qspi_complete(dev_data);
 	}
 }
+#endif /* CONFIG_NORDIC_QSPI_NOR_XIP_FLASH_SCHED_LOCK */
 
 /* QSPI send custom command.
  *
@@ -484,9 +505,14 @@ static int qspi_wait_while_writing(const struct device *dev, k_timeout_t poll_pe
 	do {
 #ifdef CONFIG_MULTITHREADING
 		if (!K_TIMEOUT_EQ(poll_period, K_NO_WAIT)) {
+#if defined(CONFIG_NORDIC_QSPI_NOR_XIP_FLASH_SCHED_LOCK)
+			/* Must not sleep while k_sched_lock held; wait same duration as k_sleep. */
+			k_busy_wait(k_ticks_to_us_floor32(poll_period.ticks));
+#else
 			k_sleep(poll_period);
-		}
 #endif
+		}
+#endif /* CONFIG_MULTITHREADING */
 		rc = qspi_rdsr(dev, 1);
 	} while ((rc >= 0)
 		 && ((rc & SPI_NOR_WIP_BIT) != 0U));
@@ -974,6 +1000,10 @@ static int qspi_nor_write(const struct device *dev, off_t addr,
 
 	qspi_acquire(dev);
 
+#if defined(CONFIG_NORDIC_QSPI_NOR_XIP_FLASH_SCHED_LOCK)
+	k_sched_lock();
+#endif
+
 	rc = qspi_nor_write_protection_set(dev, false);
 	if (rc == 0) {
 		if (size < 4U) {
@@ -989,6 +1019,10 @@ static int qspi_nor_write(const struct device *dev, off_t addr,
 
 	rc2 = qspi_nor_write_protection_set(dev, true);
 
+#if defined(CONFIG_NORDIC_QSPI_NOR_XIP_FLASH_SCHED_LOCK)
+	k_sched_unlock();
+#endif
+
 	qspi_release(dev);
 
 	return rc != 0 ? rc : rc2;
@@ -1020,8 +1054,16 @@ static int qspi_nor_erase(const struct device *dev, off_t addr, size_t size)
 
 	qspi_acquire(dev);
 
+#if defined(CONFIG_NORDIC_QSPI_NOR_XIP_FLASH_SCHED_LOCK)
+	k_sched_lock();
+#endif
+
 	rc = qspi_erase(dev, addr, size);
 
+#if defined(CONFIG_NORDIC_QSPI_NOR_XIP_FLASH_SCHED_LOCK)
+	k_sched_unlock();
+#endif
+
 	qspi_release(dev);
 
 	return rc;