Re: [Xen-devel] [PATCH v10 03/32] ARM: vGIC: move irq_to_pending() calls under the VGIC VCPU lock

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

On 06/06/17 19:46, Stefano Stabellini wrote:
> On Tue, 6 Jun 2017, Andre Przywara wrote:
>> Maybe we should consider to merge this one for 4.9 still,
>> as currently enabling the ITS in .config and running it on an ITS
>> machine will fail to boot Dom0.
> 
> Here, you are talking about this patch, patch #3, right?

No, sorry, I meant patch 12.

> Although it should be "safe", it touches a lot of common code. I think
> it is too risky to commit it now to fix an experimental feature. I would
> rather wait until it is committed to staging in the 4.10 dev window,
> then backport it.

Well, if you compile origin/staging with the ITS configured in and run
this on a machine with an ITS in the DT, it will not boot (regardless of
the ITS not being used anyway). This is because we try to map
collections on the host (using MAPC commands), but don't enable the host
ITS, so those commands are not executed and we time out.
I discovered this shortly after you merged the first ITS patches and
made a simple patch, but for some reasons this didn't end up on the
public list.

Let me send this out and we can decide whether we need this for 4.9 still.

Cheers,
Andre.

>>>> -        if ( p->desc != NULL )
>>>> +        desc = p->desc;
>>>> +        spin_unlock_irqrestore(&v_target->arch.vgic.lock, flags);
>>>> +
>>>> +        if ( desc != NULL )
>>>>          {
>>>> -            spin_lock_irqsave(&p->desc->lock, flags);
>>>> -            p->desc->handler->disable(p->desc);
>>>> -            spin_unlock_irqrestore(&p->desc->lock, flags);
>>>> +            spin_lock_irqsave(&desc->lock, flags);
>>>> +            desc->handler->disable(desc);
>>>> +            spin_unlock_irqrestore(&desc->lock, flags);
>>>>          }
>>>>          i++;
>>>>      }
>>>> @@ -349,9 +368,9 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r,
>>>> int n)
>>>>      while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
>>>>          irq = i + (32 * n);
>>>>          v_target = vgic_get_target_vcpu(v, irq);
>>>> +        spin_lock_irqsave(&v_target->arch.vgic.lock, flags);
>>>>          p = irq_to_pending(v_target, irq);
>>>>          set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
>>>> -        spin_lock_irqsave(&v_target->arch.vgic.lock, flags);
>>>>          if ( !list_empty(&p->inflight) &&
>>>> !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
>>>>              gic_raise_guest_irq(v_target, irq, p->priority);
>>>>          spin_unlock_irqrestore(&v_target->arch.vgic.lock, flags);
>>>> @@ -460,7 +479,7 @@ void vgic_clear_pending_irqs(struct vcpu *v)
>>>>  void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int virq)
>>>>  {
>>>>      uint8_t priority;
>>>> -    struct pending_irq *iter, *n = irq_to_pending(v, virq);
>>>> +    struct pending_irq *iter, *n;
>>>>      unsigned long flags;
>>>>      bool running;
>>>>
>>>> @@ -468,6 +487,8 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned
>>>> int virq)
>>>>
>>>>      spin_lock_irqsave(&v->arch.vgic.lock, flags);
>>>>
>>>> +    n = irq_to_pending(v, virq);
>>>> +
>>>>      /* vcpu offline */
>>>>      if ( test_bit(_VPF_down, &v->pause_flags) )
>>>>      {
>>>>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel