Re: [PATCH v8 09/13] xen/arm: second irq injection while the first irq is still inflight

[Julien Grall <julien.grall@linaro.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