[Xen 4.4.x] call vgic_en/disable_irqs holding the rank_lock

[Xen 4.4.x] call vgic_en/disable_irqs holding the rank_lock

[Stefano Stabellini]

Modifying ienable and calling vgic_enable_irqs or vgic_disable_irqs need
to be atomic. Move the calls to vgic_enable_irqs and vgic_disable_irqs
before releasing the rank lock.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 850006c..fa5acd6 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -509,8 +509,8 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
         vgic_lock_rank(v, rank);
         tr = rank->ienable;
         rank->ienable |= *r;
-        vgic_unlock_rank(v, rank);
         vgic_enable_irqs(v, (*r) & (~tr), gicd_reg - GICD_ISENABLER);
+        vgic_unlock_rank(v, rank);
         return 1;
 
     case GICD_ICENABLER ... GICD_ICENABLERN:
@@ -520,8 +520,8 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
         vgic_lock_rank(v, rank);
         tr = rank->ienable;
         rank->ienable &= ~*r;
-        vgic_unlock_rank(v, rank);
         vgic_disable_irqs(v, (*r) & tr, gicd_reg - GICD_ICENABLER);
+        vgic_unlock_rank(v, rank);
         return 1;
 
     case GICD_ISPENDR ... GICD_ISPENDRN:

Re: [Xen 4.4.x] call vgic_en/disable_irqs holding the rank_lock

[Ian Campbell]

On Tue, 2014-06-24 at 19:30 +0100, Stefano Stabellini wrote:
> Modifying ienable and calling vgic_enable_irqs or vgic_disable_irqs need
> to be atomic. Move the calls to vgic_enable_irqs and vgic_disable_irqs
> before releasing the rank lock.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Do I understand correctly that this is a 4.4 only fix which cannot
simply be a backport of some changeset because of other changes in 4.5?

> 
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 850006c..fa5acd6 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -509,8 +509,8 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
>          vgic_lock_rank(v, rank);
>          tr = rank->ienable;
>          rank->ienable |= *r;
> -        vgic_unlock_rank(v, rank);
>          vgic_enable_irqs(v, (*r) & (~tr), gicd_reg - GICD_ISENABLER);
> +        vgic_unlock_rank(v, rank);
>          return 1;
>  
>      case GICD_ICENABLER ... GICD_ICENABLERN:
> @@ -520,8 +520,8 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
>          vgic_lock_rank(v, rank);
>          tr = rank->ienable;
>          rank->ienable &= ~*r;
> -        vgic_unlock_rank(v, rank);
>          vgic_disable_irqs(v, (*r) & tr, gicd_reg - GICD_ICENABLER);
> +        vgic_unlock_rank(v, rank);
>          return 1;
>  
>      case GICD_ISPENDR ... GICD_ISPENDRN:

Re: [Xen 4.4.x] call vgic_en/disable_irqs holding the rank_lock

[Julien Grall]

On 06/25/2014 09:37 AM, Ian Campbell wrote:
> On Tue, 2014-06-24 at 19:30 +0100, Stefano Stabellini wrote:
>> Modifying ienable and calling vgic_enable_irqs or vgic_disable_irqs need
>> to be atomic. Move the calls to vgic_enable_irqs and vgic_disable_irqs
>> before releasing the rank lock.
>>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> Do I understand correctly that this is a 4.4 only fix which cannot
> simply be a backport of some changeset because of other changes in 4.5?

This is part of patch #1 of Stefano's migration series [1]. As the
interrupt management has been rework between 4.4 and 4.5, I asked
Stefano to send a separate patch based on Xen 4.4.

Regards,

[1] http://lists.xen.org/archives/html/xen-devel/2014-06/msg03019.html

-- 
Julien Grall