Re: [Xen-devel] [PATCH v3 12/24] xen/arm: Release IRQ routed to a domain when it's destroying

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 13 Jan 2015, Julien Grall wrote:
> Xen has to release IRQ routed to a domain in order to reuse later. Currently
> only SPIs can be routed to the guest so we only need to browse SPIs for a
> specific domain.
> 
> Futhermore, a guest can crash and let the IRQ in an incorrect state (i.e has
> not being EOIed). Xen will have to reset the IRQ in order to be able to reuse
> the IRQ later.
> 
> Introduce 2 new functions for release an IRQ routed to a domain:
>     - release_guest_irq: upper level to retrieve the IRQ, call the GIC
>     code and release the action
>     - gic_remove_guest_irq: Check if we can remove the IRQ, and reset
>     it if necessary
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>

Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>


> ---
>     Changes in v3:
>         - Take the vgic rank lock to protect p->desc
>         - Correctly check if the IRQ is disabled
>         - Extend the check on the virq in release_guest_irq
>         - Use vgic_get_target_vcpu to get the target vCPU
>         - Remove spurious change
> 
>     Changes in v2:
>         - Drop the desc->handler = &no_irq_type in release_irq as it's
>         buggy if the IRQ is routed to Xen
>         - Add release_guest_irq and gic_remove_guest_irq
> ---
>  xen/arch/arm/gic.c        | 46 +++++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/irq.c        | 48 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/vgic.c       | 16 ++++++++++++++++
>  xen/include/asm-arm/gic.h |  4 ++++
>  xen/include/asm-arm/irq.h |  2 ++
>  5 files changed, 116 insertions(+)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 240870f..bb298e9 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -162,6 +162,52 @@ out:
>      return res;
>  }
>  
> +/* This function only works with SPIs for now */
> +int gic_remove_irq_from_guest(struct domain *d, unsigned int virq,
> +                              struct irq_desc *desc)
> +{
> +    struct vcpu *v_target = vgic_get_target_vcpu(d->vcpu[0], virq);
> +    struct vgic_irq_rank *rank = vgic_rank_irq(v_target, virq);
> +    struct pending_irq *p = irq_to_pending(v_target, virq);
> +    unsigned long flags;
> +
> +    ASSERT(spin_is_locked(&desc->lock));
> +    ASSERT(test_bit(_IRQ_GUEST, &desc->status));
> +    ASSERT(p->desc == desc);
> +
> +    vgic_lock_rank(v_target, rank, flags);
> +
> +    /* If the IRQ is removed when the domain is dying, we only need to
> +     * EOI the IRQ if it has not been done by the guest
> +     */
> +    if ( d->is_dying )
> +    {
> +        desc->handler->shutdown(desc);
> +        if ( test_bit(_IRQ_INPROGRESS, &desc->status) )
> +            gic_hw_ops->deactivate_irq(desc);
> +        clear_bit(_IRQ_INPROGRESS, &desc->status);
> +        goto end;
> +    }
> +
> +    /* TODO: Handle eviction from LRs. For now, deny remove if the IRQ
> +     * is inflight and not disabled.
> +     */
> +    if ( test_bit(_IRQ_INPROGRESS, &desc->status) ||
> +         !test_bit(_IRQ_DISABLED, &desc->status) )
> +        return -EBUSY;
> +
> +end:
> +    clear_bit(_IRQ_GUEST, &desc->status);
> +    desc->handler = &no_irq_type;
> +
> +    p->desc = NULL;
> +
> +    vgic_unlock_rank(v_target, rank, flags);
> +
> +
> +    return 0;
> +}
> +
>  int gic_irq_xlate(const u32 *intspec, unsigned int intsize,
>                    unsigned int *out_hwirq,
>                    unsigned int *out_type)
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 0072347..ce5ae1a 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -504,6 +504,54 @@ free_info:
>      return retval;
>  }
>  
> +int release_guest_irq(struct domain *d, unsigned int virq)
> +{
> +    struct irq_desc *desc;
> +    struct irq_guest *info;
> +    unsigned long flags;
> +    struct pending_irq *p;
> +    int ret;
> +
> +    /* Only SPIs are supported */
> +    if ( virq < 32 || virq >= vgic_num_irqs(d) )
> +        return -EINVAL;
> +
> +    p = irq_to_pending(d->vcpu[0], virq);
> +    if ( !p->desc )
> +        return -EINVAL;
> +
> +    desc = p->desc;
> +
> +    spin_lock_irqsave(&desc->lock, flags);
> +
> +    ret = -EINVAL;
> +    if ( !test_bit(_IRQ_GUEST, &desc->status) )
> +        goto unlock;
> +
> +    ret = -EINVAL;
> +
> +    info = irq_get_guest_info(desc);
> +    if ( d != info->d )
> +        goto unlock;
> +
> +    ret = gic_remove_irq_from_guest(d, virq, desc);
> +
> +    spin_unlock_irqrestore(&desc->lock, flags);
> +
> +    if ( !ret )
> +    {
> +        release_irq(desc->irq, info);
> +        xfree(info);
> +    }
> +
> +    return ret;
> +
> +unlock:
> +    spin_unlock_irqrestore(&desc->lock, flags);
> +
> +    return ret;
> +}
> +
>  /*
>   * pirq event channels. We don't use these on ARM, instead we use the
>   * features of the GIC to inject virtualised normal interrupts.
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index fc8a270..4ddfd73 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -133,6 +133,22 @@ void register_vgic_ops(struct domain *d, const struct 
> vgic_ops *ops)
>  
>  void domain_vgic_free(struct domain *d)
>  {
> +    int i;
> +    int ret;
> +
> +    for ( i = 0; i < (d->arch.vgic.nr_spis); i++ )
> +    {
> +        struct pending_irq *p = &d->arch.vgic.pending_irqs[i];
> +
> +        if ( p->desc )
> +        {
> +            ret = release_guest_irq(d, p->irq);
> +            if ( ret )
> +                dprintk(XENLOG_G_WARNING, "d%u: Failed to release virq %u 
> ret = %d\n",
> +                        d->domain_id, p->irq, ret);
> +        }
> +    }
> +
>      xfree(d->arch.vgic.shared_irqs);
>      xfree(d->arch.vgic.pending_irqs);
>      xfree(d->arch.vgic.allocated_irqs);
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index a8ac294..5571189 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -217,6 +217,10 @@ extern int gic_route_irq_to_guest(struct domain *, 
> unsigned int virq,
>                                    struct irq_desc *desc,
>                                    unsigned int priority);
>  
> +/* Remove an IRQ passthrough to a guest */
> +int gic_remove_irq_from_guest(struct domain *d, unsigned int virq,
> +                              struct irq_desc *desc);
> +
>  extern void gic_inject(void);
>  extern void gic_clear_pending_irqs(struct vcpu *v);
>  extern int gic_events_need_delivery(void);
> diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
> index 71b39e7..34b492b 100644
> --- a/xen/include/asm-arm/irq.h
> +++ b/xen/include/asm-arm/irq.h
> @@ -44,6 +44,8 @@ void init_secondary_IRQ(void);
>  
>  int route_irq_to_guest(struct domain *d, unsigned int virq,
>                         unsigned int irq, const char *devname);
> +int release_guest_irq(struct domain *d, unsigned int irq);
> +
>  void arch_move_irqs(struct vcpu *v);
>  
>  /* Set IRQ type for an SPI */
> -- 
> 2.1.4
> 

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