boot-d678: move ML to start of user_mem

[kitor]

Moves ML to start of user_mem, like it was already on DIGIC 4/5 in boot-d45.c

---

On D6 it was placed at the end of user_mem just because assembly of cstart changed a bit and it wasn't easy to patch user_mem_start anymore (see comments in very old boot code for d6+). This argument was no longer valid from the point we decided to replace create_init_task with our function - where we can alter every value of cstart struct independently.

---

This makes RESTARTSTART "set and forget" - copy value from user_mem_start, align up to 0x10 (our code expects that). No more counting that by hand and verifying on qemu and real HW.

No need for ML_RESERVED_MEM, ML_MAX_USER_MEM_STOLEN.

Tested:

On physical: 80D.103 (not in commits), 750D.110, 200D.101, R.180, RP.160, R5.152, SX70.110, SX740.102, 850D.100, 5D4, 7D2 On QEMU: M50.110, 6D2 Not tested: R6.150 (doesn't build, missing function_overrides), 5DSR (updated to build in QEMU at all) Not updated, platforms removed: 760D, 5DS (no code, fake stubs... Not updated, disabled builds: 80D.102, 77D.102 (80D.103 will be merged soon, 77D.110 will follow soon-ish)

---

I also got rid of sys_mem stuff (ML_MAX_SYS_MEM_INCREASE) as it is only suspected to work on a single model. Equivalent of this that permanently moves SYS_MEM up for a given model should be easy to add if ever needed.

---

TODO:

• Verify and update all other D678X models we have
• Move comment about system memory layout that we have in constants.h to dryos.h added to boot-d678.h
• Add some way to verify if RESTARTSTART is aligned to 0x10 at build time done

[reticulatedpines]

Seems good to me. I haven't run any phys tests. Did some qemu. Are you concerned about missing phys tests before merging? Anything you'd like me to do there?

What testing occured around MINIMUM_USER_MEM_LEFT? Previously, we were stealing at most 0x66000 on 200d, I think (practically, max was about 0x48000?). Do you know what the max stolen can be now, across all cams?

In general, are there other checks you'd like to do at build time? I can look at adding these.

ML_MAX_SYS_MEM_INCREASE definitely worked on 200D, if I remember right the memory layout was different and there was a gap after one of the sys pools (that I assumed would be common on D7 but wasn't). I don't mind it going away; reduces complexity, it's not currently required, and we want to move to using AllocMem if/when we need more mem.

A few typos and minor suggested changes, formatted as a patch, take what you want:

diff --git a/src/boot-d678.c b/src/boot-d678.c
index 74221ec14..681cd3284 100755
--- a/src/boot-d678.c
+++ b/src/boot-d678.c
@@ -140,14 +140,15 @@ static void my_create_init_task(struct dryos_init_info *dryos, uint32_t init_tas
     // struct that it takes, which holds a bunch of OS info.
     // We adjust sizes of memory regions to reserve space for ML.
     //
-    // This may, depending on consts.h for the cam, move up both
-    // sys_objs start and sys_mem start.
-    // The effects of this have not been fully tested.
+    // We reserve space before DryOS user_mem, this means the start
+    // address is fixed per cam, by RESTARTSTART in
+    // platform/99D/Makefile.platform.default.

     // replace Canon's init_task with ours
     init_task = (uint32_t)my_init_task;

-    // do the alignment on user_mem_start
+    // user_mem_start must have the same alignment as the code section
+    // of the ELF used during building ML binaries, or things like relocs don't work.
     if(dryos->user_mem_start % ELF_ALIGNMENT != 0)
     {
         qprint("[BOOT] user_mem_start is not aligned to "); qprintn(ELF_ALIGNMENT); qprint(", adjusting\n");
@@ -161,7 +162,7 @@ static void my_create_init_task(struct dryos_init_info *dryos, uint32_t init_tas
     }

     // check if RESTARTSTART is correct
-    // note - this will fail to print due to... msisalignment. Catch 22.
+    // note - this will fail to print due to... misalignment. Catch 22.
     if (RESTARTSTART != dryos->user_mem_start)
     {
         qprint("[BOOT] Wrong or unaligned RESTARTSTART address!\n\n");
@@ -175,7 +176,7 @@ static void my_create_init_task(struct dryos_init_info *dryos, uint32_t init_tas
     qprint("[BOOT] reserving memory: "); qprintn(ml_reserved_mem); qprint("\n");
     qprint("       before: user_mem_start = "); qprintn(dryos->user_mem_start); qprint("\n");
     qprint("       before: user_mem_size  = "); qprintn(dryos->user_mem_len); qprint("\n");
-    
+
     // shrink user_mem
     dryos->user_mem_start += ml_reserved_mem;
     dryos->user_mem_len -= ml_reserved_mem;
@@ -191,6 +192,7 @@ static void my_create_init_task(struct dryos_init_info *dryos, uint32_t init_tas
     }

     if ( (dryos->user_mem_start + dryos->user_mem_len > dryos->sys_objs_start) ||
+         (RESTARTSTART + ml_reserved_mem > dryos->user_mem_start) ||
          (RESTARTSTART + ml_reserved_mem > dryos->sys_objs_start) )
     {
         qprint("[BOOT] User mem math gone wrong, aborting!");

[kitor]

Seems good to me. I haven't run any phys tests. Did some qemu. Are you concerned about missing phys tests before merging? Anything you'd like me to do there?

Yes, please add values and test it on 7D2, 5D4, 850D. I don't have (good) projects for both and I don't have 5D4 to test.

Possibly also disable/remove builds for old 7D2, 5D4 firmwares as I don't think they are worth upgrading after you ported both to the latest firmwares.

I'm not sure if we have anyone with 6D2 to test?

===

ML_MAX_SYS_MEM_INCREASE definitely worked on 200D, if I remember right the memory layout was different and there was a gap after one of the sys pools (that I assumed would be common on D7 but wasn't). I don't mind it going away; reduces complexity, it's not currently required, and we want to move to using AllocMem if/when we need more mem.

Explained at Discord. All models since Digic 6 (including MPU and secondary core firmwares) have USER_MEM size checking as the very first thing they do in init_task. Value is usually set in cstart, before cstart_struct - on all ICU firmwares D6-DX it is hardcoded to 0x80000 +0x10 (thus the default value, i need to bump it +0x10 btw), on secondary cores (zico, lime) seems to be 0x2000 + 0x8 and on MPU it is 0 + 0x8.

I wouldn't consider this a safe value. This is a value below which DryOS will simply refuse to boot. But with ifndef implementation we can set a limit per model if we ever hit unsafe size.

The only downside (?) is that this is checked on runtime, not on build time.

In general, are there other checks you'd like to do at build time? I can look at adding these.

I don't think we need any extra checks. Restartstart alignment check is already implemented. All other data are runtime checks.

ML_MAX_SYS_MEM_INCREASE definitely worked on 200D

I know, and IIRC that was the only camera it was confirmed to work, due to slightly unusual memory layout. Others seem to store DbgMgr buffers directly after system memory, 200D had a gap like they didn't expand user_mem or wanted more DbgMgr buffers but forgot to add them. And

I don't mind it going away; reduces complexity, it's not currently required, and we want to move to using AllocMem if/when we need more mem.

That was exactly my point, user_mem is enough only for early ports.

A few typos and minor suggested changes, formatted as a patch, take what you want:

Thanks, applied and force-pushed. I also reordered commits to disable platforms before introducing that change.

+ (RESTARTSTART + ml_reserved_mem > dryos->user_mem_start) || good catch btw

[reticulatedpines]

7D2, 5D4 and 850D all tested locally and boot to ML menus. 5D4 is unstable and laggy, but this is true without your changes, and I don't have uart access to easily diagnose. I think this is fine for now.

Somebody must have a 6D2 but not me. Walter has 5DS or 5DSR (currently disabled in map).

[kitor]

Builds fine from make zip in platform/ dir. I think it is ready to merge. I updated PR description to match its current state.