diff mbox

[RFC,v2,12/22] ARM: vGIC: protect gic_update_one_lr() with pending_irq lock

Message ID 20170721200010.29010-13-andre.przywara@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andre Przywara July 21, 2017, 8 p.m. UTC
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(+)

Comments

Julien Grall Aug. 15, 2017, 11:17 a.m. UTC | #1
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 mbox

Patch

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 )
     {