[PATCH 00/19] boot: Start to remove dependency on kernel_addr_r etc.

From: Simon Glass <sjg@chromium.org> Before bootstd, boot scripts were used to boot distros. These worked by using a 'load' command to load a file and a 'bootm' command to boot. Obviously these commands need addresses, so the approach taken was to define a number of environment variables to provide them, for the various situations. These are documented at [1]. With bootstd, booting is built into U-Boot so there is no real need for these variables. Also, bootstd records all loaded images in a list, so knows where and what they are. However, since bootstd uses much of the same code as the bootm command, for example, to date it has used these same variables. This is OK to some extent, since boards maintainers have got used to the need to statically configure the addresses used for a kernel, ramdisk, devicetree, etc. But is does cause problems: - since the kernel and initrd can grow quite a bit from year to year, sometimes the values need to be updated to save space [2] - some boards don't have fixed addresses (such as the EFI app) - fixed values means that much more space must be provided than is normally needed; for this reason the ramdisk is generally at the highest address The variables do have some benefit, e.g. for people who want things loaded at a known location, so it makes sense to keep them around. But they should be optional. This series starts the process of removing the requirement for these variables, focussing on the extlinux code. It mostly consists of refactoring. Several functions are updated to support lmb reservation. At least for filesystems, bootstd can query the file size and use lmb to reserve the required amount of (aligned) memory. The bootmeth read_file() method is updated to handle this. When decompressing, bootm is updated to support reservation (instead of an address) for the decompressed file. With these in place, various patches are provided to work this new feature into the pxe_utils code used by extlinux. The next step after this is to attack the bootm code itself, so that the struct bootm_info controls the addresses, without any environment variables. That will be the subject of the next series. [1] https://docs.u-boot.org/en/latest/develop/distro.html#required-environment-v... [2] https://patchwork.ozlabs.org/project/uboot/patch/20241220003447.2913443-5-sj... Simon Glass (19): boot: Given up pxe boot if kernel_addr_r is missing boot: pxe: Init initrd_str in label_boot() boot: Update bootmeth read_file() to support reservation boot: Update pxe_getfile_func() to support reservation boot: Adjust PXE get_relfile() to support reservation boot: Provide functions to set the bootm string-fields boot: Return the chosen address from get_relfile_envaddr() boot: Convert struct pxe_context initrd address to ulong boot: Convert struct pxe_context initrd_filesize to ulong boot: pxe: Rename conf_fdt to conf_fdt_str boot: Add a conf_fdt ulong to struct pxe_context boot: Drop the allocation in label_boot() boot: Support reserving memory in get_relfile_envaddr() boot: Rename kern_addr in label_boot() boot: Simplify kernel-address handling in label_boot() boot: pxe: Allow loading a kernel without kernel_addr_r boot: pxe: Rename kern_addr in struct pxe_context boot: pxe: Retain the kernel address in the context boot: Pass the full bootm_info to bootm_find_os() boot/bootm.c | 37 +++++-- boot/bootmeth-uclass.c | 24 +++-- boot/bootmeth_android.c | 2 +- boot/bootmeth_cros.c | 2 +- boot/bootmeth_efi.c | 6 +- boot/bootmeth_efi_mgr.c | 2 +- boot/bootmeth_extlinux.c | 9 +- boot/bootmeth_fel.c | 2 +- boot/bootmeth_pxe.c | 23 +++-- boot/bootmeth_qfw.c | 10 +- boot/bootmeth_sandbox.c | 2 +- boot/pxe_utils.c | 214 ++++++++++++++++++++------------------- boot/vbe_abrec.c | 6 +- boot/vbe_abrec_os.c | 16 ++- boot/vbe_simple.c | 9 +- cmd/pxe.c | 7 +- cmd/sysboot.c | 11 +- include/bootm.h | 62 ++++++++++++ include/bootmeth.h | 22 ++-- include/pxe_utils.h | 31 +++--- 20 files changed, 305 insertions(+), 192 deletions(-) -- 2.43.0 base-commit: 6cff8aa606a628ad117ccd9100c36f61d0365dd3 branch: loadk

From: Simon Glass <sjg@chromium.org> At present we require this environment variable in order to do any booting. The check for this actually later (the '"malloc fail' message) but there is a possible NULL access before getting to that point. Check it immediately after reading. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index f524b0a1dd3..97ab7ec3365 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -682,6 +682,10 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) } kernel_addr = env_get("kernel_addr_r"); + if (!kernel_addr) { + printf("No kernel_addr_r available for kernel\n"); + return 1; + } /* for FIT, append the configuration identifier */ if (label->config) { int len = strlen(kernel_addr) + strlen(label->config) + 1; -- 2.43.0

From: Simon Glass <sjg@chromium.org> When 'bootflow read' is used to read images, initrd_str is not inited before strdup() is called. Note that this is only used by developers. Set the variable to an empty string to start. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 97ab7ec3365..99d8cfbe496 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -642,7 +642,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) char *kernel_addr = NULL; char *initrd_addr_str = NULL; char initrd_filesize[10]; - char initrd_str[28]; + char initrd_str[28] = ""; char mac_str[29] = ""; char ip_str[68] = ""; char *fit_addr = NULL; -- 2.43.0

From: Simon Glass <sjg@chromium.org> In some cases we don't have a particular address to read into so would like one reserved. Adjust the read_file() method to support this. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/bootmeth-uclass.c | 24 ++++++++++++++++++------ boot/bootmeth_android.c | 2 +- boot/bootmeth_cros.c | 2 +- boot/bootmeth_efi.c | 6 +++--- boot/bootmeth_efi_mgr.c | 2 +- boot/bootmeth_extlinux.c | 2 +- boot/bootmeth_fel.c | 2 +- boot/bootmeth_pxe.c | 14 +++++++++----- boot/bootmeth_qfw.c | 2 +- boot/bootmeth_sandbox.c | 2 +- boot/vbe_abrec.c | 6 +++--- boot/vbe_abrec_os.c | 2 +- boot/vbe_simple.c | 9 +++++---- include/bootmeth.h | 22 ++++++++++++++++------ 14 files changed, 62 insertions(+), 35 deletions(-) diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c index 603ad98fc3a..6c0eff7579d 100644 --- a/boot/bootmeth-uclass.c +++ b/boot/bootmeth-uclass.c @@ -84,7 +84,7 @@ int bootmeth_boot(struct udevice *dev, struct bootflow *bflow) } int bootmeth_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, + const char *file_path, ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep) { const struct bootmeth_ops *ops = bootmeth_get_ops(dev); @@ -92,7 +92,7 @@ int bootmeth_read_file(struct udevice *dev, struct bootflow *bflow, if (!ops->read_file) return -ENOSYS; - return ops->read_file(dev, bflow, file_path, addr, type, sizep); + return ops->read_file(dev, bflow, file_path, addrp, align, type, sizep); } int bootmeth_get_bootflow(struct udevice *dev, struct bootflow *bflow) @@ -395,7 +395,7 @@ int bootmeth_alloc_other(struct bootflow *bflow, const char *fname, } int bootmeth_common_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, + const char *file_path, ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep) { struct blk_desc *desc = NULL; @@ -414,18 +414,30 @@ int bootmeth_common_read_file(struct udevice *dev, struct bootflow *bflow, if (ret) return log_msg_ret("size", ret); if (size > *sizep) - return log_msg_ret("spc", -ENOSPC); + return log_msg_ret("spc", -E2BIG); ret = bootmeth_setup_fs(bflow, desc); if (ret) return log_msg_ret("fs", ret); - ret = fs_legacy_read(file_path, addr, 0, 0, &len_read); + if (!*addrp) { + phys_addr_t addr; + + if (!IS_ENABLED(CONFIG_LMB)) + return log_msg_ret("brf", -ENOTSUPP); + addr = lmb_alloc(size, align); + + if (!addr) + return -ENOSPC; + *addrp = addr; + } + + ret = fs_legacy_read(file_path, *addrp, 0, 0, &len_read); if (ret) return ret; *sizep = len_read; - if (!bootflow_img_add(bflow, bflow->fname, type, addr, size)) + if (!bootflow_img_add(bflow, bflow->fname, type, *addrp, size)) return log_msg_ret("bci", -ENOMEM); return 0; diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c index d8c92b44a99..69b50d327e8 100644 --- a/boot/bootmeth_android.c +++ b/boot/bootmeth_android.c @@ -323,7 +323,7 @@ static int android_read_bootflow(struct udevice *dev, struct bootflow *bflow) } static int android_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, + const char *file_path, ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep) { /* diff --git a/boot/bootmeth_cros.c b/boot/bootmeth_cros.c index ea4c9ed830f..f92e4437b27 100644 --- a/boot/bootmeth_cros.c +++ b/boot/bootmeth_cros.c @@ -414,7 +414,7 @@ static int cros_read_bootflow(struct udevice *dev, struct bootflow *bflow) } static int cros_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, + const char *file_path, ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep) { return -ENOSYS; diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 2499fd3f219..78fb84334e5 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -63,7 +63,7 @@ static int efiload_read_file(struct bootflow *bflow, ulong addr, ulong max_size) size = max_size; ret = bootmeth_common_read_file(bflow->method, bflow, bflow->fname, - addr, BFI_EFI, &size); + &addr, SZ_2M, BFI_EFI, &size); if (ret) return log_msg_ret("rdf", ret); bflow->buf = map_sysmem(addr, bflow->size); @@ -139,8 +139,8 @@ static int distro_efi_try_bootflow_files(struct udevice *dev, /* Limit FDT files to 4MB */ size = SZ_4M; ret = bootmeth_common_read_file(dev, bflow, fname, - fdt_addr, (enum bootflow_img_t)IH_TYPE_FLATDT, - &size); + &fdt_addr, SZ_4K, + (enum bootflow_img_t)IH_TYPE_FLATDT, &size); } } diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c index 42b8863815e..5e83d1da103 100644 --- a/boot/bootmeth_efi_mgr.c +++ b/boot/bootmeth_efi_mgr.c @@ -74,7 +74,7 @@ static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow) } static int efi_mgr_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, + const char *file_path, ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep) { /* Files are loaded by the 'bootefi bootmgr' command */ diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c index 383f33e681c..cc086cdef0c 100644 --- a/boot/bootmeth_extlinux.c +++ b/boot/bootmeth_extlinux.c @@ -47,7 +47,7 @@ static int extlinux_getfile(struct pxe_context *ctx, const char *file_path, /* Allow up to 1GB */ *sizep = 1 << 30; - ret = bootmeth_read_file(info->dev, info->bflow, file_path, addr, + ret = bootmeth_read_file(info->dev, info->bflow, file_path, &addr, 0, type, sizep); if (ret) return log_msg_ret("read", ret); diff --git a/boot/bootmeth_fel.c b/boot/bootmeth_fel.c index b5a3f3730e3..f2593f6da74 100644 --- a/boot/bootmeth_fel.c +++ b/boot/bootmeth_fel.c @@ -34,7 +34,7 @@ static int fel_read_bootflow(struct udevice *dev, struct bootflow *bflow) } static int fel_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, + const char *file_path, ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep) { return -ENOSYS; diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c index 30caadc8f3e..1025c5acb6c 100644 --- a/boot/bootmeth_pxe.c +++ b/boot/bootmeth_pxe.c @@ -34,7 +34,7 @@ static int extlinux_pxe_getfile(struct pxe_context *ctx, const char *file_path, /* Allow up to 1GB */ *sizep = 1 << 30; - ret = bootmeth_read_file(info->dev, info->bflow, file_path, addr, + ret = bootmeth_read_file(info->dev, info->bflow, file_path, &addr, 0, type, sizep); if (ret) return log_msg_ret("read", ret); @@ -113,15 +113,19 @@ static int extlinux_pxe_read_bootflow(struct udevice *dev, } static int extlinux_pxe_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, - enum bootflow_img_t type, ulong *sizep) + const char *file_path, ulong *addrp, + ulong align, enum bootflow_img_t type, + ulong *sizep) { ulong size; int ret; if (IS_ENABLED(CONFIG_NET_LWIP)) return -ENOTSUPP; - ret = netboot_run(TFTPGET, addr, file_path, 0, false); + if (!*addrp) + return log_msg_ret("tfta", -ENOTSUPP); + + ret = netboot_run(TFTPGET, *addrp, file_path, 0, false); if (ret) return log_msg_ret("tftp", ret); ret = pxe_get_file_size(&size); @@ -131,7 +135,7 @@ static int extlinux_pxe_read_file(struct udevice *dev, struct bootflow *bflow, return log_msg_ret("spc", -ENOSPC); *sizep = size; - if (!bootflow_img_add(bflow, file_path, type, addr, size)) + if (!bootflow_img_add(bflow, file_path, type, *addrp, size)) return log_msg_ret("pxi", -ENOMEM); return 0; diff --git a/boot/bootmeth_qfw.c b/boot/bootmeth_qfw.c index 42ab7dc0a6f..3997dfa6b6f 100644 --- a/boot/bootmeth_qfw.c +++ b/boot/bootmeth_qfw.c @@ -115,7 +115,7 @@ static int qfw_read_files(struct udevice *dev, struct bootflow *bflow, } static int qfw_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, + const char *file_path, ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep) { return -ENOSYS; diff --git a/boot/bootmeth_sandbox.c b/boot/bootmeth_sandbox.c index 92ba2e3f050..92638395bbf 100644 --- a/boot/bootmeth_sandbox.c +++ b/boot/bootmeth_sandbox.c @@ -27,7 +27,7 @@ static int sandbox_read_bootflow(struct udevice *dev, struct bootflow *bflow) } static int sandbox_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, + const char *file_path, ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep) { return -ENOSYS; diff --git a/boot/vbe_abrec.c b/boot/vbe_abrec.c index 4736e6147d2..ce3bcef08a7 100644 --- a/boot/vbe_abrec.c +++ b/boot/vbe_abrec.c @@ -119,14 +119,14 @@ static int vbe_abrec_read_bootflow(struct udevice *dev, struct bootflow *bflow) } static int vbe_abrec_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, + const char *file_path, ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep) { int ret; if (vbe_phase() == VBE_PHASE_OS) { - ret = bootmeth_common_read_file(dev, bflow, file_path, addr, - type, sizep); + ret = bootmeth_common_read_file(dev, bflow, file_path, addrp, + align, type, sizep); if (ret) return log_msg_ret("os", ret); } diff --git a/boot/vbe_abrec_os.c b/boot/vbe_abrec_os.c index 53eb3d79aca..ed4e4ab3dff 100644 --- a/boot/vbe_abrec_os.c +++ b/boot/vbe_abrec_os.c @@ -54,7 +54,7 @@ static int vbe_abrec_getfile(struct pxe_context *ctx, const char *file_path, /* Allow up to 1GB */ *sizep = 1 << 30; - ret = bootmeth_read_file(info->dev, info->bflow, file_path, addr, + ret = bootmeth_read_file(info->dev, info->bflow, file_path, &addr, 0, type, sizep); if (ret) return log_msg_ret("read", ret); diff --git a/boot/vbe_simple.c b/boot/vbe_simple.c index c6766c532f2..3711a70c906 100644 --- a/boot/vbe_simple.c +++ b/boot/vbe_simple.c @@ -98,14 +98,15 @@ static int vbe_simple_read_bootflow(struct udevice *dev, struct bootflow *bflow) } static int vbe_simple_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, - enum bootflow_img_t type, ulong *sizep) + const char *file_path, ulong *addrp, + ulong align, enum bootflow_img_t type, + ulong *sizep) { int ret; if (vbe_phase() == VBE_PHASE_OS) { - ret = bootmeth_common_read_file(dev, bflow, file_path, addr, - type, sizep); + ret = bootmeth_common_read_file(dev, bflow, file_path, addrp, + align, type, sizep); if (ret) return log_msg_ret("os", ret); } diff --git a/include/bootmeth.h b/include/bootmeth.h index d25718d9a14..a34bfc91dd5 100644 --- a/include/bootmeth.h +++ b/include/bootmeth.h @@ -115,7 +115,10 @@ struct bootmeth_ops { * @dev: Bootmethod device to use * @bflow: Bootflow providing info on where to read from * @file_path: Path to file (may be absolute or relative) - * @addr: Address to load file + * @addrp: On entry, address to load file or 0 to reserve an + * address with lmb; on exit, address to which the file was + * loaded + * @align: Reservation alignment, if using lmb * @type: File type (IH_TYPE_...) * @sizep: On entry provides the maximum permitted size; on exit * returns the size of the file @@ -123,7 +126,7 @@ struct bootmeth_ops { * -ve value if something else goes wrong */ int (*read_file)(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, + const char *file_path, ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep); #if CONFIG_IS_ENABLED(BOOTSTD_FULL) /** @@ -248,7 +251,10 @@ int bootmeth_set_bootflow(struct udevice *dev, struct bootflow *bflow, * @dev: Bootmethod device to use * @bflow: Bootflow providing info on where to read from * @file_path: Path to file (may be absolute or relative) - * @addr: Address to load file + * @addrp: On entry, address to load file or 0 to reserve an + * address with lmb; on exit, address to which the file was + * loaded + * @align: Reservation alignment, if using lmb * @type: File type (IH_TYPE_...) * @sizep: On entry provides the maximum permitted size; on exit * returns the size of the file @@ -256,7 +262,7 @@ int bootmeth_set_bootflow(struct udevice *dev, struct bootflow *bflow, * -ve value if something else goes wrong */ int bootmeth_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, + const char *file_path, ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep); /** @@ -402,13 +408,17 @@ int bootmeth_alloc_other(struct bootflow *bflow, const char *fname, * @dev: bootmeth device to read from * @bflow: Bootflow information * @file_path: Path to file - * @addr: Address to load file to + * @addrp: On entry, address to load file or 0 to reserve an address with lmb; + * on exit, address to which the file was loaded + * @align: Reservation alignment, if using lmb * @type: File type (IH_TYPE_...) * @sizep: On entry, the maximum file size to accept, on exit the actual file * size read + * Return: 0 on success, -E2BIG if the file was too large, -ENOSPC if there is + * no memory available in LMB */ int bootmeth_common_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, + const char *file_path, ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep); /** -- 2.43.0

From: Simon Glass <sjg@chromium.org> In some cases we don't have a particular address to read into so would like one reserved. Adjust this function to support that. For now, sysbot_read_file() fails if the address is not provided. This will be resolved in a later patch. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/bootmeth_extlinux.c | 9 +++------ boot/bootmeth_pxe.c | 11 ++++------- boot/pxe_utils.c | 7 ++----- boot/vbe_abrec_os.c | 11 ++++------- cmd/pxe.c | 7 +++++-- cmd/sysboot.c | 11 ++++++----- include/pxe_utils.h | 8 +++++--- 7 files changed, 29 insertions(+), 35 deletions(-) diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c index cc086cdef0c..5a4fefbd868 100644 --- a/boot/bootmeth_extlinux.c +++ b/boot/bootmeth_extlinux.c @@ -36,19 +36,16 @@ static int extlinux_get_state_desc(struct udevice *dev, char *buf, int maxsize) } static int extlinux_getfile(struct pxe_context *ctx, const char *file_path, - char *file_addr, enum bootflow_img_t type, + ulong *addrp, ulong align, enum bootflow_img_t type, ulong *sizep) { struct extlinux_info *info = ctx->userdata; - ulong addr; int ret; - addr = simple_strtoul(file_addr, NULL, 16); - /* Allow up to 1GB */ *sizep = 1 << 30; - ret = bootmeth_read_file(info->dev, info->bflow, file_path, &addr, 0, - type, sizep); + ret = bootmeth_read_file(info->dev, info->bflow, file_path, addrp, + align, type, sizep); if (ret) return log_msg_ret("read", ret); diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c index 1025c5acb6c..3c558266a14 100644 --- a/boot/bootmeth_pxe.c +++ b/boot/bootmeth_pxe.c @@ -23,19 +23,16 @@ #include <pxe_utils.h> static int extlinux_pxe_getfile(struct pxe_context *ctx, const char *file_path, - char *file_addr, enum bootflow_img_t type, - ulong *sizep) + ulong *addrp, ulong align, + enum bootflow_img_t type, ulong *sizep) { struct extlinux_info *info = ctx->userdata; - ulong addr; int ret; - addr = simple_strtoul(file_addr, NULL, 16); - /* Allow up to 1GB */ *sizep = 1 << 30; - ret = bootmeth_read_file(info->dev, info->bflow, file_path, &addr, 0, - type, sizep); + ret = bootmeth_read_file(info->dev, info->bflow, file_path, addrp, + align, type, sizep); if (ret) return log_msg_ret("read", ret); diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 99d8cfbe496..da4e25128c0 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -94,7 +94,7 @@ int format_mac_pxe(char *outbuf, size_t outbuf_len) * * @ctx: PXE context * @file_path: File path to read (relative to the PXE file) - * @file_addr: Address to load file to + * @addr: Address to load file to * @filesizep: If not NULL, returns the file size in bytes * Returns 1 for success, or < 0 on error */ @@ -104,7 +104,6 @@ static int get_relfile(struct pxe_context *ctx, const char *file_path, { size_t path_len; char relfile[MAX_TFTP_PATH_LEN + 1]; - char addr_buf[18]; ulong size; int ret; @@ -125,9 +124,7 @@ static int get_relfile(struct pxe_context *ctx, const char *file_path, printf("Retrieving file: %s\n", relfile); - sprintf(addr_buf, "%lx", file_addr); - - ret = ctx->getfile(ctx, relfile, addr_buf, type, &size); + ret = ctx->getfile(ctx, relfile, &file_addr, 0, type, &size); if (ret < 0) return log_msg_ret("get", ret); if (filesizep) diff --git a/boot/vbe_abrec_os.c b/boot/vbe_abrec_os.c index ed4e4ab3dff..ea23c61f69c 100644 --- a/boot/vbe_abrec_os.c +++ b/boot/vbe_abrec_os.c @@ -43,19 +43,16 @@ static enum vbe_pick_t find_pick(const char *name) } static int vbe_abrec_getfile(struct pxe_context *ctx, const char *file_path, - char *file_addr, enum bootflow_img_t type, - ulong *sizep) + ulong *addrp, ulong align, + enum bootflow_img_t type, ulong *sizep) { struct extlinux_info *info = ctx->userdata; - ulong addr; int ret; - addr = simple_strtoul(file_addr, NULL, 16); - /* Allow up to 1GB */ *sizep = 1 << 30; - ret = bootmeth_read_file(info->dev, info->bflow, file_path, &addr, 0, - type, sizep); + ret = bootmeth_read_file(info->dev, info->bflow, file_path, addrp, + align, type, sizep); if (ret) return log_msg_ret("read", ret); diff --git a/cmd/pxe.c b/cmd/pxe.c index b5f2ff7cb6b..9f40f13c201 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -26,13 +26,16 @@ const char *pxe_default_paths[] = { }; static int do_get_tftp(struct pxe_context *ctx, const char *file_path, - char *file_addr, enum bootflow_img_t type, ulong *sizep) + ulong *addrp, ulong align, enum bootflow_img_t type, + ulong *sizep) { int ret; if (IS_ENABLED(CONFIG_NET_LWIP)) return -ENOTSUPP; - ret = netboot_run(TFTPGET, hextoul(file_addr, NULL), file_path, 0, + if (!*addrp) + return -ENOTSUPP; + ret = netboot_run(TFTPGET, *addrp, file_path, 0, ctx->use_ipv6); if (ret) return log_msg_ret("tfp", ret); diff --git a/cmd/sysboot.c b/cmd/sysboot.c index 1dad64c9ac5..e6f89c999cc 100644 --- a/cmd/sysboot.c +++ b/cmd/sysboot.c @@ -23,19 +23,20 @@ struct sysboot_info { }; static int sysboot_read_file(struct pxe_context *ctx, const char *file_path, - char *file_addr, enum bootflow_img_t type, - ulong *sizep) + ulong *addrp, ulong align, + enum bootflow_img_t type, ulong *sizep) { struct sysboot_info *info = ctx->userdata; loff_t len_read; - ulong addr; int ret; - addr = simple_strtoul(file_addr, NULL, 16); + if (!*addrp) + return -ENOTSUPP; + ret = fs_set_blk_dev(info->ifname, info->dev_part_str, info->fstype); if (ret) return ret; - ret = fs_legacy_read(file_path, addr, 0, 0, &len_read); + ret = fs_legacy_read(file_path, *addrp, 0, 0, &len_read); if (ret) return ret; *sizep = len_read; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 58301a78b03..663b631a969 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -97,13 +97,15 @@ struct pxe_context; * * @ctx: PXE context * @file_path: Full path to filename to read - * @file_addr: String containing the to which to read the file + * @addrp: On entry, address to load file or 0 to reserve an address with lmb; + * on exit, address to which the file was loaded + * @align: Reservation alignment, if using lmb * @type: File type * @fileszeip: Returns file size */ typedef int (*pxe_getfile_func)(struct pxe_context *ctx, const char *file_path, - char *file_addr, enum bootflow_img_t type, - ulong *filesizep); + ulong *addrp, ulong align, + enum bootflow_img_t type, ulong *filesizep); /** * struct pxe_context - context information for PXE parsing -- 2.43.0

From: Simon Glass <sjg@chromium.org> In some cases we don't have a particular address to read into so would like one reserved. Adjust this function to support that. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index da4e25128c0..45bfec48542 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -94,12 +94,14 @@ int format_mac_pxe(char *outbuf, size_t outbuf_len) * * @ctx: PXE context * @file_path: File path to read (relative to the PXE file) - * @addr: Address to load file to + * @addrp: On entry, address to load file or 0 to reserve an address with lmb; + * on exit, address to which the file was loaded + * @align: Reservation alignment, if using lmb * @filesizep: If not NULL, returns the file size in bytes * Returns 1 for success, or < 0 on error */ static int get_relfile(struct pxe_context *ctx, const char *file_path, - unsigned long file_addr, enum bootflow_img_t type, + ulong *addrp, ulong align, enum bootflow_img_t type, ulong *filesizep) { size_t path_len; @@ -124,7 +126,7 @@ static int get_relfile(struct pxe_context *ctx, const char *file_path, printf("Retrieving file: %s\n", relfile); - ret = ctx->getfile(ctx, relfile, &file_addr, 0, type, &size); + ret = ctx->getfile(ctx, relfile, addrp, align, type, &size); if (ret < 0) return log_msg_ret("get", ret); if (filesizep) @@ -140,7 +142,7 @@ int get_pxe_file(struct pxe_context *ctx, const char *file_path, int err; char *buf; - err = get_relfile(ctx, file_path, file_addr, BFI_EXTLINUX_CFG, + err = get_relfile(ctx, file_path, &file_addr, 0, BFI_EXTLINUX_CFG, &size); if (err < 0) return err; @@ -210,7 +212,7 @@ static int get_relfile_envaddr(struct pxe_context *ctx, const char *file_path, if (strict_strtoul(envaddr, 16, &file_addr) < 0) return -EINVAL; - return get_relfile(ctx, file_path, file_addr, type, filesizep); + return get_relfile(ctx, file_path, &file_addr, 0, type, filesizep); } /** @@ -976,7 +978,7 @@ void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg) if (IS_ENABLED(CONFIG_CMD_BMP)) { /* display BMP if available */ if (cfg->bmp) { - if (get_relfile(ctx, cfg->bmp, image_load_addr, + if (get_relfile(ctx, cfg->bmp, &image_load_addr, 0, BFI_LOGO, NULL)) { #if defined(CONFIG_VIDEO) struct udevice *dev; -- 2.43.0

From: Simon Glass <sjg@chromium.org> Provide some helper functions which can set the string value of a field in struct bootm_info from an address. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/bootm.c | 25 ++++++++++++++++-- boot/bootmeth_qfw.c | 8 +++--- boot/vbe_abrec_os.c | 5 ++-- include/bootm.h | 62 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 10 deletions(-) diff --git a/boot/bootm.c b/boot/bootm.c index 46320363270..32232654b12 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -1273,7 +1273,7 @@ int booti_run(struct bootm_info *bmi) int bootm_boot_start(ulong addr, const char *cmdline) { - char addr_str[30]; + char addr_str[BOOTM_STRLEN]; struct bootm_info bmi; int states; int ret; @@ -1296,13 +1296,34 @@ int bootm_boot_start(ulong addr, const char *cmdline) return ret; } bootm_init(&bmi); - bmi.addr_img = addr_str; + bootm_set_addr_img(&bmi, addr, addr_str); bmi.cmd_name = "bootm"; ret = bootm_run_states(&bmi, states); return ret; } +void bootm_set_addr_img_(struct bootm_info *bmi, ulong addr, + char str[BOOTM_STRLEN]) +{ + strlcpy(str, simple_xtoa(addr), BOOTM_STRLEN); + bmi->addr_img = str; +} + +void bootm_set_conf_ramdisk_(struct bootm_info *bmi, ulong addr, + char str[BOOTM_STRLEN]) +{ + strlcpy(str, simple_xtoa(addr), BOOTM_STRLEN); + bmi->conf_ramdisk = str; +} + +void bootm_set_conf_fdt_(struct bootm_info *bmi, ulong addr, + char str[BOOTM_STRLEN]) +{ + strlcpy(str, simple_xtoa(addr), BOOTM_STRLEN); + bmi->conf_fdt = str; +} + void bootm_init(struct bootm_info *bmi) { memset(bmi, '\0', sizeof(struct bootm_info)); diff --git a/boot/bootmeth_qfw.c b/boot/bootmeth_qfw.c index 3997dfa6b6f..43db8da5e24 100644 --- a/boot/bootmeth_qfw.c +++ b/boot/bootmeth_qfw.c @@ -142,7 +142,7 @@ static int qfw_read_all(struct udevice *dev, struct bootflow *bflow) static int qfw_boot(struct udevice *dev, struct bootflow *bflow) { const struct bootflow_img *simg, *kimg, *rimg; - char conf_fdt[20], conf_ramdisk[40], addr_img_str[20]; + char conf_fdt[BOOTM_STRLEN], addr_img[BOOTM_STRLEN], conf_ramdisk[40]; struct bootm_info bmi; int ret; @@ -153,10 +153,8 @@ static int qfw_boot(struct udevice *dev, struct bootflow *bflow) ret = booti_run(&bmi); bootm_init(&bmi); - snprintf(conf_fdt, sizeof(conf_fdt), "%lx", - (ulong)map_to_sysmem(gd->fdt_blob)); - snprintf(addr_img_str, sizeof(addr_img_str), "%lx", kimg->addr); - bmi.addr_img = addr_img_str; + bootm_set_conf_fdt(&bmi, map_to_sysmem(gd->fdt_blob), conf_fdt); + bootm_set_addr_img(&bmi, kimg->addr, addr_img); snprintf(conf_ramdisk, sizeof(conf_ramdisk), "%lx:%lx", rimg->addr, rimg->size); bmi.conf_ramdisk = conf_ramdisk; diff --git a/boot/vbe_abrec_os.c b/boot/vbe_abrec_os.c index ea23c61f69c..3bf2727a6e9 100644 --- a/boot/vbe_abrec_os.c +++ b/boot/vbe_abrec_os.c @@ -211,13 +211,12 @@ static int vbe_abrec_boot(struct udevice *dev, struct bootflow *bflow) img = bootflow_img_find(bflow, BFI_VBE_OEM_FIT); if (img) { struct bootm_info bmi; - char addr_str[30]; + char addr_str[BOOTM_STRLEN]; int states; printf("Loading OEM devicetree from FIT\n"); bootm_init(&bmi); - snprintf(addr_str, sizeof(addr_str), "%lx", img->addr); - bmi.addr_img = addr_str; + bootm_set_addr_img(&bmi, img->addr, addr_str); bmi.cmd_name = "vbe_os"; states = BOOTM_STATE_START | BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD | BOOTM_STATE_FINDOTHER | diff --git a/include/bootm.h b/include/bootm.h index 9ddf9fd364d..7420c657293 100644 --- a/include/bootm.h +++ b/include/bootm.h @@ -104,6 +104,68 @@ struct bootm_info { #define bootm_x86_set(_bmi, _field, _val) #endif +/* length of strings needed to hold an address within struct bootm_info */ +enum { + BOOTM_STRLEN = 2 * sizeof(long) + 1, +}; + +/** + * bootm_set_addr_img() - Set the address of an image + * + * This only supports setting a single address, with no FIT configuration, etc. + * + * @bmi: Bootm information + * @addr: Address to set + * @str: String to hold the address (must be maintained by the caller) + */ +void bootm_set_addr_img_(struct bootm_info *bmi, ulong addr, + char str[BOOTM_STRLEN]); + +#define bootm_set_addr_img(bmi, addr, str) \ + ({ \ + _Static_assert(sizeof(str) >= BOOTM_STRLEN, \ + "string buffer too small"); \ + bootm_set_addr_img_(bmi, addr, str); \ + }) + +/** + * bootm_set_conf_ramdisk() - Set the address of a ramdisk + * + * This only supports setting a single address, with no FIT configuration, etc. + * + * @bmi: Bootm information + * @addr: Address to set + * @str: String to hold the address (must be maintained by the caller) + */ +void bootm_set_conf_ramdisk_(struct bootm_info *bmi, ulong addr, + char str[BOOTM_STRLEN]); + +#define bootm_set_conf_ramdisk(bmi, addr, str) \ + ({ \ + _Static_assert(sizeof(str) >= BOOTM_STRLEN, \ + "string buffer too small"); \ + bootm_set_conf_ramdisk_(bmi, addr, str); \ + }) + +/** + * bootm_set_conf_fdt() - Set the address of the FDT + * + * This only supports setting a single address, with no FIT configuration, etc. + * + * @bmi: Bootm information + * @addr: Address to set + * @str: String to hold the address (must be maintained by the caller) + */ +void bootm_set_conf_fdt_(struct bootm_info *bmi, ulong addr, + char str[BOOTM_STRLEN]); + +#define bootm_set_conf_fdt(bmi, addr, str) \ + ({ \ + _Static_assert(sizeof(str) >= BOOTM_STRLEN, \ + "string buffer too small"); \ + bootm_set_conf_fdt_(bmi, addr, str); \ + }) + static inline ulong bootm_len(void) { #ifdef CONFIG_SYS_BOOTM_LEN -- 2.43.0

From: Simon Glass <sjg@chromium.org> Update this function to return the address obtained from the environment variable. That will allow the caller to know where the image ended up. For now this is not used. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 45bfec48542..c8532fad020 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -193,6 +193,7 @@ int get_pxelinux_path(struct pxe_context *ctx, const char *file, * @envaddr_name: Name of environment variable which contains the address to * load to * @type: File type + * @addrp: Returns the address to which the file was loaded, on success * @filesizep: Returns the file size in bytes * Returns 1 on success, -ENOENT if @envaddr_name does not exist as an * environment variable, -EINVAL if its format is not valid hex, or other @@ -200,19 +201,26 @@ int get_pxelinux_path(struct pxe_context *ctx, const char *file, */ static int get_relfile_envaddr(struct pxe_context *ctx, const char *file_path, const char *envaddr_name, - enum bootflow_img_t type, ulong *filesizep) + enum bootflow_img_t type, ulong *addrp, + ulong *filesizep) { - unsigned long file_addr; + ulong addr; char *envaddr; + int ret; envaddr = from_env(envaddr_name); if (!envaddr) return -ENOENT; - if (strict_strtoul(envaddr, 16, &file_addr) < 0) + if (strict_strtoul(envaddr, 16, &addr) < 0) return -EINVAL; - return get_relfile(ctx, file_path, &file_addr, 0, type, filesizep); + ret = get_relfile(ctx, file_path, &addr, 0, type, filesizep); + if (ret != 1) + return ret; + *addrp = addr; + + return 1; } /** @@ -330,6 +338,7 @@ static void label_boot_fdtoverlay(struct pxe_context *ctx, do { struct fdt_header *blob; char *overlayfile; + ulong addr; char *end; int len; @@ -353,7 +362,7 @@ static void label_boot_fdtoverlay(struct pxe_context *ctx, /* Load overlay file */ err = get_relfile_envaddr(ctx, overlayfile, "fdtoverlay_addr_r", (enum bootflow_img_t)IH_TYPE_FLATDT, - NULL); + &addr, NULL); if (err < 0) { printf("Failed loading overlay %s\n", overlayfile); goto skip_overlay; @@ -484,9 +493,12 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, } if (fdtfile) { - int err = get_relfile_envaddr(ctx, fdtfile, - "fdt_addr_r", - (enum bootflow_img_t)IH_TYPE_FLATDT, NULL); + ulong addr; + int err; + + err = get_relfile_envaddr(ctx, fdtfile, "fdt_addr_r", + (enum bootflow_img_t)IH_TYPE_FLATDT, + &addr, NULL); free(fdtfilefree); if (err < 0) { @@ -646,6 +658,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) char ip_str[68] = ""; char *fit_addr = NULL; const char *conf_fdt; + ulong addr; int ret; label_print(label); @@ -673,8 +686,8 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) } if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r", - (enum bootflow_img_t)IH_TYPE_KERNEL, NULL) - < 0) { + (enum bootflow_img_t)IH_TYPE_KERNEL, &addr, + NULL) < 0) { printf("Skipping %s for failure retrieving kernel\n", label->name); return 1; @@ -707,7 +720,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) ret = get_relfile_envaddr(ctx, label->initrd, "ramdisk_addr_r", (enum bootflow_img_t)IH_TYPE_RAMDISK, - &size); + &addr, &size); if (ret < 0) { printf("Skipping %s for failure retrieving initrd\n", label->name); -- 2.43.0

From: Simon Glass <sjg@chromium.org> Several of the strings in this struct would be better off as addresses, now that they are passed around a fair bit. Start by converting initrd_addr_str to ulong Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 45 +++++++++++++++++++++------------------------ include/pxe_utils.h | 9 ++++----- 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index c8532fad020..ada66d1ba7d 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -537,16 +537,15 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, * @ctx: PXE context * @label: Label to process * @kernel_addr: String containing kernel address (cannot be NULL) - * @initrd_addr_str: String containing initrd address (NULL if none) - * @initrd_filesize: String containing initrd size (only used if - * @initrd_addr_str) - * @initrd_str: initrd string to process (only used if @initrd_addr_str) + * @initrd_addr: String containing initrd address (0 if none) + * @initrd_filesize: String containing initrd size (only used if @initrd_addr) + * @initrd_str: initrd string to process (only used if @initrd_addr) * @conf_fdt: string containing the FDT address * Return: does not return on success, or returns 0 if the boot command * returned, or -ve error value on error */ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, - char *kernel_addr, char *initrd_addr_str, + char *kernel_addr, ulong initrd_addr, char *initrd_filesize, char *initrd_str, const char *conf_fdt) { @@ -562,10 +561,9 @@ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, bmi.addr_img = kernel_addr; bootm_x86_set(&bmi, bzimage_addr, hextoul(kernel_addr, NULL)); - if (initrd_addr_str) { + if (initrd_addr) { bmi.conf_ramdisk = initrd_str; - bootm_x86_set(&bmi, initrd_addr, - hextoul(initrd_addr_str, NULL)); + bootm_x86_set(&bmi, initrd_addr, initrd_addr); bootm_x86_set(&bmi, initrd_size, hextoul(initrd_filesize, NULL)); } @@ -651,7 +649,7 @@ static int generate_localboot(struct pxe_label *label) static int label_boot(struct pxe_context *ctx, struct pxe_label *label) { char *kernel_addr = NULL; - char *initrd_addr_str = NULL; + ulong initrd_addr = 0; char initrd_filesize[10]; char initrd_str[28] = ""; char mac_str[29] = ""; @@ -713,23 +711,22 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) /* For FIT, the label can be identical to kernel one */ if (label->initrd && !strcmp(label->kernel_label, label->initrd)) { - initrd_addr_str = kernel_addr; + initrd_addr = hextoul(kernel_addr, NULL); } else if (label->initrd) { ulong size; int ret; ret = get_relfile_envaddr(ctx, label->initrd, "ramdisk_addr_r", (enum bootflow_img_t)IH_TYPE_RAMDISK, - &addr, &size); + &initrd_addr, &size); if (ret < 0) { printf("Skipping %s for failure retrieving initrd\n", label->name); goto cleanup; } strcpy(initrd_filesize, simple_xtoa(size)); - initrd_addr_str = env_get("ramdisk_addr_r"); - size = snprintf(initrd_str, sizeof(initrd_str), "%s:%lx", - initrd_addr_str, size); + size = snprintf(initrd_str, sizeof(initrd_str), "%lx:%lx", + initrd_addr, size); if (size >= sizeof(initrd_str)) goto cleanup; } @@ -806,8 +803,8 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (IS_ENABLED(CONFIG_BOOTSTD_FULL) && ctx->no_boot) { ctx->label = label; ctx->kernel_addr = strdup(kernel_addr); - if (initrd_addr_str) { - ctx->initrd_addr_str = strdup(initrd_addr_str); + if (initrd_addr) { + ctx->initrd_addr = initrd_addr; ctx->initrd_filesize = strdup(initrd_filesize); ctx->initrd_str = strdup(initrd_str); } @@ -815,22 +812,22 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) log_debug("Saving label '%s':\n", label->name); log_debug("- kernel_addr '%s' conf_fdt '%s'\n", ctx->kernel_addr, ctx->conf_fdt); - if (initrd_addr_str) { - log_debug("- initrd addr '%s' filesize '%s' str '%s'\n", - ctx->initrd_addr_str, ctx->initrd_filesize, + if (initrd_addr) { + log_debug("- initrd addr %lx filesize '%s' str '%s'\n", + ctx->initrd_addr, ctx->initrd_filesize, ctx->initrd_str); } if (!ctx->kernel_addr || (conf_fdt && !ctx->conf_fdt) || - (initrd_addr_str && (!ctx->initrd_addr_str || - !ctx->initrd_filesize || !ctx->initrd_str))) { + (initrd_addr && + (!ctx->initrd_filesize || !ctx->initrd_str))) { printf("malloc fail (saving label)\n"); return 1; } return 0; } - label_run_boot(ctx, label, kernel_addr, initrd_addr_str, - initrd_filesize, initrd_str, conf_fdt); + label_run_boot(ctx, label, kernel_addr, initrd_addr, initrd_filesize, + initrd_str, conf_fdt); /* ignore the error value since we are going to fail anyway */ cleanup: @@ -1128,7 +1125,7 @@ int pxe_do_boot(struct pxe_context *ctx) return log_msg_ret("pxb", -ENOENT); ret = label_run_boot(ctx, ctx->label, ctx->kernel_addr, - ctx->initrd_addr_str, ctx->initrd_filesize, + ctx->initrd_addr, ctx->initrd_filesize, ctx->initrd_str, ctx->conf_fdt); if (ret) return log_msg_ret("lrb", ret); diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 663b631a969..448108a3886 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -126,10 +126,9 @@ typedef int (*pxe_getfile_func)(struct pxe_context *ctx, const char *file_path, * The following are only used when probing for a label * @label: Label to process * @kernel_addr: String containing kernel address (cannot be NULL) - * @initrd_addr_str: String containing initaddr address (NULL if none) - * @initrd_filesize: String containing initrd size (only used if - * @initrd_addr_str) - * @initrd_str: initrd string to process (only used if @initrd_addr_str) + * @initrd_addr: initaddr address (0 if none) + * @initrd_filesize: String containing initrd size (only used if @initrd_addr) + * @initrd_str: initrd string to process (only used if @initrd_addr) * @conf_fdt: string containing the FDT address * @restart: true to use BOOTM_STATE_RESTART instead of BOOTM_STATE_START (only * supported with FIT / bootm) @@ -160,7 +159,7 @@ struct pxe_context { /* information on the selected label to boot */ struct pxe_label *label; char *kernel_addr; - char *initrd_addr_str; + ulong initrd_addr; char *initrd_filesize; char *initrd_str; char *conf_fdt; -- 2.43.0

From: Simon Glass <sjg@chromium.org> Convert initrd_filesize to a ulong and rename it. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 24 +++++++++++------------- include/pxe_utils.h | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index ada66d1ba7d..c063089c720 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -538,7 +538,7 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, * @label: Label to process * @kernel_addr: String containing kernel address (cannot be NULL) * @initrd_addr: String containing initrd address (0 if none) - * @initrd_filesize: String containing initrd size (only used if @initrd_addr) + * @initrd_size: initrd size (only used if @initrd_addr) * @initrd_str: initrd string to process (only used if @initrd_addr) * @conf_fdt: string containing the FDT address * Return: does not return on success, or returns 0 if the boot command @@ -546,7 +546,7 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, */ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, char *kernel_addr, ulong initrd_addr, - char *initrd_filesize, char *initrd_str, + ulong initrd_size, char *initrd_str, const char *conf_fdt) { struct bootm_info bmi; @@ -564,8 +564,7 @@ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, if (initrd_addr) { bmi.conf_ramdisk = initrd_str; bootm_x86_set(&bmi, initrd_addr, initrd_addr); - bootm_x86_set(&bmi, initrd_size, - hextoul(initrd_filesize, NULL)); + bootm_x86_set(&bmi, initrd_size, initrd_size); } kernel_addr_r = genimg_get_kernel_addr(kernel_addr); @@ -650,7 +649,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) { char *kernel_addr = NULL; ulong initrd_addr = 0; - char initrd_filesize[10]; + ulong initrd_size = 0; char initrd_str[28] = ""; char mac_str[29] = ""; char ip_str[68] = ""; @@ -724,7 +723,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) label->name); goto cleanup; } - strcpy(initrd_filesize, simple_xtoa(size)); + initrd_size = size; size = snprintf(initrd_str, sizeof(initrd_str), "%lx:%lx", initrd_addr, size); if (size >= sizeof(initrd_str)) @@ -805,7 +804,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) ctx->kernel_addr = strdup(kernel_addr); if (initrd_addr) { ctx->initrd_addr = initrd_addr; - ctx->initrd_filesize = strdup(initrd_filesize); + ctx->initrd_size = initrd_size; ctx->initrd_str = strdup(initrd_str); } ctx->conf_fdt = strdup(conf_fdt); @@ -813,20 +812,19 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) log_debug("- kernel_addr '%s' conf_fdt '%s'\n", ctx->kernel_addr, ctx->conf_fdt); if (initrd_addr) { - log_debug("- initrd addr %lx filesize '%s' str '%s'\n", - ctx->initrd_addr, ctx->initrd_filesize, + log_debug("- initrd addr %lx filesize %lx str '%s'\n", + ctx->initrd_addr, ctx->initrd_size, ctx->initrd_str); } if (!ctx->kernel_addr || (conf_fdt && !ctx->conf_fdt) || - (initrd_addr && - (!ctx->initrd_filesize || !ctx->initrd_str))) { + (initrd_addr && !ctx->initrd_str)) { printf("malloc fail (saving label)\n"); return 1; } return 0; } - label_run_boot(ctx, label, kernel_addr, initrd_addr, initrd_filesize, + label_run_boot(ctx, label, kernel_addr, initrd_addr, initrd_size, initrd_str, conf_fdt); /* ignore the error value since we are going to fail anyway */ @@ -1125,7 +1123,7 @@ int pxe_do_boot(struct pxe_context *ctx) return log_msg_ret("pxb", -ENOENT); ret = label_run_boot(ctx, ctx->label, ctx->kernel_addr, - ctx->initrd_addr, ctx->initrd_filesize, + ctx->initrd_addr, ctx->initrd_size, ctx->initrd_str, ctx->conf_fdt); if (ret) return log_msg_ret("lrb", ret); diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 448108a3886..8567eb7ec3c 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -127,7 +127,7 @@ typedef int (*pxe_getfile_func)(struct pxe_context *ctx, const char *file_path, * @label: Label to process * @kernel_addr: String containing kernel address (cannot be NULL) * @initrd_addr: initaddr address (0 if none) - * @initrd_filesize: String containing initrd size (only used if @initrd_addr) + * @initrd_size: initrd size (only used if @initrd_addr) * @initrd_str: initrd string to process (only used if @initrd_addr) * @conf_fdt: string containing the FDT address * @restart: true to use BOOTM_STATE_RESTART instead of BOOTM_STATE_START (only @@ -160,7 +160,7 @@ struct pxe_context { struct pxe_label *label; char *kernel_addr; ulong initrd_addr; - char *initrd_filesize; + ulong initrd_size; char *initrd_str; char *conf_fdt; bool restart; -- 2.43.0

From: Simon Glass <sjg@chromium.org> This is a string, so add a string suffix to make that clear. Switch the assignment order in label_run_boot(), since the FDT is loaded after the kernel. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 36 ++++++++++++++++++------------------ include/pxe_utils.h | 2 +- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index c063089c720..e25eefcdc7d 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -540,14 +540,14 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, * @initrd_addr: String containing initrd address (0 if none) * @initrd_size: initrd size (only used if @initrd_addr) * @initrd_str: initrd string to process (only used if @initrd_addr) - * @conf_fdt: string containing the FDT address + * @conf_fdt_str: string containing the FDT address * Return: does not return on success, or returns 0 if the boot command * returned, or -ve error value on error */ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, char *kernel_addr, ulong initrd_addr, ulong initrd_size, char *initrd_str, - const char *conf_fdt) + const char *conf_fdt_str) { struct bootm_info bmi; ulong kernel_addr_r; @@ -557,8 +557,8 @@ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, bootm_init(&bmi); - bmi.conf_fdt = conf_fdt; bmi.addr_img = kernel_addr; + bmi.conf_fdt = conf_fdt_str; bootm_x86_set(&bmi, bzimage_addr, hextoul(kernel_addr, NULL)); if (initrd_addr) { @@ -654,7 +654,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) char mac_str[29] = ""; char ip_str[68] = ""; char *fit_addr = NULL; - const char *conf_fdt; + const char *conf_fdt_str; ulong addr; int ret; @@ -772,18 +772,18 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) printf("append: %s\n", finalbootargs); } - conf_fdt = env_get("fdt_addr_r"); - ret = label_process_fdt(ctx, label, kernel_addr, &conf_fdt); + conf_fdt_str = env_get("fdt_addr_r"); + ret = label_process_fdt(ctx, label, kernel_addr, &conf_fdt_str); if (ret) return ret; - if (!conf_fdt) { + if (!conf_fdt_str) { if (!IS_ENABLED(CONFIG_SUPPORT_PASSING_ATAGS) || strcmp("-", label->fdt)) - conf_fdt = env_get("fdt_addr"); + conf_fdt_str = env_get("fdt_addr"); } - if (!conf_fdt) { + if (!conf_fdt_str) { ulong kernel_addr_r; void *buf; @@ -792,12 +792,12 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (genimg_get_format(buf) != IMAGE_FORMAT_FIT) { if (!IS_ENABLED(CONFIG_SUPPORT_PASSING_ATAGS) || strcmp("-", label->fdt)) - conf_fdt = env_get("fdtcontroladdr"); + conf_fdt_str = env_get("fdtcontroladdr"); } unmap_sysmem(buf); } - if (ctx->bflow && conf_fdt) - ctx->bflow->fdt_addr = hextoul(conf_fdt, NULL); + if (ctx->bflow && conf_fdt_str) + ctx->bflow->fdt_addr = hextoul(conf_fdt_str, NULL); if (IS_ENABLED(CONFIG_BOOTSTD_FULL) && ctx->no_boot) { ctx->label = label; @@ -807,16 +807,16 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) ctx->initrd_size = initrd_size; ctx->initrd_str = strdup(initrd_str); } - ctx->conf_fdt = strdup(conf_fdt); + ctx->conf_fdt_str = strdup(conf_fdt_str); log_debug("Saving label '%s':\n", label->name); - log_debug("- kernel_addr '%s' conf_fdt '%s'\n", - ctx->kernel_addr, ctx->conf_fdt); + log_debug("- kernel_addr '%s' conf_fdt_str '%s'\n", + ctx->kernel_addr, ctx->conf_fdt_str); if (initrd_addr) { log_debug("- initrd addr %lx filesize %lx str '%s'\n", ctx->initrd_addr, ctx->initrd_size, ctx->initrd_str); } - if (!ctx->kernel_addr || (conf_fdt && !ctx->conf_fdt) || + if (!ctx->kernel_addr || (conf_fdt_str && !ctx->conf_fdt_str) || (initrd_addr && !ctx->initrd_str)) { printf("malloc fail (saving label)\n"); return 1; @@ -825,7 +825,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) } label_run_boot(ctx, label, kernel_addr, initrd_addr, initrd_size, - initrd_str, conf_fdt); + initrd_str, conf_fdt_str); /* ignore the error value since we are going to fail anyway */ cleanup: @@ -1124,7 +1124,7 @@ int pxe_do_boot(struct pxe_context *ctx) ret = label_run_boot(ctx, ctx->label, ctx->kernel_addr, ctx->initrd_addr, ctx->initrd_size, - ctx->initrd_str, ctx->conf_fdt); + ctx->initrd_str, ctx->conf_fdt_str); if (ret) return log_msg_ret("lrb", ret); diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 8567eb7ec3c..d9bcea89d87 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -162,7 +162,7 @@ struct pxe_context { ulong initrd_addr; ulong initrd_size; char *initrd_str; - char *conf_fdt; + char *conf_fdt_str; bool restart; }; -- 2.43.0

From: Simon Glass <sjg@chromium.org> Keep this value around separately from the string, since the string needs to be converted before use. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 18 ++++++++++++------ include/pxe_utils.h | 4 +++- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index e25eefcdc7d..b9a88fb11d2 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -541,13 +541,14 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, * @initrd_size: initrd size (only used if @initrd_addr) * @initrd_str: initrd string to process (only used if @initrd_addr) * @conf_fdt_str: string containing the FDT address + * @conf_fdt: FDT address (0 if none) * Return: does not return on success, or returns 0 if the boot command * returned, or -ve error value on error */ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, char *kernel_addr, ulong initrd_addr, ulong initrd_size, char *initrd_str, - const char *conf_fdt_str) + const char *conf_fdt_str, ulong conf_fdt) { struct bootm_info bmi; ulong kernel_addr_r; @@ -655,6 +656,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) char ip_str[68] = ""; char *fit_addr = NULL; const char *conf_fdt_str; + ulong conf_fdt = 0; ulong addr; int ret; @@ -796,8 +798,11 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) } unmap_sysmem(buf); } + if (conf_fdt_str) + conf_fdt = hextoul(conf_fdt_str, NULL); + if (ctx->bflow && conf_fdt_str) - ctx->bflow->fdt_addr = hextoul(conf_fdt_str, NULL); + ctx->bflow->fdt_addr = conf_fdt; if (IS_ENABLED(CONFIG_BOOTSTD_FULL) && ctx->no_boot) { ctx->label = label; @@ -808,9 +813,10 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) ctx->initrd_str = strdup(initrd_str); } ctx->conf_fdt_str = strdup(conf_fdt_str); + ctx->conf_fdt = conf_fdt; log_debug("Saving label '%s':\n", label->name); - log_debug("- kernel_addr '%s' conf_fdt_str '%s'\n", - ctx->kernel_addr, ctx->conf_fdt_str); + log_debug("- kernel_addr '%s' conf_fdt_str '%s' conf_fdt %lx\n", + ctx->kernel_addr, ctx->conf_fdt_str, conf_fdt); if (initrd_addr) { log_debug("- initrd addr %lx filesize %lx str '%s'\n", ctx->initrd_addr, ctx->initrd_size, @@ -825,7 +831,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) } label_run_boot(ctx, label, kernel_addr, initrd_addr, initrd_size, - initrd_str, conf_fdt_str); + initrd_str, conf_fdt_str, conf_fdt); /* ignore the error value since we are going to fail anyway */ cleanup: @@ -1124,7 +1130,7 @@ int pxe_do_boot(struct pxe_context *ctx) ret = label_run_boot(ctx, ctx->label, ctx->kernel_addr, ctx->initrd_addr, ctx->initrd_size, - ctx->initrd_str, ctx->conf_fdt_str); + ctx->initrd_str, ctx->conf_fdt_str, ctx->conf_fdt); if (ret) return log_msg_ret("lrb", ret); diff --git a/include/pxe_utils.h b/include/pxe_utils.h index d9bcea89d87..a17af953d2a 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -129,7 +129,8 @@ typedef int (*pxe_getfile_func)(struct pxe_context *ctx, const char *file_path, * @initrd_addr: initaddr address (0 if none) * @initrd_size: initrd size (only used if @initrd_addr) * @initrd_str: initrd string to process (only used if @initrd_addr) - * @conf_fdt: string containing the FDT address + * @conf_fdt_str: FDT-address string + * @conf_fdt: FDT address * @restart: true to use BOOTM_STATE_RESTART instead of BOOTM_STATE_START (only * supported with FIT / bootm) */ @@ -163,6 +164,7 @@ struct pxe_context { ulong initrd_size; char *initrd_str; char *conf_fdt_str; + ulong conf_fdt; bool restart; }; -- 2.43.0

From: Simon Glass <sjg@chromium.org> It doesn't seem worth allocating space for a label when it is normally going to be under 20 characters. Use a fixed-sized string (with plenty of space) instead. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index b9a88fb11d2..000ca0bd8a6 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -654,7 +654,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) char initrd_str[28] = ""; char mac_str[29] = ""; char ip_str[68] = ""; - char *fit_addr = NULL; + char fit_addr[200]; const char *conf_fdt_str; ulong conf_fdt = 0; ulong addr; @@ -699,14 +699,8 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) } /* for FIT, append the configuration identifier */ if (label->config) { - int len = strlen(kernel_addr) + strlen(label->config) + 1; - - fit_addr = malloc(len); - if (!fit_addr) { - printf("malloc fail (FIT address)\n"); - return 1; - } - snprintf(fit_addr, len, "%s%s", kernel_addr, label->config); + snprintf(fit_addr, sizeof(fit_addr), "%s%s", kernel_addr, + label->config); kernel_addr = fit_addr; } @@ -723,13 +717,13 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (ret < 0) { printf("Skipping %s for failure retrieving initrd\n", label->name); - goto cleanup; + return 1; } initrd_size = size; size = snprintf(initrd_str, sizeof(initrd_str), "%lx:%lx", initrd_addr, size); if (size >= sizeof(initrd_str)) - goto cleanup; + return 1; } if (label->ipappend & 0x1) { @@ -759,7 +753,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) strlen(label->append ?: ""), strlen(ip_str), strlen(mac_str), sizeof(bootargs)); - goto cleanup; + return 1; } if (label->append) @@ -834,9 +828,6 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) initrd_str, conf_fdt_str, conf_fdt); /* ignore the error value since we are going to fail anyway */ -cleanup: - free(fit_addr); - return 1; /* returning is always failure */ } -- 2.43.0

From: Simon Glass <sjg@chromium.org> We normally require environment variables when loading files using PXE/ syslinux. This is not really necessary, since we can use lmb to find a suitable space. Add support for reserving space in get_relfile_envaddr(), which is the commonly used function for reading a file. Use env_get() so no error is shown if the environment variable does not exist. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 000ca0bd8a6..00554ec056e 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -191,7 +191,8 @@ int get_pxelinux_path(struct pxe_context *ctx, const char *file, * @ctx: PXE context * @file_path: File path to read (relative to the PXE file) * @envaddr_name: Name of environment variable which contains the address to - * load to + * load to. If this doesn't exist, an address is reserved using LMB + * @align: Reservation alignment, if using lmb * @type: File type * @addrp: Returns the address to which the file was loaded, on success * @filesizep: Returns the file size in bytes @@ -200,22 +201,23 @@ int get_pxelinux_path(struct pxe_context *ctx, const char *file, * value < 0 on other error */ static int get_relfile_envaddr(struct pxe_context *ctx, const char *file_path, - const char *envaddr_name, + const char *envaddr_name, ulong align, enum bootflow_img_t type, ulong *addrp, ulong *filesizep) { - ulong addr; + ulong addr = 0; char *envaddr; int ret; - envaddr = from_env(envaddr_name); - if (!envaddr) - return -ENOENT; - - if (strict_strtoul(envaddr, 16, &addr) < 0) + /* + * set the address if we have it, otherwise get_relfile() will reserve + * a space + */ + envaddr = env_get(envaddr_name); + if (envaddr && strict_strtoul(envaddr, 16, &addr) < 0) return -EINVAL; - ret = get_relfile(ctx, file_path, &addr, 0, type, filesizep); + ret = get_relfile(ctx, file_path, &addr, align, type, filesizep); if (ret != 1) return ret; *addrp = addr; @@ -361,6 +363,7 @@ static void label_boot_fdtoverlay(struct pxe_context *ctx, /* Load overlay file */ err = get_relfile_envaddr(ctx, overlayfile, "fdtoverlay_addr_r", + SZ_4K, (enum bootflow_img_t)IH_TYPE_FLATDT, &addr, NULL); if (err < 0) { @@ -497,6 +500,7 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, int err; err = get_relfile_envaddr(ctx, fdtfile, "fdt_addr_r", + SZ_4K, (enum bootflow_img_t)IH_TYPE_FLATDT, &addr, NULL); @@ -684,9 +688,9 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) return 1; } - if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r", - (enum bootflow_img_t)IH_TYPE_KERNEL, &addr, - NULL) < 0) { + if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r", SZ_2M, + (enum bootflow_img_t)IH_TYPE_KERNEL, + &addr, NULL) < 0) { printf("Skipping %s for failure retrieving kernel\n", label->name); return 1; @@ -712,6 +716,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) int ret; ret = get_relfile_envaddr(ctx, label->initrd, "ramdisk_addr_r", + SZ_2M, (enum bootflow_img_t)IH_TYPE_RAMDISK, &initrd_addr, &size); if (ret < 0) { -- 2.43.0

From: Simon Glass <sjg@chromium.org> Rename this variable to kern_addr_str so that it is clear it is a string and not an address. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 00554ec056e..48804157d8f 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -652,7 +652,7 @@ static int generate_localboot(struct pxe_label *label) */ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) { - char *kernel_addr = NULL; + char *kern_addr_str = NULL; ulong initrd_addr = 0; ulong initrd_size = 0; char initrd_str[28] = ""; @@ -696,21 +696,21 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) return 1; } - kernel_addr = env_get("kernel_addr_r"); - if (!kernel_addr) { + kern_addr_str = env_get("kernel_addr_r"); + if (!kern_addr_str) { printf("No kernel_addr_r available for kernel\n"); return 1; } /* for FIT, append the configuration identifier */ if (label->config) { - snprintf(fit_addr, sizeof(fit_addr), "%s%s", kernel_addr, + snprintf(fit_addr, sizeof(fit_addr), "%s%s", kern_addr_str, label->config); - kernel_addr = fit_addr; + kern_addr_str = fit_addr; } /* For FIT, the label can be identical to kernel one */ if (label->initrd && !strcmp(label->kernel_label, label->initrd)) { - initrd_addr = hextoul(kernel_addr, NULL); + initrd_addr = hextoul(kern_addr_str, NULL); } else if (label->initrd) { ulong size; int ret; @@ -774,7 +774,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) } conf_fdt_str = env_get("fdt_addr_r"); - ret = label_process_fdt(ctx, label, kernel_addr, &conf_fdt_str); + ret = label_process_fdt(ctx, label, kern_addr_str, &conf_fdt_str); if (ret) return ret; @@ -788,7 +788,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) ulong kernel_addr_r; void *buf; - kernel_addr_r = genimg_get_kernel_addr(kernel_addr); + kernel_addr_r = genimg_get_kernel_addr(kern_addr_str); buf = map_sysmem(kernel_addr_r, 0); if (genimg_get_format(buf) != IMAGE_FORMAT_FIT) { if (!IS_ENABLED(CONFIG_SUPPORT_PASSING_ATAGS) || @@ -805,7 +805,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (IS_ENABLED(CONFIG_BOOTSTD_FULL) && ctx->no_boot) { ctx->label = label; - ctx->kernel_addr = strdup(kernel_addr); + ctx->kernel_addr = strdup(kern_addr_str); if (initrd_addr) { ctx->initrd_addr = initrd_addr; ctx->initrd_size = initrd_size; @@ -829,7 +829,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) return 0; } - label_run_boot(ctx, label, kernel_addr, initrd_addr, initrd_size, + label_run_boot(ctx, label, kern_addr_str, initrd_addr, initrd_size, initrd_str, conf_fdt_str, conf_fdt); /* ignore the error value since we are going to fail anyway */ -- 2.43.0

From: Simon Glass <sjg@chromium.org> This function is needlessly convoluted, reading the kernel address as a string, possibly adding the FIT configuration, then parsing that to obtain the kernel address again. Create a kern_addr variable to hold the kernel address, so we can simplify the code. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 48804157d8f..3b487b2ed8e 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -652,7 +652,8 @@ static int generate_localboot(struct pxe_label *label) */ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) { - char *kern_addr_str = NULL; + char *kern_addr_str; + ulong kern_addr = 0; ulong initrd_addr = 0; ulong initrd_size = 0; char initrd_str[28] = ""; @@ -696,21 +697,20 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) return 1; } - kern_addr_str = env_get("kernel_addr_r"); - if (!kern_addr_str) { + kern_addr = env_get_hex("kernel_addr_r", 0); + if (!kern_addr) { printf("No kernel_addr_r available for kernel\n"); return 1; } + /* for FIT, append the configuration identifier */ - if (label->config) { - snprintf(fit_addr, sizeof(fit_addr), "%s%s", kern_addr_str, - label->config); - kern_addr_str = fit_addr; - } + snprintf(fit_addr, sizeof(fit_addr), "%lx%s", kern_addr, + label->config ? label->config : ""); + kern_addr_str = fit_addr; /* For FIT, the label can be identical to kernel one */ if (label->initrd && !strcmp(label->kernel_label, label->initrd)) { - initrd_addr = hextoul(kern_addr_str, NULL); + initrd_addr = kern_addr; } else if (label->initrd) { ulong size; int ret; @@ -785,11 +785,9 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) } if (!conf_fdt_str) { - ulong kernel_addr_r; void *buf; - kernel_addr_r = genimg_get_kernel_addr(kern_addr_str); - buf = map_sysmem(kernel_addr_r, 0); + buf = map_sysmem(kern_addr, 0); if (genimg_get_format(buf) != IMAGE_FORMAT_FIT) { if (!IS_ENABLED(CONFIG_SUPPORT_PASSING_ATAGS) || strcmp("-", label->fdt)) -- 2.43.0

From: Simon Glass <sjg@chromium.org> When this environment variable is missing, reserve space using lmb. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 3b487b2ed8e..0cfe8a4fe1b 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -691,18 +691,12 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r", SZ_2M, (enum bootflow_img_t)IH_TYPE_KERNEL, - &addr, NULL) < 0) { + &kern_addr, NULL) < 0) { printf("Skipping %s for failure retrieving kernel\n", label->name); return 1; } - kern_addr = env_get_hex("kernel_addr_r", 0); - if (!kern_addr) { - printf("No kernel_addr_r available for kernel\n"); - return 1; - } - /* for FIT, append the configuration identifier */ snprintf(fit_addr, sizeof(fit_addr), "%lx%s", kern_addr, label->config ? label->config : ""); -- 2.43.0

From: Simon Glass <sjg@chromium.org> This variable is a string, so add a _str suffix to make this clear. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 22 +++++++++++----------- include/pxe_utils.h | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 0cfe8a4fe1b..d6076efcbf7 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -540,7 +540,8 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, * * @ctx: PXE context * @label: Label to process - * @kernel_addr: String containing kernel address (cannot be NULL) + * @kern_addr_str: String containing kernel address and possible FIT + * configuration (cannot be NULL) * @initrd_addr: String containing initrd address (0 if none) * @initrd_size: initrd size (only used if @initrd_addr) * @initrd_str: initrd string to process (only used if @initrd_addr) @@ -550,7 +551,7 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, * returned, or -ve error value on error */ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, - char *kernel_addr, ulong initrd_addr, + char *kern_addr_str, ulong initrd_addr, ulong initrd_size, char *initrd_str, const char *conf_fdt_str, ulong conf_fdt) { @@ -562,9 +563,9 @@ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, bootm_init(&bmi); - bmi.addr_img = kernel_addr; + bmi.addr_img = kern_addr_str; bmi.conf_fdt = conf_fdt_str; - bootm_x86_set(&bmi, bzimage_addr, hextoul(kernel_addr, NULL)); + bootm_x86_set(&bmi, bzimage_addr, hextoul(kern_addr_str, NULL)); if (initrd_addr) { bmi.conf_ramdisk = initrd_str; @@ -572,7 +573,7 @@ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, bootm_x86_set(&bmi, initrd_size, initrd_size); } - kernel_addr_r = genimg_get_kernel_addr(kernel_addr); + kernel_addr_r = genimg_get_kernel_addr(kern_addr_str); buf = map_sysmem(kernel_addr_r, 0); /* @@ -662,7 +663,6 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) char fit_addr[200]; const char *conf_fdt_str; ulong conf_fdt = 0; - ulong addr; int ret; label_print(label); @@ -797,7 +797,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (IS_ENABLED(CONFIG_BOOTSTD_FULL) && ctx->no_boot) { ctx->label = label; - ctx->kernel_addr = strdup(kern_addr_str); + ctx->kern_addr_str = strdup(kern_addr_str); if (initrd_addr) { ctx->initrd_addr = initrd_addr; ctx->initrd_size = initrd_size; @@ -806,14 +806,14 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) ctx->conf_fdt_str = strdup(conf_fdt_str); ctx->conf_fdt = conf_fdt; log_debug("Saving label '%s':\n", label->name); - log_debug("- kernel_addr '%s' conf_fdt_str '%s' conf_fdt %lx\n", - ctx->kernel_addr, ctx->conf_fdt_str, conf_fdt); + log_debug("- kern_addr_str '%s' conf_fdt_str '%s' conf_fdt %lx\n", + ctx->kern_addr_str, ctx->conf_fdt_str, conf_fdt); if (initrd_addr) { log_debug("- initrd addr %lx filesize %lx str '%s'\n", ctx->initrd_addr, ctx->initrd_size, ctx->initrd_str); } - if (!ctx->kernel_addr || (conf_fdt_str && !ctx->conf_fdt_str) || + if (!ctx->kern_addr_str || (conf_fdt_str && !ctx->conf_fdt_str) || (initrd_addr && !ctx->initrd_str)) { printf("malloc fail (saving label)\n"); return 1; @@ -1116,7 +1116,7 @@ int pxe_do_boot(struct pxe_context *ctx) if (!ctx->label) return log_msg_ret("pxb", -ENOENT); - ret = label_run_boot(ctx, ctx->label, ctx->kernel_addr, + ret = label_run_boot(ctx, ctx->label, ctx->kern_addr_str, ctx->initrd_addr, ctx->initrd_size, ctx->initrd_str, ctx->conf_fdt_str, ctx->conf_fdt); if (ret) diff --git a/include/pxe_utils.h b/include/pxe_utils.h index a17af953d2a..523ec68adc7 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -125,7 +125,7 @@ typedef int (*pxe_getfile_func)(struct pxe_context *ctx, const char *file_path, * * The following are only used when probing for a label * @label: Label to process - * @kernel_addr: String containing kernel address (cannot be NULL) + * @kern_addr_str: String containing kernel address (cannot be NULL) * @initrd_addr: initaddr address (0 if none) * @initrd_size: initrd size (only used if @initrd_addr) * @initrd_str: initrd string to process (only used if @initrd_addr) @@ -159,7 +159,7 @@ struct pxe_context { /* information on the selected label to boot */ struct pxe_label *label; - char *kernel_addr; + char *kern_addr_str; ulong initrd_addr; ulong initrd_size; char *initrd_str; -- 2.43.0

From: Simon Glass <sjg@chromium.org> Converting back and forth between a string and a ulong does not make much sense. Simplify the code by adding a separate value for the kernel address. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 19 ++++++++++--------- include/pxe_utils.h | 2 ++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index d6076efcbf7..2970d1fa2ed 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -542,6 +542,7 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, * @label: Label to process * @kern_addr_str: String containing kernel address and possible FIT * configuration (cannot be NULL) + * @kern_addr: Kernel address (cannot be 0) * @initrd_addr: String containing initrd address (0 if none) * @initrd_size: initrd size (only used if @initrd_addr) * @initrd_str: initrd string to process (only used if @initrd_addr) @@ -551,12 +552,12 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, * returned, or -ve error value on error */ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, - char *kern_addr_str, ulong initrd_addr, - ulong initrd_size, char *initrd_str, - const char *conf_fdt_str, ulong conf_fdt) + char *kern_addr_str, ulong kern_addr, + ulong initrd_addr, ulong initrd_size, + char *initrd_str, const char *conf_fdt_str, + ulong conf_fdt) { struct bootm_info bmi; - ulong kernel_addr_r; int ret = 0; void *buf; enum image_fmt_t fmt; @@ -573,8 +574,7 @@ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, bootm_x86_set(&bmi, initrd_size, initrd_size); } - kernel_addr_r = genimg_get_kernel_addr(kern_addr_str); - buf = map_sysmem(kernel_addr_r, 0); + buf = map_sysmem(kern_addr, 0); /* * Try bootm for legacy and FIT format image, assume booti if @@ -798,6 +798,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (IS_ENABLED(CONFIG_BOOTSTD_FULL) && ctx->no_boot) { ctx->label = label; ctx->kern_addr_str = strdup(kern_addr_str); + ctx->kern_addr = kern_addr; if (initrd_addr) { ctx->initrd_addr = initrd_addr; ctx->initrd_size = initrd_size; @@ -821,8 +822,8 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) return 0; } - label_run_boot(ctx, label, kern_addr_str, initrd_addr, initrd_size, - initrd_str, conf_fdt_str, conf_fdt); + label_run_boot(ctx, label, kern_addr_str, kern_addr, initrd_addr, + initrd_size, initrd_str, conf_fdt_str, conf_fdt); /* ignore the error value since we are going to fail anyway */ return 1; /* returning is always failure */ @@ -1117,7 +1118,7 @@ int pxe_do_boot(struct pxe_context *ctx) return log_msg_ret("pxb", -ENOENT); ret = label_run_boot(ctx, ctx->label, ctx->kern_addr_str, - ctx->initrd_addr, ctx->initrd_size, + ctx->kern_addr, ctx->initrd_addr, ctx->initrd_size, ctx->initrd_str, ctx->conf_fdt_str, ctx->conf_fdt); if (ret) return log_msg_ret("lrb", ret); diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 523ec68adc7..1eb0230445b 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -126,6 +126,7 @@ typedef int (*pxe_getfile_func)(struct pxe_context *ctx, const char *file_path, * The following are only used when probing for a label * @label: Label to process * @kern_addr_str: String containing kernel address (cannot be NULL) + * @kern_addr: Kernel address (cannot be 0) * @initrd_addr: initaddr address (0 if none) * @initrd_size: initrd size (only used if @initrd_addr) * @initrd_str: initrd string to process (only used if @initrd_addr) @@ -160,6 +161,7 @@ struct pxe_context { /* information on the selected label to boot */ struct pxe_label *label; char *kern_addr_str; + ulong kern_addr; ulong initrd_addr; ulong initrd_size; char *initrd_str; -- 2.43.0

From: Simon Glass <sjg@chromium.org> At present this function only uses two fields, but it also needs to access the kernel-compression info. In preparation for that, pass in the whole struct. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/bootm.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/boot/bootm.c b/boot/bootm.c index 32232654b12..b7c7bf11f0a 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -334,11 +334,10 @@ static int found_booti_os(enum image_comp_t comp) /** * bootm_find_os(): Find the OS to boot * - * @cmd_name: Command name that started this boot, e.g. "bootm" - * @addr_fit: Address and/or FIT specifier (first arg of bootm command) + * @bmi: Bootm info (uses cmd_name, addr_img) * Return: 0 on success, -ve on error */ -static int bootm_find_os(const char *cmd_name, const char *addr_fit) +static int bootm_find_os(struct bootm_info *bmi) { const void *os_hdr; #ifdef CONFIG_ANDROID_BOOT_IMAGE @@ -349,7 +348,7 @@ static int bootm_find_os(const char *cmd_name, const char *addr_fit) int ret; /* get kernel image header, start address and length */ - ret = boot_get_kernel(addr_fit, &images, &images.os.image_start, + ret = boot_get_kernel(bmi->addr_img, &images, &images.os.image_start, &images.os.image_len, &os_hdr); if (ret) { /* no OS present, but that is OK */ @@ -358,7 +357,8 @@ static int bootm_find_os(const char *cmd_name, const char *addr_fit) return 0; } if (ret == -EPROTOTYPE) - printf("Wrong Image Type for %s command\n", cmd_name); + printf("Wrong Image Type for %s command\n", + bmi->cmd_name); printf("ERROR %dE: can't get kernel image!\n", ret); return 1; @@ -1076,7 +1076,7 @@ int bootm_run_states(struct bootm_info *bmi, int states) ret = bootm_pre_load(bmi->addr_img); if (!ret && (states & BOOTM_STATE_FINDOS)) - ret = bootm_find_os(bmi->cmd_name, bmi->addr_img); + ret = bootm_find_os(bmi); if (!ret && (states & BOOTM_STATE_FINDOTHER)) { ulong img_addr; -- 2.43.0
participants (1)
-
Simon Glass