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

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

[Simon Glass]

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(-)

-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 01/16] x86: Update the bootparam header

[Simon Glass]

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 {
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 01/16] x86: Update the bootparam header

[Bin Meng]

+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
> + *
> + * See https://www.kernel.org/doc/html/latest/x86/boot.html

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

[PATCH v2 01/16] x86: Update the bootparam header

[Andy Shevchenko]

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
> > + *
> > + * See https://www.kernel.org/doc/html/latest/x86/boot.html
> 
> 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

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 01/16] x86: Update the bootparam header

[Bin Meng]

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
> > > + *
> > > + * See https://www.kernel.org/doc/html/latest/x86/boot.html
> >
> > 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

[PATCH v2 02/16] x86: zimage: Use a state struct to hold the state

[Simon Glass]

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(
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 02/16] x86: zimage: Use a state struct to hold the state

[Bin Meng]

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>

[PATCH v2 03/16] x86: zimage: Avoid using #ifdef

[Simon Glass]

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;
 }
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 03/16] x86: zimage: Avoid using #ifdef

[Bin Meng]

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>

[PATCH v2 04/16] x86: zboot: Move kernel-version code into a function

[Simon Glass]

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 = &params->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);
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 04/16] x86: zboot: Move kernel-version code into a function

[Bin Meng]

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>

[PATCH v2 05/16] x86: zboot: Correct image type

[Simon Glass]

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",
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 05/16] x86: zboot: Correct image type

[Bin Meng]

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>

[PATCH v2 06/16] x86: zimage: Disable interrupts just before booting

[Simon Glass]

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);
 }
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 06/16] x86: zimage: Disable interrupts just before booting

[Bin Meng]

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>

[PATCH v2 07/16] x86: zboot: Set up a sub-command structure

[Simon Glass]

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
 );
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 07/16] x86: zboot: Set up a sub-command structure

[Bin Meng]

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

[PATCH v2 07/16] x86: zboot: Set up a sub-command structure

[Simon Glass]

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

[PATCH v2 08/16] x86: zboot: Add a 'go' subcommand

[Simon Glass]

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;
 }
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 08/16] x86: zboot: Add a 'go' subcommand

[Bin Meng]

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>

[PATCH v2 09/16] x86: zboot: Add an 'info' subcommand

[Simon Glass]

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;
 }
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 09/16] x86: zboot: Add an 'info' subcommand

[Bin Meng]

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>

[PATCH v2 10/16] x86: zboot: Add an 'setup' subcommand

[Simon Glass]

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
 );
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 10/16] x86: zboot: Add an 'setup' subcommand

[Bin Meng]

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

[PATCH v2 11/16] x86: zboot: Set environment variables for image locations

[Simon Glass]

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;
 }
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 11/16] x86: zboot: Set environment variables for image locations

[Bin Meng]

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

[PATCH v2 12/16] x86: zboot: Allow setting a separate setup base address

[Simon Glass]

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"
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 12/16] x86: zboot: Allow setting a separate setup base address

[Bin Meng]

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>

[PATCH v2 13/16] x86: zboot: Add an option to dump the setup information

[Simon Glass]

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
 );
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 13/16] x86: zboot: Add an option to dump the setup information

[Bin Meng]

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

[PATCH v2 14/16] x86: zboot: Allow overriding the command line

[Simon Glass]

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"
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 14/16] x86: zboot: Allow overriding the command line

[Bin Meng]

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>

[PATCH v2 15/16] cros: Update chromium documentation

[Simon Glass]

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
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 15/16] cros: Update chromium documentation

[Bin Meng]

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>

[PATCH v2 16/16] cros: Add information about booting Chrome OS on x86

[Simon Glass]

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
 -----
 
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 16/16] cros: Add information about booting Chrome OS on x86

[Bin Meng]

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>

[PATCH v2 07/16] x86: zboot: Set up a sub-command structure

[Wolfgang Wallner]

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>