From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v6 2/5] xen/arm: inflight irqs during migration Date: Tue, 24 Jun 2014 13:17:15 +0100 Message-ID: <53A96C4B.3020900@linaro.org> References: <1403541463-23734-2-git-send-email-stefano.stabellini@eu.citrix.com> <53A88AB1.2080007@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: julien.grall@citrix.com, xen-devel@lists.xensource.com, Ian.Campbell@citrix.com List-Id: xen-devel@lists.xenproject.org On 24/06/14 12:57, Stefano Stabellini wrote: > On Mon, 23 Jun 2014, Julien Grall wrote: >> Hi Stefano, >> >> On 23/06/14 17:37, Stefano Stabellini wrote: >>> @@ -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; >> >> Why do you kick the current VCPU here? It looks like useless because the migration will take care of it. > > With the physical follow virtual patch, the vcpu_unblock below is going > to be mostly useless but harmless as vgic_vcpu_inject_irq is going to be called on > the pcpu running the destination vcpu. smp_send_event_check_mask won't be called as > v == current. > > On the other hand without that patch, the pcpu receiving the physical > interrupt could be different from any of the pcpus running the vcpus > involved in vcpu migration, therefore we would need the kick to wake up > the destination vcpu. If the MIGRATING bit is set it means that an IRQ is already inflight, and therefore gic_update_one_lr will take care of injecting this IRQ to the new VCPU by calling vgic_vcpu_inject_irq. So kicking the new VCPU here is pointless and may unschedule another VCPU for nothing. The latter may be able to run a bit more. With your patch #4 (physical IRQ follow virtual IRQ), there is a tiny range the VCPU may not run on the same pCPU where the physical IRQ as been routed. This is the case when the VCPU is unscheduled. Regards, -- Julien Grall