From e1f04ce57d6d167e501883b82514d2fbfb87f9d3 Mon Sep 17 00:00:00 2001 From: "NathanNeurotic (Ripto)" <109461996+NathanNeurotic@users.noreply.github.com> Date: Mon, 20 Jul 2026 08:39:30 -0700 Subject: [PATCH 1/2] fix(hdd): un-poison the APA module-load latch + surface silent probe failures (Vapor) Vapor: APA internal HDD lists ZERO games in RiptOPL under both Auto and Manual, while the same drive lists fine in wOPL and official OPL; another tester on the SAME build has APA working. Root-caused from code (adversarially verified); two confirmed mechanisms, all fixed here except the one that is a legitimate user setting: 1. THE ONE-SHOT LATCH (upstream-verbatim code, fork-widened exposure). hddLoadModules consumes its load count on the FIRST call; if that first ATAD load fails, the failure was permanent for the session: every later call returned BUSYLOADING(2), which all `>= 0` caller tests read as SUCCESS while nothing was loaded -- then hddDetectNonSonyFileSystem devctl'd a nonexistent xhdd0:, got -1, and bailed WITH NO ERROR MESSAGE. Net effect: an empty APA page all session, Auto and Manual alike, indistinguishable from a dead drive. Upstream's earliest callers run at HDD-tab entry when the drive is ready; OUR fork added the earliest, least-gated caller -- bdmResolveBootDir's ATA escalation on a mass: boot, seconds after power-on with the platter still spinning up -- which is exactly how a USB-booting APA user (Vapor boots MMCE/USB) consumes the latch and poisons the page. FIX: a failed load resets the count (retryable -- the HDD tab gets a real second attempt); HDD_LOADMODULES_OK() replaces the four `>= 0` tests (bdmsupport.c x2, opl.c x2) so BUSYLOADING is no longer success; the silent -1 probe-failure bail now raises the HDD-not-detected error box. The genuine MBR/GPT/exFAT result (1) stays deliberately silent -- that is the normal coexistence case for BDM-HDD users and must not toast every boot. 2. THE DEFAULT-GAME-VIEW LOCK (mechanism confirmed; the value itself is a legitimate setting). default_game_view=VCD makes every VCD-capable page serve its PS1 list; on a pure-PS2 APA drive that page is EMPTY with the L3 toggle inert, its hint hidden, no rescan on SELECT, and the normal "HDD Games" title -- unlabeled and unrecoverable on-console. This commit only CLAMPS an out-of-enum config value to BOTH (the coverflow ints beside it were always clamped; this one was not). Whether a deliberate VCD lock should stay L3-escapable is a design call left to the maintainer -- flagged in the PR, not changed here. Builds clean (make clean + make opl.elf, exit 0). HW-pending: Vapor's rig. Co-Authored-By: Claude Fable 5 --- include/hddsupport.h | 7 +++++++ src/bdmsupport.c | 4 ++-- src/hddsupport.c | 21 +++++++++++++++++++-- src/opl.c | 10 ++++++++-- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/include/hddsupport.h b/include/hddsupport.h index 0304beb5e3..4536ac2d04 100644 --- a/include/hddsupport.h +++ b/include/hddsupport.h @@ -66,6 +66,13 @@ typedef enum { HDD_LOADMODULES_STATUS_COUNT, } hdd_loadmodules_status; +// TRUE only when the ATA stack is actually resident (fresh load or already loaded). Callers used to +// test `hddLoadModules() >= 0`, which also accepted BUSYLOADING(2) -- returned for the whole session +// after a FAILED first load consumed the one-shot count -- so "nothing is loaded" read as success and +// the APA page sat silently empty under both Auto and Manual (Vapor). Pair with the retryable-failure +// change in hddLoadModules. +#define HDD_LOADMODULES_OK(r) ((r) == HDD_LOADMODULES_STATUS_NOERROR || (r) == HDD_LOADMODULES_STATUS_ALREADYLOADED) + int hddCheck(void); u32 hddGetTotalSectors(void); int hddIs48bit(void); diff --git a/src/bdmsupport.c b/src/bdmsupport.c index 68c6e94be8..6f28febe59 100644 --- a/src/bdmsupport.c +++ b/src/bdmsupport.c @@ -353,7 +353,7 @@ static void bdmLoadBlockDeviceModules(void) // Only mark loaded on success -- a failure (no HDD/interface) must not // suppress future retries via bdmShouldQueueModuleLoad() (matches the // conditional pattern used for USB/MX4SIO and opl.c's hddLoadModules gate). - if (hddLoadModules() >= 0) + if (HDD_LOADMODULES_OK(hddLoadModules())) hddModLoaded = 1; } @@ -1660,7 +1660,7 @@ static int bdmEnsureTransportLoaded(int bdmType) } return iLinkModLoaded; case BDM_TYPE_ATA: - if (!hddModLoaded && hddLoadModules() >= 0) + if (!hddModLoaded && HDD_LOADMODULES_OK(hddLoadModules())) hddModLoaded = 1; return hddModLoaded; default: diff --git a/src/hddsupport.c b/src/hddsupport.c index f93a59bf38..5e11fbeec5 100644 --- a/src/hddsupport.c +++ b/src/hddsupport.c @@ -224,6 +224,15 @@ int hddLoadModules(void) LOG("HDD: No HardDisk Drive detected.\n"); setErrorMessageWithCode(_STR_HDD_NOT_CONNECTED_ERROR, ERROR_HDD_IF_NOT_DETECTED); retStatus = HDD_LOADMODULES_STATUS_ERROR; + // Make the failure RETRYABLE. Leaving the count consumed turned one bad first probe into a + // whole-session poison: every later call took the else branch and returned BUSYLOADING(2), + // which the >= 0 caller tests read as SUCCESS while nothing was loaded -- so the APA page + // sat silently empty under BOTH Auto and Manual (Vapor's report; the drive lists fine in + // wOPL/upstream because their earliest callers run at tab entry, when the drive is ready -- + // our fork adds boot-time callers like bdmResolveBootDir's ATA escalation, seconds after + // power-on). sysInitDev9/sysLoadModuleBuffer are safe to re-run; a later caller (e.g. the + // HDD tab) now gets a real second attempt instead of a poisoned latch. + hddModulesLoadCount = 0; } else { retStatus = HDD_LOADMODULES_STATUS_NOERROR; hddModulesLoaded = 1; @@ -313,9 +322,17 @@ void hddLoadSupportModules(void) // Check if the drive contains MBR/GPT partition data before we load the APA/PFS modules. If the drive is not // APA then loading the APA irx modules can corrupt the drive as it will try to write APA partition data. - if (hddDetectNonSonyFileSystem() != 0) { + int nonSony = hddDetectNonSonyFileSystem(); + if (nonSony != 0) { // Drive is MBR/GPT style, or unknown, bail out or risk corrupting the drive. - LOG("HDDSUPPORT LoadSupportModules bailing out early...\n"); + LOG("HDDSUPPORT LoadSupportModules bailing out early (%d)...\n", nonSony); + // A PROBE FAILURE (-1: the xhdd0: partition-sector devctl errored -- drive not ready, module + // missing, transient bus fault) was completely SILENT: no error box, no list, an APA page that + // just looks like a dead drive for the whole session. Surface it. The 1 (genuine MBR/GPT/exFAT) + // branch stays silent on purpose -- that is the NORMAL coexistence case for BDM-HDD users and + // must not toast at every boot. + if (nonSony < 0) + setErrorMessageWithCode(_STR_HDD_NOT_CONNECTED_ERROR, ERROR_HDD_NOT_DETECTED); return; } diff --git a/src/opl.c b/src/opl.c index 18b9efc2f7..0a243cf83d 100644 --- a/src/opl.c +++ b/src/opl.c @@ -1313,7 +1313,7 @@ static int checkLoadConfigBDMHDD(int types) int value; // Bounded wait so BDM-on-HDD can be detected without long black-screen stalls. - if (hddLoadModules() >= 0 && bdmHDDIsPresent(500)) { + if (HDD_LOADMODULES_OK(hddLoadModules()) && bdmHDDIsPresent(500)) { if (bdmFindPartition(path, CONFIG_OPL_FILENAME, 0) || bdmFindPartition(path, CONFIG_OPL_FILENAME_LEGACY, 0)) { configEnd(); configInit(path); @@ -1580,6 +1580,12 @@ static void _loadConfig() configGetInt(configOPL, CONFIG_OPL_ENABLE_ART_TAR, &gEnableArtTar); configGetInt(configOPL, CONFIG_OPL_WIDESCREEN, &gWideScreen); configGetInt(configOPL, CONFIG_OPL_DEFAULT_GAME_VIEW, &gDefaultGameView); + // Clamp: an out-of-enum value (hand-edited/corrupt config) is UNRECOVERABLE on-console -- + // vcdViewActive() treats anything but BOTH as a lock, the L3 toggle goes inert and its hint + // is hidden, so every VCD-capable page silently serves one (possibly empty) list with no + // way back. The coverflow ints just below have always been clamped; this one was not. + if (gDefaultGameView < GAME_VIEW_BOTH || gDefaultGameView > GAME_VIEW_VCD) + gDefaultGameView = GAME_VIEW_BOTH; // A boot default-view locked to one type (VCD or ISO) must force the same one-shot // rescan the settings dialog does on a view change (gui.c). Without it, vcdViewActive() // short-circuits mmce/bdm/hdd/eth NeedsUpdate before the initial-scan trigger and the @@ -1895,7 +1901,7 @@ static int trySaveConfigBDMHDD(int types) char path[64]; // Bounded wait so save can target BDM-on-HDD without long stalls. - if (hddLoadModules() >= 0 && bdmHDDIsPresent(500)) { + if (HDD_LOADMODULES_OK(hddLoadModules()) && bdmHDDIsPresent(500)) { if (bdmFindPartition(path, CONFIG_OPL_FILENAME, 1)) { configSetMove(path); return configWriteMulti(types); From df7e15e840020d6e046e0cff1955805738accb83 Mon Sep 17 00:00:00 2001 From: "NathanNeurotic (Ripto)" <109461996+NathanNeurotic@users.noreply.github.com> Date: Mon, 20 Jul 2026 09:13:39 -0700 Subject: [PATCH 2/2] fix(hdd): evaluate the loader result exactly once (CodeRabbit review of #241 -- vetted, real) HDD_LOADMODULES_OK(hddLoadModules()) double-evaluated its argument: any non-NOERROR first result ran hddLoadModules() a SECOND time inside the same condition. Combined with this PR's retryable-failure change, a failed load would immediately re-run the entire load -- second dev9 init, second module attempt, second error toast -- per test site. Replaced the macro with static inline hddLoadModulesReady(), which calls once and tests the captured result. All four call sites updated. Builds clean (make clean + make opl.elf, exit 0). Co-Authored-By: Claude Fable 5 --- include/hddsupport.h | 22 ++++++++++++++++------ src/bdmsupport.c | 4 ++-- src/opl.c | 4 ++-- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/include/hddsupport.h b/include/hddsupport.h index 4536ac2d04..133cb3d2d4 100644 --- a/include/hddsupport.h +++ b/include/hddsupport.h @@ -66,12 +66,11 @@ typedef enum { HDD_LOADMODULES_STATUS_COUNT, } hdd_loadmodules_status; -// TRUE only when the ATA stack is actually resident (fresh load or already loaded). Callers used to -// test `hddLoadModules() >= 0`, which also accepted BUSYLOADING(2) -- returned for the whole session -// after a FAILED first load consumed the one-shot count -- so "nothing is loaded" read as success and -// the APA page sat silently empty under both Auto and Manual (Vapor). Pair with the retryable-failure -// change in hddLoadModules. -#define HDD_LOADMODULES_OK(r) ((r) == HDD_LOADMODULES_STATUS_NOERROR || (r) == HDD_LOADMODULES_STATUS_ALREADYLOADED) +// See hddLoadModulesReady() below the prototypes -- the single evaluate-once way to ask "is the ATA +// stack actually resident?". Callers used to test `hddLoadModules() >= 0`, which also accepted +// BUSYLOADING(2) -- returned for the whole session after a FAILED first load consumed the one-shot +// count -- so "nothing is loaded" read as success and the APA page sat silently empty under both +// Auto and Manual (Vapor). Pair with the retryable-failure change in hddLoadModules. int hddCheck(void); u32 hddGetTotalSectors(void); @@ -98,6 +97,17 @@ void hddVcdInvalidateCache(void); void hddInit(item_list_t *itemList); item_list_t *hddGetObject(int initOnly); int hddLoadModules(void); + +// Load (or confirm) the ATA stack and report residency, evaluating hddLoadModules EXACTLY ONCE. +// A function, not a macro taking the call as its argument: the earlier HDD_LOADMODULES_OK( +// hddLoadModules()) macro double-evaluated on any non-NOERROR first result -- and with the +// retryable-failure change, a FAILED load would have immediately re-run the whole load (second +// dev9 init + second error toast) inside one condition (CodeRabbit review of #241; vetted). +static inline int hddLoadModulesReady(void) +{ + int r = hddLoadModules(); + return r == HDD_LOADMODULES_STATUS_NOERROR || r == HDD_LOADMODULES_STATUS_ALREADYLOADED; +} void hddLoadSupportModules(void); void hddLaunchGame(item_list_t *itemList, int id, config_set_t *configSet); int hddIsPresent(); diff --git a/src/bdmsupport.c b/src/bdmsupport.c index 6f28febe59..5369ae3e91 100644 --- a/src/bdmsupport.c +++ b/src/bdmsupport.c @@ -353,7 +353,7 @@ static void bdmLoadBlockDeviceModules(void) // Only mark loaded on success -- a failure (no HDD/interface) must not // suppress future retries via bdmShouldQueueModuleLoad() (matches the // conditional pattern used for USB/MX4SIO and opl.c's hddLoadModules gate). - if (HDD_LOADMODULES_OK(hddLoadModules())) + if (hddLoadModulesReady()) hddModLoaded = 1; } @@ -1660,7 +1660,7 @@ static int bdmEnsureTransportLoaded(int bdmType) } return iLinkModLoaded; case BDM_TYPE_ATA: - if (!hddModLoaded && HDD_LOADMODULES_OK(hddLoadModules())) + if (!hddModLoaded && hddLoadModulesReady()) hddModLoaded = 1; return hddModLoaded; default: diff --git a/src/opl.c b/src/opl.c index 0a243cf83d..e4d1866cd9 100644 --- a/src/opl.c +++ b/src/opl.c @@ -1313,7 +1313,7 @@ static int checkLoadConfigBDMHDD(int types) int value; // Bounded wait so BDM-on-HDD can be detected without long black-screen stalls. - if (HDD_LOADMODULES_OK(hddLoadModules()) && bdmHDDIsPresent(500)) { + if (hddLoadModulesReady() && bdmHDDIsPresent(500)) { if (bdmFindPartition(path, CONFIG_OPL_FILENAME, 0) || bdmFindPartition(path, CONFIG_OPL_FILENAME_LEGACY, 0)) { configEnd(); configInit(path); @@ -1901,7 +1901,7 @@ static int trySaveConfigBDMHDD(int types) char path[64]; // Bounded wait so save can target BDM-on-HDD without long stalls. - if (HDD_LOADMODULES_OK(hddLoadModules()) && bdmHDDIsPresent(500)) { + if (hddLoadModulesReady() && bdmHDDIsPresent(500)) { if (bdmFindPartition(path, CONFIG_OPL_FILENAME, 1)) { configSetMove(path); return configWriteMulti(types);