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 4CD99C2BBCA for ; Tue, 25 Jun 2024 13:59:26 +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:MIME-Version:References:In-Reply-To:Subject:Cc:To:From: Message-ID:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Sv/lMQ9W5/pob+MBdKtFPm9OvyslhUweMHdUdjwsto4=; b=MGvnb13Dwca1LtH1QCO74xEH7E BZekVwIqHBgdGg6TqjelcJjmXixhlIhTpV/g/33eyVLyl/2IWNzcg0C1KXS/z7EUdtu/2c8C8lYkw 1rZ8XHpPwMgpGOYsnVukZTDF3arbTHOCF0kfpgPYEDwz2l0eDbx6vGqhhew5MITyQxVzHnmtHLSGr A/9HDqfOBXzthjmlAEoBtC2tMFSEbS9Lj4FX5WIj3c4coLtMWjTU8kEQOpuLrn7ORDLDL5odxyifZ yFivA4HGmnvlsXPoitPnR9lvlyEOEYpZ50veERP2QkUIcN0JaGSeQdctMSIeocpsUcqf1seaxA+I9 l+v1P95w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sM6hQ-000000035i0-3Di1; Tue, 25 Jun 2024 13:59:08 +0000 Received: from sin.source.kernel.org ([2604:1380:40e1:4800::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sM6hK-000000035g1-1LAU for linux-arm-kernel@lists.infradead.org; Tue, 25 Jun 2024 13:59:03 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 4946ECE1B04; Tue, 25 Jun 2024 13:59:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id ABB89C32781; Tue, 25 Jun 2024 13:58:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1719323938; bh=LhJ6xSwgC+isVyrfMS2bZZDUrhNmtanpSDGFTa+w9j4=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=N35Cg0yEDqVFCfMV6szY8nhMfh79bTCP/px23OE7C9stj06oDaKbhuIt22ksAwZuu JgJCScKrwe+tIhWnwrMPN5Nw5LghZySS0cnaYQKhUzYGHjVV8S4huZ9JfaokXKCmK1 lbK9ooaHJvmMhFwtaFhQwXIXxUQuHaOs9rDUeXuLCmINnEaSWkI3/eYKJ6k2pdfHYa ssSWh2C1/soVuaPn1gKKmCrb0ot0B07kXa/5vIgyO++9PAdc2eQpFu3tAXW4/vj8YC O7lK7uQsKadpdIrIgRaXML1EdM5FqEq7s/gFvVCBe368Rk+9jl+z8F/YKlLd9Fv3m+ y4TPvWjIZlsMg== 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 1sM6hE-007Amz-Gb; Tue, 25 Jun 2024 14:58:56 +0100 Date: Tue, 25 Jun 2024 14:58:56 +0100 Message-ID: <86tthhi0nz.wl-maz@kernel.org> From: Marc Zyngier To: Anshuman Khandual Cc: linux-arm-kernel@lists.infradead.org, Oliver Upton , James Morse , Suzuki K Poulose , Catalin Marinas , Will Deacon , Mark Brown , kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [RFC 10/10] KVM: arm64: nv: Add new HDFGRTR2_GROUP & HDFGRTR2_GROUP based FGU handling In-Reply-To: <47dc4299-52cc-4f98-929b-fb86bd9757ae@arm.com> References: <20240620065807.151540-1-anshuman.khandual@arm.com> <20240620065807.151540-11-anshuman.khandual@arm.com> <865xu3kh4r.wl-maz@kernel.org> <4d256df7-1ec7-4300-b5c8-355f46c0e869@arm.com> <878qyy35e5.wl-maz@kernel.org> <47dc4299-52cc-4f98-929b-fb86bd9757ae@arm.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.2 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: anshuman.khandual@arm.com, linux-arm-kernel@lists.infradead.org, oliver.upton@linux.dev, james.morse@arm.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, broonie@kernel.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240625_065902_741089_3E7A0054 X-CRM114-Status: GOOD ( 34.55 ) 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 Tue, 25 Jun 2024 10:03:29 +0100, Anshuman Khandual wrote: >=20 > >>>> +#define __HDFGRTR2_EL2_nMASK ~(__HDFGRTR2_EL2_RES0 | __HDFGRTR2_EL2= _MASK) > >>> > >>> Note the *nMASK* here. 'n' is for *negative*. Just look at how > >>> __HDFGRTR_EL2_MASK and __HDFGRTR_EL2_nMASK are written. > >> > >> Still trying to understand what does these three masks represent > >> for a given FGT register XXXX > >> > >> - __XXXX_RES0 > >=20 > > RES0 bits. > >=20 > >> - __XXXX_MASK > >=20 > > Positive trap bits. > >=20 > >> - __XXXX_nMASK > >=20 > > Negative trap bits. >=20 > Right, figured that eventually but these were not very clear at > first. I keep hearing I'm not clear. But at this stage, I don't know what to do to make it (or myself) clearer. >=20 > >=20 > >> > >> But from the mentioned example for HDFGRTR_EL2. > >> > >> #define __HDFGRTR_EL2_RES0 HDFGRTR_EL2_RES0 > >> #define __HDFGRTR_EL2_MASK (BIT(63) | GENMASK(58, 50) | GENMASK(4= 8, 43) | \ > >> GENMASK(41, 40) | GENMASK(37, 22) | \ > >> GENMASK(19, 9) | GENMASK(7, 0)) > >> #define __HDFGRTR_EL2_nMASK ~(__HDFGRTR_EL2_RES0 | __HDFGRTR_EL2_M= ASK) > >> > >> Looking at HDFGRTR_EL2 definition inside arch/arm64/tools/sysreg > >> > >> HDFGRTR_EL2_RES0 =3D BIT(49) | GENMASK(39, 38) | GENMASK(21, 20) | BIT= (8) > >> > >> is representing the entire mask in the register which are RES0. But th= en what > >> does __HDFGRTR_EL2_MASK signify ? Positive trap bits mask ? > >> > >> The following bits belong in __HDFGRTR_EL2_MASK > >> > >> HDFGRTR_EL2.PMBIDR_EL1 (63) > >> HDFGRTR_EL2.PMCEIDn_EL0 (58) > >> > >> Where as the following bits belong in __HDFGRTR_EL2_nMASK > >> > >> HDFGRTR_EL2.nPMSNEVFR_EL1 (61) > >> HDFGRTR_EL2.nBRBCTL (60)=20 > >> > >> Reworking proposed HDFGRTR2_EL2 and HDFGWTR2_EL2.=20 > >> > >> #define __HDFGRTR2_EL2_RES0 HDFGRTR2_EL2_RES0 > >> #define __HDFGRTR2_EL2_MASK 0 > >> #define __HDFGRTR2_EL2_nMASK ~(__HDFGRTR2_EL2_RES0 | __HDFGRTR2_EL2= _MASK) > >> > >> #define __HDFGWTR2_EL2_RES0 HDFGWTR2_EL2_RES0 > >> #define __HDFGWTR2_EL2_MASK 0 > >> #define __HDFGWTR2_EL2_nMASK ~(__HDFGWTR2_EL2_RES0 | __HDFGWTR2_EL2= _MASK) > >> > >> Please note that all trap bits in both these registers are negative on= es > >> hence __HDFGRTR2_EL2_MASK/__HDFGWTR2_EL2_MASK should be 0. > >=20 > > That's because you're looking at the XML, and not the ARM ARM this was > > written against. >=20 > Did not follow that. Both in ddi0601/2024-03 XML and ARM ARM DDI 0487K.a > there are no positive trap bits, either in HDFGRTR2_EL2 or HDFGWTR2_EL2. > OR did I miss something here. =46rom this very file: /* * FGT register definitions * * RES0 and polarity masks as of DDI0487J.a, to be updated as needed. * We're not using the generated masks as they are usually ahead of * the published ARM ARM, which we use as a reference. * * Once we get to a point where the two describe the same thing, we'll * merge the definitions. One day. */ In case it wasn't written *CLEARLY* enough. > Sure, will add the following new registers in arch/arm64/tools/sysreg for= mat > except for the ones that require a formula based enumeration. Those will = be > added into arch/arm64/include/asm/sysreg.h directly e.g >=20 > +#define SYS_SPMEVCNTR_EL0(m) sys_reg(2, 3, 14, (0 | (m >> 3)),= (m & 7)) > +#define SYS_SPMEVTYPER_EL0(m) sys_reg(2, 3, 14, (2 | (m >> 3)),= (m & 7)) > +#define SYS_SPMEVFILTR_EL0(m) sys_reg(2, 3, 14, (4 | (m >> 3)),= (m & 7)) > +#define SYS_SPMEVFILT2R_EL0(m) sys_reg(2, 3, 14, (6 | (m >> 3)),= (m & 7)) >=20 > 9d93fc432f1a arm64/sysreg: Add remaining debug registers affected by HDFG= xTR2_EL2 > 56b8830f0a5e arm64/sysreg: Add register fields for SPMCGCR1_EL1 > ad674ae52178 arm64/sysreg: Add register fields for SPMCGCR0_EL1 > 8538a282d208 arm64/sysreg: Add register fields for PMZR_EL0 > c88f0b8d898e arm64/sysreg: Add register fields for PMSSCR_EL1 > 8788ad49b7b6 arm64/sysreg: Add register fields for SPMCFGR_EL1 > 142de2bc3d7b arm64/sysreg: Add register fields for SPMDEVARCH_EL1 > 3d903ba35e8c arm64/sysreg: Add register fields for SPMIIDR_EL1 > 1a4db0b8b100 arm64/sysreg: Add register fields for PMICNTSVR_EL1 > 6bee8f139ba5 arm64/sysreg: Add register fields for SPMSELR_EL0 > b208ab4cd54d arm64/sysreg: Add register fields for SPMCNTENSET_EL0 > 644abf522c8a arm64/sysreg: Add register fields for SPMCNTENCLR_EL0 > fad9b7751359 arm64/sysreg: Add register fields for SPMINTENSET_EL1 > c28299b8df76 arm64/sysreg: Add register fields for SPMINTENCLR_EL1 > b9c283b27980 arm64/sysreg: Add register fields for SPMOVSSET_EL0 > 03dd01a26c46 arm64/sysreg: Add register fields for SPMOVSCLR_EL0 > c8a2f1b688de arm64/sysreg: Add register fields for SPMCR_EL0 > 422ca4026aa7 arm64/sysreg: Add register fields for SPMACCESSR_EL1 > d790b2570461 arm64/sysreg: Add register fields for SPMSCR_EL1 > c2cdd0fecdcb arm64/sysreg: Add register fields for PMCCNTSVR_EL1 > 3b793f3f07b8 arm64/sysreg: Add register fields for PMUACR_EL1 > a1804742ee8c arm64/sysreg: Add register fields for PMICFILTR_EL0 > c88476b9c52c arm64/sysreg: Add register fields for PMICNTR_EL0 > 6d1520f3477b arm64/sysreg: Add register fields for PMECR_EL1 > 257ea3ec7950 arm64/sysreg: Add register fields for PMIAR_EL1 > a29f787102f0 arm64/sysreg: Add register fields for SPMDEVAFF_EL1 > 3b7c4d4cf0eb arm64/sysreg: Add register fields for PMSDSFR_EL1 > 2a14e5dc1903 arm64/sysreg: Add register fields for TRBMPAM_EL1 Have you done this by hand? Or have you used an automated generator that parses the XML? If it's the former, please do the latter and compare the results. [...] > > > >>>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > >>>> index f921af014d0c..8029f408855d 100644 > >>>> --- a/arch/arm64/kvm/sys_regs.c > >>>> +++ b/arch/arm64/kvm/sys_regs.c > >>>> @@ -4110,6 +4110,51 @@ void kvm_init_sysreg(struct kvm_vcpu *vcpu) > >>>> kvm->arch.fgu[HAFGRTR_GROUP] |=3D ~(HAFGRTR_EL2_RES0 | > >>>> HAFGRTR_EL2_RES1); > >>>> =20 > >>>> + /* FEAT_TRBE_MPAM is not exposed to the guest */ > >>>> + kvm->arch.fgu[HDFGRTR2_GROUP] |=3D ~(HDFGRTR2_EL2_nTRBMPAM_EL1); > >>> > >>> Same thing about dynamic configuration. > >>> > >>> But more importantly, you are disabling anything *but* MPAM. Does it > >>> seem right to you? > >> > >> Did not get that, should the inverse ~ be dropped here and also for all > >> other negative trap bits across the register ? > >=20 > > Look at the way FGU works. A bit set to 1 means that if we have > > trapped because of this bit (as per the FGT table), we inject an > > UNDEF. >=20 > Seems like positive and negative trap bits do not make any difference > here, while setting respective bits in kvm->arch.fgu[HDFGRTR2_GROUP].=20 > In this case the inverse should be dropped for all bits. Yup, that's what "A bit set to 1" means here. We don't need to follow the convolutions of the HW here. >=20 > Should the lone trap bit (nPMZR_EL0) which is present in HDFGWTR2_EL2 > but not in HDFGRTR2_EL2, be set in kvm->arch.fgu[HDFGWTR2_GROUP] as > well ? >=20 > if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMUVer, V3P9)) { > kvm->arch.fgu[HDFGRTR2_GROUP] |=3D HDFGRTR2_EL2_nPMUACR_EL1; > kvm->arch.fgu[HDFGWTR2_GROUP] |=3D HDFGWTR2_EL2_nPMZR_EL0; > } Possibly. At the time this code was written, ARMv8.9 wasn't in the ARM ARM. M. --=20 Without deviation from the norm, progress is not possible.