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 C3ED1EB64DD for ; Mon, 31 Jul 2023 02:34:08 +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:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=vDJYWDVpuPcS3ls1zz0NvcyUlmzaPn0IueglEXaClTk=; b=s2v9YDg+cwjB9I q7zix6XU0ZfaQPOabt+TAyYe8o75L+BYcS7r6ZZS7nstTvTChdR4iDy4z9e7W7pSbFPL8lgzT+//9 bteObLBOrJC+OgX5Y++zt7j+Oc8KNvEHN4Qtbf6ps4GJNHU3+Lg/CHeqyn+EuKsVavRwi+0uFZULR 6GFPlFmaiWizTWDXYFYIkJ/WHx7MWS89aLLz0720ZZYl4AzrW3raQfbaB38W47FaBZI6E0RCIPoNv nYeoHUXblVS80xULNgwWQTsn0oyCw3U93ppUwnZKwW0hcP3wtYBQUvGVbwGLAVxIQtCtAd8D4FFvg WmdKfxuD6ADldrQNnbYg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qQIjC-00DWpl-1Q; Mon, 31 Jul 2023 02:33:46 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qQIj9-00DWne-02 for linux-arm-kernel@lists.infradead.org; Mon, 31 Jul 2023 02:33:45 +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 118A01650; Sun, 30 Jul 2023 19:34:12 -0700 (PDT) Received: from [10.162.41.7] (a077893.blr.arm.com [10.162.41.7]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 34B663F6C4; Sun, 30 Jul 2023 19:33:23 -0700 (PDT) Message-ID: <937468a1-b325-7d05-8daf-765f911c9240@arm.com> Date: Mon, 31 Jul 2023 08:03:21 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH V13 - RESEND 02/10] arm64/perf: Add BRBE registers and fields Content-Language: en-US To: James Clark , Will Deacon Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, catalin.marinas@arm.com, mark.rutland@arm.com, Mark Brown , Rob Herring , Marc Zyngier , Suzuki Poulose , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , linux-perf-users@vger.kernel.org References: <20230711082455.215983-1-anshuman.khandual@arm.com> <20230711082455.215983-3-anshuman.khandual@arm.com> <20230728162011.GA22050@willie-the-truck> <89ce4bc4-00c5-a763-3179-e1d3e9f198b7@arm.com> From: Anshuman Khandual In-Reply-To: <89ce4bc4-00c5-a763-3179-e1d3e9f198b7@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230730_193343_168626_2DC53734 X-CRM114-Status: GOOD ( 26.26 ) 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 7/28/23 22:22, James Clark wrote: > > > On 28/07/2023 17:20, Will Deacon wrote: >> On Tue, Jul 11, 2023 at 01:54:47PM +0530, Anshuman Khandual wrote: >>> This adds BRBE related register definitions and various other related field >>> macros there in. These will be used subsequently in a BRBE driver which is >>> being added later on. >>> >>> Cc: Catalin Marinas >>> Cc: Will Deacon >>> Cc: Marc Zyngier >>> Cc: Mark Rutland >>> Cc: linux-arm-kernel@lists.infradead.org >>> Cc: linux-kernel@vger.kernel.org >>> Tested-by: James Clark >>> Reviewed-by: Mark Brown >>> Signed-off-by: Anshuman Khandual >>> --- >>> arch/arm64/include/asm/sysreg.h | 103 +++++++++++++++++++++ >>> arch/arm64/tools/sysreg | 158 ++++++++++++++++++++++++++++++++ >>> 2 files changed, 261 insertions(+) >>> >>> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h >>> index b481935e9314..f95e30c13c8b 100644 >>> --- a/arch/arm64/include/asm/sysreg.h >>> +++ b/arch/arm64/include/asm/sysreg.h >>> @@ -163,6 +163,109 @@ >>> #define SYS_DBGDTRTX_EL0 sys_reg(2, 3, 0, 5, 0) >>> #define SYS_DBGVCR32_EL2 sys_reg(2, 4, 0, 7, 0) >>> >>> +#define __SYS_BRBINFO(n) sys_reg(2, 1, 8, ((n) & 0xf), ((((n) & 0x10)) >> 2 + 0)) >>> +#define __SYS_BRBSRC(n) sys_reg(2, 1, 8, ((n) & 0xf), ((((n) & 0x10)) >> 2 + 1)) >>> +#define __SYS_BRBTGT(n) sys_reg(2, 1, 8, ((n) & 0xf), ((((n) & 0x10)) >> 2 + 2)) >> >> It's that time on a Friday but... aren't these macros busted? I think you >> need brackets before adding the offset, otherwise wouldn't, for example, >> target registers 0-15 all access info register 0 and __SYS_BRBTGT(16) would >> then start accessing source register 0? >> >> I'm surprised that the compiler doesn't warn about this, but even more >> surprised that you managed to test this. >> >> Please tell me I'm wrong! >> >> Will > > No I think you are right, it is wrong. Luckily there is already an > extraneous bracket so you you can fix it by moving one a place down: > > sys_reg(2, 1, 8, ((n) & 0xf), ((((n) & 0x10) >> 2) + 2)) > > It's interesting because the test [1] is doing quite a bit and looking > at the branch info, and that src and targets match up to function names. > I also manually looked at the branch buffers and didn't see anything > obviously wrong like things that looked like branch infos in the source > or target fields. Will have to take another look to see if it would be > possible for the test to catch this. > > James > > [1]: > https://gitlab.arm.com/linux-arm/linux-jc/-/commit/3a7ddce70c2daadb63fcc511de0a89055ca48b32 ((((n) & 0x10)) >> 2 + 2) ---> ((((n) & 0x10) >> 2) + 2) The additional brackets are useful in explicitly telling the compiler but what it the compiler is just doing the right thing implicitly i.e computing the shifting operation before doing the offset addition. During testing, all those captured branch records looked alright. But that is no excuse, for not doing the right thing to begin with i.e adding explicit brackets. I will fix these in next version. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel