Re: [Xen-devel] [PATCH v3a 03/39] ARM: GIC: Allow tweaking the active and pending state of an IRQ

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 22 Mar 2018, Andre Przywara wrote:
> When playing around with hardware mapped, level triggered virtual IRQs,
> there is the need to explicitly set the active or pending state of an
> interrupt at some point.
> To prepare the GIC for that, we introduce a set_active_state() and a
> set_pending_state() function to let the VGIC manipulate the state of
> an associated hardware IRQ.
> This takes care of properly setting the _IRQ_INPROGRESS bit.
> 
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

> ---
> Changelog v3 ... v3a:
> - always set/clear _IRQ_INPROGRESS bit (not only for guest IRQs)
> - add comments
> 
> Changelog v2 ... v3:
> - extend comments to note preliminary nature of vgic_get_lpi()
> 
> Changelog v1 ... v2:
> - reorder header file inclusion
> 
>  xen/arch/arm/gic-v2.c     | 41 +++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/gic-v3.c     | 37 +++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/gic.h | 24 ++++++++++++++++++++++++
>  3 files changed, 102 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index aa0fc6c1a1..7374686235 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -243,6 +243,45 @@ static void gicv2_poke_irq(struct irq_desc *irqd, 
> uint32_t offset)
>      writel_gicd(1U << (irqd->irq % 32), offset + (irqd->irq / 32) * 4);
>  }
>  
> +/*
> + * This is forcing the active state of an interrupt, somewhat circumventing
> + * the normal interrupt flow and the GIC state machine. So use with care
> + * and only if you know what you are doing. For this reason we also have to
> + * tinker with the _IRQ_INPROGRESS bit here, since the normal IRQ handler
> + * will not be involved.
> + */
> +static void gicv2_set_active_state(struct irq_desc *irqd, bool active)
> +{
> +    ASSERT(spin_is_locked(&irqd->lock));
> +
> +    if ( active )
> +    {
> +        set_bit(_IRQ_INPROGRESS, &irqd->status);
> +        gicv2_poke_irq(irqd, GICD_ISACTIVER);
> +    }
> +    else
> +    {
> +        clear_bit(_IRQ_INPROGRESS, &irqd->status);
> +        gicv2_poke_irq(irqd, GICD_ICACTIVER);
> +    }
> +}
> +
> +static void gicv2_set_pending_state(struct irq_desc *irqd, bool pending)
> +{
> +    ASSERT(spin_is_locked(&irqd->lock));
> +
> +    if ( pending )
> +    {
> +        /* The _IRQ_INPROGRESS bit will be set when the interrupt fires. */
> +        gicv2_poke_irq(irqd, GICD_ISPENDR);
> +    }
> +    else
> +    {
> +        /* The _IRQ_INPROGRESS remains unchanged. */
> +        gicv2_poke_irq(irqd, GICD_ICPENDR);
> +    }
> +}
> +
>  static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int type)
>  {
>      uint32_t cfg, actual, edgebit;
> @@ -1278,6 +1317,8 @@ const static struct gic_hw_operations gicv2_ops = {
>      .eoi_irq             = gicv2_eoi_irq,
>      .deactivate_irq      = gicv2_dir_irq,
>      .read_irq            = gicv2_read_irq,
> +    .set_active_state    = gicv2_set_active_state,
> +    .set_pending_state   = gicv2_set_pending_state,
>      .set_irq_type        = gicv2_set_irq_type,
>      .set_irq_priority    = gicv2_set_irq_priority,
>      .send_SGI            = gicv2_send_SGI,
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index cb41844af2..a5105ac9e7 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -477,6 +477,41 @@ static unsigned int gicv3_read_irq(void)
>      return irq;
>  }
>  
> +/*
> + * This is forcing the active state of an interrupt, somewhat circumventing
> + * the normal interrupt flow and the GIC state machine. So use with care
> + * and only if you know what you are doing. For this reason we also have to
> + * tinker with the _IRQ_INPROGRESS bit here, since the normal IRQ handler
> + * will not be involved.
> + */
> +static void gicv3_set_active_state(struct irq_desc *irqd, bool active)
> +{
> +    ASSERT(spin_is_locked(&irqd->lock));
> +
> +    if ( active )
> +    {
> +        set_bit(_IRQ_INPROGRESS, &irqd->status);
> +        gicv3_poke_irq(irqd, GICD_ISACTIVER, false);
> +    }
> +    else
> +    {
> +        clear_bit(_IRQ_INPROGRESS, &irqd->status);
> +        gicv3_poke_irq(irqd, GICD_ICACTIVER, false);
> +    }
> +}
> +
> +static void gicv3_set_pending_state(struct irq_desc *irqd, bool pending)
> +{
> +    ASSERT(spin_is_locked(&irqd->lock));
> +
> +    if ( pending )
> +        /* The _IRQ_INPROGRESS bit will be set when the interrupt fires. */
> +        gicv3_poke_irq(irqd, GICD_ISPENDR, false);
> +    else
> +        /* The _IRQ_INPROGRESS bit will remain unchanged. */
> +        gicv3_poke_irq(irqd, GICD_ICPENDR, false);
> +}
> +
>  static inline uint64_t gicv3_mpidr_to_affinity(int cpu)
>  {
>       uint64_t mpidr = cpu_logical_map(cpu);
> @@ -1769,6 +1804,8 @@ static const struct gic_hw_operations gicv3_ops = {
>      .eoi_irq             = gicv3_eoi_irq,
>      .deactivate_irq      = gicv3_dir_irq,
>      .read_irq            = gicv3_read_irq,
> +    .set_active_state    = gicv3_set_active_state,
> +    .set_pending_state   = gicv3_set_pending_state,
>      .set_irq_type        = gicv3_set_irq_type,
>      .set_irq_priority    = gicv3_set_irq_priority,
>      .send_SGI            = gicv3_send_sgi,
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 3079387e06..2aca243ac3 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -345,6 +345,10 @@ struct gic_hw_operations {
>      void (*deactivate_irq)(struct irq_desc *irqd);
>      /* Read IRQ id and Ack */
>      unsigned int (*read_irq)(void);
> +    /* Force the active state of an IRQ by accessing the distributor */
> +    void (*set_active_state)(struct irq_desc *irqd, bool state);
> +    /* Force the pending state of an IRQ by accessing the distributor */
> +    void (*set_pending_state)(struct irq_desc *irqd, bool state);
>      /* Set IRQ type */
>      void (*set_irq_type)(struct irq_desc *desc, unsigned int type);
>      /* Set IRQ priority */
> @@ -393,6 +397,26 @@ static inline unsigned int gic_get_nr_lrs(void)
>      return gic_hw_ops->info->nr_lrs;
>  }
>  
> +/*
> + * Set the active state of an IRQ. This should be used with care, as this
> + * directly forces the active bit, without considering the GIC state machine.
> + * For private IRQs this only works for those of the current CPU.
> + */
> +static inline void gic_set_active_state(struct irq_desc *irqd, bool state)
> +{
> +    gic_hw_ops->set_active_state(irqd, state);
> +}
> +
> +/*
> + * Set the pending state of an IRQ. This should be used with care, as this
> + * directly forces the pending bit, without considering the GIC state 
> machine.
> + * For private IRQs this only works for those of the current CPU.
> + */
> +static inline void gic_set_pending_state(struct irq_desc *irqd, bool state)
> +{
> +    gic_hw_ops->set_pending_state(irqd, state);
> +}
> +
>  void register_gic_ops(const struct gic_hw_operations *ops);
>  int gic_make_hwdom_dt_node(const struct domain *d,
>                             const struct dt_device_node *gic,
> -- 
> 2.14.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel