Re: [Xen-devel] [RFC PATCH v2 12/22] ARM: vGIC: protect gic_update_one_lr() with pending_irq lock

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxx>
> ---
>  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,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel