| Message ID | 20170526173540.10066-6-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andre, On 26/05/17 18:35, Andre Przywara wrote: > To avoid code duplication in a later patch, introduce a generic function > to remove a virtual IRQ from the VGIC. > Call that function instead of the open-coded version in vgic_migrate_irq(). > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/gic.c | 9 +++++++++ > xen/arch/arm/vgic.c | 1 + > xen/include/asm-arm/gic.h | 1 + > 3 files changed, 11 insertions(+) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 9dde146..3a6184a 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -408,6 +408,15 @@ void gic_remove_from_lr_pending(struct vcpu *v, struct pending_irq *p) > list_del_init(&p->lr_queue); > } > > +void gic_remove_irq(struct vcpu *v, struct pending_irq *p) The name is too generic. Remove IRQ from what? > +{ > + ASSERT(spin_is_locked(&v->arch.vgic.lock)); > + > + clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status); > + list_del_init(&p->inflight); > + gic_remove_from_lr_pending(v, p); > +} > + > void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq) > { > struct pending_irq *n = irq_to_pending(v, virtual_irq); > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > index 3993965..b2acc99 100644 > --- a/xen/arch/arm/vgic.c > +++ b/xen/arch/arm/vgic.c > @@ -266,6 +266,7 @@ bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq) > /* If the IRQ is still lr_pending, re-inject it to the new vcpu */ > if ( !list_empty(&p->lr_queue) ) > { > + gic_remove_irq(old, p); > clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status); > list_del_init(&p->lr_queue); > list_del_init(&p->inflight); I was expecting those 3 lines to be removed since you now call gic_remove_irq. > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 3130634..5d5b4cc 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -244,6 +244,7 @@ extern void gic_raise_guest_irq(struct vcpu *v, unsigned int irq, > unsigned int priority); > extern void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq); > extern void gic_remove_from_lr_pending(struct vcpu *v, struct pending_irq *p); > +extern void gic_remove_irq(struct vcpu *v, struct pending_irq *p); > > /* Accept an interrupt from the GIC and dispatch its handler */ > extern void gic_interrupt(struct cpu_user_regs *regs, int is_fiq); > Cheers,
Hi, On 30/05/17 12:31, Julien Grall wrote: > Hi Andre, > > On 26/05/17 18:35, Andre Przywara wrote: >> To avoid code duplication in a later patch, introduce a generic function >> to remove a virtual IRQ from the VGIC. >> Call that function instead of the open-coded version in >> vgic_migrate_irq(). >> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> --- >> xen/arch/arm/gic.c | 9 +++++++++ >> xen/arch/arm/vgic.c | 1 + >> xen/include/asm-arm/gic.h | 1 + >> 3 files changed, 11 insertions(+) >> >> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c >> index 9dde146..3a6184a 100644 >> --- a/xen/arch/arm/gic.c >> +++ b/xen/arch/arm/gic.c >> @@ -408,6 +408,15 @@ void gic_remove_from_lr_pending(struct vcpu *v, >> struct pending_irq *p) >> list_del_init(&p->lr_queue); >> } >> >> +void gic_remove_irq(struct vcpu *v, struct pending_irq *p) > > The name is too generic. Remove IRQ from what? > >> +{ >> + ASSERT(spin_is_locked(&v->arch.vgic.lock)); >> + >> + clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status); >> + list_del_init(&p->inflight); >> + gic_remove_from_lr_pending(v, p); >> +} >> + >> void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq) >> { >> struct pending_irq *n = irq_to_pending(v, virtual_irq); >> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c >> index 3993965..b2acc99 100644 >> --- a/xen/arch/arm/vgic.c >> +++ b/xen/arch/arm/vgic.c >> @@ -266,6 +266,7 @@ bool vgic_migrate_irq(struct vcpu *old, struct >> vcpu *new, unsigned int irq) >> /* If the IRQ is still lr_pending, re-inject it to the new vcpu */ >> if ( !list_empty(&p->lr_queue) ) >> { >> + gic_remove_irq(old, p); >> clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status); >> list_del_init(&p->lr_queue); >> list_del_init(&p->inflight); > > I was expecting those 3 lines to be removed since you now call > gic_remove_irq. Argh, of course. I actually fixed this, but somehow it sneaked in again when I moved this patch around in the series. Sorry for that. Cheers, Andre. >> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h >> index 3130634..5d5b4cc 100644 >> --- a/xen/include/asm-arm/gic.h >> +++ b/xen/include/asm-arm/gic.h >> @@ -244,6 +244,7 @@ extern void gic_raise_guest_irq(struct vcpu *v, >> unsigned int irq, >> unsigned int priority); >> extern void gic_raise_inflight_irq(struct vcpu *v, unsigned int >> virtual_irq); >> extern void gic_remove_from_lr_pending(struct vcpu *v, struct >> pending_irq *p); >> +extern void gic_remove_irq(struct vcpu *v, struct pending_irq *p); >> >> /* Accept an interrupt from the GIC and dispatch its handler */ >> extern void gic_interrupt(struct cpu_user_regs *regs, int is_fiq); >> > > Cheers, >
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 9dde146..3a6184a 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -408,6 +408,15 @@ void gic_remove_from_lr_pending(struct vcpu *v, struct pending_irq *p) list_del_init(&p->lr_queue); } +void gic_remove_irq(struct vcpu *v, struct pending_irq *p) +{ + ASSERT(spin_is_locked(&v->arch.vgic.lock)); + + clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status); + list_del_init(&p->inflight); + gic_remove_from_lr_pending(v, p); +} + void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq) { struct pending_irq *n = irq_to_pending(v, virtual_irq); diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index 3993965..b2acc99 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -266,6 +266,7 @@ bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq) /* If the IRQ is still lr_pending, re-inject it to the new vcpu */ if ( !list_empty(&p->lr_queue) ) { + gic_remove_irq(old, p); clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status); list_del_init(&p->lr_queue); list_del_init(&p->inflight); diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h index 3130634..5d5b4cc 100644 --- a/xen/include/asm-arm/gic.h +++ b/xen/include/asm-arm/gic.h @@ -244,6 +244,7 @@ extern void gic_raise_guest_irq(struct vcpu *v, unsigned int irq, unsigned int priority); extern void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq); extern void gic_remove_from_lr_pending(struct vcpu *v, struct pending_irq *p); +extern void gic_remove_irq(struct vcpu *v, struct pending_irq *p); /* Accept an interrupt from the GIC and dispatch its handler */ extern void gic_interrupt(struct cpu_user_regs *regs, int is_fiq);
To avoid code duplication in a later patch, introduce a generic function to remove a virtual IRQ from the VGIC. Call that function instead of the open-coded version in vgic_migrate_irq(). Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- xen/arch/arm/gic.c | 9 +++++++++ xen/arch/arm/vgic.c | 1 + xen/include/asm-arm/gic.h | 1 + 3 files changed, 11 insertions(+)