search

blocksds / sdk

Main BlocksDS SDK repository
https://blocksds.github.io/docs/
130 stars 5 forks source link

Initialize MBK9 to a known state on boot #133

Closed AntonioND closed 3 months ago

AntonioND commented 4 months ago

Currently, when applications start they don't setup MBK9 to any specific value. In tests, the default value with Unlaunch is 0xF for me. This locks NWRAM A, and unlocks B and C. This is enough to let DSP binaries be loaded, which explains why the DSP examples work. However, we can't rely on this being the case, and we should always try to set the register to a well-known value.

https://problemkaputt.de/gbatek.htm#dsinewsharedwramforarm7arm9dsp

This issue involves:

AngelTomkins commented 4 months ago

Would it be a good idea to also put a check to MBK9 whenever using nwramMapWramASlot, nwramMapWramBSlot, nwramMapWramCSlot, and nwramSetBlockMapping? They could return a status by checking if the writes to the registers were successful or if MBK9 was set to not give permissions.

AntonioND commented 4 months ago

I agree that the checks should be there, I'll create a patch for that.

However, now I understand how the default MBK values are set, and I have some thoughts.

The NDS ROM header has some fields to be used as default MBK values: https://problemkaputt.de/gbatek.htm#dsicartridgeheader

New DSi Header Entries

180h 20   Global MBK1..MBK5 Settings, WRAM Slots
194h 12   Local MBK6..MBK8 Settings, WRAM Areas for ARM9
1A0h 12   Local MBK6..MBK8 Settings, WRAM Areas for ARM7
1ACh 3    Global MBK9 Setting, WRAM Slot Write Protect

In theory, ndstool could be modified to allow setting the values, but it looks like not all loaders respect the values, and some of them simply set the MBK registers to common values used by applications.

In short: We have some default values that seem to be quite stable, and it doesn't look like changing the NDS header will be realiable, so the only way to change the values reliably is to do it at runtime.

However, it's not even clear if changing the values is a good idea. The only bank that is locked is NWRAM-A, which is hardcoded to be mapped to the ARM7, and the linkerscript expects the memory to be there. NWRAM-A is used to store twl sections in the ARM7, so unmapping it would crash the ARM7.

Is there a good reason to not want this RAM to be allocated to the ARM7? On DS, the ARM7 only has 96 KB of WRAM, which is essentially nothing, so reserving those additional 256 KB of WRAM feels right. The ARM9 gets 12 additional MB of RAM, so the 256 KB of WRAM don't feel very useful.

AntonioND commented 3 months ago

I've added some checks to NWRAM mapping functions: https://github.com/blocksds/libnds/commit/904797421e49eac713153c35ef1076454fa1135d and I've added some checks to the DSP functions too: https://github.com/blocksds/libnds/commit/fc06e46073ae59894158eb70c0567ad0fcca6889

I've tested it by doing REG_MBK9 = -1 right at the top of the ARM7 main() of this example: https://github.com/blocksds/sdk/tree/c0e5c14aa007e320ece2546d39b657f8123692c1/examples/dsp/fifo_transfers

AngelTomkins commented 3 months ago

The default values that are loaded into those registers through ndstool and twilightmenu+ndsboostrap are as follows:

WRAM-A arm7 256KiB starting at 0x03000000 ending at 0x0303FFFF(This is what ndstool uses, did not test on hardware) WRAM-B arm9 256KiB starting at 0x03740000 ending at 0x037BFFFF (this is a length of 512KiB, not sure why/how) WRAM-C arm9 256KiB starting at 0x03700000 ending at 0x0373FFFF

Would it be a good idea since this memory is mapped to the arm9 to give the option to put functions in WRAM-B and WRAM-C? A new section label could be used that is specifically for this nwram section since it is faster than main ram. the .twl section label could be moved here, but using .twl is more so functions that are only existing on DSi, and not required to be in faster ram. And changing that could break older projects where .twl has more than 512KiB of code/data.

I propose having a new section such as .nwram that would do the same as the arm7 does, and copy everything labeled with .nwram into the banks B and C. Is this something that Blocksds would want to support?

AntonioND commented 3 months ago

(this is a length of 512KiB, not sure why/how)

I guess it's just mirrored after the first 256 KB.

Is this something that Blocksds would want to support?

I'm not sure.

Being able to setup NWRAM is not a guarantee. With some entrypoints/exploits you need to stick to the settings that you get.

There are already a few checks in the code in case you're trying to setup NWRAM and you don't have enough permissions to do that, so adding new labels for something that may or may not be there, based purely on the exploit you're using to run code, feels a bit unreliable.

I think that devkitPro's libnds relied on WRAM-A being mapped to a specific address because DSi games use that setting, so that's just the permission that you inherit when you're running through any exploit. Nintendo decided to use that convention, so it's reliable.

It doesn't look like there is any convention for B or C (I may be mistaken here, I'd like to be corrected if that's the case).

Basically, I don't want to add a new region to the linkerscript unless it is reliable. What if different exploits map WRAM-B/C to different addresses? Which mapping do we use?

I'd need to know more about how that works, I don't know enough about the entrypoints to decide what to do.

AntonioND commented 3 months ago

I think I'm going to close this. Some checks have been added to the functions that control NWRAM, and the behaviour is generally documented in https://github.com/blocksds/sdk/blob/2ada1c5510770653559a01b8c9af4b2f8a94a233/docs/content/technical/memory_map.md#61-dsi-shared-wram-nwram

I'm not sure it's a good idea to make it possible to make the linkerscript even more complicated than it is now. Adding a *.nwram filter here seems like a bad idea, specially considering that at some point we really need to add support for overlays https://github.com/blocksds/sdk/issues/44 and that will already require extensive changes to the linkerscript.