From patchwork Wed Nov 20 19:13:17 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 21658 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ob0-f197.google.com (mail-ob0-f197.google.com [209.85.214.197]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id BFAD6202E6 for ; Wed, 20 Nov 2013 19:13:22 +0000 (UTC) Received: by mail-ob0-f197.google.com with SMTP id va2sf2486298obc.4 for ; Wed, 20 Nov 2013 11:13:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mime-version:in-reply-to:references :date:message-id:subject:from:to:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :list-post:list-help:list-archive:list-unsubscribe:content-type; bh=Y3LOTXBbKUgm099WScqeRaQxIWHqGKBDRcxA4XeFCXE=; b=KBctiNdQwsqSCNY/LYFAzfQhRR6vT7JVBOHEUVKUVcg2k4URwcnLMwx6giImga3xHA Ug2Y477nAxci6oybWA7OuRXIHyHSoj3uYgNOB355HeAdN8PSXVMix1N/aJJbMsYQoSh4 OkMveOLRcnFBKLNIK1i6e4R3dnI0+VtRg8PbX+s5VVM0j3fk1PZvq8MtCAg3nDa5d6OI vELOO3jyVT32tsRxkWQBPBTejYRX/vFGpnHt8wsp9S7SRvnPhBzUi+GajzN1SkV8Iqtf /05sd/SBCSITjXF0oiGUUKwP8zmkgJBxBJtlpuFH//DdgX/f3gyxiwSKyZLOBC+NwMC7 LVsw== X-Gm-Message-State: ALoCoQlNKPP0EhG5Q/YXfIJ7Uw7I04NACjfm+SENqR8Bju8+9ky/Ar88Y3fBnEKOZ6dWvFGu0/Z3 X-Received: by 10.182.246.39 with SMTP id xt7mr724185obc.40.1384974801834; Wed, 20 Nov 2013 11:13:21 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.119.196 with SMTP id kw4ls185684qeb.90.gmail; Wed, 20 Nov 2013 11:13:21 -0800 (PST) X-Received: by 10.58.186.173 with SMTP id fl13mr1734418vec.31.1384974801692; Wed, 20 Nov 2013 11:13:21 -0800 (PST) Received: from mail-vc0-f178.google.com (mail-vc0-f178.google.com [209.85.220.178]) by mx.google.com with ESMTPS id dp7si9807547ved.149.2013.11.20.11.13.21 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 20 Nov 2013 11:13:21 -0800 (PST) Received-SPF: neutral (google.com: 209.85.220.178 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.220.178; Received: by mail-vc0-f178.google.com with SMTP id lh4so1074256vcb.23 for ; Wed, 20 Nov 2013 11:13:21 -0800 (PST) X-Received: by 10.58.168.205 with SMTP id zy13mr1745390veb.19.1384974801596; Wed, 20 Nov 2013 11:13:21 -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.220.174.196 with SMTP id u4csp374757vcz; Wed, 20 Nov 2013 11:13:18 -0800 (PST) X-Received: by 10.194.2.108 with SMTP id 12mr2141015wjt.64.1384974798338; Wed, 20 Nov 2013 11:13:18 -0800 (PST) Received: from mail-wi0-f179.google.com (mail-wi0-f179.google.com [209.85.212.179]) by mx.google.com with ESMTPS id fe18si7495608wic.71.2013.11.20.11.13.17 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 20 Nov 2013 11:13:18 -0800 (PST) Received-SPF: neutral (google.com: 209.85.212.179 is neither permitted nor denied by best guess record for domain of christoffer.dall@linaro.org) client-ip=209.85.212.179; Received: by mail-wi0-f179.google.com with SMTP id ez12so206056wid.6 for ; Wed, 20 Nov 2013 11:13:17 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.180.108.42 with SMTP id hh10mr11346090wib.15.1384974797395; Wed, 20 Nov 2013 11:13:17 -0800 (PST) Received: by 10.227.155.74 with HTTP; Wed, 20 Nov 2013 11:13:17 -0800 (PST) In-Reply-To: <1384973499-30659-1-git-send-email-christoffer.dall@linaro.org> References: <1384973499-30659-1-git-send-email-christoffer.dall@linaro.org> Date: Wed, 20 Nov 2013 11:13:17 -0800 Message-ID: Subject: Fwd: [PATCH] arm: KVM: Don't return PSCI_INVAL if waitqueue is inactive From: Christoffer Dall To: Patch Tracking 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.220.178 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: , ---------- Forwarded message ---------- From: Christoffer Dall Date: 20 November 2013 10:51 Subject: [PATCH] arm: KVM: Don't return PSCI_INVAL if waitqueue is inactive To: kvmarm@lists.cs.columbia.edu Cc: linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, Peter Maydell , Christoffer Dall , Marc Zyngier 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(-) /* 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; -- 1.8.4.3 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);