|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 09/45] ARM: GIC: Allow tweaking the active and pending state of an IRQ
Hi,
On 15/03/18 20:30, 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.
after having discussed this with Julien off-line, we can simplify the
_IRQ_INPROGRESS setting. See below:
> For this it adds gicv2/3_peek_irq() helper functions, to read a bit in a
> bitmap spread over several MMIO registers.
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> ---
> Changelog v1 ... v2:
> - properly set _IRQ_INPROGRESS bit
> - add gicv[23]_peek_irq() (pulled in from later patch)
> - move wrappers functions into gic.h
>
> xen/arch/arm/gic-v2.c | 48 +++++++++++++++++++++++++++++++++++++++++++
> xen/arch/arm/gic-v3.c | 52
> +++++++++++++++++++++++++++++++++++++++++++++++
> xen/include/asm-arm/gic.h | 24 ++++++++++++++++++++++
> 3 files changed, 124 insertions(+)
>
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 7dfe6fc68d..c6fcbf59d0 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -243,6 +243,52 @@ static void gicv2_poke_irq(struct irq_desc *irqd,
> uint32_t offset)
> writel_gicd(1U << (irqd->irq % 32), offset + (irqd->irq / 32) * 4);
> }
>
> +static bool gicv2_peek_irq(struct irq_desc *irqd, uint32_t offset)
> +{
> + uint32_t reg;
> +
> + reg = readl_gicd(offset + (irqd->irq / 32) * 4) & (1U << (irqd->irq %
> 32));
> +
> + return reg;
> +}
> +
> +static void gicv2_set_active_state(struct irq_desc *irqd, bool active)
> +{
> + ASSERT(spin_is_locked(&irqd->lock));
> +
> + if ( active )
> + {
> + if ( test_bit(_IRQ_GUEST, &irqd->status) )
> + set_bit(_IRQ_INPROGRESS, &irqd->status);
> + gicv2_poke_irq(irqd, GICD_ISACTIVER);
> + }
> + else
> + {
> + gicv2_poke_irq(irqd, GICD_ICACTIVER);
> + if ( !gicv2_peek_irq(irqd, GICD_ISPENDR) &&
> + test_bit(_IRQ_GUEST, &irqd->status) )
> + clear_bit(_IRQ_INPROGRESS, &irqd->status);
The check for the pending bit shouldn't be necessary:
- If the interrupt is (still) pending, clearing the active bit should
trigger it again. The _IRQ_INPROGRESS bit will be set in turn.
- If the interrupt is not pending, we need to clear the bit anyway.
So reading the pending state is not necessary, we can always
unconditionally clear the _IRQ_INPROGRESS bit, ideally before writing to
ICACTIVER.
> + }
> +}
> +
> +static void gicv2_set_pending_state(struct irq_desc *irqd, bool pending)
> +{
> + ASSERT(spin_is_locked(&irqd->lock));
> +
> + if ( pending )
> + {
> + /* The INPROGRESS bit will be set when the interrupt fires. */
> + gicv2_poke_irq(irqd, GICD_ISPENDR);
> + }
> + else
> + {
> + gicv2_poke_irq(irqd, GICD_ICPENDR);
> + if ( !gicv2_peek_irq(irqd, GICD_ISACTIVER) &&
> + test_bit(_IRQ_GUEST, &irqd->status) )
> + clear_bit(_IRQ_INPROGRESS, &irqd->status);
We should not need to touch the _IRQ_INPROGRESS bit here. That bit
really shadows the *active* bit, so changing the pending state should
not matter here:
- If the h/w IRQ is active, the bit is set already and should remain so,
as Xen and the guest are still dealing with it. Clearing the h/w pending
state does not change that.
- If the h/w IRQ is not active, the _IRQ_INPROGRESS bit is not set, so
clearing it would be a NOP.
So we can remove the _IRQ_INPROGRESS handling here completely.
I will amend the code accordingly, including the respective GICv3 parts.
Cheers,
Andre.
> + }
> +}
> +
> static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int type)
> {
> uint32_t cfg, actual, edgebit;
> @@ -1277,6 +1323,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 392cf91b58..316f2c4142 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -444,6 +444,19 @@ static void gicv3_poke_irq(struct irq_desc *irqd, u32
> offset, bool wait_for_rwp)
> gicv3_wait_for_rwp(irqd->irq);
> }
>
> +static bool gicv3_peek_irq(struct irq_desc *irqd, u32 offset)
> +{
> + void __iomem *base;
> + unsigned int irq = irqd->irq;
> +
> + if ( irq >= NR_GIC_LOCAL_IRQS)
> + base = GICD + (irq / 32) * 4;
> + else
> + base = GICD_RDIST_SGI_BASE;
> +
> + return !!(readl(base + offset) & (1U << (irq % 32)));
> +}
> +
> static void gicv3_unmask_irq(struct irq_desc *irqd)
> {
> gicv3_poke_irq(irqd, GICD_ISENABLER, false);
> @@ -477,6 +490,43 @@ static unsigned int gicv3_read_irq(void)
> return irq;
> }
>
> +static void gicv3_set_active_state(struct irq_desc *irqd, bool active)
> +{
> + ASSERT(spin_is_locked(&irqd->lock));
> +
> + if ( active )
> + {
> + if ( test_bit(_IRQ_GUEST, &irqd->status) )
> + set_bit(_IRQ_INPROGRESS, &irqd->status);
> + gicv3_poke_irq(irqd, GICD_ISACTIVER, false);
> + }
> + else
> + {
> + gicv3_poke_irq(irqd, GICD_ICACTIVER, false);
> + if ( !gicv3_peek_irq(irqd, GICD_ISPENDR) &&
> + test_bit(_IRQ_GUEST, &irqd->status) )
> + clear_bit(_IRQ_INPROGRESS, &irqd->status);
> + }
> +}
> +
> +static void gicv3_set_pending_state(struct irq_desc *irqd, bool pending)
> +{
> + ASSERT(spin_is_locked(&irqd->lock));
> +
> + if ( pending )
> + {
> + /* The INPROGRESS bit will be set when the interrupt fires. */
> + gicv3_poke_irq(irqd, GICD_ISPENDR, false);
> + }
> + else
> + {
> + gicv3_poke_irq(irqd, GICD_ICPENDR, false);
> + if ( !gicv3_peek_irq(irqd, GICD_ISACTIVER) &&
> + test_bit(_IRQ_GUEST, &irqd->status) )
> + clear_bit(_IRQ_INPROGRESS, &irqd->status);
> + }
> +}
> +
> static inline uint64_t gicv3_mpidr_to_affinity(int cpu)
> {
> uint64_t mpidr = cpu_logical_map(cpu);
> @@ -1766,6 +1816,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 565b0875ca..21cf35f106 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -344,6 +344,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 */
> @@ -392,6 +396,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,
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |