Try to get the base of the DRAM from a DTB to use it as a lowest address
in physical memory to relocate the kernel. If it is not possible to
obtain the base from a /memory node of the DTB let's make an assumption
that the DRAM base at the beginning of the memory.
Signed-off-by: Nikita Ermakov <[email protected]>
---
drivers/firmware/efi/libstub/riscv-stub.c | 39 ++++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
index 380e4e251399..1b5944276e1a 100644
--- a/drivers/firmware/efi/libstub/riscv-stub.c
+++ b/drivers/firmware/efi/libstub/riscv-stub.c
@@ -46,6 +46,39 @@ static u32 get_boot_hartid_from_fdt(void)
return fdt32_to_cpu(*prop);
}
+static unsigned long get_dram_base_from_fdt(void)
+{
+ const void *fdt;
+ int node, len;
+ const fdt32_t *addr_cells;
+ const void *prop;
+
+ fdt = get_efi_config_table(DEVICE_TREE_GUID);
+ if (!fdt)
+ return ULONG_MAX;
+
+ node = fdt_path_offset(fdt, "/");
+ if (node < 0)
+ return ULONG_MAX;
+
+ addr_cells = fdt_getprop((void *)fdt, node, "#address-cells", &len);
+ if (!addr_cells)
+ return ULONG_MAX;
+
+ node = fdt_path_offset(fdt, "/memory");
+ if (node < 0)
+ return ULONG_MAX;
+
+ prop = fdt_getprop((void *)fdt, node, "reg", &len);
+ if (!prop)
+ return ULONG_MAX;
+
+ if (fdt32_to_cpu(*addr_cells) > 1)
+ return fdt64_to_cpu(*((fdt64_t *)prop));
+ else
+ return fdt32_to_cpu(*((fdt32_t *)prop));
+}
+
efi_status_t check_platform_features(void)
{
hartid = get_boot_hartid_from_fdt();
@@ -97,7 +130,11 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
* lowest possible memory region as long as the address and size meets
* the alignment constraints.
*/
- preferred_addr = MIN_KIMG_ALIGN;
+ preferred_addr = get_dram_base_from_fdt();
+ if (preferred_addr == ULONG_MAX)
+ preferred_addr = MIN_KIMG_ALIGN;
+ else
+ preferred_addr += MIN_KIMG_ALIGN;
status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
preferred_addr, MIN_KIMG_ALIGN, 0x0);
--
2.29.3
On Tue, 27 Apr 2021 at 14:07, Nikita Ermakov <[email protected]> wrote:
>
> Try to get the base of the DRAM from a DTB to use it as a lowest address
> in physical memory to relocate the kernel. If it is not possible to
> obtain the base from a /memory node of the DTB let's make an assumption
> that the DRAM base at the beginning of the memory.
>
Why?
> Signed-off-by: Nikita Ermakov <[email protected]>
> ---
> drivers/firmware/efi/libstub/riscv-stub.c | 39 ++++++++++++++++++++++-
> 1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
> index 380e4e251399..1b5944276e1a 100644
> --- a/drivers/firmware/efi/libstub/riscv-stub.c
> +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> @@ -46,6 +46,39 @@ static u32 get_boot_hartid_from_fdt(void)
> return fdt32_to_cpu(*prop);
> }
>
> +static unsigned long get_dram_base_from_fdt(void)
> +{
> + const void *fdt;
> + int node, len;
> + const fdt32_t *addr_cells;
> + const void *prop;
> +
> + fdt = get_efi_config_table(DEVICE_TREE_GUID);
> + if (!fdt)
> + return ULONG_MAX;
> +
> + node = fdt_path_offset(fdt, "/");
> + if (node < 0)
> + return ULONG_MAX;
> +
> + addr_cells = fdt_getprop((void *)fdt, node, "#address-cells", &len);
> + if (!addr_cells)
> + return ULONG_MAX;
> +
> + node = fdt_path_offset(fdt, "/memory");
> + if (node < 0)
> + return ULONG_MAX;
> +
> + prop = fdt_getprop((void *)fdt, node, "reg", &len);
> + if (!prop)
> + return ULONG_MAX;
> +
> + if (fdt32_to_cpu(*addr_cells) > 1)
> + return fdt64_to_cpu(*((fdt64_t *)prop));
> + else
> + return fdt32_to_cpu(*((fdt32_t *)prop));
> +}
> +
> efi_status_t check_platform_features(void)
> {
> hartid = get_boot_hartid_from_fdt();
> @@ -97,7 +130,11 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> * lowest possible memory region as long as the address and size meets
> * the alignment constraints.
> */
> - preferred_addr = MIN_KIMG_ALIGN;
> + preferred_addr = get_dram_base_from_fdt();
> + if (preferred_addr == ULONG_MAX)
> + preferred_addr = MIN_KIMG_ALIGN;
> + else
> + preferred_addr += MIN_KIMG_ALIGN;
> status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> preferred_addr, MIN_KIMG_ALIGN, 0x0);
>
> --
> 2.29.3
>
On Tue, 27 Apr 2021 at 15:43, Ard Biesheuvel <[email protected]> wrote:
>
> On Tue, 27 Apr 2021 at 14:07, Nikita Ermakov <[email protected]> wrote:
> >
> > Try to get the base of the DRAM from a DTB to use it as a lowest address
> > in physical memory to relocate the kernel. If it is not possible to
> > obtain the base from a /memory node of the DTB let's make an assumption
> > that the DRAM base at the beginning of the memory.
> >
>
> Why?
>
You right, this patch is useless. I made it because I overlooked
efi_low_alloc_above() in libstub/relocate.c
Sorry for bothering.
I had an issue with booting a Linux kernel with EFI stub on a HiFive
Unleashed board.
My boot chain was as follows: ZSBL -> FSBL -> OpenSBI -> U-Boot
(bootefi) -> Linux
When I tried to boot the kernel, U-Boot throws an Store/AMO access
fault exception.
I checked the riscv-stub.c and libstub/relocate.c and I thought that
it tries to relocate the kernel at the 2 MiB.
With this patch the kernel started to boot and I thought that the
theory was right.
But, after further investigation I found out that U-Boot EFI does not
mark the first 4 KiB page of DRAM as EFI_BOOT_SERVICE_DATA on the
board and efi_low_alloc_above() allocates this region as conventional
memory. The lowest region of DRAM is protected and that is why I got
the exception when the kernel tries to relocate to the 0x80000000. The
reason why U-Boot EFI does not mark the first 4 KiB of DRAM as
EFI_BOOT_SERVICE_DATA was that there wasn't /reserved-memory node in a
DTB. I had the FSBL with a patch which enables an entire L2 cache and
it does not preserve a0, a1 registers before a jump to the OpenSBI.
So, OpenSBI didn't fixup the DTB where it adds the /reserved-memory
node.
> > Signed-off-by: Nikita Ermakov <[email protected]>
> > ---
> > drivers/firmware/efi/libstub/riscv-stub.c | 39 ++++++++++++++++++++++-
> > 1 file changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
> > index 380e4e251399..1b5944276e1a 100644
> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > @@ -46,6 +46,39 @@ static u32 get_boot_hartid_from_fdt(void)
> > return fdt32_to_cpu(*prop);
> > }
> >
> > +static unsigned long get_dram_base_from_fdt(void)
> > +{
> > + const void *fdt;
> > + int node, len;
> > + const fdt32_t *addr_cells;
> > + const void *prop;
> > +
> > + fdt = get_efi_config_table(DEVICE_TREE_GUID);
> > + if (!fdt)
> > + return ULONG_MAX;
> > +
> > + node = fdt_path_offset(fdt, "/");
> > + if (node < 0)
> > + return ULONG_MAX;
> > +
> > + addr_cells = fdt_getprop((void *)fdt, node, "#address-cells", &len);
> > + if (!addr_cells)
> > + return ULONG_MAX;
> > +
> > + node = fdt_path_offset(fdt, "/memory");
> > + if (node < 0)
> > + return ULONG_MAX;
> > +
> > + prop = fdt_getprop((void *)fdt, node, "reg", &len);
> > + if (!prop)
> > + return ULONG_MAX;
> > +
> > + if (fdt32_to_cpu(*addr_cells) > 1)
> > + return fdt64_to_cpu(*((fdt64_t *)prop));
> > + else
> > + return fdt32_to_cpu(*((fdt32_t *)prop));
> > +}
> > +
> > efi_status_t check_platform_features(void)
> > {
> > hartid = get_boot_hartid_from_fdt();
> > @@ -97,7 +130,11 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > * lowest possible memory region as long as the address and size meets
> > * the alignment constraints.
> > */
> > - preferred_addr = MIN_KIMG_ALIGN;
> > + preferred_addr = get_dram_base_from_fdt();
> > + if (preferred_addr == ULONG_MAX)
> > + preferred_addr = MIN_KIMG_ALIGN;
> > + else
> > + preferred_addr += MIN_KIMG_ALIGN;
> > status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > preferred_addr, MIN_KIMG_ALIGN, 0x0);
> >
> > --
> > 2.29.3
> >
--
Thanks,
Nikita
B8 00 4C CD 21
On Wed, 28 Apr 2021 at 18:00, Nikita Ermakov <[email protected]> wrote:
>
> On Tue, 27 Apr 2021 at 15:43, Ard Biesheuvel <[email protected]> wrote:
> >
> > On Tue, 27 Apr 2021 at 14:07, Nikita Ermakov <[email protected]> wrote:
> > >
> > > Try to get the base of the DRAM from a DTB to use it as a lowest address
> > > in physical memory to relocate the kernel. If it is not possible to
> > > obtain the base from a /memory node of the DTB let's make an assumption
> > > that the DRAM base at the beginning of the memory.
> > >
> >
> > Why?
> >
>
> You right, this patch is useless. I made it because I overlooked
> efi_low_alloc_above() in libstub/relocate.c
> Sorry for bothering.
>
> I had an issue with booting a Linux kernel with EFI stub on a HiFive
> Unleashed board.
> My boot chain was as follows: ZSBL -> FSBL -> OpenSBI -> U-Boot
> (bootefi) -> Linux
> When I tried to boot the kernel, U-Boot throws an Store/AMO access
> fault exception.
> I checked the riscv-stub.c and libstub/relocate.c and I thought that
> it tries to relocate the kernel at the 2 MiB.
> With this patch the kernel started to boot and I thought that the
> theory was right.
> But, after further investigation I found out that U-Boot EFI does not
> mark the first 4 KiB page of DRAM as EFI_BOOT_SERVICE_DATA on the
> board and efi_low_alloc_above() allocates this region as conventional
> memory. The lowest region of DRAM is protected and that is why I got
> the exception when the kernel tries to relocate to the 0x80000000. The
> reason why U-Boot EFI does not mark the first 4 KiB of DRAM as
> EFI_BOOT_SERVICE_DATA was that there wasn't /reserved-memory node in a
> DTB. I had the FSBL with a patch which enables an entire L2 cache and
> it does not preserve a0, a1 registers before a jump to the OpenSBI.
> So, OpenSBI didn't fixup the DTB where it adds the /reserved-memory
> node.
>
OK, thanks for the explanation. I will disregard this patch then.