riscv: look for bootargs in FDT

[freebsdphab-AX9_cmx.ietfng.org]

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.

[jrtc27]

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

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.

[jrtc27]

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]

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?

[freebsdphab-AX9_cmx.ietfng.org]

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

[freebsdphab-AX9_cmx.ietfng.org]

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

[mhorne]

sys/riscv/riscv/machdep.c

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.

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.

[freebsdphab-AX9_cmx.ietfng.org]

sys/riscv/riscv/machdep.c

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)
  parse_fdt_bootargs()

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

[mhorne]

sys/riscv/riscv/machdep.c

846 ↗

(On Diff #74022)

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

[freebsdphab-AX9_cmx.ietfng.org]

Incorporate feedback from @mhorne

[mhorne]

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