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 E5AB4C4167B for ; Thu, 7 Dec 2023 15:00:49 +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=6AI3NYFPt5YumCRt7OSKvNbhgrrna7veY0NvDnGmoaM=; b=zUiiP2hnWZ/VX3 SxgjGL35WxnBGqjn5K7I3EdH4p2niwez54wmjpgA0IUuLOCNwJqAoBfHOWPXJwXxIaxZiWhDz8Zha M1zcXLqxs+bCtxm8pa2lzQILk0xJZB+MHbEbynxl/vU9NfZEZ+3X4rk1d1wcttGxmARnvdyM4dWW/ bhsCD5xLbu//g1PRVc3U30T1lteP1j7ez32+KFZh9w6RqGpMdaVzf85wp8oes/xvKrYqDL30NW6f/ Fqe0ZLyB4SMrTzBvM++AwWr/GdH05JBr2ldObXYPVkSvI/V6QHh3zT94obTby7ErTXXltJzAqh0Q+ wqBCLkmAJ6XZ2o9RWhsw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rBFrN-00D67a-37; Thu, 07 Dec 2023 15:00:17 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rBFrL-00D673-0N for linux-arm-kernel@lists.infradead.org; Thu, 07 Dec 2023 15:00:16 +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 B630B2F4; Thu, 7 Dec 2023 07:00:56 -0800 (PST) Received: from e124191.cambridge.arm.com (e124191.cambridge.arm.com [10.1.197.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3925E3F762; Thu, 7 Dec 2023 07:00:09 -0800 (PST) Date: Thu, 7 Dec 2023 15:00:01 +0000 From: Joey Gouly To: Fuad Tabba Cc: kvmarm@lists.linux.dev, maz@kernel.org, oliver.upton@linux.dev, james.morse@arm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com, catalin.marinas@arm.com, will@kernel.org, eric.auger@redhat.com, jingzhangos@google.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 06/12] KVM: arm64: Update and fix FGT register masks Message-ID: <20231207150001.GA29745@e124191.cambridge.arm.com> References: <20231206100503.564090-1-tabba@google.com> <20231206100503.564090-7-tabba@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20231206100503.564090-7-tabba@google.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231207_070015_275693_CA1F7E45 X-CRM114-Status: GOOD ( 24.15 ) 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 Hello Fuad, On Wed, Dec 06, 2023 at 10:04:56AM +0000, Fuad Tabba wrote: > New trap bits have been defined in the 2023-09 Arm Architecture > System Registers xml specification [*]. Moreover, the existing > definitions of some of the mask and the RES0 bits overlap, which > could be wrong, confusing, or both. > > Update the bits to represent the latest spec (as of this patch, > 2023-09), and ensure that the existing bits are consistent. > > Subsequent patches will use the generated RES0 fields instead of > specifying them manually. This patch keeps the manual encoding of > the bits to make it easier to review the series. > > [*] https://developer.arm.com/downloads/-/exploration-tools > > Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps") > Signed-off-by: Fuad Tabba > --- > arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------ > 1 file changed, 24 insertions(+), 15 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h > index 7de0a7062625..b0dc3249d5cd 100644 > --- a/arch/arm64/include/asm/kvm_arm.h > +++ b/arch/arm64/include/asm/kvm_arm.h > @@ -344,30 +344,39 @@ > * Once we get to a point where the two describe the same thing, we'll > * merge the definitions. One day. > */ > -#define __HFGRTR_EL2_RES0 (GENMASK(63, 56) | GENMASK(53, 51)) > +#define __HFGRTR_EL2_RES0 BIT(51) > #define __HFGRTR_EL2_MASK GENMASK(49, 0) > -#define __HFGRTR_EL2_nMASK (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50)) > +#define __HFGRTR_EL2_nMASK (GENMASK(63, 52) | BIT(50)) > > -#define __HFGWTR_EL2_RES0 (GENMASK(63, 56) | GENMASK(53, 51) | \ > - BIT(46) | BIT(42) | BIT(40) | BIT(28) | \ > - GENMASK(26, 25) | BIT(21) | BIT(18) | \ > +#define __HFGWTR_EL2_RES0 (BIT(51) | BIT(46) | BIT(42) | BIT(40) | \ > + BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \ > GENMASK(15, 14) | GENMASK(10, 9) | BIT(2)) > -#define __HFGWTR_EL2_MASK GENMASK(49, 0) > -#define __HFGWTR_EL2_nMASK (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50)) > +#define __HFGWTR_EL2_MASK (GENMASK(49, 47) | GENMASK(45, 43) | \ > + BIT(41) | GENMASK(39, 29) | BIT(27) | \ > + GENMASK(24, 22) | GENMASK(20, 19) | \ > + GENMASK(17, 16) | GENMASK(13, 11) | \ > + GENMASK(8, 3) | GENMASK(1, 0)) > +#define __HFGWTR_EL2_nMASK (GENMASK(63, 52) | BIT(50)) By adding all these bits to *_nMASK, we're allowing a guest to access registers which KVM doesn't (currently) deal with. For example if I apply this patch series, a guest can access S2POR_EL1, previously it would print something like: kvm [80]: Unsupported guest sys_reg access at: ffffc42969c1f270 [600000c5] { Op0( 3), Op1( 0), CRn(10), CRm( 2), Op2( 5), func_read }, After applying this patch series, the guest can read S2POR_EL1. We don't expose S2POE to the guest through ID_AA64MMFR3_EL1, so a well behaved guest shouldn't access it, but there's nothing stopping it. My question is, is this intended? Or do we need to update the following code (and comment!) to trap all the stuff we don't currently handle (along with ACCDATA_EL1): static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu) { .. /* The default is not to trap anything but ACCDATA_EL1 */ r_val = __HFGRTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1; r_val |= r_set; r_val &= ~r_clr; Thanks, Joey > > -#define __HFGITR_EL2_RES0 GENMASK(63, 57) > -#define __HFGITR_EL2_MASK GENMASK(54, 0) > -#define __HFGITR_EL2_nMASK GENMASK(56, 55) > +#define __HFGITR_EL2_RES0 (BIT(63) | BIT(61)) > +#define __HFGITR_EL2_MASK (BIT(62) | BIT(60) | GENMASK(54, 0)) > +#define __HFGITR_EL2_nMASK GENMASK(59, 55) > > #define __HDFGRTR_EL2_RES0 (BIT(49) | BIT(42) | GENMASK(39, 38) | \ > GENMASK(21, 20) | BIT(8)) > -#define __HDFGRTR_EL2_MASK ~__HDFGRTR_EL2_nMASK > +#define __HDFGRTR_EL2_MASK (BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \ > + GENMASK(41, 40) | GENMASK(37, 22) | \ > + GENMASK(19, 9) | GENMASK(7, 0)) > #define __HDFGRTR_EL2_nMASK GENMASK(62, 59) > > #define __HDFGWTR_EL2_RES0 (BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \ > BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \ > BIT(22) | BIT(9) | BIT(6)) > -#define __HDFGWTR_EL2_MASK ~__HDFGWTR_EL2_nMASK > +#define __HDFGWTR_EL2_MASK (GENMASK(57, 52) | GENMASK(50, 48) | \ > + GENMASK(46, 44) | GENMASK(42, 41) | \ > + GENMASK(37, 35) | GENMASK(33, 31) | \ > + GENMASK(29, 23) | GENMASK(21, 10) | \ > + GENMASK(8, 7) | GENMASK(5, 0)) > #define __HDFGWTR_EL2_nMASK GENMASK(62, 60) > > #define __HAFGRTR_EL2_RES0 (GENMASK(63, 50) | GENMASK(16, 5)) > @@ -375,9 +384,9 @@ > #define __HAFGRTR_EL2_nMASK 0UL > > /* Similar definitions for HCRX_EL2 */ > -#define __HCRX_EL2_RES0 (GENMASK(63, 16) | GENMASK(13, 12)) > -#define __HCRX_EL2_MASK (0) > -#define __HCRX_EL2_nMASK (GENMASK(15, 14) | GENMASK(4, 0)) > +#define __HCRX_EL2_RES0 (GENMASK(63, 25) | GENMASK(13, 12)) > +#define __HCRX_EL2_MASK (BIT(6)) > +#define __HCRX_EL2_nMASK (GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0)) > > /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */ > #define HPFAR_MASK (~UL(0xf)) > -- > 2.43.0.rc2.451.g8631bc7472-goog > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel