From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 08A08D302 for ; Tue, 5 Dec 2023 11:20:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="uuOWRTwd" Received: by smtp.kernel.org (Postfix) with ESMTPSA id C90E0C433C8; Tue, 5 Dec 2023 11:20:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1701775200; bh=YIEwepVup/ivgxiaxvr0x9a78D5l5mWN0axFm+F2+ZI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=uuOWRTwdEpze4IFZfcTtJsJImLyk2qNvTldVn/lQV6ID8hx+cQhX+9X2CIILhikKn EkZ8yd5DpEsjzKjCMbA4JZbtap9QioaVF8ea7YZpkC35+yCFNerNZt/ohHWVYGLz+/ TPcMGAOuZZtrZ+j9rLa+UDHgbu1lDlB94Q6gggpcpmU097v5ztGiSkOviY+xYcPcYZ dlC3JwHkvw6K97VTUyjsT5X+bQUtlvv9QnWHm7LBPGkOc02lA2FyezvMYVz45kiZmH QfCCbozpM7hQF+/QIj2YneHyNZtyQgSIugOryxKohrrn7zxJchAoydGmhb6XPLY1x/ UcS+2T1Njv2GQ== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1rATT4-001WlW-BF; Tue, 05 Dec 2023 11:19:58 +0000 Date: Tue, 05 Dec 2023 11:19:58 +0000 Message-ID: <86edg0c2w1.wl-maz@kernel.org> From: Marc Zyngier To: Fuad Tabba Cc: kvmarm@lists.linux.dev, 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, joey.gouly@arm.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v1 1/6] KVM: arm64: Update and fix FGT register masks In-Reply-To: <20231205102248.1915895-2-tabba@google.com> References: <20231205102248.1915895-1-tabba@google.com> <20231205102248.1915895-2-tabba@google.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/29.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Precedence: bulk X-Mailing-List: kvmarm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: tabba@google.com, kvmarm@lists.linux.dev, 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, joey.gouly@arm.com, linux-arm-kernel@lists.infradead.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Tue, 05 Dec 2023 10:22:43 +0000, Fuad Tabba wrote: > > New trap bits have been defined in the latest Arm Architecture > System Registers xml specification [*]. Moreover, the existing > definitions of some of the mask and the res0 bits overlap, which nit: s/res0/RES0/ > could be wrong, confusing, or potentially both. > > Update the bits to represent the latest spec (as of this patch), > and ensure that the existing bits are consistent. > > [*] https://developer.arm.com/downloads/-/exploration-tools Please indicate which version of the XML you are referring to (it changes on average every 3 months). > > 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 b85f46a73e21..b1061647e837 100644 > --- a/arch/arm64/include/asm/kvm_arm.h > +++ b/arch/arm64/include/asm/kvm_arm.h > @@ -344,36 +344,45 @@ > * 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) If we are now using the current architectural definition for RES0, can we please move over to the generated values, as indicated in the above comment? > #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)) > > -#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) > > /* 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)) The main driver for not following the latest and greatest version published in the architecture (including the FGT2 stuff) was to limit the amount of work we need to do in NV. Can you please update the FGT tables in emulated-nested.c so that we have a matching description of the added bits? This also overlaps with the work Joey is doing on POE, so there is scope for collaboration between you two here. Thanks, M. -- Without deviation from the norm, progress is not possible.