[PATCH v2] cmd: pxe_utils: Limit fdtcontroladdr usage to non-fitImage
Commit d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") forces '$fdtcontroladdr' DT address as a third parameter of bootm command even if the PXE transfer pulls in a fitImage which contains configuration node with its own DT that is preferrable to be passed to Linux. Limit the $fdtcontroladdr fallback utilization to non-fitImages, since it is highly likely a fitImage would come with its own DT, while single-file images do need a separate DT.
Fixes: d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") Signed-off-by: Marek Vasut marex@denx.de --- Cc: Peter Hoyes Peter.Hoyes@arm.com Cc: Ramon Fried rfried.dev@gmail.com Cc: Simon Glass sjg@chromium.org --- V1: Map the kernel buffer before testing image type V2: Update code comment to reflect the change --- boot/pxe_utils.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 8133006875f..099aa2f4bc7 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -617,7 +617,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) * bootm, and adjust argc appropriately. * * Scenario 3: If there is an fdtcontroladdr specified, pass it along to - * bootm, and adjust argc appropriately. + * bootm, and adjust argc appropriately, unless the image type is fitImage. * * Scenario 4: fdt blob is not available. */ @@ -724,7 +724,10 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (!bootm_argv[3]) bootm_argv[3] = env_get("fdt_addr");
- if (!bootm_argv[3]) + kernel_addr_r = genimg_get_kernel_addr(kernel_addr); + buf = map_sysmem(kernel_addr_r, 0); + + if (!bootm_argv[3] && genimg_get_format(buf) != IMAGE_FORMAT_FIT) bootm_argv[3] = env_get("fdtcontroladdr");
if (bootm_argv[3]) { @@ -733,8 +736,6 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) bootm_argc = 4; }
- kernel_addr_r = genimg_get_kernel_addr(kernel_addr); - buf = map_sysmem(kernel_addr_r, 0); /* Try bootm for legacy and FIT format image */ if (genimg_get_format(buf) != IMAGE_FORMAT_INVALID && IS_ENABLED(CONFIG_CMD_BOOTM))
Hi Marek,
On 12/14/22 07:45, Marek Vasut wrote:
Commit d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") forces '$fdtcontroladdr' DT address as a third parameter of bootm command even if the PXE transfer pulls in a fitImage which contains configuration node with its own DT that is preferrable to be passed to Linux. Limit the $fdtcontroladdr fallback utilization to non-fitImages, since it is highly likely a fitImage would come with its own DT, while single-file images do need a separate DT.
Reviewed-by: Quentin Schulz quentin.schulz@theobroma-systems.com Tested-by: Quentin Schulz quentin.schulz@theobroma-systems.com
Tested on top of an almost vanilla v2022.10 on Ringneck PX30, the FDT from the selected fitimage configuration (via #conf in kernel field in extlinux.conf) is taken into account.
Not sure overriding the DT gotten from the fit image wasn't a use-case Peter wanted to support though.
Cheers, Quentin
Fixes: d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") Signed-off-by: Marek Vasut marex@denx.de
Cc: Peter Hoyes Peter.Hoyes@arm.com Cc: Ramon Fried rfried.dev@gmail.com Cc: Simon Glass sjg@chromium.org
V1: Map the kernel buffer before testing image type V2: Update code comment to reflect the change
boot/pxe_utils.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 8133006875f..099aa2f4bc7 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -617,7 +617,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) * bootm, and adjust argc appropriately. * * Scenario 3: If there is an fdtcontroladdr specified, pass it along to
* bootm, and adjust argc appropriately.
* bootm, and adjust argc appropriately, unless the image type is fitImage.*/
- Scenario 4: fdt blob is not available.
@@ -724,7 +724,10 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (!bootm_argv[3]) bootm_argv[3] = env_get("fdt_addr");
- if (!bootm_argv[3])
kernel_addr_r = genimg_get_kernel_addr(kernel_addr);
buf = map_sysmem(kernel_addr_r, 0);
if (!bootm_argv[3] && genimg_get_format(buf) != IMAGE_FORMAT_FIT) bootm_argv[3] = env_get("fdtcontroladdr");
if (bootm_argv[3]) {
@@ -733,8 +736,6 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) bootm_argc = 4; }
- kernel_addr_r = genimg_get_kernel_addr(kernel_addr);
- buf = map_sysmem(kernel_addr_r, 0); /* Try bootm for legacy and FIT format image */ if (genimg_get_format(buf) != IMAGE_FORMAT_INVALID && IS_ENABLED(CONFIG_CMD_BOOTM))
On 12/14/22 16:23, Quentin Schulz wrote:
Hi Marek,
Hi,
On 12/14/22 07:45, Marek Vasut wrote:
Commit d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") forces '$fdtcontroladdr' DT address as a third parameter of bootm command even if the PXE transfer pulls in a fitImage which contains configuration node with its own DT that is preferrable to be passed to Linux. Limit the $fdtcontroladdr fallback utilization to non-fitImages, since it is highly likely a fitImage would come with its own DT, while single-file images do need a separate DT.
Reviewed-by: Quentin Schulz quentin.schulz@theobroma-systems.com Tested-by: Quentin Schulz quentin.schulz@theobroma-systems.com
Tested on top of an almost vanilla v2022.10 on Ringneck PX30, the FDT from the selected fitimage configuration (via #conf in kernel field in extlinux.conf) is taken into account.
Not sure overriding the DT gotten from the fit image wasn't a use-case Peter wanted to support though.
I am hoping to get feedback from Peter, but that kind of behavior would be rather odd. If user wants to use fdtcontroladdr DT, then just don't add DT fdt property into the configuration node entry in the fitImage.
On 14/12/2022 15:25, Marek Vasut wrote:
On 12/14/22 16:23, Quentin Schulz wrote:
Hi Marek,
Hi,
On 12/14/22 07:45, Marek Vasut wrote:
Commit d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") forces '$fdtcontroladdr' DT address as a third parameter of bootm command even if the PXE transfer pulls in a fitImage which contains configuration node with its own DT that is preferrable to be passed to Linux. Limit the $fdtcontroladdr fallback utilization to non-fitImages, since it is highly likely a fitImage would come with its own DT, while single-file images do need a separate DT.
Reviewed-by: Quentin Schulz quentin.schulz@theobroma-systems.com Tested-by: Quentin Schulz quentin.schulz@theobroma-systems.com
Tested on top of an almost vanilla v2022.10 on Ringneck PX30, the FDT from the selected fitimage configuration (via #conf in kernel field in extlinux.conf) is taken into account.
Not sure overriding the DT gotten from the fit image wasn't a use-case Peter wanted to support though.
I am hoping to get feedback from Peter, but that kind of behavior would be rather odd. If user wants to use fdtcontroladdr DT, then just don't add DT fdt property into the configuration node entry in the fitImage.
Reviewed-by: Peter Hoyes peter.hoyes@arm.com Tested-by: Peter Hoyes peter.hoyes@arm.com
Tested with an fdtcontroladdr provided by a prior boot stage (TF-A) on an Arm FVP.
The implemented behavior works for me. We do not have any need to override the DT from a FIT image.
Thanks,
Peter
On Tue, 13 Dec 2022 at 22:45, Marek Vasut marex@denx.de wrote:
Commit d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") forces '$fdtcontroladdr' DT address as a third parameter of bootm command even if the PXE transfer pulls in a fitImage which contains configuration node with its own DT that is preferrable to be passed to Linux. Limit the $fdtcontroladdr fallback utilization to non-fitImages, since it is highly likely a fitImage would come with its own DT, while single-file images do need a separate DT.
Fixes: d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") Signed-off-by: Marek Vasut marex@denx.de
Cc: Peter Hoyes Peter.Hoyes@arm.com Cc: Ramon Fried rfried.dev@gmail.com Cc: Simon Glass sjg@chromium.org
V1: Map the kernel buffer before testing image type V2: Update code comment to reflect the change
boot/pxe_utils.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 8133006875f..099aa2f4bc7 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -617,7 +617,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) * bootm, and adjust argc appropriately. * * Scenario 3: If there is an fdtcontroladdr specified, pass it along to
* bootm, and adjust argc appropriately.
* bootm, and adjust argc appropriately, unless the image type is fitImage. * * Scenario 4: fdt blob is not available. */@@ -724,7 +724,10 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (!bootm_argv[3]) bootm_argv[3] = env_get("fdt_addr");
if (!bootm_argv[3])
kernel_addr_r = genimg_get_kernel_addr(kernel_addr);buf = map_sysmem(kernel_addr_r, 0);if (!bootm_argv[3] && genimg_get_format(buf) != IMAGE_FORMAT_FIT) bootm_argv[3] = env_get("fdtcontroladdr"); if (bootm_argv[3]) {@@ -733,8 +736,6 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) bootm_argc = 4; }
kernel_addr_r = genimg_get_kernel_addr(kernel_addr);buf = map_sysmem(kernel_addr_r, 0); /* Try bootm for legacy and FIT format image */ if (genimg_get_format(buf) != IMAGE_FORMAT_INVALID && IS_ENABLED(CONFIG_CMD_BOOTM))-- 2.35.1
Reviewed-by: Simon Glass sjg@chromium.org
On 14/12/2022 07:45, Marek Vasut wrote:
Commit d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") forces '$fdtcontroladdr' DT address as a third parameter of bootm command even if the PXE transfer pulls in a fitImage which contains configuration node with its own DT that is preferrable to be passed to Linux. Limit the $fdtcontroladdr fallback utilization to non-fitImages, since it is highly likely a fitImage would come with its own DT, while single-file images do need a separate DT.
Fixes: d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") Signed-off-by: Marek Vasut marex@denx.de
Cc: Peter Hoyes Peter.Hoyes@arm.com Cc: Ramon Fried rfried.dev@gmail.com Cc: Simon Glass sjg@chromium.org
V1: Map the kernel buffer before testing image type V2: Update code comment to reflect the change
boot/pxe_utils.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 8133006875f..099aa2f4bc7 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -617,7 +617,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) * bootm, and adjust argc appropriately. * * Scenario 3: If there is an fdtcontroladdr specified, pass it along to
* bootm, and adjust argc appropriately.
* bootm, and adjust argc appropriately, unless the image type is fitImage.*/
- Scenario 4: fdt blob is not available.
@@ -724,7 +724,10 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (!bootm_argv[3]) bootm_argv[3] = env_get("fdt_addr");
- if (!bootm_argv[3])
kernel_addr_r = genimg_get_kernel_addr(kernel_addr);
buf = map_sysmem(kernel_addr_r, 0);
if (!bootm_argv[3] && genimg_get_format(buf) != IMAGE_FORMAT_FIT) bootm_argv[3] = env_get("fdtcontroladdr");
if (bootm_argv[3]) {
@@ -733,8 +736,6 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) bootm_argc = 4; }
- kernel_addr_r = genimg_get_kernel_addr(kernel_addr);
- buf = map_sysmem(kernel_addr_r, 0); /* Try bootm for legacy and FIT format image */ if (genimg_get_format(buf) != IMAGE_FORMAT_INVALID && IS_ENABLED(CONFIG_CMD_BOOTM))
Reviewed-by: Neil Armstrong neil.armstrong@linaro.org
On Wed, Dec 14, 2022 at 07:45:18AM +0100, Marek Vasut wrote:
Commit d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") forces '$fdtcontroladdr' DT address as a third parameter of bootm command even if the PXE transfer pulls in a fitImage which contains configuration node with its own DT that is preferrable to be passed to Linux. Limit the $fdtcontroladdr fallback utilization to non-fitImages, since it is highly likely a fitImage would come with its own DT, while single-file images do need a separate DT.
Fixes: d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") Signed-off-by: Marek Vasut marex@denx.de Reviewed-by: Quentin Schulz quentin.schulz@theobroma-systems.com Tested-by: Quentin Schulz quentin.schulz@theobroma-systems.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Peter Hoyes peter.hoyes@arm.com Tested-by: Peter Hoyes peter.hoyes@arm.com Reviewed-by: Neil Armstrong neil.armstrong@linaro.org
Applied to u-boot/master, thanks!
participants (6)
-
Marek Vasut -
Neil Armstrong -
Peter Hoyes -
Quentin Schulz -
Simon Glass -
Tom Rini