Re: [PATCH v7 10/28] xen/arm: ITS: Implement hw_irq_controller for LPIs

[Julien Grall <julien.grall@citrix.com>]

Hi Vijay,

On 18/09/15 14:08, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> 
> Implement hw_irq_controller callbacks required to
> handle LPIs.
> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>

With the 3 changes requested below:

Reviewed-by: Julien Grall <julien.grall@citrix.com>

> ---
>  xen/arch/arm/gic-v3-its.c         |  119 +++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/gic-v3.c             |    2 +-
>  xen/include/asm-arm/gic_v3_defs.h |    2 +
>  3 files changed, 122 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index bc3b73c..0d5c61c 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -444,6 +444,125 @@ static void its_send_discard(struct its_device *dev, u32 event)
>      its_send_single_command(dev->its, &cmd, col);
>  }
>  
> +static void its_flush_and_invalidate_prop(struct irq_desc *desc, u8 *cfg)

NIT: cfg could be const i.e

const u8 *cfg

> +{
> +    struct its_device *its_dev = irqdesc_get_its_device(desc);
> +    u32 vid = irqdesc_get_lpi_event(desc);
> +
> +    ASSERT(vid < its_dev->event_map.nr_lpis);
> +
> +    /*
> +     * Make the above write visible to the redistributors.
> +     * And yes, we're flushing exactly: One. Single. Byte.
> +     * Humpf...
> +     */
> +    if ( gic_rdists->flags & RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING )
> +        clean_and_invalidate_dcache_va_range(cfg, sizeof(*cfg));
> +    else
> +        dsb(ishst);
> +
> +    its_send_inv(its_dev, vid);
> +}

> +const hw_irq_controller its_host_lpi_type = {

As said on v6, the const is not necessary.

> +    .typename     = "gic-its",
> +    .startup      = its_irq_startup,
> +    .shutdown     = its_irq_shutdown,
> +    .enable       = its_irq_enable,
> +    .disable      = its_irq_disable,
> +    .ack          = its_irq_ack,
> +    .end          = its_host_irq_end,
> +    .set_affinity = its_irq_set_affinity,
> +};
> +
> +const hw_irq_controller its_guest_lpi_type = {

Ditto.

> +    .typename     = "gic-its",
> +    .startup      = its_irq_startup,
> +    .shutdown     = its_irq_shutdown,
> +    .enable       = its_irq_enable,
> +    .disable      = its_irq_disable,
> +    .ack          = its_irq_ack,
> +    .end          = its_guest_irq_end,
> +    .set_affinity = its_irq_set_affinity,
> +};
> +
>  /*
>   * How we allocate LPIs:
>   *

Regards,

-- 
Julien Grall