Use linker sets for loader variant dependent forth words
ClosedPublic
Actions

Authored by imp on Oct 4 2016, 5:30 AM.

Details

Reviewers

dteske
emaste
allanjude
sbruno
tsoome

Commits

rS307324: Create a new linker set, Xficl_compile_set which contains a list of

Summary

Create a new linker set, Xficl_compile_set which contains a list of
functions to call at the appropriate time to register new forth
words. In the past we've done this with ifdef soup, but now if the
file is included in the build, we'll get the new forth words.

Use this new functionality to move the pci bios stuff out of loader.c
by moving it to biospci.c.

Move the pnp functionality to common/pnp.c.

Move the inb/outb forth words to the i386 sysdep.c file where their
implementation is defined.

Adjust the efi linker scripts and build machinery to cope.

Test Plan

boot i386 bios machine and see if the pcibios forth words are defined

boot i386 uefi machine and see if they are missing (and maybe define a efi one to test the linker set stuff there)
make universe

I have a Intel Galileo board that will boot UEFI (tested with another review). I'll verify this bit there.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

imp updated this revision to Diff 21010.Oct 4 2016, 5:30 AM

imp retitled this revision from to Use linker sets for loader variant dependent forth words .

imp updated this object.

imp edited the test plan for this revision. (Show Details)

imp added reviewers: emaste, dteske, tsoome, allanjude, sbruno.

imp set the repository for this revision to rS FreeBSD src repository - subversion.

Herald added subscribers: rpokala, andrew. · View Herald TranscriptOct 4 2016, 5:30 AM

sbruno edited the test plan for this revision. (Show Details)Oct 4 2016, 2:27 PM

sbruno added a child revision: D7801: Start of EFI32 boot loader support.Oct 4 2016, 3:34 PM

Does sys/boot/efi/Makefile need i386 added to the EFI architectures?

Index: sys/boot/efi/Makefile
===================================================================
--- sys/boot/efi/Makefile
+++ sys/boot/efi/Makefile
@@ -14,7 +14,8 @@
 
 .if ${MACHINE_CPUARCH} == "aarch64" || \
     ${MACHINE_CPUARCH} == "amd64" || \
-    ${MACHINE_CPUARCH} == "arm"
+    ${MACHINE_CPUARCH} == "arm" || \
+    ${MACHINE_CPUARCH} == "i386"
 SUBDIR+=	libefi loader boot1
 .endif

sbruno mentioned this in D7801: Start of EFI32 boot loader support.Oct 4 2016, 4:18 PM

In D8145#168795, @sbruno wrote:

Does sys/boot/efi/Makefile need i386 added to the EFI architectures?

Index: sys/boot/efi/Makefile
===================================================================
--- sys/boot/efi/Makefile
+++ sys/boot/efi/Makefile
@@ -14,7 +14,8 @@
 
 .if ${MACHINE_CPUARCH} == "aarch64" || \
     ${MACHINE_CPUARCH} == "amd64" || \
-    ${MACHINE_CPUARCH} == "arm"
+    ${MACHINE_CPUARCH} == "arm" || \
+    ${MACHINE_CPUARCH} == "i386"
 SUBDIR+=	libefi loader boot1
 .endif

Likely, but not by me. I don't think this affects that.

@bsdimp can you generate the diff with context (e.g. git diff -U 9999 or svn diff --diff-cmd=diff -x -U999999 on https://wiki.freebsd.org/action/show/Phabricator)

 |  ____|             |  _ \ / ____|  __ \ 
 | |___ _ __ ___  ___ | |_) | (___ | |  | |
 |  ___| '__/ _ \/ _ \|  _ < \___ \| |  | |
 | |   | | |  __/  __/| |_) |____) | |__| |
 | |   | | |    |    ||     |      |      |
 |_|   |_|  \___|\___||____/|_____/|_____/    ```                        `
                                             s` `.....---.......--.```   -/
 +============Welcome to FreeBSD===========+ +o   .--`         /y:`      +.
 |                                         |  yo`:.            :o      `+-
 |  1. Boot Multi User [Enter]             |   y/               -/`   -o/
 |  2. Boot [S]ingle User                  |  .-                  ::/sy+:.
 |  3. [Esc]ape to loader prompt           |  /                     `--  /
 |  4. Reboot                              | `:                          :`
 |                                         | `:                          :`
 |  Options:                               |  /                          /
 |  5. [K]ernel: kernel (1 of 2)           |  .-                        -.
 |  6. Configure Boot [O]ptions...         |   --                      -.
 |                                         |    `:`                  `:`
 |                                         |      .--             `--.
 |                                         |         .---.....----. 
 +=========================================+
                                          

/boot/kernel/kernel text=0x13c1e63 data=0xe6474+0x2dc6b4 syms=[0x4+0xeb6e0+0x4+0
x179f5e]
Booting...

This revision is now accepted and ready to land.Oct 4 2016, 6:09 PM

sbruno removed a child revision: D7801: Start of EFI32 boot loader support.Oct 4 2016, 6:16 PM

sbruno added a parent revision: D7801: Start of EFI32 boot loader support.

emaste added inline comments.Oct 4 2016, 6:54 PM

sys/boot/efi/loader/arch/amd64/ldscript.amd64
43–47 ↗	(On Diff #21010)	Does it work if you don't add this? ld should put the linker sets as orphan sections in the right place and generate the `__start_` and `__stop_` symbols automatically I believe. Assuming this is true we should remove the explicit `set_Xcommand_set` ones too.
sys/boot/ficl/ficl.h
1156 ↗	(On Diff #21010)	Linker sets are by definition a compile/build time construct so `COMPILE` seems somehow redundant. What about FICL_ADDWORDS_SET?
1158 ↗	(On Diff #21010)	What is the `X` prefix for?

In D8145#168842, @emaste wrote:

@bsdimp can you generate the diff with context (e.g. git diff -U 9999 or svn diff --diff-cmd=diff -x -U999999 on https://wiki.freebsd.org/action/show/Phabricator)

I used git to generate this, so I'm still trying to get a feel for how to cope.

imp added inline comments.Oct 4 2016, 7:46 PM

sys/boot/efi/loader/arch/amd64/ldscript.amd64
43–47 ↗	(On Diff #21010)	I'll investigate.
sys/boot/ficl/ficl.h
1156 ↗	(On Diff #21010)	Compile was intentional because all the other ficl functions that registered words were ficlCompileFoo.
1158 ↗	(On Diff #21010)	pattern matching to Xcommand_set

emaste added inline comments.Oct 4 2016, 7:51 PM

sys/boot/ficl/ficl.h
1156 ↗	(On Diff #21010)	Oh, because they're compiled-in ficl words rather than added by Forth I guess -- OK.
1158 ↗	(On Diff #21010)	We should probably drop it in both spots