Re: [Xen-devel] [PATCH 10/12] ARM: VGIC: factor out vgic_connect_hw_irq()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 19 Oct 2017, Andre Przywara wrote:
> At the moment we happily access VGIC internal data structures like
> the rank and struct pending_irq in gic.c, which should be VGIC agnostic.
> 
> Factor out a new function vgic_connect_hw_irq(), which allows a virtual
> IRQ to be connected to a hardware IRQ (using the hw bit in the LR).
> 
> This removes said accesses to VGIC data structures and improves abstraction.
> 
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>  xen/arch/arm/gic-vgic.c    | 31 +++++++++++++++++++++++++++++++
>  xen/arch/arm/gic.c         | 42 ++++++------------------------------------
>  xen/include/asm-arm/vgic.h |  2 ++
>  3 files changed, 39 insertions(+), 36 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
> index 66cae21e82..bf9455a34e 100644
> --- a/xen/arch/arm/gic-vgic.c
> +++ b/xen/arch/arm/gic-vgic.c
> @@ -385,6 +385,37 @@ void gic_inject(struct vcpu *v)
>          gic_hw_ops->update_hcr_status(GICH_HCR_UIE, 1);
>  }
>  
> +int vgic_connect_hw_irq(struct domain *d, struct vcpu *v, unsigned int virq,
> +                        struct irq_desc *desc)
> +{
> +    unsigned long flags;
> +    /* Use vcpu0 to retrieve the pending_irq struct. Given that we only
> +     * route SPIs to guests, it doesn't make any difference. */
> +    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);
> +    int ret = 0;
> +
> +    /* We are taking to rank lock to prevent parallel connections. */
> +    vgic_lock_rank(v_target, rank, flags);
> +
> +    if ( desc )
> +    {
> +        /* The VIRQ should not be already enabled by the guest */
> +        if ( !p->desc &&
> +             !test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) )
> +            p->desc = desc;
> +        else
> +            ret = -EBUSY;
> +    }
> +    else
> +        p->desc = NULL;
> +
> +    vgic_unlock_rank(v_target, rank, flags);
> +
> +    return ret;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 4cb74d449e..d46a6d54b3 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -128,27 +128,12 @@ void gic_route_irq_to_xen(struct irq_desc *desc, 
> unsigned int priority)
>  int gic_route_irq_to_guest(struct domain *d, unsigned int virq,
>                             struct irq_desc *desc, unsigned int priority)
>  {
> -    unsigned long flags;
> -    /* Use vcpu0 to retrieve the pending_irq struct. Given that we only
> -     * route SPIs to guests, it doesn't make any difference. */
> -    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);
> -    int res = -EBUSY;
> -
>      ASSERT(spin_is_locked(&desc->lock));
>      /* Caller has already checked that the IRQ is an SPI */
>      ASSERT(virq >= 32);
>      ASSERT(virq < vgic_num_irqs(d));
>      ASSERT(!is_lpi(virq));
>  
> -    vgic_lock_rank(v_target, rank, flags);
> -
> -    if ( p->desc ||
> -         /* The VIRQ should not be already enabled by the guest */
> -         test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) )
> -        goto out;
> -
>      desc->handler = gic_hw_ops->gic_guest_irq_type;
>      set_bit(_IRQ_GUEST, &desc->status);
>  
> @@ -156,31 +141,19 @@ int gic_route_irq_to_guest(struct domain *d, unsigned 
> int virq,
>          gic_set_irq_type(desc, desc->arch.type);
>      gic_set_irq_priority(desc, priority);
>  
> -    p->desc = desc;
> -    res = 0;
> -
> -out:
> -    vgic_unlock_rank(v_target, rank, flags);
> -
> -    return res;
> +    return vgic_connect_hw_irq(d, NULL, virq, desc);
>  }
>  
>  /* 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;
> +    int ret;
>  
>      ASSERT(spin_is_locked(&desc->lock));
>      ASSERT(test_bit(_IRQ_GUEST, &desc->status));
> -    ASSERT(p->desc == desc);
>      ASSERT(!is_lpi(virq));
>  
> -    vgic_lock_rank(v_target, rank, flags);
> -
>      if ( d->is_dying )
>      {
>          desc->handler->shutdown(desc);
> @@ -198,19 +171,16 @@ int gic_remove_irq_from_guest(struct domain *d, 
> unsigned int virq,
>           */
>          if ( test_bit(_IRQ_INPROGRESS, &desc->status) ||
>               !test_bit(_IRQ_DISABLED, &desc->status) )
> -        {
> -            vgic_unlock_rank(v_target, rank, flags);
>              return -EBUSY;
> -        }
>      }
>  
> +    ret = vgic_connect_hw_irq(d, NULL, virq, NULL);
> +    if ( ret )
> +        return ret;
> +
>      clear_bit(_IRQ_GUEST, &desc->status);
>      desc->handler = &no_irq_type;
>  
> -    p->desc = NULL;
> -
> -    vgic_unlock_rank(v_target, rank, flags);
> -
>      return 0;
>  }
>  
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index dcdb1acaf3..cf02dc6394 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -220,6 +220,8 @@ int vgic_v2_init(struct domain *d, int *mmio_count);
>  int vgic_v3_init(struct domain *d, int *mmio_count);
>  
>  bool vgic_evtchn_irq_pending(struct vcpu *v);
> +int vgic_connect_hw_irq(struct domain *d, struct vcpu *v, unsigned int virq,
> +                        struct irq_desc *desc);
>  
>  extern int domain_vgic_register(struct domain *d, int *mmio_count);
>  extern int vcpu_vgic_free(struct vcpu *v);
> -- 
> 2.14.1
> 

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