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 B4188C021B8 for ; Tue, 4 Mar 2025 17:29:53 +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=BRYVBNT3UX5tKseqbkeR8Ky6v2RDsGR/416NhR0ne6c=; b=PG8xw9nBHYF9izjZTrnW78QobU Kar8/ygm4EmqHG6DUvT1NXenO4/Cd9hWSQHxSdDOjG3LyHIhQSX17s160dBwVWou28WWO93yZHMGX +d6LAy1c4B430XVpMyk+OEW3nOlLuE1bCFBwvi3sfspXEw2QA+SxXjbTKoGGGFbKxiIHgDnXhfvYr +StEXZBiL1YOofmu8atUIC0+Xh99Tc9KZWzOW56l0txYSMTrbZm1Guu3GzF3Idyd/3t+7tPXm5omp v2Dk8OZnZvplzE71XJhkhpxQve3bax8ssw+Z+buQHfNlSBp9+7GPyNcHIEhBLpphW7vWuPBX89E2n PLrpJ6Tw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tpW5R-00000005dB2-3N2M; Tue, 04 Mar 2025 17:29:45 +0000 Received: from nyc.source.kernel.org ([2604:1380:45d1:ec00::3]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1tpU9E-00000005CcL-1Nif for linux-arm-kernel@lists.infradead.org; Tue, 04 Mar 2025 15:25:34 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 9FA01A4577F; Tue, 4 Mar 2025 15:20:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 26B05C4CEE5; Tue, 4 Mar 2025 15:25:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741101931; bh=Q3MX4rkE4OTeTa/jKAc1daOdke8WhwOrg3oQElBMyBI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=ochd6w06w2ZkuDI3gJgfDWEZAlMwLfvaOSP+Wp4+GDsolwX4NxSeF+OYTX7Gy4uwo 7bMS/G7TgXyclh+c/cRhO8xVOnQv7i4+2m5hrMM7o7pyjUYP7/JLLtmLpqAtWCN048 JTt6qLBduehqTXwMjboEXQvpb/YBcCi0mNTghujFDvc8VYk2KU9gdhm41nt98Ou2xL DeTY39HMi641ZAPySmps05l0RGTg72Cu4t/FX3oObi4bzhGwwAN3U78sBxToOg57vv BrR4UA4xwlR0uBFWQ/vW0/atfHnH/UU2y1/jETtQeysgfREG3I/nnIuimAQ34ieYM3 NsuBwvN1sNRQA== 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 1tpU9A-00AIEh-SR; Tue, 04 Mar 2025 15:25:28 +0000 Date: Tue, 04 Mar 2025 15:25:28 +0000 Message-ID: <86frjsq0lz.wl-maz@kernel.org> From: Marc Zyngier To: Fuad Tabba Cc: kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Joey Gouly , Suzuki K Poulose , Oliver Upton , Zenghui Yu , Mark Rutland Subject: Re: [PATCH 03/18] KVM: arm64: Handle trapping of FEAT_LS64* instructions In-Reply-To: References: <20250210184150.2145093-1-maz@kernel.org> <20250210184150.2145093-4-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.4 (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: tabba@google.com, kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, joey.gouly@arm.com, suzuki.poulose@arm.com, oliver.upton@linux.dev, yuzenghui@huawei.com, mark.rutland@arm.com 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-20250304_072532_506951_02304516 X-CRM114-Status: GOOD ( 32.78 ) 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 Hi Fuad, On Tue, 04 Mar 2025 14:36:19 +0000, Fuad Tabba wrote: > > Hi Marc, > > On Mon, 10 Feb 2025 at 18:42, Marc Zyngier wrote: > > > > We generally don't expect FEAT_LS64* instructions to trap, unless > > they are trapped by a guest hypervisor. > > > > Otherwise, this is just the guest playing tricks on us by using > > an instruction that isn't advertised, which we handle with a well > > deserved UNDEF. > > > > Signed-off-by: Marc Zyngier > > --- > > arch/arm64/kvm/handle_exit.c | 64 ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 64 insertions(+) > > > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > > index 512d152233ff2..4f8354bf7dc5f 100644 > > --- a/arch/arm64/kvm/handle_exit.c > > +++ b/arch/arm64/kvm/handle_exit.c > > @@ -294,6 +294,69 @@ static int handle_svc(struct kvm_vcpu *vcpu) > > return 1; > > } > > > > +static int handle_ls64b(struct kvm_vcpu *vcpu) > > +{ > > + struct kvm *kvm = vcpu->kvm; > > + u64 esr = kvm_vcpu_get_esr(vcpu); > > + u64 iss = ESR_ELx_ISS(esr); > > + bool allowed; > > + > > + switch (iss) { > > + case ESR_ELx_ISS_ST64BV: > > + allowed = kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64_V); > > + break; > > + case ESR_ELx_ISS_ST64BV0: > > + allowed = kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64_ACCDATA); > > + break; > > + case ESR_ELx_ISS_LDST64B: > > + allowed = kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64); > > + break; > > + default: > > + /* Clearly, we're missing something. */ > > + goto unknown_trap; > > + } > > + > > + if (!allowed) > > + goto undef; > > + > > + if (vcpu_has_nv(vcpu) && !is_hyp_ctxt(vcpu)) { > > + u64 hcrx = __vcpu_sys_reg(vcpu, HCRX_EL2); > > + bool fwd; > > + > > + switch (iss) { > > + case ESR_ELx_ISS_ST64BV: > > + fwd = !(hcrx & HCRX_EL2_EnASR); > > + break; > > + case ESR_ELx_ISS_ST64BV0: > > + fwd = !(hcrx & HCRX_EL2_EnAS0); > > + break; > > + case ESR_ELx_ISS_LDST64B: > > + fwd = !(hcrx & HCRX_EL2_EnALS); > > + break; > > + default: > > + /* We don't expect to be here */ > > + fwd = false; > > + } > > + > > + if (fwd) { > > + kvm_inject_nested_sync(vcpu, esr); > > + return 1; > > + } > > + } > > + > > +unknown_trap: > > + /* > > + * If we land here, something must be very wrong, because we > > + * have no idea why we trapped at all. Warn and undef as a > > + * fallback. > > + */ > > + WARN_ON(1); > > nit: should this be WARN_ONCE() instead? > > > + > > +undef: > > + kvm_inject_undefined(vcpu); > > + return 1; > > +} > > I'm wondering if this can be simplified by having one switch() > statement that toggles both allowed and fwd (or maybe even only fwd), > and then inject depending on that, e.g., > > +static int handle_ls64b(struct kvm_vcpu *vcpu) > +{ > + struct kvm *kvm = vcpu->kvm; > + bool is_nv = vcpu_has_nv(vcpu) && !is_hyp_ctxt(vcpu); > + u64 hcrx = __vcpu_sys_reg(vcpu, HCRX_EL2); > + u64 esr = kvm_vcpu_get_esr(vcpu); > + u64 iss = ESR_ELx_ISS(esr); > + bool fwd = false; > + > + switch (iss) { > + case ESR_ELx_ISS_ST64BV: > + fwd = kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64_V) && > + !(hcrx & HCRX_EL2_EnASR) > + break; > ... > + default: > + WARN_ONCE(1); > + } > + > + if (is_nv && fwd) { > + kvm_inject_nested_sync(vcpu, esr); > + else > + kvm_inject_undefined(vcpu); > + > + return 1; > +} > > I think this has the same effect as the code above. Yeah, I think something like that could work. I initially was trying to handle all 4 states (allowed, fwd), but obviously some states do not exist (you don't forward something that isn't allowed). My only concern here is that we don't have clear rules on the init of EL2 guest registers such as HCRX_EL2 when !NV, but that's something that can be easily done (or even worked around locally). Maybe we should introduce the dreaded notion of "effective value" for EL2 registers when NV is not enabled... Thanks, M. -- Without deviation from the norm, progress is not possible.