Re: [Xen-devel] [PATCH] xen/arm: Don't hardcode event channel IRQ

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-05-17 at 11:49 +0100, Julien Grall wrote:
> @@ -467,9 +477,41 @@ static int handle_node(struct domain *d, const struct 
> dt_device_node *np)
>  
>  static int map_devices_from_device_tree(struct domain *d)
>  {
> +    bool_t ppis[NR_PPI_IRQS] = {0};
> +    int res;
> +    unsigned int i;
> +
>      ASSERT(dt_host && (dt_host->sibling == NULL));
>  
> -    return handle_node(d, dt_host);
> +    res = handle_node(d, dt_host, ppis);
> +
> +    /*
> +     * Fill by hand timer IRQS ppis as we don't map it in Dom0
> +     * We assume the host IRQs and the dom0 IRQs are the same for the timer.

This seems a bit fragile, what happens if new interrupts are added in
the future?

Can the gic.c code not track the PPIs which are either assigned to Xen
or a guest?

Or perhaps better vgic.c should know if which if any IRQs are routed for
a given guest. We may need to add a function to allow the vtimer code to
reserve the virtual interrupts which it is going to use. Seems odd that
we don't have an existing equivalent to gic_route_irq_to_guest in
vtimer.c...

> +     */
> +    for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
> +    {
> +        const struct dt_irq *irq = timer_dt_irq(i);
> +
> +        if ( irq_is_ppi(irq->irq) )
> +            ppis[irq->irq - FIRST_PPI_IRQ] = 1;
> +    }
> +
> +    /* Start from the last PPIs and decrease to find a free PPI */
> +    for ( i = NR_PPI_IRQS; i != 0; i-- )
> +    {
> +        if ( !ppis[i - 1] )
> +            break;
> +    }
> +
> +    if ( i == 0 )
> +        panic("Can't find a free PPI for the event channel IRQ\n");
> +
> +    d->arch.evtchn_irq = (i - 1) + FIRST_PPI_IRQ;
> +
> +    printk(XENLOG_INFO "DOM0 Event channel IRQ: %u\n", d->arch.evtchn_irq);
> +
> +    return res;
>  }
>  
>  static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
> @@ -546,16 +588,16 @@ int construct_dom0(struct domain *d)
>  
>      d->max_pages = ~0U;
>  
> -    rc = prepare_dtb(d, &kinfo);
> +    map_devices_from_device_tree(d);

May as well make map_devices_from_device_tree void then, it seems ot
always succeeds or panics.

Ian.


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