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 BCEA1E77180 for ; Wed, 11 Dec 2024 04:34:15 +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:Content-Transfer-Encoding: Content-Type: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=X95sJKvPhcWFXLDCAdCAwQ6cpcsYRSvmBY73jJj6TgE=; b=L3BOgFoxDsq81yR3BCMuUYON66 Zs+l8xdESkdatm8wkzxu3EyT71jqWE9wyuy0ilQYAGp7ra1FzQqU36SI3YazVBHuF/a6Zmga9c/mT Nr0uU2KuxJqFPK0PnSGSNZwNBGjvVyJAXzPGhFtW7Dtf+htPG7T/8w7guckXgJMbfrxya9RLR+XKw V95H+k/BvMYvv0r23ENYEK58V3G4+JdJLVh2y73fY6XhtGITaQ1gZc8ZCK7IjCloyrmMuRwZs5JKV tQlh8zR8XQDmCtgJb+4r5KSMoeVrshmTwEnpZ3clJx0ET0ezTiFbtEctliIc+PURznZYMu4y9w4Zv ANG1hPrg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tLEQ9-0000000Dl5i-1urE; Wed, 11 Dec 2024 04:33:57 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tLEP6-0000000Dkxb-12mJ for linux-arm-kernel@lists.infradead.org; Wed, 11 Dec 2024 04:32:53 +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 9AD7B1007; Tue, 10 Dec 2024 20:33:16 -0800 (PST) Received: from [10.162.16.49] (a077893.blr.arm.com [10.162.16.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 287173F720; Tue, 10 Dec 2024 20:32:44 -0800 (PST) Message-ID: Date: Wed, 11 Dec 2024 10:02:42 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH V2 5/7] arm64/cpufeature: Add field details for ID_AA64DFR1_EL1 register To: Will Deacon Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Jonathan Corbet , Marc Zyngier , Oliver Upton , James Morse , Suzuki K Poulose , Catalin Marinas , Mark Brown , Mark Rutland , kvmarm@lists.linux.dev References: <20241028053426.2486633-1-anshuman.khandual@arm.com> <20241028053426.2486633-6-anshuman.khandual@arm.com> <20241210164144.GA16039@willie-the-truck> Content-Language: en-US From: Anshuman Khandual In-Reply-To: <20241210164144.GA16039@willie-the-truck> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241210_203252_383303_FFBA389C X-CRM114-Status: GOOD ( 24.62 ) 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 12/10/24 22:11, Will Deacon wrote: > On Mon, Oct 28, 2024 at 11:04:24AM +0530, Anshuman Khandual wrote: >> This adds required field details for ID_AA64DFR1_EL1, and also drops dummy >> ftr_raz[] array which is now redundant. These register fields will be used >> to enable increased breakpoint and watchpoint registers via FEAT_Debugv8p9 >> later. >> >> Cc: Catalin Marinas >> Cc: Will Deacon >> cc: Mark Brown >> Cc: Mark Rutland >> Cc: Marc Zyngier >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual >> --- >> arch/arm64/kernel/cpufeature.c | 21 ++++++++++++++++----- >> 1 file changed, 16 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c >> index 718728a85430..bd4d85f5dd92 100644 >> --- a/arch/arm64/kernel/cpufeature.c >> +++ b/arch/arm64/kernel/cpufeature.c >> @@ -530,6 +530,21 @@ static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = { >> ARM64_FTR_END, >> }; >> >> +static const struct arm64_ftr_bits ftr_id_aa64dfr1[] = { >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_ABL_CMPs_SHIFT, 8, 0), >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_DPFZS_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_EBEP_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_ITE_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_ABLE_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_PMICNTR_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_SPMU_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_CTX_CMPs_SHIFT, 8, 0), >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_WRPs_SHIFT, 8, 0), >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_BRPs_SHIFT, 8, 0), >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_SYSPMUID_SHIFT, 8, 0), >> + ARM64_FTR_END, >> +}; > > I think I mentioned this on an earlier series, but it would be useful to > see some justification in the commit message as to why some of these > features are considered STRICT vs NONSTRICT and why LOWER_SAFE is > preferred over EXACT. I have updated the commit message regarding STRICT vs NONSTRICT as Mark had mentioned earlier (see below). But just wondering about FTR_LOWER_SAFE. All similar fields have been marked as FTR_LOWER_SAFE. > > For example, why is EBEP strict whereas other PMU-related fields aren't? IIUC there are two types of fields here. - Feature presence (EBEP, ITE, ABLE etc) ---> FTR_STRICT - Debug resource count (WRPs, BRPs, CTX_CMPs etc) ---> FTR_NONSTRICT > Why is the CTX_CMPs field treated differently to the same field in DFR0? There is mismatch not only for CTX_CMPs but for WRPs and BRPs as well. As mentioned earlier, being resource count type, I believe these should be marked as FTR_NONSTRICT in existing DFR0 as well. I could send a patch fixing DFR0 first. > > I'm not saying the above table is wrong, it just looks arbitrary without > the justification. Please find the updated version (v3) for this patch here for reference. -------------- [PATCH] arm64/cpufeature: Add field details for ID_AA64DFR1_EL1 register This adds required field details for ID_AA64DFR1_EL1, and also drops dummy ftr_raz[] array which is now redundant. These register fields will be used to enable increased breakpoint and watchpoint registers via FEAT_Debugv8p9 later. The register fields have been marked as FTR_STRICT, unless there is a known variation in practice. Signed-off-by: Anshuman Khandual --- arch/arm64/kernel/cpufeature.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 6ce71f444ed8..0dc22fde104e 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -534,6 +534,21 @@ static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = { ARM64_FTR_END, }; +static const struct arm64_ftr_bits ftr_id_aa64dfr1[] = { + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_ABL_CMPs_SHIFT, 8, 0), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_DPFZS_SHIFT, 4, 0), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_EBEP_SHIFT, 4, 0), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_ITE_SHIFT, 4, 0), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_ABLE_SHIFT, 4, 0), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_PMICNTR_SHIFT, 4, 0), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_SPMU_SHIFT, 4, 0), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_CTX_CMPs_SHIFT, 8, 0), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_WRPs_SHIFT, 8, 0), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_BRPs_SHIFT, 8, 0), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR1_EL1_SYSPMUID_SHIFT, 8, 0), + ARM64_FTR_END, +}; + static const struct arm64_ftr_bits ftr_mvfr0[] = { ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR0_EL1_FPRound_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR0_EL1_FPShVec_SHIFT, 4, 0), @@ -720,10 +735,6 @@ static const struct arm64_ftr_bits ftr_single32[] = { ARM64_FTR_END, }; -static const struct arm64_ftr_bits ftr_raz[] = { - ARM64_FTR_END, -}; - #define __ARM64_FTR_REG_OVERRIDE(id_str, id, table, ovr) { \ .sys_id = id, \ .reg = &(struct arm64_ftr_reg){ \ @@ -796,7 +807,7 @@ static const struct __ftr_reg_entry { /* Op1 = 0, CRn = 0, CRm = 5 */ ARM64_FTR_REG(SYS_ID_AA64DFR0_EL1, ftr_id_aa64dfr0), - ARM64_FTR_REG(SYS_ID_AA64DFR1_EL1, ftr_raz), + ARM64_FTR_REG(SYS_ID_AA64DFR1_EL1, ftr_id_aa64dfr1), /* Op1 = 0, CRn = 0, CRm = 6 */ ARM64_FTR_REG(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0), -- 2.25.1 -------------- Changes in V3: - Updated commit message regarding FTR_STRICT - Updated ID_AA64DFR1_EL1.ABLE as FTR_NONSTRICT