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 78002C27C6E for ; Fri, 14 Jun 2024 13:10:04 +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=IObfs7M1IdXQST7giuvwO43N944y+BwuqJtNhZRpJCY=; b=uJ9Gt6OEzqrlILKWDvmWG+7Lek VHPuf4LnxGbiZ+NAHfyAjgGbOJTweRJsWZZLiEBwgz6EjvnqXxhXLMGEDguFWT7OnN8S1FBsbw9md z7w+OgooigB2bm0DhqTWYxnYwMW+8felcsDq+z0Bm3IrGgZKokI1uCljGQBVJiug0lPthyyfRiWIL vx2VrGuKNcH6EH8u/LYTfZ6nOLuii8TaPchFTwwqp/drnUyjGRB5S+7kifGAw5dM1kbIYmPj7k94S KHvZFoiBjqSFQq0yDWc4RtHk/M2dD0Vq9ZwqnYHBRUTtg+WVE3TTEBUUY4YDFIvX1Oh6V4x7N7XLv vGnF43mw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sI6ge-00000002pmJ-0Fs1; Fri, 14 Jun 2024 13:09:48 +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 1sI6ga-00000002plY-363E for linux-arm-kernel@lists.infradead.org; Fri, 14 Jun 2024 13:09:46 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 8572FCE2A04; Fri, 14 Jun 2024 13:09:42 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8DA75C2BD10; Fri, 14 Jun 2024 13:09:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1718370581; bh=e6kELu63/VjfYReNnjC+nzRVVQndtg244gQ0BkXtEas=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=GMdkC05sdOfXAbzWVw0x4cllrfBOw1WbwFkTaZEPbGHlTbCEV8CdBnTOTAfbqtV81 L/dc5s0btl5IxUF5kDlDfPylNBSwg3XrdCLfwSZ6ofWioZqGGKC6LqLU/chSlmVnFc r07SteGAipSte7fIC4OHHpBFdCdw4rSkyAD++A1aSWu9QgJfJEOIpUUJ0cvVZ/75Gl iZMq94ZyrhE2eDTbvZpGt+0H/KV7vD9oaz91xb1HLDnV0a0F9GFEMLmV6iSi+3xlJ5 OJLf03OciKF0H67qiS6ng98tIuGQwMLW741PPu1XMIpwgPSaCBk8EJYhktskfGhhMP VmW6eYg2DXWoQ== 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 1sI6gV-003vHX-96; Fri, 14 Jun 2024 14:09:39 +0100 Date: Fri, 14 Jun 2024 14:09:38 +0100 Message-ID: <86r0czk6wd.wl-maz@kernel.org> From: Marc Zyngier To: Anshuman Khandual Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, will@kernel.org, catalin.marinas@arm.com, mark.rutland@arm.com, Mark Brown , James Clark , Rob Herring , Suzuki Poulose , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , linux-perf-users@vger.kernel.org, Oliver Upton , James Morse , kvmarm@lists.linux.dev Subject: Re: [PATCH V18 2/9] KVM: arm64: Explicitly handle BRBE traps as UNDEFINED In-Reply-To: <86sexfk8ke.wl-maz@kernel.org> References: <20240613061731.3109448-1-anshuman.khandual@arm.com> <20240613061731.3109448-3-anshuman.khandual@arm.com> <86sexfk8ke.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.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 X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: anshuman.khandual@arm.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, will@kernel.org, catalin.marinas@arm.com, mark.rutland@arm.com, broonie@kernel.org, james.clark@arm.com, robh@kernel.org, suzuki.poulose@arm.com, peterz@infradead.org, mingo@redhat.com, acme@kernel.org, linux-perf-users@vger.kernel.org, oliver.upton@linux.dev, james.morse@arm.com, kvmarm@lists.linux.dev 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-20240614_060945_320550_2891F06B X-CRM114-Status: GOOD ( 45.61 ) 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 Fri, 14 Jun 2024 13:33:37 +0100, Marc Zyngier wrote: > > On Thu, 13 Jun 2024 07:17:24 +0100, > Anshuman Khandual wrote: > > > > The Branch Record Buffer Extension (BRBE) adds a number of system registers > > and instructions, which we don't currently intend to expose to guests. Our > > existing logic handles this safely, but this could be improved with some > > explicit handling of BRBE. > > > > The presence of BRBE is currently hidden from guests as the cpufeature > > code's ftr_id_aa64dfr0[] table doesn't have an entry for the BRBE field, > > and so this will be zero in the sanitised value of ID_AA64DFR0 exposed to > > guests via read_sanitised_id_aa64dfr0_el1(). As the ftr_id_aa64dfr0[] table > > may gain an entry for the BRBE field in future, for robustness we should > > explicitly mask out the BRBE field in read_sanitised_id_aa64dfr0_el1(). > > > > The BRBE system registers and instructions are currently trapped by the > > existing configuration of the fine-grained traps. As neither the registers > > nor the instructions are described in the sys_reg_descs[] table, > > emulate_sys_reg() will warn that these are unknown before injecting an > > UNDEFINED exception into the guest. > > > > Well-behaved guests shouldn't try to use the registers or instructions, but > > badly-behaved guests could use these, resulting in unnecessary warnings. To > > avoid those warnings, we should explicitly handle the BRBE registers and > > instructions as UNDEFINED. > > > > Address the above by having read_sanitised_id_aa64dfr0_el1() mask out the > > ID_AA64DFR0.BRBE field, and explicitly handling all of the BRBE system > > registers and instructions as UNDEFINED. > > > > Cc: Marc Zyngier > > Cc: Oliver Upton > > Cc: James Morse > > Cc: Suzuki K Poulose > > Cc: Catalin Marinas > > Cc: Will Deacon > > Cc: kvmarm@lists.linux.dev > > Cc: linux-arm-kernel@lists.infradead.org > > Cc: linux-kernel@vger.kernel.org > > Signed-off-by: Anshuman Khandual > > ---- > > Changes in V18: > > > > - Updated the commit message > > > > arch/arm64/kvm/sys_regs.c | 56 +++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 56 insertions(+) > > Reviewed-by: Mark Rutland > > --- > > arch/arm64/kvm/sys_regs.c | 56 +++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 56 insertions(+) > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index 22b45a15d068..3d4686abe5ee 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -1304,6 +1304,11 @@ static int set_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r, > > return 0; > > } > > > > +#define BRB_INF_SRC_TGT_EL1(n) \ > > + { SYS_DESC(SYS_BRBINF_EL1(n)), undef_access }, \ > > + { SYS_DESC(SYS_BRBSRC_EL1(n)), undef_access }, \ > > + { SYS_DESC(SYS_BRBTGT_EL1(n)), undef_access } \ > > + > > /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */ > > #define DBG_BCR_BVR_WCR_WVR_EL1(n) \ > > { SYS_DESC(SYS_DBGBVRn_EL1(n)), \ > > @@ -1722,6 +1727,9 @@ static u64 read_sanitised_id_aa64dfr0_el1(struct kvm_vcpu *vcpu, > > /* Hide SPE from guests */ > > val &= ~ID_AA64DFR0_EL1_PMSVer_MASK; > > > > + /* Hide BRBE from guests */ > > + val &= ~ID_AA64DFR0_EL1_BRBE_MASK; > > + > > return val; > > } > > > > @@ -2240,6 +2248,52 @@ static const struct sys_reg_desc sys_reg_descs[] = { > > { SYS_DESC(SYS_DBGCLAIMCLR_EL1), trap_raz_wi }, > > { SYS_DESC(SYS_DBGAUTHSTATUS_EL1), trap_dbgauthstatus_el1 }, > > > > + /* > > + * BRBE branch record sysreg address space is interleaved between > > + * corresponding BRBINF_EL1, BRBSRC_EL1, and BRBTGT_EL1. > > + */ > > + BRB_INF_SRC_TGT_EL1(0), > > + BRB_INF_SRC_TGT_EL1(16), > > + BRB_INF_SRC_TGT_EL1(1), > > + BRB_INF_SRC_TGT_EL1(17), > > + BRB_INF_SRC_TGT_EL1(2), > > + BRB_INF_SRC_TGT_EL1(18), > > + BRB_INF_SRC_TGT_EL1(3), > > + BRB_INF_SRC_TGT_EL1(19), > > + BRB_INF_SRC_TGT_EL1(4), > > + BRB_INF_SRC_TGT_EL1(20), > > + BRB_INF_SRC_TGT_EL1(5), > > + BRB_INF_SRC_TGT_EL1(21), > > + BRB_INF_SRC_TGT_EL1(6), > > + BRB_INF_SRC_TGT_EL1(22), > > + BRB_INF_SRC_TGT_EL1(7), > > + BRB_INF_SRC_TGT_EL1(23), > > + BRB_INF_SRC_TGT_EL1(8), > > + BRB_INF_SRC_TGT_EL1(24), > > + BRB_INF_SRC_TGT_EL1(9), > > + BRB_INF_SRC_TGT_EL1(25), > > + BRB_INF_SRC_TGT_EL1(10), > > + BRB_INF_SRC_TGT_EL1(26), > > + BRB_INF_SRC_TGT_EL1(11), > > + BRB_INF_SRC_TGT_EL1(27), > > + BRB_INF_SRC_TGT_EL1(12), > > + BRB_INF_SRC_TGT_EL1(28), > > + BRB_INF_SRC_TGT_EL1(13), > > + BRB_INF_SRC_TGT_EL1(29), > > + BRB_INF_SRC_TGT_EL1(14), > > + BRB_INF_SRC_TGT_EL1(30), > > + BRB_INF_SRC_TGT_EL1(15), > > + BRB_INF_SRC_TGT_EL1(31), > > + > > + /* Remaining BRBE sysreg addresses space */ > > + { SYS_DESC(SYS_BRBCR_EL1), undef_access }, > > + { SYS_DESC(SYS_BRBFCR_EL1), undef_access }, > > + { SYS_DESC(SYS_BRBTS_EL1), undef_access }, > > + { SYS_DESC(SYS_BRBINFINJ_EL1), undef_access }, > > + { SYS_DESC(SYS_BRBSRCINJ_EL1), undef_access }, > > + { SYS_DESC(SYS_BRBTGTINJ_EL1), undef_access }, > > + { SYS_DESC(SYS_BRBIDR0_EL1), undef_access }, > > + > > { SYS_DESC(SYS_MDCCSR_EL0), trap_raz_wi }, > > { SYS_DESC(SYS_DBGDTR_EL0), trap_raz_wi }, > > // DBGDTR[TR]X_EL0 share the same encoding > > @@ -2751,6 +2805,8 @@ static struct sys_reg_desc sys_insn_descs[] = { > > { SYS_DESC(SYS_DC_CISW), access_dcsw }, > > { SYS_DESC(SYS_DC_CIGSW), access_dcgsw }, > > { SYS_DESC(SYS_DC_CIGDSW), access_dcgsw }, > > + { SYS_DESC(OP_BRB_IALL), undef_access }, > > + { SYS_DESC(OP_BRB_INJ), undef_access }, > > }; > > > > static const struct sys_reg_desc *first_idreg; > > I don't think we need any update to the sys_reg table to handle > this. Instead, we should make use of the FGU infrastructure that has > been in since 6.9 to make this stuff UNDEF unconditionally. > > It should be as simple as: > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index ee33f5467ce5..7cafe3f72c01 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -4964,6 +4964,11 @@ void kvm_init_sysreg(struct kvm_vcpu *vcpu) > kvm->arch.fgu[HAFGRTR_GROUP] |= ~(HAFGRTR_EL2_RES0 | > HAFGRTR_EL2_RES1); > > + if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, BRBE, IMP)) > + kvm->arch.fgu[HDFGRTR_GROUP] |= (HDFGRTR_nBRBDATA | > + HDFGRTR_nBRBCTL | > + HDFGRTR_nBRBIDR); > + > set_bit(KVM_ARCH_FLAG_FGU_INITIALIZED, &kvm->arch.flags); > out: > mutex_unlock(&kvm->arch.config_lock); > > which is of course untested, but that I expect to be correct. Actually, to disable the *instructions*, a similar hack must be applied to HFGITR_EL2. The resulting patch should be something like: diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index ee33f5467ce5..49d86dae8d80 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -4964,6 +4964,15 @@ void kvm_init_sysreg(struct kvm_vcpu *vcpu) kvm->arch.fgu[HAFGRTR_GROUP] |= ~(HAFGRTR_EL2_RES0 | HAFGRTR_EL2_RES1); + if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, BRBE, IMP)) { + kvm->arch.fgu[HDFGRTR_GROUP] |= (HDFGRTR_nBRBDATA | + HDFGRTR_nBRBCTL | + HDFGRTR_nBRBIDR); + kvm->arch.fgu[HFGITR_GROUP] |= (HFGITR_EL2_nBRBINJ | + HFGITR_EL2_nBRBIALL); + } + + set_bit(KVM_ARCH_FLAG_FGU_INITIALIZED, &kvm->arch.flags); out: mutex_unlock(&kvm->arch.config_lock); The implicit dependency here is that FGT is always present on a system that implements BRBE. The architecture supports this assertion: - BRBE is not available before ARMv9.1 - FGT is mandatory from ARMv8.6 Given that v9.1 is congruent to v8.6, we have the required overlap. Thanks, M. -- Without deviation from the norm, progress is not possible.