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 X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D565DC433EF for ; Mon, 13 Sep 2021 02:08:16 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 2789A61027 for ; Mon, 13 Sep 2021 02:08:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 2789A61027 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 09CF6839CD; Mon, 13 Sep 2021 04:08:14 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1631498894; bh=DlDMk9GQQbBoMAxvTPX7U4II+lOLIi/bCckOzCeQv2M=; h=Subject:To:Cc:References:From:Date:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=BI9EjICCEz9eEH3rDlzHekSSK0Dl0dCVJaWmW2BmslLzQofKauLdCsLgwxJvWrKeV 3lngWn1vetEpipcZXX+HB+mDOKVOLu0Nta0OI8uXpa5xLYSAvv7KlFSSoKC1iXA6Sf RG5CdRJHuQ0EXqAAqcSsTXo7lFhJDe+nv41fAleUMYiw0QXcq1ZuEzX5UH5S22QW06 95CrxkmQQQ2yQ06754OjWI1GgZxEpjrNLIVmyspYaeTPH+beLpba9skwaxtbK6K0Sw EvFXiRGeTv9FC+v3kOHPWrktxYCY9lHVvBMkRo8136smWMKQObo7osBzu+lnEvn8d3 7pAR2H8Lwpu5w== Received: from [IPv6:::1] (p578adb1c.dip0.t-ipconnect.de [87.138.219.28]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: marex@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 7B74B839CB; Mon, 13 Sep 2021 04:08:10 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1631498891; bh=DlDMk9GQQbBoMAxvTPX7U4II+lOLIi/bCckOzCeQv2M=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=YMi8DC5Wa07XCt8kruYJBzwYCMgzX3C3gVgBmb8dEs8aJWPzrENtyzypkbxgE3Do0 baPyydIVd2jcQR8AT+Kb/TlJ+LVNgmYXb7pbC0zRLKudlDAEbiFA7jE7nB0LL4qmFA EqgqSJyzim4idEeY+Cy8x4r8gks14rnceCt/iW9mVIQojL8xM3h2GzD2DdWuVqNWuH p+QfaFS2eNBhACBvxMvULQGdypy6AfXNAXKo4NpNSqXvX7s8zqnG32MstYWhROS8Zg zzOG8M6VKsU6rw+IHQ6HRUGVKQDB5ZfZHxEd++RRpZhU8See6Dl4PeR7p2dh4jX68p X0JAD6WxfnGRA== Subject: Re: [PATCH 03/12] lmb: Add generic arch_lmb_reserve_generic() To: Daniel Schwierzeck , u-boot@lists.denx.de Cc: Alexey Brodkin , Angelo Dureghello , Eugeniy Paltsev , Hai Pham , Michal Simek , Simon Goldschmidt , Tom Rini , Wolfgang Denk References: <20210910204718.17765-1-marek.vasut+renesas@gmail.com> <20210910204718.17765-3-marek.vasut+renesas@gmail.com> <4d503594c9da872302e9afc79f070f7037ff2316.camel@gmail.com> From: Marek Vasut Message-ID: Date: Mon, 13 Sep 2021 04:08:09 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <4d503594c9da872302e9afc79f070f7037ff2316.camel@gmail.com> 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.34 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.2 at phobos.denx.de X-Virus-Status: Clean On 9/12/21 9:27 PM, Daniel Schwierzeck wrote: [...] >> diff --git a/lib/lmb.c b/lib/lmb.c >> index 7bd1255f7a..793647724c 100644 >> --- a/lib/lmb.c >> +++ b/lib/lmb.c >> @@ -12,6 +12,10 @@ >> #include >> #include >> >> +#include >> + >> +DECLARE_GLOBAL_DATA_PTR; >> + >> #define LMB_ALLOC_ANYWHERE 0 >> >> static void lmb_dump_region(struct lmb_region *rgn, char *name) >> @@ -113,6 +117,37 @@ void lmb_init(struct lmb *lmb) >> lmb->reserved.cnt = 0; >> } >> >> +void arch_lmb_reserve_generic(struct lmb *lmb, ulong sp, ulong end, >> ulong align) >> +{ >> + ulong bank_end; >> + int bank; >> + >> + /* >> + * Reserve memory from aligned address below the bottom of U- >> Boot stack >> + * until end of U-Boot area using LMB to prevent U-Boot from >> overwriting >> + * that memory. >> + */ >> + debug("## Current stack ends at 0x%08lx ", sp); >> + >> + /* adjust sp by 4K to be safe */ > > nit: comment doesn't fit anymore > >> + sp -= align; >> + for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { >> + if (!gd->bd->bi_dram[bank].size || >> + sp < gd->bd->bi_dram[bank].start) >> + continue; >> + /* Watch out for RAM at end of address space! */ >> + bank_end = gd->bd->bi_dram[bank].start + >> + gd->bd->bi_dram[bank].size - 1; >> + if (sp > bank_end) >> + continue; >> + if (bank_end > end) >> + bank_end = end - 1; > > isn't that all already taken care of when initializing gd->ram_top as > well as gd->relocaddr and gd->start_addr_sp (based on gd->ram_top)? So > gd->ram_top is already guaranteed to be less or equal some DRAM bank > end address. AFAIK arch_lmb_reserve() should just protect the U-Boot > area from gd->start_addr_sp up to gd->ram_top as well as the stack area > from the current stack pointer up to the initial stack pointer in gd- >> start_addr_sp. Also you changed all callers to always pass gd->ram_top > in "ulong end". Thus I think arch_lmb_reserve_generic() could omit > those redundant checks and could be simplified to: > > sp -= align; > lmb_reserve(lmb, sp, gd->ram_top - sp); > Or am I overlooking something? Is that a valid use case to have U-Boot This would not allow reserving space at the end of DRAM bank for e.g. DSP firmware which just has to be at the end of DRAM bank. That's why there is this passing of gd->ram_top into this function and it's not part of the function, to prepare for this use case. > area and stack area in different memory banks? If yes, shouldn't then > lmb_reserve() be called twice by arch_lmb_reserve() to protect stack > area AND U-Boot area? The U-Boot stack is below relocated U-Boot, and there is other stuff between those two, so that's why this isn't called twice for stack and U-Boot.