[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 39/41] arm : acpi configure interrupts dynamically



Hi Parth,

On 17/05/2015 21:04, Parth Dixit wrote:
Interrupt information is described in DSDT and is not available at
the time of booting. Configure the interrupts dynamically when requested
by Dom0

Missing "."

Also, I'm sure we talked about it multiple time. I'd like to keep the ACPI changes very contained to Xen boot. Your change is not ACPI specific and could be used for DT.


Signed-off-by: Parth Dixit <parth.dixit@xxxxxxxxxx>
---
  xen/arch/arm/vgic.c | 18 ++++++++++++++++++
  1 file changed, 18 insertions(+)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 73a6f7e..f63deb4 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -25,6 +25,7 @@
  #include <xen/irq.h>
  #include <xen/sched.h>
  #include <xen/perfc.h>
+#include <xen/acpi.h>

  #include <asm/current.h>

@@ -285,6 +286,8 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
      }
  }

+#define VGIC_ICFG_MASK(intr) ( 1 << ( ( 2 * ( intr % 16 ) ) + 1 ) )
+
  void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
  {
      struct domain *d = v->domain;
@@ -296,7 +299,22 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
      struct vcpu *v_target;

      while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
+#ifdef CONFIG_ACPI
+        struct vgic_irq_rank *vr = vgic_get_rank(v, n);
+        uint32_t tr;

New line.

+        irq = i + (32 * n);
+        if( ( !acpi_disabled ) && ( n != 0 ) && is_hardware_domain(d) )

You need to add a comment explaining the ( n != 0 ) i.e we don't SGIs and PPIs are RO. It's implementation defined for PPI but it's preferable to let Xen take care of it.

Also, we should set the type only for IRQ assigned to DOM0 (i.e p->desc != NULL). With your current solution, DOM0 may change the configuration of the serial IRQ by mistake and take down Xen because the physical IRQ is enabled and the behavior will be unpredictable.

Furthermore, during passthrough, the IRQ may not have been configured by DOM0. So we have to let the guest configure the IRQ.

+        {
+            tr = vr->icfg[i >> 4] ;
+
+            if( ( tr & VGIC_ICFG_MASK(i) ) )
+                set_irq_type(irq, ACPI_IRQ_TYPE_EDGE_BOTH);
+            else
+                set_irq_type(irq, ACPI_IRQ_TYPE_LEVEL_MASK);

Given that only SPI can be configured it would have been better to call irq_set_type.

Although, those 2 functions don't do what you think. They are setting the type internally in Xen but don't change the GIC interrupt configuration register.

Lastly, they may fail because the configuration as been set earlier (as you did in patch #41.

Regards,

--
Julien Grall

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.