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 D8D70C433F5 for ; Thu, 14 Oct 2021 11:12:04 +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 A1D9260230 for ; Thu, 14 Oct 2021 11:12:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A1D9260230 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=OtB9LX5jYMu1pcBh0A97SliQd1UpgQ5nFp+Y8IOr6Uc=; b=xU7tMSkTsXV7bs C5/rWdogrMJbHTbvQtFOglWHzdwURPNcUzRtanIV40vBmu5Jo8Fvdw5ve/4x/clKhWumRHgJLJF97 G5ZZ1wOja8CIKNsbS1T8yOUjEYgg8iC6HM+dl/ph6T9FKbt5QDViKUwcpmP82rbe1lyllOfbvuHe/ gbZbUaeaB0kHgdOAD9oBROwndh4F1f5bVrYs4K5t7DRR2uEVGPGnqVacOc+ghSEfCIr2c4SkzRgPG hlAWejPDjXeJHI4fH6zhsQkicBvdc5BrhRU7njallRkLjls7KC8JYT2LrteiJ3pKUdIyw924rHst/ 1lWE1LQOoefbrBFV65sA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1maycQ-002mCp-OY; Thu, 14 Oct 2021 11:09:50 +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 1maycF-002mAU-Kj for linux-arm-kernel@lists.infradead.org; Thu, 14 Oct 2021 11:09:41 +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 958482F; Thu, 14 Oct 2021 04:09:37 -0700 (PDT) Received: from C02TD0UTHF1T.local (unknown [10.57.73.197]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A7C603F66F; Thu, 14 Oct 2021 04:09:30 -0700 (PDT) Date: Thu, 14 Oct 2021 12:09:27 +0100 From: Mark Rutland To: Robin Murphy 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, suzuki.poulose@arm.com, will@kernel.org Subject: Re: [PATCH 01/13] arm64: lib: __arch_clear_user(): fold fixups into body Message-ID: <20211014110927.GC13770@C02TD0UTHF1T.local> References: <20211013110059.10324-1-mark.rutland@arm.com> <20211013110059.10324-2-mark.rutland@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211014_040939_820510_BD777F43 X-CRM114-Status: GOOD ( 29.65 ) 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 Wed, Oct 13, 2021 at 08:55:31PM +0100, Robin Murphy wrote: > On 2021-10-13 12:00, Mark Rutland wrote: > > Like other functions, __arch_clear_user() places its exception fixups in > > the `.fixup` section without any clear association with > > __arch_clear_user() itself. If we backtrace the fixup code, it will be > > symbolized as an offset from the nearest prior symbol, which happens to > > be `__entry_tramp_text_end`. Further, since the PC adjustment for the > > fixup is akin to a direct branch rather than a function call, > > __arch_clear_user() itself will be missing from the backtrace. > > > > This is confusing and hinders debugging. In general this pattern will > > also be problematic for CONFIG_LIVEPATCH, since fixups often return to > > their associated function, but this isn't accurately captured in the > > stacktrace. > > > > To solve these issues for assembly functions, we must move fixups into > > the body of the functions themselves, after the usual fast-path returns. > > This patch does so for __arch_clear_user(). > > > > Inline assembly will be dealt with in subsequent patches. > > > > Other than the improved backtracing, there should be no functional > > change as a result of this patch. > > Oh, I always assumed the .fixup section might have some special significance > of its own. If not, all the better - modulo one possible nit below, this is > fine by me. Cheers, I'll go fix those up. > Also it's led me to see that copy_in_user() has finally left us, > hooray! Guess I've got no more excuses to put off that promised usercopy > rewrite other than finding the time now... ;) > > diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S > > index a7efb2ad2a1c..dac13df4a1ed 100644 > > --- a/arch/arm64/lib/clear_user.S > > +++ b/arch/arm64/lib/clear_user.S > > @@ -45,13 +45,10 @@ USER(9f, sttrh wzr, [x0]) > > USER(7f, sttrb wzr, [x2, #-1]) > > 5: mov x0, #0 > > ret > > -SYM_FUNC_END(__arch_clear_user) > > -EXPORT_SYMBOL(__arch_clear_user) > > - .section .fixup,"ax" > > The one useful purpose this did serve is to provide a handy visual cue - if > you have any more concrete reason to respin the series, would you mind > sticking in a little comment like "// Exception fixup" here and in the other > two assembly routines, just so it's harder to overlook that the preceding > ret is the normal exit path? I've added: | // Exception fixups ... to the start of the fixups in patches 1-3. > Either way, for patches 1-3, > > Acked-by: Robin Murphy Thanks! > I got totally lost trying to follow the _ASM_EXTABLE_UACCESS_* business, but > I don't think the rest of the series gets in the way of any outstanding > plans either (and FWIW I always thought "fixup->fixup" was pretty awful so > feel free to have an ack for patch #9 as well). Thanks again! > Indeed, I guess the new type field should mean that we can implement > "proper" address-aware fault handlers without the Itanium trick if we > still need to. Yes -- my thinking was that we can capture the address register(s) and offset(s) in the data field, and *also* provide the ESR and/or FAR if necessary in the exception handler. What's the Itanium trick? Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel