From 2c57e9df65ff8f240341bc365b066ec16b3a02a0 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 23 Apr 2026 13:55:03 +1000 Subject: [PATCH 1/3] stm32: Conditionally disable USB interrupts during SD operations. - This is necessary if the USB MSC device is exposing the SD Card, as a USB operation might read/write SD. - However, rather than disabling USB interrupts (and all lower interrupts) unconditionally, we can check if MSC is enabled & SDCard is configured as one of the USB MSC LUNs. This happens to be necessary to reliably trigger SDCard DMA alignment bugs from hard ISRs. However, it's good to do anyhow - allows interrupt processing to continue while the CPU is blocked waiting for the SD operation. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton --- ports/stm32/sdcard.c | 41 +++++++++++++++++++++++++++----- ports/stm32/usbd_msc_interface.c | 9 +++++++ ports/stm32/usbd_msc_interface.h | 7 +++++- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/ports/stm32/sdcard.c b/ports/stm32/sdcard.c index 3e755bf9f2602..98bfe090f38d3 100644 --- a/ports/stm32/sdcard.c +++ b/ports/stm32/sdcard.c @@ -38,6 +38,15 @@ #include "dma.h" #include "irq.h" +#if !BUILDING_MBOOT +#include "usbd_msc_interface.h" +#else +// For mboot, act like the SDCard is always exposed via USB MSC +inline static bool usbd_msc_lu_includes_sdcard(void) { + return true; +} +#endif // !BUILDING_MBOOT + #if MICROPY_HW_ENABLE_SDCARD || MICROPY_HW_ENABLE_MMCARD #if defined(STM32F7) || defined(STM32H5) || defined(STM32H7) || defined(STM32L4) || defined(STM32N6) @@ -541,8 +550,14 @@ mp_uint_t sdcard_read_blocks(uint8_t *dest, uint32_t block_num, uint32_t num_blo } if (query_irq() == IRQ_STATE_ENABLED) { - // we must disable USB irqs to prevent MSC contention with SD card - uint32_t basepri = raise_irq_pri(IRQ_PRI_OTG_FS); + #if MICROPY_HW_USB_MSC + uint32_t basepri; + bool usb_msc_sdcard = usbd_msc_lu_includes_sdcard(); + if (usb_msc_sdcard) { + // we must disable USB irqs to prevent MSC contention with SD card + basepri = raise_irq_pri(IRQ_PRI_OTG_FS); + } + #endif #if SDIO_USE_GPDMA DMA_HandleTypeDef sd_dma; @@ -586,7 +601,11 @@ mp_uint_t sdcard_read_blocks(uint8_t *dest, uint32_t block_num, uint32_t num_blo } #endif - restore_irq_pri(basepri); + #if MICROPY_HW_USB_MSC + if (usb_msc_sdcard) { + restore_irq_pri(basepri); + } + #endif } else { #if MICROPY_HW_ENABLE_MMCARD if (pyb_sdmmc_flags & PYB_SDMMC_FLAG_MMC) { @@ -635,8 +654,14 @@ mp_uint_t sdcard_write_blocks(const uint8_t *src, uint32_t block_num, uint32_t n } if (query_irq() == IRQ_STATE_ENABLED) { - // we must disable USB irqs to prevent MSC contention with SD card - uint32_t basepri = raise_irq_pri(IRQ_PRI_OTG_FS); + #if MICROPY_HW_USB_MSC + uint32_t basepri; + bool usb_msc_sdcard = usbd_msc_lu_includes_sdcard(); + if (usb_msc_sdcard) { + // we must disable USB irqs to prevent MSC contention with SD card + basepri = raise_irq_pri(IRQ_PRI_OTG_FS); + } + #endif #if SDIO_USE_GPDMA DMA_HandleTypeDef sd_dma; @@ -679,7 +704,11 @@ mp_uint_t sdcard_write_blocks(const uint8_t *src, uint32_t block_num, uint32_t n } #endif - restore_irq_pri(basepri); + #if MICROPY_HW_USB_MSC + if (usb_msc_sdcard) { + restore_irq_pri(basepri); + } + #endif } else { #if MICROPY_HW_ENABLE_MMCARD if (pyb_sdmmc_flags & PYB_SDMMC_FLAG_MMC) { diff --git a/ports/stm32/usbd_msc_interface.c b/ports/stm32/usbd_msc_interface.c index 68236e5c51205..29bf46aaa661a 100644 --- a/ports/stm32/usbd_msc_interface.c +++ b/ports/stm32/usbd_msc_interface.c @@ -109,6 +109,15 @@ void usbd_msc_init_lu(size_t lu_n, const void *lu_data) { usbd_msc_lu_flags = 0; } +bool usbd_msc_lu_includes_sdcard(void) { + for (int i = 0; i < usbd_msc_lu_num; i++) { + if (usbd_msc_lu_data[i] == &pyb_sdcard_type) { + return true; + } + } + return false; +} + // Helper function to perform an ioctl on a logical unit static int lu_ioctl(uint8_t lun, int op, uint32_t *data) { if (lun >= usbd_msc_lu_num) { diff --git a/ports/stm32/usbd_msc_interface.h b/ports/stm32/usbd_msc_interface.h index 9d25a72a3a4ba..4980bb7b7e6a8 100644 --- a/ports/stm32/usbd_msc_interface.h +++ b/ports/stm32/usbd_msc_interface.h @@ -26,8 +26,13 @@ #ifndef MICROPY_INCLUDED_STM32_USBD_MSC_INTERFACE_H #define MICROPY_INCLUDED_STM32_USBD_MSC_INTERFACE_H -extern const USBD_StorageTypeDef usbd_msc_fops; +#include +#include + +extern const struct _USBD_STORAGE usbd_msc_fops; void usbd_msc_init_lu(size_t lu_n, const void *lu_data); +bool usbd_msc_lu_includes_sdcard(void); + #endif // MICROPY_INCLUDED_STM32_USBD_MSC_INTERFACE_H From 7a5eae80dfb8d6ed953cbb5b7e5302fd9d040c54 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 23 Apr 2026 13:54:09 +1000 Subject: [PATCH 2/3] tests/ports/stm32: Add DMA alignment test for SDCard driver. Rename the existing dma_alignment test to spi_dma_align, as this one uses the SPI driver. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton --- .../ports/stm32_hardware/sdcard_dma_align.py | 188 ++++++++++++++++++ .../{dma_alignment.py => spi_dma_align.py} | 0 2 files changed, 188 insertions(+) create mode 100644 tests/ports/stm32_hardware/sdcard_dma_align.py rename tests/ports/stm32_hardware/{dma_alignment.py => spi_dma_align.py} (100%) diff --git a/tests/ports/stm32_hardware/sdcard_dma_align.py b/tests/ports/stm32_hardware/sdcard_dma_align.py new file mode 100644 index 0000000000000..4af1ef543e49e --- /dev/null +++ b/tests/ports/stm32_hardware/sdcard_dma_align.py @@ -0,0 +1,188 @@ +# Test DMA read operations when the buffer alignment in RAM varies. +# +# Test requirements: +# A mostly empty FAT formatted SDCard installed in SD socket +import errno +import os +import pyb +import machine +import micropython +import vfs +import unittest + +from micropython import const + +_BLOCK_SZ = const(512) +_OFFS_WIDTH = const(64) # Should be at least the cache line size plus the GC block size +_TEST_BUF_SZ = const(_BLOCK_SZ + _OFFS_WIDTH) + +# More repeats = longer test run, more chance of triggering a cache coherence issue +_REPEATS = const(8) + +MOUNT_POINT = "/sd" +FILE_PATH = "/sd/stm32_align.blk" + +# Skip the whole test if there isn't a mountable SDCard +try: + sd = pyb.SDCard() + vfs.mount(sd, MOUNT_POINT) + vfs.umount(MOUNT_POINT) + del sd +except (OSError, AttributeError): + print("SKIP") + raise SystemExit + + +def verify_contents(buf, silent=False): + # Verify that each byte in 'buf' has the value of its index in the buffer + bad_pos = [] + for i in range(_BLOCK_SZ): + bi = buf[i] + if bi != i & 0xFF: + if silent: + return False + bad_pos.append((i, bi)) + if not bad_pos: + return True + + assert not silent + + print("{} bad readback values in sector:".format(len(bad_pos)), end="") + for i, v in bad_pos: + print(" {:#x}={:#x}".format(i, v), end="") + print() + return False + + +class TestSDAlign(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.sd = pyb.SDCard() + vfs.mount(cls.sd, MOUNT_POINT) + buf = bytearray(_BLOCK_SZ) + try: + with open(FILE_PATH, "rb") as f: + rlen = f.readinto(buf) + if rlen != _BLOCK_SZ: + raise RuntimeError("Unexpected length of temporary file ", FILE_PATH, rlen) + if not verify_contents(buf): + raise RuntimeError("Corrupt test block in temporary file ", FILE_PATH) + except OSError as e: + if e.errno != errno.ENOENT: + raise + + print("Creating new temp file...") + with open(FILE_PATH, "wb") as f: + f.write(bytes(i & 0xFF for i in range(_BLOCK_SZ))) + + # Now look for the sector which holds the temporary file + # (assume is in the first 20MB of the SD Card) + for b in range(1, 40960): + if not cls.sd.readblocks(b, buf): + raise RuntimeError("Failed to call readblocks on SDCard. block:", b) + if verify_contents(buf, True): + print("Temporary file contents found in block {}".format(b)) + cls.block = b + return + + raise RuntimeError( + "Contents of temporary file not found near start of SDCard. Too many files?" + ) + + @classmethod + def tearDownClass(cls): + try: + os.unlink(FILE_PATH) + print("Deleted temp file") + except OSError: + pass + vfs.umount(MOUNT_POINT) + del cls.sd + + def setUp(self): + self.offs = 0 + + @micropython.native + def _test_reads_inner(self, buf, repeats=_REPEATS): + for offs in range(_OFFS_WIDTH): + with self.subTest(offs=offs): + self.offs = offs + slice = memoryview(buf)[offs : offs + _BLOCK_SZ] + assert len(slice) == _BLOCK_SZ + for r in range(repeats): + self.assertTrue( + self.sd.readblocks(self.block, slice), + "Read failed for block {} offs {} repeat {}/{}".format( + self.block, offs, r, repeats + ), + ) + self.assertTrue( + verify_contents(slice), + "Verify failed for block {} offs {} repeat {}/{}".format( + self.block, offs, r, repeats + ), + ) + + def test_reads(self): + # Test reading at all available offsets in a buffer + buf = bytearray(_TEST_BUF_SZ) + # This test is the most random as any failure depends on speculative reads while + # the DMA operation is in progress. We run the test more times to increase the chance + # of hitting one of these cases, but even if the issue is present the test only fails + # once per ~250 iterations. The other tests inject explicit reads and writes so they fail + # more or less immediately. + self._test_reads_inner(buf, repeats=_REPEATS * 4) + + def test_interrupted_reads(self): + # Test reading at all available offsets in a buffer, while an interrupt is + # scanning through the whole buffer + buf = bytearray(_TEST_BUF_SZ) + t = None + self.scan = 0 + self.val = None + try: + + @micropython.native + def timer_cb(_): + # Arbitrary read from somewhere in the buffer + self.val = buf[self.scan % _TEST_BUF_SZ] + self.scan += 1 + + t = pyb.Timer(1, freq=30_000, callback=timer_cb, hard=True) + self._test_reads_inner(buf) + finally: + if t: + t.deinit() + # print("scan count", self.scan, self.val) + + def test_interrupted_read_write(self): + # Test reading at all available offsets in a buffer, while an interrupt is + # writing before & after the DMA buffer + buf = bytearray(_TEST_BUF_SZ) + t = None + try: + + @micropython.native + def timer_cb(t): + # Arbitrary CPU write just before and after the buffer, trying to dirty a DMA cache line + # + # Note: we never write into a word overlapping the DMA buffer, because if the buffer is not 4-byte aligned + # sdcard_read_blocks() will do a trick to align it temporarily, and this will race with that trick and + # corrupt the memory. + offs = self.offs + if offs > 3: + buf[offs - 4] = 0x55 + offs += _BLOCK_SZ + if offs < _TEST_BUF_SZ - 3: + buf[offs] = 0x56 + + # pybd SF2 at default CPU frequency can manage >30kHz <40kHz + t = pyb.Timer(1, freq=35_000, callback=timer_cb, hard=True) + self._test_reads_inner(buf) + finally: + if t: + t.deinit() + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/ports/stm32_hardware/dma_alignment.py b/tests/ports/stm32_hardware/spi_dma_align.py similarity index 100% rename from tests/ports/stm32_hardware/dma_alignment.py rename to tests/ports/stm32_hardware/spi_dma_align.py From d2e87fd16cce187836f9d918b11ab73081435f8c Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 15 Apr 2026 10:45:06 +1000 Subject: [PATCH 3/3] stm32/sdcard: Use dma_protect_rx_region on read operations. Will avoid corruption for unaligned reads, unless a SPI DMA operation is in progress at the same time (to be fixed separately). This fixes the unit test added in the parent commit. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton --- ports/stm32/sdcard.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ports/stm32/sdcard.c b/ports/stm32/sdcard.c index 98bfe090f38d3..f2714ab5865a0 100644 --- a/ports/stm32/sdcard.c +++ b/ports/stm32/sdcard.c @@ -574,7 +574,7 @@ mp_uint_t sdcard_read_blocks(uint8_t *dest, uint32_t block_num, uint32_t num_blo // make sure cache is flushed and invalidated so when DMA updates the RAM // from reading the peripheral the CPU then reads the new data - MP_HAL_CLEANINVALIDATE_DCACHE(dest, num_blocks * SDCARD_BLOCK_SIZE); + dma_protect_rx_region(dest, num_blocks * SDCARD_BLOCK_SIZE); sdcard_reset_periph(); #if MICROPY_HW_ENABLE_MMCARD @@ -589,6 +589,8 @@ mp_uint_t sdcard_read_blocks(uint8_t *dest, uint32_t block_num, uint32_t num_blo err = sdcard_wait_finished(); } + dma_unprotect_rx_region(dest, num_blocks * SDCARD_BLOCK_SIZE); + #if SDIO_USE_GPDMA dma_deinit(&SDMMC_DMA); #if MICROPY_HW_ENABLE_MMCARD