From patchwork Fri May 20 13:30:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 68240 Delivered-To: patch@linaro.org Received: by 10.140.92.199 with SMTP id b65csp3780816qge; Fri, 20 May 2016 06:31:37 -0700 (PDT) X-Received: by 10.66.41.43 with SMTP id c11mr4857104pal.96.1463751096932; Fri, 20 May 2016 06:31:36 -0700 (PDT) Return-Path: Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id q9si28012869pap.136.2016.05.20.06.31.36 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 20 May 2016 06:31:36 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) client-ip=2001:1868:205::9; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) smtp.mailfrom=linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1b3kVE-0008Q6-4t; Fri, 20 May 2016 13:30:08 +0000 Received: from mail-wm0-x22c.google.com ([2a00:1450:400c:c09::22c]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1b3kVA-0007FM-Ed for linux-arm-kernel@lists.infradead.org; Fri, 20 May 2016 13:30:05 +0000 Received: by mail-wm0-x22c.google.com with SMTP id n129so272099399wmn.1 for ; Fri, 20 May 2016 06:29:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=RWfIwE1Q5nyP+XCZCivRP3jK2SMQFaje2Yh/CowGBhc=; b=aiwm1iM5DUqwd98uSXRXUmK3/azff1LSOd7TKZ7VftzhKiL0cmRWqpclmhg3jHPuKy 2e1hdsHZ854LYTWCRU5sxvZ0mogBFLyrjoNEzuhHYI5UyVlsy9wTo/wxZCybF5SA1rHp QF1Sd9gj2rEfMtWlplYTIAawMiQW5RryiRRII= 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=RWfIwE1Q5nyP+XCZCivRP3jK2SMQFaje2Yh/CowGBhc=; b=FMYoldxsEZ92PCfPt3KdR81cg8h4oiXJPM/1szBXXPt9VzgxzcNMKd8jZU+k8eFGos AOg05xydxaWmboDc6Osu//wtDkBKk1d9FhxX/cE7czUOWn5tEO7VtqbMBcif92p57Ygd aoo8FswfBWuW31lPMGe3XIaIPog8VbrdwdwpNjFSBGpqy3IX0e7RYz0LFtU0edNyeEMV BJ6olZqfprnGFFHIUGAOtH89IC0Q/aNUrJPBLjHqAX0FXgIPQxvawnE+Ti90MD3gv4tH Hr2jXx/1yV6Lrr17k098QSlyCVYFjDs61o1ypupalwMiWIWC2aA44/q43JD8v9VjGiQq uC3g== X-Gm-Message-State: AOPr4FWZ3rti4r+6eHTF5iDcmtVQN7BYdlhy5ilW4mfbCtxgKXT749JNma6zPQyzp/oVORUg X-Received: by 10.194.55.10 with SMTP id n10mr3378217wjp.28.1463750981402; Fri, 20 May 2016 06:29:41 -0700 (PDT) Received: from localhost.localdomain ([94.18.191.146]) by smtp.gmail.com with ESMTPSA id s125sm4593793wms.14.2016.05.20.06.29.38 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 20 May 2016 06:29:39 -0700 (PDT) From: Christoffer Dall To: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: [PATCH] KVM: arm/arm64: vgic-new: Synchronize changes to active state Date: Fri, 20 May 2016 15:30:27 +0200 Message-Id: <1463751027-17517-1-git-send-email-christoffer.dall@linaro.org> X-Mailer: git-send-email 2.1.2.330.g565301e.dirty X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160520_063004_665851_1EE2BE86 X-CRM114-Status: GOOD ( 17.82 ) X-Spam-Score: -2.7 (--) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-2.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [2a00:1450:400c:c09:0:0:0:22c listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Marc Zyngier , Andre Przywara , Christoffer Dall , Eric Auger MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org When modifying the active state of an interrupt via the MMIO interface, we should ensure that the write has the intended effect. If a guest sets an interrupt to active, but that interrupt is already flushed into a list register on a running VCPU, then that VCPU will write the active state back into the struct vgic_irq upon returning from the guest and syncing its state. This is a non-benign race, because the guest can observe that an interrupt is not active, and it can have a reasonable expectations that other VCPUs will not ack any IRQs, and then set the state to active, and expect it to stay that way. Currently we are not honoring this case. Thefore, change both the SACTIVE and CACTIVE mmio handlers to stop the world, change the irq state, potentially queue the irq if we're setting it to active, and then continue. Signed-off-by: Christoffer Dall --- virt/kvm/arm/vgic/vgic-mmio.c | 69 ++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 37 deletions(-) -- 2.1.2.330.g565301e.dirty _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c index 4ef3571..90b0dc4 100644 --- a/virt/kvm/arm/vgic/vgic-mmio.c +++ b/virt/kvm/arm/vgic/vgic-mmio.c @@ -173,6 +173,34 @@ unsigned long vgic_mmio_read_active(struct kvm_vcpu *vcpu, return value; } +static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq, + bool new_active_state) +{ + spin_lock(&irq->irq_lock); + /* + * If this virtual IRQ was written into a list register, we + * have to make sure the CPU that runs the VCPU thread has + * synced back LR state to the struct vgic_irq. We can only + * know this for sure, when either this irq is not assigned to + * anyone's AP list anymore, or the VCPU thread is not + * running on any CPUs. + * + * In the opposite case, we know the VCPU thread may be on its + * way back from the guest and still has to sync back this + * IRQ, so we release and re-acquire the spin_lock to let the + * other thread sync back the IRQ. + */ + while (irq->vcpu && /* IRQ may have state in an LR somewhere */ + irq->vcpu->cpu != -1) /* VCPU thread is running */ + cond_resched_lock(&irq->irq_lock); + + irq->active = new_active_state; + if (new_active_state) + vgic_queue_irq_unlock(vcpu->kvm, irq); + else + spin_unlock(&irq->irq_lock); +} + void vgic_mmio_write_cactive(struct kvm_vcpu *vcpu, gpa_t addr, unsigned int len, unsigned long val) @@ -183,27 +211,7 @@ void vgic_mmio_write_cactive(struct kvm_vcpu *vcpu, kvm_arm_halt_guest(vcpu->kvm); for_each_set_bit(i, &val, len * 8) { struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i); - - spin_lock(&irq->irq_lock); - /* - * If this virtual IRQ was written into a list register, we - * have to make sure the CPU that runs the VCPU thread has - * synced back LR state to the struct vgic_irq. We can only - * know this for sure, when either this irq is not assigned to - * anyone's AP list anymore, or the VCPU thread is not - * running on any CPUs. - * - * In the opposite case, we know the VCPU thread may be on its - * way back from the guest and still has to sync back this - * IRQ, so we release and re-acquire the spin_lock to let the - * other thread sync back the IRQ. - */ - while (irq->vcpu && /* IRQ may have state in an LR somewhere */ - irq->vcpu->cpu != -1) /* VCPU thread is running */ - cond_resched_lock(&irq->irq_lock); - - irq->active = false; - spin_unlock(&irq->irq_lock); + vgic_mmio_change_active(vcpu, irq, false); } kvm_arm_resume_guest(vcpu->kvm); } @@ -215,25 +223,12 @@ void vgic_mmio_write_sactive(struct kvm_vcpu *vcpu, u32 intid = VGIC_ADDR_TO_INTID(addr, 1); int i; + kvm_arm_halt_guest(vcpu->kvm); for_each_set_bit(i, &val, len * 8) { struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i); - - spin_lock(&irq->irq_lock); - - /* - * If the IRQ was already active or there is no target VCPU - * assigned at the moment, then just proceed. - */ - if (irq->active || !irq->target_vcpu) { - irq->active = true; - - spin_unlock(&irq->irq_lock); - continue; - } - - irq->active = true; - vgic_queue_irq_unlock(vcpu->kvm, irq); + vgic_mmio_change_active(vcpu, irq, true); } + kvm_arm_resume_guest(vcpu->kvm); } unsigned long vgic_mmio_read_priority(struct kvm_vcpu *vcpu,