| Message ID | 20180305160415.16760-23-andre.przywara@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | New VGIC(-v2) implementation | expand |
Hi Andre, On 05/03/18 16:03, 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. > > Signed-off-by: Andre Przywara <andre.przywara@linaro.org> > --- > Changelog RFC ... v1: > - use struct irq_desc* in the interface (instead of just the IRQ number) > - add set_pending_state() (needed later) > > xen/arch/arm/gic-v2.c | 32 ++++++++++++++++++++++++++++++++ > xen/arch/arm/gic-v3.c | 28 ++++++++++++++++++++++++++++ > xen/arch/arm/gic.c | 10 ++++++++++ > xen/include/asm-arm/gic.h | 10 ++++++++++ > 4 files changed, 80 insertions(+) > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index c5ec0d4d35..74169b5633 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -241,6 +241,36 @@ static void gicv2_poke_irq(struct irq_desc *irqd, uint32_t offset) > writel_gicd(1U << (irqd->irq % 32), offset + (irqd->irq / 32) * 4); > } > > +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 > + { Why don't you clear _IRQ_INPROGRESS here? > + 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 ) > + { > + set_bit(_IRQ_INPROGRESS, &irqd->status); Why do you set _IRQ_INPROGRESS here? If you set the hardware interrupt pending, it will fire and then set this bit for you. > + gicv2_poke_irq(irqd, GICD_ISPENDR); > + } > + else > + { > + gicv2_poke_irq(irqd, GICD_ICPENDR); > + } > +} > + > static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int type) > { > uint32_t cfg, actual, edgebit; > @@ -1251,6 +1281,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 44dfba2267..c96469f09d 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c My remark are the same as GICv2. > @@ -477,6 +477,32 @@ 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 ) > + { > + set_bit(_IRQ_INPROGRESS, &irqd->status); > + gicv3_poke_irq(irqd, GICD_ISACTIVER, false); > + } > + else > + 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 ) > + { > + set_bit(_IRQ_INPROGRESS, &irqd->status); > + gicv3_poke_irq(irqd, GICD_ISPENDR, false); > + } > + else > + gicv3_poke_irq(irqd, GICD_ICPENDR, false); > +} > + > static inline uint64_t gicv3_mpidr_to_affinity(int cpu) > { > uint64_t mpidr = cpu_logical_map(cpu); > @@ -1723,6 +1749,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/arch/arm/gic.c b/xen/arch/arm/gic.c > index 968e46fabb..f1329a630a 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -87,6 +87,16 @@ void gic_restore_state(struct vcpu *v) > isb(); > } > > +void gic_set_active_state(struct irq_desc *irqd, bool state) > +{ > + gic_hw_ops->set_active_state(irqd, state); > +} > + > +void gic_set_pending_state(struct irq_desc *irqd, bool state) > +{ > + gic_hw_ops->set_pending_state(irqd, state); > +} This possibly can be static inline in gic.h? > + > /* desc->irq needs to be disabled before calling this function */ > void gic_set_irq_type(struct irq_desc *desc, unsigned int type) > { > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 89a07ae6b4..46dcb0fe7c 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -239,6 +239,12 @@ DECLARE_PER_CPU(uint64_t, lr_mask); > > extern enum gic_version gic_hw_version(void); > > +/* Force the active state of an IRQ. */ > +void gic_set_active_state(struct irq_desc *irqd, bool state); > + > +/* Force the pending state of an IRQ. */ > +void gic_set_pending_state(struct irq_desc *irqd, bool state); > + > /* Program the IRQ type into the GIC */ > void gic_set_irq_type(struct irq_desc *desc, unsigned int type); > > @@ -348,6 +354,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); Based on the discussion we had today, could expand the comment saying that anyone who wants to use those 2 helpers need to carefully think before calling them? > /* Set IRQ type */ > void (*set_irq_type)(struct irq_desc *desc, unsigned int type); > /* Set IRQ priority */ > Cheers,
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c index c5ec0d4d35..74169b5633 100644 --- a/xen/arch/arm/gic-v2.c +++ b/xen/arch/arm/gic-v2.c @@ -241,6 +241,36 @@ static void gicv2_poke_irq(struct irq_desc *irqd, uint32_t offset) writel_gicd(1U << (irqd->irq % 32), offset + (irqd->irq / 32) * 4); } +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 + { + 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 ) + { + set_bit(_IRQ_INPROGRESS, &irqd->status); + gicv2_poke_irq(irqd, GICD_ISPENDR); + } + else + { + gicv2_poke_irq(irqd, GICD_ICPENDR); + } +} + static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int type) { uint32_t cfg, actual, edgebit; @@ -1251,6 +1281,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 44dfba2267..c96469f09d 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -477,6 +477,32 @@ 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 ) + { + set_bit(_IRQ_INPROGRESS, &irqd->status); + gicv3_poke_irq(irqd, GICD_ISACTIVER, false); + } + else + 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 ) + { + set_bit(_IRQ_INPROGRESS, &irqd->status); + gicv3_poke_irq(irqd, GICD_ISPENDR, false); + } + else + gicv3_poke_irq(irqd, GICD_ICPENDR, false); +} + static inline uint64_t gicv3_mpidr_to_affinity(int cpu) { uint64_t mpidr = cpu_logical_map(cpu); @@ -1723,6 +1749,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/arch/arm/gic.c b/xen/arch/arm/gic.c index 968e46fabb..f1329a630a 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -87,6 +87,16 @@ void gic_restore_state(struct vcpu *v) isb(); } +void gic_set_active_state(struct irq_desc *irqd, bool state) +{ + gic_hw_ops->set_active_state(irqd, state); +} + +void gic_set_pending_state(struct irq_desc *irqd, bool state) +{ + gic_hw_ops->set_pending_state(irqd, state); +} + /* desc->irq needs to be disabled before calling this function */ void gic_set_irq_type(struct irq_desc *desc, unsigned int type) { diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h index 89a07ae6b4..46dcb0fe7c 100644 --- a/xen/include/asm-arm/gic.h +++ b/xen/include/asm-arm/gic.h @@ -239,6 +239,12 @@ DECLARE_PER_CPU(uint64_t, lr_mask); extern enum gic_version gic_hw_version(void); +/* Force the active state of an IRQ. */ +void gic_set_active_state(struct irq_desc *irqd, bool state); + +/* Force the pending state of an IRQ. */ +void gic_set_pending_state(struct irq_desc *irqd, bool state); + /* Program the IRQ type into the GIC */ void gic_set_irq_type(struct irq_desc *desc, unsigned int type); @@ -348,6 +354,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 */
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. Signed-off-by: Andre Przywara <andre.przywara@linaro.org> --- Changelog RFC ... v1: - use struct irq_desc* in the interface (instead of just the IRQ number) - add set_pending_state() (needed later) xen/arch/arm/gic-v2.c | 32 ++++++++++++++++++++++++++++++++ xen/arch/arm/gic-v3.c | 28 ++++++++++++++++++++++++++++ xen/arch/arm/gic.c | 10 ++++++++++ xen/include/asm-arm/gic.h | 10 ++++++++++ 4 files changed, 80 insertions(+)