Page MenuHomeFreeBSD
Differential D9108

Fix UEFI support for ARM
ClosedPublic
Actions

Authored by gonzo on Jan 9 2017, 7:52 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 8 2024, 6:25 PM
Unknown Object (File)
Dec 22 2023, 12:02 PM
Unknown Object (File)
Dec 22 2023, 12:01 PM
Unknown Object (File)
Dec 20 2023, 12:50 AM
Unknown Object (File)
Nov 6 2023, 10:33 AM
Unknown Object (File)
Oct 5 2023, 9:31 AM
Unknown Object (File)
Aug 19 2023, 8:36 PM
Unknown Object (File)
Jul 21 2023, 9:19 PM
View All 17 Files
Subscribers
imp

Details

Reviewers
andrew
Commits
rS311891: [efi] Build EFI bits with -fPIC on ARM
rS311890: [efi] Fix .rel.data.* being erroneously merged into .data on ARM
rS311888: [efi] Fix off-by-one error in ARM .bss zeroing code in loader's _start
Summary

ARM UEFI support is borken on clang 3.9.0 because for two reasons:

  • without -fPIC clang generates abosulte jump tables for switch statement
  • ldscript merges .rel.data.rel.ro into .data section, so some relocations are lost in final binary (boot1.sym.full)
Test Plan

boot QEMU VIRT kernel using EFI

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

gonzo updated this revision to Diff 23791.Jan 9 2017, 7:52 PM
gonzo retitled this revision from to Fix UEFI support for ARM.
gonzo updated this object.
gonzo edited the test plan for this revision. (Show Details)
gonzo added a reviewer: andrew.
Herald added a subscriber: imp. · View Herald TranscriptJan 9 2017, 7:52 PM
andrew added inline comments.Jan 9 2017, 9:45 PM
sys/boot/efi/boot1/Makefile
73 ↗(On Diff #23791)

Do we need this in the loader.efi Makefile? If so it should be moved to sys/boot/efi/Makefile.inc

sys/boot/efi/loader/bootinfo.c
354–368 ↗(On Diff #23791)

Is this intentional?

gonzo added inline comments.Jan 9 2017, 10:47 PM
sys/boot/efi/boot1/Makefile
73 ↗(On Diff #23791)

I assume that it's required. But I will check

sys/boot/efi/loader/bootinfo.c
354–368 ↗(On Diff #23791)

Yes. Without this change relocation happens twice. I think previously it wasn't relocated because .rel.data.rel.ro was merged into .data and that relocations were lost. With fixed ldscript these values relocated EFI loader.

andrew added inline comments.Jan 9 2017, 11:45 PM
sys/boot/efi/loader/bootinfo.c
354–368 ↗(On Diff #23791)

On arm when building for EFI __elfN(relocation_offset) is set to 0 so this code should have been a nop. If it is now non-zero that would indicate a bug somewhere that this is picking up.

gonzo updated this revision to Diff 23818.Jan 10 2017, 4:56 AM

Remove unnecessary -fPIC and also fix one-off error in .bss zeroing code

gonzo updated this revision to Diff 23820.Jan 10 2017, 5:12 AM

Move -fPIC to proper location

gonzo added a comment.Jan 10 2017, 5:16 AM
  • -fPIC is required by loader.efi so I moved the flag to proper location
  • the relocation_offset bug was due to off-by-one error in .bss zeroing routine. It wrote zero to __bss_end which AFAIU should be excluded from range. That address was actually entry for elf32_relocation_offset in offset table.
andrew accepted this revision.Jan 10 2017, 9:25 AM
andrew edited edge metadata.

Looks good, although it might pay to split into a few commits. We should also look at removing relocation_offset from the EFI code in a follow up.

This revision is now accepted and ready to land.Jan 10 2017, 9:25 AM
Closed by commit rS311888: [efi] Fix off-by-one error in ARM .bss zeroing code in loader's _start (authored by gonzo). · Explain WhyJan 10 2017, 8:08 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
head/
sys/
boot/
efi/
loader/
arch/
arm/
2 lines

Diff 23845

View Options

head/sys/boot/efi/loader/arch/arm/start.S

Loading...