[PATCH v3 0/4] bootflow: bootmeth_efi: Fix network efi boot.
Currently bootmeth_efi crashes while doing a network (dhcp) boot. This patch series fixes issues and both network and disk boot works.
Shantur Rathore (4): bootflow: bootmeth_efi: Set bootp_arch as hex bootflow: bootmeth_efi: set bflow->fname from bootfile name bootflow: bootmeth_efi: Handle fdt not available. bootflow: bootmeth_efi: don't free buffer
boot/bootmeth_efi.c | 36 +++++++++++++++++++++++++++--------- include/bootflow.h | 2 ++ 2 files changed, 29 insertions(+), 9 deletions(-)
bootmeth_efi sets up bootp_arch which is read later in bootp.c Currently bootp_arch is being set as integer string and being read in bootp.c as hex, this sends incorrect arch value to dhcp server which in return sends wrong file for network boot.
For ARM64 UEFI Arch value is 0xb (11), here we set environment as 11 and later is read as 0x11 and 17 is sent to dhcp server.
Setting it as hex string fixes the problem.
Signed-off-by: Shantur Rathore i@shantur.com --- boot/bootmeth_efi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 9ba7734911..682cf5b23b 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -339,7 +339,7 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow) ret = env_set("bootp_vci", str); if (ret) return log_msg_ret("vcs", ret); - ret = env_set_ulong("bootp_arch", arch); + ret = env_set_hex("bootp_arch", arch); if (ret) return log_msg_ret("ars", ret);
Hi Shantur,
On Sat, 18 Nov 2023 at 15:04, Shantur Rathore i@shantur.com wrote:
bootmeth_efi sets up bootp_arch which is read later in bootp.c Currently bootp_arch is being set as integer string and being read in bootp.c as hex, this sends incorrect arch value to dhcp server which in return sends wrong file for network boot.
For ARM64 UEFI Arch value is 0xb (11), here we set environment as 11 and later is read as 0x11 and 17 is sent to dhcp server.
Setting it as hex string fixes the problem.
Signed-off-by: Shantur Rathore i@shantur.com
boot/bootmeth_efi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
I think you dropped my review tag on this version.
You can use 'patman status' to collect review tags automatically.
Regards, Simon
We need to set boot->fname before calling efi_set_bootdev otherwise this crashes as bflow->fname is null.
Signed-off-by: Shantur Rathore i@shantur.com --- boot/bootmeth_efi.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 682cf5b23b..183daad8f1 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -323,7 +323,7 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow) char file_addr[17], fname[256]; char *tftp_argv[] = {"tftp", file_addr, fname, NULL}; struct cmd_tbl cmdtp = {}; /* dummy */ - const char *addr_str, *fdt_addr_str; + const char *addr_str, *fdt_addr_str, *bootfile_name; int ret, arch, size; ulong addr, fdt_addr; char str[36]; @@ -360,6 +360,12 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow) return log_msg_ret("sz", -EINVAL); bflow->size = size;
+ /* bootfile should be setup by dhcp*/ + bootfile_name = env_get("bootfile"); + if (!bootfile_name) + return log_msg_ret("bootfile_name", ret); + bflow->fname = strdup(bootfile_name); + /* do the hideous EFI hack */ efi_set_bootdev("Net", "", bflow->fname, map_sysmem(addr, 0), bflow->size);
While booting with efi, if fdt isn't available externally, just use the built-in one.
Signed-off-by: Shantur Rathore i@shantur.com --- boot/bootmeth_efi.c | 19 +++++++++++++------ include/bootflow.h | 2 ++ 2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 183daad8f1..370d6fba9f 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -313,6 +313,7 @@ static int distro_efi_try_bootflow_files(struct udevice *dev, */ } else { log_debug("No device tree available\n"); + bflow->flags |= BOOTFLOWF_USE_BUILTIN_FDT; }
return 0; @@ -391,6 +392,7 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow) bflow->fdt_addr = fdt_addr; } else { log_debug("No device tree available\n"); + bflow->flags |= BOOTFLOWF_USE_BUILTIN_FDT; }
bflow->state = BOOTFLOWST_READY; @@ -429,13 +431,11 @@ static int distro_efi_boot(struct udevice *dev, struct bootflow *bflow) return log_msg_ret("read", ret);
/* - * use the provided device tree if available, else fall back to - * the control FDT + * use the provided device tree if not using the built-in fdt */ - if (bflow->fdt_fname) + if (bflow->flags & ~BOOTFLOWF_USE_BUILTIN_FDT) fdt = bflow->fdt_addr; - else - fdt = (ulong)map_to_sysmem(gd->fdt_blob); + } else { /* * This doesn't actually work for network devices: @@ -452,7 +452,14 @@ static int distro_efi_boot(struct udevice *dev, struct bootflow *bflow) * At some point we can add a real interface to bootefi so we can call * this directly. For now, go through the CLI, like distro boot. */ - snprintf(cmd, sizeof(cmd), "bootefi %lx %lx", kernel, fdt); + if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) { + log_debug("Booting with built-in fdt\n"); + snprintf(cmd, sizeof(cmd), "bootefi %lx", kernel); + } else { + log_debug("Booting with external fdt\n"); + snprintf(cmd, sizeof(cmd), "bootefi %lx %lx", kernel, fdt); + } + if (run_command(cmd, 0)) return log_msg_ret("run", -EINVAL);
diff --git a/include/bootflow.h b/include/bootflow.h index fede8f22a2..42112874f6 100644 --- a/include/bootflow.h +++ b/include/bootflow.h @@ -45,10 +45,12 @@ enum bootflow_state_t { * CONFIG_OF_HAS_PRIOR_STAGE is enabled * @BOOTFLOWF_STATIC_BUF: Indicates that @bflow->buf is statically set, rather * than being allocated by malloc(). + * @BOOTFLOWF_USE_BUILTIN_FDT : Indicates that current bootflow uses built-in FDT */ enum bootflow_flags_t { BOOTFLOWF_USE_PRIOR_FDT = 1 << 0, BOOTFLOWF_STATIC_BUF = 1 << 1, + BOOTFLOWF_USE_BUILTIN_FDT = 1 << 2, };
/**
On Sat, 18 Nov 2023 at 15:07, Shantur Rathore i@shantur.com wrote:
While booting with efi, if fdt isn't available externally, just use the built-in one.
Signed-off-by: Shantur Rathore i@shantur.com
Change log needed when you send a new version
boot/bootmeth_efi.c | 19 +++++++++++++------ include/bootflow.h | 2 ++ 2 files changed, 15 insertions(+), 6 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
bootmeth_efi doesn't allocate any buffer to load efi in any case. enable static buffer flag for all cases.
Signed-off-by: Shantur Rathore i@shantur.com --- boot/bootmeth_efi.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 370d6fba9f..93f096896c 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -160,7 +160,6 @@ static int efiload_read_file(struct bootflow *bflow, ulong addr) if (ret) return log_msg_ret("read", ret); bflow->buf = map_sysmem(addr, bflow->size); - bflow->flags |= BOOTFLOWF_STATIC_BUF;
set_efi_bootdev(desc, bflow);
@@ -404,6 +403,12 @@ static int distro_efi_read_bootflow(struct udevice *dev, struct bootflow *bflow) { int ret;
+ /* + * bootmeth_efi doesn't allocate any buffer neither for bulk nor net device + * set flag to avoid freeing static buffer. + */ + bflow->flags |= BOOTFLOWF_STATIC_BUF; + if (bootmeth_uses_network(bflow)) { /* we only support reading from one device, so ignore 'dev' */ ret = distro_efi_read_bootflow_net(bflow);
Hi Shantur,
On Sat, 18 Nov 2023 at 15:07, Shantur Rathore i@shantur.com wrote:
bootmeth_efi doesn't allocate any buffer to load efi in any case. enable static buffer flag for all cases.
Signed-off-by: Shantur Rathore i@shantur.com
boot/bootmeth_efi.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 370d6fba9f..93f096896c 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -160,7 +160,6 @@ static int efiload_read_file(struct bootflow *bflow, ulong addr) if (ret) return log_msg_ret("read", ret); bflow->buf = map_sysmem(addr, bflow->size);
bflow->flags |= BOOTFLOWF_STATIC_BUF; set_efi_bootdev(desc, bflow);@@ -404,6 +403,12 @@ static int distro_efi_read_bootflow(struct udevice *dev, struct bootflow *bflow) { int ret;
/** bootmeth_efi doesn't allocate any buffer neither for bulk nor net device
s/bulk/blk/ ?
* set flag to avoid freeing static buffer.*/bflow->flags |= BOOTFLOWF_STATIC_BUF;if (bootmeth_uses_network(bflow)) { /* we only support reading from one device, so ignore 'dev' */ ret = distro_efi_read_bootflow_net(bflow);-- 2.40.1
Regards, Simon
On Sat, 18 Nov 2023 at 15:07, Shantur Rathore i@shantur.com wrote:
We need to set boot->fname before calling efi_set_bootdev otherwise this crashes as bflow->fname is null.
Signed-off-by: Shantur Rathore i@shantur.com
boot/bootmeth_efi.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
We definitely need to figure out a way to get this code. The boostd tests don't go much into the bothmeths unfortunately
- Simon
participants (2)
-
Shantur Rathore -
Simon Glass