Re: [Xen-devel] [PATCH 2/2] xen/arm: initialize the GIC irq properties of interrupts routed to guests

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-01-09 at 16:17 +0000, Stefano Stabellini wrote:
> We are currently initializing GIC irq properties (ITARGETSR, IPRIORITYR,
> and GICD_ICFGR) only in gic_route_irq, that is not called for guest
> interrupts at all.
> Move the initialization into a separate function
> (gic_set_irq_properties) and call it from gic_route_irq_to_guest.

Should this all be done in response to emulating the writes to the GICD
which the guest itself makes?

e.g. ITARGETSR needs to be emulated to map to VCPUs etc. Or maybe we
want such interrupts to always come to any PCPU and we then take VCPUs
based purely on an emulated ITARGETSR?

Hrm, yes, I think I've convinced myself your approach is right, at least
until we start doing direct interrupt injection to guests.

(I'll review the actual patch later)

> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
>  xen/arch/arm/gic.c |   49 +++++++++++++++++++++++++++++++------------------
>  1 files changed, 31 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 8c00a1c..9f20c0f 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -174,12 +174,36 @@ static hw_irq_controller gic_guest_irq_type = {
>      .set_affinity = gic_irq_set_affinity,
>  };
>  
> +/* needs to be called with gic.lock held */
> +static void gic_set_irq_properties(unsigned int irq, bool_t level,
> +        unsigned int cpu_mask, unsigned int priority)
> +{
> +    volatile unsigned char *bytereg;
> +    uint32_t cfg, edgebit;
> +
> +    /* Set edge / level */
> +    cfg = GICD[GICD_ICFGR + irq / 16];
> +    edgebit = 2u << (2 * (irq % 16));
> +    if ( level )
> +        cfg &= ~edgebit;
> +    else
> +        cfg |= edgebit;
> +    GICD[GICD_ICFGR + irq / 16] = cfg;
> +
> +    /* Set target CPU mask (RAZ/WI on uniprocessor) */
> +    bytereg = (unsigned char *) (GICD + GICD_ITARGETSR);
> +    bytereg[irq] = cpu_mask;
> +
> +    /* Set priority */
> +    bytereg = (unsigned char *) (GICD + GICD_IPRIORITYR);
> +    bytereg[irq] = priority;
> +
> +}
> +
>  /* Program the GIC to route an interrupt */
>  static int gic_route_irq(unsigned int irq, bool_t level,
>                           unsigned int cpu_mask, unsigned int priority)
>  {
> -    volatile unsigned char *bytereg;
> -    uint32_t cfg, edgebit;
>      struct irq_desc *desc = irq_to_desc(irq);
>      unsigned long flags;
>  
> @@ -202,22 +226,7 @@ static int gic_route_irq(unsigned int irq, bool_t level,
>      /* Disable interrupt */
>      desc->handler->shutdown(desc);
>  
> -    /* Set edge / level */
> -    cfg = GICD[GICD_ICFGR + irq / 16];
> -    edgebit = 2u << (2 * (irq % 16));
> -    if ( level )
> -        cfg &= ~edgebit;
> -    else
> -        cfg |= edgebit;
> -    GICD[GICD_ICFGR + irq / 16] = cfg;
> -
> -    /* Set target CPU mask (RAZ/WI on uniprocessor) */
> -    bytereg = (unsigned char *) (GICD + GICD_ITARGETSR);
> -    bytereg[irq] = cpu_mask;
> -
> -    /* Set priority */
> -    bytereg = (unsigned char *) (GICD + GICD_IPRIORITYR);
> -    bytereg[irq] = priority;
> +    gic_set_irq_properties(irq, level, cpu_mask, priority);
>  
>      spin_unlock(&gic.lock);
>      spin_unlock_irqrestore(&desc->lock, flags);
> @@ -555,10 +564,13 @@ int gic_route_irq_to_guest(struct domain *d, unsigned 
> int irq,
>      action->name = devname;
>  
>      spin_lock_irqsave(&desc->lock, flags);
> +    spin_lock(&gic.lock);
>  
>      desc->handler = &gic_guest_irq_type;
>      desc->status |= IRQ_GUEST;
>  
> +    gic_set_irq_properties(irq, 1, 1u << smp_processor_id(), 0xa0);
> +
>      retval = __setup_irq(desc, irq, action);
>      if (retval) {
>          xfree(action);
> @@ -566,6 +578,7 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int 
> irq,
>      }
>  
>  out:
> +    spin_unlock(&gic.lock);
>      spin_unlock_irqrestore(&desc->lock, flags);
>      return retval;
>  }



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