Re: [Xen-devel] [PATCH v5 15/22] xen/arm: ITS: implement hw_irq_controller for LPIs

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Tue, Aug 4, 2015 at 7:15 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> Hi Vijay,
>
> On 27/07/15 12:11, vijay.kilari@xxxxxxxxx wrote:
>> +
>> +static const hw_irq_controller its_host_lpi_type = {
>> +    .typename     = "gic-its",
>> +    .startup      = its_irq_startup,
>> +    .shutdown     = its_irq_shutdown,
>> +    .enable       = its_irq_enable,
>> +    .disable      = its_irq_disable,
>> +    .ack          = its_irq_ack,
>> +    .end          = gicv3_host_irq_end,
>> +    .set_affinity = its_irq_set_affinity,
>> +};
>> +
>> +static const hw_irq_controller its_guest_lpi_type = {
>> +    .typename     = "gic-its",
>> +    .startup      = its_irq_startup,
>> +    .shutdown     = its_irq_shutdown,
>> +    .enable       = its_irq_enable,
>> +    .disable      = its_irq_disable,
>> +    .ack          = its_irq_ack,
>> +    .end          = gicv3_guest_irq_end,
>> +    .set_affinity = its_irq_set_affinity,
>> +};
>> +
>> +hw_irq_controller *its_get_host_lpi_type(void)
>> +{
>> +    return &its_host_lpi_type;
>> +}
>> +
>> +hw_irq_controller *its_get_guest_lpi_type(void)
>> +{
>> +    return &its_guest_lpi_type;
>> +}
>> +
>
> Please directly export the variables.

These API's are called to assign hw_irq_handler to LPI
by generic code

>
>>  /*
>>   * How we allocate LPIs:
>>   *
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index 98d45bc..58e878e 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -40,6 +40,7 @@
>>  #include <asm/device.h>
>>  #include <asm/gic.h>
>>  #include <asm/gic_v3_defs.h>
>> +#include <asm/gic-its.h>
>>  #include <asm/cpufeature.h>
>>
>>  /* Global state */
>> @@ -1033,15 +1034,19 @@ static void gicv3_irq_ack(struct irq_desc *desc)
>>      /* No ACK -- reading IAR has done this for us */
>>  }
>>
>> -static void gicv3_host_irq_end(struct irq_desc *desc)
>> +void gicv3_host_irq_end(struct irq_desc *desc)
>>  {
>>      /* Lower the priority */
>>      gicv3_eoi_irq(desc);
>> -    /* Deactivate */
>> -    gicv3_dir_irq(desc);
>> +    /*
>> +     * LPIs does not have active state. Do do not deactivate,
>> +     *  when EOI mode is set to 1.
>> +     */
>> +    if ( !gic_is_lpi(desc->irq) )
>> +        gicv3_dir_irq(desc);
>
> I already told you that the goal of the hw_irq_controller is to avoid
> checking the interrupt for every callback.
>
> If the current function doesn't work for you, introduce a new one. But
> don't put an if inside.

This is what I have done in patch v4

http://lists.xen.org/archives/html/xen-devel/2015-07/msg02128.html

But Ian suggested to export gicv3_host_irq_end instead of calling
gicv3_eoi_irq()

Regards
Vijay

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