risc-v/spike: reserve memory for SBI

[axel-h]

No description provided.

[axel-h]

@alistair23: I'd appreciate your feedback also about this patch and any additional thoughts on reserving space for SBI.

[alistair23]

OpenSBI will do the same thing automatically at boot: https://github.com/riscv/opensbi/blob/4ef2f5d3e6b25356a8fab19574bed76ce60edeee/lib/utils/fdt/fdt_fixup.c#L156

From what I can tell seL4 kernel doesn't parse the DT at runtime, in which case this change looks good.

[axel-h]

Any objections to merge this? Seem without this spike cannot be used. The automated checks for links fails because of #371

[lsf37]

LGTM

[kent-mcleod]

Any objections to merge this? Seem without this spike cannot be used. The automated checks for links fails because of #371

There are parts of the kernel's riscv tooling scripts that already try and insert this reserved region into the memory space after the device tree is passed: https://github.com/seL4/seL4/blob/master/tools/hardware/config.py#L51

I think adding it into the device tree is better, but that means we should remove the assumptions that it isn't there from elsewhere.

[kent-mcleod]

This would also likely allow this discussion to be resolved: https://github.com/seL4/seL4/pull/363/files#r633223133

[kent-mcleod]

I think the treatment of avoiding the SBI firmware is also encoded here (https://github.com/seL4/seL4/blob/master/include/arch/riscv/arch/64/mode/hardware.h#L95)

[axel-h]

Thanks for the comments, I will rework this PR to address them. However this turns out as not trivial and needs some more time

[axel-h]

With all the hacks removed, This works now when I pass FW_PAYLOAD_OFFSET=0x04000000 to the OpenSBI build. All that is missing here now is taking the ElfLoder (calculated) base address here instead of hard-coding it.

[axel-h]

I'm also removing include/plat/spike/plat/instance/rocket-chip/hardware.h. This file should be obsolete by now, as this "instance hack" has been removed already.

[kent-mcleod]

@ssrg-bamboo proof

[axel-h]

Seem we have to apply the DTS overlay in all RISC-V platform now, otherwise thins will not work. I wonder if we should add DTB parsing code to the ELF-Loder to catch some error. As @alistair23 said, OpenSBI modifies the DTB that is passes to the ELF-Loader and carves out the reserved memory. Since we don't do runtime DTB parsing in the kernel ,as least the ELF-Loader could do this and do a consistency check when it copies around the images. We saw a crash there when to tries to move the kernel image to 0x80000000 on the HiFive, because this was missing the DTS overlay patch.

[kent-mcleod]

Can you give an update of what you're attempting here @axel-h? Adding the reserved region to the device tree should be sufficient to ensure that the kernel doesn't use it as untypeds. Is the rest of the changes to try and ensure that the reserved memory isn't also given to user level as device untypeds? This may not be something the kernel needs to restrict. If you do wish that the kernel restrict this, then hardware.yml is already used to select device nodes in the device tree that are not to be given to user level as untypeds. This would avoid having to make all the code changes for adding new types of reserved regions to the kernel boot code.

[axel-h]

Yes, this is trying to ensure the kernel does not give any untypeds that cover the memory used by the SBI. Rational is, that this memory should be considered unusable practically, so there is not much point making this avaiable. In the long run, a proper RISC-V S-Mode should have ways to lock this downs somehow (PMP ...), so accesses eventually trap anway. But having a generic mechanism in seL4 to carve out memory region to they don't even become device untypeds seem useful to me, too.

[kent-mcleod]

But having a generic mechanism in seL4 to carve out memory region to they don't even become device untypeds seem useful to me, too.

What is your threat model in this case? Any policy to restrict access to hardware resources that the kernel doesn't use can already be implemented within the existing access control mechanisms. Adding an extra layer of restrictions doesn't enhance the existing mechanisms unless you are assuming that the roottask is compromised.