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 BA3DCC27C5E for ; Mon, 10 Jun 2024 11:55:32 +0000 (UTC) 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=Lh64SIehflcJSYh6byvvgc33eXLU0mdcj/shoCwFipk=; b=a3ToCrWsMWmXuW Q0yUekuiFKhEv9/UjAjtZlW9A+j/wbN1mAYPxhekMc2MLBnePNoLvUbrsrNnLXhBcFinhyU3B15Zz JkJaMLJNwWQzL00F8oG0nvTUyRubjyu7vNIMAwcvU40BpJOTuwvtV1V9W4bueDFCOMuURDIIplcuW g28NGTXq2taACeTmlF6O0/pZBxDfQGxLF0RM6xK6NCnNDAtYVEIehfb0LWaln8YrIlIj4qaA7dIS9 iuxizQo2/WQa1Nrpdl0unYKX5Wp2RWZIgumy2ao3eih0b9DIY3dqREbauoknPLs3RvSMc03l7WPWi RII7x5lEqxB9qM4sKwJA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sGdcM-00000004slM-18zt; Mon, 10 Jun 2024 11:55:18 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sGdcI-00000004sk8-3r1f for linux-arm-kernel@lists.infradead.org; Mon, 10 Jun 2024 11:55:16 +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 B3BD112FC; Mon, 10 Jun 2024 04:55:38 -0700 (PDT) Received: from J2N7QTR9R3 (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D8F903F58B; Mon, 10 Jun 2024 04:55:10 -0700 (PDT) Date: Mon, 10 Jun 2024 12:55:08 +0100 From: Mark Rutland To: "Rob Herring (Arm)" Cc: Russell King , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Namhyung Kim , Alexander Shishkin , Jiri Olsa , Ian Rogers , Adrian Hunter , Will Deacon , Marc Zyngier , Oliver Upton , James Morse , Suzuki K Poulose , Zenghui Yu , Catalin Marinas , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, kvmarm@lists.linux.dev Subject: Re: [PATCH 9/9] perf: arm_pmuv3: Add support for Armv9.4 PMU instruction counter Message-ID: References: <20240607-arm-pmu-3-9-icntr-v1-0-c7bd2dceff3b@kernel.org> <20240607-arm-pmu-3-9-icntr-v1-9-c7bd2dceff3b@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240607-arm-pmu-3-9-icntr-v1-9-c7bd2dceff3b@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240610_045515_083559_076F6A7B X-CRM114-Status: GOOD ( 28.79 ) 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 Fri, Jun 07, 2024 at 02:31:34PM -0600, Rob Herring (Arm) wrote: > Armv9.4/8.9 PMU adds optional support for a fixed instruction counter > similar to the fixed cycle counter. Support for the feature is indicated > in the ID_AA64DFR1_EL1 register PMICNTR field. The counter is not > accessible in AArch32. > > Existing userspace using direct counter access won't know how to handle > the fixed instruction counter, so we have to avoid using the counter > when user access is requested. > > Signed-off-by: Rob Herring (Arm) > --- > arch/arm/include/asm/arm_pmuv3.h | 21 +++++++++++++++++++++ > arch/arm64/include/asm/arm_pmuv3.h | 29 +++++++++++++++++++++++++++++ > arch/arm64/kvm/pmu.c | 8 ++++++-- > arch/arm64/tools/sysreg | 25 +++++++++++++++++++++++++ > drivers/perf/arm_pmuv3.c | 28 ++++++++++++++++++++++++++-- > include/linux/perf/arm_pmu.h | 8 ++++++-- > include/linux/perf/arm_pmuv3.h | 4 +++- > 7 files changed, 116 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/include/asm/arm_pmuv3.h b/arch/arm/include/asm/arm_pmuv3.h > index ac2cf37b57e3..b836537ddfbf 100644 > --- a/arch/arm/include/asm/arm_pmuv3.h > +++ b/arch/arm/include/asm/arm_pmuv3.h > @@ -10,6 +10,7 @@ > #include > > #define ARMV8_PMU_CYCLE_IDX 31 > +#define ARMV8_PMU_INSTR_IDX 32 /* Not accessible from AArch32 */ As with ARMV8_PMU_CYCLE_IDX, I reckon this should live in (with the comment above) so that we don't need separate definitions for arm & arm64. [...] > diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg > index 231817a379b5..8ab6e09871de 100644 > --- a/arch/arm64/tools/sysreg > +++ b/arch/arm64/tools/sysreg > @@ -2029,6 +2029,31 @@ Sysreg FAR_EL1 3 0 6 0 0 > Field 63:0 ADDR > EndSysreg > > +Sysreg PMICNTR_EL0 3 3 9 4 0 > +Field 63:0 ICNT > +EndSysreg LGTM per ARM DDI 0487K.a, section D23.5.15, pages 8989 to 8992. > + > +Sysreg PMICFILTR_EL0 3 3 9 6 0 > +Res0 63:59 > +Field 58 SYNC > +Field 57:56 VS The 'VS' field doesn't seem to be in the ARM ARM (ARM DDI 0487K.a); is that defined in a supplement? > +Res0 55:32 > +Field 31 P > +Field 30 U > +Field 29 NSK > +Field 28 NSU > +Field 27 NSH > +Field 26 M > +Res0 25 > +Field 24 SH > +Field 23 T > +Field 22 RLK > +Field 21 RLU > +Field 20 RLH > +Res0 19:16 > +Field 15:0 evtCount > +EndSysreg Aside from 'VS', this LGTM per ARM DDI 0487K.a, section D23.5.14, pages 8981 to 8988. One important thing to note is that this doesn't have the threshold controls (TC, TE, TH); so if threshold events make sense for instruction events, we cannot place those in the dedicated isntruction counter. [...] > @@ -931,6 +939,18 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc, > return -EAGAIN; > } > > + /* > + * Always prefer to place a instruction counter into the instruction counter, > + * but don't expose the instruction counter to userspace access as userspace > + * may not know how to handle it. > + */ > + if (test_bit(ARMV8_PMU_INSTR_IDX, cpu_pmu->cntr_mask) && > + (evtype == ARMV8_PMUV3_PERFCTR_INST_RETIRED) && > + !armv8pmu_event_want_user_access(event)) { > + if (!test_and_set_bit(ARMV8_PMU_INSTR_IDX, cpuc->used_mask)) > + return ARMV8_PMU_INSTR_IDX; > + } I reckon this'd be a bit clearer if we check the evtype first, as with cycles, e.g. /* * Always prefer to place a instruction counter into the instruction counter, * but don't expose the instruction counter to userspace access as userspace * may not know how to handle it. */ if (evtype == ARMV8_PMUV3_PERFCTR_INST_RETIRED) { if (test_bit(ARMV8_PMU_INSTR_IDX, cpu_pmu->cntr_mask) && !armv8pmu_event_want_user_access(event) && !test_and_set_bit(ARMV8_PMU_INSTR_IDX, cpuc->used_mask)) return ARMV8_PMU_INSTR_IDX; } As above, we might need to check for threshold controls, but I'll need to go page that in. Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel