Page MenuHomeFreeBSD

loader: add LoongArch EFI loader support
Needs ReviewPublic

Authored by zhaoxiaoqiang007_gmail.com on Tue, Sep 22, 2:23 PM.
Tags
None
Referenced Files
F173241803: D59889.id187405.diff
Thu, Sep 24, 3:08 PM
F173235773: D59889.id.diff
Thu, Sep 24, 1:54 PM
F173218072: D59889.id187450.diff
Thu, Sep 24, 9:36 AM
F173216652: D59889.diff
Thu, Sep 24, 9:17 AM
Unknown Object (File)
Wed, Sep 23, 8:40 AM
Unknown Object (File)
Wed, Sep 23, 1:36 AM
Unknown Object (File)
Tue, Sep 22, 10:04 PM
Unknown Object (File)
Tue, Sep 22, 9:53 PM

Details

Summary

Add LoongArch support to the EFI loader: loader/arch/loongarch
(start.S, exec.c, ldscript), ficl sysdep files and libsa/libefi
build glue, following the existing riscv loader.

Clang FreeBSD target for LoongArch is landed upstream after clang 20.1.0,
see https://github.com/llvm/llvm-project/pull/119191

Signed-off-by: Xiaoqiang Zhao <zhaoxiaoqiang007@gmail.com>
Signed-off-by: yu shan wei <mpysw@vip.163.com>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 77189
Build 74072: arc lint + arc unit

Event Timeline

This is cool! I'd love to take it for a test-spin!
One way to help that out would be to add loongarch support to src/tools/boot/boot-test.sh. That way I'd have a qemu recipe and can make sure my various loader changes don't break anything.

lib/libc/loongarch/gen/_setjmp.S
3

I think you want your copyrights generally. This file was completely rewritten, so it should have your copyright on it :).

stand/efi/loader/main.c
1068

how is left uninitialized in loongarch. Could we initialize it to zero before the #ifndef?

In D59889#1374842, @imp wrote:

This is cool! I'd love to take it for a test-spin!
One way to help that out would be to add loongarch support to src/tools/boot/boot-test.sh. That way I'd have a qemu recipe and can make sure my various loader changes don't break anything.

Thanks for reviewing!

I think I need to figure out how to write this test script ;-)
If you want to give it a quick spin,this is pretty much what my development script looks like:
(replace the directory with your own, of course)

1. build kernel

MAKEOBJDIRPREFIX=/home/hitmoon/fb16 \
MK_TOOLCHAIN=no \
MK_WERROR=no \
MK_BOOT=yes \
TARGET=loongarch TARGET_ARCH=loongarch64 KERNCONF=QEMU make kernel-toolchain NOCLEAN=yes -j10

MAKEOBJDIRPREFIX=/home/hitmoon/fb16 \
MK_TOOLCHAIN=no \
MK_WERROR=no \
MK_BOOT=yes \
TARGET=loongarch TARGET_ARCH=loongarch64 KERNCONF=QEMU make buildkernel -j10

2. build world

MAKEOBJDIRPREFIX=/home/hitmoon/fb16 \
MK_TOOLCHAIN=no \
MK_WERROR=no \
MK_BOOT=yes \
MK_RESCUE=no \
TARGET=loongarch TARGET_ARCH=loongarch64 make  buildworld -j10

3. make qemu image

TARGET=loongarch
TARGET_ARCH=loongarch64
SRCTOP=/home/hitmoon/fb16_loongarch
OBJTOP=/home/hitmoon/fb16/home/hitmoon/fb16_loongarch/loongarch.loongarch64
STAGE=/home/hitmoon/fb16-stage
IMG=/home/hitmoon/loongarch64.img
MAKEOBJDIRPREFIX=/home/hitmoon/fb16

# --- Step 1: installworld + installkernel into staging dir ---
echo "==> Installing world into ${STAGE} ..."

# Clean staging dir
if [ -d "${STAGE}" ]; then
    chflags -R noschg "${STAGE}" 2>/dev/null || true
    rm -rf "${STAGE}"
fi
mkdir -p "${STAGE}"

cd ${SRCTOP}

MAKEOBJDIRPREFIX=${MAKEOBJDIRPREFIX} \
    MK_TOOLCHAIN=no \
    MK_WERROR=no \
    MK_BOOT=yes \
    MK_RESCUE=no \
    TARGET=${TARGET} TARGET_ARCH=${TARGET_ARCH} \
    DESTDIR=${STAGE} \
    NO_ROOT=yes \
    make installworld distribution

echo "==> Installing kernel ..."
MAKEOBJDIRPREFIX=${MAKEOBJDIRPREFIX} \
    MK_TOOLCHAIN=no \
    MK_WERROR=no \
    MK_BOOT=yes \
    MK_RESCUE=no \
    TARGET=${TARGET} TARGET_ARCH=${TARGET_ARCH} \
    DESTDIR=${STAGE} \
    KERNCONF=QEMU \
    NO_ROOT=yes \
    make installkernel

# --- Step 1.5: site customization (rc.conf, sshd root login, root password) ---
echo "==> Writing /etc/rc.conf ..."
cat > ${STAGE}/etc/rc.conf <<'EOF'
hostname="loong"
ifconfig_vtnet0="DHCP"
sshd_enable="YES"
EOF

echo "==> Enabling root login in /etc/ssh/sshd_config ..."
# The stock sshd_config has "#PermitRootLogin ..." commented out; append an
# uncommented directive (sshd uses the first uncommented occurrence).
echo "PermitRootLogin yes" >> ${STAGE}/etc/ssh/sshd_config

echo "==> Setting root password ..."
# SHA-512 crypt hash of "freebsd" (openssl passwd -6 -salt loongsalt freebsd).
ROOT_HASH='$6$loongsalt$8pil6bJ4atMUDICBnyn5Txgtjpa9SbOYMGY5fU8fROzCXLZ69.mYdRXx7bWuPx4/bH2/u4Nc0yJHWMrWW21v8.'
sed -i '' "s|^root:[^:]*:|root:${ROOT_HASH}:|" ${STAGE}/etc/master.passwd
# -d sets the output directory for pwd.db/spwd.db (default /etc); use the
# staging /etc so the distribution's locked-root databases are overwritten.
pwd_mkdb -d ${STAGE}/etc ${STAGE}/etc/master.passwd

# --- Step 1.6: /boot/loader.conf ---
echo "==> Writing /boot/loader.conf ..."
cat > ${STAGE}/boot/loader.conf <<'EOF'
#kern.maxcpus=1
#vm.memguard.frequency="0"
debug.vmmap_check=2
autoboot_delay="5"
EOF

cat > ${STAGE}/root/t.sh << EOF
kyua test -k /usr/tests/Kyuafile
EOF

# --- Step 2: Create EFI partition contents ---
echo "==> Preparing EFI partition ..."

# Find the EFI loader
EFI_LOADER=$(find ${OBJTOP}/stand/efi -name loader_lua.efi -type f 2>/dev/null | head -1)
if [ -z "${EFI_LOADER}" ]; then
    echo "WARNING: EFI loader not found, trying alternative locations ..."
    EFI_LOADER=$(find ${OBJTOP} -path '*/efi/*loader*.efi' -type f 2>/dev/null | head -1)
fi
if [ -z "${EFI_LOADER}" ]; then
    echo "ERROR: Cannot find EFI loader. Build stand/ first."
    echo "  Run: cd ${SRCTOP} && ./bl2-qemu.sh"
    exit 1
fi
echo "    EFI loader: ${EFI_LOADER}"

ESP_DIR=/tmp/esp.$$
rm -rf ${ESP_DIR}
mkdir -p ${ESP_DIR}/EFI/BOOT
cp ${EFI_LOADER} ${ESP_DIR}/EFI/BOOT/BOOTLOONGARCH64.EFI

# --- Step 3: Create filesystem images ---
echo "==> Creating UFS root filesystem image ..."

# Build the rootfs from METALOG (manifest mode): ownership comes from
# the manifest (root:wheel), not the on-disk staging tree, which is
# owned by the build user under -DNO_ROOT.

# Record the files created by this script (install(1) does not know
# about them).  -D below tolerates duplicates.
for f in ./etc/rc.conf ./boot/loader.conf ./root/t.sh ; do
    printf '%s type=file uname=root gname=wheel mode=0644\n' ${f} >> ${STAGE}/METALOG
done
# Sort: makefs needs directory entries before their contents, else
# directories get mode 000.
env -i LC_COLLATE=C sort -u ${STAGE}/METALOG > ${STAGE}/METALOG.sorted
mv ${STAGE}/METALOG.sorted ${STAGE}/METALOG

ROOTFS=/tmp/rootfs.$$.img
(cd ${STAGE} && makefs -B little -s 4g -D -N ./etc -o label=root ${ROOTFS} ./METALOG)

echo "==> Creating ESP (FAT) image ..."
ESPFS=/tmp/esp.$$.img
makefs -t msdos -s 64m ${ESPFS} ${ESP_DIR}

# --- Step 4: Assemble disk image ---
echo "==> Assembling disk image ..."
mkimg -s gpt \
    -p efi:=${ESPFS} \
    -p freebsd-ufs:=${ROOTFS} \
    -o ${IMG}

# Cleanup
rm -rf ${ESP_DIR} ${ROOTFS} ${ESPFS}

echo ""
echo "============================================================"
echo "==> Done: ${IMG}"
ls -lh ${IMG}

4. run in qemu ( version 10.0 or higher is preferred)

default root password is: freebsd

qemu-system-loongarch64 -M virt -smp 8 -m 8G \
-cpu la464,ptw=on \
-drive if=pflash,format=raw,readonly=on,file=/usr/share/edk2/loongarch64/QEMU_EFI.fd \
-drive if=pflash,format=raw,file=/usr/share/edk2/loongarch64/QEMU_VARS.fd \
-drive if=none,file=loongarch64.img,format=raw,id=rootfs \
-device virtio-blk-pci,drive=rootfs,disable-legacy=on,disable-modern=off,ioeventfd=off \
-device virtio-rng-pci \
-netdev user,id=net0,hostfwd=tcp::5588-:22 \
-device virtio-net-pci,netdev=net0 \
-nographic \
-serial stdio \
-monitor telnet::4444,server,nowait \
-qmp unix:/tmp/qmp.sock,server,nowait $@

It is suggested to remove it so that FP save/restore only covers $fs0–$fs7, consistent with the LoongArch-ELF-ABI Register Convention and other FreeBSD architectures.

$fcsr0 is not in any callee-saved list, so an ordinary function call does not guarantee that it is preserved across calls. FreeBSD's _setjmp/_longjmp traditionally restores only the callee-saved registers, $ra, $sp, and the signal mask of setjmp; also saving the caller-saved $fcsr0 goes beyond this convention.

LoongArch ELF ABI: https://loongson.github.io/LoongArch-Documentation/LoongArch-ELF-ABI-EN.html (see the "Register Convention" section).

lib/libc/loongarch/gen/_setjmp.S
72–74

It is suggested to remove it so that FP save/restore only covers $fs0–$fs7, consistent with the LoongArch-ELF-ABI and other FreeBSD architectures.

123–125

It is suggested to remove it so that FP save/restore only covers $fs0–$fs7, consistent with the LoongArch-ELF-ABI and other FreeBSD architectures.

It is suggested to remove it so that FP save/restore only covers $fs0–$fs7, consistent with the LoongArch-ELF-ABI Register Convention and other FreeBSD architectures.

$fcsr0 is not in any callee-saved list, so an ordinary function call does not guarantee that it is preserved across calls. FreeBSD's _setjmp/_longjmp traditionally restores only the callee-saved registers, $ra, $sp, and the signal mask of setjmp; also saving the caller-saved $fcsr0 goes beyond this convention.

LoongArch ELF ABI: https://loongson.github.io/LoongArch-Documentation/LoongArch-ELF-ABI-EN.html (see the "Register Convention" section).

Thanks for pointing this out

This HOB FDT fallback functionality is useful for both LoongArch and ArmVirt (as is also explicitly noted in the code comments), and it is not LoongArch-specific logic. It is suggested to split it out from the LoongArch port patch set and submit it separately as a standalone . This way:

  • ArmVirt can proceed independently and does not need to wait for the entire LoongArch port;
  • the review can be more focused and will not be mistaken for a LoongArch-specific workaround;
  • the commit history will be clearer, making future rollbacks or porting easier.
stand/efi/libefi/libefi.c
31–34

It is recommended that it be submitted separately from the LoongArch port patch set.

41–67

This HOB FDT fallback functionality applies to both LoongArch and ArmVirt (as is also explicitly stated in the code comments), and it is not LoongArch-specific logic. It is suggested to separate it from the LoongArch port patch set and submit it as a standalone patch.

83–96

It is recommended that it be submitted separately from the LoongArch port patch set.

MP Service Protocol query serves no real purpose, suggested to remove.

This code retrieves and prints the CPU count, but it has no actual impact on the boot flow. It introduces unnecessary dependencies on <Protocol/MpService.h> and <Pi/PiMultiPhase.h>.

If the MP Service Protocol is needed for multicore startup, it should be used where the CPUs are actually woken up, rather than being queried in exec() just for printing.

stand/efi/loader/arch/loongarch/exec.c
40–41

MP Service Protocol query serves no real purpose, suggested to remove.

68–84

MP Service Protocol query serves no real purpose, suggested to remove.

Passing entry itself and kernend to the kernel via registers is a custom ABI. If the kernel needs this information, it should obtain it from the modulep metadata (FreeBSD already has mechanisms such as MODINFOMD_KERNEND and MODINFOMD_MODULEP), rather than breaking cross-architecture consistency.

stand/efi/loader/arch/loongarch/exec.c
55

The kernel entry calling convention is inconsistent with other FreeBSD architectures. The current code passes three arguments to the kernel entry, whereas the EFI loaders of other FreeBSD architectures (ARM64, RISC-V, powerpc64, etc.) pass only one argument to the kernel.

In D59889#1375157, @imp wrote:

Much thanks!

stand/efi/loader/arch/loongarch/exec.c
68–84

MP Service Protocol query serves no real purpose, suggested to remove.

Yes, it's a debug print that should be removed.

dev_cleanup() must be called before bi_load().

dev_cleanup() must be called before bi_load(), because bi_load() internally calls ExitBootServices(), after which the Boot Service Protocols are no longer valid, and neither dev_cleanup() nor printf() can be used anymore.

stand/efi/loader/arch/loongarch/exec.c
86–97

dev_cleanup() must be called before bi_load().

It is suggested to use designated initializers for the struct file_format initialization.

stand/efi/loader/arch/loongarch/exec.c
102–105

It is suggested to use designated initializers for the struct file_format initialization.

Suggestion:

 struct file_format loongarch_elf = {
	.l_load = __elfN(loadfile),
	.l_exec = __elfN(exec)
};

Redundant includes and extern declaration.

stand/efi/loader/arch/loongarch/exec.c
28

<sys/cdefs.h> do not appear to be used in this file.

32–33

<machine/md_var.h>, and <machine/metadata.h> do not appear to be used in this file.

46

extern int bi_load(...) is redundant, as bootstrap.h or loader_efi.h already provides the declaration.

This comment was removed by shipujin.t_gmail.com.
stand/efi/loader/arch/loongarch/loongarch.ldscript
2

OUTPUT_ARCH(loongarch64) should be changed to OUTPUT_ARCH(loongarch).

Upstream reference: https://reviews.freebsd.org/D54962

It is suggested to remove the explicitly defined setcommand_set and set_Xficl_compile_set output sections, and just retain them via objcopy -j.

stand/efi/loader/arch/loongarch/loongarch.ldscript
38–47

It is suggested to remove the explicitly defined setcommand_set and set_Xficl_compile_set output sections, and just retain them via objcopy -j.

Conduct code review

stand/efi/loader/arch/loongarch/start.S
30

It is suggested to remove the 32-bit machine type definition.

59

Characteristics field of the COFF header needs to set IMAGE_FILE_EXECUTABLE (0x0002) for executable images.

.short  IMAGE_FILE_EXECUTABLE           /* Characteristics */
157

efi_main does not return, so it is suggested to use a tail jump b _C_LABEL(efi_main) instead of bl, to avoid unnecessarily saving the return address and to stay consistent with the RISC-V implementation.

code review

stand/efi/loader/main.c
49–51

main is machine-independent code, so it should not directly include machine-dependent headers such as <machine/loongarchreg.h>.
Please move the DMW0 mapping logic into an architecture-specific helper under stand/efi/loader/arch/loongarch/ or stand/libsa/, and have main.c only call an abstract interface.

834–842

Move the LoongArch DMW0 mapping out of Machine-Independent main.c into an arch-specific helper.

874–875

You wrote in the code comments that in the case where PA != VA, the "Memory Management of Page Table Mapping" mode is selected.
Calling acpi_map_sdt() for rsdt/xsdt in that fix is itself correct, but it should not be a side effect of a LoongArch #ifdef. On architectures where PA == VA, acpi_map_sdt() is just an identity mapping, so it is safe for all architectures. Please submit this change as a separate, architecture-independent bugfix.

Reference materials
https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#virtual-address-space-and-address-translation-mode

zhaoxiaoqiang007_gmail.com added inline comments.
lib/libc/loongarch/gen/_setjmp.S
3

will add in next reversion

stand/efi/loader/arch/loongarch/exec.c
86–97

Yes, it's origin from my code cleanup mass.

stand/efi/loader/arch/loongarch/loongarch.ldscript
2

very helpful

stand/efi/loader/arch/loongarch/start.S
30

make sense

stand/efi/loader/arch/loongarch/exec.c
102–105

okay

stand/efi/loader/arch/loongarch/loongarch.ldscript
38–47

okay

stand/efi/loader/arch/loongarch/start.S
157

efi_main does not return, so it is suggested to use a tail jump b _C_LABEL(efi_main) instead of bl, to avoid unnecessarily saving the return address and to stay consistent with the RISC-V implementation.

sounds better

stand/efi/loader/main.c
1068

I will remove the macro check around