[PATCH v2 00/16] x86: zboot: Enhance the 'zboot' command

This command is currently monolithic and does not support scripts which want to adjust the boot process. This series updates it to be more like 'bootm', in that it has sub-commands for each stage of the boot. This allows some stages to be adjusted or skipped.
It also adds a way to dump out the setup block.
With these changes it is possible to boot an x86 Chrome OS image from a script.
Changes in v2: - Fix comment about argv[0] in do_zboot_parent() - Add a comment explaining the logic for a specified setup-base address
Simon Glass (16): x86: Update the bootparam header x86: zimage: Use a state struct to hold the state x86: zimage: Avoid using #ifdef x86: zboot: Move kernel-version code into a function x86: zboot: Correct image type x86: zimage: Disable interrupts just before booting x86: zboot: Set up a sub-command structure x86: zboot: Add a 'go' subcommand x86: zboot: Add an 'info' subcommand x86: zboot: Add an 'setup' subcommand x86: zboot: Set environment variables for image locations x86: zboot: Allow setting a separate setup base address x86: zboot: Add an option to dump the setup information x86: zboot: Allow overriding the command line cros: Update chromium documentation cros: Add information about booting Chrome OS on x86
README | 4 + arch/x86/include/asm/bootparam.h | 25 +- arch/x86/include/asm/e820.h | 1 + arch/x86/include/asm/zimage.h | 30 +- arch/x86/lib/bootm.c | 2 +- arch/x86/lib/zimage.c | 483 +++++++++++++++++++++++++++---- doc/README.chromium | 41 ++- 7 files changed, 520 insertions(+), 66 deletions(-)

This header is missing a few of the newer features from the specification. Add these as well as a link to the spec. Also use the BIT() macros where appropriate.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
arch/x86/include/asm/bootparam.h | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h index d961dddc9e1..7a3c1f51554 100644 --- a/arch/x86/include/asm/bootparam.h +++ b/arch/x86/include/asm/bootparam.h @@ -24,6 +24,11 @@ struct setup_data { __u8 data[0]; };
+/** + * struct setup_header - Information needed by Linux to boot + * + * See https://www.kernel.org/doc/html/latest/x86/boot.html + */ struct setup_header { __u8 setup_sects; __u16 root_flags; @@ -43,15 +48,16 @@ struct setup_header { __u16 kernel_version; __u8 type_of_loader; __u8 loadflags; -#define LOADED_HIGH (1<<0) -#define QUIET_FLAG (1<<5) -#define KEEP_SEGMENTS (1<<6) -#define CAN_USE_HEAP (1<<7) +#define LOADED_HIGH BIT(0) +#define KASLR_FLAG BIT(1) +#define QUIET_FLAG BIT(5) +#define KEEP_SEGMENTS BIT(6) /* Obsolete */ +#define CAN_USE_HEAP BIT(7) __u16 setup_move_size; __u32 code32_start; __u32 ramdisk_image; __u32 ramdisk_size; - __u32 bootsect_kludge; + __u32 bootsect_kludge; /* Obsolete */ __u16 heap_end_ptr; __u8 ext_loader_ver; __u8 ext_loader_type; @@ -59,7 +65,13 @@ struct setup_header { __u32 initrd_addr_max; __u32 kernel_alignment; __u8 relocatable_kernel; - __u8 _pad2[3]; + u8 min_alignment; +#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) + u16 xloadflags; __u32 cmdline_size; __u32 hardware_subarch; __u64 hardware_subarch_data; @@ -69,6 +81,7 @@ struct setup_header { __u64 pref_address; __u32 init_size; __u32 handover_offset; + u32 kernel_info_offset; } __attribute__((packed));
struct sys_desc_table {

+Andy Shevchenko
On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
This header is missing a few of the newer features from the specification. Add these as well as a link to the spec. Also use the BIT() macros where appropriate.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
arch/x86/include/asm/bootparam.h | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h index d961dddc9e1..7a3c1f51554 100644 --- a/arch/x86/include/asm/bootparam.h +++ b/arch/x86/include/asm/bootparam.h @@ -24,6 +24,11 @@ struct setup_data { __u8 data[0]; };
+/**
- struct setup_header - Information needed by Linux to boot
Now I am confused. This kernel document says:
Protocol 2.14 BURNT BY INCORRECT COMMIT ae7e1238e68f2a472a125673ab506d49158c1889 (x86/boot: Add ACPI RSDP address to setup_header) DO NOT USE!!! ASSUME SAME AS 2.13.
However in U-Boot there was a commit from Andy saying that:
commit d905aa8a4277e200e11fdf6d73a7c76d0e6f34a4 (<=== U-Boot commit) Author: Andy Shevchenko andriy.shevchenko@linux.intel.com Date: Fri Sep 13 18:42:00 2019 +0300
x86: zImage: Propagate acpi_rsdp_addr to kernel via boot parameters
...
after upstream got eventually the Linux kernel
commit e6e094e053af75cbc164e950814d3d084fb1e698 Author: Juergen Gross jgross@suse.com Date: Tue Nov 20 08:25:29 2018 +0100
x86/acpi, x86/boot: Take RSDP address from boot params if available
So there are 2 commits in the Linux kernel that do the same thing, one in boot_params, the other one in setup_header?
e6e094e053af75cbc164e950814d3d084fb1e698 : x86/acpi, x86/boot: Take RSDP address from boot params if available ae7e1238e68f2a472a125673ab506d49158c1889 : x86/boot: Add ACPI RSDP address to setup_header
- */
struct setup_header { __u8 setup_sects; __u16 root_flags; @@ -43,15 +48,16 @@ struct setup_header { __u16 kernel_version; __u8 type_of_loader; __u8 loadflags; -#define LOADED_HIGH (1<<0) -#define QUIET_FLAG (1<<5) -#define KEEP_SEGMENTS (1<<6) -#define CAN_USE_HEAP (1<<7) +#define LOADED_HIGH BIT(0) +#define KASLR_FLAG BIT(1) +#define QUIET_FLAG BIT(5) +#define KEEP_SEGMENTS BIT(6) /* Obsolete */ +#define CAN_USE_HEAP BIT(7) __u16 setup_move_size; __u32 code32_start; __u32 ramdisk_image; __u32 ramdisk_size;
__u32 bootsect_kludge;
__u32 bootsect_kludge; /* Obsolete */ __u16 heap_end_ptr; __u8 ext_loader_ver; __u8 ext_loader_type;
@@ -59,7 +65,13 @@ struct setup_header { __u32 initrd_addr_max; __u32 kernel_alignment; __u8 relocatable_kernel;
__u8 _pad2[3];
u8 min_alignment;
+#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)
u16 xloadflags; __u32 cmdline_size; __u32 hardware_subarch; __u64 hardware_subarch_data;
@@ -69,6 +81,7 @@ struct setup_header { __u64 pref_address; __u32 init_size; __u32 handover_offset;
u32 kernel_info_offset;
} __attribute__((packed));
struct sys_desc_table {
Regards, Bin

On Tue, Sep 01, 2020 at 04:23:45PM +0800, Bin Meng wrote:
On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
This header is missing a few of the newer features from the specification. Add these as well as a link to the spec. Also use the BIT() macros where appropriate.
+/**
- struct setup_header - Information needed by Linux to boot
Now I am confused. This kernel document says:
Protocol 2.14 BURNT BY INCORRECT COMMIT ae7e1238e68f2a472a125673ab506d49158c1889 (x86/boot: Add ACPI RSDP address to setup_header) DO NOT USE!!! ASSUME SAME AS 2.13.
Where did you get this "DO NOT USE!!! ASSUME SAME AS 2.13" from?
However in U-Boot there was a commit from Andy saying that:
commit d905aa8a4277e200e11fdf6d73a7c76d0e6f34a4 (<=== U-Boot commit) Author: Andy Shevchenko andriy.shevchenko@linux.intel.com Date: Fri Sep 13 18:42:00 2019 +0300
x86: zImage: Propagate acpi_rsdp_addr to kernel via boot parameters
...
after upstream got eventually the Linux kernel commit e6e094e053af75cbc164e950814d3d084fb1e698 Author: Juergen Gross <jgross@suse.com> Date: Tue Nov 20 08:25:29 2018 +0100 x86/acpi, x86/boot: Take RSDP address from boot params if available
So there are 2 commits in the Linux kernel that do the same thing, one in boot_params, the other one in setup_header?
I think this 384184044981 ("x86/boot: Mostly revert commit ae7e1238e68f2a ("Add ACPI RSDP address to setup_header")") explains what happened.
e6e094e053af75cbc164e950814d3d084fb1e698 : x86/acpi, x86/boot: Take RSDP address from boot params if available ae7e1238e68f2a472a125673ab506d49158c1889 : x86/boot: Add ACPI RSDP address to setup_header
- */
struct setup_header { __u8 setup_sects; __u16 root_flags; @@ -43,15 +48,16 @@ struct setup_header { __u16 kernel_version; __u8 type_of_loader; __u8 loadflags; -#define LOADED_HIGH (1<<0) -#define QUIET_FLAG (1<<5) -#define KEEP_SEGMENTS (1<<6) -#define CAN_USE_HEAP (1<<7) +#define LOADED_HIGH BIT(0) +#define KASLR_FLAG BIT(1) +#define QUIET_FLAG BIT(5) +#define KEEP_SEGMENTS BIT(6) /* Obsolete */ +#define CAN_USE_HEAP BIT(7) __u16 setup_move_size; __u32 code32_start; __u32 ramdisk_image; __u32 ramdisk_size;
__u32 bootsect_kludge;
__u32 bootsect_kludge; /* Obsolete */ __u16 heap_end_ptr; __u8 ext_loader_ver; __u8 ext_loader_type;
@@ -59,7 +65,13 @@ struct setup_header { __u32 initrd_addr_max; __u32 kernel_alignment; __u8 relocatable_kernel;
__u8 _pad2[3];
u8 min_alignment;
+#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)
u16 xloadflags; __u32 cmdline_size; __u32 hardware_subarch; __u64 hardware_subarch_data;
@@ -69,6 +81,7 @@ struct setup_header { __u64 pref_address; __u32 init_size; __u32 handover_offset;
u32 kernel_info_offset;
} __attribute__((packed));
struct sys_desc_table {
Regards, Bin

Hi Andy,
On Tue, Sep 8, 2020 at 10:06 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Tue, Sep 01, 2020 at 04:23:45PM +0800, Bin Meng wrote:
On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
This header is missing a few of the newer features from the specification. Add these as well as a link to the spec. Also use the BIT() macros where appropriate.
+/**
- struct setup_header - Information needed by Linux to boot
Now I am confused. This kernel document says:
Protocol 2.14 BURNT BY INCORRECT COMMIT ae7e1238e68f2a472a125673ab506d49158c1889 (x86/boot: Add ACPI RSDP address to setup_header) DO NOT USE!!! ASSUME SAME AS 2.13.
Where did you get this "DO NOT USE!!! ASSUME SAME AS 2.13" from?
Please see: https://www.kernel.org/doc/html/latest/x86/boot.html
Chapter "1. The Linux/x86 Boot Protocol"
Protocol 2.14 BURNT BY INCORRECT COMMIT ae7e1238e68f2a472a125673ab506d49158c1889 (x86/boot: Add ACPI RSDP address to setup_header) DO NOT USE!!! ASSUME SAME AS 2.13.
However in U-Boot there was a commit from Andy saying that:
commit d905aa8a4277e200e11fdf6d73a7c76d0e6f34a4 (<=== U-Boot commit) Author: Andy Shevchenko andriy.shevchenko@linux.intel.com Date: Fri Sep 13 18:42:00 2019 +0300
x86: zImage: Propagate acpi_rsdp_addr to kernel via boot parameters
...
after upstream got eventually the Linux kernel commit e6e094e053af75cbc164e950814d3d084fb1e698 Author: Juergen Gross <jgross@suse.com> Date: Tue Nov 20 08:25:29 2018 +0100 x86/acpi, x86/boot: Take RSDP address from boot params if available
So there are 2 commits in the Linux kernel that do the same thing, one in boot_params, the other one in setup_header?
I think this 384184044981 ("x86/boot: Mostly revert commit ae7e1238e68f2a ("Add ACPI RSDP address to setup_header")") explains what happened.
Thanks for the info.
e6e094e053af75cbc164e950814d3d084fb1e698 : x86/acpi, x86/boot: Take RSDP address from boot params if available ae7e1238e68f2a472a125673ab506d49158c1889 : x86/boot: Add ACPI RSDP address to setup_header
Regards, Bin

At present the 'zboot' command does everything in one go. It would be better if it supported sub-commands like bootm, so it is possible to examine what will be booted before actually booting it.
In preparation for this, move the 'state' of the command into a struct. This will allow it to be shared among multiple functions in this file.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
arch/x86/lib/zimage.c | 44 ++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index d2b6002008a..a79971f052c 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -45,6 +45,24 @@
#define COMMAND_LINE_SIZE 2048
+/** + * struct zboot_state - Current state of the boot + * + * @bzimage_addr: Address of the bzImage to boot + * @bzimage_size: Size of the bzImage, or 0 to detect this + * @initrd_addr: Address of the initial ramdisk, or 0 if none + * @initrd_size: Size of the initial ramdisk, or 0 if none + * @load_address: Address where the bzImage is moved before booting, either + * BZIMAGE_LOAD_ADDR or ZIMAGE_LOAD_ADDR + */ +struct zboot_state { + ulong bzimage_addr; + ulong bzimage_size; + ulong initrd_addr; + ulong initrd_size; + ulong load_address; +} state; + static void build_command_line(char *command_line, int auto_boot) { char *env_command_line; @@ -307,15 +325,10 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct boot_params *base_ptr; - void *bzImage_addr = NULL; - ulong load_address; char *s; - ulong bzImage_size = 0; - ulong initrd_addr = 0; - ulong initrd_size = 0;
disable_interrupts(); - + memset(&state, '\0', sizeof(state)); if (argc >= 2) { /* argv[1] holds the address of the bzImage */ s = argv[1]; @@ -324,33 +337,34 @@ int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) }
if (s) - bzImage_addr = (void *)simple_strtoul(s, NULL, 16); + state.bzimage_addr = simple_strtoul(s, NULL, 16);
if (argc >= 3) { /* argv[2] holds the size of the bzImage */ - bzImage_size = simple_strtoul(argv[2], NULL, 16); + state.bzimage_size = simple_strtoul(argv[2], NULL, 16); }
if (argc >= 4) - initrd_addr = simple_strtoul(argv[3], NULL, 16); + state.initrd_addr = simple_strtoul(argv[3], NULL, 16); if (argc >= 5) - initrd_size = simple_strtoul(argv[4], NULL, 16); + state.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((void *)state.bzimage_addr, state.bzimage_size, + &state.load_address); if (!base_ptr) { puts("## Kernel loading failed ...\n"); return -1; } - if (setup_zimage(base_ptr, (char *)base_ptr + COMMAND_LINE_OFFSET, - 0, initrd_addr, initrd_size)) { + + if (setup_zimage(base_ptr, (char *)base_ptr + COMMAND_LINE_OFFSET, 0, + state.initrd_addr, state.initrd_size)) { puts("Setting up boot parameters failed ...\n"); 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, state.load_address, false); }
U_BOOT_CMD(

On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
At present the 'zboot' command does everything in one go. It would be better if it supported sub-commands like bootm, so it is possible to examine what will be booted before actually booting it.
In preparation for this, move the 'state' of the command into a struct. This will allow it to be shared among multiple functions in this file.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
arch/x86/lib/zimage.c | 44 ++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Use IS_ENABLED() instead of #ifdef in this file.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
arch/x86/lib/zimage.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index a79971f052c..3c8081a48bb 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -303,21 +303,17 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, build_command_line(cmd_line, auto_boot); }
-#ifdef CONFIG_INTEL_MID - if (bootproto >= 0x0207) + if (IS_ENABLED(CONFIG_INTEL_MID) && bootproto >= 0x0207) hdr->hardware_subarch = X86_SUBARCH_INTEL_MID; -#endif
-#ifdef CONFIG_GENERATE_ACPI_TABLE - setup_base->acpi_rsdp_addr = acpi_get_rsdp_addr(); -#endif + if (IS_ENABLED(CONFIG_GENERATE_ACPI_TABLE)) + setup_base->acpi_rsdp_addr = acpi_get_rsdp_addr();
setup_device_tree(hdr, (const void *)env_get_hex("fdtaddr", 0)); setup_video(&setup_base->screen_info);
-#ifdef CONFIG_EFI_STUB - setup_efi_info(&setup_base->efi_info); -#endif + if (IS_ENABLED(CONFIG_EFI_STUB)) + setup_efi_info(&setup_base->efi_info);
return 0; }

On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
Use IS_ENABLED() instead of #ifdef in this file.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
arch/x86/lib/zimage.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

To help reduce the size and complexity of load_zimage(), move the code that reads the kernel version into a separate function. Update get_boot_protocol() to allow printing the 'Magic signature' message only once, under control of its callers.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
arch/x86/lib/zimage.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index 3c8081a48bb..b2c3daf225d 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -103,21 +103,23 @@ static int kernel_magic_ok(struct setup_header *hdr) } }
-static int get_boot_protocol(struct setup_header *hdr) +static int get_boot_protocol(struct setup_header *hdr, bool verbose) { if (hdr->header == KERNEL_V2_MAGIC) { - printf("Magic signature found\n"); + if (verbose) + printf("Magic signature found\n"); return hdr->version; } else { /* Very old kernel */ - printf("Magic signature not found\n"); + if (verbose) + printf("Magic signature not found\n"); return 0x0100; } }
static int setup_device_tree(struct setup_header *hdr, const void *fdt_blob) { - int bootproto = get_boot_protocol(hdr); + int bootproto = get_boot_protocol(hdr, false); struct setup_data *sd; int size;
@@ -147,10 +149,24 @@ static int setup_device_tree(struct setup_header *hdr, const void *fdt_blob) return 0; }
+static const char *get_kernel_version(struct boot_params *params, + void *kernel_base) +{ + struct setup_header *hdr = ¶ms->hdr; + int bootproto; + + bootproto = get_boot_protocol(hdr, false); + if (bootproto < 0x0200 || hdr->setup_sects < 15) + return NULL; + + return kernel_base + hdr->kernel_version + 0x200; +} + struct boot_params *load_zimage(char *image, unsigned long kernel_size, ulong *load_addressp) { struct boot_params *setup_base; + const char *version; int setup_size; int bootproto; int big_image; @@ -178,21 +194,16 @@ struct boot_params *load_zimage(char *image, unsigned long kernel_size, printf("Error: Setup is too large (%d bytes)\n", setup_size);
/* determine boot protocol version */ - bootproto = get_boot_protocol(hdr); + bootproto = get_boot_protocol(hdr, true);
printf("Using boot protocol version %x.%02x\n", (bootproto & 0xff00) >> 8, bootproto & 0xff);
- if (bootproto >= 0x0200) { - if (hdr->setup_sects >= 15) { - printf("Linux kernel version %s\n", - (char *)params + - hdr->kernel_version + 0x200); - } else { - printf("Setup Sectors < 15 - " - "Cannot print kernel version.\n"); - } - } + version = get_kernel_version(params, image); + if (version) + printf("Linux kernel version %s\n", version); + else + printf("Setup Sectors < 15 - Cannot print kernel version\n");
/* Determine image type */ big_image = (bootproto >= 0x0200) && @@ -261,7 +272,7 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, unsigned long initrd_addr, unsigned long initrd_size) { struct setup_header *hdr = &setup_base->hdr; - int bootproto = get_boot_protocol(hdr); + int bootproto = get_boot_protocol(hdr, false);
setup_base->e820_entries = install_e820_map( ARRAY_SIZE(setup_base->e820_map), setup_base->e820_map);

On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
To help reduce the size and complexity of load_zimage(), move the code that reads the kernel version into a separate function. Update get_boot_protocol() to allow printing the 'Magic signature' message only once, under control of its callers.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
arch/x86/lib/zimage.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

At present U-Boot sets a loader type of 8 which means LILO version 8, according to the spec. Update it to 0x80, which means U-Boot with no particular version.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
arch/x86/lib/zimage.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index b2c3daf225d..ba9eb50b0ba 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -282,8 +282,7 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, setup_base->screen_info.cl_offset = COMMAND_LINE_OFFSET; } if (bootproto >= 0x0200) { - hdr->type_of_loader = 8; - + hdr->type_of_loader = 0x80; /* U-Boot version 0 */ if (initrd_addr) { printf("Initial RAM disk at linear address " "0x%08lx, size %ld bytes\n",

On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
At present U-Boot sets a loader type of 8 which means LILO version 8, according to the spec. Update it to 0x80, which means U-Boot with no particular version.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
arch/x86/lib/zimage.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

At present if an error occurs while setting up the boot, interrupts are left disabled. Move this call later in the sequence to avoid this problem.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
arch/x86/lib/zimage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index ba9eb50b0ba..8651dea93b3 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -333,7 +333,6 @@ int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) struct boot_params *base_ptr; char *s;
- disable_interrupts(); memset(&state, '\0', sizeof(state)); if (argc >= 2) { /* argv[1] holds the address of the bzImage */ @@ -369,6 +368,7 @@ int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return -1; }
+ disable_interrupts(); /* we assume that the kernel is in place */ return boot_linux_kernel((ulong)base_ptr, state.load_address, false); }

On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
At present if an error occurs while setting up the boot, interrupts are left disabled. Move this call later in the sequence to avoid this problem.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
arch/x86/lib/zimage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Add subcommands to zboot. At present there is only one called 'start' which does the whole boot. It is the default command so is optional.
Change the 's' string variable to const while we are here. Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Fix comment about argv[0] in do_zboot_parent()
arch/x86/lib/zimage.c | 64 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 58 insertions(+), 6 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index 8651dea93b3..e3e3efdde43 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -63,6 +63,12 @@ struct zboot_state { ulong load_address; } state;
+enum { + ZBOOT_STATE_START = BIT(0), + + ZBOOT_STATE_COUNT = 1, +}; + static void build_command_line(char *command_line, int auto_boot) { char *env_command_line; @@ -328,10 +334,11 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, return 0; }
-int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) { struct boot_params *base_ptr; - char *s; + const char *s;
memset(&state, '\0', sizeof(state)); if (argc >= 2) { @@ -373,9 +380,53 @@ int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return boot_linux_kernel((ulong)base_ptr, state.load_address, false); }
-U_BOOT_CMD( - zboot, 5, 0, do_zboot, - "Boot bzImage", +U_BOOT_SUBCMDS(zboot, + U_BOOT_CMD_MKENT(start, 8, 1, do_zboot_start, "", ""), +) + +int do_zboot_states(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[], int state_mask) +{ + int i; + + for (i = 0; i < ZBOOT_STATE_COUNT; i++) { + struct cmd_tbl *cmd = &zboot_subcmds[i]; + int mask = 1 << i; + int ret; + + if (mask & state_mask) { + ret = cmd->cmd(cmd, flag, argc, argv); + if (ret) + return ret; + } + } + + return 0; +} + +int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[], int *repeatable) +{ + /* determine if we have a sub command */ + if (argc > 1) { + char *endp; + + simple_strtoul(argv[1], &endp, 16); + /* + * endp pointing to nul means that argv[1] was just a valid + * number, so pass it along to the normal processing + */ + if (*endp) + return do_zboot(cmdtp, flag, argc, argv, repeatable); + } + + do_zboot_states(cmdtp, flag, argc, argv, ZBOOT_STATE_START); + + return CMD_RET_FAILURE; +} + +U_BOOT_CMDREP_COMPLETE( + zboot, 8, do_zboot_parent, "Boot bzImage", "[addr] [size] [initrd addr] [initrd size]\n" " addr - The optional starting address of the bzimage.\n" " If not set it defaults to the environment\n" @@ -383,5 +434,6 @@ U_BOOT_CMD( " size - The optional size of the bzimage. Defaults to\n" " zero.\n" " initrd addr - The address of the initrd image to use, if any.\n" - " initrd size - The size of the initrd image to use, if any.\n" + " initrd size - The size of the initrd image to use, if any.\n", + complete_zboot );

Hi Simon,
-----"Simon Glass" sjg@chromium.org schrieb: -----
Betreff: [PATCH v2 07/16] x86: zboot: Set up a sub-command structure
Add subcommands to zboot. At present there is only one called 'start' which does the whole boot. It is the default command so is optional.
Change the 's' string variable to const while we are here. Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Fix comment about argv[0] in do_zboot_parent()
arch/x86/lib/zimage.c | 64 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 58 insertions(+), 6 deletions(-)
Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com

Hi Simon,
On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
Add subcommands to zboot. At present there is only one called 'start' which does the whole boot. It is the default command so is optional.
Change the 's' string variable to const while we are here. Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Fix comment about argv[0] in do_zboot_parent()
arch/x86/lib/zimage.c | 64 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 58 insertions(+), 6 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index 8651dea93b3..e3e3efdde43 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -63,6 +63,12 @@ struct zboot_state { ulong load_address; } state;
+enum {
ZBOOT_STATE_START = BIT(0),
ZBOOT_STATE_COUNT = 1,
+};
static void build_command_line(char *command_line, int auto_boot) { char *env_command_line; @@ -328,10 +334,11 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, return 0; }
-int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
{ struct boot_params *base_ptr;
char *s;
const char *s; memset(&state, '\0', sizeof(state)); if (argc >= 2) {
@@ -373,9 +380,53 @@ int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return boot_linux_kernel((ulong)base_ptr, state.load_address, false); }
-U_BOOT_CMD(
zboot, 5, 0, do_zboot,
"Boot bzImage",
+U_BOOT_SUBCMDS(zboot,
U_BOOT_CMD_MKENT(start, 8, 1, do_zboot_start, "", ""),
should the maxargs be 5 instead of 8?
+)
+int do_zboot_states(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[], int state_mask)
+{
int i;
for (i = 0; i < ZBOOT_STATE_COUNT; i++) {
struct cmd_tbl *cmd = &zboot_subcmds[i];
int mask = 1 << i;
int ret;
if (mask & state_mask) {
ret = cmd->cmd(cmd, flag, argc, argv);
if (ret)
return ret;
}
}
return 0;
+}
+int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[], int *repeatable)
+{
/* determine if we have a sub command */
if (argc > 1) {
char *endp;
simple_strtoul(argv[1], &endp, 16);
/*
* endp pointing to nul means that argv[1] was just a valid
* number, so pass it along to the normal processing
*/
if (*endp)
return do_zboot(cmdtp, flag, argc, argv, repeatable);
}
do_zboot_states(cmdtp, flag, argc, argv, ZBOOT_STATE_START);
return CMD_RET_FAILURE;
+}
+U_BOOT_CMDREP_COMPLETE(
zboot, 8, do_zboot_parent, "Boot bzImage", "[addr] [size] [initrd addr] [initrd size]\n" " addr - The optional starting address of the bzimage.\n" " If not set it defaults to the environment\n"
@@ -383,5 +434,6 @@ U_BOOT_CMD( " size - The optional size of the bzimage. Defaults to\n" " zero.\n" " initrd addr - The address of the initrd image to use, if any.\n"
" initrd size - The size of the initrd image to use, if any.\n"
" initrd size - The size of the initrd image to use, if any.\n",
complete_zboot
What's "complete_zboot"?
);
Regards, Bin

Hi Bin,
On Tue, 1 Sep 2020 at 03:30, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
Add subcommands to zboot. At present there is only one called 'start' which does the whole boot. It is the default command so is optional.
Change the 's' string variable to const while we are here. Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Fix comment about argv[0] in do_zboot_parent()
arch/x86/lib/zimage.c | 64 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 58 insertions(+), 6 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index 8651dea93b3..e3e3efdde43 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -63,6 +63,12 @@ struct zboot_state { ulong load_address; } state;
+enum {
ZBOOT_STATE_START = BIT(0),
ZBOOT_STATE_COUNT = 1,
+};
static void build_command_line(char *command_line, int auto_boot) { char *env_command_line; @@ -328,10 +334,11 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, return 0; }
-int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
{ struct boot_params *base_ptr;
char *s;
const char *s; memset(&state, '\0', sizeof(state)); if (argc >= 2) {
@@ -373,9 +380,53 @@ int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return boot_linux_kernel((ulong)base_ptr, state.load_address, false); }
-U_BOOT_CMD(
zboot, 5, 0, do_zboot,
"Boot bzImage",
+U_BOOT_SUBCMDS(zboot,
U_BOOT_CMD_MKENT(start, 8, 1, do_zboot_start, "", ""),
should the maxargs be 5 instead of 8?
Yes, for this patch. I'll update it and adjust it in each following patch as needed.
+)
+int do_zboot_states(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[], int state_mask)
+{
int i;
for (i = 0; i < ZBOOT_STATE_COUNT; i++) {
struct cmd_tbl *cmd = &zboot_subcmds[i];
int mask = 1 << i;
int ret;
if (mask & state_mask) {
ret = cmd->cmd(cmd, flag, argc, argv);
if (ret)
return ret;
}
}
return 0;
+}
+int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[], int *repeatable)
+{
/* determine if we have a sub command */
if (argc > 1) {
char *endp;
simple_strtoul(argv[1], &endp, 16);
/*
* endp pointing to nul means that argv[1] was just a valid
* number, so pass it along to the normal processing
*/
if (*endp)
return do_zboot(cmdtp, flag, argc, argv, repeatable);
}
do_zboot_states(cmdtp, flag, argc, argv, ZBOOT_STATE_START);
return CMD_RET_FAILURE;
+}
+U_BOOT_CMDREP_COMPLETE(
zboot, 8, do_zboot_parent, "Boot bzImage", "[addr] [size] [initrd addr] [initrd size]\n" " addr - The optional starting address of the bzimage.\n" " If not set it defaults to the environment\n"
@@ -383,5 +434,6 @@ U_BOOT_CMD( " size - The optional size of the bzimage. Defaults to\n" " zero.\n" " initrd addr - The address of the initrd image to use, if any.\n"
" initrd size - The size of the initrd image to use, if any.\n"
" initrd size - The size of the initrd image to use, if any.\n",
complete_zboot
What's "complete_zboot"?
It is defined by the U_BOOT_SUBCMDS() macro. I'll add a comment.
Regards, SImon

Split out the code that actually boots linux into a separate sub-command. Add base_ptr to the state to support this.
Show an error if the boot fails, since this should not happen.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
arch/x86/lib/zimage.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index e3e3efdde43..82c7e118ffb 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -54,6 +54,8 @@ * @initrd_size: Size of the initial ramdisk, or 0 if none * @load_address: Address where the bzImage is moved before booting, either * BZIMAGE_LOAD_ADDR or ZIMAGE_LOAD_ADDR + * @base_ptr: Pointer to the boot parameters, typically at address + * DEFAULT_SETUP_BASE */ struct zboot_state { ulong bzimage_addr; @@ -61,12 +63,14 @@ struct zboot_state { ulong initrd_addr; ulong initrd_size; ulong load_address; + struct boot_params *base_ptr; } state;
enum { ZBOOT_STATE_START = BIT(0), + ZBOOT_STATE_GO = BIT(1),
- ZBOOT_STATE_COUNT = 1, + ZBOOT_STATE_COUNT = 2, };
static void build_command_line(char *command_line, int auto_boot) @@ -368,6 +372,7 @@ static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc, puts("## Kernel loading failed ...\n"); return -1; } + state.base_ptr = base_ptr;
if (setup_zimage(base_ptr, (char *)base_ptr + COMMAND_LINE_OFFSET, 0, state.initrd_addr, state.initrd_size)) { @@ -375,13 +380,27 @@ static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc, return -1; }
+ return 0; +} + +static int do_zboot_go(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + int ret; + disable_interrupts(); + /* we assume that the kernel is in place */ - return boot_linux_kernel((ulong)base_ptr, state.load_address, false); + ret = boot_linux_kernel((ulong)state.base_ptr, state.load_address, + false); + printf("Kernel returned! (err=%d)\n", ret); + + return CMD_RET_FAILURE; }
U_BOOT_SUBCMDS(zboot, U_BOOT_CMD_MKENT(start, 8, 1, do_zboot_start, "", ""), + U_BOOT_CMD_MKENT(go, 1, 1, do_zboot_go, "", ""), )
int do_zboot_states(struct cmd_tbl *cmdtp, int flag, int argc, @@ -420,7 +439,8 @@ int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc, return do_zboot(cmdtp, flag, argc, argv, repeatable); }
- do_zboot_states(cmdtp, flag, argc, argv, ZBOOT_STATE_START); + do_zboot_states(cmdtp, flag, argc, argv, ZBOOT_STATE_START | + ZBOOT_STATE_GO);
return CMD_RET_FAILURE; }

On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
Split out the code that actually boots linux into a separate sub-command. Add base_ptr to the state to support this.
Show an error if the boot fails, since this should not happen.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
arch/x86/lib/zimage.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Add a little subcommand that prints out where the kernel was loaded and its setup pointer. Run it by default in the normal boot.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
arch/x86/lib/zimage.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index 82c7e118ffb..460282e1312 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -68,9 +68,10 @@ struct zboot_state {
enum { ZBOOT_STATE_START = BIT(0), - ZBOOT_STATE_GO = BIT(1), + ZBOOT_STATE_INFO = BIT(1), + ZBOOT_STATE_GO = BIT(2),
- ZBOOT_STATE_COUNT = 2, + ZBOOT_STATE_COUNT = 3, };
static void build_command_line(char *command_line, int auto_boot) @@ -383,6 +384,15 @@ static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc, return 0; }
+static int do_zboot_info(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + printf("Kernel loaded at %08lx, setup_base=%p\n", + state.load_address, state.base_ptr); + + return 0; +} + static int do_zboot_go(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { @@ -400,6 +410,7 @@ static int do_zboot_go(struct cmd_tbl *cmdtp, int flag, int argc,
U_BOOT_SUBCMDS(zboot, U_BOOT_CMD_MKENT(start, 8, 1, do_zboot_start, "", ""), + U_BOOT_CMD_MKENT(info, 1, 1, do_zboot_info, "", ""), U_BOOT_CMD_MKENT(go, 1, 1, do_zboot_go, "", ""), )
@@ -440,7 +451,7 @@ int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc, }
do_zboot_states(cmdtp, flag, argc, argv, ZBOOT_STATE_START | - ZBOOT_STATE_GO); + ZBOOT_STATE_INFO | ZBOOT_STATE_GO);
return CMD_RET_FAILURE; }

On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
Add a little subcommand that prints out where the kernel was loaded and its setup pointer. Run it by default in the normal boot.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
arch/x86/lib/zimage.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Add a subcommand that sets up the kernel ready for execution.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
arch/x86/lib/zimage.c | 52 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 10 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index 460282e1312..a39ef9d288f 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -68,10 +68,12 @@ struct zboot_state {
enum { ZBOOT_STATE_START = BIT(0), - ZBOOT_STATE_INFO = BIT(1), - ZBOOT_STATE_GO = BIT(2), + ZBOOT_STATE_LOAD = BIT(1), + ZBOOT_STATE_SETUP = BIT(2), + ZBOOT_STATE_INFO = BIT(3), + ZBOOT_STATE_GO = BIT(4),
- ZBOOT_STATE_COUNT = 3, + ZBOOT_STATE_COUNT = 5, };
static void build_command_line(char *command_line, int auto_boot) @@ -342,7 +344,6 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { - struct boot_params *base_ptr; const char *s;
memset(&state, '\0', sizeof(state)); @@ -366,19 +367,40 @@ static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc, if (argc >= 5) state.initrd_size = simple_strtoul(argv[4], NULL, 16);
- /* Lets look for */ + return 0; +} + +static int do_zboot_load(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct boot_params *base_ptr; + base_ptr = load_zimage((void *)state.bzimage_addr, state.bzimage_size, &state.load_address); if (!base_ptr) { puts("## Kernel loading failed ...\n"); - return -1; + return CMD_RET_FAILURE; } state.base_ptr = base_ptr;
- if (setup_zimage(base_ptr, (char *)base_ptr + COMMAND_LINE_OFFSET, 0, - state.initrd_addr, state.initrd_size)) { + return 0; +} + +static int do_zboot_setup(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct boot_params *base_ptr = state.base_ptr; + int ret; + + if (!base_ptr) { + printf("base is not set: use 'zboot load' first\n"); + return CMD_RET_FAILURE; + } + ret = setup_zimage(base_ptr, (char *)base_ptr + COMMAND_LINE_OFFSET, + 0, state.initrd_addr, state.initrd_size); + if (ret) { puts("Setting up boot parameters failed ...\n"); - return -1; + return CMD_RET_FAILURE; }
return 0; @@ -410,6 +432,8 @@ static int do_zboot_go(struct cmd_tbl *cmdtp, int flag, int argc,
U_BOOT_SUBCMDS(zboot, U_BOOT_CMD_MKENT(start, 8, 1, do_zboot_start, "", ""), + U_BOOT_CMD_MKENT(load, 1, 1, do_zboot_load, "", ""), + U_BOOT_CMD_MKENT(setup, 1, 1, do_zboot_setup, "", ""), U_BOOT_CMD_MKENT(info, 1, 1, do_zboot_info, "", ""), U_BOOT_CMD_MKENT(go, 1, 1, do_zboot_go, "", ""), ) @@ -451,6 +475,7 @@ int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc, }
do_zboot_states(cmdtp, flag, argc, argv, ZBOOT_STATE_START | + ZBOOT_STATE_LOAD | ZBOOT_STATE_SETUP | ZBOOT_STATE_INFO | ZBOOT_STATE_GO);
return CMD_RET_FAILURE; @@ -465,6 +490,13 @@ U_BOOT_CMDREP_COMPLETE( " size - The optional size of the bzimage. Defaults to\n" " zero.\n" " initrd addr - The address of the initrd image to use, if any.\n" - " initrd size - The size of the initrd image to use, if any.\n", + " initrd size - The size of the initrd image to use, if any.\n" + "\n" + "Sub-commands to do part of the zboot sequence:\n" + "\tstart [addr [arg ...]] - specify arguments\n" + "\tload - load OS image\n" + "\tsetup - set up table\n" + "\tinfo - show sumary info\n" + "\tgo - start OS\n", complete_zboot );

Hi Simon,
On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
Add a subcommand that sets up the kernel ready for execution.
This commit actually adds 2 subcommands.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
arch/x86/lib/zimage.c | 52 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 10 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index 460282e1312..a39ef9d288f 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -68,10 +68,12 @@ struct zboot_state {
enum { ZBOOT_STATE_START = BIT(0),
ZBOOT_STATE_INFO = BIT(1),
ZBOOT_STATE_GO = BIT(2),
ZBOOT_STATE_LOAD = BIT(1),
ZBOOT_STATE_SETUP = BIT(2),
ZBOOT_STATE_INFO = BIT(3),
ZBOOT_STATE_GO = BIT(4),
ZBOOT_STATE_COUNT = 3,
ZBOOT_STATE_COUNT = 5,
};
static void build_command_line(char *command_line, int auto_boot) @@ -342,7 +344,6 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) {
struct boot_params *base_ptr; const char *s; memset(&state, '\0', sizeof(state));
@@ -366,19 +367,40 @@ static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc, if (argc >= 5) state.initrd_size = simple_strtoul(argv[4], NULL, 16);
/* Lets look for */
return 0;
+}
+static int do_zboot_load(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
+{
struct boot_params *base_ptr;
base_ptr = load_zimage((void *)state.bzimage_addr, state.bzimage_size, &state.load_address); if (!base_ptr) { puts("## Kernel loading failed ...\n");
return -1;
return CMD_RET_FAILURE; } state.base_ptr = base_ptr;
if (setup_zimage(base_ptr, (char *)base_ptr + COMMAND_LINE_OFFSET, 0,
state.initrd_addr, state.initrd_size)) {
return 0;
+}
+static int do_zboot_setup(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
+{
struct boot_params *base_ptr = state.base_ptr;
int ret;
if (!base_ptr) {
printf("base is not set: use 'zboot load' first\n");
return CMD_RET_FAILURE;
}
ret = setup_zimage(base_ptr, (char *)base_ptr + COMMAND_LINE_OFFSET,
0, state.initrd_addr, state.initrd_size);
if (ret) { puts("Setting up boot parameters failed ...\n");
return -1;
return CMD_RET_FAILURE; } return 0;
@@ -410,6 +432,8 @@ static int do_zboot_go(struct cmd_tbl *cmdtp, int flag, int argc,
U_BOOT_SUBCMDS(zboot, U_BOOT_CMD_MKENT(start, 8, 1, do_zboot_start, "", ""),
U_BOOT_CMD_MKENT(load, 1, 1, do_zboot_load, "", ""),
U_BOOT_CMD_MKENT(setup, 1, 1, do_zboot_setup, "", ""), U_BOOT_CMD_MKENT(info, 1, 1, do_zboot_info, "", ""), U_BOOT_CMD_MKENT(go, 1, 1, do_zboot_go, "", ""),
) @@ -451,6 +475,7 @@ int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc, }
do_zboot_states(cmdtp, flag, argc, argv, ZBOOT_STATE_START |
ZBOOT_STATE_LOAD | ZBOOT_STATE_SETUP | ZBOOT_STATE_INFO | ZBOOT_STATE_GO); return CMD_RET_FAILURE;
@@ -465,6 +490,13 @@ U_BOOT_CMDREP_COMPLETE( " size - The optional size of the bzimage. Defaults to\n" " zero.\n" " initrd addr - The address of the initrd image to use, if any.\n"
" initrd size - The size of the initrd image to use, if any.\n",
" initrd size - The size of the initrd image to use, if any.\n"
"\n"
"Sub-commands to do part of the zboot sequence:\n"
"\tstart [addr [arg ...]] - specify arguments\n"
"\tload - load OS image\n"
"\tsetup - set up table\n"
"\tinfo - show sumary info\n"
typo: summary
Previous commits should be adjusted to include their description in their commit, not here
"\tgo - start OS\n", complete_zboot
);
Regards, Bin

At present it is not possible to tell from a script where the setup block is, or where the image was loaded to. Add environment variables for this.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
README | 4 ++++ arch/x86/lib/zimage.c | 3 +++ 2 files changed, 7 insertions(+)
diff --git a/README b/README index 6cb0567ba66..eb2260134e6 100644 --- a/README +++ b/README @@ -3425,6 +3425,10 @@ List of environment variables (most likely not complete): mempos - Index position of the last match found by the 'ms' command, in units of the size (.b, .w, .l) of the search
+ zbootbase - Base address of the bzImage 'setup' block + + zbootaddr - Address of the loaded bzImage, typically BZIMAGE_LOAD_ADDR + which is 0x100000
The following image location variables contain the location of images used in booting. The "Image" column gives the role of the image and is diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index a39ef9d288f..d0e44c331b7 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -382,6 +382,9 @@ static int do_zboot_load(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_FAILURE; } state.base_ptr = base_ptr; + if (env_set_hex("zbootbase", (ulong)base_ptr) || + env_set_hex("zbootaddr", state.load_address)) + return CMD_RET_FAILURE;
return 0; }

Hi Simon,
On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
At present it is not possible to tell from a script where the setup block is, or where the image was loaded to. Add environment variables for this.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
README | 4 ++++ arch/x86/lib/zimage.c | 3 +++ 2 files changed, 7 insertions(+)
diff --git a/README b/README index 6cb0567ba66..eb2260134e6 100644 --- a/README +++ b/README @@ -3425,6 +3425,10 @@ List of environment variables (most likely not complete): mempos - Index position of the last match found by the 'ms' command, in units of the size (.b, .w, .l) of the search
- zbootbase - Base address of the bzImage 'setup' block
- zbootaddr - Address of the loaded bzImage, typically BZIMAGE_LOAD_ADDR
which is 0x100000
We should mention these are x86 specific environment variables.
The following image location variables contain the location of images used in booting. The "Image" column gives the role of the image and is diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index a39ef9d288f..d0e44c331b7 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -382,6 +382,9 @@ static int do_zboot_load(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_FAILURE; } state.base_ptr = base_ptr;
if (env_set_hex("zbootbase", (ulong)base_ptr) ||
env_set_hex("zbootaddr", state.load_address))
return CMD_RET_FAILURE; return 0;
}
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin

At present the setup block is always obtained from the image automatically. In some cases it can be useful to use a setup block obtained elsewhere, e.g. if the image has already been unpacked. Add an argument to support this and update the logic to use it if provided.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
Changes in v2: - Add a comment explaining the logic for a specified setup-base address
arch/x86/lib/zimage.c | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index d0e44c331b7..33df0dc682e 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -366,6 +366,22 @@ static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc, state.initrd_addr = simple_strtoul(argv[3], NULL, 16); if (argc >= 5) state.initrd_size = simple_strtoul(argv[4], NULL, 16); + if (argc >= 6) { + /* + * When the base_ptr is passed in, we assume that the image is + * already loaded at the address given by argv[1] and therefore + * the original bzImage is somewhere else, or not accessible. + * In any case, we don't need access to the bzImage since all + * the processing is assumed to be done. + * + * So set the base_ptr to the given address, use this arg as the + * load address and set bzimage_addr to 0 so we know that it + * cannot be proceesed (or processed again). + */ + state.base_ptr = (void *)simple_strtoul(argv[5], NULL, 16); + state.load_address = state.bzimage_addr; + state.bzimage_addr = 0; + }
return 0; } @@ -375,11 +391,20 @@ static int do_zboot_load(struct cmd_tbl *cmdtp, int flag, int argc, { struct boot_params *base_ptr;
- base_ptr = load_zimage((void *)state.bzimage_addr, state.bzimage_size, - &state.load_address); - if (!base_ptr) { - puts("## Kernel loading failed ...\n"); - return CMD_RET_FAILURE; + if (state.base_ptr) { + struct boot_params *from = (struct boot_params *)state.base_ptr; + + base_ptr = (struct boot_params *)DEFAULT_SETUP_BASE; + printf("Building boot_params at 0x%8.8lx\n", (ulong)base_ptr); + memset(base_ptr, '\0', sizeof(*base_ptr)); + base_ptr->hdr = from->hdr; + } else { + base_ptr = load_zimage((void *)state.bzimage_addr, state.bzimage_size, + &state.load_address); + if (!base_ptr) { + puts("## Kernel loading failed ...\n"); + return CMD_RET_FAILURE; + } } state.base_ptr = base_ptr; if (env_set_hex("zbootbase", (ulong)base_ptr) || @@ -486,7 +511,7 @@ int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc,
U_BOOT_CMDREP_COMPLETE( zboot, 8, do_zboot_parent, "Boot bzImage", - "[addr] [size] [initrd addr] [initrd size]\n" + "[addr] [size] [initrd addr] [initrd size] [setup]\n" " addr - The optional starting address of the bzimage.\n" " If not set it defaults to the environment\n" " variable "fileaddr".\n" @@ -494,6 +519,8 @@ U_BOOT_CMDREP_COMPLETE( " zero.\n" " initrd addr - The address of the initrd image to use, if any.\n" " initrd size - The size of the initrd image to use, if any.\n" + " setup - The address of the kernel setup region, if this\n" + " is not at addr\n" "\n" "Sub-commands to do part of the zboot sequence:\n" "\tstart [addr [arg ...]] - specify arguments\n"

On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
At present the setup block is always obtained from the image automatically. In some cases it can be useful to use a setup block obtained elsewhere, e.g. if the image has already been unpacked. Add an argument to support this and update the logic to use it if provided.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
Changes in v2:
- Add a comment explaining the logic for a specified setup-base address
arch/x86/lib/zimage.c | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

There is a lot of information in the setup block and it is quite hard to decode manually. Add a 'zboot dump' command to decode it into a human-readable format.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
arch/x86/include/asm/e820.h | 1 + arch/x86/lib/zimage.c | 199 +++++++++++++++++++++++++++++++++++- 2 files changed, 199 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h index 9d29f82f972..d7f8a4ba1df 100644 --- a/arch/x86/include/asm/e820.h +++ b/arch/x86/include/asm/e820.h @@ -8,6 +8,7 @@ #define E820_ACPI 3 #define E820_NVS 4 #define E820_UNUSABLE 5 +#define E820_COUNT 6 /* Number of types */
#ifndef __ASSEMBLY__ #include <linux/types.h> diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index 33df0dc682e..aacfffcee7d 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -73,6 +73,8 @@ enum { ZBOOT_STATE_INFO = BIT(3), ZBOOT_STATE_GO = BIT(4),
+ /* This one doesn't execute automatically, so stop the count before 5 */ + ZBOOT_STATE_DUMP = BIT(5), ZBOOT_STATE_COUNT = 5, };
@@ -458,12 +460,206 @@ static int do_zboot_go(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_FAILURE; }
+static void print_num(const char *name, ulong value) +{ + printf("%-20s: %lx\n", name, value); +} + +static void print_num64(const char *name, u64 value) +{ + printf("%-20s: %llx\n", name, value); +} + +static const char *const e820_type_name[E820_COUNT] = { + [E820_RAM] = "RAM", + [E820_RESERVED] = "Reserved", + [E820_ACPI] = "ACPI", + [E820_NVS] = "ACPI NVS", + [E820_UNUSABLE] = "Unusable", +}; + +static const char *const bootloader_id[] = { + "LILO", + "Loadlin", + "bootsect-loader", + "Syslinux", + "Etherboot/gPXE/iPXE", + "ELILO", + "undefined", + "GRUB", + "U-Boot", + "Xen", + "Gujin", + "Qemu", + "Arcturus Networks uCbootloader", + "kexec-tools", + "Extended", + "Special", + "Reserved", + "Minimal Linux Bootloader", + "OVMF UEFI virtualization stack", +}; + +struct flag_info { + uint bit; + const char *name; +}; + +struct flag_info load_flags[] = { + { LOADED_HIGH, "loaded-high" }, + { QUIET_FLAG, "quiet" }, + { KEEP_SEGMENTS, "keep-segments" }, + { CAN_USE_HEAP, "can-use-heap" }, +}; + +struct flag_info xload_flags[] = { + { XLF_KERNEL_64, "64-bit-entry" }, + { XLF_CAN_BE_LOADED_ABOVE_4G, "can-load-above-4gb" }, + { XLF_EFI_HANDOVER_32, "32-efi-handoff" }, + { XLF_EFI_HANDOVER_64, "64-efi-handoff" }, + { XLF_EFI_KEXEC, "kexec-efi-runtime" }, +}; + +static void print_flags(struct flag_info *flags, int count, uint value) +{ + int i; + + printf("%-20s:", ""); + for (i = 0; i < count; i++) { + uint mask = flags[i].bit; + + if (value & mask) + printf(" %s", flags[i].name); + } + printf("\n"); +} + +static void show_loader(struct setup_header *hdr) +{ + bool version_valid = false; + int type, version; + const char *name; + + type = hdr->type_of_loader >> 4; + version = hdr->type_of_loader & 0xf; + if (type == 0xe) + type = 0x10 + hdr->ext_loader_type; + version |= hdr->ext_loader_ver << 4; + if (!hdr->type_of_loader) { + name = "pre-2.00 bootloader"; + } else if (hdr->type_of_loader == 0xff) { + name = "unknown"; + } else if (type < ARRAY_SIZE(bootloader_id)) { + name = bootloader_id[type]; + version_valid = true; + } else { + name = "undefined"; + } + printf("%20s %s", "", name); + if (version_valid) + printf(", version %x", version); + printf("\n"); +} + +int do_zboot_dump(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{ + struct boot_params *base_ptr = state.base_ptr; + struct setup_header *hdr; + const char *version; + int i; + + if (argc > 1) + base_ptr = (void *)simple_strtoul(argv[1], NULL, 16); + if (!base_ptr) { + printf("No zboot setup_base\n"); + return CMD_RET_FAILURE; + } + printf("Setup located at %p:\n\n", base_ptr); + print_num64("ACPI RSDP addr", base_ptr->acpi_rsdp_addr); + + printf("E820: %d entries\n", base_ptr->e820_entries); + if (base_ptr->e820_entries) { + printf("%18s %16s %s\n", "Addr", "Size", "Type"); + for (i = 0; i < base_ptr->e820_entries; i++) { + struct e820_entry *entry = &base_ptr->e820_map[i]; + + printf("%12llx %10llx %s\n", entry->addr, entry->size, + entry->type < E820_COUNT ? + e820_type_name[entry->type] : + simple_itoa(entry->type)); + } + } + + hdr = &base_ptr->hdr; + print_num("Setup sectors", hdr->setup_sects); + print_num("Root flags", hdr->root_flags); + print_num("Sys size", hdr->syssize); + print_num("RAM size", hdr->ram_size); + print_num("Video mode", hdr->vid_mode); + print_num("Root dev", hdr->root_dev); + print_num("Boot flag", hdr->boot_flag); + print_num("Jump", hdr->jump); + print_num("Header", hdr->header); + if (hdr->header == KERNEL_V2_MAGIC) + printf("%-20s %s\n", "", "Kernel V2"); + else + printf("%-20s %s\n", "", "Ancient kernel, using version 100"); + print_num("Version", hdr->version); + print_num("Real mode switch", hdr->realmode_swtch); + print_num("Start sys", hdr->start_sys); + print_num("Kernel version", hdr->kernel_version); + version = get_kernel_version(base_ptr, (void *)state.bzimage_addr); + if (version) + printf(" @%p: %s\n", version, version); + print_num("Type of loader", hdr->type_of_loader); + show_loader(hdr); + print_num("Load flags", hdr->loadflags); + print_flags(load_flags, ARRAY_SIZE(load_flags), hdr->loadflags); + print_num("Setup move size", hdr->setup_move_size); + print_num("Code32 start", hdr->code32_start); + print_num("Ramdisk image", hdr->ramdisk_image); + print_num("Ramdisk size", hdr->ramdisk_size); + print_num("Bootsect kludge", hdr->bootsect_kludge); + print_num("Heap end ptr", hdr->heap_end_ptr); + print_num("Ext loader ver", hdr->ext_loader_ver); + print_num("Ext loader type", hdr->ext_loader_type); + print_num("Commandline ptr", hdr->cmd_line_ptr); + if (hdr->cmd_line_ptr) { + printf(" "); + /* Use puts() to avoid limits from CONFIG_SYS_PBSIZE */ + puts((char *)hdr->cmd_line_ptr); + printf("\n"); + } + print_num("Initrd addr max", hdr->initrd_addr_max); + print_num("Kernel alignment", hdr->kernel_alignment); + print_num("Relocatable kernel", hdr->relocatable_kernel); + print_num("Min alignment", hdr->min_alignment); + if (hdr->min_alignment) + printf("%-20s: %x\n", "", 1 << hdr->min_alignment); + print_num("Xload flags", hdr->xloadflags); + print_flags(xload_flags, ARRAY_SIZE(xload_flags), hdr->xloadflags); + print_num("Cmdline size", hdr->cmdline_size); + print_num("Hardware subarch", hdr->hardware_subarch); + print_num64("HW subarch data", hdr->hardware_subarch_data); + print_num("Payload offset", hdr->payload_offset); + print_num("Payload length", hdr->payload_length); + print_num64("Setup data", hdr->setup_data); + print_num64("Pref address", hdr->pref_address); + print_num("Init size", hdr->init_size); + print_num("Handover offset", hdr->handover_offset); + if (get_boot_protocol(hdr, false) >= 0x215) + print_num("Kernel info offset", hdr->kernel_info_offset); + + return 0; +} + U_BOOT_SUBCMDS(zboot, U_BOOT_CMD_MKENT(start, 8, 1, do_zboot_start, "", ""), U_BOOT_CMD_MKENT(load, 1, 1, do_zboot_load, "", ""), U_BOOT_CMD_MKENT(setup, 1, 1, do_zboot_setup, "", ""), U_BOOT_CMD_MKENT(info, 1, 1, do_zboot_info, "", ""), U_BOOT_CMD_MKENT(go, 1, 1, do_zboot_go, "", ""), + U_BOOT_CMD_MKENT(dump, 2, 1, do_zboot_dump, "", ""), )
int do_zboot_states(struct cmd_tbl *cmdtp, int flag, int argc, @@ -527,6 +723,7 @@ U_BOOT_CMDREP_COMPLETE( "\tload - load OS image\n" "\tsetup - set up table\n" "\tinfo - show sumary info\n" - "\tgo - start OS\n", + "\tgo - start OS\n" + "\tdump [addr] - dump info (optional address of boot params)", complete_zboot );

On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
There is a lot of information in the setup block and it is quite hard to decode manually. Add a 'zboot dump' command to decode it into a human-readable format.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
arch/x86/include/asm/e820.h | 1 + arch/x86/lib/zimage.c | 199 +++++++++++++++++++++++++++++++++++- 2 files changed, 199 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h index 9d29f82f972..d7f8a4ba1df 100644 --- a/arch/x86/include/asm/e820.h +++ b/arch/x86/include/asm/e820.h @@ -8,6 +8,7 @@ #define E820_ACPI 3 #define E820_NVS 4 #define E820_UNUSABLE 5 +#define E820_COUNT 6 /* Number of types */
#ifndef __ASSEMBLY__ #include <linux/types.h> diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index 33df0dc682e..aacfffcee7d 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -73,6 +73,8 @@ enum { ZBOOT_STATE_INFO = BIT(3), ZBOOT_STATE_GO = BIT(4),
/* This one doesn't execute automatically, so stop the count before 5 */
ZBOOT_STATE_DUMP = BIT(5), ZBOOT_STATE_COUNT = 5,
};
@@ -458,12 +460,206 @@ static int do_zboot_go(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_FAILURE; }
+static void print_num(const char *name, ulong value) +{
printf("%-20s: %lx\n", name, value);
+}
+static void print_num64(const char *name, u64 value) +{
printf("%-20s: %llx\n", name, value);
+}
+static const char *const e820_type_name[E820_COUNT] = {
[E820_RAM] = "RAM",
[E820_RESERVED] = "Reserved",
[E820_ACPI] = "ACPI",
[E820_NVS] = "ACPI NVS",
[E820_UNUSABLE] = "Unusable",
+};
+static const char *const bootloader_id[] = {
"LILO",
"Loadlin",
"bootsect-loader",
"Syslinux",
"Etherboot/gPXE/iPXE",
"ELILO",
"undefined",
"GRUB",
"U-Boot",
"Xen",
"Gujin",
"Qemu",
"Arcturus Networks uCbootloader",
"kexec-tools",
"Extended",
"Special",
"Reserved",
"Minimal Linux Bootloader",
"OVMF UEFI virtualization stack",
+};
+struct flag_info {
uint bit;
const char *name;
+};
+struct flag_info load_flags[] = {
should be static
{ LOADED_HIGH, "loaded-high" },
{ QUIET_FLAG, "quiet" },
{ KEEP_SEGMENTS, "keep-segments" },
{ CAN_USE_HEAP, "can-use-heap" },
+};
+struct flag_info xload_flags[] = {
ditto
{ XLF_KERNEL_64, "64-bit-entry" },
{ XLF_CAN_BE_LOADED_ABOVE_4G, "can-load-above-4gb" },
{ XLF_EFI_HANDOVER_32, "32-efi-handoff" },
{ XLF_EFI_HANDOVER_64, "64-efi-handoff" },
{ XLF_EFI_KEXEC, "kexec-efi-runtime" },
+};
+static void print_flags(struct flag_info *flags, int count, uint value) +{
int i;
printf("%-20s:", "");
for (i = 0; i < count; i++) {
uint mask = flags[i].bit;
if (value & mask)
printf(" %s", flags[i].name);
}
printf("\n");
+}
+static void show_loader(struct setup_header *hdr) +{
bool version_valid = false;
int type, version;
const char *name;
type = hdr->type_of_loader >> 4;
version = hdr->type_of_loader & 0xf;
if (type == 0xe)
type = 0x10 + hdr->ext_loader_type;
version |= hdr->ext_loader_ver << 4;
if (!hdr->type_of_loader) {
name = "pre-2.00 bootloader";
} else if (hdr->type_of_loader == 0xff) {
name = "unknown";
} else if (type < ARRAY_SIZE(bootloader_id)) {
name = bootloader_id[type];
version_valid = true;
} else {
name = "undefined";
}
printf("%20s %s", "", name);
if (version_valid)
printf(", version %x", version);
printf("\n");
+}
+int do_zboot_dump(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{
struct boot_params *base_ptr = state.base_ptr;
struct setup_header *hdr;
const char *version;
int i;
if (argc > 1)
base_ptr = (void *)simple_strtoul(argv[1], NULL, 16);
if (!base_ptr) {
printf("No zboot setup_base\n");
return CMD_RET_FAILURE;
}
printf("Setup located at %p:\n\n", base_ptr);
print_num64("ACPI RSDP addr", base_ptr->acpi_rsdp_addr);
printf("E820: %d entries\n", base_ptr->e820_entries);
if (base_ptr->e820_entries) {
printf("%18s %16s %s\n", "Addr", "Size", "Type");
for (i = 0; i < base_ptr->e820_entries; i++) {
struct e820_entry *entry = &base_ptr->e820_map[i];
printf("%12llx %10llx %s\n", entry->addr, entry->size,
entry->type < E820_COUNT ?
e820_type_name[entry->type] :
simple_itoa(entry->type));
}
}
hdr = &base_ptr->hdr;
print_num("Setup sectors", hdr->setup_sects);
print_num("Root flags", hdr->root_flags);
print_num("Sys size", hdr->syssize);
print_num("RAM size", hdr->ram_size);
print_num("Video mode", hdr->vid_mode);
print_num("Root dev", hdr->root_dev);
print_num("Boot flag", hdr->boot_flag);
print_num("Jump", hdr->jump);
print_num("Header", hdr->header);
if (hdr->header == KERNEL_V2_MAGIC)
printf("%-20s %s\n", "", "Kernel V2");
else
printf("%-20s %s\n", "", "Ancient kernel, using version 100");
print_num("Version", hdr->version);
print_num("Real mode switch", hdr->realmode_swtch);
print_num("Start sys", hdr->start_sys);
print_num("Kernel version", hdr->kernel_version);
version = get_kernel_version(base_ptr, (void *)state.bzimage_addr);
if (version)
printf(" @%p: %s\n", version, version);
print_num("Type of loader", hdr->type_of_loader);
show_loader(hdr);
print_num("Load flags", hdr->loadflags);
print_flags(load_flags, ARRAY_SIZE(load_flags), hdr->loadflags);
print_num("Setup move size", hdr->setup_move_size);
print_num("Code32 start", hdr->code32_start);
print_num("Ramdisk image", hdr->ramdisk_image);
print_num("Ramdisk size", hdr->ramdisk_size);
print_num("Bootsect kludge", hdr->bootsect_kludge);
print_num("Heap end ptr", hdr->heap_end_ptr);
print_num("Ext loader ver", hdr->ext_loader_ver);
print_num("Ext loader type", hdr->ext_loader_type);
print_num("Commandline ptr", hdr->cmd_line_ptr);
nits: Command line?
if (hdr->cmd_line_ptr) {
printf(" ");
/* Use puts() to avoid limits from CONFIG_SYS_PBSIZE */
puts((char *)hdr->cmd_line_ptr);
printf("\n");
}
print_num("Initrd addr max", hdr->initrd_addr_max);
print_num("Kernel alignment", hdr->kernel_alignment);
print_num("Relocatable kernel", hdr->relocatable_kernel);
print_num("Min alignment", hdr->min_alignment);
if (hdr->min_alignment)
printf("%-20s: %x\n", "", 1 << hdr->min_alignment);
print_num("Xload flags", hdr->xloadflags);
print_flags(xload_flags, ARRAY_SIZE(xload_flags), hdr->xloadflags);
print_num("Cmdline size", hdr->cmdline_size);
print_num("Hardware subarch", hdr->hardware_subarch);
print_num64("HW subarch data", hdr->hardware_subarch_data);
print_num("Payload offset", hdr->payload_offset);
print_num("Payload length", hdr->payload_length);
print_num64("Setup data", hdr->setup_data);
print_num64("Pref address", hdr->pref_address);
print_num("Init size", hdr->init_size);
print_num("Handover offset", hdr->handover_offset);
if (get_boot_protocol(hdr, false) >= 0x215)
print_num("Kernel info offset", hdr->kernel_info_offset);
return 0;
+}
U_BOOT_SUBCMDS(zboot, U_BOOT_CMD_MKENT(start, 8, 1, do_zboot_start, "", ""), U_BOOT_CMD_MKENT(load, 1, 1, do_zboot_load, "", ""), U_BOOT_CMD_MKENT(setup, 1, 1, do_zboot_setup, "", ""), U_BOOT_CMD_MKENT(info, 1, 1, do_zboot_info, "", ""), U_BOOT_CMD_MKENT(go, 1, 1, do_zboot_go, "", ""),
U_BOOT_CMD_MKENT(dump, 2, 1, do_zboot_dump, "", ""),
)
int do_zboot_states(struct cmd_tbl *cmdtp, int flag, int argc, @@ -527,6 +723,7 @@ U_BOOT_CMDREP_COMPLETE( "\tload - load OS image\n" "\tsetup - set up table\n" "\tinfo - show sumary info\n"
"\tgo - start OS\n",
"\tgo - start OS\n"
"\tdump [addr] - dump info (optional address of boot params)", complete_zboot
);
Regards, Bin

When booting Chrome OS images the command line is stored separately from the kernel. Add a way to specify this address so that images boot correctly.
Also add comments to the zimage.h header.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
arch/x86/include/asm/zimage.h | 30 +++++++++++++++++++++++++++++- arch/x86/lib/bootm.c | 2 +- arch/x86/lib/zimage.c | 21 ++++++++++++++++----- 3 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/zimage.h b/arch/x86/include/asm/zimage.h index 80e128ccf36..64c0e6e857b 100644 --- a/arch/x86/include/asm/zimage.h +++ b/arch/x86/include/asm/zimage.h @@ -30,10 +30,38 @@ #define BZIMAGE_LOAD_ADDR 0x100000 #define ZIMAGE_LOAD_ADDR 0x10000
+/** + * load_zimage() - Load a zImage or bzImage + * + * This copies an image into the standard location ready for setup + * + * @image: Address of image to load + * @kernel_size: Size of kernel including setup block (or 0 if the kernel is + * new enough to have a 'syssize' value) + * @load_addressp: Returns the address where the kernel has been loaded + * @return address of setup block, or NULL if something went wrong + */ struct boot_params *load_zimage(char *image, unsigned long kernel_size, ulong *load_addressp); + +/** + * setup_zimage() - Set up a loaded zImage or bzImage ready for booting + * + * @setup_base: Pointer to the boot parameters, typically at address + * DEFAULT_SETUP_BASE + * @cmd_line: Place to put the command line, or NULL to use the one in the setup + * block + * @initrd_addr: Address of the initial ramdisk, or 0 if none + * @initrd_size: Size of the initial ramdisk, or 0 if none + * @load_address: Address where the bzImage is moved before booting, either + * BZIMAGE_LOAD_ADDR or ZIMAGE_LOAD_ADDR + * @cmdline_force: Address of 'override' command line, or 0 to use the one in + * the * setup block + * @return 0 (always) + */ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, - unsigned long initrd_addr, unsigned long initrd_size); + ulong initrd_addr, ulong initrd_size, ulong cmdline_force); + void setup_video(struct screen_info *screen_info); void setup_efi_info(struct efi_info *efi_info);
diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c index 1198a52ecac..da6b8ce1ec1 100644 --- a/arch/x86/lib/bootm.c +++ b/arch/x86/lib/bootm.c @@ -136,7 +136,7 @@ static int boot_prep_linux(bootm_headers_t *images) printf("Setup at %#08lx\n", images->ep); ret = setup_zimage((void *)images->ep, cmd_line_dest, 0, images->rd_start, - images->rd_end - images->rd_start); + images->rd_end - images->rd_start, 0);
if (ret) { printf("## Setting up boot parameters failed ...\n"); diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index aacfffcee7d..8c3d677debc 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -56,6 +56,8 @@ * BZIMAGE_LOAD_ADDR or ZIMAGE_LOAD_ADDR * @base_ptr: Pointer to the boot parameters, typically at address * DEFAULT_SETUP_BASE + * @cmdline: Address of 'override' command line, or 0 to use the one in the + * setup block */ struct zboot_state { ulong bzimage_addr; @@ -64,6 +66,7 @@ struct zboot_state { ulong initrd_size; ulong load_address; struct boot_params *base_ptr; + ulong cmdline; } state;
enum { @@ -284,7 +287,7 @@ struct boot_params *load_zimage(char *image, unsigned long kernel_size, }
int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, - unsigned long initrd_addr, unsigned long initrd_size) + ulong initrd_addr, ulong initrd_size, ulong cmdline_force) { struct setup_header *hdr = &setup_base->hdr; int bootproto = get_boot_protocol(hdr, false); @@ -325,7 +328,10 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, }
/* build command line at COMMAND_LINE_OFFSET */ - build_command_line(cmd_line, auto_boot); + if (cmdline_force) + strcpy(cmd_line, (char *)cmdline_force); + else + build_command_line(cmd_line, auto_boot); }
if (IS_ENABLED(CONFIG_INTEL_MID) && bootproto >= 0x0207) @@ -384,6 +390,8 @@ static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc, state.load_address = state.bzimage_addr; state.bzimage_addr = 0; } + if (argc >= 7) + state.cmdline = simple_strtoul(argv[6], NULL, 16);
return 0; } @@ -427,7 +435,8 @@ static int do_zboot_setup(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_FAILURE; } ret = setup_zimage(base_ptr, (char *)base_ptr + COMMAND_LINE_OFFSET, - 0, state.initrd_addr, state.initrd_size); + 0, state.initrd_addr, state.initrd_size, + state.cmdline); if (ret) { puts("Setting up boot parameters failed ...\n"); return CMD_RET_FAILURE; @@ -627,7 +636,7 @@ int do_zboot_dump(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (hdr->cmd_line_ptr) { printf(" "); /* Use puts() to avoid limits from CONFIG_SYS_PBSIZE */ - puts((char *)hdr->cmd_line_ptr); + puts((char *)(ulong)hdr->cmd_line_ptr); printf("\n"); } print_num("Initrd addr max", hdr->initrd_addr_max); @@ -707,7 +716,7 @@ int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc,
U_BOOT_CMDREP_COMPLETE( zboot, 8, do_zboot_parent, "Boot bzImage", - "[addr] [size] [initrd addr] [initrd size] [setup]\n" + "[addr] [size] [initrd addr] [initrd size] [setup] [cmdline]\n" " addr - The optional starting address of the bzimage.\n" " If not set it defaults to the environment\n" " variable "fileaddr".\n" @@ -717,6 +726,8 @@ U_BOOT_CMDREP_COMPLETE( " initrd size - The size of the initrd image to use, if any.\n" " setup - The address of the kernel setup region, if this\n" " is not at addr\n" + " cmdline - The address of the kernel command line, to\n" + " override U-Boot's normal cmdline generation\n" "\n" "Sub-commands to do part of the zboot sequence:\n" "\tstart [addr [arg ...]] - specify arguments\n"

On Sun, Aug 30, 2020 at 5:42 AM Simon Glass sjg@chromium.org wrote:
When booting Chrome OS images the command line is stored separately from the kernel. Add a way to specify this address so that images boot correctly.
Also add comments to the zimage.h header.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
arch/x86/include/asm/zimage.h | 30 +++++++++++++++++++++++++++++- arch/x86/lib/bootm.c | 2 +- arch/x86/lib/zimage.c | 21 ++++++++++++++++----- 3 files changed, 46 insertions(+), 7 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

A few things have changed since this was written about 18 months ago. Update the README.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
doc/README.chromium | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/doc/README.chromium b/doc/README.chromium index 8f67da6c728..c56545cb7ff 100644 --- a/doc/README.chromium +++ b/doc/README.chromium @@ -23,6 +23,10 @@ available: from U-Boot in 2013) and coreboot. See below for more information on this.
+ - Running U-Boot from coreboot. This allows U-Boot to run on more devices + since many of them only support coreboot as the bootloader and have + no bare-metal support in U-Boot. For this, use the 'coreboot' target. +
U-Boot with Chromium OS verified boot ------------------------------------- @@ -168,14 +172,13 @@ existed in U-Boot were not brought over to coreboot or depthcharge. The U-Boot tests ('make check') do operate, but at present there are no Chromium OS tests available. These will hopefully come together over time. Of course the above sandbox feature provides a sort of functional test and can -detecte problems that affect the flow or particular vboot features. +detect problems that affect the flow or particular vboot features.
TO DO -----
-- Support for booting from coreboot (patches expected March 2019) -- Support for booting from an ARM board, e.g. bob +Get the full ACPI tables working with Coral
Simon Glass

On Sun, Aug 30, 2020 at 5:45 AM Simon Glass sjg@chromium.org wrote:
A few things have changed since this was written about 18 months ago. Update the README.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
doc/README.chromium | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Recent versions of Chrome OS do not have a kernel in the root disk, to save space.
With the improvements to the 'zboot' command it is fairly easy to load the kernel from the raw partition. Add instructions on how to do this.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---
(no changes since v1)
doc/README.chromium | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
diff --git a/doc/README.chromium b/doc/README.chromium index c56545cb7ff..75f2f24042c 100644 --- a/doc/README.chromium +++ b/doc/README.chromium @@ -27,6 +27,9 @@ available: since many of them only support coreboot as the bootloader and have no bare-metal support in U-Boot. For this, use the 'coreboot' target.
+ - Running U-Boot and booting into a Chrome OS image, but without verified + boot. This can be useful for testing. +
U-Boot with Chromium OS verified boot ------------------------------------- @@ -175,6 +178,35 @@ course the above sandbox feature provides a sort of functional test and can detect problems that affect the flow or particular vboot features.
+U-Boot without Chromium OS verified boot +---------------------------------------- + +The following script can be used to boot a Chrome OS image on coral: + + # Read the image header and obtain the address of the kernel + # The offset 4f0 is defined by verified boot and may change for other + # Chromebooks + read mmc 2:2 100000 0 80; setexpr loader *001004f0; + + # Get the kernel size and calculate the number of blocks (0x200 bytes each) + setexpr size *00100518; setexpr blocks $size / 200; + + # Read the full kernel and calculate the address of the setup block + read mmc 2:2 100000 80 $blocks; setexpr setup $loader - 1000; + + # Locate the command line + setexpr cmdline $loader - 2000; + + # Start the zboot process with the loaded kernel, setup block and cmdline + zboot start 100000 0 0 0 $setup $cmdline; + + # Load the kernel, fix up the 'setup' block, dump information + zboot load; zboot setup; zboot dump + + # Boot into Chrome OS + zboot go + + TO DO -----

On Sun, Aug 30, 2020 at 5:44 AM Simon Glass sjg@chromium.org wrote:
Recent versions of Chrome OS do not have a kernel in the root disk, to save space.
With the improvements to the 'zboot' command it is fairly easy to load the kernel from the raw partition. Add instructions on how to do this.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
doc/README.chromium | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
participants (4)
-
Andy Shevchenko
-
Bin Meng
-
Simon Glass
-
Wolfgang Wallner