[PATCH v2 0/9] xen/arm: Support SPIs routing

[PATCH v2 0/9] xen/arm: Support SPIs routing

[Julien Grall]

Hello all,

Currently, Xen does not route SPIs to DOM0 when ACPI is inuse after
the functionality has been reverted in Xen 4.7 by commit 909bd14.

In the previous approach, the SPIs was routed when DOM0 was writing into
ISENABLER. However, this has resulted to deadlock (see more details in [1])
as soon as the IRQ was enabled by DOM0.

We have multiple solutions to route the IRQ:
    1) Rework route_irq_to_guest to avoid the deadlock
    2) Route and enable the IRQ outside of the emulation of ISENABLER
    3) Remove the dependency on the IRQ type in the routing function
    and route all the unused IRQs during domain building
    4) Add a new hypercall to let DOM0 routing the IRQ

I think that 1) and 2) are not resilient because route_irq_to_guest may fail
and there is no way to report this error to the guest (except by killing it).

Furthermore, in solution 2) enabling the interrupt would need to be defer
until the routing has been done. This would require a lot of code duplication.

Which leave solution 3) and 4). The solution 4) requires to introduce a new
(or re-use one) stable hypercall. I am not sure why we ruled out this
solution when we reviewed the ACPI design document.

This patch series is implementing the 3rd solution which defer the IRQ
type configuration for DOM0 IRQ when ACPI is inuse. However, this will
slightly increase the memory usage of Xen (54KB).

I am happy to consider any other solutions.

I only tested briefly this patch series, Shanker can you give a try on
your hardware?

A branch with all the patches can be found here:

git://xenbits.xen.org/people/julieng/xen-unstable.git branch irq-routing-acpi-v2

Yours sincerely,

[1] http://lists.xenproject.org/archives/html/xen-devel/2016-05/msg02633.html

Julien Grall (9):
  xen/arm: gic: Consolidate the IRQ affinity set in a single place
  xen/arm: gic: Do not configure affinity during routing
  xen/arm: gic: split set_irq_properties
  xen/arm: gic: set_type: Pass the type in parameter rather than in
    desc->arch.type
  xen/arm: gic: Document how gic_set_irq_type should be called
  Revert "xen/arm: warn the user that we cannot route SPIs to Dom0 on
    ACPI"
  xen/arm: Allow DOM0 to set the IRQ type
  xen/arm: acpi: route all unused IRQs to DOM0
  xen/arm: Fix coding style and update comment in acpi_route_spis

 xen/arch/arm/domain_build.c | 33 +++++++++++++++------------------
 xen/arch/arm/gic-v2.c       | 28 +++++++++++++---------------
 xen/arch/arm/gic-v3.c       | 22 ++++++++++------------
 xen/arch/arm/gic.c          | 36 ++++++++++++++++++++++--------------
 xen/arch/arm/irq.c          | 17 ++++++++++++++---
 xen/arch/arm/vgic.c         | 32 ++++++++++++++++++--------------
 xen/include/asm-arm/gic.h   | 14 ++++++++------
 xen/include/asm-arm/irq.h   |  6 ++++++
 8 files changed, 106 insertions(+), 82 deletions(-)

-- 
1.9.1


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

[PATCH v2 1/9] xen/arm: gic: Consolidate the IRQ affinity set in a single place

[Julien Grall]

The code to set the IRQ affinity is duplicated: once in
gicv{2,3}_set_properties and the other is gicv{2,3}_irq_set_affinity.

Remove the code from gicv{2,3}_set_properties and call directly the
affinity set helper from the common code.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v2:
        - Add Stefano's reviewed-by
---
 xen/arch/arm/gic-v2.c     | 10 +---------
 xen/arch/arm/gic-v3.c     | 10 ----------
 xen/arch/arm/gic.c        |  3 ++-
 xen/include/asm-arm/gic.h |  1 -
 4 files changed, 3 insertions(+), 21 deletions(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 3893ece..6c7dbfe 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -236,16 +236,10 @@ static unsigned int gicv2_read_irq(void)
     return (readl_gicc(GICC_IAR) & GICC_IA_IRQ);
 }
 
-/*
- * needs to be called with a valid cpu_mask, ie each cpu in the mask has
- * already called gic_cpu_init
- */
 static void gicv2_set_irq_properties(struct irq_desc *desc,
-                                   const cpumask_t *cpu_mask,
-                                   unsigned int priority)
+                                     unsigned int priority)
 {
     uint32_t cfg, actual, edgebit;
-    unsigned int mask = gicv2_cpu_mask(cpu_mask);
     unsigned int irq = desc->irq;
     unsigned int type = desc->arch.type;
 
@@ -276,8 +270,6 @@ static void gicv2_set_irq_properties(struct irq_desc *desc,
             IRQ_TYPE_LEVEL_HIGH;
     }
 
-    /* Set target CPU mask (RAZ/WI on uniprocessor) */
-    writeb_gicd(mask, GICD_ITARGETSR + irq);
     /* Set priority */
     writeb_gicd(priority, GICD_IPRIORITYR + irq);
 
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index cbda066..d6ab0e9 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -472,13 +472,10 @@ static inline uint64_t gicv3_mpidr_to_affinity(int cpu)
 }
 
 static void gicv3_set_irq_properties(struct irq_desc *desc,
-                                     const cpumask_t *cpu_mask,
                                      unsigned int priority)
 {
     uint32_t cfg, actual, edgebit;
-    uint64_t affinity;
     void __iomem *base;
-    unsigned int cpu = gicv3_get_cpu_from_mask(cpu_mask);
     unsigned int irq = desc->irq;
     unsigned int type = desc->arch.type;
 
@@ -516,13 +513,6 @@ static void gicv3_set_irq_properties(struct irq_desc *desc,
             IRQ_TYPE_LEVEL_HIGH;
     }
 
-    affinity = gicv3_mpidr_to_affinity(cpu);
-    /* Make sure we don't broadcast the interrupt */
-    affinity &= ~GICD_IROUTER_SPI_MODE_ANY;
-
-    if ( irq >= NR_GIC_LOCAL_IRQS )
-        writeq_relaxed(affinity, (GICD + GICD_IROUTER + irq * 8));
-
     /* Set priority */
     if ( irq < NR_GIC_LOCAL_IRQS )
         writeb_relaxed(priority, GICD_RDIST_SGI_BASE + GICR_IPRIORITYR0 + irq);
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 12bb0ab..5726a05 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -106,7 +106,8 @@ static void gic_set_irq_properties(struct irq_desc *desc,
                                    const cpumask_t *cpu_mask,
                                    unsigned int priority)
 {
-   gic_hw_ops->set_irq_properties(desc, cpu_mask, priority);
+    gic_hw_ops->set_irq_properties(desc, priority);
+    desc->handler->set_affinity(desc, cpu_mask);
 }
 
 /* Program the GIC to route an interrupt to the host (i.e. Xen)
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index b073c53..2fc6126 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -331,7 +331,6 @@ struct gic_hw_operations {
     unsigned int (*read_irq)(void);
     /* Set IRQ property */
     void (*set_irq_properties)(struct irq_desc *desc,
-                               const cpumask_t *cpu_mask,
                                unsigned int priority);
     /* Send SGI */
     void (*send_SGI)(enum gic_sgi sgi, enum gic_sgi_mode irqmode,
-- 
1.9.1


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

[PATCH v2 2/9] xen/arm: gic: Do not configure affinity during routing

[Julien Grall]

The affinity of a guest IRQ is set every time the guest enable it (see
vgic_enable_irqs).

It is not necessary to set the affinity when the IRQ is routed to the
guest because Xen will never receive the IRQ until it hass been enabled
by the guest.

To keep gic_route_irq_to_{xen,guest} behaving the same way (i.e just
setting up the routing), the affinity of IRQ routed to Xen is moved into
__setup_irq.

Signed-off-by: Julien grall <julien.grall@arm.com>

---
    Changes in v2:
        - Patch renamed
        - Set the affinity for IRQ routed to Xen in __setup_irq
---
 xen/arch/arm/gic.c        | 11 +++--------
 xen/arch/arm/irq.c        |  4 ++--
 xen/include/asm-arm/gic.h |  3 +--
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 5726a05..bc814a0 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -97,24 +97,19 @@ void gic_restore_state(struct vcpu *v)
 }
 
 /*
- * needs to be called with a valid cpu_mask, ie each cpu in the mask has
- * already called gic_cpu_init
  * - desc.lock must be held
  * - arch.type must be valid (i.e != IRQ_TYPE_INVALID)
  */
 static void gic_set_irq_properties(struct irq_desc *desc,
-                                   const cpumask_t *cpu_mask,
                                    unsigned int priority)
 {
     gic_hw_ops->set_irq_properties(desc, priority);
-    desc->handler->set_affinity(desc, cpu_mask);
 }
 
 /* Program the GIC to route an interrupt to the host (i.e. Xen)
  * - needs to be called with desc.lock held
  */
-void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
-                          unsigned int priority)
+void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority)
 {
     ASSERT(priority <= 0xff);     /* Only 8 bits of priority */
     ASSERT(desc->irq < gic_number_lines());/* Can't route interrupts that don't exist */
@@ -123,7 +118,7 @@ void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
 
     desc->handler = gic_hw_ops->gic_host_irq_type;
 
-    gic_set_irq_properties(desc, cpu_mask, priority);
+    gic_set_irq_properties(desc, priority);
 }
 
 /* Program the GIC to route an interrupt to a guest
@@ -155,7 +150,7 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int virq,
     desc->handler = gic_hw_ops->gic_guest_irq_type;
     set_bit(_IRQ_GUEST, &desc->status);
 
-    gic_set_irq_properties(desc, cpumask_of(v_target->processor), priority);
+    gic_set_irq_properties(desc, priority);
 
     p->desc = desc;
     res = 0;
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 2f8af72..3fc22f2 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -370,6 +370,7 @@ int setup_irq(unsigned int irq, unsigned int irqflags, struct irqaction *new)
     /* First time the IRQ is setup */
     if ( disabled )
     {
+        gic_route_irq_to_xen(desc, GIC_PRI_IRQ);
         /* It's fine to use smp_processor_id() because:
          * For PPI: irq_desc is banked
          * For SPI: we don't care for now which CPU will receive the
@@ -377,8 +378,7 @@ int setup_irq(unsigned int irq, unsigned int irqflags, struct irqaction *new)
          * TODO: Handle case where SPI is setup on different CPU than
          * the targeted CPU and the priority.
          */
-        gic_route_irq_to_xen(desc, cpumask_of(smp_processor_id()),
-                             GIC_PRI_IRQ);
+        irq_set_affinity(desc, cpumask_of(smp_processor_id()));
         desc->handler->startup(desc);
     }
 
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 2fc6126..7ba3846 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -223,8 +223,7 @@ enum gic_version {
 extern enum gic_version gic_hw_version(void);
 
 /* Program the GIC to route an interrupt */
-extern void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
-                                 unsigned int priority);
+extern void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority);
 extern int gic_route_irq_to_guest(struct domain *, unsigned int virq,
                                   struct irq_desc *desc,
                                   unsigned int priority);
-- 
1.9.1


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

[PATCH v2 3/9] xen/arm: gic: split set_irq_properties

[Julien Grall]

The callback set_irq_properties will configure the GIC for a specific
IRQ with the type and the priority.

In a follow-up patch, Xen will configure the type and the priority at
different stage of the routing. So split it in 2 separate callbacks.

At the same time, move the ASSERT to check the validity of the type and
if the desc->lock is locked in the common code (gic.c). This is because
the constraint are the same between GICv2 and GICv3, however the driver
of the latter did not contain any sanity check.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v2:
        - Add Stefano's reviewed-by
---
 xen/arch/arm/gic-v2.c     | 19 +++++++++++++------
 xen/arch/arm/gic-v3.c     | 15 ++++++++++++---
 xen/arch/arm/gic.c        | 23 ++++++++++++++---------
 xen/include/asm-arm/gic.h |  7 ++++---
 4 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 6c7dbfe..69ed72d 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -236,16 +236,12 @@ static unsigned int gicv2_read_irq(void)
     return (readl_gicc(GICC_IAR) & GICC_IA_IRQ);
 }
 
-static void gicv2_set_irq_properties(struct irq_desc *desc,
-                                     unsigned int priority)
+static void gicv2_set_irq_type(struct irq_desc *desc)
 {
     uint32_t cfg, actual, edgebit;
     unsigned int irq = desc->irq;
     unsigned int type = desc->arch.type;
 
-    ASSERT(type != IRQ_TYPE_INVALID);
-    ASSERT(spin_is_locked(&desc->lock));
-
     spin_lock(&gicv2.lock);
     /* Set edge / level */
     cfg = readl_gicd(GICD_ICFGR + (irq / 16) * 4);
@@ -270,6 +266,16 @@ static void gicv2_set_irq_properties(struct irq_desc *desc,
             IRQ_TYPE_LEVEL_HIGH;
     }
 
+    spin_unlock(&gicv2.lock);
+}
+
+static void gicv2_set_irq_priority(struct irq_desc *desc,
+                                   unsigned int priority)
+{
+    unsigned int irq = desc->irq;
+
+    spin_lock(&gicv2.lock);
+
     /* Set priority */
     writeb_gicd(priority, GICD_IPRIORITYR + irq);
 
@@ -1217,7 +1223,8 @@ const static struct gic_hw_operations gicv2_ops = {
     .eoi_irq             = gicv2_eoi_irq,
     .deactivate_irq      = gicv2_dir_irq,
     .read_irq            = gicv2_read_irq,
-    .set_irq_properties  = gicv2_set_irq_properties,
+    .set_irq_type        = gicv2_set_irq_type,
+    .set_irq_priority    = gicv2_set_irq_priority,
     .send_SGI            = gicv2_send_SGI,
     .disable_interface   = gicv2_disable_interface,
     .update_lr           = gicv2_update_lr,
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index d6ab0e9..781f25c 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -471,8 +471,7 @@ static inline uint64_t gicv3_mpidr_to_affinity(int cpu)
              MPIDR_AFFINITY_LEVEL(mpidr, 0));
 }
 
-static void gicv3_set_irq_properties(struct irq_desc *desc,
-                                     unsigned int priority)
+static void gicv3_set_irq_type(struct irq_desc *desc)
 {
     uint32_t cfg, actual, edgebit;
     void __iomem *base;
@@ -512,6 +511,15 @@ static void gicv3_set_irq_properties(struct irq_desc *desc,
             IRQ_TYPE_EDGE_RISING :
             IRQ_TYPE_LEVEL_HIGH;
     }
+    spin_unlock(&gicv3.lock);
+}
+
+static void gicv3_set_irq_priority(struct irq_desc *desc,
+                                   unsigned int priority)
+{
+    unsigned int irq = desc->irq;
+
+    spin_lock(&gicv3.lock);
 
     /* Set priority */
     if ( irq < NR_GIC_LOCAL_IRQS )
@@ -1579,7 +1587,8 @@ static const struct gic_hw_operations gicv3_ops = {
     .eoi_irq             = gicv3_eoi_irq,
     .deactivate_irq      = gicv3_dir_irq,
     .read_irq            = gicv3_read_irq,
-    .set_irq_properties  = gicv3_set_irq_properties,
+    .set_irq_type        = gicv3_set_irq_type,
+    .set_irq_priority    = gicv3_set_irq_priority,
     .send_SGI            = gicv3_send_sgi,
     .disable_interface   = gicv3_disable_interface,
     .update_lr           = gicv3_update_lr,
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index bc814a0..c63c862 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -96,14 +96,17 @@ void gic_restore_state(struct vcpu *v)
     gic_restore_pending_irqs(v);
 }
 
-/*
- * - desc.lock must be held
- * - arch.type must be valid (i.e != IRQ_TYPE_INVALID)
- */
-static void gic_set_irq_properties(struct irq_desc *desc,
-                                   unsigned int priority)
+static void gic_set_irq_type(struct irq_desc *desc)
+{
+    ASSERT(spin_is_locked(&desc->lock));
+    ASSERT(desc->arch.type != IRQ_TYPE_INVALID);
+
+    gic_hw_ops->set_irq_type(desc);
+}
+
+static void gic_set_irq_priority(struct irq_desc *desc, unsigned int priority)
 {
-    gic_hw_ops->set_irq_properties(desc, priority);
+    gic_hw_ops->set_irq_priority(desc, priority);
 }
 
 /* Program the GIC to route an interrupt to the host (i.e. Xen)
@@ -118,7 +121,8 @@ void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority)
 
     desc->handler = gic_hw_ops->gic_host_irq_type;
 
-    gic_set_irq_properties(desc, priority);
+    gic_set_irq_type(desc);
+    gic_set_irq_priority(desc, priority);
 }
 
 /* Program the GIC to route an interrupt to a guest
@@ -150,7 +154,8 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int virq,
     desc->handler = gic_hw_ops->gic_guest_irq_type;
     set_bit(_IRQ_GUEST, &desc->status);
 
-    gic_set_irq_properties(desc, priority);
+    gic_set_irq_type(desc);
+    gic_set_irq_priority(desc, priority);
 
     p->desc = desc;
     res = 0;
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 7ba3846..3f39f79 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -328,9 +328,10 @@ struct gic_hw_operations {
     void (*deactivate_irq)(struct irq_desc *irqd);
     /* Read IRQ id and Ack */
     unsigned int (*read_irq)(void);
-    /* Set IRQ property */
-    void (*set_irq_properties)(struct irq_desc *desc,
-                               unsigned int priority);
+    /* Set IRQ type - type is taken from desc->arch.type */
+    void (*set_irq_type)(struct irq_desc *desc);
+    /* Set IRQ priority */
+    void (*set_irq_priority)(struct irq_desc *desc, unsigned int priority);
     /* Send SGI */
     void (*send_SGI)(enum gic_sgi sgi, enum gic_sgi_mode irqmode,
                      const cpumask_t *online_mask);
-- 
1.9.1


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

[PATCH v2 4/9] xen/arm: gic: set_type: Pass the type in parameter rather than in desc->arch.type

[Julien Grall]

A follow-up patch will not store the type in desc->arch.type. Also, the
callback prototype is more logical.

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

---
    Changes in v2:
        - gic_set_irq_type has been dropped by mistake in
        gic_route_irq_to_xen. Re-add it!
---
 xen/arch/arm/gic-v2.c     |  3 +--
 xen/arch/arm/gic-v3.c     |  3 +--
 xen/arch/arm/gic.c        | 10 +++++-----
 xen/include/asm-arm/gic.h |  4 ++--
 4 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 69ed72d..9bd9d0b 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -236,11 +236,10 @@ static unsigned int gicv2_read_irq(void)
     return (readl_gicc(GICC_IAR) & GICC_IA_IRQ);
 }
 
-static void gicv2_set_irq_type(struct irq_desc *desc)
+static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int type)
 {
     uint32_t cfg, actual, edgebit;
     unsigned int irq = desc->irq;
-    unsigned int type = desc->arch.type;
 
     spin_lock(&gicv2.lock);
     /* Set edge / level */
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 781f25c..b8be395 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -471,12 +471,11 @@ static inline uint64_t gicv3_mpidr_to_affinity(int cpu)
              MPIDR_AFFINITY_LEVEL(mpidr, 0));
 }
 
-static void gicv3_set_irq_type(struct irq_desc *desc)
+static void gicv3_set_irq_type(struct irq_desc *desc, unsigned int type)
 {
     uint32_t cfg, actual, edgebit;
     void __iomem *base;
     unsigned int irq = desc->irq;
-    unsigned int type = desc->arch.type;
 
     /* SGI's are always edge-triggered not need to call GICD_ICFGR0 */
     ASSERT(irq >= NR_GIC_SGI);
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index c63c862..b9371a7 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -96,12 +96,12 @@ void gic_restore_state(struct vcpu *v)
     gic_restore_pending_irqs(v);
 }
 
-static void gic_set_irq_type(struct irq_desc *desc)
+static void gic_set_irq_type(struct irq_desc *desc, unsigned int type)
 {
     ASSERT(spin_is_locked(&desc->lock));
-    ASSERT(desc->arch.type != IRQ_TYPE_INVALID);
+    ASSERT(type != IRQ_TYPE_INVALID);
 
-    gic_hw_ops->set_irq_type(desc);
+    gic_hw_ops->set_irq_type(desc, type);
 }
 
 static void gic_set_irq_priority(struct irq_desc *desc, unsigned int priority)
@@ -121,7 +121,7 @@ void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority)
 
     desc->handler = gic_hw_ops->gic_host_irq_type;
 
-    gic_set_irq_type(desc);
+    gic_set_irq_type(desc, desc->arch.type);
     gic_set_irq_priority(desc, priority);
 }
 
@@ -154,7 +154,7 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int virq,
     desc->handler = gic_hw_ops->gic_guest_irq_type;
     set_bit(_IRQ_GUEST, &desc->status);
 
-    gic_set_irq_type(desc);
+    gic_set_irq_type(desc, desc->arch.type);
     gic_set_irq_priority(desc, priority);
 
     p->desc = desc;
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 3f39f79..2214e87 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -328,8 +328,8 @@ struct gic_hw_operations {
     void (*deactivate_irq)(struct irq_desc *irqd);
     /* Read IRQ id and Ack */
     unsigned int (*read_irq)(void);
-    /* Set IRQ type - type is taken from desc->arch.type */
-    void (*set_irq_type)(struct irq_desc *desc);
+    /* Set IRQ type */
+    void (*set_irq_type)(struct irq_desc *desc, unsigned int type);
     /* Set IRQ priority */
     void (*set_irq_priority)(struct irq_desc *desc, unsigned int priority);
     /* Send SGI */
-- 
1.9.1


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

[PATCH v2 5/9] xen/arm: gic: Document how gic_set_irq_type should be called

[Julien Grall]

Changing the value of Int_config is UNPREDICTABLE when the corresponding
interrupt is not disabled.

The driver is assuming the interrupt will be disabled by the caller of
gic_set_irq_type. Add an ASSERT to ensure it.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v2:
        - Add Stefano's acked-by
---
 xen/arch/arm/gic.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index b9371a7..72bb885 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -96,8 +96,14 @@ void gic_restore_state(struct vcpu *v)
     gic_restore_pending_irqs(v);
 }
 
+/* desc->irq needs to be disabled before calling this function */
 static void gic_set_irq_type(struct irq_desc *desc, unsigned int type)
 {
+    /*
+     * IRQ must be disabled before configuring it (see 4.3.13 in ARM IHI
+     * 0048B.b). We rely on the caller to do it.
+     */
+    ASSERT(test_bit(_IRQ_DISABLED, &desc->status));
     ASSERT(spin_is_locked(&desc->lock));
     ASSERT(type != IRQ_TYPE_INVALID);
 
-- 
1.9.1


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

[PATCH v2 6/9] Revert "xen/arm: warn the user that we cannot route SPIs to Dom0 on ACPI"

[Julien Grall]

This reverts commit f91c84edebe67296e4051af055dbf0adafb13a37. SPI
routing for ACPI support will be added in a follow-up patch.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sttabellini@kernel.org>

---
    Changes in v2:
        - Add Stefano's reviewed-by
---
 xen/arch/arm/vgic.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 35723c9..5070452 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -25,8 +25,6 @@
 #include <xen/irq.h>
 #include <xen/sched.h>
 #include <xen/perfc.h>
-#include <xen/iocap.h>
-#include <xen/acpi.h>
 
 #include <asm/current.h>
 
@@ -354,22 +352,9 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
     unsigned long flags;
     int i = 0;
     struct vcpu *v_target;
-    struct domain *d = v->domain;
 
     while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
         irq = i + (32 * n);
-        /* Set the irq type and route it to guest only for SPI and Dom0 */
-        if( irq_access_permitted(d, irq) && is_hardware_domain(d) &&
-            ( irq >= 32 ) && ( !acpi_disabled ) )
-        {
-            static int log_once = 0;
-            if ( !log_once )
-            {
-                gprintk(XENLOG_WARNING, "Routing SPIs to Dom0 on ACPI systems is unimplemented.\n");
-                log_once++;
-            }
-        }
-
         v_target = __vgic_get_target_vcpu(v, irq);
         p = irq_to_pending(v_target, irq);
         set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
-- 
1.9.1


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

[PATCH v2 7/9] xen/arm: Allow DOM0 to set the IRQ type

[Julien Grall]

The function route_irq_to_guest mandates the IRQ type, stored in
desc->arch.type, to be valid. However, in case of ACPI, these
information is not part of the static tables. Therefore Xen needs to
rely on DOM0 to provide a valid type based on the firmware tables.

A new helper, irq_type_set_by_domain is provided to check whether a
domain is allowed to set the IRQ type. For now, only DOM0 is allowed to
configure.

When the helper returns 1, the routing function will not check whether
the IRQ type is correctly set and configure the GIC. Instead, this will
be done when the domain will enable the interrupt.

Note that irq_set_spi_type is not called because it validates the type
and does not allow it the domain to change the type after the first
write. It means that desc->arch.type may never be set, which is fine
because the field is only used to configure the type during the routing.

Based on 4.3.13 in ARM IHI 0048B.b, changing the value of Int_config is
UNPREDICTABLE when the corresponding interrupt is not disabled.

Therefore, setting the IRQ type when the guest is writing into ICFGR
would require more work to make sure the IRQ has been disabled before
writing into the host ICFGR. As the behavior is UNPREDICTABLE, the type
will be set before enabling the physical IRQ associated to the virtual IRQ.

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

---

It might be possible to let any domain configure the IRQ
type (could be useful when passthrough an IRQ with ACPI). However, we would
need to consider any potential security impact beforehand.

    Changes in v2:
        - Rename the patch
        - Allow any DOM0 to set the IRQ type
        - Re-use in part of vgic_get_virq_type from
        "Configure SPI interrupt type and route to Dom0 dynamically".
        - Add rationale why the IRQ type is set in enable
---
 xen/arch/arm/gic.c        |  5 +++--
 xen/arch/arm/irq.c        | 13 ++++++++++++-
 xen/arch/arm/vgic.c       | 19 +++++++++++++++++++
 xen/include/asm-arm/gic.h |  3 +++
 xen/include/asm-arm/irq.h |  6 ++++++
 5 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 72bb885..63c744a 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -97,7 +97,7 @@ void gic_restore_state(struct vcpu *v)
 }
 
 /* desc->irq needs to be disabled before calling this function */
-static void gic_set_irq_type(struct irq_desc *desc, unsigned int type)
+void gic_set_irq_type(struct irq_desc *desc, unsigned int type)
 {
     /*
      * IRQ must be disabled before configuring it (see 4.3.13 in ARM IHI
@@ -160,7 +160,8 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int virq,
     desc->handler = gic_hw_ops->gic_guest_irq_type;
     set_bit(_IRQ_GUEST, &desc->status);
 
-    gic_set_irq_type(desc, desc->arch.type);
+    if ( !irq_type_set_by_domain(d) )
+        gic_set_irq_type(desc, desc->arch.type);
     gic_set_irq_priority(desc, priority);
 
     p->desc = desc;
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 3fc22f2..06d4843 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -395,6 +395,17 @@ bool_t is_assignable_irq(unsigned int irq)
 }
 
 /*
+ * Only the hardware domain is allowed to set the configure the
+ * interrupt type for now.
+ *
+ * XXX: See whether it is possible to let any domain configure the type.
+ */
+bool_t irq_type_set_by_domain(const struct domain *d)
+{
+    return (d == hardware_domain);
+}
+
+/*
  * Route an IRQ to a specific guest.
  * For now only SPIs are assignable to the guest.
  */
@@ -449,7 +460,7 @@ int route_irq_to_guest(struct domain *d, unsigned int virq,
 
     spin_lock_irqsave(&desc->lock, flags);
 
-    if ( desc->arch.type == IRQ_TYPE_INVALID )
+    if ( !irq_type_set_by_domain(d) && desc->arch.type == IRQ_TYPE_INVALID )
     {
         printk(XENLOG_G_ERR "IRQ %u has not been configured\n", irq);
         retval = -EIO;
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 5070452..a7ccfe7 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -344,6 +344,22 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
     }
 }
 
+#define VGIC_ICFG_MASK(intr) (1 << ((2 * ((intr) % 16)) + 1))
+
+/* The function should be called with the rank lock taken */
+static inline unsigned int vgic_get_virq_type(struct vcpu *v, int n, int index)
+{
+    struct vgic_irq_rank *r = vgic_get_rank(v, n);
+    uint32_t tr = r->icfg[index >> 4];
+
+    ASSERT(spin_is_locked(&r->lock));
+
+    if ( tr & VGIC_ICFG_MASK(index) )
+        return IRQ_TYPE_EDGE_RISING;
+    else
+        return IRQ_TYPE_LEVEL_HIGH;
+}
+
 void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
 {
     const unsigned long mask = r;
@@ -352,6 +368,7 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
     unsigned long flags;
     int i = 0;
     struct vcpu *v_target;
+    struct domain *d = v->domain;
 
     while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
         irq = i + (32 * n);
@@ -366,6 +383,8 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
         {
             irq_set_affinity(p->desc, cpumask_of(v_target->processor));
             spin_lock_irqsave(&p->desc->lock, flags);
+            if ( irq_type_set_by_domain(d) )
+                gic_set_irq_type(p->desc, vgic_get_virq_type(v, n, i));
             p->desc->handler->enable(p->desc);
             spin_unlock_irqrestore(&p->desc->lock, flags);
         }
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 2214e87..836a103 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -222,6 +222,9 @@ enum gic_version {
 
 extern enum gic_version gic_hw_version(void);
 
+/* Program the IRQ type into the GIC */
+void gic_set_irq_type(struct irq_desc *desc, unsigned int type);
+
 /* Program the GIC to route an interrupt */
 extern void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority);
 extern int gic_route_irq_to_guest(struct domain *, unsigned int virq,
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index 493773c..8f7a167 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -58,6 +58,12 @@ int platform_get_irq(const struct dt_device_node *device, int index);
 
 void irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask);
 
+/*
+ * Use this helper in places that need to know whether the IRQ type is
+ * set by the domain.
+ */
+bool_t irq_type_set_by_domain(const struct domain *d);
+
 #endif /* _ASM_HW_IRQ_H */
 /*
  * Local variables:
-- 
1.9.1


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

[PATCH v2 8/9] xen/arm: acpi: route all unused IRQs to DOM0

[Julien Grall]

It is not possible to know which IRQs will be used by DOM0 when ACPI is
inuse. The approach implemented by this patch, will route all unused
IRQs to DOM0 before it has booted.

The number of IRQs routed is based on the maximum SPIs supported by the
hardware (up to ~1000). However, some of them might not be wired. So we
would allocate resource for nothing.

For each IRQ routed, Xen is allocating memory for irqaction (40 bytes)
and irq_guest (16 bytes). So in the worst case scenario ~54KB of memory
will be allocated. Given that ACPI will mostly be used by server, I
think it is a small drawback.

map_irq_to_domain is slightly reworked to remove the dependency on
device-tree. So the function can be also be used for ACPI and will
avoid code duplication.

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

---
    Changes in v2:
        - Rename acpi_permit_spi_access to acpi_route_spis
        - Update the comment in the function acpi_route_spis
---
 xen/arch/arm/domain_build.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 60db9e4..5b2f8ad 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -903,11 +903,10 @@ static int make_timer_node(const struct domain *d, void *fdt,
     return res;
 }
 
-static int map_irq_to_domain(const struct dt_device_node *dev,
-                             struct domain *d, unsigned int irq)
+static int map_irq_to_domain(struct domain *d, unsigned int irq,
+                             bool_t need_mapping, const char *devname)
 
 {
-    bool_t need_mapping = !dt_device_for_passthrough(dev);
     int res;
 
     res = irq_permit_access(d, irq);
@@ -927,7 +926,7 @@ static int map_irq_to_domain(const struct dt_device_node *dev,
          */
         vgic_reserve_virq(d, irq);
 
-        res = route_irq_to_guest(d, irq, irq, dt_node_name(dev));
+        res = route_irq_to_guest(d, irq, irq, devname);
         if ( res < 0 )
         {
             printk(XENLOG_ERR "Unable to map IRQ%"PRId32" to dom%d\n",
@@ -947,6 +946,7 @@ static int map_dt_irq_to_domain(const struct dt_device_node *dev,
     struct domain *d = data;
     unsigned int irq = dt_irq->irq;
     int res;
+    bool_t need_mapping = !dt_device_for_passthrough(dev);
 
     if ( irq < NR_LOCAL_IRQS )
     {
@@ -965,7 +965,7 @@ static int map_dt_irq_to_domain(const struct dt_device_node *dev,
         return res;
     }
 
-    res = map_irq_to_domain(dev, d, irq);
+    res = map_irq_to_domain(d, irq, need_mapping, dt_node_name(dev));
 
     return 0;
 }
@@ -1103,7 +1103,7 @@ static int handle_device(struct domain *d, struct dt_device_node *dev)
             return res;
         }
 
-        res = map_irq_to_domain(dev, d, res);
+        res = map_irq_to_domain(d, res, need_mapping, dt_node_name(dev));
         if ( res )
             return res;
     }
@@ -1343,15 +1343,14 @@ static int acpi_iomem_deny_access(struct domain *d)
     return gic_iomem_deny_access(d);
 }
 
-static int acpi_permit_spi_access(struct domain *d)
+static int acpi_route_spis(struct domain *d)
 {
     int i, res;
     struct irq_desc *desc;
 
     /*
-     * Here just permit Dom0 to access the SPIs which Xen doesn't use. Then when
-     * Dom0 configures the interrupt, set the interrupt type and route it to
-     * Dom0.
+     * Route the IRQ to hardware domain and permit the access.
+     * The interrupt type will be set by set by the hardware domain.
      */
     for( i = NR_LOCAL_IRQS; i < vgic_num_irqs(d); i++ )
     {
@@ -1362,13 +1361,10 @@ static int acpi_permit_spi_access(struct domain *d)
         if ( desc->action != NULL)
             continue;
 
-        res = irq_permit_access(d, i);
+        /* XXX: Shall we use a proper devname? */
+        res = map_irq_to_domain(d, i, true, "ACPI");
         if ( res )
-        {
-            printk(XENLOG_ERR "Unable to permit to dom%u access to IRQ %u\n",
-                   d->domain_id, i);
             return res;
-        }
     }
 
     return 0;
@@ -1902,7 +1898,7 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     if ( rc != 0 )
         return rc;
 
-    rc = acpi_permit_spi_access(d);
+    rc = acpi_route_spis(d);
     if ( rc != 0 )
         return rc;
 
-- 
1.9.1


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

[PATCH v2 9/9] xen/arm: Fix coding style and update comment in acpi_route_spis

[Julien Grall]

The comment was not correctly indented. Also the preferred name for the
initial domain is "hardware domain" and not "dom0, so replace it.

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

---
    Changes in v2:
        - Patch added
---
 xen/arch/arm/domain_build.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 5b2f8ad..35ab08d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1355,8 +1355,9 @@ static int acpi_route_spis(struct domain *d)
     for( i = NR_LOCAL_IRQS; i < vgic_num_irqs(d); i++ )
     {
         /*
-	 * TODO: Exclude the SPIs SMMU uses which should not be routed to Dom0.
-	 */
+         * TODO: Exclude the SPIs SMMU uses which should not be routed to
+         * the hardware domain.
+         */
         desc = irq_to_desc(i);
         if ( desc->action != NULL)
             continue;
-- 
1.9.1


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

Re: [PATCH v2 6/9] Revert "xen/arm: warn the user that we cannot route SPIs to Dom0 on ACPI"

[Julien Grall]

Hi,

On 14/07/16 17:22, Julien Grall wrote:
> This reverts commit f91c84edebe67296e4051af055dbf0adafb13a37. SPI
> routing for ACPI support will be added in a follow-up patch.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Stefano Stabellini <sttabellini@kernel.org>

I made a typo in Stefano's address mail. This should be 
sstabellini@kernel.org.

Regards,

>
> ---
>      Changes in v2:
>          - Add Stefano's reviewed-by
> ---
>   xen/arch/arm/vgic.c | 15 ---------------
>   1 file changed, 15 deletions(-)
>
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 35723c9..5070452 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -25,8 +25,6 @@
>   #include <xen/irq.h>
>   #include <xen/sched.h>
>   #include <xen/perfc.h>
> -#include <xen/iocap.h>
> -#include <xen/acpi.h>
>
>   #include <asm/current.h>
>
> @@ -354,22 +352,9 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>       unsigned long flags;
>       int i = 0;
>       struct vcpu *v_target;
> -    struct domain *d = v->domain;
>
>       while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
>           irq = i + (32 * n);
> -        /* Set the irq type and route it to guest only for SPI and Dom0 */
> -        if( irq_access_permitted(d, irq) && is_hardware_domain(d) &&
> -            ( irq >= 32 ) && ( !acpi_disabled ) )
> -        {
> -            static int log_once = 0;
> -            if ( !log_once )
> -            {
> -                gprintk(XENLOG_WARNING, "Routing SPIs to Dom0 on ACPI systems is unimplemented.\n");
> -                log_once++;
> -            }
> -        }
> -
>           v_target = __vgic_get_target_vcpu(v, irq);
>           p = irq_to_pending(v_target, irq);
>           set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
>

-- 
Julien Grall

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

Re: [PATCH v2 0/9] xen/arm: Support SPIs routing

[Shanker Donthineni]

Hi Julien,

Tested-by: Shanker Donthineni<shankerd@codeaurora.org>

I have tested this patchset on Qualcomm Technologies QDF2XXX server platform without any issue.


On 07/14/2016 11:21 AM, Julien Grall wrote:
> Hello all,
>
> Currently, Xen does not route SPIs to DOM0 when ACPI is inuse after
> the functionality has been reverted in Xen 4.7 by commit 909bd14.
>
> In the previous approach, the SPIs was routed when DOM0 was writing into
> ISENABLER. However, this has resulted to deadlock (see more details in [1])
> as soon as the IRQ was enabled by DOM0.
>
> We have multiple solutions to route the IRQ:
>      1) Rework route_irq_to_guest to avoid the deadlock
>      2) Route and enable the IRQ outside of the emulation of ISENABLER
>      3) Remove the dependency on the IRQ type in the routing function
>      and route all the unused IRQs during domain building
>      4) Add a new hypercall to let DOM0 routing the IRQ
>
> I think that 1) and 2) are not resilient because route_irq_to_guest may fail
> and there is no way to report this error to the guest (except by killing it).
>
> Furthermore, in solution 2) enabling the interrupt would need to be defer
> until the routing has been done. This would require a lot of code duplication.
>
> Which leave solution 3) and 4). The solution 4) requires to introduce a new
> (or re-use one) stable hypercall. I am not sure why we ruled out this
> solution when we reviewed the ACPI design document.
>
> This patch series is implementing the 3rd solution which defer the IRQ
> type configuration for DOM0 IRQ when ACPI is inuse. However, this will
> slightly increase the memory usage of Xen (54KB).
>
> I am happy to consider any other solutions.
>
> I only tested briefly this patch series, Shanker can you give a try on
> your hardware?
>
> A branch with all the patches can be found here:
>
> git://xenbits.xen.org/people/julieng/xen-unstable.git branch irq-routing-acpi-v2
>
> Yours sincerely,
>
> [1] http://lists.xenproject.org/archives/html/xen-devel/2016-05/msg02633.html
>
> Julien Grall (9):
>    xen/arm: gic: Consolidate the IRQ affinity set in a single place
>    xen/arm: gic: Do not configure affinity during routing
>    xen/arm: gic: split set_irq_properties
>    xen/arm: gic: set_type: Pass the type in parameter rather than in
>      desc->arch.type
>    xen/arm: gic: Document how gic_set_irq_type should be called
>    Revert "xen/arm: warn the user that we cannot route SPIs to Dom0 on
>      ACPI"
>    xen/arm: Allow DOM0 to set the IRQ type
>    xen/arm: acpi: route all unused IRQs to DOM0
>    xen/arm: Fix coding style and update comment in acpi_route_spis
>
>   xen/arch/arm/domain_build.c | 33 +++++++++++++++------------------
>   xen/arch/arm/gic-v2.c       | 28 +++++++++++++---------------
>   xen/arch/arm/gic-v3.c       | 22 ++++++++++------------
>   xen/arch/arm/gic.c          | 36 ++++++++++++++++++++++--------------
>   xen/arch/arm/irq.c          | 17 ++++++++++++++---
>   xen/arch/arm/vgic.c         | 32 ++++++++++++++++++--------------
>   xen/include/asm-arm/gic.h   | 14 ++++++++------
>   xen/include/asm-arm/irq.h   |  6 ++++++
>   8 files changed, 106 insertions(+), 82 deletions(-)
>

-- 
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.


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

Re: [PATCH v2 2/9] xen/arm: gic: Do not configure affinity during routing

[Stefano Stabellini]

On Thu, 14 Jul 2016, Julien Grall wrote:
> The affinity of a guest IRQ is set every time the guest enable it (see
> vgic_enable_irqs).
> 
> It is not necessary to set the affinity when the IRQ is routed to the
> guest because Xen will never receive the IRQ until it hass been enabled
> by the guest.
> 
> To keep gic_route_irq_to_{xen,guest} behaving the same way (i.e just
> setting up the routing), the affinity of IRQ routed to Xen is moved into
> __setup_irq.
> 
> Signed-off-by: Julien grall <julien.grall@arm.com>

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


> ---
>     Changes in v2:
>         - Patch renamed
>         - Set the affinity for IRQ routed to Xen in __setup_irq
> ---
>  xen/arch/arm/gic.c        | 11 +++--------
>  xen/arch/arm/irq.c        |  4 ++--
>  xen/include/asm-arm/gic.h |  3 +--
>  3 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 5726a05..bc814a0 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -97,24 +97,19 @@ void gic_restore_state(struct vcpu *v)
>  }
>  
>  /*
> - * needs to be called with a valid cpu_mask, ie each cpu in the mask has
> - * already called gic_cpu_init
>   * - desc.lock must be held
>   * - arch.type must be valid (i.e != IRQ_TYPE_INVALID)
>   */
>  static void gic_set_irq_properties(struct irq_desc *desc,
> -                                   const cpumask_t *cpu_mask,
>                                     unsigned int priority)
>  {
>      gic_hw_ops->set_irq_properties(desc, priority);
> -    desc->handler->set_affinity(desc, cpu_mask);
>  }
>  
>  /* Program the GIC to route an interrupt to the host (i.e. Xen)
>   * - needs to be called with desc.lock held
>   */
> -void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
> -                          unsigned int priority)
> +void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority)
>  {
>      ASSERT(priority <= 0xff);     /* Only 8 bits of priority */
>      ASSERT(desc->irq < gic_number_lines());/* Can't route interrupts that don't exist */
> @@ -123,7 +118,7 @@ void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
>  
>      desc->handler = gic_hw_ops->gic_host_irq_type;
>  
> -    gic_set_irq_properties(desc, cpu_mask, priority);
> +    gic_set_irq_properties(desc, priority);
>  }
>  
>  /* Program the GIC to route an interrupt to a guest
> @@ -155,7 +150,7 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int virq,
>      desc->handler = gic_hw_ops->gic_guest_irq_type;
>      set_bit(_IRQ_GUEST, &desc->status);
>  
> -    gic_set_irq_properties(desc, cpumask_of(v_target->processor), priority);
> +    gic_set_irq_properties(desc, priority);
>  
>      p->desc = desc;
>      res = 0;
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 2f8af72..3fc22f2 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -370,6 +370,7 @@ int setup_irq(unsigned int irq, unsigned int irqflags, struct irqaction *new)
>      /* First time the IRQ is setup */
>      if ( disabled )
>      {
> +        gic_route_irq_to_xen(desc, GIC_PRI_IRQ);
>          /* It's fine to use smp_processor_id() because:
>           * For PPI: irq_desc is banked
>           * For SPI: we don't care for now which CPU will receive the
> @@ -377,8 +378,7 @@ int setup_irq(unsigned int irq, unsigned int irqflags, struct irqaction *new)
>           * TODO: Handle case where SPI is setup on different CPU than
>           * the targeted CPU and the priority.
>           */
> -        gic_route_irq_to_xen(desc, cpumask_of(smp_processor_id()),
> -                             GIC_PRI_IRQ);
> +        irq_set_affinity(desc, cpumask_of(smp_processor_id()));
>          desc->handler->startup(desc);
>      }
>  
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 2fc6126..7ba3846 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -223,8 +223,7 @@ enum gic_version {
>  extern enum gic_version gic_hw_version(void);
>  
>  /* Program the GIC to route an interrupt */
> -extern void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
> -                                 unsigned int priority);
> +extern void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority);
>  extern int gic_route_irq_to_guest(struct domain *, unsigned int virq,
>                                    struct irq_desc *desc,
>                                    unsigned int priority);
> -- 
> 1.9.1
> 

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

Re: [PATCH v2 4/9] xen/arm: gic: set_type: Pass the type in parameter rather than in desc->arch.type

[Stefano Stabellini]

On Thu, 14 Jul 2016, Julien Grall wrote:
> A follow-up patch will not store the type in desc->arch.type. Also, the
> callback prototype is more logical.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

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

> ---
>     Changes in v2:
>         - gic_set_irq_type has been dropped by mistake in
>         gic_route_irq_to_xen. Re-add it!
> ---
>  xen/arch/arm/gic-v2.c     |  3 +--
>  xen/arch/arm/gic-v3.c     |  3 +--
>  xen/arch/arm/gic.c        | 10 +++++-----
>  xen/include/asm-arm/gic.h |  4 ++--
>  4 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 69ed72d..9bd9d0b 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -236,11 +236,10 @@ static unsigned int gicv2_read_irq(void)
>      return (readl_gicc(GICC_IAR) & GICC_IA_IRQ);
>  }
>  
> -static void gicv2_set_irq_type(struct irq_desc *desc)
> +static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int type)
>  {
>      uint32_t cfg, actual, edgebit;
>      unsigned int irq = desc->irq;
> -    unsigned int type = desc->arch.type;
>  
>      spin_lock(&gicv2.lock);
>      /* Set edge / level */
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 781f25c..b8be395 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -471,12 +471,11 @@ static inline uint64_t gicv3_mpidr_to_affinity(int cpu)
>               MPIDR_AFFINITY_LEVEL(mpidr, 0));
>  }
>  
> -static void gicv3_set_irq_type(struct irq_desc *desc)
> +static void gicv3_set_irq_type(struct irq_desc *desc, unsigned int type)
>  {
>      uint32_t cfg, actual, edgebit;
>      void __iomem *base;
>      unsigned int irq = desc->irq;
> -    unsigned int type = desc->arch.type;
>  
>      /* SGI's are always edge-triggered not need to call GICD_ICFGR0 */
>      ASSERT(irq >= NR_GIC_SGI);
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index c63c862..b9371a7 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -96,12 +96,12 @@ void gic_restore_state(struct vcpu *v)
>      gic_restore_pending_irqs(v);
>  }
>  
> -static void gic_set_irq_type(struct irq_desc *desc)
> +static void gic_set_irq_type(struct irq_desc *desc, unsigned int type)
>  {
>      ASSERT(spin_is_locked(&desc->lock));
> -    ASSERT(desc->arch.type != IRQ_TYPE_INVALID);
> +    ASSERT(type != IRQ_TYPE_INVALID);
>  
> -    gic_hw_ops->set_irq_type(desc);
> +    gic_hw_ops->set_irq_type(desc, type);
>  }
>  
>  static void gic_set_irq_priority(struct irq_desc *desc, unsigned int priority)
> @@ -121,7 +121,7 @@ void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority)
>  
>      desc->handler = gic_hw_ops->gic_host_irq_type;
>  
> -    gic_set_irq_type(desc);
> +    gic_set_irq_type(desc, desc->arch.type);
>      gic_set_irq_priority(desc, priority);
>  }
>  
> @@ -154,7 +154,7 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int virq,
>      desc->handler = gic_hw_ops->gic_guest_irq_type;
>      set_bit(_IRQ_GUEST, &desc->status);
>  
> -    gic_set_irq_type(desc);
> +    gic_set_irq_type(desc, desc->arch.type);
>      gic_set_irq_priority(desc, priority);
>  
>      p->desc = desc;
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 3f39f79..2214e87 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -328,8 +328,8 @@ struct gic_hw_operations {
>      void (*deactivate_irq)(struct irq_desc *irqd);
>      /* Read IRQ id and Ack */
>      unsigned int (*read_irq)(void);
> -    /* Set IRQ type - type is taken from desc->arch.type */
> -    void (*set_irq_type)(struct irq_desc *desc);
> +    /* Set IRQ type */
> +    void (*set_irq_type)(struct irq_desc *desc, unsigned int type);
>      /* Set IRQ priority */
>      void (*set_irq_priority)(struct irq_desc *desc, unsigned int priority);
>      /* Send SGI */
> -- 
> 1.9.1
> 

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

Re: [PATCH v2 7/9] xen/arm: Allow DOM0 to set the IRQ type

[Stefano Stabellini]

On Thu, 14 Jul 2016, Julien Grall wrote:
> The function route_irq_to_guest mandates the IRQ type, stored in
> desc->arch.type, to be valid. However, in case of ACPI, these
> information is not part of the static tables. Therefore Xen needs to
> rely on DOM0 to provide a valid type based on the firmware tables.
> 
> A new helper, irq_type_set_by_domain is provided to check whether a
> domain is allowed to set the IRQ type. For now, only DOM0 is allowed to
> configure.
> 
> When the helper returns 1, the routing function will not check whether
> the IRQ type is correctly set and configure the GIC. Instead, this will
> be done when the domain will enable the interrupt.
> 
> Note that irq_set_spi_type is not called because it validates the type
> and does not allow it the domain to change the type after the first
> write. It means that desc->arch.type may never be set, which is fine
> because the field is only used to configure the type during the routing.
> 
> Based on 4.3.13 in ARM IHI 0048B.b, changing the value of Int_config is
> UNPREDICTABLE when the corresponding interrupt is not disabled.
> 
> Therefore, setting the IRQ type when the guest is writing into ICFGR
> would require more work to make sure the IRQ has been disabled before
> writing into the host ICFGR. As the behavior is UNPREDICTABLE, the type
> will be set before enabling the physical IRQ associated to the virtual IRQ.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
> 
> It might be possible to let any domain configure the IRQ
> type (could be useful when passthrough an IRQ with ACPI). However, we would
> need to consider any potential security impact beforehand.
> 
>     Changes in v2:
>         - Rename the patch
>         - Allow any DOM0 to set the IRQ type
>         - Re-use in part of vgic_get_virq_type from
>         "Configure SPI interrupt type and route to Dom0 dynamically".
>         - Add rationale why the IRQ type is set in enable
> ---
>  xen/arch/arm/gic.c        |  5 +++--
>  xen/arch/arm/irq.c        | 13 ++++++++++++-
>  xen/arch/arm/vgic.c       | 19 +++++++++++++++++++
>  xen/include/asm-arm/gic.h |  3 +++
>  xen/include/asm-arm/irq.h |  6 ++++++
>  5 files changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 72bb885..63c744a 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -97,7 +97,7 @@ void gic_restore_state(struct vcpu *v)
>  }
>  
>  /* desc->irq needs to be disabled before calling this function */
> -static void gic_set_irq_type(struct irq_desc *desc, unsigned int type)
> +void gic_set_irq_type(struct irq_desc *desc, unsigned int type)
>  {
>      /*
>       * IRQ must be disabled before configuring it (see 4.3.13 in ARM IHI
> @@ -160,7 +160,8 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int virq,
>      desc->handler = gic_hw_ops->gic_guest_irq_type;
>      set_bit(_IRQ_GUEST, &desc->status);
>  
> -    gic_set_irq_type(desc, desc->arch.type);
> +    if ( !irq_type_set_by_domain(d) )
> +        gic_set_irq_type(desc, desc->arch.type);
>      gic_set_irq_priority(desc, priority);
>  
>      p->desc = desc;
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 3fc22f2..06d4843 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -395,6 +395,17 @@ bool_t is_assignable_irq(unsigned int irq)
>  }
>  
>  /*
> + * Only the hardware domain is allowed to set the configure the
> + * interrupt type for now.
> + *
> + * XXX: See whether it is possible to let any domain configure the type.
> + */
> +bool_t irq_type_set_by_domain(const struct domain *d)
> +{
> +    return (d == hardware_domain);
> +}
> +
> +/*
>   * Route an IRQ to a specific guest.
>   * For now only SPIs are assignable to the guest.
>   */
> @@ -449,7 +460,7 @@ int route_irq_to_guest(struct domain *d, unsigned int virq,
>  
>      spin_lock_irqsave(&desc->lock, flags);
>  
> -    if ( desc->arch.type == IRQ_TYPE_INVALID )
> +    if ( !irq_type_set_by_domain(d) && desc->arch.type == IRQ_TYPE_INVALID )
>      {
>          printk(XENLOG_G_ERR "IRQ %u has not been configured\n", irq);
>          retval = -EIO;
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 5070452..a7ccfe7 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -344,6 +344,22 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
>      }
>  }
>  
> +#define VGIC_ICFG_MASK(intr) (1 << ((2 * ((intr) % 16)) + 1))
> +
> +/* The function should be called with the rank lock taken */
> +static inline unsigned int vgic_get_virq_type(struct vcpu *v, int n, int index)
> +{
> +    struct vgic_irq_rank *r = vgic_get_rank(v, n);
> +    uint32_t tr = r->icfg[index >> 4];
> +
> +    ASSERT(spin_is_locked(&r->lock));
> +
> +    if ( tr & VGIC_ICFG_MASK(index) )
> +        return IRQ_TYPE_EDGE_RISING;
> +    else
> +        return IRQ_TYPE_LEVEL_HIGH;
> +}
> +
>  void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>  {
>      const unsigned long mask = r;
> @@ -352,6 +368,7 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>      unsigned long flags;
>      int i = 0;
>      struct vcpu *v_target;
> +    struct domain *d = v->domain;
>  
>      while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
>          irq = i + (32 * n);
> @@ -366,6 +383,8 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>          {
>              irq_set_affinity(p->desc, cpumask_of(v_target->processor));
>              spin_lock_irqsave(&p->desc->lock, flags);
> +            if ( irq_type_set_by_domain(d) )
> +                gic_set_irq_type(p->desc, vgic_get_virq_type(v, n, i));

The patch looks good, but we should probably set the type only for irq >= 32.


>              p->desc->handler->enable(p->desc);
>              spin_unlock_irqrestore(&p->desc->lock, flags);
>          }
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 2214e87..836a103 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -222,6 +222,9 @@ enum gic_version {
>  
>  extern enum gic_version gic_hw_version(void);
>  
> +/* Program the IRQ type into the GIC */
> +void gic_set_irq_type(struct irq_desc *desc, unsigned int type);
> +
>  /* Program the GIC to route an interrupt */
>  extern void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority);
>  extern int gic_route_irq_to_guest(struct domain *, unsigned int virq,
> diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
> index 493773c..8f7a167 100644
> --- a/xen/include/asm-arm/irq.h
> +++ b/xen/include/asm-arm/irq.h
> @@ -58,6 +58,12 @@ int platform_get_irq(const struct dt_device_node *device, int index);
>  
>  void irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask);
>  
> +/*
> + * Use this helper in places that need to know whether the IRQ type is
> + * set by the domain.
> + */
> +bool_t irq_type_set_by_domain(const struct domain *d);
> +
>  #endif /* _ASM_HW_IRQ_H */
>  /*
>   * Local variables:
> -- 
> 1.9.1
> 

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

Re: [PATCH v2 9/9] xen/arm: Fix coding style and update comment in acpi_route_spis

[Stefano Stabellini]

On Thu, 14 Jul 2016, Julien Grall wrote:
> The comment was not correctly indented. Also the preferred name for the
> initial domain is "hardware domain" and not "dom0, so replace it.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

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


> ---
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/domain_build.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 5b2f8ad..35ab08d 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1355,8 +1355,9 @@ static int acpi_route_spis(struct domain *d)
>      for( i = NR_LOCAL_IRQS; i < vgic_num_irqs(d); i++ )
>      {
>          /*
> -	 * TODO: Exclude the SPIs SMMU uses which should not be routed to Dom0.
> -	 */
> +         * TODO: Exclude the SPIs SMMU uses which should not be routed to
> +         * the hardware domain.
> +         */
>          desc = irq_to_desc(i);
>          if ( desc->action != NULL)
>              continue;
> -- 
> 1.9.1
> 

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

Re: [PATCH v2 8/9] xen/arm: acpi: route all unused IRQs to DOM0

[Stefano Stabellini]

On Thu, 14 Jul 2016, Julien Grall wrote:
> It is not possible to know which IRQs will be used by DOM0 when ACPI is
> inuse. The approach implemented by this patch, will route all unused
> IRQs to DOM0 before it has booted.
> 
> The number of IRQs routed is based on the maximum SPIs supported by the
> hardware (up to ~1000). However, some of them might not be wired. So we
> would allocate resource for nothing.
> 
> For each IRQ routed, Xen is allocating memory for irqaction (40 bytes)
> and irq_guest (16 bytes). So in the worst case scenario ~54KB of memory
> will be allocated. Given that ACPI will mostly be used by server, I
> think it is a small drawback.
> 
> map_irq_to_domain is slightly reworked to remove the dependency on
> device-tree. So the function can be also be used for ACPI and will
> avoid code duplication.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

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


> ---
>     Changes in v2:
>         - Rename acpi_permit_spi_access to acpi_route_spis
>         - Update the comment in the function acpi_route_spis
> ---
>  xen/arch/arm/domain_build.c | 28 ++++++++++++----------------
>  1 file changed, 12 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 60db9e4..5b2f8ad 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -903,11 +903,10 @@ static int make_timer_node(const struct domain *d, void *fdt,
>      return res;
>  }
>  
> -static int map_irq_to_domain(const struct dt_device_node *dev,
> -                             struct domain *d, unsigned int irq)
> +static int map_irq_to_domain(struct domain *d, unsigned int irq,
> +                             bool_t need_mapping, const char *devname)
>  
>  {
> -    bool_t need_mapping = !dt_device_for_passthrough(dev);
>      int res;
>  
>      res = irq_permit_access(d, irq);
> @@ -927,7 +926,7 @@ static int map_irq_to_domain(const struct dt_device_node *dev,
>           */
>          vgic_reserve_virq(d, irq);
>  
> -        res = route_irq_to_guest(d, irq, irq, dt_node_name(dev));
> +        res = route_irq_to_guest(d, irq, irq, devname);
>          if ( res < 0 )
>          {
>              printk(XENLOG_ERR "Unable to map IRQ%"PRId32" to dom%d\n",
> @@ -947,6 +946,7 @@ static int map_dt_irq_to_domain(const struct dt_device_node *dev,
>      struct domain *d = data;
>      unsigned int irq = dt_irq->irq;
>      int res;
> +    bool_t need_mapping = !dt_device_for_passthrough(dev);
>  
>      if ( irq < NR_LOCAL_IRQS )
>      {
> @@ -965,7 +965,7 @@ static int map_dt_irq_to_domain(const struct dt_device_node *dev,
>          return res;
>      }
>  
> -    res = map_irq_to_domain(dev, d, irq);
> +    res = map_irq_to_domain(d, irq, need_mapping, dt_node_name(dev));
>  
>      return 0;
>  }
> @@ -1103,7 +1103,7 @@ static int handle_device(struct domain *d, struct dt_device_node *dev)
>              return res;
>          }
>  
> -        res = map_irq_to_domain(dev, d, res);
> +        res = map_irq_to_domain(d, res, need_mapping, dt_node_name(dev));
>          if ( res )
>              return res;
>      }
> @@ -1343,15 +1343,14 @@ static int acpi_iomem_deny_access(struct domain *d)
>      return gic_iomem_deny_access(d);
>  }
>  
> -static int acpi_permit_spi_access(struct domain *d)
> +static int acpi_route_spis(struct domain *d)
>  {
>      int i, res;
>      struct irq_desc *desc;
>  
>      /*
> -     * Here just permit Dom0 to access the SPIs which Xen doesn't use. Then when
> -     * Dom0 configures the interrupt, set the interrupt type and route it to
> -     * Dom0.
> +     * Route the IRQ to hardware domain and permit the access.
> +     * The interrupt type will be set by set by the hardware domain.
>       */
>      for( i = NR_LOCAL_IRQS; i < vgic_num_irqs(d); i++ )
>      {
> @@ -1362,13 +1361,10 @@ static int acpi_permit_spi_access(struct domain *d)
>          if ( desc->action != NULL)
>              continue;
>  
> -        res = irq_permit_access(d, i);
> +        /* XXX: Shall we use a proper devname? */
> +        res = map_irq_to_domain(d, i, true, "ACPI");
>          if ( res )
> -        {
> -            printk(XENLOG_ERR "Unable to permit to dom%u access to IRQ %u\n",
> -                   d->domain_id, i);
>              return res;
> -        }
>      }
>  
>      return 0;
> @@ -1902,7 +1898,7 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>      if ( rc != 0 )
>          return rc;
>  
> -    rc = acpi_permit_spi_access(d);
> +    rc = acpi_route_spis(d);
>      if ( rc != 0 )
>          return rc;
>  
> -- 
> 1.9.1
> 

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

Re: [PATCH v2 7/9] xen/arm: Allow DOM0 to set the IRQ type

[Julien Grall]

Hi Stefano,

On 20/07/2016 00:43, Stefano Stabellini wrote:
> On Thu, 14 Jul 2016, Julien Grall wrote:
>>  void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>>  {
>>      const unsigned long mask = r;
>> @@ -352,6 +368,7 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>>      unsigned long flags;
>>      int i = 0;
>>      struct vcpu *v_target;
>> +    struct domain *d = v->domain;
>>
>>      while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
>>          irq = i + (32 * n);
>> @@ -366,6 +383,8 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>>          {
>>              irq_set_affinity(p->desc, cpumask_of(v_target->processor));
>>              spin_lock_irqsave(&p->desc->lock, flags);
>> +            if ( irq_type_set_by_domain(d) )
>> +                gic_set_irq_type(p->desc, vgic_get_virq_type(v, n, i));
>
> The patch looks good, but we should probably set the type only for irq >= 32.

It is not possible to route PPIs to a guest for the moment. I remembered 
that Ian Campbell sent a series to route PPIs to the guest [1]. I would 
have to look whether we still want this series upstream (I guess so).

For the time being, I would prefer to add an ASSERT(irq >= 32). We could 
handle PPI properly when it will be supported.

Cheers,

[1] https://lists.xen.org/archives/html/xen-devel/2015-11/msg00921.html

-- 
Julien Grall

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

Re: [PATCH v2 7/9] xen/arm: Allow DOM0 to set the IRQ type

[Stefano Stabellini]

On Wed, 20 Jul 2016, Julien Grall wrote:
> Hi Stefano,
> 
> On 20/07/2016 00:43, Stefano Stabellini wrote:
> > On Thu, 14 Jul 2016, Julien Grall wrote:
> > >  void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
> > >  {
> > >      const unsigned long mask = r;
> > > @@ -352,6 +368,7 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int
> > > n)
> > >      unsigned long flags;
> > >      int i = 0;
> > >      struct vcpu *v_target;
> > > +    struct domain *d = v->domain;
> > > 
> > >      while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
> > >          irq = i + (32 * n);
> > > @@ -366,6 +383,8 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int
> > > n)
> > >          {
> > >              irq_set_affinity(p->desc, cpumask_of(v_target->processor));
> > >              spin_lock_irqsave(&p->desc->lock, flags);
> > > +            if ( irq_type_set_by_domain(d) )
> > > +                gic_set_irq_type(p->desc, vgic_get_virq_type(v, n, i));
> > 
> > The patch looks good, but we should probably set the type only for irq >=
> > 32.
> 
> It is not possible to route PPIs to a guest for the moment. I remembered that
> Ian Campbell sent a series to route PPIs to the guest [1]. I would have to
> look whether we still want this series upstream (I guess so).
> 
> For the time being, I would prefer to add an ASSERT(irq >= 32). We could
> handle PPI properly when it will be supported.

Sure.

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

Re: [PATCH v2 0/9] xen/arm: Support SPIs routing

[Julien Grall]

On 14/07/16 19:17, Shanker Donthineni wrote:
> Hi Julien,

Hello Shanker,

> Tested-by: Shanker Donthineni<shankerd@codeaurora.org>
>
> I have tested this patchset on Qualcomm Technologies QDF2XXX server
> platform without any issue.

Thank you for testing this series on your platform. I have added your 
tested-by on patch #7 and #8 which contain meat of this series.

Regards,

-- 
Julien Grall

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