From patchwork Wed Nov 20 18:51:39 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 3214951 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 7CA279F26C for ; Wed, 20 Nov 2013 18:50:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 28C1B20738 for ; Wed, 20 Nov 2013 18:50:56 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AC59720725 for ; Wed, 20 Nov 2013 18:50:54 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VjCrQ-0002SQ-RN; Wed, 20 Nov 2013 18:50:48 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VjCrO-0002ju-BN; Wed, 20 Nov 2013 18:50:46 +0000 Received: from mail-pd0-f174.google.com ([209.85.192.174]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VjCrK-0002ih-1z for linux-arm-kernel@lists.infradead.org; Wed, 20 Nov 2013 18:50:43 +0000 Received: by mail-pd0-f174.google.com with SMTP id y13so3826623pdi.5 for ; Wed, 20 Nov 2013 10:50:19 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=UKPYNC14LODEcwlRR7nqY6un9kkOc2PwukmppCj0Njw=; b=Lp+7PLQZIEPw/vPFzBF+iIbM0sAF6lMW/I5MbIDcQJczBNvH8rlGWQ51pKFqti8zWR IhEWGHltamSxiWBjaezZ6oD64Cw7Agq050kfz7t1PXLwP+oyFWk2A/KOYGS1NzfaCDSQ 6e7It5uum8GZypRmqgBlQxIjrOlAD7CtETE2i99EVYo87dRG+zXdIdUDv/70CRiiRRwR dURlw8bpGZ8QPCcAfpUBZQJWUdy/k/DJFlHIYtXbCP1emp6mleigffUMuc4IRGCbet4/ syKg5iPr4qa5I8MM8azCc5N545N1cQna5B6w4q2jfuyQBRHCn9ZHo64PErJxHxyacEMu 7G4Q== X-Gm-Message-State: ALoCoQm1yzkxOteIB6ehHE6DeDD5BC+fl1i9/GmXRw09dMZsnWuhaJhPl+btnU5+ENsuI5ytcbCo X-Received: by 10.68.241.5 with SMTP id we5mr2274162pbc.34.1384973419500; Wed, 20 Nov 2013 10:50:19 -0800 (PST) Received: from localhost.localdomain (c-67-169-181-221.hsd1.ca.comcast.net. [67.169.181.221]) by mx.google.com with ESMTPSA id m2sm39642059pbn.19.2013.11.20.10.50.17 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 20 Nov 2013 10:50:18 -0800 (PST) From: Christoffer Dall To: kvmarm@lists.cs.columbia.edu Subject: [PATCH] arm: KVM: Don't return PSCI_INVAL if waitqueue is inactive Date: Wed, 20 Nov 2013 10:51:39 -0800 Message-Id: <1384973499-30659-1-git-send-email-christoffer.dall@linaro.org> X-Mailer: git-send-email 1.8.4.3 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20131120_135042_197292_A5F8637F X-CRM114-Status: GOOD ( 20.46 ) X-Spam-Score: -1.9 (-) Cc: Christoffer Dall , Peter Maydell , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Marc Zyngier X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The current KVM implementation of PSCI returns INVALID_PARAMETERS if the waitqueue for the corresponding CPU is not active. This does not seem correct, since KVM should not care what the specific thread is doing, for example, user space may not have called KVM_RUN on this VCPU yet or the thread may be busy looping to user space because it received a signal; this is really up to the user space implementation. We should simply clear the pause flag on the CPU and wake up the thread if it happens to be blocked for us. Further, the implementation seems to be racy when executing multiple VCPU threads. There really isn't a reasonable user space programming scheme to ensure all secondary CPUs have reached kvm_vcpu_first_run_init before turning on the boot CPU. It also does not make much sense to call into the PSCI code for a CPU that is turned off - after all, it cannot do anything if it is turned off and PSCI code could reasonably be written with the assumption that the VCPU is actually up, in some shape or form. Therefore, set the pause flag on the vcpu at VCPU init time (which can reasonably be expected to be completed for all CPUs by user space before running any VCPUs) and clear both this flag and the feature (in case the feature can somehow get set again in the future) and ping the waitqueue on turning on a VCPU using PSCI. Cc: Marc Zyngier Reported-by: Peter Maydell Signed-off-by: Christoffer Dall --- arch/arm/kvm/arm.c | 30 +++++++++++++++++++----------- arch/arm/kvm/psci.c | 6 ++---- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index e312e4a..1140e0e 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -478,15 +478,6 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu) return ret; } - /* - * Handle the "start in power-off" case by calling into the - * PSCI code. - */ - if (test_and_clear_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features)) { - *vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF; - kvm_psci_call(vcpu); - } - return 0; } @@ -700,6 +691,24 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level, return -EINVAL; } +static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu, + struct kvm_vcpu_init *init) +{ + int ret; + + ret = kvm_vcpu_set_target(vcpu, init); + if (ret) + return ret; + + /* + * Handle the "start in power-off" case by marking the VCPU as paused. + */ + if (test_and_clear_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features)) + vcpu->arch.pause = true; + + return 0; +} + long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) { @@ -713,8 +722,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp, if (copy_from_user(&init, argp, sizeof(init))) return -EFAULT; - return kvm_vcpu_set_target(vcpu, &init); - + return kvm_arch_vcpu_ioctl_vcpu_init(vcpu, &init); } case KVM_SET_ONE_REG: case KVM_GET_ONE_REG: { diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c index 0881bf1..2e72ef5 100644 --- a/arch/arm/kvm/psci.c +++ b/arch/arm/kvm/psci.c @@ -59,10 +59,6 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) target_pc = *vcpu_reg(source_vcpu, 2); - wq = kvm_arch_vcpu_wq(vcpu); - if (!waitqueue_active(wq)) - return KVM_PSCI_RET_INVAL; - kvm_reset_vcpu(vcpu); /* Gracefully handle Thumb2 entry point */ @@ -76,9 +72,11 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) kvm_vcpu_set_be(vcpu); *vcpu_pc(vcpu) = target_pc; + clear_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features); vcpu->arch.pause = false; smp_mb(); /* Make sure the above is visible */ + wq = kvm_arch_vcpu_wq(vcpu); wake_up_interruptible(wq); return KVM_PSCI_RET_SUCCESS;