Re: [Xen-devel] [PATCH v6 08/10] xen/arm: second irq injection while the first irq is still inflight

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-04-02 at 16:01 +0100, Stefano Stabellini wrote:
(picking up where I left on v5 when I realised there had been a v6)


> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 40e768e..218e3c6 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -680,6 +680,14 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int 
> virtual_irq,
>  {
>      int i;
>      unsigned long flags;
> +    struct pending_irq *n = irq_to_pending(v, virtual_irq);
> +
> +    if ( test_bit(GIC_IRQ_GUEST_VISIBLE, &n->status))
> +    {
> +        if ( v == current )
> +            gic_clear_one_lr(v, n->lr);

Maybe this is the same confusion I had with v5 in a different hat, but
why?

If the IRQ is visible to the guest (present in an LR, either active or
pending) and a new one comes along why would we clear the LR?

Is it so that we can reinsert it again right after? If so please could
you add a comment saying that.

By doing thing this way don't we lose the active bit in the LR if it was
set?

> +        list_add_tail(&n->inflight, &v->arch.vgic.inflight_irqs);
> +    } else if ( n->priority != priority )
> +        gdprintk(XENLOG_WARNING, "Changing priority of an inflight interrupt 
> is not supported");

Would it be OK to silently ignore this? Changing the priority of an
interrupt which isn't masked is going to be racy even on real hardware,
i.e. you might receive one more at the old priority. I haven't checked
what the GIC docs say about this -- it could well be unpredicatable or
something...

Ian.


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