[PATCH 7/8] x86: enable 64-bit kernel boot from 64-bit U-Boot
Park, Aiden
aiden.park at intel.com
Wed Apr 29 08:17:21 CEST 2020
Hi Simon,
> -----Original Message-----
> From: Simon Glass <sjg at chromium.org>
> Sent: Sunday, April 26, 2020 1:16 PM
> To: Park, Aiden <aiden.park at intel.com>
> Cc: Bin Meng <bmeng.cn at gmail.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>
> Subject: Re: [PATCH 7/8] x86: enable 64-bit kernel boot from 64-bit U-Boot
>
> Hi Aiden,
>
> On Tue, 21 Apr 2020 at 18:45, <aiden.park at intel.com> wrote:
> >
> > From: Aiden Park <aiden.park at intel.com>
> >
> > Signed-off-by: Aiden Park <aiden.park at intel.com>
> > ---
> > arch/x86/cpu/x86_64/Makefile | 2 +-
> > arch/x86/cpu/x86_64/call64.S | 21 +++++++++++++++++++++
> > arch/x86/cpu/x86_64/cpu.c | 10 ++++++++++
> > arch/x86/include/asm/bootparam.h | 10 +++++++++-
> > arch/x86/include/asm/zimage.h | 2 +-
> > arch/x86/lib/bootm.c | 10 +++++++---
> > arch/x86/lib/zimage.c | 24 ++++++++++++++++--------
> > 7 files changed, 65 insertions(+), 14 deletions(-) create mode
> > 100644 arch/x86/cpu/x86_64/call64.S
> >
> > diff --git a/arch/x86/cpu/x86_64/Makefile
> > b/arch/x86/cpu/x86_64/Makefile index 400f0ffe39..951e503a1f 100644
> > --- a/arch/x86/cpu/x86_64/Makefile
> > +++ b/arch/x86/cpu/x86_64/Makefile
> > @@ -3,4 +3,4 @@
> > # Written by Simon Glass <sjg at chromium.org> #
> >
> > -obj-y += cpu.o interrupts.o setjmp.o
> > +obj-y += cpu.o interrupts.o setjmp.o call64.o
> > diff --git a/arch/x86/cpu/x86_64/call64.S
> > b/arch/x86/cpu/x86_64/call64.S new file mode 100644 index
> > 0000000000..e2cfe15080
> > --- /dev/null
> > +++ b/arch/x86/cpu/x86_64/call64.S
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (C) 2020 Intel Corporation <www.intel.com> */
> > +
> > +.code64
> > +.globl cpu_call64
> > +cpu_call64:
> > + /*
> > + * cpu_call64(ulong pgtable, ulong setup_base, ulong target)
> > + *
> > + * rdi - pgtable (unused - already in 64-bit with paging)
> > + * rsi - setup_base
> > + * rdx - target
> > + *
> > + */
> > + cli
> > + mov %rdx, %rcx
> > + mov %rsi, %rdx
> > + call *%rcx
> > + ret
> > diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c
> > index 90925e46ea..0730c43f1c 100644
> > --- a/arch/x86/cpu/x86_64/cpu.c
> > +++ b/arch/x86/cpu/x86_64/cpu.c
> > @@ -7,6 +7,7 @@
> > #include <common.h>
> > #include <cpu_func.h>
> > #include <debug_uart.h>
> > +#include <asm/cpu.h>
> >
> > /*
> > * Global declaration of gd.
> > @@ -67,3 +68,12 @@ int x86_cpu_reinit_f(void) {
> > return 0;
> > }
> > +
> > +int cpu_jump_to_64bit(ulong setup_base, ulong load_address) {
> > + ulong target = load_address + 0x200;
> > +
> > + cpu_call64(0, setup_base, target);
> > +
> > + return -EFAULT;
> > +}
> > diff --git a/arch/x86/include/asm/bootparam.h
> > b/arch/x86/include/asm/bootparam.h
> > index d961dddc9e..73c94a33ec 100644
> > --- a/arch/x86/include/asm/bootparam.h
> > +++ b/arch/x86/include/asm/bootparam.h
> > @@ -59,7 +59,15 @@ struct setup_header {
> > __u32 initrd_addr_max;
> > __u32 kernel_alignment;
> > __u8 relocatable_kernel;
> > - __u8 _pad2[3];
> > + __u8 min_alignment;
> > + __u16 xloadflags;
> > +#define XLF_KERNEL_64 BIT(0)
> > +#define XLF_CAN_BE_LOADED_ABOVE_4G BIT(1)
> > +#define XLF_EFI_HANDOVER_32 BIT(2)
> > +#define XLF_EFI_HANDOVER_64 BIT(3)
> > +#define XLF_EFI_KEXEC BIT(4)
> > +#define XLF_5LEVEL BIT(5)
> > +#define XLF_5LEVEL_ENABLED BIT(6)
> > __u32 cmdline_size;
> > __u32 hardware_subarch;
> > __u64 hardware_subarch_data;
> > diff --git a/arch/x86/include/asm/zimage.h
> > b/arch/x86/include/asm/zimage.h index 80e128ccf3..cadeb04168 100644
> > --- a/arch/x86/include/asm/zimage.h
> > +++ b/arch/x86/include/asm/zimage.h
> > @@ -31,7 +31,7 @@
> > #define ZIMAGE_LOAD_ADDR 0x10000
> >
> > struct boot_params *load_zimage(char *image, unsigned long kernel_size,
> > - ulong *load_addressp);
> > + ulong *load_addressp, bool
> > + *image_64bit);
>
> Please put a 'p' on the end, so image_64bitp, since it a return value.
Thanks. I will do that.
>
> > int setup_zimage(struct boot_params *setup_base, char *cmd_line, int
> auto_boot,
> > unsigned long initrd_addr, unsigned long
> > initrd_size); void setup_video(struct screen_info *screen_info); diff
> > --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c index
> > 07d8f1f279..ebed9e4605 100644
> > --- a/arch/x86/lib/bootm.c
> > +++ b/arch/x86/lib/bootm.c
> > @@ -74,6 +74,7 @@ static int boot_prep_linux(bootm_headers_t *images)
> > void *data = NULL;
> > size_t len;
> > int ret;
> > + bool image_64bit;
> >
> > #ifdef CONFIG_OF_LIBFDT
> > if (images->ft_len) {
> > @@ -116,7 +117,8 @@ static int boot_prep_linux(bootm_headers_t *images)
> > ulong load_address;
> > char *base_ptr;
> >
> > - base_ptr = (char *)load_zimage(data, len, &load_address);
> > + base_ptr = (char *)load_zimage(data, len, &load_address,
> > + &image_64bit);
> > if (!base_ptr) {
> > puts("## Kernel loading failed ...\n");
> > goto error;
> > @@ -124,6 +126,10 @@ static int boot_prep_linux(bootm_headers_t
> *images)
> > images->os.load = load_address;
> > cmd_line_dest = base_ptr + COMMAND_LINE_OFFSET;
> > images->ep = (ulong)base_ptr;
> > +#if CONFIG_IS_ENABLED(X86_64)
> > + if (image_64bit)
>
> if (CONFIG_IS_ENABLED(X86_64) && image_64bit)
>
> > + images->os.arch = IH_ARCH_X86_64; #endif
> > } else if (images->ep) {
> > cmd_line_dest = (void *)images->ep + COMMAND_LINE_OFFSET;
> > } else {
> > @@ -164,9 +170,7 @@ int boot_linux_kernel(ulong setup_base, ulong
> load_address, bool image_64bit)
> > * TODO(sjg at chromium.org): Support booting both 32-bit and
> > * 64-bit kernels from 64-bit U-Boot.
> > */
> > -#if !CONFIG_IS_ENABLED(X86_64)
>
> if()
I will do that.
>
> > return cpu_jump_to_64bit(setup_base, load_address);
> > -#endif
> > } else {
> > /*
> > * Set %ebx, %ebp, and %edi to 0, %esi to point to the
> > diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index
> > ffc09630b7..c84e2d39b2 100644
> > --- a/arch/x86/lib/zimage.c
> > +++ b/arch/x86/lib/zimage.c
> > @@ -129,7 +129,7 @@ static int setup_device_tree(struct setup_header
> > *hdr, const void *fdt_blob) }
> >
> > struct boot_params *load_zimage(char *image, unsigned long kernel_size,
> > - ulong *load_addressp)
> > + ulong *load_addressp, bool
> > + *image_64bit)
>
> image_64bitp
I will do that.
>
> > {
> > struct boot_params *setup_base;
> > int setup_size;
> > @@ -179,6 +179,9 @@ struct boot_params *load_zimage(char *image,
> unsigned long kernel_size,
> > big_image = (bootproto >= 0x0200) &&
> > (hdr->loadflags & BIG_KERNEL_FLAG);
> >
> > + /* Determine 64-bit kernel */
> > + *image_64bit = (hdr->xloadflags & XLF_KERNEL_64) ? true :
> > + false;
> > +
>
> Drop ()
>
> In fact I suspect you can do:
>
> *image_64bit = hdr->xloadflags & XLF_KERNEL_64)
I will do that.
>
> > /* Determine load address */
> > if (big_image)
> > *load_addressp = BZIMAGE_LOAD_ADDR; @@ -313,12 +316,13
> > @@ void __setup_pcat_compatibility(void) int do_zboot(cmd_tbl_t
> > *cmdtp, int flag, int argc, char *const argv[]) {
> > struct boot_params *base_ptr;
> > - void *bzImage_addr = NULL;
> > + void *bzimage_addr = NULL;
>
> Good change, but really should be in a separate patch.
Let me make a separate patch for this.
>
> > ulong load_address;
> > char *s;
> > - ulong bzImage_size = 0;
> > + ulong bzimage_size = 0;
> > ulong initrd_addr = 0;
> > ulong initrd_size = 0;
> > + bool image_64bit;
> >
> > disable_interrupts();
> >
> > @@ -333,11 +337,11 @@ int do_zboot(cmd_tbl_t *cmdtp, int flag, int argc,
> char *const argv[])
> > }
> >
> > if (s)
> > - bzImage_addr = (void *)simple_strtoul(s, NULL, 16);
> > + bzimage_addr = (void *)simple_strtoul(s, NULL, 16);
> >
> > if (argc >= 3) {
> > /* argv[2] holds the size of the bzImage */
> > - bzImage_size = simple_strtoul(argv[2], NULL, 16);
> > + bzimage_size = simple_strtoul(argv[2], NULL, 16);
> > }
> >
> > if (argc >= 4)
> > @@ -346,8 +350,13 @@ int do_zboot(cmd_tbl_t *cmdtp, int flag, int argc,
> char *const argv[])
> > initrd_size = simple_strtoul(argv[4], NULL, 16);
> >
> > /* Lets look for */
> > - base_ptr = load_zimage(bzImage_addr, bzImage_size, &load_address);
> > + base_ptr = load_zimage(bzimage_addr, bzimage_size, &load_address,
> > + &image_64bit); #if
> > +!CONFIG_IS_ENABLED(X86_64)
>
> if()
>
> But why not just set it always?
This was just to keep original boot flow in zboot to boot 32-bit kernel entry from 32-bit U-Boot.
I also agree with you. 32-bit U-Boot is able to boot 64-bit kernel entry with 64-bit paging.
Let me remove this.
>
> > + image_64bit = false;
> > +#endif
> >
> > + /* we assume that the kernel is in place */
> > if (!base_ptr) {
> > puts("## Kernel loading failed ...\n");
> > return -1;
> > @@ -358,8 +367,7 @@ int do_zboot(cmd_tbl_t *cmdtp, int flag, int argc,
> char *const argv[])
> > return -1;
> > }
> >
> > - /* we assume that the kernel is in place */
> > - return boot_linux_kernel((ulong)base_ptr, load_address, false);
> > + return boot_linux_kernel((ulong)base_ptr, load_address,
> > + image_64bit);
> > }
> >
> > U_BOOT_CMD(
> > --
> > 2.20.1
> >
>
> Regards,
> Simon
Best Regards,
Aiden
More information about the U-Boot
mailing list