Memory reserved by opensbi not indicated in devicetree on qemu virt platform?

[ghost]

This is with the fw_jump.elf binary from the 0.1 release. I'm loading it with qemu's "-kernel" option. According to the ELF header, it occupies memory address 0x80000000.

So far so good. But the flattened devicetree passed via a1 indicates that this is normal, usable memory (if I'm parsing the devicetree data correctly). If my payload attempts to write to this memory, qemu's emulated CPU mysteriously stops doing anything useful.

Apparently this memory is reserved. By a conservative estimate, this means that all memory from 0x80000000 to 0x80200000 (fw_jump.elf 64 bit load address) is reserved by the SBI. Indeed, SBI logs this to the qemu serial console:

PMP0: 0x0000000080000000-0x000000008001ffff (A)

The problem here is that there is no indication anywhere to the SBI client that it is not allowed to use this memory. (The PMP registers are not readable from supervisor mode, if I read riscv-privileged-v1.10.pdf correctly.)

I believe opensbi should mark this as reserved memory range using the devicetree Memory Reservation Block. Or alternatively, not include this memory in any "/memory" node.

I've only tested with the 0.1 release (sorry), but I didn't see any commits in the git log that seemed to touch this. Not sure how exactly Linux handles this. I might be overlooking something.

[avpatel]

Yes, we are aware of the need to update reserved memory details in DTB.

Many thanks for bringing up this topic.

We are mostly booting Linux as S-mode OS so we never noticed this issue because we boot Linux from next 2MB aligned address after RAM start at 0x80200000 and Linux OF automatically reserves memory range from RAM start to Linux boot address (i.e. 0x80000000 to 0x80200000). Due to this, we kept ignoring this issue.

Of course, we should consider any possible S-Mode client and we should reserve 128KB of memory from RAM start (i.e. memory typically used by OpenSBI firmware) using reserved memory DT binding.

[atishp04]

This would mean we edit device tree for all the platforms. The approach would be to provide additional SBI calls to provide PMP protected address, which is not super nice either.

This should be a common scenario for any other ISA as well. Do we know how does it work in ARM world ?

I am not entirely opposed to the idea. Just thinking if there is a better way to do it.

[jmi2k]

What about providing a standard way of querying memory regions from supervisor mode through an extension? It's required very early during boot, yet a FDT parser is needed in the payload in order to obtain a memory map (which is redundant given that OpenSBI already walks the DT). Doing so would allow the kernel to get reliable information about the memory available on-demand from the firmware, and the firmware could override that information at will.

[ghost]

Why not use another argument register to pass a pointer to a list with free/usable memory regions? That would be the simplest solution.

[jmi2k]

I imagine an scenario where changing that information at runtime would be desirable and an extension would give more flexibility, but probably I'm over-engineering. A pointer as an argument is simple and desirable if the info is not going to change though. I think some discussion and careful consideration is needed.

[fintelia]

You don't need a full device tree parser to extract the memory reservation block from a DTB. Getting that information is super simple: there is a header with a field specifying the offset from the start of the DTB to an array of 64-bit address+size pairs listing out the reserved regions of memory. The first entry with size=0 indicates the end of the list. The only complication is that all the values are in big endian but implementing a bswap function isn't that hard.

[ghost]

It sounded to me like the concern seemed to be mostly about having to touch the FDT in the firmware at all. You could also argue that it'd still kind of annoying to get to that part of the FDT (I know, it's basically just about reading an offset in the FDT header and then jumping to it).
If you parse the FDT much later in the boot process, you'd still need to have 2 pieces of code which read the FDT somehow.
But sure, could be a non-issue.

[jmi2k]

Well I think I'm going to create another issue as it seems I've hijacked this one

[avpatel]

I believe we can close this issue because memory reservation in DT is now done by OpenSBI.