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=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 447FAC43461 for ; Mon, 14 Sep 2020 13:11:30 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D04AF208DB for ; Mon, 14 Sep 2020 13:11:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="xpnv2ZME"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="HJEzeySp" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D04AF208DB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject: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=ZMdshxF52HHw1tqlP5MxfuqLczotW8BPwX8J4gi7hMA=; b=xpnv2ZMEGT15++x8Igh8/RZ9R VaNVXdgw3qWHECaZSSBOoPMkYzE0JFQGQF5Spi4gPmKiPGl2OccscQBnwjb32jrMd7yeLd4uwlU45 RkWn9Oq1KxGpYSikLKu8gUZnGJz3T30sM/bffK0a2wR/VDhnp+mm/H7na65TpAEB5UU/XIMM416/R Vh8WxT4z0XZXtJuhIo8I07hyoqjefIQZ6U2eIvxrSPrTXnBt216YlMtvenwMIoFE+Z8bjKtSl7nLS 9td8pDRtuMCVj2LdiWSXD2ybtqiU/c2NXupQTxU/PTZq7mfB34u2+6pIPlxiigjKBEQtsAbg9ZkDO mDX10ERRw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kHoF5-0004c9-Kl; Mon, 14 Sep 2020 13:09:59 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kHoEr-0004XN-8o for linux-arm-kernel@lists.infradead.org; Mon, 14 Sep 2020 13:09:51 +0000 Received: from willie-the-truck (236.31.169.217.in-addr.arpa [217.169.31.236]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 7AF03206B2; Mon, 14 Sep 2020 13:09:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600088984; bh=enr+iyTv6u2XzskZFJ5qT83Kq/MSJgsJ+u1/0SVC2DE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=HJEzeySpEP9Li+pydqi15kb1u831aaAa+xIym6H9E2N3vCvslGr2iwd+P8M1q2/KV oSZObq1fkwLf0vFtUgCRsKaEc92nsIxzoi2BG5AQAEfuEj1PR858lINKG6gctQ6lsa BjSmPK+oikK1hmXW8zjoRAQ2wZdbtaPoD7suvK/g= Date: Mon, 14 Sep 2020 14:09:38 +0100 From: Will Deacon To: David Brazdil Subject: Re: [PATCH v2 02/10] kvm: arm64: Partially link nVHE hyp code, simplify HYPCOPY Message-ID: <20200914130937.GC24441@willie-the-truck> References: <20200903091712.46456-1-dbrazdil@google.com> <20200903091712.46456-3-dbrazdil@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200903091712.46456-3-dbrazdil@google.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200914_090945_501765_2325D2D7 X-CRM114-Status: GOOD ( 34.78 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-arch@vger.kernel.org, Arnd Bergmann , Suzuki K Poulose , Marc Zyngier , linux-kernel@vger.kernel.org, James Morse , linux-arm-kernel@lists.infradead.org, Catalin Marinas , Tejun Heo , Dennis Zhou , Christoph Lameter , kernel-team@android.com, kvmarm@lists.cs.columbia.edu, Julien Thierry 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 Thu, Sep 03, 2020 at 11:17:04AM +0200, David Brazdil wrote: > Previous series introduced custom build rules for nVHE hyp code, using > objcopy to prefix ELF section and symbol names to separate nVHE code > into its own "namespace". This approach was limited by the expressiveness > of objcopy's command line interface, eg. missing support for wildcards. nit: "Previous series" isn't a lot of use here or in the git log. You can just say something like: "Relying on objcopy to prefix the ELF section names of the nVHE hyp code is brittle and prevents us from using wildcards to match specific section names." and then go on to explain what the change is doing (see Documentation/process/submitting-patches.rst for more help here) Also, given that this is independent of the other patches, please can you move it right to the start of the series? I'm a bit worried about the potential for regressions given the changes to the way in which we link, so the sooner we can get this patch some more exposure, the better. > Improve the build rules by partially linking all '.hyp.o' files and > prefixing their ELF section names using a linker script. Continue using > objcopy for prefixing ELF symbol names. > > One immediate advantage of this approach is that all subsections > matching a pattern can be merged into a single prefixed section, eg. > .text and .text.* can be linked into a single '.hyp.text'. This removes > the need for -fno-reorder-functions on GCC and will be useful in the > future too: LTO builds use .text subsections, compilers routinely > generate .rodata subsections, etc. > > Partially linking all hyp code into a single object file also makes it > easier to analyze. > > Signed-off-by: David Brazdil > --- > arch/arm64/kvm/hyp/nvhe/Makefile | 56 ++++++++++++++++--------------- > arch/arm64/kvm/hyp/nvhe/hyp.lds.S | 14 ++++++++ > 2 files changed, 43 insertions(+), 27 deletions(-) > create mode 100644 arch/arm64/kvm/hyp/nvhe/hyp.lds.S > > diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile > index aef76487edc2..1b2fbb19f3e8 100644 > --- a/arch/arm64/kvm/hyp/nvhe/Makefile > +++ b/arch/arm64/kvm/hyp/nvhe/Makefile > @@ -10,40 +10,42 @@ obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o > obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \ > ../fpsimd.o ../hyp-entry.o > > -obj-y := $(patsubst %.o,%.hyp.o,$(obj-y)) > -extra-y := $(patsubst %.hyp.o,%.hyp.tmp.o,$(obj-y)) > +## > +## Build rules for compiling nVHE hyp code > +## Output of this folder is `hyp.o`, a partially linked object file containing > +## all nVHE hyp code and data. > +## > > -$(obj)/%.hyp.tmp.o: $(src)/%.c FORCE > +hyp-obj := $(patsubst %.o,%.hyp.o,$(obj-y)) > +obj-y := hyp.o > +extra-y := $(hyp-obj) hyp.tmp.o hyp.lds > + > +# 1) Compile all source files to `.hyp.o` object files. The file extension > +# avoids file name clashes for files shared with VHE. > +$(obj)/%.hyp.o: $(src)/%.c FORCE > $(call if_changed_rule,cc_o_c) > -$(obj)/%.hyp.tmp.o: $(src)/%.S FORCE > +$(obj)/%.hyp.o: $(src)/%.S FORCE > $(call if_changed_rule,as_o_S) > -$(obj)/%.hyp.o: $(obj)/%.hyp.tmp.o FORCE > - $(call if_changed,hypcopy) > > -# Disable reordering functions by GCC (enabled at -O2). > -# This pass puts functions into '.text.*' sections to aid the linker > -# in optimizing ELF layout. See HYPCOPY comment below for more info. > -ccflags-y += $(call cc-option,-fno-reorder-functions) > +# 2) Compile linker script. > +$(obj)/hyp.lds: $(src)/hyp.lds.S FORCE > + $(call if_changed_dep,cpp_lds_S) Why is it not sufficient just to list the linker script as a target, like we do for vmlinux.lds in extra-y? > +# 3) Partially link all '.hyp.o' files and apply the linker script. > +# Prefixes names of ELF sections with '.hyp', eg. '.hyp.text'. > +LDFLAGS_hyp.tmp.o := -r -T $(obj)/hyp.lds > +$(obj)/hyp.tmp.o: $(addprefix $(obj)/,$(hyp-obj)) $(obj)/hyp.lds FORCE > + $(call if_changed,ld) > + > +# 4) Produce the final 'hyp.o', ready to be linked into 'vmlinux'. > +# Prefixes names of ELF symbols with '__kvm_nvhe_'. > +$(obj)/hyp.o: $(obj)/hyp.tmp.o FORCE > + $(call if_changed,hypcopy) > > # The HYPCOPY command uses `objcopy` to prefix all ELF symbol names > -# and relevant ELF section names to avoid clashes with VHE code/data. > -# > -# Hyp code is assumed to be in the '.text' section of the input object > -# files (with the exception of specialized sections such as > -# '.hyp.idmap.text'). This assumption may be broken by a compiler that > -# divides code into sections like '.text.unlikely' so as to optimize > -# ELF layout. HYPCOPY checks that no such sections exist in the input > -# using `objdump`, otherwise they would be linked together with other > -# kernel code and not memory-mapped correctly at runtime. > +# to avoid clashes with VHE code/data. > quiet_cmd_hypcopy = HYPCOPY $@ > - cmd_hypcopy = \ > - if $(OBJDUMP) -h $< | grep -F '.text.'; then \ > - echo "$@: function reordering not supported in nVHE hyp code" >&2; \ > - /bin/false; \ > - fi; \ > - $(OBJCOPY) --prefix-symbols=__kvm_nvhe_ \ > - --rename-section=.text=.hyp.text \ > - $< $@ > + cmd_hypcopy = $(OBJCOPY) --prefix-symbols=__kvm_nvhe_ $< $@ > > # Remove ftrace and Shadow Call Stack CFLAGS. > # This is equivalent to the 'notrace' and '__noscs' annotations. > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp.lds.S b/arch/arm64/kvm/hyp/nvhe/hyp.lds.S > new file mode 100644 > index 000000000000..aaa0ce133a32 > --- /dev/null > +++ b/arch/arm64/kvm/hyp/nvhe/hyp.lds.S > @@ -0,0 +1,14 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Linker script used during partial linking of nVHE EL2 object files. > + * Written by David Brazdil > + */ > + > +/* > + * Defines an ELF hyp section from input section @NAME and its subsections. > + */ > +#define HYP_SECTION(NAME) .hyp##NAME : { *(NAME NAME##.[0-9a-zA-Z_]*) } Is 'NAME##.*' likely to cause a problem here? Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel