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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F0FD7C433EF for ; Tue, 19 Oct 2021 13:03:19 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id C5CC661360 for ; Tue, 19 Oct 2021 13:03:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C5CC661360 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org 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:In-Reply-To:MIME-Version:References: 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=97i4d+1ci2R5eGx00OSQhqy6o1AJ/ExyaFm1jwx7u6E=; b=0UcIv1ASGvwZp2 FMQ5uvaGhto+cBqFPe/XlJegYSdFui8DOjiQy7KpBldrOaFQXYiPmKgYqIhCXbxfoc3uSLnN1k3k+ DJi5ilOZVzymscTgPvoLYgc/tfn4XWIu8GJGbBdTmhp2vFtXdew1tEw6u3KRpxa4bM56r2ZJJJdKm 4OHyPUa7fOSTy1N1icT0XCHOKXJJmVlc7s8zKMFvLZGkjiq71sWPPWKFSd1ByHFVOt9exrw3IZK8y AuSl4M5cQ+kg4JoqGStBBRzEi3BPNKjlBOYXABIpiZc9GIeLScYLZU0I5edhBE3R6VAy+qga6lpYS 2L4TnZMYIUhiNgubRNwg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mcokU-001LMD-9s; Tue, 19 Oct 2021 13:01:46 +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 1mcokP-001LL6-Qq for linux-arm-kernel@lists.infradead.org; Tue, 19 Oct 2021 13:01:43 +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 2354C2F; Tue, 19 Oct 2021 06:01:39 -0700 (PDT) Received: from lakrids.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 C3B193F70D; Tue, 19 Oct 2021 06:01:36 -0700 (PDT) Date: Tue, 19 Oct 2021 14:01:34 +0100 From: Mark Rutland To: Will Deacon Cc: linux-arm-kernel@lists.infradead.org, alexandru.elisei@arm.com, andrii@kernel.org, ardb@kernel.org, ast@kernel.org, broonie@kernel.org, catalin.marinas@arm.com, daniel@iogearbox.net, dvyukov@google.com, james.morse@arm.com, jean-philippe@linaro.org, jpoimboe@redhat.com, maz@kernel.org, peterz@infradead.org, robin.murphy@arm.com, suzuki.poulose@arm.com Subject: Re: [PATCH 10/13] arm64: extable: add `type` and `data` fields Message-ID: <20211019130134.GC941@lakrids.cambridge.arm.com> References: <20211013110059.10324-1-mark.rutland@arm.com> <20211013110059.10324-11-mark.rutland@arm.com> <20211019112954.GG13251@willie-the-truck> <20211019115022.GB941@lakrids.cambridge.arm.com> <20211019120505.GI13251@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20211019120505.GI13251@willie-the-truck> User-Agent: Mutt/1.11.1+11 (2f07cb52) (2018-12-01) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211019_060142_005686_6A06C2B5 X-CRM114-Status: GOOD ( 34.40 ) 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, Oct 19, 2021 at 01:05:06PM +0100, Will Deacon wrote: > On Tue, Oct 19, 2021 at 12:50:22PM +0100, Mark Rutland wrote: > > On Tue, Oct 19, 2021 at 12:29:55PM +0100, Will Deacon wrote: > > > On Wed, Oct 13, 2021 at 12:00:56PM +0100, Mark Rutland wrote: > > > > -#define __ASM_EXTABLE_RAW(insn, fixup) \ > > > > - .pushsection __ex_table, "a"; \ > > > > - .align 3; \ > > > > - .long ((insn) - .); \ > > > > - .long ((fixup) - .); \ > > > > +#define __ASM_EXTABLE_RAW(insn, fixup, type, data) \ > > > > + .pushsection __ex_table, "a"; \ > > > > + .align 2; \ > > > > + .long ((insn) - .); \ > > > > + .long ((fixup) - .); \ > > > > + .short (type); \ > > > > + .short (data); \ > > > > > > Why are you reducing the alignment here? > > > > That's because the size of each entry is now 12 bytes, and > > `.align 3` aligns to 8 bytes, which would leave a gap between entries. > > We only require the fields are naturally aligned, so `.align 2` is > > sufficient, and doesn't waste space. > > > > I'll update the commit message to call that out. > > I think the part which is confusing me is that I would expect the alignment > here to match the alignment of the corresponding C type, but the old value > of '3' doesn't seem to do that, so is this patch fixing an earlier bug? > > Without your patches in the picture, we're using a '.align 3' in > _asm_extable, but with: > > struct exception_table_entry > { > int insn, fixup; > }; > > I suppose it works out because that over-alignment doesn't result in any > additional padding, but I think we could reduce the current alignment > without any of these other changes, no? Yes, we could reduce that first, but no, it's not a bug -- there's no functional issue today. For context, today the `__ex_table` section as a whole and the `__start___ex_table` symbol also got 8 byte alignment, since in ARch/arm64/kernel/vmlinux.lds.S we have: | #define RO_EXCEPTION_TABLE_ALIGN 8 ... and so in include/asm-generic/vmlinux.lds.h when the exception table gets output with: | EXCEPTION_TABLE(RO_EXCEPTION_TABLE_ALIGN) | #define EXCEPTION_TABLE(align) \ | . = ALIGN(align); \ | __ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) { \ | __start___ex_table = .; \ | KEEP(*(__ex_table)) \ | __stop___ex_table = .; \ | } If you want, I can split out a preparatory patch which drops the alignment to the minimum necessary, both in the asm and for RO_EXCEPTION_TABLE_ALIGN? [...] > > > > +static void arm64_sort_relative_table(char *extab_image, int image_size) > > > > +{ > > > > + int i = 0; > > > > + > > > > + while (i < image_size) { > > > > + uint32_t *loc = (uint32_t *)(extab_image + i); > > > > + > > > > + w(r(loc) + i, loc); > > > > + w(r(loc + 1) + i + 4, loc + 1); > > > > + /* Don't touch the fixup type or data */ > > > > + > > > > + i += sizeof(uint32_t) * 3; > > > > + } > > > > + > > > > + qsort(extab_image, image_size / 12, 12, compare_relative_table); > > > > + > > > > + i = 0; > > > > + while (i < image_size) { > > > > + uint32_t *loc = (uint32_t *)(extab_image + i); > > > > + > > > > + w(r(loc) - i, loc); > > > > + w(r(loc + 1) - (i + 4), loc + 1); > > > > + /* Don't touch the fixup type or data */ > > > > + > > > > + i += sizeof(uint32_t) * 3; > > > > + } > > > > +} > > > > > > This is very nearly a direct copy of x86_sort_relative_table() (magic > > > numbers and all). It would be nice to tidy that up, but I couldn't > > > immediately see a good way to do it :( > > > > Beware that's true in linux-next, but not mainline, as that changes in > > commit: > > > > 46d28947d9876fc0 ("x86/extable: Rework the exception table mechanics") > > > > A patch to unify the two is trivial, but will cause a cross-tree > > dependency, so I'd suggest having this separate for now and sending a > > unification patch come -rc1. > > > > I can note something to that effect in the commit message, if that > > helps? > > Yeah, I suppose. It's not worth tripping over the x86 changes, but we > should try to remember to come back and unify things. Sure; works for me. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel