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 C6267C61DA4 for ; Sat, 11 Feb 2023 10:09:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=OZpsCRVSpDBIodaU17sAbxcrmY+2VYMKYH9uCdY746I=; b=qOg3i1GHCdjNUt qq6s84C8qpwkLT5pJFvheOlPa/oC4qL8CXn07yKH5HEvWFHhRAwcn4rvwLQDeBBTijBXmlc+cv7iU fT3ytOucDRQtK/poeLe49q+OwPH8VZRkLYdxyDNSeAZ6Z8vayUVTgbt1/RBdg3VTJqrDx7fonbJRq WWzHdIdxRQB0CovD1zcqTQadrACE4awiNXV1JN4EXxe4TQb8jD0/xqxJt2DwtXaL6sATpj7LCVv2T BzYEXuaYGkkCrncuKcqlnWWym5my+RtnuR6rpUQ+QAQle2mHT0n211Q5NSUcwS4FqGo99QETU8oJs g6HBQ63QU1IUuxTalg2w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pQmnX-009BPe-JX; Sat, 11 Feb 2023 10:07:59 +0000 Received: from out-128.mta1.migadu.com ([95.215.58.128]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pQmnS-009BOa-UK for linux-arm-kernel@lists.infradead.org; Sat, 11 Feb 2023 10:07:57 +0000 Date: Sat, 11 Feb 2023 10:07:41 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1676110066; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=ZRjvYb4Ksjm5xGoHPb/oX2KqaO6lf9hahhWphjlh6Sw=; b=tbDRjwwuSot4VnQgVbi1KYnT0Y5ZcH/okX00hHSfyQy/lgjjbP282uHPZ0R6C8/JJUoPh+ RdCwIgZnDpZH84l7XygAiMCB9Tqym+FIGrtp3NZprj551+3ARQQSh9HPa9pilVkQJ1K3Pe drsbcbv0I7ARQI1tykk3ynZg00jM1n0= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Oliver Upton To: Marc Zyngier Cc: kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Alexandru Elisei , Andre Przywara , Catalin Marinas , Christoffer Dall , Ganapatrao Kulkarni , Russell King , James Morse , Suzuki K Poulose , Zenghui Yu Subject: Re: [PATCH 12/18] KVM: arm64: nv: Handle PSCI call via smc from the guest Message-ID: References: <20230209175820.1939006-1-maz@kernel.org> <20230209175820.1939006-13-maz@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230209175820.1939006-13-maz@kernel.org> X-Migadu-Flow: FLOW_OUT X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230211_020755_315906_85037FD0 X-CRM114-Status: GOOD ( 35.18 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Marc, On Thu, Feb 09, 2023 at 05:58:14PM +0000, Marc Zyngier wrote: > From: Jintack Lim > > VMs used to execute hvc #0 for the psci call if EL3 is not implemented. > However, when we come to provide the virtual EL2 mode to the VM, the > host OS inside the VM calls kvm_call_hyp() which is also hvc #0. So, > it's hard to differentiate between them from the host hypervisor's point > of view. > > So, let the VM execute smc instruction for the psci call. On ARMv8.3, > even if EL3 is not implemented, a smc instruction executed at non-secure > EL1 is trapped to EL2 if HCR_EL2.TSC==1, rather than being treated as > UNDEFINED. So, the host hypervisor can handle this psci call without any > confusion. I think this commit message is rather stale to the point of being rather misleading. This lets the vEL2 get at the entire gamut of SMCCC calls we have in KVM, not just PSCI. Of course, no problem with that since it is a requirement, but for posterity the commit message should reflect the current state of KVM. If I may suggest: Non-nested guests have used the hvc instruction to initiate SMCCC calls into KVM. This is quite a poor fit for NV as hvc exceptions are always taken to EL2. In other words, KVM needs to unconditionally forward the hvc exception back into vEL2 to uphold the architecture. Instead, treat the smc instruction from vEL2 as we would a guest hypercall, thereby allowing the vEL2 to interact with KVM's hypercall surface. Note that on NV-capable hardware HCR_EL2.TSC causes smc instructions executed in non-secure EL1 to trap to EL2, even if EL3 is not implemented. > Reviewed-by: Alexandru Elisei > Signed-off-by: Jintack Lim > Signed-off-by: Marc Zyngier > --- > arch/arm64/kvm/handle_exit.c | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > index e75101f2aa6c..b0c343c4e062 100644 > --- a/arch/arm64/kvm/handle_exit.c > +++ b/arch/arm64/kvm/handle_exit.c > @@ -63,6 +63,8 @@ static int handle_hvc(struct kvm_vcpu *vcpu) > > static int handle_smc(struct kvm_vcpu *vcpu) > { > + int ret; > + > /* > * "If an SMC instruction executed at Non-secure EL1 is > * trapped to EL2 because HCR_EL2.TSC is 1, the exception is a > @@ -70,10 +72,28 @@ static int handle_smc(struct kvm_vcpu *vcpu) > * > * We need to advance the PC after the trap, as it would > * otherwise return to the same address... > + * > + * If imm is non-zero, it's not defined, so just skip it. > + */ > + if (kvm_vcpu_hvc_get_imm(vcpu)) { > + vcpu_set_reg(vcpu, 0, ~0UL); > + kvm_incr_pc(vcpu); > + return 1; > + } > + > + /* > + * If imm is zero, it's a psci call. > + * Note that on ARMv8.3, even if EL3 is not implemented, SMC executed > + * at Non-secure EL1 is trapped to EL2 if HCR_EL2.TSC==1, rather than > + * being treated as UNDEFINED. > */ > - vcpu_set_reg(vcpu, 0, ~0UL); > + ret = kvm_hvc_call_handler(vcpu); > + if (ret < 0) > + vcpu_set_reg(vcpu, 0, ~0UL); > + This also has the subtle effect of allowing smc instructions from a non-nested guest to hit our hypercall surface too. I think we should avoid this and only handle smcs that actually come from a vEL2. What do you think about the following? I can squash in all of the changes I've asked for here. diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index b0c343c4e062..a798c0b4d717 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -73,16 +73,18 @@ static int handle_smc(struct kvm_vcpu *vcpu) * We need to advance the PC after the trap, as it would * otherwise return to the same address... * - * If imm is non-zero, it's not defined, so just skip it. + * Only handle SMCs from the virtual EL2 with an immediate of zero and + * skip it otherwise. */ - if (kvm_vcpu_hvc_get_imm(vcpu)) { + if (!vcpu_is_el2(vcpu) || kvm_vcpu_hvc_get_imm(vcpu)) { vcpu_set_reg(vcpu, 0, ~0UL); kvm_incr_pc(vcpu); return 1; } /* - * If imm is zero, it's a psci call. + * If imm is zero then it is likely an SMCCC call. + * * Note that on ARMv8.3, even if EL3 is not implemented, SMC executed * at Non-secure EL1 is trapped to EL2 if HCR_EL2.TSC==1, rather than * being treated as UNDEFINED. -- Thanks, Oliver _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel