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

Re: [Xen-devel] [PATCH v6 13/31] xen/arm: ITS: implement hw_irq_controller for LPIs

Hi Vijay,

On 31/08/2015 12:06, vijay.kilari@xxxxxxxxx wrote:
From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>

Implements hw_irq_controller api's required
to handle LPI's.

Changed callbacks gic_host_irq_type and gic_guest_irq_type

s/Changed/Change the/

to gic_get_host_irq_type and gic_get_guest_irq_type
in gic_hw_operations, which returns
hw_irq_controller based on irq type (SPI or LPI).

Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
CC: Zoltan Kiss <zoltan.kiss@xxxxxxxxxx>
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index f14c0f4..0865a93 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -349,6 +349,19 @@ post:
      its_wait_for_range_completion(its, cmd, next_cmd);

+static void its_send_inv(struct its_device *dev, u32 event)

See my point of view of about this function living here in my answer to patch #6.


+static void its_host_irq_end(struct irq_desc *desc)
+    /* Lower the priority */
+    gicv3_eoi_irq(desc);
+    /* LPIs does not have active state. Do not deactivate */
+static void its_guest_irq_end(struct irq_desc *desc)
+    gicv3_eoi_irq(desc);
+static void its_irq_set_affinity(struct irq_desc *desc, const cpumask_t *mask)
+    /*TODO: Yet to support */
+    printk(XENLOG_G_WARNING
+           "%pv: ITS: Setting Affinity of LPI is not supported\n", current);

Printing the current vCPU is hazardous, this function can be called at any time and the current may not be meaningful.

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index e90e0cc..c3b1a7c 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -40,6 +40,7 @@
  #include <asm/device.h>
  #include <asm/gic.h>
  #include <asm/gic_v3_defs.h>
+#include <asm/gic-its.h>
  #include <asm/cpufeature.h>

  /* Global state */
@@ -54,6 +55,8 @@ static struct {
  } gicv3;

  static struct gic_info gicv3_info;
+extern const hw_irq_controller its_host_lpi_type;
+extern const hw_irq_controller its_guest_lpi_type;

You should define the variable in gic-its.h in order to let the compiler check that we are using the same as the declaration.

  /* per-cpu re-distributor base */
  DEFINE_PER_CPU(struct rdist, rdist);


diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h
index 3599c76..7a46e21 100644
--- a/xen/include/asm-arm/gic-its.h
+++ b/xen/include/asm-arm/gic-its.h
@@ -279,6 +279,7 @@ void irqdesc_set_lpi_event(struct irq_desc *desc, unsigned 
  unsigned int irqdesc_get_lpi_event(struct irq_desc *desc);
  struct its_device *irqdesc_get_its_device(struct irq_desc *desc);
  void irqdesc_set_its_device(struct irq_desc *desc, struct its_device *dev);
+bool_t is_valid_collection(struct domain *d, uint32_t col);

Where does it come from? I don't see any usage nor implementation of it within the patch.

  int its_init(struct rdist_prop *rdists);
  int its_cpu_init(void);
  int its_add_device(u32 devid, u32 nr_ites, struct dt_device_node *dt_its);
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index d39e1b3..6ece7cc 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -317,10 +317,10 @@ struct gic_hw_operations {
      void (*dump_state)(const struct vcpu *);

      /* hw_irq_controller to enable/disable/eoi host irq */
-    hw_irq_controller *gic_host_irq_type;
+    const hw_irq_controller *(*gic_get_host_irq_type)(unsigned int irq);

      /* hw_irq_controller to enable/disable/eoi guest irq */
-    hw_irq_controller *gic_guest_irq_type;
+    const hw_irq_controller *(*gic_get_guest_irq_type)(unsigned int irq);

Actually the const was already embedded in hw_irq_controller (see the typedef in xen/include/xen/irq.h). Sorry for the inconvenience.

      /* End of Interrupt */
      void (*eoi_irq)(struct irq_desc *irqd);


Julien Grall

Xen-devel mailing list



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