riscv: honor the environment set via the kernel config file

[mhorne]

Summary

Initialize the static kernel boot environment, which makes the riscv boot process honor the environment set via the kernel config file

Sponsored by: Axiado

Test Plan

Compile & boot a riscv system/kernel, with some read only tunables set in the kernel config file, e.g.

envvar         "net.fibs=5"
envvar         "net.pf.source_nodes_hashsize=16384"

Verify the read only tunables' values have been correctly applied.

[philip]

This looks good -- and is known to work. The size of the environment should probably be a constant rather than a magic number though.

sys/riscv/riscv/machdep.c

130 ↗

(On Diff #72353)

Can this be a defined constant rather than a magic number?

[mhorne]

sys/riscv/riscv/machdep.c

130 ↗

(On Diff #72353)

Would you consider a different name? This has nothing to do with boot1.

886 ↗

(On Diff #72353)

If the only goal is to honor the environment variables from the config, then there is no need to provide a buffer at all; you can simply call init_static_kenv(NULL, 0) and it will perform the initialization. The buffer is useful only if we wanted to manually set some environment variables via calls to kern_setenv().

[kp]

sys/riscv/riscv/machdep.c

130 ↗

(On Diff #72353)

arm calls it static_kenv (and also uses 4096 as its size).
It may make sense to just use PAGE_SIZE.

886 ↗

(On Diff #72353)

I believe we currently don't have a need for kern_setenv() during boot.

I'd keep the buffer for two reasons:

• It's what the arm and mips platforms do as well
• We may load drivers or other code that does call kern_setenv(). For example, the nfs boot code calls kern_setenv("boot.netif.name", ifctx->ifp->if_xname);.

[mhorne]

sys/riscv/riscv/machdep.c

886 ↗

(On Diff #72353)

Sorry, I should have clarified: the buffer is only useful for calls to kern_setenv() that occur early in boot, such as in initriscv.

At SI_SUB_KMEM + 1 we switch to a dynamic kenv, after which the two static environments (one from config and the one we would be creating here) are no longer used. This occurs before any driver code is loaded. You can see what I mean in init_dynamic_kenv.

[syrinx]

sys/riscv/riscv/machdep.c

130 ↗

(On Diff #72353)

static char static_kenv[PAGE_SIZE]; perhaps?

886 ↗

(On Diff #72353)

Ideally, at some point we should support setting those values via entries in the device tree blob for example, or some u-boot environment area perhaps; for that we need the static buffer if I understand correctly

I could remove the buffer and call init_static_kenv(NULL, 0) so we have at least the kernel envvar config option to modify the read-only tunables during kernel build without modifying the source code, and add those other options, incl. the buffer in a later patch...

[mhorne]

sys/riscv/riscv/machdep.c

886 ↗

(On Diff #72353)

Those features would be useful, and if you intend on adding them soon then it's up to you if you want to include the buffer in this patch or later.

[mhorne]

sys/riscv/riscv/machdep.c

886 ↗

(On Diff #72353)

By the way, I have some pending changes to this section of the code in D24912 that will conflict with this. It might be better to place the call to init_static_kenv() in fake_preload_metadata.

[mhorne]

I've discovered this change should have gone in before rS363345, and is now required to avoid an early panic when trying to parse kenv variables from bootargs. Taking over the review so I can update with a rebased version.

[mhorne]

Rebase the patch. Change the buffer name and use PAGE_SIZE instead of 4096.

[mhorne]

Ping. Are we okay with this patch as is?

[jrtc27]

arm64 puts this in parse_fdt_bootargs

[mhorne]

In D25034#608641, @jrtc27 wrote:

arm64 puts this in parse_fdt_bootargs

It does. I find this placement slightly more intuitive, but if you disagree then I can move it to that function.

[kp]

I'm happy with it as-is.