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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1680ACDB46E for ; Thu, 12 Oct 2023 10:58:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1378005AbjJLK6h (ORCPT ); Thu, 12 Oct 2023 06:58:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47918 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1347143AbjJLK6f (ORCPT ); Thu, 12 Oct 2023 06:58:35 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id DCB60B7 for ; Thu, 12 Oct 2023 03:58:33 -0700 (PDT) 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 3A46213D5; Thu, 12 Oct 2023 03:59:14 -0700 (PDT) Received: from [10.163.62.137] (unknown [10.163.62.137]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6E2053F762; Thu, 12 Oct 2023 03:58:31 -0700 (PDT) Message-ID: <999f01f4-4f3f-4a55-ae43-7316c46581c5@arm.com> Date: Thu, 12 Oct 2023 16:28:28 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] driver: perf: arm_pmuv3: Read PMMIR_EL1 unconditionally To: Mark Rutland , James Clark Cc: linux-arm-kernel@lists.infradead.org, zhangshaokun@hisilicon.com, Will Deacon , linux-kernel@vger.kernel.org References: <20231009075631.193208-1-anshuman.khandual@arm.com> Content-Language: en-US From: Anshuman Khandual In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/12/23 14:54, Mark Rutland wrote: > On Mon, Oct 09, 2023 at 09:59:19AM +0100, James Clark wrote: >> On 09/10/2023 08:56, Anshuman Khandual wrote: >>> PMMIR_EL1 needs to be captured in 'armpmu->reg_pmmir', for all appropriate >>> PMU version implementations where the register is available and reading it >>> is valid . Hence checking for bus slot event presence is redundant and can >>> be dropped. >>> >>> Cc: Will Deacon >>> Cc: Mark Rutland >>> Cc: linux-arm-kernel@lists.infradead.org >>> Cc: linux-kernel@vger.kernel.org >>> Signed-off-by: Anshuman Khandual >>> --- >>> This applies on v6.6-rc5. >>> >>> drivers/perf/arm_pmuv3.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c >>> index 1e72b486c033..9fc1b6da5106 100644 >>> --- a/drivers/perf/arm_pmuv3.c >>> +++ b/drivers/perf/arm_pmuv3.c >>> @@ -1129,7 +1129,7 @@ static void __armv8pmu_probe_pmu(void *info) >>> pmceid, ARMV8_PMUV3_MAX_COMMON_EVENTS); >>> >>> /* store PMMIR register for sysfs */ >>> - if (is_pmuv3p4(pmuver) && (pmceid_raw[1] & BIT(31))) >>> + if (is_pmuv3p4(pmuver)) >>> cpu_pmu->reg_pmmir = read_pmmir(); >>> else >>> cpu_pmu->reg_pmmir = 0; >> >> >> This does have the side effect of showing non-zero values in caps/slots >> even when the STALL_SLOT event isn't implemented. I think that's the >> scenario that the original commit (f5be3a61fd) was trying to avoid: >> >> /sys/bus/event_source/devices/armv8_pmuv3_0/caps/slots is exposed >> under sysfs. [If] Both ARMv8.4-PMU and STALL_SLOT event are >> implemented, it returns the slots from PMMIR_EL1, otherwise it will >> return 0. > > We check for the STALL_SLOT event becuase (at the time) the ARM ARM said: > > | If STALL_SLOT is not implemented, it is IMPLEMENTATION DEFINED whether the > | PMMIR System registers are implemented. > > ... and this was necessary to avoid triggering an UNDEFINED exception if we > attempted to read PMMIR on a CPU which didn't actually implement it. > > See: > > https://lore.kernel.org/linux-arm-kernel/20200720101518.GA11516@willie-the-truck/ > https://lore.kernel.org/linux-arm-kernel/20200720105019.GA54220@C02TD0UTHF1T.local/ > https://lore.kernel.org/linux-arm-kernel/20200720105410.GD11516@willie-the-truck/ > > As I promised in that thread, I did raise that with our architects. According > to the bug I filed against the architecture, this was tightened such that > ARMv8.4-PMU gauaranteed the presence of PMMIR, and that should have changed > between the G.a and G.b releases of the ARM ARM. > > Anshuman, can you go and check that the wording did chaange between G.a and G.b? G.a was the last ARM ARM version to have that STALL_SLOT event dependency for PMMIR_EL1 system register which got dropped in G.b version ARM ARM. > > Assuming it did (and the wording in the latest J.a release is also fine), > please update the commit message to describe the history above. Sure, will update the commit message as follows. driver: perf: arm_pmuv3: Read PMMIR_EL1 unconditionally PMMIR_EL1 needs to be captured in 'armpmu->reg_pmmir', for all appropriate PMU version implementations i.e FEAT_PMUv3p4 onward, where the register is available and reading it is valid. However STALL_SLOT event dependency was included earlier as per previous version ARM ARM wordings [1]. But later i.e ARM ARM version G.a onward, STALL_SLOT event dependency for PMMIR_EL1 register has been dropped. Checking for that dependency is now redundant, and should be dropped. [1] https://lore.kernel.org/linux-arm-kernel/20200720105410.GD11516@willie-the-truck/