Handle load from loader(8)

[mhorne]

Summary

In locore, we must detect and handle different arguments passed by
loader(8) compared to what we have when booting directly via SBI
firmware. Currently we receive the hart ID in a0 and a pointer to the
device tree blob in a1. loader(8) provides only a pointer to its
metadata.

The only tricky part to this is that the hart ID of the boot hart is
expected to have been stored in the device tree by a previous booting
stage. This is performed by u-boot v2020.07 and onwards, but without
this we must fail to boot as there is no way for a hart to obtain its
own ID while executing in supervisor mode.

Fix-up initriscv() to parse the loader's metadata, continuing to use
fake_preload_metadata() in the direct boot case.

[mhorne]

Remove a duplicated section of code from a mis-merge.

[jrtc27]

There is no guarantee that mhartid does not collide with the kernel's VA space. Unlikely, yes (especially since the spec encourages reducing the magnitude of the largest mhartid), but completely permitted. The only restriction on mhartid is this:

Hart IDs might not necessarily be numbered contiguously in a multiprocessor system, but at least one hart must have a hart ID of zero.

Why can we not make loader conform to the standard RISC-V boot interface, putting mhartid in a0 and the dtb address in a1? It can shove the address of modulep somewhere in the mangled FDT. I really don't like the idea of guessing whether an integer is an address or not. Alternatively, we could provide a different entry point for loader, if it really needs the current interface.

[mhorne]

In D24912#548332, @jrtc27 wrote:

There is no guarantee that mhartid does not collide with the kernel's VA space. Unlikely, yes (especially since the spec encourages reducing the magnitude of the largest mhartid), but completely permitted. The only restriction on mhartid is this:

Hart IDs might not necessarily be numbered contiguously in a multiprocessor system, but at least one hart must have a hart ID of zero.

Why can we not make loader conform to the standard RISC-V boot interface, putting mhartid in a0 and the dtb address in a1? It can shove the address of modulep somewhere in the mangled FDT. I really don't like the idea of guessing whether an integer is an address or not. Alternatively, we could provide a different entry point for loader, if it really needs the current interface.

Providing modulep as the first and only argument seems to be standardized across the various loader(8) implementations, although there may be an example that differs. However, u-boot does not provide the boot hart ID as an argument to EFI payloads (nor, as I discovered, simple payloads invoked with the "go" command). The solution of stuffing the ID in the device tree and reading it later is what Linux plans on using for its EFI implementation.

Admittedly I don't love the check against the address either. I can see about providing an alternate entry point, but a hart id > 0xffffffc000000000 is an edge-case I'm not all that worried about.

[jrtc27]

https://lists.denx.de/pipermail/u-boot/2020-February/401563.html suggests Linux is going the way of an alternate entry point, FWIW.

[mhorne]

Okay, I've reworked this a bit and I am happier with it. Now, CPUs will enter the _alt_start routine from SBI firmware (placed first in .text), while loader(8) will continue to read the ELF entry point and send the boot CPU to _start. With this version there is no guesswork involved as to which way we booted, and both routines can perform their own fixups before entering the common page table setup.

[mhorne]

sys/riscv/riscv/locore.S

81 ↗

(On Diff #72493)

Note that I've dropped the #ifdef SMP requirement for storing to boot_hart as I believe it is better that the boot CPU always have access to its hart ID. The PLIC driver requires it, for example, and could cause issues if a !SMP system was running on a hart != 0.

[mhorne]

Rebase.

[mhorne]

Ping. @jrtc27 do you have any thoughts on this version?

[jrtc27]

I don't know the details of loader, but this looks like a much better approach and doesn't perturb the SBI paths much so I'm happy.

[markj]

sys/riscv/riscv/machdep.c

879 ↗

(On Diff #72829)

In this line both boot_hart and hart are uint32_t, so the cast doesn't really make sense.

We seem to consistently use uint32_t for hart IDs (e.g., boot_hart, pc_hart), but per Jessica's initial point there is no apparent requirement that the IDs fit in 32 bits on RV64.

[markj]

Looks ok to me modulo the one comment about the cast. I'm fine with committing this if the cast is dropped, since the issue around the type used for hart IDs is preexisting.

[mhorne]

In D24912#561272, @markj wrote:

Looks ok to me modulo the one comment about the cast. I'm fine with committing this if the cast is dropped, since the issue around the type used for hart IDs is preexisting.

Will do. We should probably convert hart ID types to u_long in the future, but there is no motivator for this at the moment.

sys/riscv/riscv/machdep.c

879 ↗

(On Diff #72829)

I think it was left from an earlier iteration where I converted boot_hart to a u_long. Thanks for catching this.