[Xen-devel,v2,04/10] ARM: VGIC: streamline gic_restore_pending_irqs()

Message ID 20171207161415.20380-5-andre.przywara@linaro.org
State New
Headers show
Series
  • ARM: VGIC/GIC separation cleanups
Related show

Commit Message

Andre Przywara Dec. 7, 2017, 4:14 p.m.
In gic_restore_pending_irqs() we push our pending virtual IRQs into the
list registers. This function is called once from a GIC context and once
from a VGIC context. Refactor the calls so that we have only one callsite
from the VGIC context. This will help separating the two worlds later.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 xen/arch/arm/domain.c     |  1 +
 xen/arch/arm/gic.c        | 11 +++++------
 xen/arch/arm/traps.c      |  2 +-
 xen/include/asm-arm/gic.h |  2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

Comments

Stefano Stabellini Dec. 8, 2017, 9:40 p.m. | #1
On Thu, 7 Dec 2017, Andre Przywara wrote:
> In gic_restore_pending_irqs() we push our pending virtual IRQs into the
> list registers. This function is called once from a GIC context and once
> from a VGIC context. Refactor the calls so that we have only one callsite
> from the VGIC context. This will help separating the two worlds later.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  xen/arch/arm/domain.c     |  1 +
>  xen/arch/arm/gic.c        | 11 +++++------
>  xen/arch/arm/traps.c      |  2 +-
>  xen/include/asm-arm/gic.h |  2 +-
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index a74ff1c07c..73f4d4b2b2 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -185,6 +185,7 @@ static void ctxt_switch_to(struct vcpu *n)
>  
>      /* VGIC */
>      gic_restore_state(n);
> +    gic_inject(n);
>  
>      /* VFP */
>      vfp_restore_state(n);
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index bac8ada2bb..1f00654ef5 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -36,8 +36,6 @@
>  #include <asm/vgic.h>
>  #include <asm/acpi.h>
>  
> -static void gic_restore_pending_irqs(struct vcpu *v);
> -
>  static DEFINE_PER_CPU(uint64_t, lr_mask);
>  
>  #define lr_all_full() (this_cpu(lr_mask) == ((1 << gic_hw_ops->info->nr_lrs) - 1))
> @@ -91,8 +89,6 @@ void gic_restore_state(struct vcpu *v)
>      gic_hw_ops->restore_state(v);
>  
>      isb();
> -
> -    gic_restore_pending_irqs(v);
>  }
>  
>  /* desc->irq needs to be disabled before calling this function */
> @@ -715,11 +711,14 @@ out:
>      return rc;
>  }
>  
> -void gic_inject(void)
> +void gic_inject(struct vcpu *v)
>  {
>      ASSERT(!local_irq_is_enabled());
>  
> -    gic_restore_pending_irqs(current);
> +    gic_restore_pending_irqs(v);

This looks suspicious to me: gic_restore_pending_irqs calls gic_set_lr.
It doesn't look like gic_restore_pending_irqs can actually be called for
v != current.

However, I think that we could remove the call to
gic_restore_pending_irqs from gic_restore_state safely, because we can
still rely on gic_inject being called before entering the guest.

There is no need to add a call to gic_inject in ctxt_switch_to, I think.


> +    if ( v != current )
> +        return;
>  
>      if ( !list_empty(&current->arch.vgic.lr_pending) && lr_all_full() )
>          gic_hw_ops->update_hcr_status(GICH_HCR_UIE, true);
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index ff3d6ff2aa..7fd676ed9d 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2298,7 +2298,7 @@ void leave_hypervisor_tail(void)
>      {
>          local_irq_disable();
>          if (!softirq_pending(smp_processor_id())) {
> -            gic_inject();
> +            gic_inject(current);
>  
>              /*
>               * If the SErrors handle option is "DIVERSE", we have to prevent
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 587a14f8b9..28cf16654a 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -235,7 +235,7 @@ extern int gic_route_irq_to_guest(struct domain *, unsigned int virq,
>  int gic_remove_irq_from_guest(struct domain *d, unsigned int virq,
>                                struct irq_desc *desc);
>  
> -extern void gic_inject(void);
> +extern void gic_inject(struct vcpu *v);
>  extern void gic_clear_pending_irqs(struct vcpu *v);
>  extern int gic_events_need_delivery(void);

Patch

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index a74ff1c07c..73f4d4b2b2 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -185,6 +185,7 @@  static void ctxt_switch_to(struct vcpu *n)
 
     /* VGIC */
     gic_restore_state(n);
+    gic_inject(n);
 
     /* VFP */
     vfp_restore_state(n);
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index bac8ada2bb..1f00654ef5 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -36,8 +36,6 @@ 
 #include <asm/vgic.h>
 #include <asm/acpi.h>
 
-static void gic_restore_pending_irqs(struct vcpu *v);
-
 static DEFINE_PER_CPU(uint64_t, lr_mask);
 
 #define lr_all_full() (this_cpu(lr_mask) == ((1 << gic_hw_ops->info->nr_lrs) - 1))
@@ -91,8 +89,6 @@  void gic_restore_state(struct vcpu *v)
     gic_hw_ops->restore_state(v);
 
     isb();
-
-    gic_restore_pending_irqs(v);
 }
 
 /* desc->irq needs to be disabled before calling this function */
@@ -715,11 +711,14 @@  out:
     return rc;
 }
 
-void gic_inject(void)
+void gic_inject(struct vcpu *v)
 {
     ASSERT(!local_irq_is_enabled());
 
-    gic_restore_pending_irqs(current);
+    gic_restore_pending_irqs(v);
+
+    if ( v != current )
+        return;
 
     if ( !list_empty(&current->arch.vgic.lr_pending) && lr_all_full() )
         gic_hw_ops->update_hcr_status(GICH_HCR_UIE, true);
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index ff3d6ff2aa..7fd676ed9d 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2298,7 +2298,7 @@  void leave_hypervisor_tail(void)
     {
         local_irq_disable();
         if (!softirq_pending(smp_processor_id())) {
-            gic_inject();
+            gic_inject(current);
 
             /*
              * If the SErrors handle option is "DIVERSE", we have to prevent
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 587a14f8b9..28cf16654a 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -235,7 +235,7 @@  extern int gic_route_irq_to_guest(struct domain *, unsigned int virq,
 int gic_remove_irq_from_guest(struct domain *d, unsigned int virq,
                               struct irq_desc *desc);
 
-extern void gic_inject(void);
+extern void gic_inject(struct vcpu *v);
 extern void gic_clear_pending_irqs(struct vcpu *v);
 extern int gic_events_need_delivery(void);