From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v8 09/13] xen/arm: second irq injection while the first irq is still inflight Date: Thu, 22 May 2014 19:05:37 +0100 Message-ID: <537E3C71.4070203@linaro.org> References: <1400761950-25035-9-git-send-email-stefano.stabellini@eu.citrix.com> <537E1C67.2020302@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 22/05/14 18:39, Stefano Stabellini wrote: > On Thu, 22 May 2014, Julien Grall wrote: >>> while the first one is still active. >>> If the first irq is already pending (not active), clear >>> GIC_IRQ_GUEST_QUEUED because the guest doesn't need a second >>> notification.If the irq has already been EOI'ed then just clear the >>> GICH_LR right away and move the interrupt to lr_pending so that it is >>> going to be reinjected by gic_restore_pending_irqs on return to guest. >>> >>> If the target cpu is not the current cpu, then set GIC_IRQ_GUEST_QUEUED >>> and send an SGI. The target cpu is going to be interrupted and call >>> gic_clear_lrs, that is going to take the same actions. >>> >>> Do not call vgic_vcpu_inject_irq from gic_inject if >>> evtchn_upcall_pending is set. If we remove that call, we don't need to >>> special case evtchn_irq in vgic_vcpu_inject_irq anymore. >>> We need to force the first injection of evtchn_irq (call >>> gic_vcpu_inject_irq) from vgic_enable_irqs because evtchn_upcall_pending >>> is already set by common code on vcpu creation. >> >> If you only need it for the first time. Why can't you call vgic_inject_irq >> with the IRQ evtchn when the VCPU is turn on? >> >> This would remove every hack with this IRQ in the GIC code. > > In principle sounds nice, but in practice it is difficult and risks > being racy. In vgic_vcpu_inject_irq we have: > > /* vcpu offline */ > if ( test_bit(_VPF_down, &v->pause_flags) ) > { > spin_unlock_irqrestore(&v->arch.vgic.lock, flags); > return; > } > > So we can only inject the irq once the vcpu is properly up, that is > certainly later than vcpu_initialise. If we call vcpu_vgic_inject right before vcpu_wake (the _VPF_down flags has been cleared) we won't have any race condition. This can be done in both arch/arm/vpsci.c and common/domain.c (VCPUOP_up). It may require an arch specific function. Smth like arch_vcpu_prepare_up. Regards, -- Julien Grall