From patchwork Mon Jun 23 16:37:40 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 32379 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ig0-f197.google.com (mail-ig0-f197.google.com [209.85.213.197]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id C790F203AC for ; Mon, 23 Jun 2014 16:40:24 +0000 (UTC) Received: by mail-ig0-f197.google.com with SMTP id r10sf12442238igi.8 for ; Mon, 23 Jun 2014 09:40:24 -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:from:to:date:message-id:in-reply-to :references:mime-version:cc:subject:precedence:list-id :list-unsubscribe:list-post:list-help:list-subscribe:sender :errors-to:x-original-sender:x-original-authentication-results :mailing-list:list-archive:content-type:content-transfer-encoding; bh=SLvqX0qIFDlXD61XrmYW4K+oRQqDntSylZ6Dl0h7lOY=; b=ThVG0vLOfbMvCz8MMbhWXJq/4fI9a1fkblNzFM1EB7t56LXaNG54kZfOloofKm/UEM 1yEdeeiICPfcfqVWJhmE3MnltGXVLrE0vk13vefpSVM6McftKVIRP6d3uigDlq988OnZ Wut2hB/JymOepiUECQF/L9P1aOmjQzOg1toBY1HmQ+6QzqYpt2KWrIFmk6UdhQpZyneu wsD8jbH2Nw5u5V8MY8uVzDTFBBEOc775T0WE+a4WHrUV4iKFKFVZrI1IQbtCeyFIWtsS C3Co10lavaAfYMZlFCJ3oXIw19+5ckKd0Sn2rUseJ7y6jz62zzawHKQnODqN56fTrmzM ouSg== X-Gm-Message-State: ALoCoQm+oAWcszQi0gn1AgFEsKFaI3w9GxCMEspa0TF27TbU/xMKJSKNJ402H77JmYkX8DoYYTrE X-Received: by 10.42.12.141 with SMTP id y13mr777366icy.20.1403541624391; Mon, 23 Jun 2014 09:40:24 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.36.176 with SMTP id p45ls2041471qgp.33.gmail; Mon, 23 Jun 2014 09:40:24 -0700 (PDT) X-Received: by 10.52.138.232 with SMTP id qt8mr2560151vdb.44.1403541624268; Mon, 23 Jun 2014 09:40:24 -0700 (PDT) Received: from mail-vc0-f173.google.com (mail-vc0-f173.google.com [209.85.220.173]) by mx.google.com with ESMTPS id cl2si9346582vcb.98.2014.06.23.09.40.24 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 23 Jun 2014 09:40:24 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.173 as permitted sender) client-ip=209.85.220.173; Received: by mail-vc0-f173.google.com with SMTP id lf12so6170767vcb.4 for ; Mon, 23 Jun 2014 09:40:24 -0700 (PDT) X-Received: by 10.58.220.230 with SMTP id pz6mr20465495vec.9.1403541624160; Mon, 23 Jun 2014 09:40:24 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.221.37.5 with SMTP id tc5csp144329vcb; Mon, 23 Jun 2014 09:40:23 -0700 (PDT) X-Received: by 10.50.61.234 with SMTP id t10mr27758424igr.38.1403541623281; Mon, 23 Jun 2014 09:40:23 -0700 (PDT) Received: from lists.xen.org (lists.xen.org. [50.57.142.19]) by mx.google.com with ESMTPS id m3si21431812igx.16.2014.06.23.09.40.22 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Mon, 23 Jun 2014 09:40:23 -0700 (PDT) Received-SPF: none (google.com: xen-devel-bounces@lists.xen.org does not designate permitted sender hosts) client-ip=50.57.142.19; Received: from localhost ([127.0.0.1] helo=lists.xen.org) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Wz7GE-0000vZ-AU; Mon, 23 Jun 2014 16:38:26 +0000 Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Wz7GC-0000u1-Fm for xen-devel@lists.xensource.com; Mon, 23 Jun 2014 16:38:24 +0000 Received: from [85.158.143.35:11260] by server-3.bemta-4.messagelabs.com id 9F/66-16194-FF758A35; Mon, 23 Jun 2014 16:38:23 +0000 X-Env-Sender: Stefano.Stabellini@citrix.com X-Msg-Ref: server-13.tower-21.messagelabs.com!1403541501!5948477!1 X-Originating-IP: [66.165.176.63] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni42MyA9PiAzMDYwNDg=\n X-StarScan-Received: X-StarScan-Version: 6.11.3; banners=-,-,- X-VirusChecked: Checked Received: (qmail 25662 invoked from network); 23 Jun 2014 16:38:22 -0000 Received: from smtp02.citrix.com (HELO SMTP02.CITRIX.COM) (66.165.176.63) by server-13.tower-21.messagelabs.com with RC4-SHA encrypted SMTP; 23 Jun 2014 16:38:22 -0000 X-IronPort-AV: E=Sophos;i="5.01,531,1400025600"; d="scan'208";a="146222543" Received: from accessns.citrite.net (HELO FTLPEX01CL02.citrite.net) ([10.9.154.239]) by FTLPIPO02.CITRIX.COM with ESMTP; 23 Jun 2014 16:38:20 +0000 Received: from ukmail1.uk.xensource.com (10.80.16.128) by smtprelay.citrix.com (10.13.107.79) with Microsoft SMTP Server id 14.3.181.6; Mon, 23 Jun 2014 12:38:19 -0400 Received: from kaball.uk.xensource.com ([10.80.2.59]) by ukmail1.uk.xensource.com with esmtp (Exim 4.69) (envelope-from ) id 1Wz7G1-0000zc-SF; Mon, 23 Jun 2014 17:38:13 +0100 From: Stefano Stabellini To: Date: Mon, 23 Jun 2014 17:37:40 +0100 Message-ID: <1403541463-23734-2-git-send-email-stefano.stabellini@eu.citrix.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: References: MIME-Version: 1.0 X-DLP: MIA2 Cc: julien.grall@citrix.com, Ian.Campbell@citrix.com, Stefano Stabellini Subject: [Xen-devel] [PATCH v6 2/5] xen/arm: inflight irqs during migration X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Post: , List-Help: , List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: stefano.stabellini@eu.citrix.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.173 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Archive: We need to take special care when migrating irqs that are already inflight from one vcpu to another. See "The effect of changes to an GICD_ITARGETSR", part of chapter 4.3.12 of the ARM Generic Interrupt Controller Architecture Specification. The main issue from the Xen point of view is that the lr_pending and inflight lists are per-vcpu. The lock we take to protect them is also per-vcpu. In order to avoid issues, if the irq is still lr_pending, we can immediately move it to the new vcpu for injection. Otherwise if it is in a GICH_LR register, set a new flag GIC_IRQ_GUEST_MIGRATING, so that we can recognize when we receive an irq while the previous one is still inflight (given that we are only dealing with hardware interrupts here, it just means that its LR hasn't been cleared yet on the old vcpu). If GIC_IRQ_GUEST_MIGRATING is set, we only set GIC_IRQ_GUEST_QUEUED and interrupt the old vcpu. When clearing the LR on the old vcpu, we take special care of injecting the interrupt into the new vcpu. To do that we need to release the old vcpu lock before taking the new vcpu lock. In vgic_vcpu_inject_irq set GIC_IRQ_GUEST_QUEUED before testing GIC_IRQ_GUEST_MIGRATING to avoid races with gic_update_one_lr. Signed-off-by: Stefano Stabellini --- Changes in v6: - remove unnecessary casts to (const unsigned long *) to the arguments of find_first_bit and find_next_bit; - deal with migrating an irq that is inflight and still lr_pending; - replace the dsb with smb_wmb and smb_rmb, use them to ensure the order of accesses to GIC_IRQ_GUEST_QUEUED and GIC_IRQ_GUEST_MIGRATING. Changes in v5: - pass unsigned long to find_next_bit for alignment on aarch64; - call vgic_get_target_vcpu instead of gic_add_to_lr_pending to add the irq in the right inflight queue; - add barrier and bit tests to make sure that vgic_migrate_irq and gic_update_one_lr can run simultaneously on different cpus without issues; - rework the loop to identify the new vcpu when ITARGETSR is written; - use find_first_bit instead of find_next_bit. --- xen/arch/arm/gic.c | 26 ++++++++++++-- xen/arch/arm/vgic.c | 78 ++++++++++++++++++++++++++++++++++++------ xen/include/asm-arm/domain.h | 4 +++ 3 files changed, 95 insertions(+), 13 deletions(-) diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 4d2a92d..c7dda9b 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -683,10 +683,32 @@ static void gic_update_one_lr(struct vcpu *v, int i) clear_bit(GIC_IRQ_GUEST_ACTIVE, &p->status); p->lr = GIC_INVALID_LR; if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) && - test_bit(GIC_IRQ_GUEST_QUEUED, &p->status) ) + test_bit(GIC_IRQ_GUEST_QUEUED, &p->status) && + !test_bit(GIC_IRQ_GUEST_MIGRATING, &p->status) ) gic_raise_guest_irq(v, irq, p->priority); - else + else { + int m, q; list_del_init(&p->inflight); + + m = test_and_clear_bit(GIC_IRQ_GUEST_MIGRATING, &p->status); + /* check MIGRATING before QUEUED */ + smp_rmb(); + q = test_bit(GIC_IRQ_GUEST_QUEUED, &p->status); + if ( m && q ) + { + struct vcpu *v_target; + + /* It is safe to temporarily drop the lock because we + * are finished dealing with this LR. We'll take the + * lock before reading the next. */ + spin_unlock(&v->arch.vgic.lock); + /* vgic_get_target_vcpu takes the rank lock, ensuring + * consistency with other itarget changes. */ + v_target = vgic_get_target_vcpu(v, irq); + vgic_vcpu_inject_irq(v_target, irq); + spin_lock(&v->arch.vgic.lock); + } + } } } diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index 1e1c244..7924629 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -409,6 +409,35 @@ struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int irq) return v_target; } +static void vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq) +{ + unsigned long flags; + struct pending_irq *p = irq_to_pending(old, irq); + + /* nothing to do for virtual interrupts */ + if ( p->desc == NULL ) + return; + + /* migration already in progress, no need to do anything */ + if ( test_bit(GIC_IRQ_GUEST_MIGRATING, &p->status) ) + return; + + spin_lock_irqsave(&old->arch.vgic.lock, flags); + /* If the IRQ is still lr_pending, re-inject it to the new vcpu */ + if ( !list_empty(&p->lr_queue) ) + { + list_del_init(&p->lr_queue); + list_del_init(&p->inflight); + spin_unlock_irqrestore(&old->arch.vgic.lock, flags); + vgic_vcpu_inject_irq(new, irq); + return; + /* if the IRQ is in a GICH_LR register, set GIC_IRQ_GUEST_MIGRATING + * and wait for the EOI */ + } else if ( !list_empty(&p->inflight) ) + set_bit(GIC_IRQ_GUEST_MIGRATING, &p->status); + spin_unlock_irqrestore(&old->arch.vgic.lock, flags); +} + static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n) { const unsigned long mask = r; @@ -546,6 +575,8 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info) int offset = (int)(info->gpa - v->domain->arch.vgic.dbase); int gicd_reg = REG(offset); uint32_t tr; + unsigned long trl; + int i; switch ( gicd_reg ) { @@ -630,25 +661,45 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info) if ( rank == NULL) goto write_ignore; /* 8-bit vcpu mask for this domain */ BUG_ON(v->domain->max_vcpus > 8); - tr = (1 << v->domain->max_vcpus) - 1; + trl = (1 << v->domain->max_vcpus) - 1; if ( dabt.size == 2 ) - tr = tr | (tr << 8) | (tr << 16) | (tr << 24); + trl = trl | (trl << 8) | (trl << 16) | (trl << 24); else - tr = (tr << (8 * (offset & 0x3))); - tr &= *r; + trl = (trl << (8 * (offset & 0x3))); + trl &= *r; /* ignore zero writes */ - if ( !tr ) + if ( !trl ) goto write_ignore; if ( dabt.size == 2 && - !((tr & 0xff) && (tr & (0xff << 8)) && - (tr & (0xff << 16)) && (tr & (0xff << 24)))) + !((trl & 0xff) && (trl & (0xff << 8)) && + (trl & (0xff << 16)) && (trl & (0xff << 24)))) goto write_ignore; vgic_lock_rank(v, rank); + i = 0; + while ( (i = find_next_bit(&trl, 32, i)) < 32 ) + { + unsigned int irq, target, old_target; + unsigned long old_target_mask; + struct vcpu *v_target, *v_old; + + target = i % 8; + old_target_mask = byte_read(rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR)], 0, i/8); + old_target = find_first_bit(&old_target_mask, 8); + + if ( target != old_target ) + { + irq = offset + (i / 8); + v_target = v->domain->vcpu[target]; + v_old = v->domain->vcpu[old_target]; + vgic_migrate_irq(v_old, v_target, irq); + } + i += 8 - target; + } if ( dabt.size == 2 ) - rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR)] = tr; + rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR)] = trl; else byte_write(&rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR)], - tr, offset); + trl, offset); vgic_unlock_rank(v, rank); return 1; @@ -786,9 +837,14 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq) spin_lock_irqsave(&v->arch.vgic.lock, flags); + set_bit(GIC_IRQ_GUEST_QUEUED, &n->status); + /* update QUEUED before MIGRATING */ + smp_wmb(); + if ( test_bit(GIC_IRQ_GUEST_MIGRATING, &n->status) ) + goto out; + if ( !list_empty(&n->inflight) ) { - set_bit(GIC_IRQ_GUEST_QUEUED, &n->status); gic_raise_inflight_irq(v, irq); goto out; } @@ -796,6 +852,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq) /* vcpu offline */ if ( test_bit(_VPF_down, &v->pause_flags) ) { + clear_bit(GIC_IRQ_GUEST_QUEUED, &n->status); spin_unlock_irqrestore(&v->arch.vgic.lock, flags); return; } @@ -803,7 +860,6 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq) priority = byte_read(rank->ipriority[REG_RANK_INDEX(8, idx)], 0, byte); n->irq = irq; - set_bit(GIC_IRQ_GUEST_QUEUED, &n->status); n->priority = priority; /* the irq is enabled */ diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h index 2cb6837..12b2a1d 100644 --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -55,11 +55,15 @@ struct pending_irq * GIC_IRQ_GUEST_ENABLED: the guest IRQ is enabled at the VGICD * level (GICD_ICENABLER/GICD_ISENABLER). * + * GIC_IRQ_GUEST_MIGRATING: the irq is being migrated to a different + * vcpu. + * */ #define GIC_IRQ_GUEST_QUEUED 0 #define GIC_IRQ_GUEST_ACTIVE 1 #define GIC_IRQ_GUEST_VISIBLE 2 #define GIC_IRQ_GUEST_ENABLED 3 +#define GIC_IRQ_GUEST_MIGRATING 4 unsigned long status; struct irq_desc *desc; /* only set it the irq corresponds to a physical irq */ int irq;