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