Re: [Xen-devel] [PATCH v7 04/12] xen/arm: support HW interrupts, do not request maintenance_interrupts

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-04-08 at 16:12 +0100, Stefano Stabellini wrote:
> If the irq to be injected is an hardware irq (p->desc != NULL), set
> GICH_LR_HW. Do not set GICH_LR_MAINTENANCE_IRQ.
> 
> Remove the code to EOI a physical interrupt on behalf of the guest
> because it has become unnecessary.
> 
> Introduce a new function, gic_clear_lrs, that goes over the GICH_LR
> registers, clear the invalid ones and free the corresponding interrupts
> from the inflight queue if appropriate. Add the interrupt to lr_pending
> if the GIC_IRQ_GUEST_PENDING is still set.
> 
> Call gic_clear_lrs on entry to the hypervisor to make sure that the
> calculation in Xen of the highest priority interrupt currently inflight
> is correct and accurate and not based on stale data.
> 
> In vgic_vcpu_inject_irq, if the target is a vcpu running on another
> pcpu, we are already sending an SGI to the other pcpu so that it would
> pick up the new IRQ to inject.  Now also send an SGI to the other pcpu
> even if the IRQ is already inflight, so that it can clear the LR
> corresponding to the previous injection as well as injecting the new
> interrupt.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Some minor nits in the event you end up respinning for some other
reason:
> @@ -626,16 +628,18 @@ int __init setup_dt_irq(const struct dt_irq *irq, 
> struct irqaction *new)
>  static inline void gic_set_lr(int lr, struct pending_irq *p,
>          unsigned int state)
>  {
> -    int maintenance_int = GICH_LR_MAINTENANCE_IRQ;
> +    uint32_t lr_reg;

Calling this lr_val would be clearer (lr_reg sounds like the index of
the register or something to me)
> +static void gic_update_one_lr(struct vcpu *v, int i)
> +{
> +    struct pending_irq *p;
> +    uint32_t lr;
> +    int irq;
> +    bool_t inflight;
> +
> +    ASSERT(spin_is_locked(&v->arch.vgic.lock));
> +
> +    lr = GICH[GICH_LR + i];
> +    if ( !(lr & (GICH_LR_PENDING|GICH_LR_ACTIVE)) )

if ( lr & (GICH_...) )
    return

Then you can pull everything else in a level.



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