Fix D-10: Resolve embedded loader memory wipe and SIF exit crash#395
Fix D-10: Resolve embedded loader memory wipe and SIF exit crash#395NathanNeurotic wants to merge 5 commits intoBETA-10-playfrom
Conversation
- Fix `wipeUserMem` to correctly preserve the upper 1MB of system memory (`GetMemorySize() - 0x100000`). Wiping this block resulted in destroyed kernel/RPC structures. - Remove the unsafe `SifExitCmd()` call after the IOP reset. Calling `SifExitCmd()` immediately after `SifIopReset()` hangs the EE because it attempts to talk to the wiped IOP over SIF. - Update tracking markdown documentation (README, STATE, ROADMAP, DECISIONS, QA_REGRESSION_MATRIX) to reflect this resolution. Co-authored-by: NathanNeurotic <109461996+NathanNeurotic@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses the D-10 and D-14 black screen failures by constraining memory wiping to preserve the upper 1MB of system memory and removing an improper SifExitCmd call in the embedded loader. These fixes prevent corruption of RPC and kernel structures during IOP resets. Documentation has been updated to reflect these changes and the shift toward hardware verification. Feedback suggests replacing magic numbers with named constants in the loader code to improve clarity and maintainability.
| { | ||
| int i; | ||
| for (i = 0x100000; i < GetMemorySize(); i += 64) { | ||
| for (i = 0x100000; i < GetMemorySize() - 0x100000; i += 64) { |
There was a problem hiding this comment.
The magic number 0x100000 is used for both the starting address of the memory wipe and the size of the reserved memory region at the top. While they happen to be the same value, these represent different concepts (start of user memory vs. size of kernel area to preserve). To improve clarity and maintainability, consider defining named constants for these values. For example:
#define USER_MEM_START 0x100000
#define TOP_MEM_RESERVE_SIZE 0x100000Then the loop can be updated to use these constants, making the code's intent more explicit.
- Fix `wipeUserMem` to correctly preserve the upper 1MB of system memory (`GetMemorySize() - 0x100000`). Wiping this block resulted in destroyed kernel/RPC structures. - Remove the unsafe `SifExitIopHeap()` and `SifExitCmd()` calls in `elf.c` before jumping to the embedded loader. - Remove the unsafe `SifExitCmd()` call after the IOP reset in `loader.c`. Calling `SifExitCmd()` immediately after `SifIopReset()` hangs the EE because it attempts to talk to the wiped IOP over SIF. - Update tracking markdown documentation to reflect this resolution. Co-authored-by: NathanNeurotic <109461996+NathanNeurotic@users.noreply.github.com>
- Fix `wipeUserMem` to correctly preserve the upper 1MB of system memory (`GetMemorySize() - 0x100000`). Wiping this block resulted in destroyed kernel/RPC structures. - Remove the unsafe `SifExitIopHeap()` and `SifExitCmd()` calls in `elf.c` before jumping to the embedded loader. - Remove the unsafe `SifExitCmd()` call after the IOP reset in `loader.c`. Calling `SifExitCmd()` immediately after `SifIopReset()` hangs the EE because it attempts to talk to the wiped IOP over SIF. - Commit newly generated `loader.c` binary payload containing the bug fixes. - Implement `sleep 1` in `src/elf_loader/Makefile` to bypass checkout timestamp race condition in CI validation. - Update tracking markdown documentation (README, STATE, ROADMAP, DECISIONS, QA_REGRESSION_MATRIX) to reflect this resolution. Co-authored-by: NathanNeurotic <109461996+NathanNeurotic@users.noreply.github.com>
…6229939170846625966
- Fix `wipeUserMem` to correctly preserve the upper 1MB of system memory (`GetMemorySize() - 0x100000`). Wiping this block resulted in destroyed kernel/RPC structures. - Remove the unsafe `SifExitIopHeap()` and `SifExitCmd()` calls in `elf.c` before jumping to the embedded loader. - Remove the unsafe `SifExitCmd()` call after the IOP reset in `loader.c`. Calling `SifExitCmd()` immediately after `SifIopReset()` hangs the EE because it attempts to talk to the wiped IOP over SIF. - Call `unmount_pfs_slots_for_exec(0)` in `ExecuteHddBackedViaEmbeddedLoader` before mounting the partition to avoid `fileXioMount` `-1` return failures when another execution context mapped the partition. - Commit newly generated `loader.c` binary payload containing the bug fixes. - Implement `sleep 1` in `src/elf_loader/Makefile` to bypass checkout timestamp race condition in CI validation. - Update tracking markdown documentation (README, STATE, ROADMAP, DECISIONS, QA_REGRESSION_MATRIX) to reflect this resolution. Co-authored-by: NathanNeurotic <109461996+NathanNeurotic@users.noreply.github.com>
1 participant
This pull request resolves the
D-10HDD-backedPOPSTARTER.ELFblack screen hardware hang issue.Issue
The embedded loader (
src/elf_loader/src/loader/src/loader.c) was crashing the hardware during the Execution Engine (EE) handoff for two reasons:wipeUserMemwas clearing memory up toGetMemorySize(). Wiping the top 1MB clears essential kernel thread stacks and RPC buffers in high memory.SifExitCmd()directly afterSifIopReset(). CallingSifExitCmd()immediately after an IOP reset causes a hard hardware hang because it attempts to communicate using invalidated SIF structures with an unresponsive IOP.Fix
wipeUserMemto strictly stop atGetMemorySize() - 0x100000.SifExitCmd()from the IOP reset execution block to conform to thewLaunchELFexecution reference pattern.README.md,STATE.md,ROADMAP.md,DECISIONS.md, andQA_REGRESSION_MATRIX.mdas explicitly required by the repository documentation synchronization rules to document this resolution.PR created automatically by Jules for task 16229939170846625966 started by @NathanNeurotic