From fd3fa5c3941d4de0736d066f77d0158cf933e207 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 14 Oct 2021 12:47:56 -0600 Subject: pxe: Use a context pointer At present the PXE functions pass around a pointer to command-table entry which is very strange. It is only needed in a few places and it is odd to pass around a data structure from another module in this way. For bootmethod we will need to provide some context information when reading files. Create a PXE context struct to hold the command-table-entry pointer and pass that around instead. We can then add more things to the context as needed. Signed-off-by: Simon Glass Reviewed-by: Artem Lapkin Tested-by: Artem Lapkin Reviewed-by: Ramon Fried --- cmd/pxe.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) (limited to 'cmd/pxe.c') diff --git a/cmd/pxe.c b/cmd/pxe.c index 46ac08fa3a0..17ce54fc049 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -43,7 +43,7 @@ static int do_get_tftp(struct cmd_tbl *cmdtp, const char *file_path, * * Returns 1 on success or < 0 on error. */ -static int pxe_uuid_path(struct cmd_tbl *cmdtp, unsigned long pxefile_addr_r) +static int pxe_uuid_path(struct pxe_context *ctx, unsigned long pxefile_addr_r) { char *uuid_str; @@ -52,7 +52,7 @@ static int pxe_uuid_path(struct cmd_tbl *cmdtp, unsigned long pxefile_addr_r) if (!uuid_str) return -ENOENT; - return get_pxelinux_path(cmdtp, uuid_str, pxefile_addr_r); + return get_pxelinux_path(ctx, uuid_str, pxefile_addr_r); } /* @@ -61,7 +61,7 @@ static int pxe_uuid_path(struct cmd_tbl *cmdtp, unsigned long pxefile_addr_r) * * Returns 1 on success or < 0 on error. */ -static int pxe_mac_path(struct cmd_tbl *cmdtp, unsigned long pxefile_addr_r) +static int pxe_mac_path(struct pxe_context *ctx, unsigned long pxefile_addr_r) { char mac_str[21]; int err; @@ -71,7 +71,7 @@ static int pxe_mac_path(struct cmd_tbl *cmdtp, unsigned long pxefile_addr_r) if (err < 0) return err; - return get_pxelinux_path(cmdtp, mac_str, pxefile_addr_r); + return get_pxelinux_path(ctx, mac_str, pxefile_addr_r); } /* @@ -81,7 +81,7 @@ static int pxe_mac_path(struct cmd_tbl *cmdtp, unsigned long pxefile_addr_r) * * Returns 1 on success or < 0 on error. */ -static int pxe_ipaddr_paths(struct cmd_tbl *cmdtp, unsigned long pxefile_addr_r) +static int pxe_ipaddr_paths(struct pxe_context *ctx, unsigned long pxefile_addr_r) { char ip_addr[9]; int mask_pos, err; @@ -89,7 +89,7 @@ static int pxe_ipaddr_paths(struct cmd_tbl *cmdtp, unsigned long pxefile_addr_r) sprintf(ip_addr, "%08X", ntohl(net_ip.s_addr)); for (mask_pos = 7; mask_pos >= 0; mask_pos--) { - err = get_pxelinux_path(cmdtp, ip_addr, pxefile_addr_r); + err = get_pxelinux_path(ctx, ip_addr, pxefile_addr_r); if (err > 0) return err; @@ -118,8 +118,10 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { char *pxefile_addr_str; unsigned long pxefile_addr_r; + struct pxe_context ctx; int err, i = 0; + pxe_setup_ctx(&ctx, cmdtp); do_getfile = do_get_tftp; if (argc != 1) @@ -139,16 +141,16 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) * Keep trying paths until we successfully get a file we're looking * for. */ - if (pxe_uuid_path(cmdtp, pxefile_addr_r) > 0 || - pxe_mac_path(cmdtp, pxefile_addr_r) > 0 || - pxe_ipaddr_paths(cmdtp, pxefile_addr_r) > 0) { + if (pxe_uuid_path(&ctx, pxefile_addr_r) > 0 || + pxe_mac_path(&ctx, pxefile_addr_r) > 0 || + pxe_ipaddr_paths(&ctx, pxefile_addr_r) > 0) { printf("Config file found\n"); return 0; } while (pxe_default_paths[i]) { - if (get_pxelinux_path(cmdtp, pxe_default_paths[i], + if (get_pxelinux_path(&ctx, pxe_default_paths[i], pxefile_addr_r) > 0) { printf("Config file found\n"); return 0; @@ -172,7 +174,9 @@ do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) unsigned long pxefile_addr_r; struct pxe_menu *cfg; char *pxefile_addr_str; + struct pxe_context ctx; + pxe_setup_ctx(&ctx, cmdtp); do_getfile = do_get_tftp; if (argc == 1) { @@ -191,14 +195,14 @@ do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 1; } - cfg = parse_pxefile(cmdtp, pxefile_addr_r); + cfg = parse_pxefile(&ctx, pxefile_addr_r); if (!cfg) { printf("Error parsing config file\n"); return 1; } - handle_pxe_menu(cmdtp, cfg); + handle_pxe_menu(&ctx, cfg); destroy_pxe_menu(cfg); -- cgit v1.2.3 From b1ead6b9087f1f96cb117d72e3e5cf0d5fb708f5 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 14 Oct 2021 12:47:57 -0600 Subject: pxe: Move do_getfile() into the context Rather than having a global variable, pass the function as part of the context. Signed-off-by: Simon Glass Reviewed-by: Artem Lapkin Tested-by: Artem Lapkin Reviewed-by: Ramon Fried --- cmd/pxe.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'cmd/pxe.c') diff --git a/cmd/pxe.c b/cmd/pxe.c index 17ce54fc049..70dbde3a636 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -24,7 +24,7 @@ const char *pxe_default_paths[] = { NULL }; -static int do_get_tftp(struct cmd_tbl *cmdtp, const char *file_path, +static int do_get_tftp(struct pxe_context *ctx, const char *file_path, char *file_addr) { char *tftp_argv[] = {"tftp", NULL, NULL, NULL}; @@ -32,7 +32,7 @@ static int do_get_tftp(struct cmd_tbl *cmdtp, const char *file_path, tftp_argv[1] = file_addr; tftp_argv[2] = (void *)file_path; - if (do_tftpb(cmdtp, 0, 3, tftp_argv)) + if (do_tftpb(ctx->cmdtp, 0, 3, tftp_argv)) return -ENOENT; return 1; @@ -121,8 +121,7 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) struct pxe_context ctx; int err, i = 0; - pxe_setup_ctx(&ctx, cmdtp); - do_getfile = do_get_tftp; + pxe_setup_ctx(&ctx, cmdtp, do_get_tftp); if (argc != 1) return CMD_RET_USAGE; @@ -176,8 +175,7 @@ do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) char *pxefile_addr_str; struct pxe_context ctx; - pxe_setup_ctx(&ctx, cmdtp); - do_getfile = do_get_tftp; + pxe_setup_ctx(&ctx, cmdtp, do_get_tftp); if (argc == 1) { pxefile_addr_str = from_env("pxefile_addr_r"); -- cgit v1.2.3 From 4ad5d51edb6525402b371cc8f8a3bee1b6a42414 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 14 Oct 2021 12:47:58 -0600 Subject: pxe: Add a userdata field to the context Allow the caller to provide some info which is passed back to the readfile() method. Signed-off-by: Simon Glass Reviewed-by: Artem Lapkin Tested-by: Artem Lapkin Reviewed-by: Ramon Fried --- cmd/pxe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'cmd/pxe.c') diff --git a/cmd/pxe.c b/cmd/pxe.c index 70dbde3a636..d79b9b733d7 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -121,7 +121,7 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) struct pxe_context ctx; int err, i = 0; - pxe_setup_ctx(&ctx, cmdtp, do_get_tftp); + pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL); if (argc != 1) return CMD_RET_USAGE; @@ -175,7 +175,7 @@ do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) char *pxefile_addr_str; struct pxe_context ctx; - pxe_setup_ctx(&ctx, cmdtp, do_get_tftp); + pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL); if (argc == 1) { pxefile_addr_str = from_env("pxefile_addr_r"); -- cgit v1.2.3 From 8018b9af57b5cd0cfddf48a8d12f04dba8b77a65 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 14 Oct 2021 12:47:59 -0600 Subject: pxe: Tidy up the is_pxe global Move this into the context to avoid a global variable. Also rename it since the current name does not explain what it actually affects. Signed-off-by: Simon Glass Reviewed-by: Artem Lapkin Tested-by: Artem Lapkin Reviewed-by: Ramon Fried --- cmd/pxe.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'cmd/pxe.c') diff --git a/cmd/pxe.c b/cmd/pxe.c index d79b9b733d7..17fe364bed9 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -121,7 +121,7 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) struct pxe_context ctx; int err, i = 0; - pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL); + pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false); if (argc != 1) return CMD_RET_USAGE; @@ -175,7 +175,7 @@ do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) char *pxefile_addr_str; struct pxe_context ctx; - pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL); + pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false); if (argc == 1) { pxefile_addr_str = from_env("pxefile_addr_r"); @@ -235,8 +235,6 @@ static int do_pxe(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (argc < 2) return CMD_RET_USAGE; - is_pxe = true; - /* drop initial "pxe" arg */ argc--; argv++; -- cgit v1.2.3 From 9e62e7ca543ea94a46f30053262f67202e2435f4 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 14 Oct 2021 12:48:03 -0600 Subject: pxe: Move common parsing coding into pxe_util Both the syslinux and pxe commands use essentially the same code to parse and run extlinux.conf files. Move this into a common function. Signed-off-by: Simon Glass Reviewed-by: Artem Lapkin Tested-by: Artem Lapkin Reviewed-by: Ramon Fried --- cmd/pxe.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) (limited to 'cmd/pxe.c') diff --git a/cmd/pxe.c b/cmd/pxe.c index 17fe364bed9..4fa51d2e053 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -171,9 +171,9 @@ static int do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { unsigned long pxefile_addr_r; - struct pxe_menu *cfg; char *pxefile_addr_str; struct pxe_context ctx; + int ret; pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false); @@ -193,16 +193,9 @@ do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 1; } - cfg = parse_pxefile(&ctx, pxefile_addr_r); - - if (!cfg) { - printf("Error parsing config file\n"); - return 1; - } - - handle_pxe_menu(&ctx, cfg); - - destroy_pxe_menu(cfg); + ret = pxe_process(&ctx, pxefile_addr_r, false); + if (ret) + return CMD_RET_FAILURE; copy_filename(net_boot_file_name, "", sizeof(net_boot_file_name)); -- cgit v1.2.3 From 12df842ee324a7e188a643bfee6fe08f28127b26 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 14 Oct 2021 12:48:04 -0600 Subject: pxe: Clean up the use of bootfile The 'bootfile' environment variable is read in the bowels of pxe_util to provide a directory to which all loaded files are relative. This is not obvious from the API to PXE and it is strange to make the caller set an environment variable rather than pass this as a parameter. The code is also convoluted, which this feature implemented by get_bootfile_path(). Update the API to improve this. Unfortunately this means that pxe_setup_ctx() can fail, so add error checking. Signed-off-by: Simon Glass Reviewed-by: Artem Lapkin Tested-by: Artem Lapkin Reviewed-by: Ramon Fried --- cmd/pxe.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'cmd/pxe.c') diff --git a/cmd/pxe.c b/cmd/pxe.c index 4fa51d2e053..e319db51ef5 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -121,8 +121,6 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) struct pxe_context ctx; int err, i = 0; - pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false); - if (argc != 1) return CMD_RET_USAGE; @@ -136,6 +134,11 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (err < 0) return 1; + if (pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false, + env_get("bootfile"))) { + printf("Out of memory\n"); + return CMD_RET_FAILURE; + } /* * Keep trying paths until we successfully get a file we're looking * for. @@ -144,6 +147,7 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) pxe_mac_path(&ctx, pxefile_addr_r) > 0 || pxe_ipaddr_paths(&ctx, pxefile_addr_r) > 0) { printf("Config file found\n"); + pxe_destroy_ctx(&ctx); return 0; } @@ -152,12 +156,14 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (get_pxelinux_path(&ctx, pxe_default_paths[i], pxefile_addr_r) > 0) { printf("Config file found\n"); + pxe_destroy_ctx(&ctx); return 0; } i++; } printf("Config file not found\n"); + pxe_destroy_ctx(&ctx); return 1; } @@ -175,8 +181,6 @@ do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) struct pxe_context ctx; int ret; - pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false); - if (argc == 1) { pxefile_addr_str = from_env("pxefile_addr_r"); if (!pxefile_addr_str) @@ -193,7 +197,13 @@ do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 1; } + if (pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false, + env_get("bootfile"))) { + printf("Out of memory\n"); + return CMD_RET_FAILURE; + } ret = pxe_process(&ctx, pxefile_addr_r, false); + pxe_destroy_ctx(&ctx); if (ret) return CMD_RET_FAILURE; -- cgit v1.2.3 From 4d79e884adf6842beb94566bf5249c07a84a5b51 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 14 Oct 2021 12:48:08 -0600 Subject: pxe: Return the file size from the getfile() function It is pretty strange that the pxe code uses the 'filesize' environment variable find the size of a file it has just read. Partly this is because it uses the command-line interpreter to parse its request to load the file. As a first step towards unwinding this, return it directly from the getfile() function. This makes the code a bit longer, for now, but will be cleaned up in future patches. Signed-off-by: Simon Glass Reviewed-by: Artem Lapkin Tested-by: Artem Lapkin Reviewed-by: Ramon Fried --- cmd/pxe.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'cmd/pxe.c') diff --git a/cmd/pxe.c b/cmd/pxe.c index e319db51ef5..81703386c42 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -25,15 +25,20 @@ const char *pxe_default_paths[] = { }; static int do_get_tftp(struct pxe_context *ctx, const char *file_path, - char *file_addr) + char *file_addr, ulong *sizep) { char *tftp_argv[] = {"tftp", NULL, NULL, NULL}; + int ret; tftp_argv[1] = file_addr; tftp_argv[2] = (void *)file_path; if (do_tftpb(ctx->cmdtp, 0, 3, tftp_argv)) return -ENOENT; + ret = pxe_get_file_size(sizep); + if (ret) + return log_msg_ret("tftp", ret); + ctx->pxe_file_size = *sizep; return 1; } -- cgit v1.2.3 From d50244e9d8583378770392fb429a0283b2b47885 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 14 Oct 2021 12:48:11 -0600 Subject: pxe: Allow calling the pxe_get logic directly Refactor this code so that we can call the 'pxe get' command without going through the command-line interpreter. This makes it easier to get the information we need, without going through environment variables. Signed-off-by: Simon Glass Reviewed-by: Artem Lapkin Tested-by: Artem Lapkin Reviewed-by: Ramon Fried --- cmd/pxe.c | 92 ++++++++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 59 insertions(+), 33 deletions(-) (limited to 'cmd/pxe.c') diff --git a/cmd/pxe.c b/cmd/pxe.c index 81703386c42..db8e4697f24 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -104,6 +104,49 @@ static int pxe_ipaddr_paths(struct pxe_context *ctx, unsigned long pxefile_addr_ return -ENOENT; } + +int pxe_get(ulong pxefile_addr_r, char **bootdirp, ulong *sizep) +{ + struct cmd_tbl cmdtp[] = {}; /* dummy */ + struct pxe_context ctx; + int i; + + if (pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false, + env_get("bootfile"))) + return -ENOMEM; + /* + * Keep trying paths until we successfully get a file we're looking + * for. + */ + if (pxe_uuid_path(&ctx, pxefile_addr_r) > 0 || + pxe_mac_path(&ctx, pxefile_addr_r) > 0 || + pxe_ipaddr_paths(&ctx, pxefile_addr_r) > 0) + goto done; + + i = 0; + while (pxe_default_paths[i]) { + if (get_pxelinux_path(&ctx, pxe_default_paths[i], + pxefile_addr_r) > 0) + goto done; + i++; + } + + pxe_destroy_ctx(&ctx); + + return -ENOENT; +done: + *bootdirp = env_get("bootfile"); + + /* + * The PXE file size is returned but not the name. It is probably not + * that useful. + */ + *sizep = ctx.pxe_file_size; + pxe_destroy_ctx(&ctx); + + return 0; +} + /* * Entry point for the 'pxe get' command. * This Follows pxelinux's rules to download a config file from a tftp server. @@ -122,9 +165,10 @@ static int do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { char *pxefile_addr_str; - unsigned long pxefile_addr_r; - struct pxe_context ctx; - int err, i = 0; + ulong pxefile_addr_r; + char *fname; + ulong size; + int ret; if (argc != 1) return CMD_RET_USAGE; @@ -134,43 +178,25 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (!pxefile_addr_str) return 1; - err = strict_strtoul(pxefile_addr_str, 16, + ret = strict_strtoul(pxefile_addr_str, 16, (unsigned long *)&pxefile_addr_r); - if (err < 0) + if (ret < 0) return 1; - if (pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false, - env_get("bootfile"))) { + ret = pxe_get(pxefile_addr_r, &fname, &size); + switch (ret) { + case 0: + printf("Config file '%s' found\n", fname); + break; + case -ENOMEM: printf("Out of memory\n"); return CMD_RET_FAILURE; + default: + printf("Config file not found\n"); + return CMD_RET_FAILURE; } - /* - * Keep trying paths until we successfully get a file we're looking - * for. - */ - if (pxe_uuid_path(&ctx, pxefile_addr_r) > 0 || - pxe_mac_path(&ctx, pxefile_addr_r) > 0 || - pxe_ipaddr_paths(&ctx, pxefile_addr_r) > 0) { - printf("Config file found\n"); - pxe_destroy_ctx(&ctx); - - return 0; - } - - while (pxe_default_paths[i]) { - if (get_pxelinux_path(&ctx, pxe_default_paths[i], - pxefile_addr_r) > 0) { - printf("Config file found\n"); - pxe_destroy_ctx(&ctx); - return 0; - } - i++; - } - - printf("Config file not found\n"); - pxe_destroy_ctx(&ctx); - return 1; + return 0; } /* -- cgit v1.2.3