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 002E9C25B08 for ; Tue, 23 Aug 2022 04:25:19 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id BBF92844A8; Tue, 23 Aug 2022 06:25:17 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.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=gmail.com header.i=@gmail.com header.b="SI//iRyW"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 7F777845A4; Tue, 23 Aug 2022 06:25:16 +0200 (CEST) Received: from mail-qv1-xf31.google.com (mail-qv1-xf31.google.com [IPv6:2607:f8b0:4864:20::f31]) (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 2549784355 for ; Tue, 23 Aug 2022 06:25:13 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=seanga2@gmail.com Received: by mail-qv1-xf31.google.com with SMTP id m2so2151906qvq.11 for ; Mon, 22 Aug 2022 21:25:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject:from:to:cc; bh=qOGlPWH06ufxEPbPi7xNuFA/JlhEOMnM+BOZccnREjE=; b=SI//iRyW9nODxJTeKl5vlUOyZQm8ju3R+wKEYI3iBHCSJW7xuyVl0KHVIKN2U6hH3F LNnehUNwIoo3zYXbhD9rvdg6PukFjEH+tzKtG20xqy/o3XYydWiTcPRHwjcZ2b3or9ss E0cgJ7Dhg9/M9aCuLndZWb+Zhnt75xnEP7X8qhsFnfwc7A2V5Iusp2cvYxAhVePkecM9 IU+UQfSGfhSZ3yE9QryD7fMv4TJH9VNS5s+49pISqdj5AlfQYt9s0JNfP2hu5KiI1rXO fFX8UK5pvhFgWsM4ia+/8Tm4H7YpKOQWgnFiKNoPj0eLR4q0SM55Sd0KQGIeM6KZ80CX LEGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :x-gm-message-state:from:to:cc; bh=qOGlPWH06ufxEPbPi7xNuFA/JlhEOMnM+BOZccnREjE=; b=DM6iGq8ieF/Ff4BJcinEEBfP4CupmNqdIDO6ud0EbvjqVdzAlGFTkjUH+xU4UPr7Yy owYdqF8w3gcFjZ4buftkgiYgZ4e/HfDh11FlK6uryhMnzn8rFEp3V7cmIBgWxf3rmKPg e4uUfPlZEGx/0OsHZqWUxXDufj/b84ClDmCOaoRjA7ZUjs3vBMYxBPZt9LWm88RYOfCw zdnFQdDMtM1+1GTw9ijynUjr3+T6wcc7zvDHbktI7elmQ1/qVzOsbIvCLNGJ1TeYNXZf /onS5LIxoSKgUo0k9Puv6YNIRilMPLB3nmfIq3VRDIWePt+I2hTVLW1+GNjrLgbKwIJb mtsw== X-Gm-Message-State: ACgBeo3QUBzgyGrFZ/NvQRMpnMZdDsiVefBgb3Z5KHhc8l/3Ib6vt/xT nr/PSbarrZyNlhSoVcLNwYMEsbvF+Ek= X-Google-Smtp-Source: AA6agR4iSScp6nWd2FReAkCg5BD+TgyZQ+wYsHpu7ybSjG6UFqe/X3n39R/SToRx5PmAngF5fzVJ9g== X-Received: by 2002:a05:6214:2503:b0:496:29a5:fa5b with SMTP id gf3-20020a056214250300b0049629a5fa5bmr18584471qvb.78.1661228711523; Mon, 22 Aug 2022 21:25:11 -0700 (PDT) Received: from [192.168.1.201] (pool-173-73-95-180.washdc.fios.verizon.net. [173.73.95.180]) by smtp.gmail.com with ESMTPSA id 26-20020a05620a079a00b006b8f4ade2c9sm11561936qka.19.2022.08.22.21.25.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 22 Aug 2022 21:25:11 -0700 (PDT) Subject: Re: [RFC PATCH 1/1] spl: introduce SPL_XIP to config To: Nikita Shubin Cc: linux@yadro.com, Nikita Shubin , Rick Chen , Leo , Simon Glass , Ilias Apalodimas , Heinrich Schuchardt , Sergei Miroshnichenko , Alexandru Gagniuc , Alper Nebi Yasak , Andrew Davis , u-boot@lists.denx.de References: <20220811093706.3772-1-nikita.shubin@maquefel.me> <20220811093706.3772-2-nikita.shubin@maquefel.me> <9afeff6a-07f3-a951-c083-da8ad77a5031@gmail.com> <20220815102707.7215495c@redslave.neermore.group> From: Sean Anderson Message-ID: Date: Tue, 23 Aug 2022 00:25:10 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20220815102707.7215495c@redslave.neermore.group> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US 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.6 at phobos.denx.de X-Virus-Status: Clean On 8/15/22 3:27 AM, Nikita Shubin wrote: > Hello Sean! > > On Sat, 13 Aug 2022 00:35:59 -0400 > Sean Anderson wrote: > >> Hi Nikita, >> >> On 8/11/22 5:37 AM, Nikita Shubin wrote: >>> From: Nikita Shubin >>> >>> U-Boot and SPL don't necessary share the same location, so we might >>> end with XIP SPL and U-Boot in "normal" memory. >>> >>> This adds an option special for SPL to behave it in XIP manner and >>> don't use hart_lottery and available_harts_lock. >>> >>> Signed-off-by: Nikita Shubin >>> --- >> >> On a stylistic note, typically cover letters are not used for single >> patches (but feel free to use them for larger series). >> >>> arch/riscv/cpu/cpu.c | 2 +- >>> arch/riscv/cpu/start.S | 4 ++-- >>> arch/riscv/include/asm/global_data.h | 2 +- >>> arch/riscv/lib/asm-offsets.c | 2 +- >>> arch/riscv/lib/smp.c | 2 +- >>> common/spl/Kconfig | 5 +++++ >>> include/configs/scr7_vcu118.h | 2 ++ >>> 7 files changed, 13 insertions(+), 6 deletions(-) >>> >>> diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c >>> index 9f5fa0bcb3..89528748d7 100644 >>> --- a/arch/riscv/cpu/cpu.c >>> +++ b/arch/riscv/cpu/cpu.c >>> @@ -19,7 +19,7 @@ >>> * The variables here must be stored in the data section since >>> they are used >>> * before the bss section is available. >>> */ >>> -#ifndef CONFIG_XIP >>> +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP) >> Shouldn't this be >> >> #if CONFIG_IS_ENABLED(XIP) >> >> ? >> >> ditto for the rest of these > > I think you are right, i am a bit confused with CONFIG vs > CONFIG_IS_ENABLED. If you have something like CONFIG_XIP CONFIG_SPL_XIP CONFIG_TPL_XIP then if(CONFIG_IS_ENABLED(XIP)) works out to roughly if ((defined(TPL_BUILD) && IS_ENABLED(CONFIG_TPL_XIP)) || (defined(SPL_BUILD) && IS_ENABLED(CONFIG_SPL_XIP)) || (!defined(TPL_BUILD) && !define(SPL_BUILD) && IS_ENABLED(CONFIG_XIP))) That is, the appropriate config is enabled only when building that stage. There's some other magic as well to make it work in #if as well, but that's the gist. >> >>> u32 hart_lottery __section(".data") = 0; >>> >>> /* >>> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S >>> index 26cb877ed1..d824990778 100644 >>> --- a/arch/riscv/cpu/start.S >>> +++ b/arch/riscv/cpu/start.S >>> @@ -122,7 +122,7 @@ call_board_init_f_0: >>> call_harts_early_init: >>> jal harts_early_init >>> >>> -#ifndef CONFIG_XIP >>> +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP) >>> /* >>> * Pick hart to initialize global data and run U-Boot. >>> The other harts >>> * wait for initialization to complete. >>> @@ -155,7 +155,7 @@ call_harts_early_init: >>> /* save the boot hart id to global_data */ >>> SREG tp, GD_BOOT_HART(gp) >>> >>> -#ifndef CONFIG_XIP >>> +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP) >>> la t0, available_harts_lock >>> amoswap.w.rl zero, zero, 0(t0) >>> >>> diff --git a/arch/riscv/include/asm/global_data.h >>> b/arch/riscv/include/asm/global_data.h index 9a146d1d49..d71e09c5ab >>> 100644 --- a/arch/riscv/include/asm/global_data.h >>> +++ b/arch/riscv/include/asm/global_data.h >>> @@ -30,7 +30,7 @@ int iccm[CONFIG_NR_CPUS]; >>> #if CONFIG_IS_ENABLED(SMP) >>> struct ipi_data ipi[CONFIG_NR_CPUS]; >>> #endif >>> -#ifndef CONFIG_XIP >>> +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP) >>> ulong available_harts; >>> #endif >>> }; >>> diff --git a/arch/riscv/lib/asm-offsets.c >>> b/arch/riscv/lib/asm-offsets.c index f1fe089b3d..bcb3c78654 100644 >>> --- a/arch/riscv/lib/asm-offsets.c >>> +++ b/arch/riscv/lib/asm-offsets.c >>> @@ -16,7 +16,7 @@ int main(void) >>> { >>> DEFINE(GD_BOOT_HART, offsetof(gd_t, arch.boot_hart)); >>> DEFINE(GD_FIRMWARE_FDT_ADDR, offsetof(gd_t, >>> arch.firmware_fdt_addr)); -#ifndef CONFIG_XIP >>> +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP) >>> DEFINE(GD_AVAILABLE_HARTS, offsetof(gd_t, >>> arch.available_harts)); #endif >>> >>> diff --git a/arch/riscv/lib/smp.c b/arch/riscv/lib/smp.c >>> index ba992100ad..cef324954c 100644 >>> --- a/arch/riscv/lib/smp.c >>> +++ b/arch/riscv/lib/smp.c >>> @@ -45,7 +45,7 @@ static int send_ipi_many(struct ipi_data *ipi, >>> int wait) continue; >>> } >>> >>> -#ifndef CONFIG_XIP >>> +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP) >>> /* skip if hart is not available */ >>> if (!(gd->arch.available_harts & (1 << reg))) >>> continue; >>> diff --git a/common/spl/Kconfig b/common/spl/Kconfig >>> index 07c03d611d..f24e423fc0 100644 >>> --- a/common/spl/Kconfig >>> +++ b/common/spl/Kconfig >>> @@ -27,6 +27,11 @@ config SPL_FRAMEWORK >>> supports MMC, NAND and YMODEM and other methods loading >>> of U-Boot and the Linux Kernel. If unsure, say Y. >>> >>> +config SPL_XIP >>> + bool "Support SPL in XIP" >>> + help >>> + Enable this if SPL is in XIP memory. >> >> Can you add another sentence or two? What is "XIP memory"? How does >> it differ from the standard boot process? > > In this case i treat "XIP memory" as RO memory, if CONFIG_XIP is enabled > (at least for riscv start.S) we don't use such variables as > hart_lottery or available_harts_lock, both are locks. The problem is > that CONFIG_XIP also propagate to main U-Boot, not only SPL. Great, can you add that to the description? > For example arch/riscv/cpu/start.S: > ``` > #ifndef CONFIG_XIP > la t0, hart_lottery > li t1, 1 > amoswap.w s2, t1, 0(t0) > bnez s2, wait_for_gd_init > #else > mv gp, s0 > bnez tp, secondary_hart_loop > #endif > ``` > > where tp contains hartid, so we just skip it and all harts with > non-zero hartid's goto secondary_hart_loop. > > But if we are loading main U-Boot to RAM - this doesn't make sense for > us since only SPL is in RO memory. > >> >> Also, it seems like we already have SPL_XIP_SUPPORT. Maybe we can >> reuse that? > > AFAIK this is different, it's just used to indicate that coping U-Boot > or kernel is not required if they are located in execute in place > capable flash memory. Makes sense >> >>> config SPL_FRAMEWORK_BOARD_INIT_F >>> bool "Define a generic function board_init_f" >>> depends on SPL_FRAMEWORK >>> diff --git a/include/configs/scr7_vcu118.h >>> b/include/configs/scr7_vcu118.h index 34545255d1..8f3ba36ce1 100644 >>> --- a/include/configs/scr7_vcu118.h >>> +++ b/include/configs/scr7_vcu118.h >>> @@ -29,6 +29,8 @@ >>> #define SCR7_OCRAM_REGION_SIZE 0xffff >>> >>> #define SCR7L2_CACHE 1 >>> + >>> +#define CONFIG_SYS_UBOOT_BASE CONFIG_SYS_TEXT_BASE >>> #endif >>> >>> #define CONFIG_SYS_FLASH_BASE 0x00000fffff030000 >>> >> >> What is this part doing? It's not mentioned in the commit message. > > It doesn't have any relation to main part - my bad - a bit relaxed due > to this being an RFC, sorry. > >> >> --Sean >