Page MenuHomeFreeBSD

ubldr for arm64
AbandonedPublic

Authored by skibo on Mar 1 2016, 8:24 PM.

Details

Reviewers
None
Summary

This revision adds support for building ubldr for arm64 systems.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

skibo updated this revision to Diff 13947.Mar 1 2016, 8:24 PM
skibo retitled this revision from to ubldr for arm64.
skibo updated this object.
skibo edited the test plan for this revision. (Show Details)
andrew added a subscriber: andrew.Mar 2 2016, 1:58 PM

I have been thinking we should build a boot0 for U-Boot that then loads a U-Boot loader from UFS (and ZFS when we support it). We woul then put boot0 in the fat partition, and maybe name ubldr to something like loader.ub.

sys/boot/arm64/uboot/Makefile
136–138

Do we need this? ubldr should be position independent.

sys/boot/common/load_elf.c
382

Is this needed in the EFI case? It's been working until now without it.

imp added a subscriber: imp.Mar 2 2016, 4:55 PM

this looks ok, modulo Andy's comments. Part of me thinks that now that there's two architectures that are doing PA VA relocations we should have that as a #define in the code and set that #define based on arm or aarch64.

Also, the makefile looks like it has a ton of duplication. Perhaps you could take a look at that. The boot loader code is an area we are really really bad about duplication in.

skibo added inline comments.Mar 2 2016, 7:28 PM
sys/boot/arm64/uboot/Makefile
136–138

I'm not sure. I'll test to see if it can relocate itself. I just copied the Makefile from sys/boot/arm/uboot.

sys/boot/common/load_elf.c
382

Almost certainly not. I'll qualify it with "&& !defined(EFI)".

skibo updated this revision to Diff 14026.Mar 3 2016, 2:57 PM

Don't adjust va->pa offset in EFI loader. Clean up assembly in start.S.

skibo added inline comments.Mar 3 2016, 3:06 PM
sys/boot/arm64/uboot/start.S
79–82

I can't get the position independent ubldr.pie to work because the linker leaves zeros in these locations. In the arm version, these locations are filled with offsets. Without the value of __DYNAMIC I don't know how to find the relocation data to pass to self_reloc().

skibo added inline comments.Mar 3 2016, 3:10 PM
sys/boot/arm64/uboot/start.S
51

Sorry, destroyed this comment. Will fix.

skibo added inline comments.Mar 3 2016, 9:09 PM
sys/boot/arm64/uboot/start.S
79–82

Okay, I think I know what's going on. In aarch64, relocation data is stored in Elf64_Rela entries whereas arm uses Elf32_Rel entries (no addend field). In arm, the offsets go into the locations to be adjusted whereas in aarch64, the offsets go into the addend field in the entries in the relocation table and zeros go into locations that need to be adjusted. Any ideas how I could find the location of the dynamic section from within here so I can call self_reloc()?

andrew added inline comments.Mar 3 2016, 11:35 PM
sys/boot/arm64/uboot/start.S
79–82

Have you looked at the efi code? It's a lot simpler than this, and has been tested on various hardware.

skibo added inline comments.Mar 4 2016, 12:32 AM
sys/boot/arm64/uboot/start.S
79–82

Thanks! I got it to work. So we should just build a relocatable ubldr only.

andrew added inline comments.Mar 4 2016, 9:59 AM
sys/boot/arm64/uboot/ldscript.aarch64
52

This is probably bogus, there is no need to align to a page boundary unless we are using the MMU to map different sections, e.g. to make data non-executable.

sys/boot/efi/loader/bootinfo.c
224 ↗(On Diff #14026)

Are these changes still needed? I don't think they are needed for __arm__ as relocation_offset will be zero.

skibo added inline comments.Mar 4 2016, 3:34 PM
sys/boot/arm64/uboot/ldscript.aarch64
52

Ok. Removed and still works.

sys/boot/efi/loader/bootinfo.c
224 ↗(On Diff #14026)

Ok. I reverted this whole file and it still works. I haven't tested loading modules. New revision coming soon.

skibo updated this revision to Diff 14077.Mar 4 2016, 3:46 PM

Get relocation to work. Remove static linked target.

skibo added inline comments.Mar 6 2016, 7:14 PM
sys/boot/arm64/uboot/ldscript.aarch64
12

I should put in a comment in why I do this. The position-independent code isn't really position-independent. Because of the way adrp instructions work, binaries must be relocated on 4K boundaries. Starting the text section at 4K ensures that the 4K boundaries remain the same after the header is stripped to produce ubldr.bin. I found a -mcmodel=tiny option which gets rid of the adrp instructions but then it fails to link so I decided to punt.

skibo updated this revision to Diff 14138.Mar 7 2016, 6:19 PM

Add comment in ldscript.aarch64 explaining text section alignment.

Would it be possible to leave the position-dependent targets in the makefile as it was on arm32? The '-pie' version is rejected by U-Boot when bootelf is run, as it only recognizes ELFs with e_type == ET_EXEC. So in this version only the .bin is usable with U-Boot.

skibo added a comment.Mar 9 2016, 7:45 PM

Would it be possible to leave the position-dependent targets in the makefile as it was on arm32? The '-pie' version is rejected by U-Boot when bootelf is run, as it only recognizes ELFs with e_type == ET_EXEC. So in this version only the .bin is usable with U-Boot.

Ok. I think I have a cleaner way to link them without creating temporary ld scripts. I can define the symbol UBLDR_LOADADDR in the link command with "-Wl,--defsym,UBLDR_LOADADDR=${UBLDR_LOADADDR}". I still have to start the text area on a 4K boundary if the position-independent code is to work. I have this all coded but need to test.

skibo updated this revision to Diff 14199.Mar 9 2016, 9:18 PM

Bring back position dependent ubldr.

skibo updated this revision to Diff 14200.Mar 9 2016, 9:23 PM

Missed a clean file.

Would it be possible to leave the position-dependent targets in the makefile as it was on arm32? The '-pie' version is rejected by U-Boot when bootelf is run, as it only recognizes ELFs with e_type == ET_EXEC. So in this version only the .bin is usable with U-Boot.

If you need to load something at a specific address I would prefer it was a boot1 like tool. This would then load ubldr from UFS and branch to it.

Alternatively, what's wrong with loading the .bin file & running it with go?

Would it be possible to leave the position-dependent targets in the makefile as it was on arm32? The '-pie' version is rejected by U-Boot when bootelf is run, as it only recognizes ELFs with e_type == ET_EXEC. So in this version only the .bin is usable with U-Boot.

If you need to load something at a specific address I would prefer it was a boot1 like tool. This would then load ubldr from UFS and branch to it.
Alternatively, what's wrong with loading the .bin file & running it with go?

Yes you can always just run the .bin file. Just wanted to point out that the '-pie' version will possibly never work with U-Boot, while the non-pie version would be nice to keep as it is confirmed to be working. I remember when I tested the initial patches by Thomas (version from the mailing list) on my ARM64 board I could not get the .bin version to work - it got to the prompt but loading the kernel via network hanged indefinitely. The ELF version , however, loaded and booted the kernel.

In fact now I see the same issue occur with the newest version from this review - this time both the ELF and the .bin. I haven't confirmed it yet but it could be something with my U-Boot, perhaps ubldr goes out of memory bounds or something similar.

skibo added a comment.Mar 10 2016, 3:57 PM

Yes you can always just run the .bin file. Just wanted to point out that the '-pie' version will possibly never work with U-Boot, while the non-pie version would be nice to keep as it is confirmed to be working. I remember when I tested the initial patches by Thomas (version from the mailing list) on my ARM64 board I could not get the .bin version to work - it got to the prompt but loading the kernel via network hanged indefinitely. The ELF version , however, loaded and booted the kernel.

The pie version (ubldr.bin) is working for me now. It was broken in my first patch. I've loaded it at various locations and it has booted a kernel in my aarch64 qemu simulation.

But, I like having the fixed-location elf version around for debugging and I'd like to leave it.

skibo added a comment.Mar 10 2016, 4:05 PM

If you need to load something at a specific address I would prefer it was a boot1 like tool. This would then load ubldr from UFS and branch to it.
Alternatively, what's wrong with loading the .bin file & running it with go?

I'm not sure we should go to the trouble of creating a boot1 like tool. Alexander Graf has implemented the EFI application protocol on top of u-boot so the future of FreeBSD on systems using u-boot may be running efi/boot1 and loader.efi using that instead of the u-boot API.

I noticed there was still one compilation error not resolved here, namely:

sys/boot/uboot/lib/disk.c:160:7: error: format specifies type 'int' but the argument has type 'size_t' (aka 'unsigned long') [-Werror,-Wformat]

size, SI(dev).bsize);

Changing the format specifier for 'size' from %d to %zu should make it work on all archs.

skibo added a comment.Mar 10 2016, 4:49 PM

I noticed there was still one compilation error not resolved here, namely:
sys/boot/uboot/lib/disk.c:160:7: error: format specifies type 'int' but the argument has type 'size_t' (aka 'unsigned long') [-Werror,-Wformat]

size, SI(dev).bsize);

Changing the format specifier for 'size' from %d to %zu should make it work on all archs.

sgalabov fixed that in r296182. It doesn't break for me.

In D5512#119505, @skibo wrote:

I noticed there was still one compilation error not resolved here, namely:
sys/boot/uboot/lib/disk.c:160:7: error: format specifies type 'int' but the argument has type 'size_t' (aka 'unsigned long') [-Werror,-Wformat]

size, SI(dev).bsize);

Changing the format specifier for 'size' from %d to %zu should make it work on all archs.

sgalabov fixed that in r296182. It doesn't break for me.

My mistake, I thought I had that change already in my working tree but apparently I didn't.

Is there any objection to this getting committed? This may help with the RPI3 work we're doing at BSDCan right now.

skibo added a comment.Jun 9 2016, 4:06 AM

Is there any objection to this getting committed? This may help with the RPI3 work we're doing at BSDCan right now.

I don't have any objection but I was under the impression that Andrew booted arm64 on rpi3 without u-boot so I thought this all became unnecessary. I originally worked on this because I wanted to get FreeBSD up and running on the Zynq Ultrascale+ but I have since shelved that idea. That's why I haven't pushed to get this committed.

brd added a subscriber: brd.Jun 11 2016, 1:51 PM
skibo abandoned this revision.Oct 12 2019, 6:53 PM

Cleaning up very old stuff. We don't need ubldr on arm64.