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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EED1CC433EF for ; Tue, 18 Jan 2022 12:53:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=iQLzQL9D8F2gKUUBHcOizM1450eVic1hovYXgG398M8=; b=eJHjyHkmdbTjXK VxCj3ZmRmUqqO4tZnbbERyaf569av/ujNPpMPRn984wJJy9CiTs/sN193dhdpmeRLshlwInxYtS/u oSqvswimuqNRsX9I4OKrbPNZ6Tewv3tGvk7sBH7njonJ+s+AGCmRuZfkfx0hvUbz44mJLMFs1rcg6 R0GR9LQmQm6oZiW0LXjbF/2a1QIBEX4GmFtw+WoYv9hZvCnmJ9DWpaCpWGfF6gaPM7XsvV4o6lPFz zRWYobUXl/bR2BFcsC6QJFiIoTpa6F9v9elHwA9Uq6baLOCaJQiit1JEMrV+u99UoVdDLHtOdCVo0 CAjis0F9K3OYU4zqfgwA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9ny7-001WJP-Sk; Tue, 18 Jan 2022 12:52:12 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9ny3-001WIj-W0 for linux-arm-kernel@lists.infradead.org; Tue, 18 Jan 2022 12:52:09 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4CC78D6E; Tue, 18 Jan 2022 04:52:06 -0800 (PST) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B487B3F73D; Tue, 18 Jan 2022 04:52:05 -0800 (PST) Date: Tue, 18 Jan 2022 12:52:03 +0000 From: Andre Przywara To: Mark Rutland Cc: linux-arm-kernel@lists.infradead.org, Jaxson Han Subject: Re: [boot-wrapper PATCH v2 3/9] Makefile: Tell compiler to generate bare-metal code Message-ID: <20220118125203.272ccf8c@donnerap.cambridge.arm.com> In-Reply-To: References: <20211222181607.1203191-1-andre.przywara@arm.com> <20211222181607.1203191-4-andre.przywara@arm.com> <20220107143811.3d193704@donnerap.cambridge.arm.com> Organization: ARM X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220118_045208_176397_FBE649F5 X-CRM114-Status: GOOD ( 58.50 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, 11 Jan 2022 11:30:18 +0000 Mark Rutland wrote: Hi Mark, > On Fri, Jan 07, 2022 at 02:38:11PM +0000, Andre Przywara wrote: > > On Fri, 7 Jan 2022 13:53:50 +0000 > > Mark Rutland wrote: > > > > Hi Mark, > > > > > On Wed, Dec 22, 2021 at 06:16:01PM +0000, Andre Przywara wrote: > > > > Our GCC invocation does not provide many parameters, which lets the > > > > toolchain fill in its own default setup. > > > > In case of a native build or when using a full-featured cross-compiler, > > > > this probably means Linux userland, which is not what we want for a > > > > bare-metal application like boot-wrapper. > > > > > > > > Tell the compiler to forget about those standard settings, and only use > > > > what we explicitly ask for. In particular that means to not use toolchain > > > > provided libraries and headers, since they might pull in more code than > > > > we want, and might not run well in the boot-wrapper environment. > > > > > > > > This also enables optimisation, since it produces much better code and > > > > tends to avoid problems due to missing inlining, for instance. > > > > > > I'd appreciate if we could add some explanation for these (just in the commit > > > message is fine), as e.g. it's not clear to me why -fno-toplevel-reorder is > > > necessary. > > > > Yeah, I was expecting some discussion about this. I went over most options > > from the manpage a while ago (for some unrelated baremetal code project), > > and decided for each option whether it would make sense or not. TBH I > > don't remember every detail from that, but I can certainly just drop the > > less critical options from that list. > > Yes please. > > To be clear, I'm only suggesting dropping a few of these, and I think the > others just needs some explanation. > > I'm certain we *should* add: > > -ffreestanding > -nostdlib > > ... and I'd be happy to take a patch adding just those as an immediate fix. > > I think the folllowing are reasonable but need some explanation: > > -Wstrict-prototypes // Just to catch accidents Agreed. > -fno-stack-protector // Because we dont have any init/support code ... and it breaks the link, as we learned. > -fno-strict-aliasing // Do we rely on type punning today? I don't know for sure, but it seems like that's what this kind of low level software would like to do? Plus the kernel uses it also. > -fno-unwind-tables // Because we don't consume the result > -fno-asynchronous-unwind-tables // Because we don't consume the result Plus those two prevent sections of rather pointless data to be emitted into the ELF file. This blew up my own bare metal code considerably, though I don't see it in the boot-wrapper. > I don't understand why we need: > > -fno-common I think most projects use that to put uninitialised global variables in .BSS, to save space in .data and thus the image file. I see it used in all of Linux, U-Boot and TF-A. With the kernel image absolutely dwarfing any actual boot-wrapper code in our ELF file, I think we are not particularly sensitive about code/file size, though, are we? > -fno-toplevel-reorder Yeah, this one is nonsense, I think I had this as a workaround to one particular problem. Will drop it. > -Os AFAICT we don't use any optimisation at the moment, which typically generates horrible code. And IIRC at the least the kernel relies on some optimisation (due to some inlining tricks?), I wonder if we sooner or later inherit this requirement? > ... and I suspect we can drop those unless we have some rationale. > > I'm torn over: > > -nostdinc > > ... as IIUC that's just to catch accidental includes, but necessitates the > prior patch adding copies of some headers. We could arguably leave that as-is > and just be careful. Yeah, I see the point, this probably drops too much. I am just concerned that the standard header files these days pull in far too much *code* even, and then create some dependencies on libraries? For certain division operations, for instance? So looking at the *current* code, I see only stdint.h and stddef.h from the standard headers to be included. I'd say we can live with these coming from the toolchain. The only problem I see is when this gets extended, and someone pulls in string.h, for instance. IIUC modern toolchains put in quite some optimisation in the string routines, which might not go well in the boot-wrapper environment (NEON?). But for now we should indeed keep the standard headers, and drop our own stdint.h. I will send a patch with the remaining flags, plus some rationale as comments, similar to what you sketched already. Cheers, Andre > > And I would be happy to add some rationale for each of them, but wonder > > what the best place would be? I'd rather do this in a file than in a > > commit message, would you prefer comments in Makefile.am, or in README or > > a separate file, with a pointer from Makefile.am. Or in a commit message > > anyway? > > I think within the Makefile would be best if you're happy to organise it, e.g. > > # The boot-wrapper is a non-hosted environment and isn't linked > # with standard libraries. > CFLAGS += -ffreestanding -nostdlib > > # Avoid the pointless creation og a GOT > CFLAGS += -fno-pic -fno-pie > > ... does that work for you? > > Thanks, > Mark. > > > > > Cheers, > > Andre > > > > > > > > > > Signed-off-by: Andre Przywara > > > > --- > > > > Makefile.am | 7 ++++++- > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/Makefile.am b/Makefile.am > > > > index 3e970a3..d9ad6d1 100644 > > > > --- a/Makefile.am > > > > +++ b/Makefile.am > > > > @@ -124,9 +124,14 @@ CHOSEN_NODE := chosen { \ > > > > > > > > CPPFLAGS += $(INITRD_FLAGS) > > > > CFLAGS += -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/ > > > > -CFLAGS += -Wall -fomit-frame-pointer > > > > +CFLAGS += -Wall -Wstrict-prototypes -fomit-frame-pointer > > > > +CFLAGS += -ffreestanding -nostdinc -nostdlib > > > > +CFLAGS += -fno-common -fno-strict-aliasing -fno-stack-protector > > > > +CFLAGS += -fno-toplevel-reorder > > > > +CFLAGS += -fno-unwind-tables -fno-asynchronous-unwind-tables > > > > CFLAGS += -ffunction-sections -fdata-sections > > > > CFLAGS += -fno-pic -fno-pie > > > > +CFLAGS += -Os > > > > LDFLAGS += --gc-sections > > > > > > > > OBJ := $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ)) > > > > -- > > > > 2.25.1 > > > > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel