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=-5.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no 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 2EE10C433E0 for ; Thu, 18 Mar 2021 22:24:33 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 AB42064DC4 for ; Thu, 18 Mar 2021 22:24:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AB42064DC4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.microsoft.com 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=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:Cc:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=3Lc/4+h36Cj6cV0AW27WU5M6tKjJdKU4AwTvUhFQK3o=; b=Qpw87NE9ZLcdrV2bmnGPmEVmi feuPX4v28ozTUBG8J79mKk8/iypCMwiMIB4A68iGkYXBNb30ay5K6mki2WsVn8ZFiMKk1rBM+7Wwm C+YYJjj9LfFTGHmOq6lN6WB/HRBpxN4ht7f0Ee9Dy0ra/CnxYBpnvjQIV+I2J9JDfT5Fnf0PMEBIj DmkP/M0l+YA8t9JUv6TVO5B1s97YECMh05vKOrHID6GTvKJXwKe4PX1Od6CP2bTqL2V67pfNbxPTG aSwNJDbs5ZkdRzWekWB8lV5Oet6ydaiA3MygnnR8qjYHs2EDer0vnjnCEC5ULj7B8C6vv1vZgjP9c epgijGRLA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lN12g-00673P-9B; Thu, 18 Mar 2021 22:22:58 +0000 Received: from linux.microsoft.com ([13.77.154.182]) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lN12a-006732-Eb for linux-arm-kernel@lists.infradead.org; Thu, 18 Mar 2021 22:22:54 +0000 Received: from [192.168.254.32] (unknown [47.187.194.202]) by linux.microsoft.com (Postfix) with ESMTPSA id 47F42209C385; Thu, 18 Mar 2021 15:22:50 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 47F42209C385 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1616106170; bh=PxqNmQsDo2rsOIU7l0L48WnHUDlca3iv6Y4rPU2NjOg=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=dy+rDJewmHv0Jftz1A38quDC6GtDy/K2yVeTNmyD0ljxXwj4Eaq2HAlrt9QgTKmZz gfqxe863tIcam/QexESaStvgkdaax6Icj9dL2vHCrSXu20R3OVrQsFA2sFBsdVfkdU wVJiPYPLx6b0V8sPFLRrUM/fWdQKz6OZsh4YYJXw= Subject: Re: [RFC PATCH v2 2/8] arm64: Implement frame types To: Mark Brown Cc: mark.rutland@arm.com, jpoimboe@redhat.com, jthierry@redhat.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org References: <5997dfe8d261a3a543667b83c902883c1e4bd270> <20210315165800.5948-1-madvenka@linux.microsoft.com> <20210315165800.5948-3-madvenka@linux.microsoft.com> <20210318174029.GM5469@sirena.org.uk> From: "Madhavan T. Venkataraman" Message-ID: <6474b609-b624-f439-7bf7-61ce78ff7b83@linux.microsoft.com> Date: Thu, 18 Mar 2021 17:22:49 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210318174029.GM5469@sirena.org.uk> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210318_222252_954475_8363CD70 X-CRM114-Status: GOOD ( 34.39 ) 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 3/18/21 12:40 PM, Mark Brown wrote: > On Mon, Mar 15, 2021 at 11:57:54AM -0500, madvenka@linux.microsoft.com wrote: > >> To summarize, pt_regs->stackframe is used (or will be used) as a marker >> frame in stack traces. To enable the unwinder to detect these frames, tag >> each pt_regs->stackframe with a type. To record the type, use the unused2 >> field in struct pt_regs and rename it to frame_type. The types are: > > Unless I'm misreading what's going on here this is more trying to set a > type for the stack as a whole than for a specific stack frame. I'm also > finding this a bit confusing as the unwinder already tracks things it > calls frame types and it handles types that aren't covered here like > SDEI. At the very least there's a naming issue here. > When the unwinder gets to EL1 pt_regs->stackframe, it needs to be sure that it is indeed a frame inside an EL1 pt_regs structure. It performs the following checks: FP == pt_regs->regs[29] PC == pt_regs->pc type == EL1_FRAME to confirm that the frame is EL1 pt_regs->stackframe. Similarly, for EL0, the type is EL0_FRAME. Both these frames are on the task stack. So, it is not a stack type. > Taking a step back though do we want to be tracking this via pt_regs? > It's reliant on us robustly finding the correct pt_regs and on having > the things that make the stack unreliable explicitly go in and set the > appropriate type. That seems like it will be error prone, I'd been > expecting to do something more like using sections to filter code for > unreliable features based on the addresses of the functions we find on > the stack or similar. This could still go wrong of course but there's > fewer moving pieces, and especially fewer moving pieces specific to > reliable stack trace. > In that case, I suggest doing both. That is, check the type as well as specific functions. For instance, in the EL1 pt_regs, in addition to the above checks, check the PC against el1_sync(), el1_irq() and el1_error(). I have suggested this in the cover letter. If this is OK with you, we could do that. We want to make really sure that nothing goes wrong with detecting the exception frame. > I'm wary of tracking data that only ever gets used for the reliable > stack trace path given that it's going to be fairly infrequently used > and hence tested, especially things that only crop up in cases that are > hard to provoke reliably. If there's a way to detect things that > doesn't use special data that seems safer. > If you dislike the frame type, I could remove it and just do the following checks: FP == pt_regs->regs[29] PC == pt_regs->pc and the address check against el1_*() functions and similar changes for EL0 as well. I still think that the frame type check makes it more robust. >> EL1_FRAME >> EL1 exception frame. > > We do trap into EL2 as well, the patch will track EL2 frames as EL1 > frames. Even if we can treat them the same the naming ought to be > clear. > Are you referring to ARMv8.1 VHE extension where the kernel can run at EL2? Could you elaborate? I thought that EL2 was basically for Hypervisors. Thanks. >> FTRACE_FRAME >> FTRACE frame. > > This is implemented later in the series. If using this approach I'd > suggest pulling the change in entry-ftrace.S that sets this into this > patch, it's easier than adding a note about this being added later and > should help with any bisect issues. > OK. Good point. Madhavan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel