| Message ID | 20210427120607.2646166-1-arei@altlinux.org |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: Relocate the kernel relative to a DRAM base. | expand |
On Tue, 27 Apr 2021 at 15:43, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Tue, 27 Apr 2021 at 14:07, Nikita Ermakov <arei@altlinux.org> 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 <arei@altlinux.org> > > --- > > 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 Wed, 28 Apr 2021 at 18:00, Nikita Ermakov <arei@altlinux.org> wrote: > > On Tue, 27 Apr 2021 at 15:43, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Tue, 27 Apr 2021 at 14:07, Nikita Ermakov <arei@altlinux.org> 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.
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);
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 <arei@altlinux.org> --- drivers/firmware/efi/libstub/riscv-stub.c | 39 ++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-)