Page MenuHomeFreeBSD

riscv: look for bootargs in FDT

Authored by on Jul 2 2020, 2:13 AM.



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

rS FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

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 has landed. My guess is the best place for it is parse_metadata around where we currently call init_static_kenv, to match arm64.

1108 ↗(On Diff #74007)

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). 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.

1108 ↗(On Diff #74007)

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.

846 ↗(On Diff #74022)

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.

846 ↗(On Diff #74022)

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

if (kern_envp == NULL)

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

846 ↗(On Diff #74022)

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.