Hi Andre,

On 09/02/18 14:39, Andre Przywara wrote:
When playing around with hardware mapped, level triggered virtual IRQs,
there is the need to explicitly set the active state of an interrupt at
some point in time.
To prepare the GIC for that, we introduce a set_active_state() function
to let the VGIC manipulate the state of an associated hardware IRQ.

Signed-off-by: Andre Przywara <andre.przyw...@linaro.org>
---
  xen/arch/arm/gic-v2.c     |  9 +++++++++
  xen/arch/arm/gic-v3.c     | 16 ++++++++++++++++
  xen/arch/arm/gic.c        |  5 +++++
  xen/include/asm-arm/gic.h |  5 +++++
  4 files changed, 35 insertions(+)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 2e35892881..5339f69fbc 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -235,6 +235,14 @@ static unsigned int gicv2_read_irq(void)
      return (readl_gicc(GICC_IAR) & GICC_IA_IRQ);
  }
+static void gicv2_set_active_state(int irq, bool active)

I would much prefer to have an irq_desc in parameter. This is matching the other interface and you could update the flags such as _IRQ_INPROGRESS which you don't do at the moment.

Also, who is preventing two CPUs to clear the active bit at the same time?

+{
+    if (active)
+        writel_gicd(1U << (irq % 32), GICD_ISACTIVER + (irq / 32) * 4);
+    else
+        writel_gicd(1U << (irq % 32), GICD_ICACTIVER + (irq / 32) * 4);

You will have a few places in the code usually similar construct. It would make sense to introduce a helper poke as we have in the GICv3 code.

+}
+
  static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int type)
  {
      uint32_t cfg, actual, edgebit;
@@ -1241,6 +1249,7 @@ const static struct gic_hw_operations gicv2_ops = {
      .eoi_irq             = gicv2_eoi_irq,
      .deactivate_irq      = gicv2_dir_irq,
      .read_irq            = gicv2_read_irq,
+    .set_active_state    = gicv2_set_active_state,
      .set_irq_type        = gicv2_set_irq_type,
      .set_irq_priority    = gicv2_set_irq_priority,
      .send_SGI            = gicv2_send_SGI,
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 08d4703687..595eaef43a 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -475,6 +475,21 @@ static unsigned int gicv3_read_irq(void)
      return irq;
  }
+static void gicv3_set_active_state(int irq, bool active)
+{
+    void __iomem *base;
+
+    if ( irq >= NR_GIC_LOCAL_IRQS)
+        base = GICD + (irq / 32) * 4;
+    else
+        base = GICD_RDIST_SGI_BASE;
+
+    if ( active )
+        writel(1U << (irq % 32), base + GICD_ISACTIVER);
+    else
+        writel(1U << (irq % 32), base + GICD_ICACTIVER);

Shouldn't you wait until RWP bits is cleared here?

+}

Why don't you use the function poke?

+
  static inline uint64_t gicv3_mpidr_to_affinity(int cpu)
  {
       uint64_t mpidr = cpu_logical_map(cpu);
@@ -1722,6 +1737,7 @@ static const struct gic_hw_operations gicv3_ops = {
      .eoi_irq             = gicv3_eoi_irq,
      .deactivate_irq      = gicv3_dir_irq,
      .read_irq            = gicv3_read_irq,
+    .set_active_state    = gicv3_set_active_state,
      .set_irq_type        = gicv3_set_irq_type,
      .set_irq_priority    = gicv3_set_irq_priority,
      .send_SGI            = gicv3_send_sgi,
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 89873c1df4..dfc2108c4d 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -92,6 +92,11 @@ void gic_restore_state(struct vcpu *v)
      isb();
  }
+void gic_set_active_state(int irq, bool state)
+{
+    gic_hw_ops->set_active_state(irq, state);
+}
+
  /* desc->irq needs to be disabled before calling this function */
  void gic_set_irq_type(struct irq_desc *desc, unsigned int type)
  {
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index c4c68c7770..d330860580 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -238,6 +238,9 @@ DECLARE_PER_CPU(uint64_t, lr_mask);
  extern enum gic_version gic_hw_version(void);
  extern int gic_get_nr_lrs(void);
+/* Force the state of an IRQ to active. */
+void gic_set_active_state(int irq, bool state);
+
  /* Program the IRQ type into the GIC */
  void gic_set_irq_type(struct irq_desc *desc, unsigned int type);
@@ -347,6 +350,8 @@ struct gic_hw_operations {
      void (*deactivate_irq)(struct irq_desc *irqd);
      /* Read IRQ id and Ack */
      unsigned int (*read_irq)(void);
+    /* Force the state of an IRQ to active */
+    void (*set_active_state)(int irq, bool state);
      /* Set IRQ type */
      void (*set_irq_type)(struct irq_desc *desc, unsigned int type);
      /* Set IRQ priority */


Cheers,

--
Julien Grall

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

Reply via email to