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

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

1047

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

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

1047

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

1047

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.