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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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 ;-)

> -     * 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