Page MenuHomeFreeBSD

riscv: look for bootargs in FDT
ClosedPublic

Authored by freebsdphab-AX9_cmx.ietfng.org on Jul 2 2020, 2:13 AM.
Tags
Referenced Files
Unknown Object (File)
Feb 25 2024, 7:45 PM
Unknown Object (File)
Feb 20 2024, 5:12 AM
Unknown Object (File)
Feb 12 2024, 11:40 PM
Unknown Object (File)
Feb 12 2024, 12:54 AM
Unknown Object (File)
Jan 31 2024, 5:14 PM
Unknown Object (File)
Dec 22 2023, 11:18 AM
Unknown Object (File)
Dec 20 2023, 4:11 AM
Unknown Object (File)
Nov 25 2023, 8:39 AM
Subscribers

Details

Summary

The FDT may contain a short /chosen/bootargs string which we should pass to boot_parse_cmdline. Notably, this allows the use of qemu's -append option to pass things like -s to boot to single user mode.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

arm64's parse_fdt_bootargs deals with loader(8) which will normally handle these for us when used, so we need to copy that approach given https://reviews.freebsd.org/D24912 has landed. My guess is the best place for it is parse_metadata around where we currently call init_static_kenv, to match arm64.

sys/riscv/riscv/machdep.c
919

This should be at the start of the function. Also, is 256 enough? The arm64 port uses 512 as another arbitrary constant, and as a global; personally I'd be inclined to just make it a 4K global buffer to provide plenty of space and not waste precious stack memory.

Also, please avoid filing revisions with CheriBSD tainting the diff; it can be misleading, as well as the fact that we're always a bit behind (and in this case it does matter because of the loader(8) patch landing).

freebsdphab-AX9_cmx.ietfng.org edited the test plan for this revision. (Show Details)

Revised in light of jrtc27's comments. Unfortunately, I lack a FreeBSD box to build and test this on (sorry, CheriBSD builds fine and I didn't think about our being lagged relative to upstream); perhaps someone who does would be so kind as to let me know if I've broken something?

Slightly less code motion; tested using qemu "-append -s" on FreeBSD machine I'd forgotten about.

Sorry, newbie learning way around arc. Seems it grabbed an unintended commit.

sys/riscv/riscv/machdep.c
846

As was pointed out, if we booted via loader(8) then it has already done this parsing work. There's nothing wrong with doing it again, but we can avoid it.

919

Our init stack is small (4 pages), but I would expect there to be very little on the stack at this point in boot. Perhaps this buffer should live there since it is a one-time use. Perhaps not, since that could easily come back to bite us one day.

sys/riscv/riscv/machdep.c
846

While I believe the premise that loader(8) can have handled all this, testing suggests that perhaps that should be

if (kern_envp == NULL)
  parse_fdt_bootargs()

But I may be turned around; kern_environment.c is entirely novel to me. :)

sys/riscv/riscv/machdep.c
846

You are correct. I made a mistake and got the conditional backwards. Sorry about that!

Thanks, looks good to me, and will be quite useful! I gave it a quick test as well.

This revision is now accepted and ready to land.Jul 17 2020, 2:06 PM
This revision was automatically updated to reflect the committed changes.