Repository navigation
fs.mitm: support firmware 23 batch-read RomFS storage - #2867
AmigaPrime wants to merge 3 commits into
Conversation
|
does this PR utilize this switchbrew/libnx#741 your PR to the other repository dates 18 hours ago, while the libnx PR proposing the ipc commands backend date 10 hours ago (also can you clarify which LLM you used to do this?) |
It's in the branch name, they used Codex. |
Well then, codex seems to have skipped over 211 and 213 (and hallucinated some things that look weird) |
|
Thanks for pointing out the missing command coverage. Updated in f3acc42:
This does not depend on libnx #741. The command IDs and buffer attributes have now been cross-checked against it. Its public open functions use libnx's global FSP session, whereas fs.mitm needs the caller's forwarding session. On the LLM question: OpenAI Codex was used for the implementation, tests and PR text, including a GPT-6-based Codex agent for this revision. That disclosure is now in the description as well. The full release build and native TestFs suite pass. Firmware-client instruction checks also pass for batch serialization and command 211's 64-bit payload. These stop before IPC; console testing of this implementation is still pending, so the PR remains a draft. Could you point to the specific lines that look incorrect? That would help distinguish any remaining ABI or implementation problems from the documented scope limitation. |
probably how AMS_FSSRV_I_STORAGE_FOR_BATCH_READ_INTERFACE_INFO only has command 10 implemented; and how you aren't using the libnx bindings (which is the standard thing to do) "IStorageForBatchRead": {
"nn::fssrv::sf::IStorageForBatchRead": {
"0": "Read", ("inbytes": 0x10, "outbytes": 0, "buffers": [70])
"1": "Write", ("inbytes": 0x10, "outbytes": 0, "buffers": [69])
"2": "Flush", ("inbytes": 0, "outbytes": 0)
"3": "SetSize", ("inbytes": 8, "outbytes": 0)
"4": "GetSize", ("inbytes": 0, "outbytes": 8)
"5": "OperateRange", ("inbytes": 0x18, "outbytes": 0x40)
"10": "BatchRead" ("inbytes": 0, "outbytes": 0, "buffers": [70, 70, 70, 70, 70, 70, 70, 5])
}
},not that LLM made PR's are wanted in this repository. |
|
Updated in b9e6ff5 with an explicit seven-command table and expanded tests. The previous implementation already provided commands 0–5 through The implementation also already uses libnx: backing reads call Tests now verify command registration/version gates and the Horizon serializer's input sizes, output sizes and buffer attributes against the ABI you supplied. Runtime tests exercise all seven methods, including read-only write/resize rejection. Native tests, the Switch TestFs ELF build, and the full release build pass. Command 213's LayeredFS limitation remains documented, and the PR stays draft pending hardware testing. |
|
LLM usage will get you a repo perma ban, just saying. see: #2865 (comment) |
|
Hey hey, human here, I did do all of the research myself, but as mentioned did use Astra for the coding, I could have written the code with my meat hands but it would have taken a few days and probably had way more typos and mistakes... if there is any technical inaccuracies I'm happy to fix them. Have to be honest and say it's pretty demotivating spending all this time researching this issue trying to help just to get shit for using LLMs, very demotivating, if this was complete slop and nonsense I would maybe get it, but I definitely think this has merit, I really don't get the aggression. |
|
Atmosphere has strict 0 LLM policy. |
|
Alright, i did use an LLM to create this so ill just close this PR, it does work tho, i tested deeply, i guess i will wait for someone to retype this with meat hands, good luck! |
|
created by evil robots, only meat allowed |
|
@AmigaPrime see it from the perspective of the main devs. There are a lot of eyes on this and many people now have easy access to LLM, creating seemingly plausible code and descriptions, while having zero actual coding experience or at least zero experience with this particular code base. If the devs need to weed through an increasing amount of LLM generated PRs, just to find those who actually have merit, they won't have much time left to do actual development. So, is it possible that you actually have years of experience coding C++ and are very familiar with the Atmosphere codebase? Yes. But your profile with barely any commits and this being your first C++ PR doesn't show that kind of experience. |
he says it works, but codex revealed it hasn't been tested on an actual machine. |
|
@BassMonkey I appreciate the eye level comment, thank you for explaining this perspective in a respectful way, while I do have over 20 years of experience in RE and low level dev, I understand it's not visible from my profile and can look like a bot, and I would like to respect the rules of the repo, I do think it's a shame LLMs are banned, but of course it's not my decision to make. @borntohonk You flamed me straight off the gate and come off extremely angry and rude. I'm not sure what you're so frustrated about, but I doubt the cure is taking your anger on random people online volunteering to help. looking at the history of this repo you have changed 20 lines of code in 2 years, so I'm not sure what the high and mighty attitude is all about :) |
|
Blocked user for llm usage. |
On firmware 23.0.0, qlaunch opens its RomFS with
OpenDataStorageByCurrentProcessForBatchRead(210). fs.mitm currently forwards that command to FS, so installed LayeredFS overrides are bypassed and HOME shows the stock resources.This adds LayeredFS support for batch-read storage opens 210, 211 and 212 on 23.0.0+:
IStorageForBatchReadexplicitly declares the complete command table: Read (0), Write (1), Flush (2), SetSize (3), GetSize (4), OperateRange (5), and BatchRead (10). All are gated to 23.0.0+. Commands 0–5 reuse the existingStorageInterfaceAdapterimplementations; ordinary IStorage objects still expose only 0–5.s64offset array. Each entry uses the layered adapter'sRead, preserving validation, storage errors and bounded corruption retries. More than seven offsets are rejected before indexing an output buffer.For libnx integration, the open shims use
serviceDispatchon the original forwarding session, matching existing fs.mitm opens. The backing storage uses Atmosphere's existingfs::RemoteStorage, which calls libnx'sfsStorage*bindings. Commands 0–5 retain the IStorage wire ABI. BatchRead must read through the layered adapter; calling BatchRead directly on the backing FS object would bypass the merged RomFS address space.This does not depend on the still-unmerged libnx PR #741. Its public batch-open APIs use libnx's global FSP session, so they cannot replace these forwarding shims while preserving the caller's session. Command IDs and buffer attributes were cross-checked against revision
5246221963ade16de30e4d14a83c1ecdda921a7a; variable buffer lengths were independently checked against the firmware client.Path-based command 213 remains forwarded, as does legacy 206. Paths/filesystem types can select different content carrying the same program ID, while the layered-storage cache is keyed only by program ID. Resolving a path with
GetProgramId(618) would not distinguish those backing files. Path-based LayeredFS requires content-aware cache identity and override selection; this PR does not implement it. Forwarded opens retain FS behavior without LayeredFS overrides.Validation:
make -C tests/TestFs nx_release BOARD_TARGET_SUFFIX=.elf -j8. Compile-time checks verify command registration/version gates and the Horizon serializer's input sizes, output sizes and buffer attributes for every command. ELF output avoids the test project's absent KIP packaging rule; this is a build check, not a console test.146c3d1446d35546943c7b6cfcd479bc1f5e9b67. Used a slash-free distribution version label for this branch.Hardware validation is pending, so this remains a draft. The earlier workaround in exelix11/theme-patches#24 was tested with a visible theme and successful reboot on
23.0.0|AMS 1.12.0|E; it did not test this implementation. For a meaningful console test, remove the two legacy-read workaround instructions from the active qlaunch IPS while retaining the separate SceneEntrance heap patch, then check HOME appearance and reboot.This follows the theme-patches maintainer's recommendation to fix missing fs.mitm support in Atmosphere.
AI assistance: OpenAI Codex was used to develop the code, tests and PR text, including a GPT-6-based Codex agent for these revisions. The checks above do not replace maintainer review or hardware testing.