From patchwork Wed Dec 18 06:26:34 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 22621 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ie0-f198.google.com (mail-ie0-f198.google.com [209.85.223.198]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id AB25B23FC6 for ; Wed, 18 Dec 2013 06:26:47 +0000 (UTC) Received: by mail-ie0-f198.google.com with SMTP id tp5sf25872802ieb.9 for ; Tue, 17 Dec 2013 22:26:46 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:from:to:cc:subject :date:message-id:x-original-sender:x-original-authentication-results :precedence:mailing-list:list-id:list-post:list-help:list-archive :list-unsubscribe; bh=FMSxN9iHu8TxowipCN9PtGuNAdGHbbDxm02B/Xrpbgc=; b=EVRo5zm+DRSwa/atVBYthuP1YVteygmFu6p5rsB9bWX2yUUkgu7gY4SddB0NjcMEJn u85Di+YPQoieH7qKSb/RLPZSM48Pd0O3/md8E4F89xx36zyEVXUyYwTDnAF1oxmHnM4Q MeQBROXoPYsdUk59zhB1mGMgfbS0ap2bFchlMQwlNsDP+K0TZkfAcYKV/ePuNk6mDVm9 TcndnXyjMbC6BUihSukntREVFVcoClbigSwpFW16fqSnx892I4Al7evUa8JlSoFasSAK vGTOkTHxOYSqGwoTPHwMkiMCjQL/tPrWaOqU82aVghhpgaq1JKyw9WHD3vP/RmbyJuPw yEEA== X-Gm-Message-State: ALoCoQkbVCTg0OtuM6ytLkb0pGoDPbMJ0HN3hgUz7nHAKJAxNeaQnLRApSJeLv+Fv3131uSqHxiQ X-Received: by 10.42.65.138 with SMTP id l10mr108427ici.31.1387348006412; Tue, 17 Dec 2013 22:26:46 -0800 (PST) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.37.163 with SMTP id z3ls2774940qej.1.gmail; Tue, 17 Dec 2013 22:26:46 -0800 (PST) X-Received: by 10.58.44.72 with SMTP id c8mr356468vem.37.1387348006285; Tue, 17 Dec 2013 22:26:46 -0800 (PST) Received: from mail-ve0-f172.google.com (mail-ve0-f172.google.com [209.85.128.172]) by mx.google.com with ESMTPS id sx7si5451218vdc.55.2013.12.17.22.26.46 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 17 Dec 2013 22:26:46 -0800 (PST) Received-SPF: neutral (google.com: 209.85.128.172 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.128.172; Received: by mail-ve0-f172.google.com with SMTP id jw12so5050838veb.3 for ; Tue, 17 Dec 2013 22:26:46 -0800 (PST) X-Received: by 10.58.44.72 with SMTP id c8mr356464vem.37.1387348006174; Tue, 17 Dec 2013 22:26:46 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.59.13.131 with SMTP id ey3csp219295ved; Tue, 17 Dec 2013 22:26:45 -0800 (PST) X-Received: by 10.66.250.129 with SMTP id zc1mr19547768pac.153.1387348004680; Tue, 17 Dec 2013 22:26:44 -0800 (PST) Received: from mail-pa0-f49.google.com (mail-pa0-f49.google.com [209.85.220.49]) by mx.google.com with ESMTPS id tt8si13491328pbc.258.2013.12.17.22.26.44 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 17 Dec 2013 22:26:44 -0800 (PST) Received-SPF: neutral (google.com: 209.85.220.49 is neither permitted nor denied by best guess record for domain of christoffer.dall@linaro.org) client-ip=209.85.220.49; Received: by mail-pa0-f49.google.com with SMTP id kx10so5507179pab.22 for ; Tue, 17 Dec 2013 22:26:44 -0800 (PST) X-Received: by 10.66.142.107 with SMTP id rv11mr32165335pab.17.1387348004009; Tue, 17 Dec 2013 22:26:44 -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 nw11sm52114931pab.13.2013.12.17.22.26.41 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 17 Dec 2013 22:26:42 -0800 (PST) From: Christoffer Dall To: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: linaro-kernel@lists.linaro.org, patches@linaro.org, Christoffer Dall Subject: [PATCH v2] arm: KVM: Don't return PSCI_INVAL if waitqueue is inactive Date: Tue, 17 Dec 2013 22:26:34 -0800 Message-Id: <1387347994-3415-1-git-send-email-christoffer.dall@linaro.org> X-Mailer: git-send-email 1.8.5 X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: christoffer.dall@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.128.172 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , 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. Instead we should check specifically that the CPU is marked as being turned off, regardless of the VCPU thread state, and if it is, we shall 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. 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. Reported-by: Peter Maydell Signed-off-by: Christoffer Dall Acked-by: Marc Zyngier --- Changes[v2]: - Use non-atomic version of test_and_clear_bit instead - Check if vcpu is paused and return KVM_PSCI_RET_INVAL if not - Remove unnecessary feature bit clear arch/arm/kvm/arm.c | 30 +++++++++++++++++++----------- arch/arm/kvm/psci.c | 11 ++++++----- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index 2a700e0..151eb91 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..448f60e 100644 --- a/arch/arm/kvm/psci.c +++ b/arch/arm/kvm/psci.c @@ -54,15 +54,15 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) } } - if (!vcpu) + /* + * Make sure the caller requested a valid CPU and that the CPU is + * turned off. + */ + if (!vcpu || !vcpu->arch.pause) return KVM_PSCI_RET_INVAL; 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 */ @@ -79,6 +79,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) 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;