Message ID | 20170721200010.29010-13-andre.przywara@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Andre, On 21/07/17 21:00, Andre Przywara wrote: > When we return from a domain with the active bit set in an LR, > we update our pending_irq accordingly. This touches multiple status > bits, so requires the pending_irq lock. The commit title says "protect gic_update_one_lr()" but here you only mention about one path. Please explain why the other are safe. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/gic.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 9637682..84b282b 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -508,6 +508,7 @@ void gic_update_one_lr(struct vcpu *v, int i) > > if ( lr_val.state & GICH_LR_ACTIVE ) > { > + vgic_irq_lock(p, flags); The function has an ASSERT to check the IRQs are disabled. So it is not necessary to save/restore flags. > set_bit(GIC_IRQ_GUEST_ACTIVE, &p->status); > if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) && > test_and_clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status) ) > @@ -521,6 +522,7 @@ void gic_update_one_lr(struct vcpu *v, int i) > gdprintk(XENLOG_WARNING, "unable to inject hw irq=%d into d%dv%d: already active in LR%d\n", > irq, v->domain->domain_id, v->vcpu_id, i); > } > + vgic_irq_unlock(p, flags); > } > else if ( lr_val.state & GICH_LR_PENDING ) > { > Cheers,
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 9637682..84b282b 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -508,6 +508,7 @@ void gic_update_one_lr(struct vcpu *v, int i) if ( lr_val.state & GICH_LR_ACTIVE ) { + vgic_irq_lock(p, flags); set_bit(GIC_IRQ_GUEST_ACTIVE, &p->status); if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) && test_and_clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status) ) @@ -521,6 +522,7 @@ void gic_update_one_lr(struct vcpu *v, int i) gdprintk(XENLOG_WARNING, "unable to inject hw irq=%d into d%dv%d: already active in LR%d\n", irq, v->domain->domain_id, v->vcpu_id, i); } + vgic_irq_unlock(p, flags); } else if ( lr_val.state & GICH_LR_PENDING ) {
When we return from a domain with the active bit set in an LR, we update our pending_irq accordingly. This touches multiple status bits, so requires the pending_irq lock. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- xen/arch/arm/gic.c | 2 ++ 1 file changed, 2 insertions(+)