From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 03EBCC433EF for ; Wed, 18 May 2022 18:15:33 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 1CBF584294; Wed, 18 May 2022 20:15:31 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=baylibre-com.20210112.gappssmtp.com header.i=@baylibre-com.20210112.gappssmtp.com header.b="MsnFsgqR"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 9E08F84294; Wed, 18 May 2022 20:15:29 +0200 (CEST) Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 7DA7C8428C for ; Wed, 18 May 2022 20:15:25 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=narmstrong@baylibre.com Received: by mail-wr1-x42d.google.com with SMTP id t6so3819563wra.4 for ; Wed, 18 May 2022 11:15:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20210112.gappssmtp.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:organization:in-reply-to :content-transfer-encoding; bh=9B3bupCfvExXY1fzV6/soMJbpnp6I+bs0V3ogayuE08=; b=MsnFsgqRM/FqmoKY4gbIktTyGNccm+MSOdeJdjntrpC7Ix1AggqwwhlDFpEYQU1aoc 7sOYLqEwoCQbUuLaC0eT3ImCqNHGc8pzURxR82Oejjek7qTcda/vEt3Ug+9YWjAh4HeB czrkhnSsdzWoLpzyZaJbheqGenRFXhF26xM6oBhGo3sdmqIJKKX+u/Icp0+Ev4gkZq2Y nF0WEC2c0mTKjYlDRPCZkmebXssNNB9zcvGWPLAKwGQXhr3uRCRryUeK7WJ60a2oX8lq DalZV0sp0A4zF3EUSety8sVjtRv45nnjLjxmfmiLg4CTSrEPDQ5xY095eaUYQtzA5+4w OZ/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:organization:in-reply-to :content-transfer-encoding; bh=9B3bupCfvExXY1fzV6/soMJbpnp6I+bs0V3ogayuE08=; b=xRZwmE9hZij7Mh4W6jVc4JPWiJnPtsZ+KeqgA4dEsOVT2EC4rdDJll6gS/YvLEYhhH RveCjb3JbEDPkaR/W6EE7/qFs2EiIzJCtbNq3ph6liVP6TskfTtO5cT1uVCcxkgKsif5 A4sF1MF9JSMH1ElXJAoteU6+G5JIeDPk7HXN0BbdfujiQsHhKg73WUZsOJGCtKc15b2G 5U9CjDXREsUso8kNjOM0NEAHV8sTvOFpnEzYtUhkyYwoJOtPQGn7Xmd9Xw32YsyjhsmY PVOVF5EUrC7dOeZ9jidbAWTpKe5NoK8aS9JXDNMCDHM2cCLrtjbkjsdDPWkbYd5F48eE 2p4w== X-Gm-Message-State: AOAM532qPUva+Kz4RzRCHBPwWdFGsQoNOGa92ePQvh8hG+33utmV3NMX ujfSZfIlAJ5EeaE7tpeiKBfvdg== X-Google-Smtp-Source: ABdhPJxDtnYEd74Suk57ot+PYTZTrdIzfp4pXgGc6087+t13/EczbaatrESUT3FWLifRpDwHVBlRdQ== X-Received: by 2002:adf:e106:0:b0:20a:b31b:213d with SMTP id t6-20020adfe106000000b0020ab31b213dmr763867wrz.219.1652897724866; Wed, 18 May 2022 11:15:24 -0700 (PDT) Received: from ?IPV6:2001:861:44c0:66c0:dea9:338d:d500:c574? ([2001:861:44c0:66c0:dea9:338d:d500:c574]) by smtp.gmail.com with ESMTPSA id p3-20020a1c2903000000b003942a244ed7sm2251708wmp.28.2022.05.18.11.15.23 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 18 May 2022 11:15:24 -0700 (PDT) Message-ID: <17defab5-a104-3fe9-c87a-0ba18dae799e@baylibre.com> Date: Wed, 18 May 2022 20:15:23 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 Subject: Re: [PATCH] cmd: pxe_utils: Check fdtcontroladdr in label_boot Content-Language: en-US To: Tom Rini Cc: Peter Hoyes , u-boot@lists.denx.de, bmeng.cn@gmail.com, sjg@chromium.org, kory.maincent@bootlin.com, andre.przywara@arm.com, diego.sueiro@arm.com References: <20211014084004.3173835-1-peter.hoyes@arm.com> <20220518155253.GF3901321@bill-the-cat> From: Neil Armstrong Organization: Baylibre In-Reply-To: <20220518155253.GF3901321@bill-the-cat> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean On 18/05/2022 17:52, Tom Rini wrote: > On Wed, May 18, 2022 at 10:40:12AM +0200, Neil Armstrong wrote: >> Hi, >> >> On 14/10/2021 10:40, Peter Hoyes wrote: >>> From: Peter Hoyes >>> >>> If using OF_CONTROL, fdtcontroladdr is set to the fdt used to configure >>> U-Boot. When using PXE, if no fdt is defined in the menu file, and >>> there is no fdt at fdt_addr, add fall back on fdtcontroladdr too. >>> >>> We are developing board support for the Armv8r64 FVP using >>> config_distro_bootcmd. We are also using OF_BOARD and would like the >>> PXE boot option to default to the fdt provided by board_fdt_blob_setup. >>> >>> Signed-off-by: Peter Hoyes >>> --- >>> cmd/pxe_utils.c | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c >>> index 067c24e5ff..8f8e69ca97 100644 >>> --- a/cmd/pxe_utils.c >>> +++ b/cmd/pxe_utils.c >>> @@ -556,7 +556,10 @@ static int label_boot(struct cmd_tbl *cmdtp, struct pxe_label *label) >>> * Scenario 2: If there is an fdt_addr specified, pass it along to >>> * bootm, and adjust argc appropriately. >>> * >>> - * Scenario 3: fdt blob is not available. >>> + * Scenario 3: If there is an fdtcontroladdr specified, pass it along to >>> + * bootm, and adjust argc appropriately. >>> + * >>> + * Scenario 4: fdt blob is not available. >>> */ >>> bootm_argv[3] = env_get("fdt_addr_r"); >>> @@ -646,6 +649,9 @@ static int label_boot(struct cmd_tbl *cmdtp, struct pxe_label *label) >>> if (!bootm_argv[3]) >>> bootm_argv[3] = env_get("fdt_addr"); >>> + if (!bootm_argv[3]) >>> + bootm_argv[3] = env_get("fdtcontroladdr"); >>> + >>> if (bootm_argv[3]) { >>> if (!bootm_argv[2]) >>> bootm_argv[2] = "-"; >> >> This change makes a regression when using a FIT image as kernel image within an extlinux.conf >> >> Before this change, when a DT wasn't specified, the DT in the FIT image was selected, >> not the u-boot DT is selected. >> >> While it should work because the U-Boot DT should work in Linux, it's not always the case. >> >> Before: >> Found /extlinux/extlinux.conf >> Retrieving file: /extlinux/extlinux.conf >> 1: Yocto >> Retrieving file: /fitImage >> append: root=PARTUUID=fd26d57f-02 rootwait console=ttyAML0,115200 >> ## Loading kernel from FIT Image at 08080000 ... >> Using 'conf-amlogic_meson-gxl-s905x-libretech-cc.dtb' configuration >> Trying 'kernel-1' kernel subimage >> Description: Linux kernel >> Type: Kernel Image >> Compression: gzip compressed >> Data Start: 0x08080120 >> Data Size: 9926956 Bytes = 9.5 MiB >> Architecture: AArch64 >> OS: Linux >> Load Address: 0x01080000 >> Entry Point: 0x01080000 >> Hash algo: sha256 >> Hash value: 5181a76e4e7a728e24cd8569f8e48c543ac259bf4d66591a3dc5e166d709429e >> Verifying Hash Integrity ... sha256+ OK >> ## Loading fdt from FIT Image at 08080000 ... >> Using 'conf-amlogic_meson-gxl-s905x-libretech-cc.dtb' configuration >> Trying 'fdt-amlogic_meson-gxl-s905x-libretech-cc.dtb' fdt subimage >> Description: Flattened Device Tree blob >> Type: Flat Device Tree >> Compression: uncompressed >> Data Start: 0x089f7b78 >> Data Size: 29092 Bytes = 28.4 KiB >> Architecture: AArch64 >> Hash algo: sha256 >> Hash value: 72e5e4fcbb4aa59042377720e5636132ba790d85b6c3f6442446acc63f48cf67 >> Verifying Hash Integrity ... sha256+ OK >> Booting using the fdt blob at 0x89f7b78 >> Uncompressing Kernel Image >> Loading Device Tree to 000000007bf2f000, end 000000007bf391a3 ... OK >> >> >> After: >> Found /extlinux/extlinux.conf >> Retrieving file: /extlinux/extlinux.conf >> 1: Yocto >> Retrieving file: /fitImage >> append: root=PARTUUID=fd26d57f-02 rootwait console=ttyAML0,115200 >> ## Loading kernel from FIT Image at 08080000 ... >> Using 'conf-amlogic_meson-gxl-s905x-libretech-cc.dtb' configuration >> Trying 'kernel-1' kernel subimage >> Description: Linux kernel >> Type: Kernel Image >> Compression: gzip compressed >> Data Start: 0x08080120 >> Data Size: 9926956 Bytes = 9.5 MiB >> Architecture: AArch64 >> OS: Linux >> Load Address: 0x01080000 >> Entry Point: 0x01080000 >> Hash algo: sha256 >> Hash value: 5181a76e4e7a728e24cd8569f8e48c543ac259bf4d66591a3dc5e166d709429e >> Verifying Hash Integrity ... sha256+ OK >> ## Flattened Device Tree blob at 7bf40210 >> Booting using the fdt blob at 0x7bf40210 >> Uncompressing Kernel Image >> Loading Device Tree to 000000007bf2c000, end 000000007bf38ba7 ... OK >> >> >> Note this is the "best in town" boot method in Yocto/OE when using WIC, FIT Image and >> generating extlinux.conf from the WIC "bootimg-partition" plugin. >> >> So this change will regress all platforms using this scheme. >> >> In case we only want the FIT image and not DTB, the fdtdir is ommited at: >> https://github.com/openembedded/openembedded-core/blob/9015dec93233c7d45fd0c9885ff5d4ec23ad377d/scripts/lib/wic/plugins/source/bootimg-partition.py#L150 > > Well this is certainly tricky. Do we be "boring" and say that the > built-in device tree should be used, or "boring" and say that the distro > provided device tree should be used. I assume that in the OE case we're > not always going to be booting U-Boot that was built, so updating the > logic there to have the correct dtb isn't right either. > I'm not against this logic, it should definitely be aligned on EFI and permit using built-in DT, but the logic breaks fitImage loading via extlinux, because now we have no way to say "use the fitImage DT". This change should have been elsewhere, like in the non-FIT bootm path when no DT was supplied and DT is mandatory for the platform. Or by passing a keyword like fdt-built-in or ftdfile=builtin. ==><======================================================= diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 0c24becae3..22040c2340 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -682,7 +682,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) fdtfile = fdtfilefree; } - if (fdtfile) { + if (strcmp(fdtfile, "builtin") && fdtfile) { int err = get_relfile_envaddr(ctx, fdtfile, "fdt_addr_r", NULL); @@ -724,7 +724,7 @@ 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]) + if (!strcmp(fdtfile, "builtin")) bootm_argv[3] = env_get("fdtcontroladdr"); if (bootm_argv[3]) { ==><======================================================= Or by checking the image format before using fdtcontroladdr like: ==><======================================================= diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 0c24becae3..f4f6369de6 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -724,7 +724,7 @@ 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]) + if (genimg_get_format(buf) != IMAGE_FORMAT_FIT && !bootm_argv[3]) bootm_argv[3] = env_get("fdtcontroladdr"); if (bootm_argv[3]) { ==><======================================================= Neil