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 9B110C05027 for ; Thu, 9 Feb 2023 10:09:50 +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=ARloe6wK87aQeMguwvmG794fZvBuNVe1R+EJbGeC7bk=; b=2GQ1pZYWa3bV34 JHK+nCA2P14C61ka7cfBKV5utq1MZNm6HTRPWYhMtkm63Qr4CGY1q7xbooeNhr3YKEvHLA5Z7z85T y/gGKeeKHyxNc1U/yeo9B8HajPNlE+K6yYU/YAoepexSzjC4DfKDpq+wG6D7414iNmNkC61b55moM eECCOJKPmtlmmblZ42qrPnnnrlf6iuAoZCylSNpiPL65ovSp475onphMWi73xhm+eJWGl3ohg+dyx MORjQsqHTLfnk904f3haiB0+jbLMrrn50c6J4malK/PhfPt3F9Ddo+SIond5IYevD10hFA+z4VRZE GXzhR3oBWdXnPbdvG8IA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pQ3rJ-000yXG-LZ; Thu, 09 Feb 2023 10:08:53 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pQ3rF-000yWK-VK for linux-arm-kernel@lists.infradead.org; Thu, 09 Feb 2023 10:08:52 +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 BC3EE1515; Thu, 9 Feb 2023 02:09:28 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.89.57]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 07AB23F71E; Thu, 9 Feb 2023 02:08:44 -0800 (PST) Date: Thu, 9 Feb 2023 10:08:42 +0000 From: Mark Rutland To: Anshuman Khandual Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Catalin Marinas , Will Deacon , Marc Zyngier , Mark Brown Subject: Re: [PATCH V7 2/6] arm64/perf: Add BRBE registers and fields Message-ID: References: <20230105031039.207972-1-anshuman.khandual@arm.com> <20230105031039.207972-3-anshuman.khandual@arm.com> <16dcb986-74df-9a78-5cfc-e9f59fbe0997@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230209_020850_126402_23DD5424 X-CRM114-Status: GOOD ( 43.28 ) 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 Thu, Feb 09, 2023 at 11:19:04AM +0530, Anshuman Khandual wrote: > On 2/9/23 00:52, Mark Rutland wrote: > > I'd prefer that we fix gen-sysreg.awk to support Enum blocks within > > SysregFields blocks (patch below), then use SysregFields as described above. > > The following patch did not apply cleanly on v6.2-rc7 but eventually did so after > some changes. Is the patch against mainline or arm64-next ? Sorry I forgot to say: that needs the arm64 for-next/sysreg-hwcaps branch (which is merged into arm64 for-next/core). > Nonetheless, it does > solve the enum problem for SysregFields. With this patch in place, I was able to > > - Change Sysreg BRBINF_EL1 as SysregFields BRBINFx_EL1 > - Change BRBINF_EL1_XXX fields usage as BRBINFx_EL1_XXX fields Nice! > Should I take this patch with this series as an initial prerequisite patch or you > would like to post this now for current merge window ? I think for now it's best to add it to this series as a prerequisite. Thanks, Mark. > > > > > Thanks, > > Mark. > > > > ---->8---- > >>From 0c194d92b0b9ff3b32f666a4610b077fdf1b4b93 Mon Sep 17 00:00:00 2001 > > From: Mark Rutland > > Date: Wed, 8 Feb 2023 17:55:08 +0000 > > Subject: [PATCH] arm64/sysreg: allow *Enum blocks in SysregFields blocks > > > > We'd like to support Enum/SignedEnum/UnsignedEnum blocks within > > SysregFields blocks, so that we can define enumerations for sets of > > registers. This isn't currently supported by gen-sysreg.awk due to the > > way we track the active block, which can't handle more than a single > > layer of nesting, which imposes an awkward requirement that when ending > > a block we know what the parent block is when calling change_block() > > > > Make this nicer by using a stack of active blocks, with block_push() to > > start a block, and block_pop() to end a block. Doing so means hat when > > ending a block we don't need to know the parent block type, and checks > > of the active block become more consistent. On top of that, it's easy to > > permit *Enum blocks within both Sysreg and SysregFields blocks. > > > > To aid debugging, the stack of active blocks is reported for fatal > > errors, and an error is raised if the file is terminated without ending > > the active block. For clarity I've renamed the top-level element from > > "None" to "Root". > > > > The Fields element it intended only for use within Systeg blocks, and > > does not make sense within SysregFields blocks, and so remains forbidden > > within a SysregFields block. > > > > Signed-off-by: Mark Rutland > > Cc: Anshuman Khandual > > Cc: Catalin Marinas > > Cc: Mark Brown > > Cc: Will Deacon > > --- > > arch/arm64/tools/gen-sysreg.awk | 93 ++++++++++++++++++++------------- > > 1 file changed, 57 insertions(+), 36 deletions(-) > > > > diff --git a/arch/arm64/tools/gen-sysreg.awk b/arch/arm64/tools/gen-sysreg.awk > > index 7f27d66a17e1..066ebf5410fa 100755 > > --- a/arch/arm64/tools/gen-sysreg.awk > > +++ b/arch/arm64/tools/gen-sysreg.awk > > @@ -4,23 +4,35 @@ > > # > > # Usage: awk -f gen-sysreg.awk sysregs.txt > > > > +function block_current() { > > + return __current_block[__current_block_depth]; > > +} > > + > > # Log an error and terminate > > function fatal(msg) { > > print "Error at " NR ": " msg > "/dev/stderr" > > + > > + printf "Current block nesting:" > > + > > + for (i = 0; i <= __current_block_depth; i++) { > > + printf " " __current_block[i] > > + } > > + printf "\n" > > + > > exit 1 > > } > > > > -# Sanity check that the start or end of a block makes sense at this point in > > -# the file. If not, produce an error and terminate. > > -# > > -# @this - the $Block or $EndBlock > > -# @prev - the only valid block to already be in (value of @block) > > -# @new - the new value of @block > > -function change_block(this, prev, new) { > > - if (block != prev) > > - fatal("unexpected " this " (inside " block ")") > > - > > - block = new > > +# Enter a new block, setting the active block to @block > > +function block_push(block) { > > + __current_block[++__current_block_depth] = block > > +} > > + > > +# Exit a block, setting the active block to the parent block > > +function block_pop() { > > + if (__current_block_depth == 0) > > + fatal("error: block_pop() in root block") > > + > > + __current_block_depth--; > > } > > > > # Sanity check the number of records for a field makes sense. If not, produce > > @@ -84,10 +96,14 @@ BEGIN { > > print "/* Generated file - do not edit */" > > print "" > > > > - block = "None" > > + __current_block_depth = 0 > > + __current_block[__current_block_depth] = "Root" > > } > > > > END { > > + if (__current_block_depth != 0) > > + fatal("Missing terminator for " block_current() " block") > > + > > print "#endif /* __ASM_SYSREG_DEFS_H */" > > } > > > > @@ -95,8 +111,9 @@ END { > > /^$/ { next } > > /^[\t ]*#/ { next } > > > > -/^SysregFields/ { > > - change_block("SysregFields", "None", "SysregFields") > > +/^SysregFields/ && block_current() == "Root" { > > + block_push("SysregFields") > > + > > expect_fields(2) > > > > reg = $2 > > @@ -109,12 +126,10 @@ END { > > next > > } > > > > -/^EndSysregFields/ { > > +/^EndSysregFields/ && block_current() == "SysregFields" { > > if (next_bit > 0) > > fatal("Unspecified bits in " reg) > > > > - change_block("EndSysregFields", "SysregFields", "None") > > - > > define(reg "_RES0", "(" res0 ")") > > define(reg "_RES1", "(" res1 ")") > > print "" > > @@ -123,11 +138,13 @@ END { > > res0 = null > > res1 = null > > > > + block_pop() > > next > > } > > > > -/^Sysreg/ { > > - change_block("Sysreg", "None", "Sysreg") > > +/^Sysreg/ && block_current() == "Root" { > > + block_push("Sysreg") > > + > > expect_fields(7) > > > > reg = $2 > > @@ -156,12 +173,10 @@ END { > > next > > } > > > > -/^EndSysreg/ { > > +/^EndSysreg/ && block_current() == "Sysreg" { > > if (next_bit > 0) > > fatal("Unspecified bits in " reg) > > > > - change_block("EndSysreg", "Sysreg", "None") > > - > > if (res0 != null) > > define(reg "_RES0", "(" res0 ")") > > if (res1 != null) > > @@ -178,12 +193,13 @@ END { > > res0 = null > > res1 = null > > > > + block_pop() > > next > > } > > > > # Currently this is effectivey a comment, in future we may want to emit > > # defines for the fields. > > -/^Fields/ && (block == "Sysreg") { > > +/^Fields/ && block_current() == "Sysreg" { > > expect_fields(2) > > > > if (next_bit != 63) > > @@ -200,7 +216,7 @@ END { > > } > > > > > > -/^Res0/ && (block == "Sysreg" || block == "SysregFields") { > > +/^Res0/ && (block_current() == "Sysreg" || block_current() == "SysregFields") { > > expect_fields(2) > > parse_bitdef(reg, "RES0", $2) > > field = "RES0_" msb "_" lsb > > @@ -210,7 +226,7 @@ END { > > next > > } > > > > -/^Res1/ && (block == "Sysreg" || block == "SysregFields") { > > +/^Res1/ && (block_current() == "Sysreg" || block_current() == "SysregFields") { > > expect_fields(2) > > parse_bitdef(reg, "RES1", $2) > > field = "RES1_" msb "_" lsb > > @@ -220,7 +236,7 @@ END { > > next > > } > > > > -/^Field/ && (block == "Sysreg" || block == "SysregFields") { > > +/^Field/ && (block_current() == "Sysreg" || block_current() == "SysregFields") { > > expect_fields(3) > > field = $3 > > parse_bitdef(reg, field, $2) > > @@ -231,15 +247,16 @@ END { > > next > > } > > > > -/^Raz/ && (block == "Sysreg" || block == "SysregFields") { > > +/^Raz/ && (block_current() == "Sysreg" || block_current() == "SysregFields") { > > expect_fields(2) > > parse_bitdef(reg, field, $2) > > > > next > > } > > > > -/^SignedEnum/ { > > - change_block("Enum<", "Sysreg", "Enum") > > +/^SignedEnum/ && (block_current() == "Sysreg" || block_current() == "SysregFields") { > > + block_push("Enum") > > + > > expect_fields(3) > > field = $3 > > parse_bitdef(reg, field, $2) > > @@ -250,8 +267,9 @@ END { > > next > > } > > > > -/^UnsignedEnum/ { > > - change_block("Enum<", "Sysreg", "Enum") > > +/^UnsignedEnum/ && (block_current() == "Sysreg" || block_current() == "SysregFields") { > > + block_push("Enum") > > + > > expect_fields(3) > > field = $3 > > parse_bitdef(reg, field, $2) > > @@ -262,8 +280,9 @@ END { > > next > > } > > > > -/^Enum/ { > > - change_block("Enum", "Sysreg", "Enum") > > +/^Enum/ && (block_current() == "Sysreg" || block_current() == "SysregFields") { > > + block_push("Enum") > > + > > expect_fields(3) > > field = $3 > > parse_bitdef(reg, field, $2) > > @@ -273,16 +292,18 @@ END { > > next > > } > > > > -/^EndEnum/ { > > - change_block("EndEnum", "Enum", "Sysreg") > > +/^EndEnum/ && block_current() == "Enum" { > > + > > field = null > > msb = null > > lsb = null > > print "" > > + > > + block_pop() > > next > > } > > > > -/0b[01]+/ && block == "Enum" { > > +/0b[01]+/ && block_current() == "Enum" { > > expect_fields(2) > > val = $1 > > name = $2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel