From patchwork Sat Jul 18 09:09:13 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 51251 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-lb0-f200.google.com (mail-lb0-f200.google.com [209.85.217.200]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id B0E4922A28 for ; Sat, 18 Jul 2015 09:08:57 +0000 (UTC) Received: by lbcjf8 with SMTP id jf8sf30265150lbc.0 for ; Sat, 18 Jul 2015 02:08:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:cc:subject:message-id :references:mime-version:content-type:content-disposition :content-transfer-encoding:in-reply-to:user-agent:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :list-post:list-help:list-archive:list-unsubscribe; bh=R6F1RpUb2IIENOL15k4cVc35CDheY4mZSp9JyjtUSos=; b=NK/YCYw5s+OJ8W2Snb6D6zaAijiEw2kFruixDcqyA6v9hQ7sPSY0t+NRbzgwsBjBt6 5OfxyxHMqGF0GjgPo4/FiClxGfpr0t/tzirMjYKUVUoW/LvaSifqoaZFygXX3//Fkv+Y YOKpe/4SIzn7XhzUnrkHQ1heoIXWAxHPN4fbCbFAPJCyBfJgBmbj+eZihx2gTOmp3M6l wqbEQour+8WPsrst40izEvsLSBtKDQIIwuvt4g2HaNLcgCd0EOcBz7jf1Vp+4eAkd3i/ eEbaZqqXD8LeRGWIs/5C/4pLmK/5ZYiSPJr/3Yo9LO8sA3btAwCnSQrTIBA8ZObgx300 n3QA== X-Gm-Message-State: ALoCoQnSs3VMHRjD4pysUzU5soE7Lrqtz3V6hSj2md6tLKU2Llkmw28Buc4pZIduv2szi+SczL9q X-Received: by 10.181.13.202 with SMTP id fa10mr1019807wid.4.1437210536609; Sat, 18 Jul 2015 02:08:56 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.120.40 with SMTP id kz8ls621505lab.24.gmail; Sat, 18 Jul 2015 02:08:56 -0700 (PDT) X-Received: by 10.152.207.76 with SMTP id lu12mr5903549lac.29.1437210536317; Sat, 18 Jul 2015 02:08:56 -0700 (PDT) Received: from mail-la0-f52.google.com (mail-la0-f52.google.com. [209.85.215.52]) by mx.google.com with ESMTPS id r2si1913647lae.66.2015.07.18.02.08.55 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 18 Jul 2015 02:08:55 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.52 as permitted sender) client-ip=209.85.215.52; Received: by laem6 with SMTP id m6so72502977lae.0 for ; Sat, 18 Jul 2015 02:08:55 -0700 (PDT) X-Received: by 10.112.198.74 with SMTP id ja10mr18307751lbc.19.1437210535879; Sat, 18 Jul 2015 02:08:55 -0700 (PDT) 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.112.108.230 with SMTP id hn6csp239654lbb; Sat, 18 Jul 2015 02:08:55 -0700 (PDT) X-Received: by 10.112.162.70 with SMTP id xy6mr18601146lbb.122.1437210535111; Sat, 18 Jul 2015 02:08:55 -0700 (PDT) Received: from mail-lb0-f169.google.com (mail-lb0-f169.google.com. [209.85.217.169]) by mx.google.com with ESMTPS id x10si12057553lbo.118.2015.07.18.02.08.55 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 18 Jul 2015 02:08:55 -0700 (PDT) Received-SPF: pass (google.com: domain of christoffer.dall@linaro.org designates 209.85.217.169 as permitted sender) client-ip=209.85.217.169; Received: by lbbpo10 with SMTP id po10so71963173lbb.3 for ; Sat, 18 Jul 2015 02:08:55 -0700 (PDT) X-Received: by 10.152.87.131 with SMTP id ay3mr18129145lab.27.1437210534983; Sat, 18 Jul 2015 02:08:54 -0700 (PDT) Received: from localhost (0187900153.0.fullrate.dk. [2.110.55.193]) by smtp.gmail.com with ESMTPSA id eh3sm3488105lbb.43.2015.07.18.02.08.53 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Sat, 18 Jul 2015 02:08:54 -0700 (PDT) Date: Sat, 18 Jul 2015 11:09:13 +0200 From: Christoffer Dall To: Eric Auger Cc: eric.auger@st.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, marc.zyngier@arm.com, patches@linaro.org, alex.williamson@redhat.com, pbonzini@redhat.com Subject: Re: [PATCH 1/2] KVM: arm: rename pause into power_off Message-ID: <20150718090913.GN14024@cbox> References: <1436186996-22528-1-git-send-email-eric.auger@linaro.org> <1436186996-22528-2-git-send-email-eric.auger@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1436186996-22528-2-git-send-email-eric.auger@linaro.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: christoffer.dall@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.52 as permitted sender) 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: , On Mon, Jul 06, 2015 at 02:49:55PM +0200, Eric Auger wrote: > The kvm_vcpu_arch pause field is renamed into power_off to prepare > for the introduction of a new pause field. > > Signed-off-by: Eric Auger > > v4 -> v5: > - fix compilation issue on arm64 (add power_off field in kvm_host.h) > --- > arch/arm/include/asm/kvm_host.h | 4 ++-- > arch/arm/kvm/arm.c | 10 +++++----- > arch/arm/kvm/psci.c | 10 +++++----- > arch/arm64/include/asm/kvm_host.h | 4 ++-- > 4 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h > index e896d2c..304004d 100644 > --- a/arch/arm/include/asm/kvm_host.h > +++ b/arch/arm/include/asm/kvm_host.h > @@ -129,8 +129,8 @@ struct kvm_vcpu_arch { > * here. > */ > > - /* Don't run the guest on this vcpu */ > - bool pause; > + /* vcpu power-off state */ > + bool power_off; > > /* IO related fields */ > struct kvm_decode mmio_decode; > diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c > index bcdf799..7537e68 100644 > --- a/arch/arm/kvm/arm.c > +++ b/arch/arm/kvm/arm.c > @@ -475,7 +475,7 @@ static void vcpu_pause(struct kvm_vcpu *vcpu) > { > wait_queue_head_t *wq = kvm_arch_vcpu_wq(vcpu); > > - wait_event_interruptible(*wq, !vcpu->arch.pause); > + wait_event_interruptible(*wq, !vcpu->arch.power_off); would there be any benefit to simply calling kvm_vcpu_block() instead of vcpu_pause, and rewrite kvm_arch_vcpu_runnable to: int kvm_arch_vcpu_runnable(struct kvm_vcpu *v) { ▸ return !vcpu->arch.power_off && (!!v->arch.irq_lines || kvm_vgic_vcpu_pending_irq(v)); } Not sure really, certainly the runnable function does not become more readable. > } > > static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu) > @@ -525,7 +525,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) > > update_vttbr(vcpu->kvm); > > - if (vcpu->arch.pause) > + if (vcpu->arch.power_off) > vcpu_pause(vcpu); looking back over this code, how does this actually guarantee that we don't run a powered-off cpu? vcpu_pause() just does a wait_event_interruptible(), so if we get scheduled again, we'll just proceed running. Is there any case where we could get scheduled without signal_pending() being true and therefore inadvertedly run the vcpu? if so, we should change the line below like this: Sorry for polluting your patch with these questions, I'm otherwise fine with the rename. Thanks, -Christoffer > > /* > @@ -766,12 +766,12 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu, > vcpu_reset_hcr(vcpu); > > /* > - * Handle the "start in power-off" case by marking the VCPU as paused. > + * Handle the "start in power-off" case. > */ > if (test_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features)) > - vcpu->arch.pause = true; > + vcpu->arch.power_off = true; > else > - vcpu->arch.pause = false; > + vcpu->arch.power_off = false; > > return 0; > } > diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c > index 4b94b51..134971a 100644 > --- a/arch/arm/kvm/psci.c > +++ b/arch/arm/kvm/psci.c > @@ -63,7 +63,7 @@ static unsigned long kvm_psci_vcpu_suspend(struct kvm_vcpu *vcpu) > > static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu) > { > - vcpu->arch.pause = true; > + vcpu->arch.power_off = true; > } > > static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) > @@ -87,7 +87,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) > */ > if (!vcpu) > return PSCI_RET_INVALID_PARAMS; > - if (!vcpu->arch.pause) { > + if (!vcpu->arch.power_off) { > if (kvm_psci_version(source_vcpu) != KVM_ARM_PSCI_0_1) > return PSCI_RET_ALREADY_ON; > else > @@ -115,7 +115,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) > * the general puspose registers are undefined upon CPU_ON. > */ > *vcpu_reg(vcpu, 0) = context_id; > - vcpu->arch.pause = false; > + vcpu->arch.power_off = false; > smp_mb(); /* Make sure the above is visible */ > > wq = kvm_arch_vcpu_wq(vcpu); > @@ -152,7 +152,7 @@ static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu) > kvm_for_each_vcpu(i, tmp, kvm) { > mpidr = kvm_vcpu_get_mpidr_aff(tmp); > if (((mpidr & target_affinity_mask) == target_affinity) && > - !tmp->arch.pause) { > + !tmp->arch.power_off) { > return PSCI_0_2_AFFINITY_LEVEL_ON; > } > } > @@ -175,7 +175,7 @@ static void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type) > * re-initialized. > */ > kvm_for_each_vcpu(i, tmp, vcpu->kvm) { > - tmp->arch.pause = true; > + tmp->arch.power_off = true; > kvm_vcpu_kick(tmp); > } > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 2709db2..009da6b 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -122,8 +122,8 @@ struct kvm_vcpu_arch { > * here. > */ > > - /* Don't run the guest */ > - bool pause; > + /* vcpu power-off state */ > + bool power_off; > > /* IO related fields */ > struct kvm_decode mmio_decode; > -- > 1.9.1 > diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index bc738d2..98f31e6 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -542,7 +542,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) run->exit_reason = KVM_EXIT_INTR; } - if (ret <= 0 || need_new_vmid_gen(vcpu->kvm)) { + if (ret <= 0 || need_new_vmid_gen(vcpu->kvm) || + vcpu->arch.power_off) { local_irq_enable(); preempt_enable(); kvm_timer_sync_hwstate(vcpu);