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=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 B99B7C433EF for ; Sun, 12 Sep 2021 19:27:22 +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 0DC7D60F51 for ; Sun, 12 Sep 2021 19:27:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 0DC7D60F51 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com 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 6306983989; Sun, 12 Sep 2021 21:27:19 +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="n/+M2UmJ"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 937028398D; Sun, 12 Sep 2021 21:27:16 +0200 (CEST) Received: from mail-ej1-x630.google.com (mail-ej1-x630.google.com [IPv6:2a00:1450:4864:20::630]) (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 2709183989 for ; Sun, 12 Sep 2021 21:27:12 +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=daniel.schwierzeck@gmail.com Received: by mail-ej1-x630.google.com with SMTP id a25so16174872ejv.6 for ; Sun, 12 Sep 2021 12:27:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:subject:from:to:cc:date:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=ZHrrK7FbF0u19VhQyNXyzWbVGh+QRR+ysVljt/OkzOw=; b=n/+M2UmJuuhyXUOtq1oEgz8FfvnZXpMenCtc93h9VT88iPJ1wyzy1gW4yKcuOg9Jfi x6uP7Ph+y3UIBc+a+9n6RZqPXszeCLV0uKA04/7Mc6FdC9dsONGzqRXqILDDqDnXkp80 wKR3jG0GJ9eDzCMUzpnC3xveaDHVgqsNmiiaQkWnzx3sWr5yeWHJhRr1tnVzDvWgeaYn mqUbq1+SZbeatP9Ow6jyFzzvUANM1dCG7lyjQUysxpRXnSQUJX0WqDduGg73+YiX+jCG phQXAqsBdzl6y+giLh5WMslMYn4bSlJKyy+4nmGA8D+DEpKipJuMsf2MTpx80zM6nUDI rUTw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=ZHrrK7FbF0u19VhQyNXyzWbVGh+QRR+ysVljt/OkzOw=; b=TxxFL4RamEFADsbzo96gIvTaE7hhcsgXiDKIxtk+SiqpaLvbop+A+HK3PGTU7JPBXI 15fxa9nYJvycErMtqxsB2UaOinNWBn6pVyQZSuLJDmDLMnOztrpo9tl3oxvOR+DIAIN9 fAMJzA5o2M1WY8caUPdw4yNerN24Ag2wOvA53xgqUy2jbv9gR7Abse58rZoIOkizaIMY i6EeMp4xWOZgKrrdTU8ckn5XU5YrcolESnQlDOrlbHTmgW7ALe4/Y2c+D4JRwNvUQMzg 9jNTufH2dDr1aUYbYH3gQjkQBZykNzmCSukeH71yBHDMPyvjkFHcJf/LMg3MBq8/ay7k MV9w== X-Gm-Message-State: AOAM530j5BeAZGDdYhw9DvgcwBVKESSLeWhx7u3a8EUQh6ZFxkffdcbt Bc00wFr7bEyKK9xsWY8kUp8= X-Google-Smtp-Source: ABdhPJyCYqrAjaYMYp9ZcoseHZuhnhjGCf9xvZ4VhXygGmBuj7bgZWy3UOj7o4ADF0JJhq+LxXVURQ== X-Received: by 2002:a17:906:c1c9:: with SMTP id bw9mr8835967ejb.3.1631474831745; Sun, 12 Sep 2021 12:27:11 -0700 (PDT) Received: from workstation.lan.schwierd.dedyn.io (i5E8606A7.versanet.de. [94.134.6.167]) by smtp.googlemail.com with ESMTPSA id ml12sm2355452ejb.29.2021.09.12.12.27.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 12 Sep 2021 12:27:11 -0700 (PDT) Message-ID: <4d503594c9da872302e9afc79f070f7037ff2316.camel@gmail.com> Subject: Re: [PATCH 03/12] lmb: Add generic arch_lmb_reserve_generic() From: Daniel Schwierzeck To: Marek Vasut , u-boot@lists.denx.de Cc: Marek Vasut , Alexey Brodkin , Angelo Dureghello , Eugeniy Paltsev , Hai Pham , Michal Simek , Simon Goldschmidt , Tom Rini , Wolfgang Denk Date: Sun, 12 Sep 2021 21:27:10 +0200 In-Reply-To: <20210910204718.17765-3-marek.vasut+renesas@gmail.com> References: <20210910204718.17765-1-marek.vasut+renesas@gmail.com> <20210910204718.17765-3-marek.vasut+renesas@gmail.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5-0ubuntu1 MIME-Version: 1.0 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 Am Freitag, dem 10.09.2021 um 22:47 +0200 schrieb Marek Vasut: > The arc/arm/m68k/microblaze/mips/ppc arch_lmb_reserve() > implementations > are all mostly the same, except for a couple of details. Implement a > generic arch_lmb_reserve_generic() function which can be parametrized > enough to cater for those differences between architectures. This can > also be parametrized enough so it can handle cases where U-Boot is > not > relocated to the end of DRAM e.g. because there is some other > reserved > memory past U-Boot (e.g. unmovable firmware for coprocessor), it is > not > relocated at all, and other such use cases. > > Signed-off-by: Marek Vasut > Cc: Alexey Brodkin > Cc: Angelo Dureghello > Cc: Daniel Schwierzeck > Cc: Eugeniy Paltsev > Cc: Hai Pham > Cc: Michal Simek > Cc: Simon Goldschmidt > Cc: Tom Rini > Cc: Wolfgang Denk > --- > V2: Reword code comment > --- > include/lmb.h | 1 + > lib/lmb.c | 35 +++++++++++++++++++++++++++++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/include/lmb.h b/include/lmb.h > index 3c4afdf9f0..1984291132 100644 > --- a/include/lmb.h > +++ b/include/lmb.h > @@ -122,6 +122,7 @@ lmb_size_bytes(struct lmb_region *type, unsigned > long region_nr) > > void board_lmb_reserve(struct lmb *lmb); > void arch_lmb_reserve(struct lmb *lmb); > +void arch_lmb_reserve_generic(struct lmb *lmb, ulong sp, ulong end, > ulong align); > > /* Low level functions */ > > 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 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? > + > + lmb_reserve(lmb, sp, bank_end - sp + 1); > + break; > + } > +} > + > static void lmb_reserve_common(struct lmb *lmb, void *fdt_blob) > { > arch_lmb_reserve(lmb); -- - Daniel