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 mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by smtp.lore.kernel.org (Postfix) with ESMTP id D363FC433F5 for ; Thu, 24 Feb 2022 14:02:45 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 1E7594BF04; Thu, 24 Feb 2022 09:02:45 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@kernel.org Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eU9CcukoI7vK; Thu, 24 Feb 2022 09:02:43 -0500 (EST) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id BBC1E4BF4D; Thu, 24 Feb 2022 09:02:43 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id ED9114BF04 for ; Thu, 24 Feb 2022 09:02:41 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id az58w-3WmwBS for ; Thu, 24 Feb 2022 09:02:40 -0500 (EST) Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 5FD974BDAC for ; Thu, 24 Feb 2022 09:02:40 -0500 (EST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id DAFD2B82520; Thu, 24 Feb 2022 14:02:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 69DD8C340E9; Thu, 24 Feb 2022 14:02:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1645711357; bh=Uu4hPx7FLwDAOG6pOtRm9yq1V8Rv43iZR9djicM5fJk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=t6wOwimUMXu7YTHZorBLpGypQo0xAWpe+raFiQPpmk7pXmCDPT4T4GA9RsvZNZ0c2 KqLKlBIL7hkiCBvGdur83b/Ae4z3WozC6TBFq0GmdqeeniraDsfv+1KpZtrREWcYH/ RFkIPbce9SGE0LF/ATglJSXFHbJLOLYze0F3rV84msVFleNfVB1jXqP6r7coeTSLBS ks4KV4aayuS63xPmEQLL8koc8pOu5iSaNjRJXvvm1q2VQENTbXbN93pfHb76/3qSKJ 61NwnfYKkCGBepF79tg/u4pFbfFvxFFgMpR2MWNOVKtGzISt3ihX7YtZKMiCni/U+m kcTbeM53jmYUw== Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1nNEhW-00ACPA-V4; Thu, 24 Feb 2022 14:02:35 +0000 Date: Thu, 24 Feb 2022 14:02:34 +0000 Message-ID: <87wnhk2whx.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton Subject: Re: [PATCH v3 09/19] KVM: arm64: Implement PSCI SYSTEM_SUSPEND In-Reply-To: <20220223041844.3984439-10-oupton@google.com> References: <20220223041844.3984439-1-oupton@google.com> <20220223041844.3984439-10-oupton@google.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/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: oupton@google.com, kvmarm@lists.cs.columbia.edu, pbonzini@redhat.com, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, anup@brainfault.org, atishp@atishpatra.org, seanjc@google.com, vkuznets@redhat.com, wanpengli@tencent.com, jmattson@google.com, joro@8bytes.org, kvm@vger.kernel.org, kvm-riscv@lists.infradead.org, pshier@google.com, reijiw@google.com, ricarkol@google.com, rananta@google.com, jingzhangos@google.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Cc: Wanpeng Li , kvm@vger.kernel.org, Joerg Roedel , Peter Shier , kvm-riscv@lists.infradead.org, Atish Patra , Paolo Bonzini , Vitaly Kuznetsov , kvmarm@lists.cs.columbia.edu, Jim Mattson X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu On Wed, 23 Feb 2022 04:18:34 +0000, Oliver Upton wrote: > > ARM DEN0022D.b 5.19 "SYSTEM_SUSPEND" describes a PSCI call that allows > software to request that a system be placed in the deepest possible > low-power state. Effectively, software can use this to suspend itself to > RAM. Note that the semantics of this PSCI call are very similar to > CPU_SUSPEND, which is already implemented in KVM. > > Implement the SYSTEM_SUSPEND in KVM. Similar to CPU_SUSPEND, the > low-power state is implemented as a guest WFI. Synchronously reset the > calling CPU before entering the WFI, such that the vCPU may immediately > resume execution when a wakeup event is recognized. > > Signed-off-by: Oliver Upton > --- > arch/arm64/kvm/psci.c | 51 ++++++++++++++++++++++++++++++++++++++++++ > arch/arm64/kvm/reset.c | 3 ++- > 2 files changed, 53 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/psci.c b/arch/arm64/kvm/psci.c > index 77a00913cdfd..41adaaf2234a 100644 > --- a/arch/arm64/kvm/psci.c > +++ b/arch/arm64/kvm/psci.c > @@ -208,6 +208,50 @@ static void kvm_psci_system_reset(struct kvm_vcpu *vcpu) > kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_RESET); > } > > +static int kvm_psci_system_suspend(struct kvm_vcpu *vcpu) > +{ > + struct vcpu_reset_state reset_state; > + struct kvm *kvm = vcpu->kvm; > + struct kvm_vcpu *tmp; > + bool denied = false; > + unsigned long i; > + > + reset_state.pc = smccc_get_arg1(vcpu); > + if (!kvm_ipa_valid(kvm, reset_state.pc)) { > + smccc_set_retval(vcpu, PSCI_RET_INVALID_ADDRESS, 0, 0, 0); > + return 1; > + } > + > + reset_state.r0 = smccc_get_arg2(vcpu); > + reset_state.be = kvm_vcpu_is_be(vcpu); > + reset_state.reset = true; > + > + /* > + * The SYSTEM_SUSPEND PSCI call requires that all vCPUs (except the > + * calling vCPU) be in an OFF state, as determined by the > + * implementation. > + * > + * See ARM DEN0022D, 5.19 "SYSTEM_SUSPEND" for more details. > + */ > + mutex_lock(&kvm->lock); > + kvm_for_each_vcpu(i, tmp, kvm) { > + if (tmp != vcpu && !kvm_arm_vcpu_powered_off(tmp)) { > + denied = true; > + break; > + } > + } > + mutex_unlock(&kvm->lock); This looks dodgy. Nothing seems to prevent userspace from setting the mp_state to RUNNING in parallel with this, as only the vcpu mutex is held when this ioctl is issued. It looks to me that what you want is what lock_all_vcpus() does (Alexandru has a patch moving it out of the vgic code as part of his SPE series). It is also pretty unclear what the interaction with userspace is once you have released the lock. If the VMM starts a vcpu other than the suspending one, what is its state? The spec doesn't see to help here. I can see two options: - either all the vcpus have the same reset state applied to them as they come up, unless they are started with CPU_ON by a vcpu that has already booted (but there is a single 'context_id' provided, and I fear this is going to confuse the OS)... - or only the suspending vcpu can resume the system, and we must fail a change of mp_state for the other vcpus. What do you think? > + > + if (denied) { > + smccc_set_retval(vcpu, PSCI_RET_DENIED, 0, 0, 0); > + return 1; > + } > + > + __kvm_reset_vcpu(vcpu, &reset_state); > + kvm_vcpu_wfi(vcpu); I have mixed feelings about this. The vcpu has reset before being in WFI, while it really should be the other way around and userspace could rely on observing the transition. What breaks if you change this? Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm