[Xen-devel,v2,01/45] ARM: VGIC: rename gic_event_needs_delivery()

Message ID 20180315203050.19791-2-andre.przywara@linaro.org
State New
Headers show
Series
  • New VGIC(-v2) implementation
Related show

Commit Message

Andre Przywara March 15, 2018, 8:30 p.m.
gic_event_needs_delivery() is not named very intuitively, especially
the gic_ prefix is somewhat misleading.
Rename it to vgic_vcpu_pending_irq(), which makes it clear that this
relates to the virtual GIC and is about interrupts.
Also add a VCPU parameter, which makes the code more flexible in the
future. The current VGIC expect this to be the current VCPU, so add
an assert to spot any regressions.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
Changelog v1 .. v2:
- rename to vgic_vcpu_pending_irq()
- add VCPU parameter                                                            

 xen/arch/arm/gic-vgic.c     | 16 ++++++++++++++--
 xen/include/asm-arm/event.h |  2 +-
 xen/include/asm-arm/gic.h   |  2 +-
 3 files changed, 16 insertions(+), 4 deletions(-)

Comments

Julien Grall March 16, 2018, 10:58 a.m. | #1
Hi Andre,

On 15/03/18 20:30, Andre Przywara wrote:
> gic_event_needs_delivery() is not named very intuitively, especially
> the gic_ prefix is somewhat misleading.
> Rename it to vgic_vcpu_pending_irq(), which makes it clear that this
> relates to the virtual GIC and is about interrupts.
> Also add a VCPU parameter, which makes the code more flexible in the
> future. The current VGIC expect this to be the current VCPU, so add
> an assert to spot any regressions.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,

> ---
> Changelog v1 .. v2:
> - rename to vgic_vcpu_pending_irq()
> - add VCPU parameter
> 
>   xen/arch/arm/gic-vgic.c     | 16 ++++++++++++++--
>   xen/include/asm-arm/event.h |  2 +-
>   xen/include/asm-arm/gic.h   |  2 +-
>   3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
> index ecb07ceb40..61f093db50 100644
> --- a/xen/arch/arm/gic-vgic.c
> +++ b/xen/arch/arm/gic-vgic.c
> @@ -339,9 +339,18 @@ void gic_clear_pending_irqs(struct vcpu *v)
>           gic_remove_from_lr_pending(v, p);
>   }
>   
> -int gic_events_need_delivery(void)
> +/**
> + * vgic_vcpu_pending_irq() - determine if interrupts need to be injected
> + * @vcpu: The vCPU on which to check for interrupts.
> + *
> + * Checks whether there is an interrupt on the given VCPU which needs
> + * handling in the guest. This requires at least one IRQ to be pending
> + * and enabled.
> + *
> + * Returns: 1 if the guest should run to handle interrupts, 0 otherwise.
> + */
> +int vgic_vcpu_pending_irq(struct vcpu *v)
>   {
> -    struct vcpu *v = current;
>       struct pending_irq *p;
>       unsigned long flags;
>       const unsigned long apr = gic_hw_ops->read_apr(0);
> @@ -349,6 +358,9 @@ int gic_events_need_delivery(void)
>       int active_priority;
>       int rc = 0;
>   
> +    /* We rely on reading the VMCR, which is only accessible locally. */
> +    ASSERT(v == current);
> +
>       mask_priority = gic_hw_ops->read_vmcr_priority();
>       active_priority = find_next_bit(&apr, 32, 0);
>   
> diff --git a/xen/include/asm-arm/event.h b/xen/include/asm-arm/event.h
> index e8c2a6cb44..c7a415ef57 100644
> --- a/xen/include/asm-arm/event.h
> +++ b/xen/include/asm-arm/event.h
> @@ -24,7 +24,7 @@ static inline int local_events_need_delivery_nomask(void)
>        * interrupts disabled so this shouldn't be a problem in the general
>        * case.
>        */
> -    if ( gic_events_need_delivery() )
> +    if ( vgic_vcpu_pending_irq(current) )
>           return 1;
>   
>       if ( !vcpu_info(current, evtchn_upcall_pending) )
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index d568957dd1..49cb94f792 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -238,7 +238,7 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned int virq,
>   
>   extern void vgic_sync_to_lrs(void);
>   extern void gic_clear_pending_irqs(struct vcpu *v);
> -extern int gic_events_need_delivery(void);
> +extern int vgic_vcpu_pending_irq(struct vcpu *v);
>   
>   extern void init_maintenance_interrupt(void);
>   extern void gic_raise_guest_irq(struct vcpu *v, unsigned int irq,
>
Stefano Stabellini March 16, 2018, 9:21 p.m. | #2
On Thu, 15 Mar 2018, Andre Przywara wrote:
> gic_event_needs_delivery() is not named very intuitively, especially
> the gic_ prefix is somewhat misleading.
> Rename it to vgic_vcpu_pending_irq(), which makes it clear that this
> relates to the virtual GIC and is about interrupts.
> Also add a VCPU parameter, which makes the code more flexible in the
> future. The current VGIC expect this to be the current VCPU, so add
> an assert to spot any regressions.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changelog v1 .. v2:
> - rename to vgic_vcpu_pending_irq()
> - add VCPU parameter                                                            
> 
>  xen/arch/arm/gic-vgic.c     | 16 ++++++++++++++--
>  xen/include/asm-arm/event.h |  2 +-
>  xen/include/asm-arm/gic.h   |  2 +-
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
> index ecb07ceb40..61f093db50 100644
> --- a/xen/arch/arm/gic-vgic.c
> +++ b/xen/arch/arm/gic-vgic.c
> @@ -339,9 +339,18 @@ void gic_clear_pending_irqs(struct vcpu *v)
>          gic_remove_from_lr_pending(v, p);
>  }
>  
> -int gic_events_need_delivery(void)
> +/**
> + * vgic_vcpu_pending_irq() - determine if interrupts need to be injected
> + * @vcpu: The vCPU on which to check for interrupts.
> + *
> + * Checks whether there is an interrupt on the given VCPU which needs
> + * handling in the guest. This requires at least one IRQ to be pending
> + * and enabled.
> + *
> + * Returns: 1 if the guest should run to handle interrupts, 0 otherwise.
> + */
> +int vgic_vcpu_pending_irq(struct vcpu *v)
>  {
> -    struct vcpu *v = current;
>      struct pending_irq *p;
>      unsigned long flags;
>      const unsigned long apr = gic_hw_ops->read_apr(0);
> @@ -349,6 +358,9 @@ int gic_events_need_delivery(void)
>      int active_priority;
>      int rc = 0;
>  
> +    /* We rely on reading the VMCR, which is only accessible locally. */
> +    ASSERT(v == current);
> +
>      mask_priority = gic_hw_ops->read_vmcr_priority();
>      active_priority = find_next_bit(&apr, 32, 0);
>  
> diff --git a/xen/include/asm-arm/event.h b/xen/include/asm-arm/event.h
> index e8c2a6cb44..c7a415ef57 100644
> --- a/xen/include/asm-arm/event.h
> +++ b/xen/include/asm-arm/event.h
> @@ -24,7 +24,7 @@ static inline int local_events_need_delivery_nomask(void)
>       * interrupts disabled so this shouldn't be a problem in the general
>       * case.
>       */
> -    if ( gic_events_need_delivery() )
> +    if ( vgic_vcpu_pending_irq(current) )
>          return 1;
>  
>      if ( !vcpu_info(current, evtchn_upcall_pending) )
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index d568957dd1..49cb94f792 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -238,7 +238,7 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned int virq,
>  
>  extern void vgic_sync_to_lrs(void);
>  extern void gic_clear_pending_irqs(struct vcpu *v);
> -extern int gic_events_need_delivery(void);
> +extern int vgic_vcpu_pending_irq(struct vcpu *v);
>  
>  extern void init_maintenance_interrupt(void);
>  extern void gic_raise_guest_irq(struct vcpu *v, unsigned int irq,
> -- 
> 2.14.1
>

Patch

diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
index ecb07ceb40..61f093db50 100644
--- a/xen/arch/arm/gic-vgic.c
+++ b/xen/arch/arm/gic-vgic.c
@@ -339,9 +339,18 @@  void gic_clear_pending_irqs(struct vcpu *v)
         gic_remove_from_lr_pending(v, p);
 }
 
-int gic_events_need_delivery(void)
+/**
+ * vgic_vcpu_pending_irq() - determine if interrupts need to be injected
+ * @vcpu: The vCPU on which to check for interrupts.
+ *
+ * Checks whether there is an interrupt on the given VCPU which needs
+ * handling in the guest. This requires at least one IRQ to be pending
+ * and enabled.
+ *
+ * Returns: 1 if the guest should run to handle interrupts, 0 otherwise.
+ */
+int vgic_vcpu_pending_irq(struct vcpu *v)
 {
-    struct vcpu *v = current;
     struct pending_irq *p;
     unsigned long flags;
     const unsigned long apr = gic_hw_ops->read_apr(0);
@@ -349,6 +358,9 @@  int gic_events_need_delivery(void)
     int active_priority;
     int rc = 0;
 
+    /* We rely on reading the VMCR, which is only accessible locally. */
+    ASSERT(v == current);
+
     mask_priority = gic_hw_ops->read_vmcr_priority();
     active_priority = find_next_bit(&apr, 32, 0);
 
diff --git a/xen/include/asm-arm/event.h b/xen/include/asm-arm/event.h
index e8c2a6cb44..c7a415ef57 100644
--- a/xen/include/asm-arm/event.h
+++ b/xen/include/asm-arm/event.h
@@ -24,7 +24,7 @@  static inline int local_events_need_delivery_nomask(void)
      * interrupts disabled so this shouldn't be a problem in the general
      * case.
      */
-    if ( gic_events_need_delivery() )
+    if ( vgic_vcpu_pending_irq(current) )
         return 1;
 
     if ( !vcpu_info(current, evtchn_upcall_pending) )
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index d568957dd1..49cb94f792 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -238,7 +238,7 @@  int gic_remove_irq_from_guest(struct domain *d, unsigned int virq,
 
 extern void vgic_sync_to_lrs(void);
 extern void gic_clear_pending_irqs(struct vcpu *v);
-extern int gic_events_need_delivery(void);
+extern int vgic_vcpu_pending_irq(struct vcpu *v);
 
 extern void init_maintenance_interrupt(void);
 extern void gic_raise_guest_irq(struct vcpu *v, unsigned int irq,