Re: [Xen-devel] [PATCH v9 03/28] ARM: GIC: Add checks for NULL pointer pending_irq's

2017-05-25 Thread Andre Przywara 
Hi,

On 22/05/17 18:15, Julien Grall wrote:
> 
> 
> On 22/05/17 17:49, Andre Przywara wrote:
>> Hi,
> 
> Hi Andre,
> 
>> On 12/05/17 15:19, Julien Grall wrote:
>>> Hi Andre,
>>>
>>> On 11/05/17 18:53, Andre Przywara wrote:
 For LPIs the struct pending_irq's are dynamically allocated and the
 pointers will be stored in a radix tree. Since an LPI can be "unmapped"
 at any time, teach the VGIC how to deal with irq_to_pending() returning
 a NULL pointer.
 We just do nothing in this case or clean up the LR if the virtual LPI
 number was still in an LR.

 Those are all call sites for irq_to_pending(), as per:
 "git grep irq_to_pending", and their evaluations:
 (PROTECTED means: added NULL check and bailing out)

 xen/arch/arm/gic.c:
 gic_route_irq_to_guest(): only called for SPIs, added ASSERT()
 gic_remove_irq_from_guest(): only called for SPIs, added ASSERT()
 gic_remove_from_queues(): PROTECTED, called within VCPU VGIC lock
 gic_raise_inflight_irq(): PROTECTED, called under VCPU VGIC lock
 gic_raise_guest_irq(): PROTECTED, called under VCPU VGIC lock
 gic_update_one_lr(): PROTECTED, called under VCPU VGIC lock
>>>
>>> Even they are protected, an ASSERT would be useful.
>>
>> I am not sure I get what you mean here.
>> With PROTECTED I meant that the code checks for a irq_to_pending()
>> returning NULL and reacts accordingly.
>> ASSERTs are only for making sure that those functions are never called
>> for LPIs(), but the other functions can be called with an LPI, and they
>> can now cope with a NULL pending_irq.
>>
>> So what do I miss here?
> 
> I mean adding an ASSERT(spin_is_locked(vgic->vcpu)) in those functions
> if it is not done yet.

Well, that's what I meant with PROTECTED: all of them have that ASSERT
already.
So I consider this done then.

Cheers,
Andre.

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

Re: [Xen-devel] [PATCH v9 03/28] ARM: GIC: Add checks for NULL pointer pending_irq's

2017-05-22 Thread Julien Grall 



On 22/05/17 17:49, Andre Przywara wrote:

Hi,


Hi Andre,


On 12/05/17 15:19, Julien Grall wrote:

Hi Andre,

On 11/05/17 18:53, Andre Przywara wrote:

For LPIs the struct pending_irq's are dynamically allocated and the
pointers will be stored in a radix tree. Since an LPI can be "unmapped"
at any time, teach the VGIC how to deal with irq_to_pending() returning
a NULL pointer.
We just do nothing in this case or clean up the LR if the virtual LPI
number was still in an LR.

Those are all call sites for irq_to_pending(), as per:
"git grep irq_to_pending", and their evaluations:
(PROTECTED means: added NULL check and bailing out)

xen/arch/arm/gic.c:
gic_route_irq_to_guest(): only called for SPIs, added ASSERT()
gic_remove_irq_from_guest(): only called for SPIs, added ASSERT()
gic_remove_from_queues(): PROTECTED, called within VCPU VGIC lock
gic_raise_inflight_irq(): PROTECTED, called under VCPU VGIC lock
gic_raise_guest_irq(): PROTECTED, called under VCPU VGIC lock
gic_update_one_lr(): PROTECTED, called under VCPU VGIC lock


Even they are protected, an ASSERT would be useful.


I am not sure I get what you mean here.
With PROTECTED I meant that the code checks for a irq_to_pending()
returning NULL and reacts accordingly.
ASSERTs are only for making sure that those functions are never called
for LPIs(), but the other functions can be called with an LPI, and they
can now cope with a NULL pending_irq.

So what do I miss here?


I mean adding an ASSERT(spin_is_locked(vgic->vcpu)) in those functions 
if it is not done yet.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v9 03/28] ARM: GIC: Add checks for NULL pointer pending_irq's

2017-05-22 Thread Andre Przywara 
Hi,

On 12/05/17 15:19, Julien Grall wrote:
> Hi Andre,
> 
> On 11/05/17 18:53, Andre Przywara wrote:
>> For LPIs the struct pending_irq's are dynamically allocated and the
>> pointers will be stored in a radix tree. Since an LPI can be "unmapped"
>> at any time, teach the VGIC how to deal with irq_to_pending() returning
>> a NULL pointer.
>> We just do nothing in this case or clean up the LR if the virtual LPI
>> number was still in an LR.
>>
>> Those are all call sites for irq_to_pending(), as per:
>> "git grep irq_to_pending", and their evaluations:
>> (PROTECTED means: added NULL check and bailing out)
>>
>> xen/arch/arm/gic.c:
>> gic_route_irq_to_guest(): only called for SPIs, added ASSERT()
>> gic_remove_irq_from_guest(): only called for SPIs, added ASSERT()
>> gic_remove_from_queues(): PROTECTED, called within VCPU VGIC lock
>> gic_raise_inflight_irq(): PROTECTED, called under VCPU VGIC lock
>> gic_raise_guest_irq(): PROTECTED, called under VCPU VGIC lock
>> gic_update_one_lr(): PROTECTED, called under VCPU VGIC lock
> 
> Even they are protected, an ASSERT would be useful.

I am not sure I get what you mean here.
With PROTECTED I meant that the code checks for a irq_to_pending()
returning NULL and reacts accordingly.
ASSERTs are only for making sure that those functions are never called
for LPIs(), but the other functions can be called with an LPI, and they
can now cope with a NULL pending_irq.

So what do I miss here?

Cheers,
Andre.

>>
>> xen/arch/arm/vgic.c:
>> vgic_migrate_irq(): not called for LPIs (virtual IRQs), added ASSERT()
>> arch_move_irqs(): not iterating over LPIs, added ASSERT()
>> vgic_disable_irqs(): not called for LPIs, added ASSERT()
>> vgic_enable_irqs(): not called for LPIs, added ASSERT()
>> vgic_vcpu_inject_irq(): PROTECTED, moved under VCPU VGIC lock
>>
>> xen/include/asm-arm/event.h:
>> local_events_need_delivery_nomask(): only called for a PPI, added
>> ASSERT()
>>
>> xen/include/asm-arm/vgic.h:
>> (prototype)
>>

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

Re: [Xen-devel] [PATCH v9 03/28] ARM: GIC: Add checks for NULL pointer pending_irq's

2017-05-19 Thread Stefano Stabellini 
On Thu, 11 May 2017, Andre Przywara wrote:
> For LPIs the struct pending_irq's are dynamically allocated and the
> pointers will be stored in a radix tree. Since an LPI can be "unmapped"
> at any time, teach the VGIC how to deal with irq_to_pending() returning
> a NULL pointer.
> We just do nothing in this case or clean up the LR if the virtual LPI
> number was still in an LR.
> 
> Those are all call sites for irq_to_pending(), as per:
> "git grep irq_to_pending", and their evaluations:
> (PROTECTED means: added NULL check and bailing out)
> 
> xen/arch/arm/gic.c:
> gic_route_irq_to_guest(): only called for SPIs, added ASSERT()
> gic_remove_irq_from_guest(): only called for SPIs, added ASSERT()
> gic_remove_from_queues(): PROTECTED, called within VCPU VGIC lock
> gic_raise_inflight_irq(): PROTECTED, called under VCPU VGIC lock
> gic_raise_guest_irq(): PROTECTED, called under VCPU VGIC lock
> gic_update_one_lr(): PROTECTED, called under VCPU VGIC lock
> 
> xen/arch/arm/vgic.c:
> vgic_migrate_irq(): not called for LPIs (virtual IRQs), added ASSERT()
> arch_move_irqs(): not iterating over LPIs, added ASSERT()
> vgic_disable_irqs(): not called for LPIs, added ASSERT()
> vgic_enable_irqs(): not called for LPIs, added ASSERT()
> vgic_vcpu_inject_irq(): PROTECTED, moved under VCPU VGIC lock
> 
> xen/include/asm-arm/event.h:
> local_events_need_delivery_nomask(): only called for a PPI, added ASSERT()
> 
> xen/include/asm-arm/vgic.h:
> (prototype)
> 
> Signed-off-by: Andre Przywara 

Reviewed-by: Stefano Stabellini 


>  xen/arch/arm/gic.c  | 34 ++
>  xen/arch/arm/vgic.c | 24 
>  xen/include/asm-arm/event.h |  3 +++
>  3 files changed, 57 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index dcb1783..46bb306 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -148,6 +148,7 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int 
> virq,
>  /* Caller has already checked that the IRQ is an SPI */
>  ASSERT(virq >= 32);
>  ASSERT(virq < vgic_num_irqs(d));
> +ASSERT(!is_lpi(virq));
>  
>  vgic_lock_rank(v_target, rank, flags);
>  
> @@ -184,6 +185,7 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned 
> int virq,
>  ASSERT(spin_is_locked(>lock));
>  ASSERT(test_bit(_IRQ_GUEST, >status));
>  ASSERT(p->desc == desc);
> +ASSERT(!is_lpi(virq));
>  
>  vgic_lock_rank(v_target, rank, flags);
>  
> @@ -408,9 +410,13 @@ void gic_remove_from_queues(struct vcpu *v, unsigned int 
> virtual_irq)
>  spin_lock_irqsave(>arch.vgic.lock, flags);
>  
>  p = irq_to_pending(v, virtual_irq);
> -
> -if ( !list_empty(>lr_queue) )
> +/*
> + * If an LPIs has been removed meanwhile, it has been cleaned up
> + * already, so nothing to remove here.
> + */
> +if ( likely(p) && !list_empty(>lr_queue) )
>  list_del_init(>lr_queue);
> +
>  spin_unlock_irqrestore(>arch.vgic.lock, flags);
>  }
>  
> @@ -418,6 +424,10 @@ void gic_raise_inflight_irq(struct vcpu *v, unsigned int 
> virtual_irq)
>  {
>  struct pending_irq *n = irq_to_pending(v, virtual_irq);
>  
> +/* If an LPI has been removed meanwhile, there is nothing left to raise. 
> */
> +if ( unlikely(!n) )
> +return;
> +
>  ASSERT(spin_is_locked(>arch.vgic.lock));
>  
>  if ( list_empty(>lr_queue) )
> @@ -437,20 +447,25 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int 
> virtual_irq,
>  {
>  int i;
>  unsigned int nr_lrs = gic_hw_ops->info->nr_lrs;
> +struct pending_irq *p = irq_to_pending(v, virtual_irq);
>  
>  ASSERT(spin_is_locked(>arch.vgic.lock));
>  
> +if ( unlikely(!p) )
> +/* An unmapped LPI does not need to be raised. */
> +return;
> +
>  if ( v == current && list_empty(>arch.vgic.lr_pending) )
>  {
>  i = find_first_zero_bit(_cpu(lr_mask), nr_lrs);
>  if (i < nr_lrs) {
>  set_bit(i, _cpu(lr_mask));
> -gic_set_lr(i, irq_to_pending(v, virtual_irq), GICH_LR_PENDING);
> +gic_set_lr(i, p, GICH_LR_PENDING);
>  return;
>  }
>  }
>  
> -gic_add_to_lr_pending(v, irq_to_pending(v, virtual_irq));
> +gic_add_to_lr_pending(v, p);
>  }
>  
>  static void gic_update_one_lr(struct vcpu *v, int i)
> @@ -465,6 +480,17 @@ static void gic_update_one_lr(struct vcpu *v, int i)
>  gic_hw_ops->read_lr(i, _val);
>  irq = lr_val.virq;
>  p = irq_to_pending(v, irq);
> +/* An LPI might have been unmapped, in which case we just clean up here. 
> */
> +if ( unlikely(!p) )
> +{
> +ASSERT(is_lpi(irq));
> +
> +gic_hw_ops->clear_lr(i);
> +clear_bit(i, _cpu(lr_mask));
> +
> +return;
> +}
> +
>  if ( lr_val.state & GICH_LR_ACTIVE )
>  {
>  set_bit(GIC_IRQ_GUEST_ACTIVE, >status);
> diff --git

Re: [Xen-devel] [PATCH v9 03/28] ARM: GIC: Add checks for NULL pointer pending_irq's

2017-05-12 Thread Julien Grall 

Hi Andre,

On 11/05/17 18:53, Andre Przywara wrote:

For LPIs the struct pending_irq's are dynamically allocated and the
pointers will be stored in a radix tree. Since an LPI can be "unmapped"
at any time, teach the VGIC how to deal with irq_to_pending() returning
a NULL pointer.
We just do nothing in this case or clean up the LR if the virtual LPI
number was still in an LR.

Those are all call sites for irq_to_pending(), as per:
"git grep irq_to_pending", and their evaluations:
(PROTECTED means: added NULL check and bailing out)

xen/arch/arm/gic.c:
gic_route_irq_to_guest(): only called for SPIs, added ASSERT()
gic_remove_irq_from_guest(): only called for SPIs, added ASSERT()
gic_remove_from_queues(): PROTECTED, called within VCPU VGIC lock
gic_raise_inflight_irq(): PROTECTED, called under VCPU VGIC lock
gic_raise_guest_irq(): PROTECTED, called under VCPU VGIC lock
gic_update_one_lr(): PROTECTED, called under VCPU VGIC lock


Even they are protected, an ASSERT would be useful.



xen/arch/arm/vgic.c:
vgic_migrate_irq(): not called for LPIs (virtual IRQs), added ASSERT()
arch_move_irqs(): not iterating over LPIs, added ASSERT()
vgic_disable_irqs(): not called for LPIs, added ASSERT()
vgic_enable_irqs(): not called for LPIs, added ASSERT()
vgic_vcpu_inject_irq(): PROTECTED, moved under VCPU VGIC lock

xen/include/asm-arm/event.h:
local_events_need_delivery_nomask(): only called for a PPI, added ASSERT()

xen/include/asm-arm/vgic.h:
(prototype)

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/gic.c  | 34 ++
 xen/arch/arm/vgic.c | 24 
 xen/include/asm-arm/event.h |  3 +++
 3 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index dcb1783..46bb306 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -148,6 +148,7 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int 
virq,
 /* Caller has already checked that the IRQ is an SPI */
 ASSERT(virq >= 32);
 ASSERT(virq < vgic_num_irqs(d));
+ASSERT(!is_lpi(virq));

 vgic_lock_rank(v_target, rank, flags);

@@ -184,6 +185,7 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned 
int virq,
 ASSERT(spin_is_locked(>lock));
 ASSERT(test_bit(_IRQ_GUEST, >status));
 ASSERT(p->desc == desc);
+ASSERT(!is_lpi(virq));

 vgic_lock_rank(v_target, rank, flags);

@@ -408,9 +410,13 @@ void gic_remove_from_queues(struct vcpu *v, unsigned int 
virtual_irq)
 spin_lock_irqsave(>arch.vgic.lock, flags);

 p = irq_to_pending(v, virtual_irq);
-
-if ( !list_empty(>lr_queue) )
+/*
+ * If an LPIs has been removed meanwhile, it has been cleaned up
+ * already, so nothing to remove here.
+ */
+if ( likely(p) && !list_empty(>lr_queue) )
 list_del_init(>lr_queue);
+
 spin_unlock_irqrestore(>arch.vgic.lock, flags);
 }

@@ -418,6 +424,10 @@ void gic_raise_inflight_irq(struct vcpu *v, unsigned int 
virtual_irq)
 {
 struct pending_irq *n = irq_to_pending(v, virtual_irq);

+/* If an LPI has been removed meanwhile, there is nothing left to raise. */
+if ( unlikely(!n) )
+return;
+
 ASSERT(spin_is_locked(>arch.vgic.lock));

 if ( list_empty(>lr_queue) )
@@ -437,20 +447,25 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int 
virtual_irq,
 {
 int i;
 unsigned int nr_lrs = gic_hw_ops->info->nr_lrs;
+struct pending_irq *p = irq_to_pending(v, virtual_irq);

 ASSERT(spin_is_locked(>arch.vgic.lock));

+if ( unlikely(!p) )
+/* An unmapped LPI does not need to be raised. */
+return;
+
 if ( v == current && list_empty(>arch.vgic.lr_pending) )
 {
 i = find_first_zero_bit(_cpu(lr_mask), nr_lrs);
 if (i < nr_lrs) {
 set_bit(i, _cpu(lr_mask));
-gic_set_lr(i, irq_to_pending(v, virtual_irq), GICH_LR_PENDING);
+gic_set_lr(i, p, GICH_LR_PENDING);
 return;
 }
 }

-gic_add_to_lr_pending(v, irq_to_pending(v, virtual_irq));
+gic_add_to_lr_pending(v, p);
 }

 static void gic_update_one_lr(struct vcpu *v, int i)
@@ -465,6 +480,17 @@ static void gic_update_one_lr(struct vcpu *v, int i)
 gic_hw_ops->read_lr(i, _val);
 irq = lr_val.virq;
 p = irq_to_pending(v, irq);
+/* An LPI might have been unmapped, in which case we just clean up here. */
+if ( unlikely(!p) )
+{
+ASSERT(is_lpi(irq));
+
+gic_hw_ops->clear_lr(i);
+clear_bit(i, _cpu(lr_mask));
+
+return;
+}
+
 if ( lr_val.state & GICH_LR_ACTIVE )
 {
 set_bit(GIC_IRQ_GUEST_ACTIVE, >status);
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index d30f324..8a5d93b 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -242,6 +242,9 @@ bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, 
unsigned int irq)
 unsigned long flags;
 struct pending_irq *p =

[Xen-devel] [PATCH v9 03/28] ARM: GIC: Add checks for NULL pointer pending_irq's

2017-05-11 Thread Andre Przywara 
For LPIs the struct pending_irq's are dynamically allocated and the
pointers will be stored in a radix tree. Since an LPI can be "unmapped"
at any time, teach the VGIC how to deal with irq_to_pending() returning
a NULL pointer.
We just do nothing in this case or clean up the LR if the virtual LPI
number was still in an LR.

Those are all call sites for irq_to_pending(), as per:
"git grep irq_to_pending", and their evaluations:
(PROTECTED means: added NULL check and bailing out)

xen/arch/arm/gic.c:
gic_route_irq_to_guest(): only called for SPIs, added ASSERT()
gic_remove_irq_from_guest(): only called for SPIs, added ASSERT()
gic_remove_from_queues(): PROTECTED, called within VCPU VGIC lock
gic_raise_inflight_irq(): PROTECTED, called under VCPU VGIC lock
gic_raise_guest_irq(): PROTECTED, called under VCPU VGIC lock
gic_update_one_lr(): PROTECTED, called under VCPU VGIC lock

xen/arch/arm/vgic.c:
vgic_migrate_irq(): not called for LPIs (virtual IRQs), added ASSERT()
arch_move_irqs(): not iterating over LPIs, added ASSERT()
vgic_disable_irqs(): not called for LPIs, added ASSERT()
vgic_enable_irqs(): not called for LPIs, added ASSERT()
vgic_vcpu_inject_irq(): PROTECTED, moved under VCPU VGIC lock

xen/include/asm-arm/event.h:
local_events_need_delivery_nomask(): only called for a PPI, added ASSERT()

xen/include/asm-arm/vgic.h:
(prototype)

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/gic.c  | 34 ++
 xen/arch/arm/vgic.c | 24 
 xen/include/asm-arm/event.h |  3 +++
 3 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index dcb1783..46bb306 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -148,6 +148,7 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int 
virq,
 /* Caller has already checked that the IRQ is an SPI */
 ASSERT(virq >= 32);
 ASSERT(virq < vgic_num_irqs(d));
+ASSERT(!is_lpi(virq));
 
 vgic_lock_rank(v_target, rank, flags);
 
@@ -184,6 +185,7 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned 
int virq,
 ASSERT(spin_is_locked(>lock));
 ASSERT(test_bit(_IRQ_GUEST, >status));
 ASSERT(p->desc == desc);
+ASSERT(!is_lpi(virq));
 
 vgic_lock_rank(v_target, rank, flags);
 
@@ -408,9 +410,13 @@ void gic_remove_from_queues(struct vcpu *v, unsigned int 
virtual_irq)
 spin_lock_irqsave(>arch.vgic.lock, flags);
 
 p = irq_to_pending(v, virtual_irq);
-
-if ( !list_empty(>lr_queue) )
+/*
+ * If an LPIs has been removed meanwhile, it has been cleaned up
+ * already, so nothing to remove here.
+ */
+if ( likely(p) && !list_empty(>lr_queue) )
 list_del_init(>lr_queue);
+
 spin_unlock_irqrestore(>arch.vgic.lock, flags);
 }
 
@@ -418,6 +424,10 @@ void gic_raise_inflight_irq(struct vcpu *v, unsigned int 
virtual_irq)
 {
 struct pending_irq *n = irq_to_pending(v, virtual_irq);
 
+/* If an LPI has been removed meanwhile, there is nothing left to raise. */
+if ( unlikely(!n) )
+return;
+
 ASSERT(spin_is_locked(>arch.vgic.lock));
 
 if ( list_empty(>lr_queue) )
@@ -437,20 +447,25 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int 
virtual_irq,
 {
 int i;
 unsigned int nr_lrs = gic_hw_ops->info->nr_lrs;
+struct pending_irq *p = irq_to_pending(v, virtual_irq);
 
 ASSERT(spin_is_locked(>arch.vgic.lock));
 
+if ( unlikely(!p) )
+/* An unmapped LPI does not need to be raised. */
+return;
+
 if ( v == current && list_empty(>arch.vgic.lr_pending) )
 {
 i = find_first_zero_bit(_cpu(lr_mask), nr_lrs);
 if (i < nr_lrs) {
 set_bit(i, _cpu(lr_mask));
-gic_set_lr(i, irq_to_pending(v, virtual_irq), GICH_LR_PENDING);
+gic_set_lr(i, p, GICH_LR_PENDING);
 return;
 }
 }
 
-gic_add_to_lr_pending(v, irq_to_pending(v, virtual_irq));
+gic_add_to_lr_pending(v, p);
 }
 
 static void gic_update_one_lr(struct vcpu *v, int i)
@@ -465,6 +480,17 @@ static void gic_update_one_lr(struct vcpu *v, int i)
 gic_hw_ops->read_lr(i, _val);
 irq = lr_val.virq;
 p = irq_to_pending(v, irq);
+/* An LPI might have been unmapped, in which case we just clean up here. */
+if ( unlikely(!p) )
+{
+ASSERT(is_lpi(irq));
+
+gic_hw_ops->clear_lr(i);
+clear_bit(i, _cpu(lr_mask));
+
+return;
+}
+
 if ( lr_val.state & GICH_LR_ACTIVE )
 {
 set_bit(GIC_IRQ_GUEST_ACTIVE, >status);
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index d30f324..8a5d93b 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -242,6 +242,9 @@ bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, 
unsigned int irq)
 unsigned long flags;
 struct pending_irq *p = irq_to_pending(old, irq);
 
+/* This will never be called for an LPI, as we don't migrate them. */
+