Prepare sys/boot for MIPS (32 and 64 bit) ubldr support

[Sgalabov_gmail.com]

Summary

This changes attempt to put things in order before the introduction of MIPS ubldr.

The changes are mostly dealing with removing unnecessary casts from the U-Boot API (we're passing only pointers, no obvious reason to cast them to uint32_t), cleaning up some compiler warnings and using the proper printf format specifiers in order to be able to compile cleanly for both 32-bit and 64-bit MIPS targets.

[Sgalabov_gmail.com]

Cleaned up some leftover issues.

[andrew]

sys/boot/fdt/fdt_loader_cmd.c

300–301 ↗

(On Diff #13393)

%p

sys/boot/uboot/common/main.c

138 ↗

(On Diff #13393)

Cast to a uintmax_t and use %ju

431 ↗

(On Diff #13393)

%p

516–517 ↗

(On Diff #13393)

I think a ptrdiff_t is %td

sys/boot/uboot/lib/disk.c

160 ↗

(On Diff #13393)

Should be %zu as size is a size_t

sys/boot/uboot/lib/elf_freebsd.c

34 ↗

(On Diff #13393)

Missing the trailing __?

89 ↗

(On Diff #13393)

why not %p?

sys/boot/uboot/lib/glue.c

86–92 ↗

(On Diff #13393)

What are these numbers?

441 ↗

(On Diff #13393)

%p

[imp]

I'm unclear how this supports 64-bit mips. Perhaps a quick note about what I'm missing would be in order?

sys/boot/common/dev_net.c

167 ↗

(On Diff #13393)

This looks independent, but likely good.

sys/boot/common/self_reloc.c

43 ↗

(On Diff #13393)

don't you need mips64 in this list?

59–61 ↗

(On Diff #13393)

Ditto?

sys/boot/uboot/lib/elf_freebsd.c

34 ↗

(On Diff #13393)

And do you need 64-bit flavors?

[Sgalabov_gmail.com]

Thanks for the reviews. I've responded to some of the comments and will take care of them and submit a new version.

In order to clarify: this revision in itself does not add the support for 64-bit (or 32-bit) MIPS, it just makes it possible to add.
The changes to sys/boot/uboot/lib/glue.c are the actual part that allows this to happen (removing the uint32_t casts in calls to syscall())). The rest is mostly fixing compiler warnings due to improper printf formats used, etc.

sys/boot/common/dev_net.c

167 ↗

(On Diff #13393)

Yes, this is not directly related to the preparation for MIPS ubldr, but it was producing a warning during build and I decided to clean it up. Interestingly, due to this it appears that the ARM ubldr had its error on warnings disabled in the Makefiles. I thought it was a better idea to fix the warning instead of doing the same for MIPS :-)

sys/boot/common/self_reloc.c

43 ↗

(On Diff #13393)

Initially I intended to support self-relocation on MIPS as it was done for ARM, but so far I am failing to produce a proper PIC executable with proper dynamic symbols and proper relocation data for MIPS (it may be due to the combination of parameters being passed to gcc and ld). Since there are other MIPS targets in sys/boot (e.g., BERI) which I wouldn't want to break if I were to drastically change compile and link flags and since dynamic relocation is "nice to have", but not mandatory, I decided to defer its support for now. So I may simply remove the mips case altogether or leave it as it is for now and get back to it when dynamic relocation support is on the table.

59–61 ↗

(On Diff #13393)

Please see above comment.

sys/boot/uboot/lib/elf_freebsd.c

34 ↗

(On Diff #13393)

Yes, I meant to use mips instead of __mips, although GCC defines both. I'll fix this.

34 ↗

(On Diff #13393)

We don't need anything 64-bit specific here, this compiles (and works) properly for both 32- and 64-bit MIPS under QEMU.

[Sgalabov_gmail.com]

The actual MIPS ubldr support will come with D5313.

[Sgalabov_gmail.com]

Addressed review comments.
I decided to drop sys/boot/common/self_reloc.c changes, as currently self relocation is not supported by the MIPS ubldr.

[imp]

This looks ready to go. If you happen to know why MIPS is different, it would be cool if you could address that in the comment. If not, then no big deal. Differences like that will be a mystery later if we don't document what we know today.

sys/boot/uboot/lib/glue.h

53 ↗

(On Diff #13760)

Is this ARM specific, or does PPC do that too? Or is MIPS really the only odd-man out? I'm left wondering why mips is different. If you know the reason, perhaps it would be good to add that here. If not, it isn't a huge deal.

[adrian]

go!

sys/boot/fdt/fdt_loader_cmd.c

431 ↗

(On Diff #13393)

why's this ifdef'ed out?

[Sgalabov_gmail.com]

In D5312#116404, @imp wrote:

This looks ready to go. If you happen to know why MIPS is different, it would be cool if you could address that in the comment. If not, then no big deal. Differences like that will be a mystery later if we don't document what we know today.

I had actually figured out that the API signature is very near to the end of RAM on QEMU-MIPS in the very beginning of my work on ubldr for MIPS and have left it like that since.
Thanks for bringing it up - now I decided to look at it a bit deeper and realised that it is actually very dependent on U-Boot's CONFIG_SYS_MALLOC_LEN, which in the case of QEMU-MIPS is set to 128*1024 (128KiB) :-)
Unfortunately it may be set to whatever value in other boards' configs and there's no way to know that at the stage where we need it. With quick tests I have done just now it seems that even having the MIPS case be the same as the non-MIPS case according to the definitions in glue.h (e.g., have MIPS search within 3MiB) seems to work fine now.

However, if we fail to find a valid API signature before we hit the end of memory we'd risk getting an exception. As I am not sure what is the best way to handle this at the moment I'll commit as is (with MIPS searching within 1MiB and ARM within 3MiB) and I'll come back to this if I get a better idea in the coming days...

[Sgalabov_gmail.com]

sys/boot/fdt/fdt_loader_cmd.c

430 ↗

(On Diff #13760)

This is a static function that is not called by any other function in sys/boot/fdt/fdt_loader_cmd.c and gcc was spitting out warnings about it. Not being sure whether it's still useful to someone or not I decided to ifdef it out.