diff --git a/ports/stm32/sdcard.c b/ports/stm32/sdcard.c index 3e755bf9f2602..f2714ab5865a0 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; @@ -559,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 @@ -574,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 @@ -586,7 +603,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 +656,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 +706,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 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