From patchwork Fri Feb 22 18:40:46 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marc Zyngier X-Patchwork-Id: 2177661 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id 3F800DFABD for ; Fri, 22 Feb 2013 18:43:56 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1U8xYZ-0008DD-1c; Fri, 22 Feb 2013 18:41:15 +0000 Received: from inca-roads.misterjones.org ([213.251.177.50]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1U8xYM-0008Ab-0Z for linux-arm-kernel@lists.infradead.org; Fri, 22 Feb 2013 18:41:03 +0000 Received: from 5e074981.bb.sky.com ([94.7.73.129] helo=why.wild-wind.fr.eu.org) by inca-roads.misterjones.org with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1U8xYI-0002Ke-Ae; Fri, 22 Feb 2013 19:40:58 +0100 From: Marc Zyngier To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 2/2] ARM: KVM: vgic: take distributor lock on sync_hwstate path Date: Fri, 22 Feb 2013 18:40:46 +0000 Message-Id: <1361558446-8375-3-git-send-email-marc.zyngier@arm.com> X-Mailer: git-send-email 1.7.12.4 In-Reply-To: <1361558446-8375-1-git-send-email-marc.zyngier@arm.com> References: <1361558446-8375-1-git-send-email-marc.zyngier@arm.com> X-SA-Exim-Connect-IP: 94.7.73.129 X-SA-Exim-Rcpt-To: linux-arm-kernel@lists.infradead.org, will.deacon@arm.com, c.dall@virtualopensystems.com X-SA-Exim-Mail-From: marc.zyngier@arm.com X-SA-Exim-Scanned: No (on inca-roads.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130222_134102_146611_1FE75964 X-CRM114-Status: GOOD ( 15.52 ) X-Spam-Score: -1.2 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- 0.7 SPF_SOFTFAIL SPF: sender does not match SPF record (softfail) -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: c.dall@virtualopensystems.com, will.deacon@arm.com X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org Now that the maintenance interrupt handling is actually out of the handler itself, the code becomes quite racy as we can get preempted while we process the state. Wrapping this code around the distributor lock ensures that we're not preempted and relatively race-free. Signed-off-by: Marc Zyngier --- arch/arm/kvm/vgic.c | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c index 76ea1aa..0e4cfe1 100644 --- a/arch/arm/kvm/vgic.c +++ b/arch/arm/kvm/vgic.c @@ -1016,21 +1016,6 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu) kvm_debug("MISR = %08x\n", vgic_cpu->vgic_misr); - /* - * We do not need to take the distributor lock here, since the only - * action we perform is clearing the irq_active_bit for an EOIed - * level interrupt. There is a potential race with - * the queuing of an interrupt in __kvm_vgic_flush_hwstate(), where we - * check if the interrupt is already active. Two possibilities: - * - * - The queuing is occurring on the same vcpu: cannot happen, - * as we're already in the context of this vcpu, and - * executing the handler - * - The interrupt has been migrated to another vcpu, and we - * ignore this interrupt for this run. Big deal. It is still - * pending though, and will get considered when this vcpu - * exits. - */ if (vgic_cpu->vgic_misr & GICH_MISR_EOI) { /* * Some level interrupts have been EOIed. Clear their @@ -1069,9 +1054,8 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu) } /* - * Sync back the VGIC state after a guest run. We do not really touch - * the distributor here (the irq_pending_on_cpu bit is safe to set), - * so there is no need for taking its lock. + * Sync back the VGIC state after a guest run. The distributor lock is + * needed so we don't get preempted in the middle of the state processing. */ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu) { @@ -1117,10 +1101,14 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu) void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu) { + struct vgic_dist *dist = &vcpu->kvm->arch.vgic; + if (!irqchip_in_kernel(vcpu->kvm)) return; + spin_lock(&dist->lock); __kvm_vgic_sync_hwstate(vcpu); + spin_unlock(&dist->lock); } int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu)