On Tue, Nov 23, 2021 at 12:53 PM Jessica Clarke [off-list ref] wrote:

On 23 Nov 2021, at 03:44, Anup Patel [off-list ref] wrote:

quoted

+Alex

On Tue, Nov 23, 2021 at 7:27 AM [off-list ref] wrote:

quoted

From: Guo Ren <redacted>

Using riscv.fw_size in cmdline to tell the kernel what the
firmware (opensbi) size is. Then reserve the proper size of
firmware to save memory instead of the whole 2MB. It's helpful
to satisfy a small memory system (D1s/F133 from Allwinner).

Signed-off-by: Guo Ren <redacted>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Anup Patel <redacted>
Cc: Atish Patra <redacted>
---
arch/riscv/mm/init.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 920e78f8c3e4..f7db6d40213d 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c

@@ -159,6 +159,15 @@ static int __init early_mem(char *p)

}
early_param("mem", early_mem);

+static phys_addr_t firmware_size __initdata;
+static int __init early_get_firmware_size(char *arg)
+{
+       firmware_size = memparse(arg, &arg);
+
+       return 0;
+}
+early_param("riscv.fwsz", early_get_firmware_size);
+

We have avoided any RISC-V specific kernel parameter till now
and I don't think adding "riscv.fwsz" is the right approach.

OpenSBI adds a reserved memory node (mmode_resv@8000000)
to mark the memory where it is running as reserved. In fact, all
M-mode runtime firmware should be adding a reserved memory
node just like OpenSBI.

Yes I agree that this should be in the device tree, IMO there is no
need to introduce a new kernel parameter.

BBL does not do this and, even if it’s modified today, older versions
will still need to be supported for quite a while longer.

It's fair to expect the firmware to advertise its existence: we
briefly discussed that last year with Atish [1] and he proposed to
introduce a document that describes what the kernel expects from the
'platform' when it boots, that would be a way to drop those old legacy
bootloaders.

[1] https://lkml.org/lkml/2020/6/3/696

In FreeBSD[1] we only reserve the first 2 MiB of DRAM (we don’t care
about RV32) if there is no reserved memory node covering the DRAM base
address, which avoids this issue. The only downside with that approach
is that if firmware occupies a different region than the beginning of
DRAM (or there is no firmware resident in the supervisor’s physical
address space, as is the case for a virtualised guest) then it
unnecessarily reserves that first 2 MiB, but that’s not a huge deal,
and can’t be avoided so long as BBL continues to exist (well, I guess
you could probe the SBI implementation ID if you really cared about
that, but I’ve yet to hear of a platform where the SBI implementation,
if it exists, isn’t at the start of DRAM, and if you’re virtualising
then you probably have enough DRAM that you don’t notice 2 MiB going
missing).

Jess

[1] https://github.com/freebsd/freebsd-src/blob/main/sys/riscv/riscv/machdep.c#L554-L568