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 6E36EC3DA42 for ; Mon, 8 Jul 2024 07:38:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=+JO92pEYx4L3Ve/o9tRJ/weO2h9oGuUkMq71ljyT/1Q=; b=34WNrXH1crYniN6hChBRc25ilw XRu0mdHmNW5dkOQDqkZoJC/wrc7Oz0c25upNHM88ssO4H0G/1TvsSUIKQWKrk7Cz3wtPbvYu6Cd6H kmGipmv8KGbec0IFpj3/tOSjByrvwWK2MmFSDSBr7nk7v5jnIh+nq2sd8GG6Yqab+Q5N9ZEAZHApT CMjHD1E6jfxJMJyA9xv/PgLTW0zpDLfsRmcBas7HKYxUh2Tg9aXTA0K3dV0RKcjkeG6q3Yj4SwB4p XUXZ90esyNxRZ3u/W4ofAo0Nn4wBMe9AinSdXOu96Oi7LhfqAba7/4x2UDcu06vlMfd97ZtjVPRfV Ve6YcLXg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sQiwZ-0000000331z-38Gl; Mon, 08 Jul 2024 07:37:51 +0000 Received: from desiato.infradead.org ([2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sQiwM-000000032yq-0VWN for linux-arm-kernel@bombadil.infradead.org; Mon, 08 Jul 2024 07:37:38 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=+JO92pEYx4L3Ve/o9tRJ/weO2h9oGuUkMq71ljyT/1Q=; b=cp1GYETUXi0udWp/tFqovjeI+Q TxP2JiTPB4qQ6Z6Q3KqGLtXQ/2wJq4HmR1dCNjwquNSLdll5GLWvFgUnmNz/jE5bLSseM7V8dgCAX OPy7ikU/pMO46Yc/6dgsMqo+BiwvbsIjoZGG/xTHwwk4r0U9B4yAcYmFaFDjqqsRG39QtySIBKPlf brIIA+BwEKF7Hxhl647iBWyj0e2YDNCI30MWPkZPglVvhxJEtiwStjrB7yDE+KQUZBkWW6sHzj1MU t3qeseUAmGmuJY+HiuaSoAVdqepB8jVaaNWsDkEpM35ylIHnO9yvfrLlWYc+U4nq94qWSsO3LnOrS NsTRY3kQ==; Received: from j130084.upc-j.chello.nl ([24.132.130.84] helo=noisy.programming.kicks-ass.net) by desiato.infradead.org with esmtpsa (Exim 4.97.1 #2 (Red Hat Linux)) id 1sQivg-00000000ZEc-0Hnk; Mon, 08 Jul 2024 07:37:15 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id C0595300694; Mon, 8 Jul 2024 09:36:52 +0200 (CEST) Date: Mon, 8 Jul 2024 09:36:52 +0200 From: Peter Zijlstra To: Tiezhu Yang Cc: Will Deacon , Mark Rutland , Russell King , Catalin Marinas , Oleg Nesterov , Ingo Molnar , Arnaldo Carvalho de Melo , Namhyung Kim , linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/3] perf: Add perf_event_attr::bp_priv Message-ID: <20240708073652.GE11386@noisy.programming.kicks-ass.net> References: <20240621073910.8465-1-yangtiezhu@loongson.cn> <20240621073910.8465-2-yangtiezhu@loongson.cn> <20240705103413.GA8971@willie-the-truck> <7522fc14-aacc-a8e3-3258-9064d7e2936f@loongson.cn> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7522fc14-aacc-a8e3-3258-9064d7e2936f@loongson.cn> 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sat, Jul 06, 2024 at 01:31:03PM +0800, Tiezhu Yang wrote: > > > On 07/05/2024 06:34 PM, Will Deacon wrote: > > On Fri, Jun 21, 2024 at 03:39:08PM +0800, Tiezhu Yang wrote: > > > Add a member "bp_priv" at the end of the uapi struct perf_event_attr > > > to make a bridge between ptrace and hardware breakpoint. > > > > > > This is preparation for later patch on some archs such as ARM, ARM64 > > > and LoongArch which have privilege level of breakpoint. > > > > > > Signed-off-by: Tiezhu Yang > > > --- > > > include/uapi/linux/perf_event.h | 3 +++ > > > kernel/events/hw_breakpoint.c | 1 + > > > 2 files changed, 4 insertions(+) > > > > > > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h > > > index 3a64499b0f5d..f9f917e854e6 100644 > > > --- a/include/uapi/linux/perf_event.h > > > +++ b/include/uapi/linux/perf_event.h > > > @@ -379,6 +379,7 @@ enum perf_event_read_format { > > > #define PERF_ATTR_SIZE_VER6 120 /* add: aux_sample_size */ > > > #define PERF_ATTR_SIZE_VER7 128 /* add: sig_data */ > > > #define PERF_ATTR_SIZE_VER8 136 /* add: config3 */ > > > +#define PERF_ATTR_SIZE_VER9 144 /* add: bp_priv */ > > > > > > /* > > > * Hardware event_id to monitor via a performance monitoring event: > > > @@ -522,6 +523,8 @@ struct perf_event_attr { > > > __u64 sig_data; > > > > > > __u64 config3; /* extension of config2 */ > > > + > > > + __u8 bp_priv; /* privilege level of breakpoint */ > > > }; > > > > Why are we extending the user ABI for this? Perf events already have the > > privilege encoded (indirectly) by the exclude_{user,kernel,hv} fields in > > 'struct perf_event_attr'. > > IMO, add bp_priv is to keep consistent with the other fields > bp_type, bp_addr and bp_len, the meaning of bp_priv field is > explicit and different with exclude_{user,kernel,hv} fields. In case it wasn't obvious, this structure has __u64 granularity. You don't just add a __u8 to the end. Also, since you mention consistency, you might have noticed those other bp_ fields are in a union on config[12], so why can't this live in a union on config3 ? > Additionally, there is only 1 bit for exclude_{user,kernel,hv}, > but bp_priv field has at least 2 bit according to the explanation > of Arm Reference Manual. At last, the initial aim is to remove > the check condition to assign the value of hw->ctrl.privilege. > > https://developer.arm.com/documentation/ddi0487/latest/ > > 1. D23: AArch64 System Register Descriptions (Page 8562) > D23.3.11 DBGWCR_EL1, Debug Watchpoint Control Registers, n = 0 - 63 > PAC, bits [2:1] > Privilege of access control. Determines the Exception level or levels at > which a Watchpoint debug > event for watchpoint n is generated. > > 2. G8: AArch32 System Register Descriptions (Page 12334) > G8.3.26 DBGWCR, Debug Watchpoint Control Registers, n = 0 - 15 > PAC, bits [2:1] > Privilege of access control. Determines the Exception level or levels at > which a Watchpoint debug > event for watchpoint n is generated. That's all clear as mud for someone that don't speak arm. Can you please provide a coherent reason for all this that does not rely on external resources?