Hi Tom,
On 12/13/22 14:37, Tom Rini wrote:
This reverts commits 51c5c28af59c ("cmd: pxe: use strdup to copy config"), a5dacef7380e ("cmd: pxe: support INITRD and FDT selection with FIT") and f723c2778cf8 ("cmd: pxe: reorder kernel treatment in label_boot").
As reported by Quentin Schulz, this introduces a regression with previously generated and working extlinux.conf files.
For reference: https://lore.kernel.org/u-boot/b7e891d1-d134-b489-eb2d-6125d4c7b6c6@theobrom... for my worry https://lore.kernel.org/u-boot/f0dd213c-4a34-926d-3f3b-f2ed49bb92c3@linaro.o... for confirmation from Neil it is breaking existing extlinux.conf
Thanks, Quentin
Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Patrick Delaunay patrick.delaunay@foss.st.com Reported-by: Quentin Schulz quentin.schulz@theobroma-systems.com Signed-off-by: Tom Rini trini@konsulko.com
boot/pxe_utils.c | 69 ++++++++++++++++++++------------------------- doc/README.pxe | 8 ------ include/pxe_utils.h | 2 -- 3 files changed, 30 insertions(+), 49 deletions(-)
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 272431381763..075a0f28309e 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -258,7 +258,6 @@ static struct pxe_label *label_create(void) static void label_destroy(struct pxe_label *label) { free(label->name);
- free(label->kernel_label); free(label->kernel); free(label->config); free(label->append);
@@ -522,44 +521,28 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) return 1; }
- if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r",
NULL) < 0) {printf("Skipping %s for failure retrieving kernel\n",label->name);return 1;- }
- kernel_addr = env_get("kernel_addr_r");
- /* 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);kernel_addr = fit_addr;- }
- /* For FIT, the label can be identical to kernel one */
- if (label->initrd && !strcmp(label->kernel_label, label->initrd)) {
initrd_addr_str = kernel_addr;- } else if (label->initrd) {
- if (label->initrd) { ulong size;
- if (get_relfile_envaddr(ctx, label->initrd, "ramdisk_addr_r", &size) < 0) { printf("Skipping %s for failure retrieving initrd\n", label->name);
goto cleanup;
return 1;}
initrd_addr_str = env_get("ramdisk_addr_r"); size = snprintf(initrd_str, sizeof(initrd_str), "%s:%lx", initrd_addr_str, size); if (size >= sizeof(initrd_str))
goto cleanup;
return 1;}
if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r",
NULL) < 0) {printf("Skipping %s for failure retrieving kernel\n",label->name);return 1;}
if (label->ipappend & 0x1) {
@@ -589,7 +572,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)
@@ -604,6 +587,21 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) printf("append: %s\n", finalbootargs); }
- kernel_addr = env_get("kernel_addr_r");
- /* 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);kernel_addr = fit_addr;- }
- /*
- fdt usage is optional:
- It handles the following scenarios.
@@ -625,11 +623,8 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) */ bootm_argv[3] = env_get("fdt_addr_r");
- /* For FIT, the label can be identical to kernel one */
- if (label->fdt && !strcmp(label->kernel_label, label->fdt)) {
bootm_argv[3] = kernel_addr;- } else if (bootm_argv[3]) {
- if (bootm_argv[3]) { char *fdtfile = NULL; char *fdtfilefree = NULL;
@@ -1170,19 +1165,15 @@ static int parse_label_kernel(char **c, struct pxe_label *label) if (err < 0) return err;
/* copy the kernel label to compare with FDT / INITRD when FIT is used */
label->kernel_label = strdup(label->kernel);
if (!label->kernel_label)
return -ENOMEM;s = strstr(label->kernel, "#"); if (!s) return 1;
label->config = strdup(s);
label->config = malloc(strlen(s) + 1); if (!label->config) return -ENOMEM;
strcpy(label->config, s); *s = 0;
return 1;
diff --git a/doc/README.pxe b/doc/README.pxe index 172201093d02..d14d2bdcc9b0 100644 --- a/doc/README.pxe +++ b/doc/README.pxe @@ -179,19 +179,11 @@ initrd <path> - if this label is chosen, use tftp to retrieve the initrd at <path>. it will be stored at the address indicated in the initrd_addr_r environment variable, and that address will be passed to bootm.
For FIT image, the initrd can be provided with the same value thankernel, including configuration:<path>#<conf>[#<extra-conf[#...]]In this case, kernel_addr_r is passed to bootm.fdt <path> - if this label is chosen, use tftp to retrieve the fdt blob at <path>. it will be stored at the address indicated in the fdt_addr_r environment variable, and that address will be passed to bootm.
For FIT image, the device tree can be provided with the same valuethan kernel, including configuration:<path>#<conf>[#<extra-conf[#...]]In this case, kernel_addr_r is passed to bootm.devicetree <path> - if this label is chosen, use tftp to retrieve the fdt blob at <path>. it will be stored at the address indicated in
diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 1e5e8424f53f..4a73b2aace34 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -28,7 +28,6 @@
- Create these with the 'label_create' function given below.
- name - the name of the menu as given on the 'menu label' line.
- kernel_label - the kernel label, including FIT config if present.
- kernel - the path to the kernel file to use for this label.
- append - kernel command line to use when booting this label
- initrd - path to the initrd to use for this label.
@@ -41,7 +40,6 @@ struct pxe_label { char num[4]; char *name; char *menu;
- char *kernel_label; char *kernel; char *config; char *append;