SDHCI: fix silent read failures and unbootable warm reset with UHS-I cards

.gitignore

@@ -191,6 +191,7 @@ tools/unit-tests/unit-update-flash-hwswap
 tools/unit-tests/unit-uart-flash
 tools/unit-tests/unit-max-space
 tools/unit-tests/unit-sdhci-disk-unaligned
+tools/unit-tests/unit-sdhci-dma-error
 tools/unit-tests/unit-fwtpm-stub
 tools/unit-tests/unit-gzip
 tools/unit-tests/unit-fit-gzip

include/sdhci.h

@@ -313,6 +313,9 @@
 #define SDHCI_SRS15_HV4E        (1U << 28)  /* Host version 4 enable */
 #define SDHCI_SRS15_UMS_MASK    (0x7U << 16)
 #define SDHCI_SRS15_UMS_SDR25   (0x1U << 16)
+/* Host Control 2 occupies the upper 16 bits of SRS15, so 1.8V Signaling
+ * Enable (bit 3 of Host Control 2) lands at bit 19. */
+#define SDHCI_SRS15_V18SE       (1U << 19)  /* 1.8V signaling enable */
 #define SDHCI_SRS15_DSS_MASK    (0x3U << 20)
 #define SDHCI_SRS15_DSS_TYPE_B  (0x0U << 20)
 #define SDHCI_SRS15_EXTNG       (1U << 22)  /* Execute tuning */

src/sdhci.c

@@ -76,6 +76,11 @@ static uint32_t g_sector_count;
 static uint32_t g_sector_size;
 static uint32_t g_bus_width = 1;
 static uint32_t g_rca = 0; /* SD Card Relative Address */
+#ifdef DISK_SDCARD
+/* Set once sdhci_uhs_recover() has switched the host to 1.8V signaling.
+ * SD-only: UHS-I signaling does not apply to eMMC. */
+static int g_uhs_recovered = 0;
+#endif /* DISK_SDCARD */
 
 /* MMC Interrupt state - volatile for interrupt handler access */
 static volatile uint32_t g_mmc_irq_status = 0;
@@ -369,6 +374,53 @@ static int sdhci_set_power(uint32_t voltage)
     return 0;
 }
 
+#ifdef DISK_SDCARD
+/* Recover a card that a previous stage left in UHS-I 1.8V signaling.
+ *
+ * A card that negotiated UHS-I only returns to 3.3V when VDD is removed --
+ * CMD0 does not do it, and on boards where the card supply is a fixed rail
+ * (ZCU102 among them) software cannot remove it at all. After a warm reset
+ * following an OS that used UHS, the card is therefore still at 1.8V while
+ * the host has come up at 3.3V. The command path tolerates the mismatch, so
+ * initialization and isolated reads appear to work, but sustained data
+ * transfers corrupt: Data CRC Error (SRS12 error bit 5) on PIO, and on SDMA
+ * a transfer that never completes.
+ *
+ * The condition cannot be detected up front. The warm reset clears the
+ * controller registers, so the inherited 1.8V Signaling Enable bit is gone
+ * before the bootloader runs, and the corruption is marginal enough that a
+ * short probe read usually succeeds. So this is driven from an actual data
+ * failure: switch the host to meet the card, then let the caller retry.
+ *
+ * This restores signaling the card is already using rather than initiating a
+ * voltage switch, so no CMD11 sequence is involved. It runs at most once per
+ * boot, and only after a transfer has already failed, so a cold boot never
+ * reaches it.
+ *
+ * Returns 0 if the switch was applied and the caller should retry. */
+static int sdhci_uhs_recover(void)
+{
+    if (g_uhs_recovered) {
+        return -1; /* already tried; the failure is something else */
+    }
+    g_uhs_recovered = 1;
+
+    wolfBoot_printf("SDHCI: data transfer failed at 3.3V; card appears to be "
+                    "left in UHS-I by a previous stage, retrying at 1.8V\n");
+
+    /* Stop the SD clock while the signaling level changes */
+    sdhci_reg_and(SDHCI_SRS11, ~SDHCI_SRS11_SDCE);
+
+    sdhci_reg_or(SDHCI_SRS15, SDHCI_SRS15_V18SE);
+    udelay(5000); /* let the level shifter settle */
+
+    sdhci_reg_or(SDHCI_SRS11, SDHCI_SRS11_SDCE);
+    udelay(1000);
+
+    return 0;
+}
+#endif /* DISK_SDCARD */
+
 /* ============================================================================
  * Clock Control
  * ============================================================================ */
@@ -1435,28 +1487,42 @@ static int sdhci_transfer(int dir, uint32_t cmd_index, uint32_t block_addr,
         }
     }
 
-    /* Check for errors */
+    /* Check for errors.
+     *
+     * An earlier failure (e.g. the SDMA wait above timing out) must survive
+     * this block. A wolfBoot-side wait timeout does not necessarily set an
+     * SRS12 error bit, so without preserving `status` the CMD12 / wait-busy
+     * results below would overwrite it and sdhci_transfer() would report
+     * success for a transfer that never completed. The caller then uses a
+     * partially filled buffer, which surfaces much later as a bogus image
+     * integrity failure rather than as the I/O error it actually is. */
     reg = SDHCI_REG(SDHCI_SRS12);
     if ((reg & SDHCI_SRS12_ERR_STAT) == 0) {
-        /* If multi-block, send CMD12 to stop transfer */
+        /* If multi-block, send CMD12 to stop transfer. This is issued even
+         * when `status` is already an error, to leave the bus in a sane
+         * state, but its result must not clear that error. */
         if (is_multi_block) {
         #ifdef DISK_EMMC
             uint32_t stop_arg = 0;
         #else
             uint32_t stop_arg = (g_rca << SD_RCA_SHIFT);
         #endif
+            int stop_status;
 
             SDHCI_REG_SET(SDHCI_SRS12, SDHCI_SRS12_TC); /* Clear transfer complete */
 
             /* Send stop multi-block transfer */
-            status = sdhci_send_cmd_internal(SDHCI_SRS03_CMD_ABORT,
+            stop_status = sdhci_send_cmd_internal(SDHCI_SRS03_CMD_ABORT,
                 MMC_CMD12_STOP_TRANS, stop_arg, SDHCI_RESP_R1B);
             /* Card may be busy programming data after CMD12 */
-            if (status == DEVICE_BUSY) {
-                status = sdhci_wait_busy(1);
+            if (stop_status == DEVICE_BUSY) {
+                stop_status = sdhci_wait_busy(1);
             }
-            if (status != 0) {
+            if (stop_status != 0) {
                 wolfBoot_printf("sdhci_transfer: CMD12 error\n");
+                if (status == 0) {
+                    status = stop_status;
+                }
             }
         }
         if (status == 0) {
@@ -1777,6 +1843,11 @@ int disk_read(int drv, uint64_t start, uint32_t count, uint8_t *buf)
                 block_addr, (uint32_t*)buf, read_sz);
         #endif
         }
+#ifdef DISK_SDCARD
+        if (status != 0 && sdhci_uhs_recover() == 0) {
+            continue; /* retry this chunk with matched signaling */
+        }
+#endif /* DISK_SDCARD */
         if (status != 0) {
             break;
         }

src/update_disk.c

@@ -496,9 +496,15 @@ void RAMFUNCTION wolfBoot_start(void)
             load_off += ret;
         } while (load_off < os_image.fw_size);
 
-        if (ret < 0) {
-            wolfBoot_printf("Error reading image from disk: p%d\r\n",
-                    cur_part);
+        /* A short read must fail here, as an I/O error. `ret == 0` breaks the
+         * loop above without being negative, and a truncated load would
+         * otherwise sail through to the integrity check and be reported as a
+         * corrupt image -- pointing the operator at the wrong problem, and on
+         * a system with anti-rollback leaving no bootable slot at all. */
+        if (ret <= 0 || load_off != os_image.fw_size) {
+            wolfBoot_printf("Error reading image from disk: p%d "
+                    "(%u of %u bytes)\r\n", cur_part,
+                    (unsigned int)load_off, (unsigned int)os_image.fw_size);
             selected ^= 1;
             continue;
         }

tools/unit-tests/Makefile

@@ -65,7 +65,7 @@ TESTS:=unit-parser unit-fdt unit-extflash unit-string unit-spi-flash unit-aes128
        unit-update-disk unit-update-disk-oob unit-multiboot unit-boot-x86-fsp unit-loader-tpm-init unit-qspi-flash unit-fwtpm-stub unit-tpm-rsa-exp \
        unit-image-nopart unit-image-sha384 unit-image-sha3-384 unit-store-sbrk \
        unit-tpm-blob unit-policy-create unit-policy-sign unit-rot-auth unit-sdhci-response-bits \
-       unit-sdhci-disk-unaligned unit-sign-encrypted-output \
+       unit-sdhci-disk-unaligned unit-sdhci-dma-error unit-sign-encrypted-output \
        unit-sign-hybrid-keyload \
        unit-sign-header-failure \
        unit-keygen-xmss-params
@@ -477,6 +477,10 @@ unit-sdhci-disk-unaligned: ../../include/target.h unit-sdhci-disk-unaligned.c
 	gcc -o $@ $^ $(CFLAGS) -ffunction-sections -fdata-sections $(LDFLAGS) \
 		-Wl,--gc-sections
 
+unit-sdhci-dma-error: ../../include/target.h unit-sdhci-dma-error.c
+	gcc -o $@ $^ $(CFLAGS) -ffunction-sections -fdata-sections $(LDFLAGS) \
+		-Wl,--gc-sections
+
 unit-aes128: ../../include/target.h unit-extflash.c
 	gcc -o $@ $^ $(CFLAGS) $(LDFLAGS)
 

tools/unit-tests/unit-sdhci-dma-error.c

@@ -0,0 +1,183 @@
+/* unit-sdhci-dma-error.c
+ *
+ * Regression test: a failed SDMA data transfer must be reported as an error.
+ *
+ * sdhci_transfer() waits for Transfer Complete on the SDMA path and sets
+ * status = -1 if that wait times out. The post-transfer "check for errors"
+ * block then runs unconditionally, and a wolfBoot-side wait timeout does not
+ * necessarily leave an error bit set in SRS12. Before the fix, the CMD12
+ * (stop transmission) result overwrote that -1, so the function returned
+ * success for a transfer that never moved any data.
+ *
+ * The caller then treats a partially filled buffer as a good read. On a
+ * verified-boot target that surfaces much later as an image integrity
+ * failure rather than as the I/O error it actually is -- and where
+ * anti-rollback is enabled, the good lower-versioned slot is refused too,
+ * leaving nothing bootable.
+ *
+ * Copyright (C) 2026 wolfSSL Inc.
+ *
+ * This file is part of wolfBoot.
+ *
+ * wolfBoot is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * wolfBoot is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA
+ */
+
+#define DISK_SDCARD 1
+
+#include <check.h>
+#include <stdint.h>
+#include <string.h>
+
+#include "sdhci.h"
+
+static uint32_t mock_regs[0x260 / sizeof(uint32_t)];
+
+/* When 0, a data transfer never signals Transfer Complete -- the SDMA wait
+ * times out, which is the condition under test. When 1, the transfer
+ * completes normally (positive control). */
+static int mock_complete_transfer;
+
+/* Monotonic, so udelay() in the included sdhci.c always terminates. A
+ * constant timer makes its "while (hal_get_timer_us() < end)" loop spin
+ * forever if any code path under test ever calls it. */
+uint64_t hal_get_timer_us(void)
+{
+    static uint64_t now;
+    return ++now;
+}
+
+uint32_t sdhci_reg_read(uint32_t offset)
+{
+    return mock_regs[offset / sizeof(uint32_t)];
+}
+
+void sdhci_reg_write(uint32_t offset, uint32_t val)
+{
+    uint32_t *reg = &mock_regs[offset / sizeof(uint32_t)];
+
+    /* Reset bits self-clear, and the internal clock reports stable */
+    if (offset == SDHCI_SRS11) {
+        *reg = val | (val & SDHCI_SRS11_ICE ? SDHCI_SRS11_ICS : 0);
+        *reg &= ~SDHCI_SRS11_RESET_DAT_CMD;
+        return;
+    }
+
+    /* SRS12 is write-1-to-clear */
+    if (offset == SDHCI_SRS12) {
+        *reg &= ~val;
+        return;
+    }
+
+    *reg = val;
+
+    if (offset == SDHCI_SRS03) {
+        if (val & SDHCI_SRS03_DPS) {
+            /* Data-present command. Signal Transfer Complete only when the
+             * test asks for it. No error bits are set either way: this
+             * reproduces a wait timeout with a clean SRS12, which is what
+             * made the old code discard the error. */
+            if (mock_complete_transfer) {
+                mock_regs[SDHCI_SRS12 / sizeof(uint32_t)] |= SDHCI_SRS12_TC;
+            }
+        }
+        else {
+            /* Command without data (e.g. CMD12 stop) completes normally */
+            mock_regs[SDHCI_SRS04 / sizeof(uint32_t)] = (1U << 8);
+            mock_regs[SDHCI_SRS12 / sizeof(uint32_t)] |= SDHCI_SRS12_CC;
+        }
+    }
+}
+
+void sdhci_platform_init(void)
+{
+}
+
+void sdhci_platform_irq_init(void)
+{
+}
+
+void sdhci_platform_set_bus_mode(int is_emmc)
+{
+    (void)is_emmc;
+}
+
+#include "../../src/sdhci.c"
+
+/* Large enough to take the SDMA path (>= SDHCI_DMA_THRESHOLD) */
+#define TEST_XFER_SZ (8U * 1024U)
+
+static uint32_t test_buf[TEST_XFER_SZ / sizeof(uint32_t)];
+
+static void reset_mock_state(int complete)
+{
+    memset(mock_regs, 0, sizeof(mock_regs));
+    memset(test_buf, 0, sizeof(test_buf));
+    mock_complete_transfer = complete;
+    g_mmc_irq_pending = 0;
+    g_mmc_irq_status = 0;
+}
+
+/* The bug: SDMA never completes, SRS12 carries no error bit, and the CMD12
+ * issued afterwards succeeds -- which used to overwrite the error. */
+START_TEST(test_sdma_timeout_is_reported)
+{
+    int ret;
+
+    reset_mock_state(0);
+    ret = sdhci_read(MMC_CMD18_READ_MULTIPLE, 0, test_buf, TEST_XFER_SZ);
+    ck_assert_msg(ret != 0,
+        "sdhci_read must fail when the SDMA transfer never completes "
+        "(returned %d)", ret);
+}
+END_TEST
+
+/* Positive control: with Transfer Complete signalled the same path succeeds,
+ * so the assertions above are detecting the timeout and not simply a mock
+ * that can never succeed. */
+START_TEST(test_completed_transfer_succeeds)
+{
+    int ret;
+
+    reset_mock_state(1);
+    ret = sdhci_read(MMC_CMD18_READ_MULTIPLE, 0, test_buf, TEST_XFER_SZ);
+    ck_assert_msg(ret == 0,
+        "sdhci_read must succeed when the transfer completes (returned %d)",
+        ret);
+}
+END_TEST
+
+Suite *sdhci_dma_suite(void)
+{
+    Suite *s = suite_create("sdhci-dma-error");
+    TCase *tc = tcase_create("dma-error");
+
+    /* The timeout path spins a large retry count before giving up */
+    tcase_set_timeout(tc, 60);
+    tcase_add_test(tc, test_sdma_timeout_is_reported);
+    tcase_add_test(tc, test_completed_transfer_succeeds);
+    suite_add_tcase(s, tc);
+    return s;
+}
+
+int main(void)
+{
+    int fails;
+    SRunner *sr = srunner_create(sdhci_dma_suite());
+
+    srunner_run_all(sr, CK_NORMAL);
+    fails = srunner_ntests_failed(sr);
+    srunner_free(sr);
+    return (fails == 0) ? 0 : 1;
+}