Page MenuHomeFreeBSD

[ARM64] Provided initrd in "linux,initrd-start" and "linux,initrd-end" props of FDT /chosen node, mount it as an in-memory startup disk (mfsroot).
Needs ReviewPublic

Authored by dmitry_kernelgen.org on Mar 18 2023, 7:06 PM.
Tags
None
Referenced Files
F80158740: D39164.id119075.diff
Thu, Mar 28, 5:06 PM
Unknown Object (File)
Feb 4 2024, 7:14 PM
Unknown Object (File)
Jan 29 2024, 3:44 PM
Unknown Object (File)
Jan 23 2024, 7:09 AM
Unknown Object (File)
Dec 25 2023, 12:56 PM
Unknown Object (File)
Dec 24 2023, 11:42 AM
Unknown Object (File)
Dec 20 2023, 6:48 AM
Unknown Object (File)
Dec 15 2023, 4:21 PM

Details

Reviewers
manu
imp
Summary

The pupose of this is to support U-boot managed FreeBSD bootup by using
only binary images, without any device with a partition table.

This functionality has been ported from sys/powerpc/ofw/ofw_initrd.c

Please find the example U-boot usage below:

setenv kernel_comp_addr_r 0x3000000
setenv kernel_comp_size 0x04000000
setenv fdt_addr_r 0x2000000
dhcp
tftp $fdt_addr_r rk3328-rock64.dtb
tftp $kernel_addr_r kernel.bin.gz
setenv ramdisk_addr_r 0x07000000
tftp $ramdisk_addr_r mfsroot
fdt addr $fdt_addr_r
fdt chosen $ramdisk_addr_r $filesize
booti $kernel_addr_r - $fdt_addr_r

The "fdt chosen" command above embeds ramdisk setting into the actual fdt tree.

The "mfsroot" image is an uncompressed UFS disk image.

FreeBSD boot log should contain the following:

md0: Embedded image 11534336 bytes at 0xffffa0007c42b000

mountroot> ?

List of GEOM managed disk devices:
  ufsid/6416031dd9f2971da ufsid/6416031dd9f2971d md0a md0

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 50475
Build 47366: arc lint + arc unit

Event Timeline

One style nit that I can see. Otherwise looks good to me.

sys/arm64/ofw/ofw_initrd.c
96

A space is needed here after u_char

This revision is now accepted and ready to land.Mar 18 2023, 8:19 PM

Why is this in an arm64 specific directory? It doesn't appear to be arm64 specific.

sys/arm64/ofw/ofw_initrd.c
26–27

No need for __FBSDID, and cdefs.h can be removed as it's only needed for the macro.

sys/conf/files.arm64
341

The indentation looks wrong here and it should be optional fdt md_root_mem

Why is this in an arm64 specific directory? It doesn't appear to be arm64 specific.

Hi @andrew , good point, but I have no idea tbh. Actually, I'm copy-pasting the code which is already present in sys/powerpc/ofw/ofw_initrd.c. Should I instead move it to some common scope? If yes, which one? (My own understanding of source tree is insufficient).

Hi @luporl @leandro.lupori_gmail.com @bdragon ! I'm writing concering your work on https://reviews.freebsd.org/D15705 , https://reviews.freebsd.org/D20553 , https://reviews.freebsd.org/D23156 .The method of loading initrd from /chosen FDT node you have implemented in sys/powerpc/ofw/ofw_initrd.c is universally useful across many targets.

In this revision, I'm copy-pasting your code into ARM64 target. But what do you think about sharing sys/powerpc/ofw/ofw_initrd.c at a higher-level directory? Would it be possible to mask out the following PowerPC-specific lines (please see below). Would you be able to test the new revision on PowerPC target? Kindly looking forward to hearing your thoughts!

--- sys/powerpc/ofw/ofw_initrd.c	2023-03-08 11:45:31.308506000 +0100
+++ sys/arm64/ofw/ofw_initrd.c	2023-03-18 17:49:18.256364000 +0100
@@ -38,6 +38,7 @@
 #include <machine/bus.h>
 #include <machine/elf.h>
 #include <machine/param.h>
+#include <machine/vmparam.h>
 
 #include <dev/ofw/openfirm.h>
 #include <dev/ofw/ofw_bus.h>
@@ -61,11 +62,7 @@
 	pcell_t cell[2];
 	ssize_t size;
 	u_char *taste;
-	Elf_Ehdr ehdr;
 
-	if (!hw_direct_map)
-		return;
-
 	chosen = OF_finddevice("/chosen");
 	if (chosen <= 0)
 		return;
@@ -96,13 +93,6 @@
 
 	if (end - start > 0) {
 		taste = (u_char*) PHYS_TO_DMAP(start);
-		memcpy(&ehdr, taste, sizeof(ehdr));
-
-		if (IS_ELF(ehdr)) {
-			printf("ofw_initrd: initrd is kernel image!\n");
-			return;
-		}
-
 		mfs_root = taste;
 		mfs_root_size = end - start;
 		printf("ofw_initrd: initrd loaded at 0x%08lx-0x%08lx\n",
  • No need for __FBSDID, and cdefs.h can be removed as it's only needed for the macro
  • Correcting identation, adding fdt
This revision now requires review to proceed.Mar 18 2023, 11:21 PM

Updating D39164: [ARM64] Provided initrd in "linux,initrd-start" and "linux,initrd-end" props of FDT /chosen node, mount it as an in-memory startup disk (mfsroot).

Done with the changes!

  • Adding description into the file, so that everyone could know how to use it with U-boot
-	if (!hw_direct_map)
-		return;

You can write this as:

if (!PMAP_HAS_DMAP)
    return;

On powerpc it's defined to hw_direct_map, on arm64 it's 1.

-		if (IS_ELF(ehdr)) {
-			printf("ofw_initrd: initrd is kernel image!\n");
-			return;
-		}

Is there a reason to remove this? It's checking the file type of the passed in file.

Is there a reason to remove this? It's checking the file type of the passed in file.

This part looked to me too much as a dirty hack.

Hi @luporl @leandro.lupori_gmail.com @bdragon ! I'm writing concering your work on https://reviews.freebsd.org/D15705 , https://reviews.freebsd.org/D20553 , https://reviews.freebsd.org/D23156 .The method of loading initrd from /chosen FDT node you have implemented in sys/powerpc/ofw/ofw_initrd.c is universally useful across many targets.

In this revision, I'm copy-pasting your code into ARM64 target. But what do you think about sharing sys/powerpc/ofw/ofw_initrd.c at a higher-level directory? Would it be possible to mask out the following PowerPC-specific lines (please see below). Would you be able to test the new revision on PowerPC target? Kindly looking forward to hearing your thoughts!

Hi @dmitry_kernelgen.org ! I think it is a good idea to share this code with other targets. But I guess someone else may give you a better advice on where to move it to.

I saw that @andrew already handled the hw_direct_map part. It is needed because this code uses PHYS_TO_DMAP.

The IS_ELF(ehdr) part was added in D23156 to allow kernel images to be handled differently. When running on PowerNV, boot is done through kexec and we end up loosing some information from the kernel image.
When a kernel image is passed as initrd, this allows PowerPC specific code to retrieve more accurate symbol information and provide better stack tracing and debugging.
It may be removed if it isn't useful for other targets, but it would be nice to keep this behavior for PowerPC, if this code is moved to another location.

About testing it, I'm not very active on PowerPC development anymore. But maybe @jhibbits, @pkubaj or @alfredo can help with it or provide additional feedback.