From patchwork Tue Aug 16 17:08:35 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Reza Alizadeh Majd X-Patchwork-Id: 41710 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id ADE5527BBEA; Tue, 16 Aug 2022 18:09:30 +0100 (BST) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI,SPF_HELO_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id 8A3E727BBE9 for ; Tue, 16 Aug 2022 18:09:29 +0100 (BST) Received: from localhost ([::1]:34924 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oO04F-0001tZ-7k for patchwork@mira.cbaines.net; Tue, 16 Aug 2022 13:09:27 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:59436) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oO03q-0001t6-8N for guix-patches@gnu.org; Tue, 16 Aug 2022 13:09:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:58424) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oO03p-0006s5-W0 for guix-patches@gnu.org; Tue, 16 Aug 2022 13:09:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oO03p-0001M2-Sb for guix-patches@gnu.org; Tue, 16 Aug 2022 13:09:01 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#57070] [PATCH] bootloader: extlinux: support for optional FDTDIR Resent-From: Reza Alizadeh Majd Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 16 Aug 2022 17:09:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 57070 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 57070@debbugs.gnu.org Cc: Mathieu Othacehe , Maxime Devos Received: via spool by 57070-submit@debbugs.gnu.org id=B57070.16606697325189 (code B ref 57070); Tue, 16 Aug 2022 17:09:01 +0000 Received: (at 57070) by debbugs.gnu.org; 16 Aug 2022 17:08:52 +0000 Received: from localhost ([127.0.0.1]:48173 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oO03f-0001Lc-M5 for submit@debbugs.gnu.org; Tue, 16 Aug 2022 13:08:52 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:56143) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oO03e-0001LP-0a for 57070@debbugs.gnu.org; Tue, 16 Aug 2022 13:08:51 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id ADF995C01A5; Tue, 16 Aug 2022 13:08:44 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Tue, 16 Aug 2022 13:08:44 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantherx.org; h= cc:cc:content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm2; t=1660669724; x=1660756124; bh=9BTTCloGBh vQl+hnxdqS+upsU8DKTlrNK3VyivKQVC0=; b=Fjfr6mHPf6wrdX4StAJeXMn/ZU uELpkgXsnxCMs8xOdo3srjRTkq9QXoW7R/cS8iYCzDaUYe++17lg2xYuWSC7IogY 0KPMdYNwucARw0GC/fQi3T0CGl7au6v1itz53Hu7PD2RQ4DG36g8kUQa0svSECMq P3reTbtVrEvhPluPAfHckySuH4j4yC7e3IysQD1nL0X4FfYcPm6by1HP+A1QBGEc 9OWjcfgyadspVzn/E/l0BvxyOe5lWYmWc1JSkxvUZ02Eq30ohBX5EE2pBItVvOwm ifczxFZu4d6pdCxnqxjtUG+ymekQDY+jsWDSBfmt6xl9TskGrCSkg8T5eK8g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1660669724; x=1660756124; bh=9BTTCloGBhvQl+hnxdqS+upsU8DK TlrNK3VyivKQVC0=; b=DpBpPFiilo/BvNlC9P7WsanFwX6jh35S/qDna5hMhndo F3M+GLPoeTVZgYeTaDWlpwtgi07pPJS1x6LKWflp1nbelCT3MrT5l0rGl3vJeDn8 sGrJ0wh6zfTDvgDL9mXtQUJdLYa9f7hzoyKdd/dtZexhHccmmWhXC9RcshKFivDX 6XB9ZSB8lD5W+fbQ6QzssuDfYBPpYVaFAuEV/PxQyQwu2pmyXdDcig2sAvSFUTGw 2hcO+KV7TbVHDhWldD+a0+NOFsxjwI7hwd/Bz4Kl3EcxzmgGzRK7jK/Pnni0HWBo 6tpT/DETJZroda4XFcyiWACuhhh61GUt4MLWNwmnHQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrvdehgedgudduudcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfgjfhhoofggtgesmhdtreerredtvdenucfhrhhomheptfgv iigrucetlhhiiigruggvhhcuofgrjhguuceorhdrmhgrjhgusehprghnthhhvghrgidroh hrgheqnecuggftrfgrthhtvghrnhepvefhvdejueekhedthfelkeejjeduveeugfehhfel teduieduvefhgffhjefgfeeknecuffhomhgrihhnpehprghnthhhvghrgidrohhrghenuc evlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehrrdhmrghj ugesphgrnhhthhgvrhigrdhorhhg X-ME-Proxy: Feedback-ID: i063944dc:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 16 Aug 2022 13:08:42 -0400 (EDT) Date: Tue, 16 Aug 2022 21:38:35 +0430 From: Reza Alizadeh Majd Message-ID: <20220816213835.3e0dd301@pantherx.org> In-Reply-To: <59ee7050-d5b1-695a-4be4-c1f1856fbda7@telenet.be> References: <20220809145730.435ef8d0@pantherx.org> <59ee7050-d5b1-695a-4be4-c1f1856fbda7@telenet.be> Organization: PantherX X-Mailer: Claws Mail 4.1.0 (GTK 3.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org Sender: "Guix-patches" X-getmail-retrieved-from-mailbox: Patches Sorry for the late response, I was a little busy to apply the suggested changes. On Wed, 10 Aug 2022 11:32:38 +0200 Maxime Devos wrote: >The VM thing sounds like a good system test, add it to gnu/tests/ so >we can verify it and avoid breaking things with future changes. I implemented the `gnu/tests/bootloader.scm` tests related to the changes we are proposing. also about other remarks you mentioned me directly, I applied them on my recent patch. > Can FDTDIR be set automatically or unset automatically depending on > the hardware? That would reduce the required configuration No, that's not possible. As I mentioned in the initial message, some hardware may or may not require it depending on the kernel. On Mon, 15 Aug 2022 11:27:19 +0200 Mathieu Othacehe wrote: >The bootloader record is supposed to be bootloader agnostic. This >fdtdir naming thing seems to be extlinux specific. > >Should we maybe rename this field "device-tree-support?" in the recent patch I renamed the `ignore-fdtdir?` parameter to the `device-tree-support?` Regards, Reza From 06edac43226f8ef4bd45c973cd47c1ccd9d7da7a Mon Sep 17 00:00:00 2001 From: Reza Alizadeh Majd Date: Fri, 5 Aug 2022 20:00:42 +0430 Subject: [PATCH] bootloader: extlinux: support for optional FDTDIR There are situations that u-boot doesn't have to load from the device tree. some provide the device tree using a vendor bootloader (like what raspberry-pi does) or with an external bootloader that chainloads the u-boot (what Asahi does for m1n1 bootloader). Unfortunately we couldn't find any reliable document to enforce u-boot to pass the device tree via `extlinux.conf`, however during our tests, we found that removing the `FDTDIR` line from the `extlinux.conf` tend us to do so. There is also no reliable way to guess if u-boot bootloader should load device tree or not on a specific hardware. in addition, there are hardware that can be booted with both firmware device tree on some kernels and with special device tree on other (modified) kernels. the following changes provided to define an optional parameter in record, called which by default is set to #t to keep the current behavior unchanged. if this paramter is set to #f, the FDTDIR line will be discarded from the and u-boot doesn't load the device tree automatically. * gnu/bootloader.scm ()[ignore-fdtdir?]: new field. * gnu/bootloader/extlinux.scm (extlinux-configuration-file): add FDTDIR line based on bootloader field of . * gnu/tests/bootloader.scm: add tests for FDTDIR modification. * gnu/local.mk (GNU_SYSTEM_MODULES): Add gnu/tests/bootloader.scm --- gnu/bootloader.scm | 6 +- gnu/bootloader/extlinux.scm | 13 +++- gnu/local.mk | 2 + gnu/tests/bootloader.scm | 119 ++++++++++++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 3 deletions(-) create mode 100644 gnu/tests/bootloader.scm diff --git a/gnu/bootloader.scm b/gnu/bootloader.scm index 9cf5457873..3793cc981f 100644 --- a/gnu/bootloader.scm +++ b/gnu/bootloader.scm @@ -4,6 +4,7 @@ ;;; Copyright © 2017 Leo Famulari ;;; Copyright © 2019, 2021 Ludovic Courtès ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen +;;; Copyright © 2022 Reza Alizadeh Majd ;;; ;;; This file is part of GNU Guix. ;;; @@ -54,6 +55,7 @@ (define-module (gnu bootloader) bootloader-disk-image-installer bootloader-configuration-file bootloader-configuration-file-generator + bootloader-device-tree-support? bootloader-configuration bootloader-configuration? @@ -173,7 +175,9 @@ (define-record-type* (disk-image-installer bootloader-disk-image-installer (default #f)) (configuration-file bootloader-configuration-file) - (configuration-file-generator bootloader-configuration-file-generator)) + (configuration-file-generator bootloader-configuration-file-generator) + (device-tree-support? bootloader-device-tree-support? + (default #t))) ;;; diff --git a/gnu/bootloader/extlinux.scm b/gnu/bootloader/extlinux.scm index 6b5ff298e7..f3d69c0cc0 100644 --- a/gnu/bootloader/extlinux.scm +++ b/gnu/bootloader/extlinux.scm @@ -1,6 +1,7 @@ ;;; GNU Guix --- Functional package management for GNU ;;; Copyright © 2017 David Craven ;;; Copyright © 2017 Mathieu Othacehe +;;; Copyright © 2022 Reza Alizadeh Majd ;;; ;;; This file is part of GNU Guix. ;;; @@ -38,6 +39,10 @@ (define* (extlinux-configuration-file config entries (define all-entries (append entries (bootloader-configuration-menu-entries config))) + (define with-fdtdir? + (let ((bootloader (bootloader-configuration-bootloader config))) + (bootloader-device-tree-support? bootloader))) + (define (menu-entry->gexp entry) (let ((label (menu-entry-label entry)) (kernel (menu-entry-linux entry)) @@ -46,12 +51,16 @@ (define (menu-entry->gexp entry) #~(format port "LABEL ~a MENU LABEL ~a KERNEL ~a - FDTDIR ~a/lib/dtbs + ~a INITRD ~a APPEND ~a ~%" #$label #$label - #$kernel (dirname #$kernel) #$initrd + #$kernel + (if #$with-fdtdir? + (string-append "FDTDIR " (dirname #$kernel) "/lib/dtbs") + "") + #$initrd (string-join (list #$@kernel-arguments))))) (define builder diff --git a/gnu/local.mk b/gnu/local.mk index 88100416d5..b430c664ea 100644 --- a/gnu/local.mk +++ b/gnu/local.mk @@ -51,6 +51,7 @@ # Copyright © 2022 Remco van 't Veer # Copyright © 2022 Artyom V. Poptsov # Copyright © 2022 John Kehayias +# Copyright © 2022 Reza Alizadeh Majd # # This file is part of GNU Guix. # @@ -736,6 +737,7 @@ GNU_SYSTEM_MODULES = \ %D%/tests.scm \ %D%/tests/audio.scm \ %D%/tests/base.scm \ + %D%/tests/bootloader.scm \ %D%/tests/ci.scm \ %D%/tests/cups.scm \ %D%/tests/databases.scm \ diff --git a/gnu/tests/bootloader.scm b/gnu/tests/bootloader.scm new file mode 100644 index 0000000000..0c3bffa58d --- /dev/null +++ b/gnu/tests/bootloader.scm @@ -0,0 +1,119 @@ +;;; GNU Guix --- Functional package management for GNU +;;; Copyright © 2022 Reza Alizadeh Majd +;;; +;;; This file is part of GNU Guix. +;;; +;;; GNU Guix is free software; you can redistribute it and/or modify it +;;; under the terms of the GNU General Public License as published by +;;; the Free Software Foundation; either version 3 of the License, or (at +;;; your option) any later version. +;;; +;;; GNU Guix is distributed in the hope that it will be useful, but +;;; WITHOUT ANY WARRANTY; without even the implied warranty of +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;;; GNU General Public License for more details. +;;; +;;; You should have received a copy of the GNU General Public License +;;; along with GNU Guix. If not, see . + + +(define-module (gnu tests bootloader) + #:use-module (gnu) + #:use-module (gnu bootloader u-boot) + #:use-module (gnu system vm) + #:use-module (gnu tests) + #:use-module (guix scripts system reconfigure) + #:export (%test-uboot-with-fdtdir + %test-uboot-without-fdtdir)) + + +(define %u-boot-with-fdtdir-bootloader + (bootloader + (inherit u-boot-bootloader))) + + +(define %u-boot-without-fdtdir-bootloader + (bootloader + (inherit u-boot-bootloader) + (device-tree-support? #f))) + + +(define (u-boot-os with-fdtdir?) + (operating-system + (inherit %simple-os) + (bootloader (bootloader-configuration + (bootloader (if with-fdtdir? + %u-boot-with-fdtdir-bootloader + %u-boot-without-fdtdir-bootloader)))))) + + +(define* (run-uboot-fdtdir-test name #:key (with-fdtdir? #t)) + "Run u-boot-bootloader installation with/without FDTDIR record for +extlinux.conf" + + (define os + (marionette-operating-system + (u-boot-os with-fdtdir?))) + + (define vm (virtual-machine + (operating-system os) + (volatile? #f))) + + (define (test script) + (with-imported-modules '((gnu build marionette)) + #~(begin + (use-modules (gnu build marionette) + (srfi srfi-64)) + + (define marionette + (make-marionette (list #$vm))) + + (test-runner-current (system-test-runner #$output)) + (test-begin #$name) + + (test-assert "bootloader installed" + (marionette-eval + '(primitive-load #$script) + marionette)) + + (test-assert "extlinux.conf file created" + (marionette-eval + '(file-exists? "/boot/extlinux/extlinux.conf") + marionette)) + + (let ((content (wait-for-file "/boot/extlinux/extlinux.conf" marionette + #:read 'get-string-all + #:timeout 30))) + (if #$with-fdtdir? + (test-assert "FDTDIR exists" + (string-contains content "FDTDIR")) + (test-assert "FDTDIR removed" + (not (string-contains content "FDTDIR"))))) + + (test-end #$name)))) + + (let* ((bootcfg (operating-system-bootcfg os '())) + (bootloader ((compose bootloader-configuration-bootloader + operating-system-bootloader) os)) + (bootcfg-file (bootloader-configuration-file bootloader))) + (gexp->derivation "uboot" + (test (install-bootloader-program #f #f #f bootcfg bootcfg-file + '(#f) "/"))))) + + +(define %test-uboot-with-fdtdir + (system-test + (name "uboot-with-fdtdir") + (description "test uboot installation with fdtdir") + (value + (run-uboot-fdtdir-test "uboot-with-fdtdir" + #:with-fdtdir? #t)))) + + +(define %test-uboot-without-fdtdir + (system-test + (name "uboot-without-fdtdir") + (description "test uboot installation without fdtdir") + (value + (run-uboot-fdtdir-test "uboot-without-fdtdir" + #:with-fdtdir? #f)))) -- 2.37.1