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

[Julien Grall <julien.grall@linaro.org>]

On 05/20/2013 04:32 PM, Ian Campbell wrote:

> 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?


Right, but it's only for the timer as timer is "shared" between Xen and
dom0 (ie: we fake the timer IRQs).

> 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...


vtimer will manually inject the virtual IRQ as IRQs are forwarded to the
running vCPU. gic_route_irq_to_guest will always route to a specific domain.

I though to add a new value for dt_used_by which parse the device node
IRQ/range but not map them in the domain. Because we don't need to know
if the IRQ is a PPI after the end of construct_dom0.

>> +     */
>> +    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.


Will be fixed on the next version of the patch.

-- 
Julien