Re: [v3 PATCH 5/5] RISC-V: Add EFI stub support.

[Ard Biesheuvel]

On Mon, 20 Apr 2020 at 09:59, Atish Patra <atishp@xxxxxxxxxxxxxx> wrote:
>
> On Mon, Apr 20, 2020 at 12:04 AM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> >
> > On Mon, 20 Apr 2020 at 06:20, Atish Patra <Atish.Patra@xxxxxxx> wrote:
> > >
...
> > >
> > > "If the preferred address is set as the base of DRAM, efi_bs_call is
> > > bound to fail as well because the base of DRAM is reserved by the
> > > firmware. So the efi memory allocator can't allocate at that address.
> > > Technically, it will work but it is no different than passing
> > > ULONG_MAX. So I thought ULONG_MAX will avoid the confusion.
> > >
> > > We try to allocate as low as possible so I am passing dram_base as the
> > > minimum address anyways. As the firmware reserved the first few KBs,
> > >
> >
> >
> > OK, so the preferred address *is* the base of DRAM (assuming it is 2
> > MB aligned). However, in the general case, you keep some firmware
> > state there (couple of KB) and so you typically end up at DRAM base
> > plus 2 MB?
> >
>
> Yes.
>
> > So first question: why does the firmware put this stuff at the base of
> > DRAM in the first place? Does it *have* to live there?
> >
>
> The firmware includes the RISC-V specific supervisor binary interface (SBI)[[1]
> implementation. As it is a RISC-V specific run time service provider,
> it has to be resident in memory.
> The arm equivalent of SBI would be PSCI.
>
> [1] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
>

I am not asking why it has to be resident in memory. I am asking why
it has to be at the *base* of memory.

> > Then, if the base of DRAM is guaranteed to be occupied, why not make
> > the preferred address base of DRAM + 2 MB ? (or 4 MB for the 32-bit
> > case)
>
> I guess that will work too. I was inclined to use low_alloc_above so
> that we ensure that the kernel
> boots from the lowest possible address given the alignment
> restriction. If the alignment restrictions are relaxed,
> in future, we just have to change the macro.
>
> However, I think calling relocate_kernel with a preferred offset
> (dram_base + KIMG_ALIGN) is
> better because it will always fall back to low_alloc_above anyways. I
> will send the patch.

Indeed. In the common case, it will just do the allocate_pages()
directly, which is preferred. It will fall back to
efi_low_alloc_aboce() [and do the memory map parsing and traversal
etc] only if needed.