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 1F113C3DA63 for ; Fri, 26 Jul 2024 06:56:03 +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=pVULuUw+fd8qGgjSStTK0FovQ5XTqoCmXg2hcJlnAmQ=; b=eRzDeZ10STSauwryoB/A+wr1m0 XvPK+ZDSfsoDD9Vkax4sPVkYSQcvgCeLLSyfwVqXF9RymKO9a+NeehUX1JJWeQ18mas5WahIAg11L fXxRPihYvGWH6fivH24NdjxfJ6LyYHu9lX55m0woymkJRrn87nvuPR0KuVH6ow55jP+JR5sgsSAym xMq++XWEaVrSBal/WxSvz+Gc5mFJvWRpYzVAJlRqnR7JkZ5uanWVf2ZARqWgHiYkFtuUaoxjCVrXd HnBI5ttOP3wacUi5xhGzgz4h7TzcKJKUZN+qT7FPwXXCG1ybxvwn+A6KIBzSbWjbIn4rnDtHMMart y6Fv+h0g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sXEri-000000035v9-1UP3; Fri, 26 Jul 2024 06:55:46 +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 1sXErI-000000035pw-3LFl for linux-arm-kernel@lists.infradead.org; Fri, 26 Jul 2024 06:55:22 +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 BD3DE1007; Thu, 25 Jul 2024 23:55:44 -0700 (PDT) Received: from [10.163.53.239] (unknown [10.163.53.239]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2ABC63F73F; Thu, 25 Jul 2024 23:55:17 -0700 (PDT) Message-ID: <498bab5b-010c-4505-b081-5570c33d4d33@arm.com> Date: Fri, 26 Jul 2024 12:25:14 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [boot-wrapper 2/3] aarch64: Enable access into SCTLR2_ELx registers from EL2 and below To: Mark Rutland Cc: linux-arm-kernel@lists.infradead.org References: <20240723110630.483871-1-anshuman.khandual@arm.com> <20240723110630.483871-3-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 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240725_235520_954956_9C16C87B X-CRM114-Status: GOOD ( 22.91 ) 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 7/25/24 14:10, Mark Rutland wrote: > On Tue, Jul 23, 2024 at 04:36:29PM +0530, Anshuman Khandual wrote: >> FEAT_SCTLR2 adds SCTLR2_EL1 and SCTLR2_EL2 system registers But access into >> these register from EL2 and below trap to EL3 unless SCR_EL3.D128En is set. Will fix a small nit here, s/SCR_EL3.D128En/SCR_EL3.SCTLR2En/ ^^^ >> >> Enable access to SCTLR2_ELx registers when they are implemented. >> >> Signed-off-by: Anshuman Khandual > > I think this should be first in the series, since SCTLR2 can exist > without D128, but not vice-versa. Sure, will move it as the first patch in the series. > >> --- >> arch/aarch64/include/asm/cpu.h | 4 +++- >> arch/aarch64/init.c | 3 +++ >> 2 files changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h >> index 0b8b463..57d66e4 100644 >> --- a/arch/aarch64/include/asm/cpu.h >> +++ b/arch/aarch64/include/asm/cpu.h >> @@ -56,6 +56,7 @@ >> #define SCR_EL3_HXEn BIT(38) >> #define SCR_EL3_EnTP2 BIT(41) >> #define SCR_EL3_TCR2EN BIT(43) >> +#define SCR_EL3_SCTLR2En BIT(44) >> #define SCR_EL3_PIEN BIT(45) >> #define SCR_EL3_D128En BIT(47) >> >> @@ -81,7 +82,8 @@ >> >> #define ID_AA64MMFR1_EL1_HCX BITS(43, 40) >> >> -#define ID_AA64MMFR3_EL1_TCRX BITS(4, 0) >> +#define ID_AA64MMFR3_EL1_TCRX BITS(3, 0) >> +#define ID_AA64MMFR3_EL1_SCTLRX BITS(7, 4) >> #define ID_AA64MMFR3_EL1_S1PIE BITS(11, 8) >> #define ID_AA64MMFR3_EL1_S2PIE BITS(15, 12) >> #define ID_AA64MMFR3_EL1_S1POE BITS(19, 16) >> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c >> index 7d9d0d9..5b21cb8 100644 >> --- a/arch/aarch64/init.c >> +++ b/arch/aarch64/init.c >> @@ -92,6 +92,9 @@ void cpu_init_el3(void) >> if (mrs_field(ID_AA64MMFR3_EL1, D128)) >> scr |= SCR_EL3_D128En; >> >> + if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX)) >> + scr |= SCR_EL3_SCTLR2En; >> + > > The SCTLR2_ELx registers reset to UNKNOWN values when the highest > implemented exception level is not ELx, so we need to initialize those > to safe values. Otherwise a kernel which is not aware of SCTLR2_ELx will > be subject to arbitrary behaviour as a result of the SCTLR2_ELx bits > which it will not have configured. Both SCTLR2_EL1 and SCTLR2_EL2 has the same register fields layout except the very last bit i.e SCTLR2_EL2.EMEC which is available in SCTLR2_EL2 but not in SCTLR2_EL1. AFAICT all the above register fields are applicable for newer arch features which the current kernel is not even aware about. So even if the kernel is not ware about SCTLR2_EL2 or SCTLR2_EL1 registers, there will not be any difference in behaviour related to these new arch features. Search for the registers in the current mainline kernel. $git grep SCTLR2_EL arch/arm64/include/asm/sysreg.h:#define SYS_SCTLR2_EL2 sys_reg(3, 4, 1, 0, 3) arch/arm64/include/asm/sysreg.h:#define SYS_SCTLR2_EL12 sys_reg(3, 5, 1, 0, 3) arch/arm64/kvm/emulate-nested.c: SR_TRAP(SYS_SCTLR2_EL2, CGT_HCR_NV), $git grep SCTLR2En arch/arm64/kvm/nested.c: res0 |= HCRX_EL2_SCTLR2En; arch/arm64/tools/sysreg:Field 15 SCTLR2En Although if we are looking for safer values, guess resetting these two registers might be sufficient here ? + if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX)) { + scr |= SCR_EL3_SCTLR2En; + msr(SCTLR2_EL2, 0); + msr(SCTLR2_EL1, 0); + } + > > I know that we've failed to do that for other things (FGT and HCRX), and > those are latent bugs / mistakes in our appraoch that I'll see about > fixing. Sure.