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 3AD89C3DA64 for ; Thu, 1 Aug 2024 16:04:56 +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-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Podm92d5lABsBjgrYgxfIFAjyteio2a6P+i7cbX1t2g=; b=f8HFMKxZzzfVsvjxzV2JFTu3z/ V8VALaOlj6+gC0mkJapfXj62GRB8/0eQhYMS8caLIk3PAKxBeNhLsqovXy5LNmhlS+5ZaU19UVAac lHGaT2nWZXO0Rxzy7ZRbeBSIhsjXl/2b2as3YNzbxrcLO5RrfUwc6PZIbXfq0Uf8mUIiPufQIdyA/ uKpRcg8diwHnXZQC5x08BcfSvadGxwgsGP18uhtEI6mwPrShLeFrDLHHvCOlQD/js3OXG+XJ1yrjr UTivoOTwKY8n1rQ7u2uT1ql3ByaY9HMsftmGF+3ny8jdxMt07irR9ZPxkBQjdjZ7aZFR1i8BdWmE6 KWtMjTFA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sZYID-00000005xkQ-05mj; Thu, 01 Aug 2024 16:04:41 +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 1sZYHF-00000005xPI-1Fvg for linux-arm-kernel@lists.infradead.org; Thu, 01 Aug 2024 16:03:43 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 0B1FDCE1905; Thu, 1 Aug 2024 16:03:39 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 095B8C32786; Thu, 1 Aug 2024 16:03:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1722528218; bh=xlimnjk7WTBdjOAC9Ce6co98zdnquZnkjWiBUSxCBFw=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=hMwnML3OlFMMrWXVqwX3ZNFHNfUK5TNgGmyIjfBoLj9kwdFxjg1kIOJRyimRiBWDV Tnsyvru/AcOiVZmupipN8CHO1fNvkd4jmQyuzDVP/4wZgY9mm82Hcnwg9mw0P90EKc s0Cz3vae8k+VQm3UE67TcrfHBa6yZ9ZJxcnRdz8O1pGU+VCxfEO3HDFP5VWB8ir6PS LkN1ANWQL9ZP/BLG+ccgy1AG5J05zTmyWY2i6X7AGnWjWOAYJ5TA/sVw1yeI+0qRV/ 8Ei8pBlUNEeDWhFW8myPWnQStbaS+ssqtSHBhAIIHqE5LFregGO2zVqOGX8W+jGv4g aJqzZDJ2SHrKg== 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 1sZYH9-00HSJO-MJ; Thu, 01 Aug 2024 17:03:35 +0100 Date: Thu, 01 Aug 2024 17:03:34 +0100 Message-ID: <86o76c1b8p.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: 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> <86tthhi0nz.wl-maz@kernel.org> 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.3 (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 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-20240801_090341_725138_4913E48B X-CRM114-Status: GOOD ( 39.42 ) 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 Thu, 01 Aug 2024 11:46:22 +0100, Anshuman Khandual wrote: > > Hello Marc, > > As you had suggested to avoid posting the entire series (which now consists > of around ~30 patches just for all required sysregs update) to get the core > part of FGT2 reviewed, please find the updated last patch here. I will post > the entire series once there is some agreement on this patch. Thank > you. Thanks for respining it in this form. Comments below. > > Changes from RFC V1: > > - Updated positive and negative masks for HDFGRTR2_EL2 and HDFGWTR2_EL2 > - Both register's positive masks are 0 (all bits are nFormat) > - Both register's negative masks are all bits but the RES0 ones > - Please note that sysreg field definitions for both registers > were added into tools are from 2024-06 XML not ARM DDI 0487K.a > - Even if ARM DDI 0487K.a gets used instead of the above XML, > there are no positive mask bits, only RES0 mask will expand. > > - Only nPMZR_EL0 gets added for HDFGWTR2_EL2 in encoding_to_fgt() > - Follows the same pattern as in HDFGWTR_EL2/HDFGWTR_EL2 > - All other entries are for HDFGRTR2_EL2 > > - Used indirect features check for FEAT_TRBE_MPAM and FEAT_SPE_FDS > - Updated kvm_init_nv_sysregs() as required > - Updated kvm_calculate_traps() as required > > - Anshuman > > -------->8---------------------------------------------------------------- > From c1e648dcdb603ad182bcd522ca489f7deee58e86 Mon Sep 17 00:00:00 2001 > From: Anshuman Khandual > Date: Mon, 8 Apr 2024 12:15:35 +0530 > Subject: [PATCH] KVM: arm64: nv: Add new HDFGRTR2_GROUP & HDFGRTR2_GROUP based > FGU handling > > This adds new HDFGRTR2_GROUP and HDFGWTR2_GROUP groups in enum fgt_group_id > for FGU handling managed with HDFGRTR2_EL2 and HDFGWTR2_EL2 registers. > > Cc: Marc Zyngier > Cc: Oliver Upton > Cc: James Morse > Cc: Suzuki K Poulose > Cc: linux-arm-kernel@lists.infradead.org > Cc: kvmarm@lists.linux.dev > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Anshuman Khandual > --- > arch/arm64/include/asm/kvm_arm.h | 8 ++ > arch/arm64/include/asm/kvm_host.h | 2 + > arch/arm64/kvm/emulate-nested.c | 158 ++++++++++++++++++++++++ > arch/arm64/kvm/hyp/include/hyp/switch.h | 10 ++ > arch/arm64/kvm/nested.c | 38 ++++++ > arch/arm64/kvm/sys_regs.c | 49 ++++++++ > 6 files changed, 265 insertions(+) > > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h > index d81cc746e0eb..66ebb2f8e0fa 100644 > --- a/arch/arm64/include/asm/kvm_arm.h > +++ b/arch/arm64/include/asm/kvm_arm.h > @@ -353,6 +353,14 @@ > #define __HFGRTR_EL2_MASK GENMASK(49, 0) > #define __HFGRTR_EL2_nMASK ~(__HFGRTR_EL2_RES0 | __HFGRTR_EL2_MASK) > > +#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) > + > /* > * The HFGWTR bits are a subset of HFGRTR bits. To ensure we don't miss any > * future additions, define __HFGWTR* macros relative to __HFGRTR* ones. > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 696bb07b4722..edf78ddb099f 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -266,6 +266,8 @@ enum fgt_group_id { > HDFGWTR_GROUP = HDFGRTR_GROUP, > HFGITR_GROUP, > HAFGRTR_GROUP, > + HDFGRTR2_GROUP, > + HDFGWTR2_GROUP = HDFGRTR2_GROUP, > > /* Must be last */ > __NR_FGT_GROUP_IDS__ > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c > index 05166eccea0a..bbeeb0ab758e 100644 > --- a/arch/arm64/kvm/emulate-nested.c > +++ b/arch/arm64/kvm/emulate-nested.c > @@ -1828,6 +1828,153 @@ static const struct encoding_to_trap_config encoding_to_fgt[] __initconst = { > SR_FGT(SYS_AMEVCNTR0_EL0(2), HAFGRTR, AMEVCNTR02_EL0, 1), > SR_FGT(SYS_AMEVCNTR0_EL0(1), HAFGRTR, AMEVCNTR01_EL0, 1), > SR_FGT(SYS_AMEVCNTR0_EL0(0), HAFGRTR, AMEVCNTR00_EL0, 1), > + > + /* > + * HDFGWTR_EL2 > + * > + * Although HDFGRTR2_EL2 and HDFGWTR2_EL2 registers largely > + * overlap in their bit assignment, there are a number of bits > + * that are RES0 on one side, and an actual trap bit on the > + * other. The policy chosen here is to describe all the > + * read-side mappings, and only the write-side mappings that > + * differ from the read side, and the trap handler will pick > + * the correct shadow register based on the access type. > + */ > + SR_FGT(SYS_PMZR_EL0, HDFGWTR2, nPMZR_EL0, 0), Be consistent with the existing order of registers. Writes are ordered after reads, and I'd like to preserve this. > + > + /* HDFGRTR2_EL2 */ > + SR_FGT(SYS_MDSTEPOP_EL1, HDFGRTR2, nMDSTEPOP_EL1, 0), > + SR_FGT(SYS_TRBMPAM_EL1, HDFGRTR2, nTRBMPAM_EL1, 0), > + SR_FGT(SYS_TRCITECR_EL1, HDFGRTR2, nTRCITECR_EL1, 0), > + SR_FGT(SYS_PMSDSFR_EL1, HDFGRTR2, nPMSDSFR_EL1, 0), > + SR_FGT(SYS_SPMDEVAFF_EL1, HDFGRTR2, nSPMDEVAFF_EL1, 0), > + > + SR_FGT(SYS_SPMCGCR0_EL1, HDFGRTR2, nSPMID, 0), > + SR_FGT(SYS_SPMCGCR1_EL1, HDFGRTR2, nSPMID, 0), > + SR_FGT(SYS_SPMIIDR_EL1, HDFGRTR2, nSPMID, 0), > + SR_FGT(SYS_SPMDEVARCH_EL1, HDFGRTR2, nSPMID, 0), > + SR_FGT(SYS_SPMCFGR_EL1, HDFGRTR2, nSPMID, 0), > + > + SR_FGT(SYS_SPMSCR_EL1, HDFGRTR2, nSPMSCR_EL1, 0), That's a pretty odd one, as it only exists on the secure side. We will never see the access, as it will UNDEF at EL1, but hey, who cares. Let's take this as documentation. > + SR_FGT(SYS_SPMACCESSR_EL1, HDFGRTR2, nSPMACCESSR_EL1, 0), This (and I take it most of the stuff here) is also gated by MDCR_EL2.SPM, which is a coarse grained trap. That needs to be described as well. For every new register that you add here. > + SR_FGT(SYS_SPMCR_EL0, HDFGRTR2, nSPMCR_EL0, 0), > + SR_FGT(SYS_SPMOVSCLR_EL0, HDFGRTR2, nSPMOVS, 0), > + SR_FGT(SYS_SPMOVSSET_EL0, HDFGRTR2, nSPMOVS, 0), > + SR_FGT(SYS_SPMINTENCLR_EL1, HDFGRTR2, nSPMINTEN, 0), > + SR_FGT(SYS_SPMINTENSET_EL1, HDFGRTR2, nSPMINTEN, 0), > + SR_FGT(SYS_SPMCNTENCLR_EL0, HDFGRTR2, nSPMCNTEN, 0), > + SR_FGT(SYS_SPMCNTENSET_EL0, HDFGRTR2, nSPMCNTEN, 0), > + SR_FGT(SYS_SPMSELR_EL0, HDFGRTR2, nSPMSELR_EL0, 0), > + > + SR_FGT(SYS_SPMEVTYPER_EL0(0), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(1), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(2), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(3), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(4), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(5), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(6), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(7), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(8), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(9), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(10), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(11), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(12), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(13), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(14), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVTYPER_EL0(15), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + > + SR_FGT(SYS_SPMEVFILTR_EL0(0), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(1), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(2), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(3), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(4), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(5), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(6), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(7), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(8), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(9), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(10), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(11), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(12), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(13), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(14), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILTR_EL0(15), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + > + SR_FGT(SYS_SPMEVFILT2R_EL0(0), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(1), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(2), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(3), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(4), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(5), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(6), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(7), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(8), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(9), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(10), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(11), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(12), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(13), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(14), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + SR_FGT(SYS_SPMEVFILT2R_EL0(15), HDFGRTR2, nSPMEVTYPERn_EL0, 0), > + > + SR_FGT(SYS_SPMEVCNTR_EL0(0), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(1), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(2), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(3), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(4), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(5), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(6), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(7), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(8), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(9), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(10), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(11), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(12), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(13), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(14), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + SR_FGT(SYS_SPMEVCNTR_EL0(15), HDFGRTR2, nSPMEVCNTRn_EL0, 0), > + > + SR_FGT(SYS_PMSSCR_EL1, HDFGRTR2, nPMSSCR_EL1, 0), > + SR_FGT(SYS_PMCCNTSVR_EL1, HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMICNTSVR_EL1, HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(0), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(1), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(2), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(3), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(4), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(5), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(6), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(7), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(8), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(9), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(10), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(11), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(12), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(13), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(14), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(15), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(16), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(17), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(18), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(19), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(20), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(21), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(22), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(23), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(24), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(25), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(26), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(27), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(28), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(29), HDFGRTR2, nPMSSDATA, 0), > + SR_FGT(SYS_PMEVCNTSVR_EL1(30), HDFGRTR2, nPMSSDATA, 0), > + > + SR_FGT(SYS_MDSELR_EL1, HDFGRTR2, nMDSELR_EL1, 0), > + SR_FGT(SYS_PMUACR_EL1, HDFGRTR2, nPMUACR_EL1, 0), > + SR_FGT(SYS_PMICFILTR_EL0, HDFGRTR2, nPMICFILTR_EL0, 0), > + SR_FGT(SYS_PMICNTR_EL0, HDFGRTR2, nPMICNTR_EL0, 0), > + SR_FGT(SYS_PMIAR_EL1, HDFGRTR2, nPMIAR_EL1, 0), > + SR_FGT(SYS_PMECR_EL1, HDFGRTR2, nPMECR_EL1, 0), > }; > > static union trap_config get_trap_config(u32 sysreg) > @@ -2083,6 +2230,10 @@ static bool check_fgt_bit(struct kvm *kvm, bool is_read, > sr = is_read ? HDFGRTR_EL2 : HDFGWTR_EL2; > break; > > + case HDFGRTR2_GROUP: > + sr = is_read ? HDFGRTR2_EL2 : HDFGWTR2_EL2; > + break; > + > case HAFGRTR_GROUP: > sr = HAFGRTR_EL2; > break; > @@ -2157,6 +2308,13 @@ bool triage_sysreg_trap(struct kvm_vcpu *vcpu, int *sr_index) > val = __vcpu_sys_reg(vcpu, HDFGWTR_EL2); > break; > > + case HDFGRTR2_GROUP: > + if (is_read) > + val = __vcpu_sys_reg(vcpu, HDFGRTR2_EL2); > + else > + val = __vcpu_sys_reg(vcpu, HDFGWTR2_EL2); > + break; > + > case HAFGRTR_GROUP: > val = __vcpu_sys_reg(vcpu, HAFGRTR_EL2); > break; > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h > index f59ccfe11ab9..af6c774d9202 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h > @@ -89,6 +89,10 @@ static inline void __activate_traps_fpsimd32(struct kvm_vcpu *vcpu) > case HDFGWTR_EL2: \ > id = HDFGRTR_GROUP; \ > break; \ > + case HDFGRTR2_EL2: \ > + case HDFGWTR2_EL2: \ > + id = HDFGRTR2_GROUP; \ > + break; \ > case HAFGRTR_EL2: \ > id = HAFGRTR_GROUP; \ > break; \ > @@ -160,6 +164,8 @@ static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu) > CHECK_FGT_MASKS(HDFGWTR_EL2); > CHECK_FGT_MASKS(HAFGRTR_EL2); > CHECK_FGT_MASKS(HCRX_EL2); > + CHECK_FGT_MASKS(HDFGRTR2_EL2); > + CHECK_FGT_MASKS(HDFGWTR2_EL2); > > if (!cpus_have_final_cap(ARM64_HAS_FGT)) > return; > @@ -171,6 +177,8 @@ static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu) > update_fgt_traps(hctxt, vcpu, kvm, HFGITR_EL2); > update_fgt_traps(hctxt, vcpu, kvm, HDFGRTR_EL2); > update_fgt_traps(hctxt, vcpu, kvm, HDFGWTR_EL2); > + update_fgt_traps(hctxt, vcpu, kvm, HDFGRTR2_EL2); > + update_fgt_traps(hctxt, vcpu, kvm, HDFGWTR2_EL2); > > if (cpu_has_amu()) > update_fgt_traps(hctxt, vcpu, kvm, HAFGRTR_EL2); > @@ -200,6 +208,8 @@ static inline void __deactivate_traps_hfgxtr(struct kvm_vcpu *vcpu) > __deactivate_fgt(hctxt, vcpu, kvm, HFGITR_EL2); > __deactivate_fgt(hctxt, vcpu, kvm, HDFGRTR_EL2); > __deactivate_fgt(hctxt, vcpu, kvm, HDFGWTR_EL2); > + __deactivate_fgt(hctxt, vcpu, kvm, HDFGRTR2_EL2); > + __deactivate_fgt(hctxt, vcpu, kvm, HDFGWTR2_EL2); > > if (cpu_has_amu()) > __deactivate_fgt(hctxt, vcpu, kvm, HAFGRTR_EL2); > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c > index de789e0f1ae9..32ac5342d340 100644 > --- a/arch/arm64/kvm/nested.c > +++ b/arch/arm64/kvm/nested.c > @@ -1146,6 +1146,44 @@ int kvm_init_nv_sysregs(struct kvm *kvm) > res0 |= HDFGRTR_EL2_nPMSNEVFR_EL1; > set_sysreg_masks(kvm, HDFGRTR_EL2, res0 | HDFGRTR_EL2_RES0, res1); > > + /* HDFG[RW]TR2_EL2 */ > + res0 = res1 = 0; > + > + if (!kvm_has_feat_enum(kvm, ID_AA64DFR0_EL1, ExtTrcBuff, IMP)) > + res1 |= HDFGRTR2_EL2_nTRBMPAM_EL1; I know I told you do use 'res1' here at some point, but I also told you that I was wrong in [1], and that this should be definitely be 'res0'. I'm really sorry to have led you down the wrong path, but that's a pretty minor change (see commit cb52b5c8b81). [1] https://lore.kernel.org/kvmarm/86bk3c3uss.wl-maz@kernel.org/ > + > + if (!kvm_has_feat_enum(kvm, ID_AA64DFR0_EL1, PMSVer, V1P4)) > + res1 |= HDFGRTR2_EL2_nPMSDSFR_EL1; > + > + if (!kvm_has_feat_enum(kvm, ID_AA64DFR2_EL1, STEP, IMP)) > + res1 |= HDFGRTR2_EL2_nMDSTEPOP_EL1; > + if (!kvm_has_feat(kvm, ID_AA64DFR1_EL1, ITE, IMP)) > + res1 |= HDFGRTR2_EL2_nTRCITECR_EL1; > + if (!kvm_has_feat(kvm, ID_AA64DFR1_EL1, SPMU, IMP)) > + res1 |= (HDFGRTR2_EL2_nSPMDEVAFF_EL1 | HDFGRTR2_EL2_nSPMID | > + HDFGRTR2_EL2_nSPMSCR_EL1 | HDFGRTR2_EL2_nSPMACCESSR_EL1 | > + HDFGRTR2_EL2_nSPMCR_EL0 | HDFGRTR2_EL2_nSPMOVS | > + HDFGRTR2_EL2_nSPMINTEN | HDFGRTR2_EL2_nSPMCNTEN | > + HDFGRTR2_EL2_nSPMSELR_EL0 | HDFGRTR2_EL2_nSPMEVTYPERn_EL0 | > + HDFGRTR2_EL2_nSPMEVCNTRn_EL0); > + > + if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMSS, IMP)) > + res1 |= (HDFGRTR2_EL2_nPMSSCR_EL1 | HDFGRTR2_EL2_nPMSSDATA); > + > + if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, DebugVer, V8P9)) > + res1 |= HDFGRTR2_EL2_nMDSELR_EL1; > + if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMUVer, V3P9)) > + res1 |= HDFGRTR2_EL2_nPMUACR_EL1; > + if (!kvm_has_feat(kvm, ID_AA64DFR1_EL1, PMICNTR, IMP)) > + res1 |= (HDFGRTR2_EL2_nPMICFILTR_EL0 | HDFGRTR2_EL2_nPMICNTR_EL0); > + if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, SEBEP, IMP)) > + res1 |= HDFGRTR2_EL2_nPMIAR_EL1; > + if (!kvm_has_feat(kvm, ID_AA64DFR1_EL1, EBEP, IMP) && > + !kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMSS, IMP)) > + res1 |= HDFGRTR2_EL2_nPMECR_EL1; > + set_sysreg_masks(kvm, HDFGRTR2_EL2, res0 | HDFGRTR2_EL2_RES0, res1 | HDFGRTR2_EL2_RES1); > + set_sysreg_masks(kvm, HDFGWTR2_EL2, res0 | HDFGWTR2_EL2_RES0, res1 | HDFGWTR2_EL2_RES1); > + This is missing nPMZR_EL0, which should only apply to the write register. > /* Reuse the bits from the read-side and add the write-specific stuff */ > if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMUVer, IMP)) > res0 |= (HDFGWTR_EL2_PMCR_EL0 | HDFGWTR_EL2_PMSWINC_EL0); > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 4ea714dcb660..c971febcafab 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -4610,6 +4610,55 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu) > kvm->arch.fgu[HAFGRTR_GROUP] |= ~(HAFGRTR_EL2_RES0 | > HAFGRTR_EL2_RES1); > > + if (!kvm_has_feat_enum(kvm, ID_AA64DFR0_EL1, ExtTrcBuff, IMP)) > + kvm->arch.fgu[HDFGRTR2_GROUP] |= HDFGRTR2_EL2_nTRBMPAM_EL1; > + > + if (!kvm_has_feat_enum(kvm, ID_AA64DFR0_EL1, PMSVer, V1P4)) > + kvm->arch.fgu[HDFGRTR2_GROUP] |= HDFGRTR2_EL2_nPMSDSFR_EL1; > + > + if (!kvm_has_feat_enum(kvm, ID_AA64DFR2_EL1, STEP, IMP)) > + kvm->arch.fgu[HDFGRTR2_GROUP] |= HDFGRTR2_EL2_nMDSTEPOP_EL1; > + > + if (!kvm_has_feat(kvm, ID_AA64DFR1_EL1, ITE, IMP)) > + kvm->arch.fgu[HDFGRTR2_GROUP] |= HDFGRTR2_EL2_nTRCITECR_EL1; > + > + if (!kvm_has_feat(kvm, ID_AA64DFR1_EL1, SPMU, IMP)) > + kvm->arch.fgu[HDFGRTR2_GROUP] |= HDFGRTR2_EL2_nSPMDEVAFF_EL1 | > + HDFGRTR2_EL2_nSPMID | > + HDFGRTR2_EL2_nSPMSCR_EL1 | > + HDFGRTR2_EL2_nSPMACCESSR_EL1 | > + HDFGRTR2_EL2_nSPMCR_EL0 | > + HDFGRTR2_EL2_nSPMOVS | > + HDFGRTR2_EL2_nSPMINTEN | > + HDFGRTR2_EL2_nSPMCNTEN | > + HDFGRTR2_EL2_nSPMSELR_EL0 | > + HDFGRTR2_EL2_nSPMEVTYPERn_EL0 | > + HDFGRTR2_EL2_nSPMEVCNTRn_EL0; > + > + if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMSS, IMP)) > + kvm->arch.fgu[HDFGRTR2_GROUP] |= HDFGRTR2_EL2_nPMSSCR_EL1 | > + HDFGRTR2_EL2_nPMSSDATA; > + > + if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, DebugVer, V8P9)) > + kvm->arch.fgu[HDFGRTR2_GROUP] |= HDFGRTR2_EL2_nMDSELR_EL1; > + > + if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMUVer, V3P9)) { > + kvm->arch.fgu[HDFGRTR2_GROUP] |= HDFGRTR2_EL2_nPMUACR_EL1; > + kvm->arch.fgu[HDFGWTR2_GROUP] |= HDFGWTR2_EL2_nPMZR_EL0; The registers are different, but the *groups* are the same: if something UNDEFs for read, it also UNDEFs for write. So you can be consistent and use the 'read' name everywhere. This is also consistent with what you add in kvm_host.h. > + } > + > + if (!kvm_has_feat(kvm, ID_AA64DFR1_EL1, PMICNTR, IMP)) { > + kvm->arch.fgu[HDFGRTR2_GROUP] |= HDFGRTR2_EL2_nPMICFILTR_EL0; > + kvm->arch.fgu[HDFGRTR2_GROUP] |= HDFGRTR2_EL2_nPMICNTR_EL0; > + } > + > + if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, SEBEP, IMP)) > + kvm->arch.fgu[HDFGRTR2_GROUP] |= HDFGRTR2_EL2_nPMIAR_EL1; > + > + if (!kvm_has_feat(kvm, ID_AA64DFR1_EL1, EBEP, IMP) && > + !kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMSS, IMP)) > + kvm->arch.fgu[HDFGRTR2_GROUP] |= HDFGRTR2_EL2_nPMECR_EL1; > + > set_bit(KVM_ARCH_FLAG_FGU_INITIALIZED, &kvm->arch.flags); > out: > mutex_unlock(&kvm->arch.config_lock); I haven't looked into the fine details, but overall, this looks much better. Now, the main issues are that: - you're missing the coarse grained trapping for all the stuff you have just added. It's not a huge amount of work, but you need, for each register, to describe what traps apply to it. The fine grained stuff is most, but not all of it. There should be enough of it already to guide you through it. - this is all part of FEAT_FGT2, and that has some side effects: HFGITR2_EL2, HFGRTR2_EL2, and HFGWTR2_EL2 must also be defined. Thankfully, there is not much in them, so this should be much easier than the above. But the architecture do not give us the option to have two but not the others. Once you have all that, we should be in a reasonable place. Thanks, M. -- Without deviation from the norm, progress is not possible.