Re: [Xen-devel] [PATCH v7 08/12] xen/arm: rename GIC_IRQ_GUEST_PENDING to GIC_IRQ_GUEST_QUEUED

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 23 Apr 2014, Ian Campbell wrote:
> On Tue, 2014-04-08 at 16:12 +0100, Stefano Stabellini wrote:
> > Rename GIC_IRQ_GUEST_PENDING to GIC_IRQ_GUEST_QUEUED and clarify its
> > meaning in xen/include/asm-arm/domain.h.
> 
> Thanks.
> 
> > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> > index 2d94d59..f96dc12 100644
> > --- a/xen/include/asm-arm/domain.h
> > +++ b/xen/include/asm-arm/domain.h
> > @@ -27,7 +27,8 @@ struct pending_irq
> >       * whether an irq added to an LR register is PENDING or ACTIVE, the
> >       * following states are just an approximation.
> >       *
> > -     * GIC_IRQ_GUEST_PENDING: the irq is asserted
> > +     * GIC_IRQ_GUEST_QUEUED: the irq is asserted and queued for
> > +     * injection into the guests LRs.
> 
>                             guest's
> 
> >       *
> >       * GIC_IRQ_GUEST_VISIBLE: the irq has been added to an LR register,
> >       * therefore the guest is aware of it. From the guest point of view
> > @@ -35,12 +36,12 @@ struct pending_irq
> >       * or active (after acking the irq).
> >       *
> >       * In order for the state machine to be fully accurate, for level
> > -     * interrupts, we should keep the GIC_IRQ_GUEST_PENDING state until
> > +     * interrupts, we should keep the GIC_IRQ_GUEST_QUEUED state until
> >       * the guest deactivates the irq. However because we are not sure
> > -     * when that happens, we simply remove the GIC_IRQ_GUEST_PENDING
> > +     * when that happens, we simply remove the GIC_IRQ_GUEST_QUEUED
> >       * state when we add the irq to an LR register. We add it back when
> >       * we receive another interrupt notification.
> 
> This paragraph was essentially clarifying the mismatch between the name
> PENDING and the actual behaviour. It doesn't seem to make much sense for
> a bit named QUEUED. In particular "we should keep the QUEUE state until
> the guest deactivates the IRQ" doesn't seem logical, so the rest doesn't
> follow as a workaround for it.
> 
> How about:
>       * In order for the state machine to be fully accurate, for level
>       * interrupts, we should keep the interrupt's pending state until
>       * the guest deactivates the irq. However because we are not sure
>       * when that happens, we instead track whether there is an interrupt
>       * queued using GIC_IRQ_GUEST_QUEUED
>       * state when we add the irq to an LR register. We add it back when
>       * we receive another interrupt notification.
> 
> (needs rewrapping). Still doesn't sound quite right to me, but you
> understand what is going on better than me so I hope you can fix it ;-)

If I make this change then this patch won't be a pure renaming anymore.
Should I add another patch for this?


> > -     * Therefore it is possible to set GIC_IRQ_GUEST_PENDING while the
> > +     * Therefore it is possible to set GIC_IRQ_GUEST_QUEUED while the
> >       * irq is GIC_IRQ_GUEST_VISIBLE. We could also change the state of
> >       * the guest irq in the LR register from active to active and
> >       * pending, but for simplicity we simply inject a second irq after
> 
> Ian.
> 

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