diff --git a/REPORT_RIPTOPL_VCD_THEMES_IGR_AUDIT.md b/REPORT_RIPTOPL_VCD_THEMES_IGR_AUDIT.md deleted file mode 100644 index d63e2fce2..000000000 --- a/REPORT_RIPTOPL_VCD_THEMES_IGR_AUDIT.md +++ /dev/null @@ -1,94 +0,0 @@ -# REPORT: Riptopl (Open-PS2-Loader) — Exhaustive Audit of VCDs, Themes, IGR, and BDM/APA HDD Conflicts -**Date**: 2026-07-13 -**Target Audience**: Claude 3.5 Sonnet / Claude 4 Opus -**Context**: Deep-dive analysis of Issue #120 (VCD Info Screens, Screenshots, Theme Freezes, and IGR pathing) along with BDM HDD vs. APA HDD conflicts. - ---- - -## 1. Executive Summary & Narrative -This audit report examines several interrelated issues in the Open-PS2-Loader (OPL) fork, focusing on the Multi-Memory Card Emulator (MMCE) environment and internal Hard Disk Drive (HDD) configurations: -1. **VCD Info Screens and Screenshots (Issue #120)**: Entering the PS1 Info screen displays a persistent spinner, fails to load the second screenshot, and eventually causes the UI to freeze or become non-interactive. -2. **Theme-Related Freezes**: High-resolution 3D themes (e.g., 1080i cover art) experience image jitter and then freeze the OPL GUI. -3. **In-Game Reset (IGR) Paths**: Custom IGR paths (like exit paths pointing to MMCE or internal HDD) fail to boot and fall back to `BOOT.ELF` on the memory card, unless the target is a real/emulated memory card. -4. **HDD Spin-Up & Concurrent Conflicts**: Enabling BDM (for USB) spins up the internal HDD and conflicts with/breaks the native APA HDD driver due to driver-level auto-mounting. - ---- - -## 2. Issues, Root Causes, and Technical Analyses - -### 2.1. PS1 Info Screen Spinner and Missing Screenshots -* **Symptom**: Opening a PS1 game's Info page shows a long loading spinner. Often, only one screenshot loads (the second only loads after exiting/re-entering). Eventually, lists go blank. -* **Root Cause**: Synchronous blocking IO operations on slow/constrained buses (Memory Card, APA HDD, and MMCE) combined with excessive/redundant file probes. -* **Mechanics**: - 1. The PS1 Info page loads screenshots and size stats. PS1 games do not have a standard ISO size, but the system still attempts to scan the device. - 2. `initGameImage()` and related prefetch functions perform synchronous `open()`, `stat()`, and `read()` commands on the UI thread. - 3. Under a flood of small file reads (such as checking multiple cover/screenshot paths), the device's driver or bus controller stalls. Because the read is synchronous on the UI thread, the entire GUI locks up, displaying the loading spinner until the driver recovers or times out. - 4. Repeated stalls/time-outs invalidate texture caches or corrupt the directory listing state, causing the game lists to disappear or show 0 games. - ---- - -### 2.2. Theme-Related Freezes (Jitter → Freeze) -* **Symptom**: Using heavy 3D coverflow themes with large 1080i assets causes jittery graphics followed by a total crash/freeze. -* **Root Cause**: High VRAM/IOP bus thrashing due to synchronous texture loads and aggressive prefetching on the UI thread. -* **Mechanics**: - 1. `prefetchGameImageTexture()` and `prefetchAdjacentGameImages()` in `src/themes.c` attempt to cache covers for adjacent games. - 2. If the theme uses massive high-resolution assets (e.g. 620x400+ PNGs), the UI thread spends too much time loading textures from the device. - 3. This saturates the IOP bus and GPU memory, causing frame rendering to drop (jitter). If the cache is cleared/swapped while the background art worker thread is loading files, pointer corruption/race conditions occur, leading to a hard freeze. - ---- - -### 2.3. PS2 IGR Path Resolution on MMCE / Internal HDD -* **Symptom**: Configuring a custom IGR (Exit) path to a folder on MMCE (`mmce0:/...`) or the internal HDD (`pfs0:/...`) fails, resetting to OSDSYS or memory card `BOOT.ELF`. -* **Root Cause**: Driver absence during the early IGR boot phase. -* **Mechanics**: - 1. During IGR, OPL's EE core handles the pad hook and executes the reset flow in [ee_core/src/padhook.c](file:///c:/Users/natha/Github/Open-PS2-Loader/ee_core/src/padhook.c#L75). - 2. At this stage, the IOP is reset. Only the BIOS `rom0:SIO2MAN` and `rom0:MCMAN` (Memory Card Manager) are loaded. - 3. If the exit path points to `mass:` (USB), `padhook.c` explicitly loads `USBD.IRX` and `USBHDFSD.IRX` from the memory card to enable USB read support. - 4. However, `padhook.c` does **not** load `mmce.irx` (for MMCE) or `ps2atad/ps2hdd/ps2fs` (for internal HDD). Hence, `LoadElf()` cannot parse paths starting with `mmce0:` or `pfs0:` / `hdd0:`, causing the boot to fail and exit to OSDSYS. -* **Resolution/Workaround**: - * **MMCE IGR**: When `gMMCEIGRSlot` is enabled, OPL loads `mmceigr.irx`. Upon IGR trigger, `mmceigr.irx` automatically switches the MMCE card slot to act as a standard memory card (`mc0:` or `mc1:`). - * Therefore, the user **must** configure the IGR path using the standard memory card prefix (e.g., `mc0:/Riptopl/BOOT.ELF` instead of `mmce0:/Riptopl/BOOT.ELF`). Because the card is emulating `mc0:`, the BIOS memory card driver can read it without any extra MMCE drivers! - ---- - -### 2.4. Concurrent Conflict Between APA HDD and BDM (exFAT BDM HDD) Drivers -* **Symptom**: Enabling BDM (for USB) and setting APA HDD to Auto causes the internal HDD to spin up and completely breaks APA HDD game scanning and launch. -* **Root Cause**: Un-arbitrated hardware access on the DEV9/ATA interface because the BDM FATFS module auto-mounts the low-level ATA helper driver. -* **Mechanics**: - 1. APA HDD mode requires `xhdd.irx` (the BDM ATA helper driver) to be loaded. OPL uses its `"xhdd0:"` devctl interface for checks (LBA48 support in `hddIs48bit()`) and settings (UDMA transfer mode in `hddSetTransferMode()`). - 2. `xhdd.irx` registers itself as a block device (`IOP_DT_BLOCK`) with `iomanX`. - 3. Because it is registered as a block device, OPL's FATFS driver (`bdmfs_fatfs.irx`) automatically detects `"xhdd0:"` and mounts it as a `mass` slot (e.g., `mass1:`). - 4. Since BDM is active (scanning for USB devices), OPL calls `fileXioDopen("mass1:/")` on every menu refresh. - 5. Since the drive is actually APA-formatted (not exFAT), the FATFS mount fails. OPL assumes the device is still connecting and retries the mount scan on *every single menu refresh or list update*. - 6. Each retry sends raw sector read commands to the drive via `xhdd_irx`. Concurrently, the APA drivers (`ps2hdd.irx` and `ps2fs_irx`) are also actively accessing the drive. - 7. These un-arbitrated command streams collide on the shared DEV9/ATA interface, causing commands to fail, drive registers to lock up, and completely breaking APA HDD browsing. - ---- - -## 3. Targeted Code-Level Fixes - -### 3.1. Mitigating the BDM / APA HDD Conflict -To prevent BDM from auto-mounting `"xhdd0:"` when BDM HDD is disabled (`gEnableBdmHDD == 0`), we must prevent it from registering as a block device: -1. **Modify `xhdd.c`**: Update the entry point `_start` in [modules/hdd/xhdd/xhdd.c](file:///c:/Users/natha/Github/Open-PS2-Loader/modules/hdd/xhdd/xhdd.c#L112) to parse a `"-nobdm"` command-line argument. -2. If `"-nobdm"` is passed, register the `"xhdd"` device as a character device (`IOP_DT_CHAR`) instead of a block device (`IOP_DT_BLOCK | IOP_DT_FSEXT`): - ```c - if (noBdm) { - xhddDevice.type = IOP_DT_CHAR; // Prevents bdmfs_fatfs from mounting it - } - ``` -3. **Modify `hddsupport.c`**: Update `hddLoadModules()` in [src/hddsupport.c](file:///c:/Users/natha/Github/Open-PS2-Loader/src/hddsupport.c#L219). If BDM HDD is disabled (`gEnableBdmHDD == 0`), pass the `"-nobdm"` argument when loading the module: - ```c - sysLoadModuleBuffer(&xhdd_irx, size_xhdd_irx, 7, "-nobdm"); - ``` - This keeps the `"xhdd0:"` devctl interface fully functional for LBA48/UDMA setting commands, but stops BDM from scanning it, resolving the conflict. - -### 3.2. Mitigating VCD Art and Theme Stalls -1. **Add Art-Loading Throttling**: Modify `src/themes.c` to enforce a conservative prefetch limit when the active device is a memory card, APA, or MMCE card (prefix check). Skip or reduce prefetching of adjacent game images. -2. **Asynchronous Art Loader**: Refactor texture loading to submit open/read jobs to OPL's background IO thread (worker queue) rather than performing them synchronously on the GUI thread. If the texture is not cached, render a placeholder image instantly and let the IO thread load the art in the background. - ---- - -## 4. Verification and Validation Plan -1. **Conflict Test**: Set BDM to Auto (USB active) and APA HDD to Auto. Verify that APA HDD game lists populate instantly and games boot without freezing. -2. **IGR Test**: Set IGR Path to `mc0:/Riptopl/BOOT.ELF` and trigger IGR from an MMCE game. Verify it successfully reboots into the Riptopl launcher. -3. **Stress Test**: Cycle through PS1 Info screens rapidly 50 times with a custom theme. Verify the GUI does not freeze and game lists remain intact. diff --git a/delivered/eth.png b/delivered/eth.png deleted file mode 100644 index a61603d13..000000000 Binary files a/delivered/eth.png and /dev/null differ diff --git a/delivered/fav.png b/delivered/fav.png deleted file mode 100644 index 2f3078365..000000000 Binary files a/delivered/fav.png and /dev/null differ diff --git a/delivered/hdd.png b/delivered/hdd.png deleted file mode 100644 index 91e6c6c65..000000000 Binary files a/delivered/hdd.png and /dev/null differ diff --git a/delivered/hdd_bd.png b/delivered/hdd_bd.png deleted file mode 100644 index 983673c5c..000000000 Binary files a/delivered/hdd_bd.png and /dev/null differ diff --git a/delivered/ilk_bd.png b/delivered/ilk_bd.png deleted file mode 100644 index 1037ead06..000000000 Binary files a/delivered/ilk_bd.png and /dev/null differ diff --git a/delivered/m4s_bd.png b/delivered/m4s_bd.png deleted file mode 100644 index b55586c1c..000000000 Binary files a/delivered/m4s_bd.png and /dev/null differ diff --git a/delivered/mmce.png b/delivered/mmce.png deleted file mode 100644 index 27b1a4a02..000000000 Binary files a/delivered/mmce.png and /dev/null differ diff --git a/delivered/udp_bd.png b/delivered/udp_bd.png deleted file mode 100644 index 1d3328e6a..000000000 Binary files a/delivered/udp_bd.png and /dev/null differ diff --git a/delivered/udp_fs.png b/delivered/udp_fs.png deleted file mode 100644 index c4c0961cf..000000000 Binary files a/delivered/udp_fs.png and /dev/null differ diff --git a/delivered/usb.png b/delivered/usb.png deleted file mode 100644 index f3e309e11..000000000 Binary files a/delivered/usb.png and /dev/null differ diff --git a/delivered/usb_bd.png b/delivered/usb_bd.png deleted file mode 100644 index d04622f00..000000000 Binary files a/delivered/usb_bd.png and /dev/null differ diff --git a/include/bdmsupport.h b/include/bdmsupport.h index e0c6b6446..7a444395b 100644 --- a/include/bdmsupport.h +++ b/include/bdmsupport.h @@ -65,9 +65,7 @@ typedef struct } bdm_device_data_t; void bdmLoadModules(void); -void bdmResetModuleState(void); void bdmLaunchGame(item_list_t *itemList, int id, config_set_t *configSet); -int bdmSupportIsATA(item_list_t *support); void bdmInitSemaphore(); void bdmEnumerateDevices(); diff --git a/include/hddsupport.h b/include/hddsupport.h index 7e886cd08..0304beb5e 100644 --- a/include/hddsupport.h +++ b/include/hddsupport.h @@ -91,8 +91,7 @@ void hddVcdInvalidateCache(void); void hddInit(item_list_t *itemList); item_list_t *hddGetObject(int initOnly); int hddLoadModules(void); -int hddLoadSupportModules(void); -void hddResetModuleState(void); +void hddLoadSupportModules(void); void hddLaunchGame(item_list_t *itemList, int id, config_set_t *configSet); int hddIsPresent(); diff --git a/include/opl.h b/include/opl.h index 5116dea2f..123bac2c1 100644 --- a/include/opl.h +++ b/include/opl.h @@ -216,6 +216,7 @@ extern int gOSDLanguageSource; extern int showCfgPopup; extern int showNetDhcpPopup; // boot toast: UDP transport selected while IP Type = DHCP (needs static IP) +extern int showHddReconcilePopup; // boot toast: APA + exFAT(BDM) HDD were both enabled -- one was auto-disabled (#154) #ifdef IGS #define IGS_VERSION "0.1" @@ -257,7 +258,6 @@ extern int gNeutrinoElfArg; // opt-in (settings key only, no UI): auto-e extern char gPopstarterPath[256]; // custom POPSTARTER.ELF path (used only when gPopstarterDevice == POPS_DEV_CUSTOM) extern char gBootDir[256]; // boot directory (cwd) OPL launched from, e.g. "mass0:/APPS"; "" if undeterminable extern int gDeinitTerminal; // 1 while deinit() runs for exit/poweroff, 0 for a game/app LAUNCH teardown. -extern int gDeinitAtaSelected; // 1 when the selected launch backend still needs the internal ATA drive. // Launch teardown must NOT power shared buses down (dev9: the post-deinit // POPSTARTER.ELF read comes off the ATA-backed massN: mount). // POPSTARTER.ELF Device picker: where PS1 VCD launches load POPS/POPSTARTER.ELF from. Default tries the diff --git a/src/bdmsupport.c b/src/bdmsupport.c index 38925423e..68c6e94be 100644 --- a/src/bdmsupport.c +++ b/src/bdmsupport.c @@ -275,15 +275,6 @@ int bdmSupportIsUDPBD(item_list_t *support) return ((bdm_device_data_t *)support->priv)->bdmDeviceType == BDM_TYPE_UDPBD; } -int bdmSupportIsATA(item_list_t *support) -{ - if (support == NULL || support->priv == NULL) - return 0; - if (support->mode < BDM_MODE || support->mode > BDM_MODE_LAST) - return 0; - return ((bdm_device_data_t *)support->priv)->bdmDeviceType == BDM_TYPE_ATA; -} - static void bdmEventHandler(void *packet, void *opt) { BdmGeneration++; @@ -418,20 +409,6 @@ void bdmLoadModules(void) LOG("BDMSUPPORT Modules loaded\n"); } -// The optional transport flags describe IOP residency. An in-process sysReset removes those IRXs -// while the EE process and these statics survive, so clear them only at that explicit reset boundary. -void bdmResetModuleState(void) -{ - iUSBModLoaded = 0; - iLinkModLoaded = 0; - iLinkManModLoaded = 0; - ieee1394ModLoaded = 0; - mx4sioModLoaded = 0; - hddModLoaded = 0; - udpbdModLoaded = 0; - bdmDeviceModeStarted = 0; -} - static void bdmInit(item_list_t *itemList) { LOG("BDMSUPPORT Init\n"); @@ -1357,17 +1334,11 @@ static void bdmShutdown(item_list_t *itemList) bdm_device_data_t *pDeviceData = (bdm_device_data_t *)itemList->priv; snprintf(path, sizeof(path), "mass%d:", itemList->mode); - const char *devicePath = (pDeviceData != NULL && pDeviceData->bdmDeviceRoot[0] != '\0') ? pDeviceData->bdmDeviceRoot : path; - int deviceType = (pDeviceData != NULL) ? pDeviceData->bdmDeviceType : BDM_TYPE_UNKNOWN; // As required by some (typically 2.5") HDDs, issue the SCSI STOP UNIT command to avoid causing an emergency park. // pDeviceData may be NULL here (shutdown without init, or a second deinit pass after priv was freed below), // so guard the dereference and fall back to the constructed mass%d: path. - // ATA_BD and APA share the internal drive. Suppress STOP only when the selected launch backend - // actually needs ATA; unrelated USB/MMCE/network launches retain the orderly park behavior. - // Use the identity already cached by enumeration -- never add blocking mount probes to teardown. - if (!gDeinitAtaSelected || deviceType != BDM_TYPE_ATA) - fileXioDevctl(devicePath, USBMASS_DEVCTL_STOP_ALL, NULL, 0, NULL, 0); + fileXioDevctl((pDeviceData != NULL && pDeviceData->bdmDeviceRoot[0] != '\0') ? pDeviceData->bdmDeviceRoot : path, USBMASS_DEVCTL_STOP_ALL, NULL, 0, NULL, 0); if (itemList->enabled && pDeviceData != NULL) { LOG("BDMSUPPORT Shutdown free data\n"); diff --git a/src/gui.c b/src/gui.c index 1c08d184d..e2c94cfbf 100644 --- a/src/gui.c +++ b/src/gui.c @@ -302,7 +302,7 @@ static void guiShowNotifications(void) int y = 10; int yadd = 35; - if (showPartPopup || showThmPopup || showLngPopup || showCfgPopup || showNetDhcpPopup) { + if (showPartPopup || showThmPopup || showLngPopup || showCfgPopup || showNetDhcpPopup || showHddReconcilePopup) { if (!popupTimer) { popupTimer = clock() + 5000 * (CLOCKS_PER_SEC / 1000); sfxPlay(SFX_MESSAGE); @@ -351,11 +351,18 @@ static void guiShowNotifications(void) y += yadd; } + // One-time notice set at config load (#154): APA + exFAT(BDM) internal HDD were both + // enabled and one was auto-disabled. Rendered here (not toasted from _loadConfig) so _l() + // resolves AFTER the language pack loads -- Gemini review of #167. + if (showHddReconcilePopup) + guiRenderNotifications(_l(_STR_HDD_BACKEND_RECONCILED), y); + if (clock() >= popupTimer) { guiResetNotifications(); showPartPopup = 0; showCfgPopup = 0; showNetDhcpPopup = 0; + showHddReconcilePopup = 0; } } } @@ -1224,6 +1231,14 @@ void guiShowNetConfig(void) static int guiDeviceUpdater(int modified) { if (modified) { + int hddMode, bdmHdd; + diaGetInt(diaDeviceConfig, CFG_HDDMODE, &hddMode); + diaGetInt(diaDeviceConfig, CFG_ENABLEBDMHDD, &bdmHdd); + // BDM HDD (GPT/MBR) and the APA HDD mode are mutually exclusive; keep the + // two interlocked live now that both live on the same page. + diaSetEnabled(diaDeviceConfig, CFG_HDDMODE, !bdmHdd); + diaSetEnabled(diaDeviceConfig, CFG_ENABLEBDMHDD, hddMode == 0); + // Network 3-row live logic. Row 1 Start (Off/Manual/Auto), Row 2 Protocol (SMB/UDPFS/UDPBD), // Row 3 Access (Files/IMG). While Start=Off, grey Protocol + Access. Lock Access to Files for // SMB and to IMG for UDPBD (only UDPFS offers the free toggle) -- snap the value so a stale IMG @@ -1271,11 +1286,13 @@ void guiShowDeviceConfig(void) diaSetInt(diaDeviceConfig, CFG_APPMODE, gAPPStartMode); diaSetInt(diaDeviceConfig, CFG_FAVMODE, gFAVStartMode); - // Block devices. ATA-BDM and APA/PFS intentionally coexist on the shared ATA stack. + // Block devices (inlined; interlocked with the APA HDD mode) diaSetInt(diaDeviceConfig, CFG_ENABLEUSB, gEnableUSB); diaSetInt(diaDeviceConfig, CFG_ENABLEILK, gEnableILK); diaSetInt(diaDeviceConfig, CFG_ENABLEMX4SIO, gEnableMX4SIO); diaSetInt(diaDeviceConfig, CFG_ENABLEBDMHDD, gEnableBdmHDD); + diaSetEnabled(diaDeviceConfig, CFG_ENABLEBDMHDD, !gHDDStartMode); + diaSetEnabled(diaDeviceConfig, CFG_HDDMODE, !gEnableBdmHDD); // Network: 3 orthogonal rows seeded from the authoritative gNetworkProtocol + gNetStartMode. // Row 1 Start: Off/Manual/Auto == gNetStartMode (START_MODE_*) // Row 2 Protocol: SMB(0)/UDPFS(1)/UDPBD(2) -- OFF and UDPFSBD both collapse to their protocol diff --git a/src/hddsupport.c b/src/hddsupport.c index 9d0346f03..f93a59bf3 100644 --- a/src/hddsupport.c +++ b/src/hddsupport.c @@ -19,7 +19,6 @@ #define NEWLIB_PORT_AWARE #include // fileXioFormat, fileXioMount, fileXioUmount, fileXioDevctl #include // FIO_MT_RDWR -#include // DelayThread -- R3Z ATA_BD settle before APA/PFS starts #include @@ -33,18 +32,9 @@ extern u8 IOBuffer[2048]; static unsigned char hddForceUpdate = 0; static unsigned char hddHDProKitDetected = 0; -static volatile unsigned char hddModulesLoading = 0; +static unsigned char hddModulesLoadCount = 0; static unsigned char hddModulesLoaded = 0; -static unsigned char hddApaModuleLoaded = 0; static unsigned char hddSupportModulesLoaded = 0; -static unsigned char hddDev9RefHeld = 0; - -enum hdd_apa_state { - HDD_APA_PROBING = 0, - HDD_APA_READY, - HDD_APA_UNAVAILABLE, -}; -static volatile unsigned char hddApaState = HDD_APA_PROBING; static char *hddPrefix = "pfs0:"; static hdl_games_list_t hddGames; @@ -62,27 +52,10 @@ static item_list_t hddGameList; static int hddLoadGameListCache(hdl_games_list_t *cache); static int hddUpdateGameListCache(hdl_games_list_t *cache, hdl_games_list_t *game_list); -// EE-side residency bookkeeping must be cleared whenever the IOP is rebooted in-process. Ordinary -// menu cleanup deliberately keeps these flags because the IRXs are still resident; sysReset is the -// point where that physical state actually disappears. -void hddResetModuleState(void) -{ - hddModulesLoading = 0; - hddModulesLoaded = 0; - hddApaModuleLoaded = 0; - hddSupportModulesLoaded = 0; - hddDev9RefHeld = 0; - hddApaState = HDD_APA_PROBING; -} - static void hddInitModules(void) { - if (hddLoadModules() < 0) { - hddApaState = HDD_APA_UNAVAILABLE; - return; - } - if (!hddLoadSupportModules()) - return; + hddLoadModules(); + hddLoadSupportModules(); // update Themes char path[256]; @@ -215,78 +188,104 @@ static int hddCreateOPLPartition(const char *name) int hddLoadModules(void) { int retLoadModule; - int wait; + int retStatus = HDD_LOADMODULES_STATUS_UNK; -retry: - LOG("HDDSUPPORT LoadModules loaded=%d loading=%d\n", hddModulesLoaded, hddModulesLoading); + LOG("HDDSUPPORT LoadModules %d\n", hddModulesLoadCount); - // R3Z-style physical residency: loading the shared ATA stack is idempotent. Logical APA and - // ATA-BDM users do not each take a fake reference to the same resident IRXs. if (hddModulesLoaded) - return HDD_LOADMODULES_STATUS_ALREADYLOADED; - if (hddModulesLoading) { - // Another caller is loading the one shared physical stack. R3Z makes its loaders - // idempotent; wait for that result instead of letting a logical backend run ahead of it. - for (wait = 0; wait < 250 && hddModulesLoading; wait++) - DelayThread(10 * 1000); - if (hddModulesLoaded) - return HDD_LOADMODULES_STATUS_ALREADYLOADED; - if (hddModulesLoading) - return HDD_LOADMODULES_STATUS_ERROR; - goto retry; // the other attempt failed cleanly; this caller gets one fresh attempt - } - hddModulesLoading = 1; + retStatus = HDD_LOADMODULES_STATUS_ALREADYLOADED; - // DEV9 must be loaded, as HDD.IRX depends on it. Keep this support's one ownership reference - // across retryable ATAD failures: DDIOC_OFF cannot be re-armed within the same IOP session. - if (!hddDev9RefHeld) { + if (hddModulesLoadCount == 0) { + // Increment the load count as soon as possible to prevent thread scheduling from allowing another thread to + // call into here and try to double load modules. + hddModulesLoadCount = 1; + + // DEV9 must be loaded, as HDD.IRX depends on it. Even if not required by the I/F (i.e. HDPro) sysInitDev9(); - hddDev9RefHeld = 1; - } - // try to detect HD Pro Kit (not the connected HDD), - // if detected it loads the specific ATAD module - hddHDProKitDetected = hddCheckHDProKit(); - if (hddHDProKitDetected) { - LOG("[ATAD_HDPRO]:\n"); - retLoadModule = sysLoadModuleBuffer(&hdpro_atad_irx, size_hdpro_atad_irx, 0, NULL); - LOG("[XHDD]:\n"); - sysLoadModuleBuffer(&xhdd_irx, size_xhdd_irx, 6, "-hdpro"); + // try to detect HD Pro Kit (not the connected HDD), + // if detected it loads the specific ATAD module + hddHDProKitDetected = hddCheckHDProKit(); + if (hddHDProKitDetected) { + LOG("[ATAD_HDPRO]:\n"); + retLoadModule = sysLoadModuleBuffer(&hdpro_atad_irx, size_hdpro_atad_irx, 0, NULL); + LOG("[XHDD]:\n"); + sysLoadModuleBuffer(&xhdd_irx, size_xhdd_irx, 6, "-hdpro"); + } else { + LOG("[BDM]:\n"); + sysLoadModuleBuffer(&bdm_irx, size_bdm_irx, 0, NULL); + LOG("[ATAD]:\n"); + retLoadModule = sysLoadModuleBuffer(&ps2atad_irx, size_ps2atad_irx, 0, NULL); + LOG("[XHDD]:\n"); + sysLoadModuleBuffer(&xhdd_irx, size_xhdd_irx, 0, NULL); + } + + if (retLoadModule < 0) { + LOG("HDD: No HardDisk Drive detected.\n"); + setErrorMessageWithCode(_STR_HDD_NOT_CONNECTED_ERROR, ERROR_HDD_IF_NOT_DETECTED); + retStatus = HDD_LOADMODULES_STATUS_ERROR; + } else { + retStatus = HDD_LOADMODULES_STATUS_NOERROR; + hddModulesLoaded = 1; + } } else { - LOG("[BDM]:\n"); - sysLoadModuleBuffer(&bdm_irx, size_bdm_irx, 0, NULL); - LOG("[ATAD]:\n"); - retLoadModule = sysLoadModuleBuffer(&ps2atad_irx, size_ps2atad_irx, 0, NULL); - // wLaunchELF-R3Z deliberately leaves a one-second settle window after ATA_BD - // becomes resident and before the APA stack touches it. Both BDM-ATA and APA - // share this same physical driver, so do this once at its first load. - if (retLoadModule >= 0) - DelayThread(1000 * 1000); - LOG("[XHDD]:\n"); - sysLoadModuleBuffer(&xhdd_irx, size_xhdd_irx, 0, NULL); + hddModulesLoadCount++; + if (!hddModulesLoaded) + retStatus = HDD_LOADMODULES_STATUS_BUSYLOADING; } - if (retLoadModule < 0) { - // Leave the attempt retryable without powering DEV9 down. sysShutdownDev9's DDIOC_OFF is a - // terminal hardware stop, not a reversible reference release within the current IOP session. - hddModulesLoading = 0; - LOG("HDD: No HardDisk Drive detected.\n"); - setErrorMessageWithCode(_STR_HDD_NOT_CONNECTED_ERROR, ERROR_HDD_IF_NOT_DETECTED); - return HDD_LOADMODULES_STATUS_ERROR; + LOG("HDDSUPPORT LoadModules done\n"); + return retStatus; +} + +// Returns 1 for MBR/GPT, 0 for APA, and -1 if an error occured +int hddDetectNonSonyFileSystem() +{ + int result = -1; + // Allocate memory for storing data for the first two sectors. + u8 *pSectorData = (u8 *)malloc(512 * 2); + if (pSectorData == NULL) { + LOG("hddDetectNonSonyFileSystem: failed to allocate scratch memory\n"); + return -1; } - // Publish success before clearing the in-progress flag. A waiter that observes loading == 0 - // must also observe the resident state, or it could race into a second module load. - hddModulesLoaded = 1; - hddModulesLoading = 0; + // Trying to load the APA/PFS irx modules when a non-sony formatted HDD is connected (ie: MBR/GPT w/ exFAT) runs + // the risk of corrupting the HDD. To avoid that get the first two sectors and perform some sanity checks. If + // we reasonably suspect the disk is not APA formatted bail out from loading the sony fs irx modules. + result = fileXioDevctl("xhdd0:", ATA_DEVCTL_READ_PARTITION_SECTOR, NULL, 0, pSectorData, 512 * 2); + if (result < 0) { + LOG("hddDetectNonSonyFileSystem: failed to read data from hdd %d\n", result); + free(pSectorData); + return -1; + } - LOG("HDDSUPPORT LoadModules done\n"); - return HDD_LOADMODULES_STATUS_NOERROR; + // Check for MBR signature. + if (pSectorData[0x1FE] == 0x55 && pSectorData[0x1FF] == 0xAA) { + // Found MBR partition type. + LOG("hddDetectNonSonyFileSystem: found MBR partition data\n"); + result = 1; + } else if (strncmp((const char *)&pSectorData[0x200], "EFI PART", 8) == 0) { + // Found GPT partition type. + LOG("hddDetectNonSonyFileSystem: found GPT partition data\n"); + result = 1; + } else if (strncmp((const char *)&pSectorData[4], "APA", 3) == 0) { + // Found APA partition type. + LOG("hddDetectNonSonyFileSystem: found APA partition data\n"); + result = 0; + } else { + // Even though we didn't find evidence of non-APA partition data, if we load the APA irx module + // it will write to the drive and potentially corrupt any data that might be there. + LOG("hddDetectNonSonyFileSystem: partition data not recognized\n"); + result = -1; + } + + // Cleanup and return. + free(pSectorData); + return result; } -int hddLoadSupportModules(void) +void hddLoadSupportModules(void) { - int ret; static char hddarg[] = "-o" "\0" "4" @@ -312,83 +311,64 @@ int hddLoadSupportModules(void) LOG("HDDSUPPORT LoadSupportModules\n"); + // 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) { + // Drive is MBR/GPT style, or unknown, bail out or risk corrupting the drive. + LOG("HDDSUPPORT LoadSupportModules bailing out early...\n"); + return; + } + if (!hddSupportModulesLoaded) { - // ps2hdd-osd remains resident even when it classifies the connected disk as non-APA. - // Track that IRX independently from the complete APA+PFS stack so a later probe never - // attempts to load the same module twice. - if (!hddApaModuleLoaded) { - LOG("[HDD]:\n"); - ret = sysLoadModuleBuffer(&ps2hdd_irx, size_ps2hdd_irx, sizeof(hddarg), hddarg); - if (ret < 0) { - hddApaState = HDD_APA_UNAVAILABLE; - LOG("HDD: No HardDisk Drive detected.\n"); - setErrorMessageWithCode(_STR_HDD_NOT_CONNECTED_ERROR, ERROR_HDD_MODULE_HDD_FAILURE); - return 0; - } - hddApaModuleLoaded = 1; + LOG("[HDD]:\n"); + int ret = sysLoadModuleBuffer(&ps2hdd_irx, size_ps2hdd_irx, sizeof(hddarg), hddarg); + if (ret < 0) { + LOG("HDD: No HardDisk Drive detected.\n"); + setErrorMessageWithCode(_STR_HDD_NOT_CONNECTED_ERROR, ERROR_HDD_MODULE_HDD_FAILURE); + return; } - // R3Z coexistence model: ATA_BD remains resident for FAT/exFAT while the direct-ATAD - // ps2hdd-osd driver independently validates APA. Its init is read-only when the disk is - // not APA; require the authoritative formatted status before PS2FS is loaded or any PFS - // partition can be mounted/created. Do not second-guess it with generic MBR/GPT bytes. - int hddStatus = hddCheck(); - if (hddStatus != 0) { - hddApaState = HDD_APA_UNAVAILABLE; - if (hddStatus == 1) { - LOG("HDD: HardDisk Drive is not APA formatted.\n"); - // Expected when both logical backends are enabled on an exFAT disk: BDM-ATA owns - // the page, while APA simply stays unpublished. Only report an error when APA was - // the sole internal-HDD backend requested. - if (!gEnableBdmHDD) - setErrorMessageWithCode(_STR_HDD_NOT_FORMATTED_ERROR, ERROR_HDD_NOT_DETECTED); - } else { - LOG("HDD: No usable APA HardDisk Drive detected.\n"); - setErrorMessageWithCode(_STR_HDD_NOT_CONNECTED_ERROR, ERROR_HDD_NOT_DETECTED); - } - return 0; + // Check if a HDD unit is connected + if (hddCheck() < 0) { + LOG("HDD: No HardDisk Drive detected.\n"); + setErrorMessageWithCode(_STR_HDD_NOT_CONNECTED_ERROR, ERROR_HDD_NOT_DETECTED); + return; } LOG("[PS2FS]:\n"); ret = sysLoadModuleBuffer(&ps2fs_irx, size_ps2fs_irx, sizeof(pfsarg), pfsarg); if (ret < 0) { - hddApaState = HDD_APA_UNAVAILABLE; LOG("HDD: HardDisk Drive not formatted (PFS).\n"); setErrorMessageWithCode(_STR_HDD_NOT_FORMATTED_ERROR, ERROR_HDD_MODULE_PFS_FAILURE); - return 0; + return; } hddSupportModulesLoaded = 1; LOG("HDDSUPPORT modules loaded\n"); - } - - hddApaState = HDD_APA_READY; - if (gOPLPart[0] == '\0') - hddFindOPLPartition(); + if (gOPLPart[0] == '\0') + hddFindOPLPartition(); - fileXioUmount(hddPrefix); + fileXioUmount(hddPrefix); - ret = fileXioMount(hddPrefix, gOPLPart, FIO_MT_RDWR); - if (ret == -ENOENT) { - // Attempt to create the partition. - if ((hddCreateOPLPartition(gOPLPart)) >= 0) - fileXioMount(hddPrefix, gOPLPart, FIO_MT_RDWR); - } + ret = fileXioMount(hddPrefix, gOPLPart, FIO_MT_RDWR); + if (ret == -ENOENT) { + // Attempt to create the partition. + if ((hddCreateOPLPartition(gOPLPart)) >= 0) + fileXioMount(hddPrefix, gOPLPart, FIO_MT_RDWR); + } - if (gOPLPart[5] != '+') { - hddCheckOPLFolder(hddPrefix); - gHDDPrefix = "pfs0:OPL/"; + if (gOPLPart[5] != '+') { + hddCheckOPLFolder(hddPrefix); + gHDDPrefix = "pfs0:OPL/"; + } } - - return 1; } void hddInit(item_list_t *itemList) { LOG("HDDSUPPORT Init\n"); hddForceUpdate = 0; // Use cache at initial startup. - hddApaState = hddSupportModulesLoaded ? HDD_APA_READY : HDD_APA_PROBING; configGetInt(configGetByType(CONFIG_OPL), "hdd_frames_delay", &hddGameList.delay); ioPutRequest(IO_CUSTOM_SIMPLEACTION, &hddInitModules); hddGameList.enabled = 1; @@ -560,17 +540,6 @@ static int hddBuildVcdGameList(void) static int hddNeedsUpdate(item_list_t *itemList) { /* Auto refresh is disabled by setting HDD_MODE_UPDATE_DELAY to MENU_UPD_DELAY_NOUPDATE, within hddsupport.h. Hence any update request would be issued by the user, which should be taken as an explicit request to re-scan the HDD. */ - // Module initialization and this menu update share the IO FIFO, so the authoritative APA result - // is ready before the first scan. A non-APA disk belongs to ATA-BDM: withhold this page instead - // of publishing the same empty ghost tab that caused the original regression report. - if (hddApaState != HDD_APA_READY) { - if (hddApaState == HDD_APA_UNAVAILABLE && itemList->owner != NULL) - ((opl_io_module_t *)itemList->owner)->menuItem.visible = 0; - return 0; - } - if (itemList->owner != NULL) - ((opl_io_module_t *)itemList->owner)->menuItem.visible = 1; - if (vcdConsumeDirty(itemList->mode)) return 1; // L3 toggle / default-view change -> rebuild the submenu (the ARRAY may be cached) if (vcdViewActive(itemList->mode)) @@ -580,9 +549,6 @@ static int hddNeedsUpdate(item_list_t *itemList) static int hddUpdateGameList(item_list_t *itemList) { - if (hddApaState != HDD_APA_READY) - return 0; - if (vcdViewActive(itemList->mode)) // Reuse the session's built list on view flips; hddBuildVcdGameList runs only when never // built, invalidated (first-disc-only change), or freed by teardown (hddFreeVcdGameList). @@ -1170,10 +1136,11 @@ static void hddCleanUp(item_list_t *itemList, int exception) fileXioUmount(hddPrefix); } - // Close logical PFS state, but keep the physical IRX residency flag. IOP modules cannot be - // unloaded here; a later settings re-init reuses them and remounts below, matching R3Z. + // UI may have loaded modules outside of HDD mode, so deinitialize regardless of the enabled status. if (hddSupportModulesLoaded) { fileXioDevctl("pfs:", PDIOC_CLOSEALL, NULL, 0, NULL, 0); + + hddSupportModulesLoaded = 0; } } @@ -1194,24 +1161,32 @@ static void hddShutdown(item_list_t *itemList) fileXioUmount(hddPrefix); } - // Close logical PFS state. Physical IRXs stay resident until the launch/exit IOP reset. + // UI may have loaded modules outside of HDD mode, so deinitialize regardless of the enabled status. if (hddSupportModulesLoaded) { /* Close all files */ fileXioDevctl("pfs:", PDIOC_CLOSEALL, NULL, 0, NULL, 0); + + hddSupportModulesLoaded = 0; } - if (hddModulesLoaded) { - // Park the internal drive when the selected launch does not use it. Keep it live only for - // APA or ATA-BDM handoff; their loaders still need the shared ATA stack after menu teardown. - if (!gDeinitAtaSelected) + if (hddModulesLoadCount > 0) { + hddModulesLoadCount -= 1; + if (hddModulesLoadCount == 0) { + // DEV9 will remain active if ETH is in use, so put the HDD in IDLE state. + // The HDD should still enter standby state after 21 minutes & 15 seconds, as per the ATAD defaults. hddSetIdleImmediate(); - } + } - // Release this support's single physical DEV9 ownership on terminal exit/poweroff even if ATAD - // never became resident. Game/app handoffs reset the IOP themselves after reading launch assets. - if (gDeinitTerminal && hddDev9RefHeld) { - sysShutdownDev9(); - hddDev9RefHeld = 0; + // Only shut down dev9 from here, if it was initialized from here before -- and only on a + // TERMINAL teardown (exit/poweroff). On the launch path this shutdown runs for every + // non-selected page, and powering DEV9 off here kills the ATA bus BEFORE bdmLaunchVcd's + // post-deinit POPSTARTER.ELF read from the ATA-backed massN: mount -- the elf-loader then + // returns into deinit'd OPL: the 4236edf6-class black-screen freeze (PCSX2 masks it; its + // emulated DEV9 power-off is inert). ee_core/POPSTARTER reset the IOP right after, so the + // launch path needs no power-off. Note the refcount asymmetry this also softens: N + // hddLoadModules calls take ONE dev9 reference, but every hddShutdown used to drop it. + if (gDeinitTerminal) + sysShutdownDev9(); } } diff --git a/src/opl.c b/src/opl.c index 7c8276e44..18b9efc2f 100644 --- a/src/opl.c +++ b/src/opl.c @@ -195,7 +195,8 @@ int gFadeDelay; int toggleSfx; int showCfgPopup; // Boot toast (rendered by guiShowNotifications alongside showCfgPopup): -int showNetDhcpPopup; // a UDP transport is selected but IP Type is DHCP -- ministack needs a static IP +int showNetDhcpPopup; // a UDP transport is selected but IP Type is DHCP -- ministack needs a static IP +int showHddReconcilePopup; // APA + exFAT(BDM) HDD were both enabled -- one was auto-disabled at load (#154) #ifdef PADEMU int gEnablePadEmu; int gPadEmuSettings; @@ -1333,8 +1334,8 @@ static int checkLoadConfigHDD(int types) int value; char path[64]; - if (hddLoadModules() < 0 || !hddLoadSupportModules()) - return 0; + hddLoadModules(); + hddLoadSupportModules(); snprintf(path, sizeof(path), "%s%s", gHDDPrefix, CONFIG_OPL_FILENAME); value = open(path, O_RDONLY); @@ -1687,6 +1688,26 @@ static void _loadConfig() configGetInt(configOPL, CONFIG_OPL_ENABLE_ILINK, &gEnableILK); configGetInt(configOPL, CONFIG_OPL_ENABLE_MX4SIO, &gEnableMX4SIO); configGetInt(configOPL, CONFIG_OPL_ENABLE_BDMHDD, &gEnableBdmHDD); + // #120 audit F-12: APA (gHDDStartMode) and BDM-ATA (gEnableBdmHDD) are mutually exclusive by + // design, but the Device-Settings interlock only guards values changed THROUGH the dialog. A + // legacy, hand-edited or cross-version config can load both, and then BOTH internal-HDD + // stacks initialize against the one drive. Normalize at load: the backend matching the boot + // device wins; otherwise the APA start mode (the older, more deliberate setting) is kept. + // configSetInt the loser so the next save persists the reconciled pair. + if (gEnableBdmHDD && gHDDStartMode != START_MODE_DISABLED) { + if (gBootDirBdmType == BDM_TYPE_ATA) + gHDDStartMode = START_MODE_DISABLED; + else + gEnableBdmHDD = 0; + configSetInt(configOPL, CONFIG_OPL_HDD_MODE, gHDDStartMode); + configSetInt(configOPL, CONFIG_OPL_ENABLE_BDMHDD, gEnableBdmHDD); + // #154 forensics: this reconciliation was SILENT -- a user whose internal-exFAT (or + // APA) page vanished after a hand-edit/cross-version config had no clue why. Flag it + // for the notification popup (same pattern as showNetDhcpPopup): the render site + // resolves _l() per frame, so the message localizes correctly even though the + // language pack loads AFTER this point (applyConfig at the end of _loadConfig). + showHddReconcilePopup = 1; + } int udpbdKeyPresent = configGetInt(configOPL, CONFIG_OPL_ENABLE_UDPBD, &gEnableUDPBD); configGetInt(configOPL, CONFIG_OPL_NET_BOOT_PROTOCOL, &gNetBootProtocol); // Unified network-protocol selector (single SMAP NIC -> at most one transport per session). @@ -1778,6 +1799,12 @@ static void _loadConfig() if (gBootDirBdmType == BDM_TYPE_ATA && !gEnableBdmHDD) { gEnableBdmHDD = 1; configSetInt(configGetByType(CONFIG_OPL), CONFIG_OPL_ENABLE_BDMHDD, gEnableBdmHDD); + // #120 audit F-12: don't leave a loaded APA start mode fighting the just-enabled BDM-ATA + // backend (the drive we booted from is exFAT, so APA is definitionally wrong for it). + if (gHDDStartMode != START_MODE_DISABLED) { + gHDDStartMode = START_MODE_DISABLED; + configSetInt(configGetByType(CONFIG_OPL), CONFIG_OPL_HDD_MODE, gHDDStartMode); + } } } @@ -2625,8 +2652,6 @@ void handleLwnbdSrv() static void reset(void) { sysReset(); - hddResetModuleState(); - bdmResetModuleState(); mcInit(MC_TYPE_XMC); } @@ -2661,14 +2686,6 @@ static void moduleCleanup(opl_io_module_t *mod, int exception, int modeSelected, // POPSTARTER.ELF read from the ATA-backed massN: mount still works; ee_core/POPSTARTER reset the IOP // themselves, so skipping the power-off on launches leaks nothing). int gDeinitTerminal = 0; -int gDeinitAtaSelected = 0; - -static int deinitModeUsesAta(int mode) -{ - if (mode == HDD_MODE) - return 1; - return mode >= 0 && mode < MODE_COUNT && bdmSupportIsATA(list_support[mode].support); -} void deinit(int exception, int modeSelected) { @@ -2678,7 +2695,6 @@ void deinit(int exception, int modeSelected) void deinitEx(int exception, int modeSelected, int modeSelected2) { gDeinitTerminal = (modeSelected == IO_MODE_SELECTED_ALL || modeSelected == IO_MODE_SELECTED_NONE); - gDeinitAtaSelected = deinitModeUsesAta(modeSelected) || deinitModeUsesAta(modeSelected2); /* Menu rumble (#172): kill the actuators FIRST, before anything below can block. Every launch and * exit path funnels through here, and closing the pad ports later does NOT clear the motors -- so a