stand: Retire arch_loadaddr, pending its replacement
ClosedPublic
Actions

Authored by imp on Wed, May 28, 9:49 PM.

Details

Reviewers

chs
kevans
jhibbits
tsoome
andrew
manu

Commits

rG6aea7b224a34: stand: Retire arch_loadaddr using md_align, pending its replacement

Summary

arch_loadaddr() is inconsistently used. In fact, I removed it entirely
from kboot because its interface wasn't wide enough (though I broke
powerpc kboot several years ago, and this commit might fix that). For
now, modify uboot to put it into copyin like all the other loaders
do. This might break arm ubldr.

In its place, use the new md_align() and any replacement will be called
from there, or via some explicit init() function.

These changes allow me to load a amd64 kernel via UEFI with all the
modules aligned to 16k.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 64518
Build 61402: arc lint + arc unit

Event Timeline

imp created this revision.Wed, May 28, 9:49 PM

Herald added a reviewer: andrew. · View Herald TranscriptWed, May 28, 9:49 PM

Herald added a reviewer: manu. · View Herald Transcript

Herald added subscribers: dab, emaste. · View Herald Transcript

imp requested review of this revision.Wed, May 28, 9:49 PM

Harbormaster completed remote builds in B64518: Diff 156234.Wed, May 28, 9:49 PM

imp added a parent revision: D50586: stand: Style nit.Wed, May 28, 9:49 PM

imp added a child revision: D50588: style: Use static consistently.

chs added inline comments.Wed, May 28, 11:15 PM

stand/kboot/kboot/arch/amd64/elf64_freebsd.c
182	This comment is now missing a word, or something. Maybe you didn't really mean to remove "to"?
stand/kboot/kboot/arch/powerpc64/ppc64_elf_freebsd.c
93	Missing a word now here too, like above.
stand/uboot/copy.c
63–67	This part of the comment is needs to be updated to match the new code.
103–104	KERN_ALIGN is defined to be the compile-time PAGE_SIZE constant for platforms other than 32-bit arm, is that really what is desired for this math? I would guess this ought to use the runtime variable that you add later, but I don't see any change that does that.

imp added inline comments.Thu, May 29, 6:14 PM

stand/uboot/copy.c
63–67	correct. will delete.
103–104	I think that's the right math. _start is put at the beginning of the ubldr. So we want to round that down to a page on non-arm platforms, or to the 2MB boundary on arm32 platforms. Since we can't load the kernel aligned to smaller than KERN_ALIGN, this creates the right bounds for ubldr in memory for us to avoid when we're looking for a place to put the kernel. That's slightly independent of the align better. the md_align stuff only is for modules subsequent to the kernel, not the kernel itself, so even then I think this is appropriate. But I also don't care if it doesn't work completely on ubldr since it's a tier-2 fast becoming a tier-3 feature. But I do know it's used. I'm unsure the right interface for this. Maybe if mmel commits his arm64 ubldr stuff I'd take a look again?