F-11032: bound the ACMD41 OCR readiness poll in SD card init

sdcard_card_full_init() polled ACMD41 in an unbounded do/while until
the card set OCR ready, so a card that answers every ACMD41 without
ever setting the bit held the bootloader in the loop forever. F-7984
bounded the separate DATA0/CMD13 waits in sdhci_wait_busy(); this is
the OCR readiness path, which still had no limit.

Bound the poll with the same shape as sdhci_wait_busy(): a
30000 ms budget (SDCARD_ACMD41_TIMEOUT_MS, #ifndef-able) measured
against hal_get_timer_us(), the watchdog serviced inside the loop,
and -1 returned to fail the SD boot path. A healthy card reports
ready in milliseconds, so the budget is far above any real
initialization time.

unit-sdhci-acmd41-timeout compiles the real driver (generated
sdhci_host.c, as in the wait-busy test) against a scripted
controller: commands complete without error, SRS12 is modeled
write-1-to-clear, and the card model sets OCR ready after a
configurable number of ACMD41 polls. A never-ready card must return
-1 inside the shipped budget (and service the watchdog); a card
ready after 5 polls must exit the loop promptly and proceed to the
end of the init path. A command-write cap turns the pre-fix infinite
loop into an abort instead of a hung build.

Verification:
- Built: gcc (host) unit-sdhci-acmd41-timeout with -DDISK_SDCARD:
  clean.
- Tested: unit-sdhci-acmd41-timeout 2/2; pre-fix the never-ready case
  aborted at the 50001st command (the loop never terminates); post-fix
  it runs 3001 polls, reaches the 30 s budget, pets the watchdog
  every iteration and returns -1.
- Pitfalls: the timeout returns -1 from the SD path, the same
  contract as a failed CMD0/CMD8; the budget is per init call, not
  shared with sdhci_wait_busy, and a card that becomes ready before
  the deadline is unaffected.
- Style: cstyle-check.sh FMT diff on src/sdhci.c byte-identical to
  the pre-change file; the new test is flag-free.
- Unverified: no SD card hardware execution.
- Message: F-11032: prefix, no co-author trailers.
pull/874/head
Daniele Lacamera 2026-08-26 17:31:56 +02:00
parent 300e5db110
commit c47135746a
3 changed files with 261 additions and 2 deletions

View File

@ -829,6 +829,13 @@ static int sdcard_card_init(uint32_t acmd41_arg, uint32_t *ocr_reg)
static int sdcard_set_bus_width(uint32_t bus_width);
static int sdcard_set_function(uint32_t function_number, uint32_t group_number);
/* A card that answers ACMD41 forever without setting OCR ready must
* not hold the boot. The budget matches the sdhci_wait_busy() wait;
* a healthy card reports ready in milliseconds. */
#ifndef SDCARD_ACMD41_TIMEOUT_MS
#define SDCARD_ACMD41_TIMEOUT_MS 30000
#endif
/* Full SD card initialization sequence
* Returns 0 on success */
static int sdcard_card_full_init(void)
@ -898,6 +905,9 @@ static int sdcard_card_full_init(void)
}
if (status == 0) {
uint64_t start = hal_get_timer_us();
const uint64_t timeout_us =
(uint64_t)SDCARD_ACMD41_TIMEOUT_MS * 1000U;
/* configure operating conditions */
uint32_t cmd_arg = SDCARD_ACMD41_HCS;
cmd_arg |= card_volts;
@ -911,10 +921,17 @@ static int sdcard_card_full_init(void)
wolfBoot_printf("sdcard_init: sending OCR arg: 0x%08X\n", cmd_arg);
#endif
/* retry until OCR ready */
/* retry until OCR ready; a card that never sets it must not
* hold the boot, so bound the poll like sdhci_wait_busy() and
* service the watchdog inside it */
do {
status = sdcard_card_init(cmd_arg, &reg);
} while (status == 0 && (reg & SDCARD_REG_OCR_READY) == 0);
if (status != 0 || (reg & SDCARD_REG_OCR_READY) != 0)
break;
sdhci_platform_wdt_pet();
if (hal_get_timer_us() - start > timeout_us)
status = -1;
} while (status == 0);
}
if (status == 0) {

View File

@ -130,6 +130,7 @@ TESTS+=unit-stm32u5-write
TESTS+=unit-nvm-cache-scrub
TESTS+=unit-sdhci-uhs-recover
TESTS+=unit-sdhci-wait-busy
TESTS+=unit-sdhci-acmd41-timeout
TESTS+=unit-ti-hercules-write
TESTS+=unit-p1021-qe-firmware
TESTS+=unit-t10xx-dts-memac
@ -1212,6 +1213,16 @@ unit-sdhci-uhs-recover: unit-sdhci-uhs-recover.c sdhci_host.c
unit-sdhci-wait-busy: unit-sdhci-wait-busy.c sdhci_host.c
gcc -o $@ unit-sdhci-wait-busy.c -DDISK_SDCARD -DWOLFBOOT_NO_PRINTF $(CFLAGS) $(LDFLAGS)
# unit-sdhci-acmd41-timeout drives sdcard_card_full_init()'s ACMD41 OCR
# readiness poll from the real src/sdhci.c (F-11032: the do/while had no
# bound, so a card answering ACMD41 without ever setting OCR ready held
# the boot forever). Same sdhci_host.c generation as the wait-busy test;
# the card model sets OCR ready after N scripted ACMD41 polls, the timer
# stub steps 10 ms per read, and a command-write cap turns the pre-fix
# infinite loop into an abort.
unit-sdhci-acmd41-timeout: unit-sdhci-acmd41-timeout.c sdhci_host.c
gcc -o $@ unit-sdhci-acmd41-timeout.c -DDISK_SDCARD -DWOLFBOOT_NO_PRINTF $(CFLAGS) $(LDFLAGS)
# unit-ti-hercules-write runs the real hal_flash_write() staging logic
# from hal/ti_hercules.c (a short write crossing a block
# boundary overran the staging buffer and lost the next block's bytes).

View File

@ -0,0 +1,231 @@
/* unit-sdhci-acmd41-timeout.c
*
* Regression test for F-11032: sdcard_card_full_init() polled ACMD41
* in an unbounded do/while until the card set OCR ready, so a card
* that answers every ACMD41 without ever setting the bit held the
* bootloader in the loop forever. (F-7984 bounded the separate
* DATA0/CMD13 waits in sdhci_wait_busy(); this is the OCR readiness
* path.)
*
* The real driver is compiled from the generated sdhci_host.c
* (identical to src/sdhci.c except the three x86-incompatible asm
* statements are blanked and sdhci_read() is renamed to
* sdhci_read_hw()); the controller is scripted through the host
* register file. The card model sets OCR READY after a configurable
* number of ACMD41 polls (0 = never). The command-write cap turns the
* pre-fix infinite loop into an abort instead of a hung CI.
* 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
*/
#include <check.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>
#include "sdhci.h"
#include "disk.h"
/* Host register file for the platform-provided register accessors. */
static uint32_t g_sdhci_regs[0x400 / sizeof(uint32_t)];
/* Command-write cap: the pre-fix ACMD41 loop never terminates, so the
* cap aborts the test (a failure) instead of hanging the build. The
* fixed loop reaches the shipped 30 s budget in well under the cap. */
static uint32_t g_cmd_writes;
#define CMD_WRITE_CAP 50000
/* Card model: after g_ready_after ACMD41 polls the card sets OCR
* ready (0 = the card never becomes ready). */
static uint32_t g_ready_after;
static uint32_t g_acmd41_polls;
uint32_t sdhci_reg_read(uint32_t offset)
{
return g_sdhci_regs[offset / sizeof(uint32_t)];
}
void sdhci_reg_write(uint32_t offset, uint32_t val)
{
if (offset == SDHCI_SRS12) {
/* interrupt status register: write-1-to-clear */
g_sdhci_regs[offset / sizeof(uint32_t)] &= ~val;
return;
}
g_sdhci_regs[offset / sizeof(uint32_t)] = val;
if (offset == SDHCI_SRS03) {
uint32_t idx;
/* the scripted controller completes every command without
* error; the response is read from the preset SRS04 */
g_sdhci_regs[SDHCI_SRS12 / sizeof(uint32_t)] |= SDHCI_SRS12_CC;
g_cmd_writes++;
if (g_cmd_writes > CMD_WRITE_CAP)
ck_abort_msg("ACMD41 poll ran away: %u commands",
g_cmd_writes);
idx = (val & SDHCI_SRS03_CIDX_MASK) >> SDHCI_SRS03_CIDX_SHIFT;
if (idx == SD_ACMD41_SEND_OP_COND && g_ready_after > 0) {
g_acmd41_polls++;
if (g_acmd41_polls >= g_ready_after) {
g_sdhci_regs[SDHCI_SRS04 / sizeof(uint32_t)] |=
SDCARD_REG_OCR_READY;
g_ready_after = 0;
}
}
}
}
/* Timer: advance by g_timer_step microseconds on every read, so the
* shipped SDCARD_ACMD41_TIMEOUT_MS is exercised as built. */
static uint64_t g_timer_us;
static uint64_t g_timer_step = 10000;
uint64_t hal_get_timer_us(void)
{
g_timer_us += g_timer_step;
return g_timer_us;
}
/* Counts watchdog services performed inside the poll loop. */
static unsigned int g_wdt_pets;
void sdhci_platform_wdt_pet(void)
{
g_wdt_pets++;
}
/* Platform hooks the SD init path touches. */
void sdhci_platform_init(void)
{
}
void sdhci_platform_irq_init(void)
{
}
void sdhci_platform_set_bus_mode(int is_emmc)
{
(void)is_emmc;
}
/* The real driver (see the Makefile for the transforms). */
#include "sdhci_host.c"
/* The data path is not under test; the stub ends the init sequence
* after the ACMD41 loop, the bus-width setup and the SCR read. */
int sdhci_read(uint32_t cmd_index, uint32_t block_addr, uint32_t *dst,
uint32_t sz)
{
(void)cmd_index; (void)block_addr; (void)dst; (void)sz;
return -1;
}
/* Script the controller for a card present at 3.3 V: every command
* completes immediately (SRS12 CC preset), responses come from the
* preset SRS04 (READY_FOR_DATA for R1, voltage bits for the OCR),
* the host is 3.3-V capable, and no 1.8-V / UHS features are
* advertised so the init stays on the plain path. */
static void script_card_present(void)
{
g_sdhci_regs[SDHCI_SRS12 / sizeof(uint32_t)] = SDHCI_SRS12_CC;
g_sdhci_regs[SDHCI_SRS04 / sizeof(uint32_t)] =
(1U << 8) | SDCARD_REG_OCR_3_3_3_4;
g_sdhci_regs[SDHCI_SRS16 / sizeof(uint32_t)] = SDHCI_SRS16_VS33;
g_sdhci_regs[SDHCI_SRS18 / sizeof(uint32_t)] = 0; /* no 1.8V current */
}
static void setup(void)
{
memset(g_sdhci_regs, 0, sizeof(g_sdhci_regs));
g_timer_us = 0;
g_timer_step = 10000; /* 10 ms per timer read */
g_wdt_pets = 0;
g_cmd_writes = 0;
g_acmd41_polls = 0;
g_ready_after = 0;
}
static void teardown(void)
{
}
/* A card that answers ACMD41 forever without setting OCR ready must
* time out with an error instead of holding the boot. Pre-fix the
* loop never terminates and the command-write cap aborts. */
START_TEST(test_acmd41_never_ready_times_out)
{
int ret;
script_card_present();
g_ready_after = 0; /* never ready */
ret = sdcard_card_full_init();
ck_assert_int_eq(ret, -1);
/* the poll ran to the shipped budget, so it must have petted */
ck_assert_uint_gt(g_wdt_pets, 0);
/* and stayed far below the runaway cap */
ck_assert_uint_lt(g_cmd_writes, CMD_WRITE_CAP);
}
END_TEST
/* A card that reports ready after a few polls must initialize: the
* loop exits promptly (well under the cap) and the sequence proceeds
* to the end of the init path (the stubbed SCR read ends it). */
START_TEST(test_acmd41_ready_after_polls)
{
int ret;
script_card_present();
g_ready_after = 5;
ret = sdcard_card_full_init();
/* the loop must have exited on OCR ready, not on the timeout */
ck_assert_uint_eq(g_acmd41_polls, 5);
ck_assert_uint_lt(g_cmd_writes, 1000);
/* the init ran to the stubbed data path, which fails by design */
ck_assert_int_eq(ret, -1);
}
END_TEST
Suite *sdhci_acmd41_suite(void)
{
Suite *s = suite_create("sdhci-acmd41-timeout");
TCase *tc = tcase_create("acmd41-timeout");
tcase_add_checked_fixture(tc, setup, teardown);
tcase_add_test(tc, test_acmd41_never_ready_times_out);
tcase_add_test(tc, test_acmd41_ready_after_polls);
suite_add_tcase(s, tc);
return s;
}
int main(void)
{
int fails;
Suite *s = sdhci_acmd41_suite();
SRunner *sr = srunner_create(s);
srunner_run_all(sr, CK_NORMAL);
fails = srunner_ntests_failed(sr);
srunner_free(sr);
return fails;
}