All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform
@ 2015-02-20  9:56 Frediano Ziglio
  2015-02-20  9:56 ` [PATCH v5 1/4] xen/arm: Make gic-v2 code handle " Frediano Ziglio
                   ` (4 more replies)
  0 siblings, 5 replies; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-20  9:56 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini, Tim Deegan, Julien Grall,
	frediano.ziglio
  Cc: zoltan.kiss, xen-devel

This set of patches add Xen support for hip04-d01 platform (see https://wiki.linaro.org/Boards/D01 for details).

Changes from v4:
- rebased to new version;
- removed patch for computing GIC addresses as it apply to all platforms;
- removed patches to platform (cpu and system operations) as now they can
  use a bootwrapper which provide them.

Changes from v3:
- change the way regs property is computed for GICv2 (Julien Grall);
- revert order of compaible names for GIC (Julien Grall).

Changes from v2:
- rewrote DTS fix patch (Ian Campbell);
- use is_hip04 macro instead of doing explicit test (Julien Grall);
- do not use quirks to distinguish this platform (Ian Cambell);
- move some GIC constants to C files instead of header (Julien Grall);
- minor changes (Julien Grall).

Changes from v1:
- style (Julien Grall);
- make gicv2_send_SGI faster (Julien Grall);
- cleanup correctly if hip04_smp_init fails (Julien Grall);
- remove quirks using compatibility (Ian Campbell);
- other minor suggestions by Julien Grall.

Frediano Ziglio (4):
  xen/arm: Make gic-v2 code handle hip04-d01 platform
  xen/arm: Add support for DTBs with strange names of Hip04 GICv2
  xen/arm: handle GICH register changes for hip04-d01 platform
  xen/arm: Force dom0 to use normal GICv2 driver on Hip04 platform

 xen/arch/arm/gic-v2.c     | 119 ++++++++++++++++++++++++++++++++++++----------
 xen/arch/arm/gic.c        |   3 +-
 xen/include/asm-arm/gic.h |   6 ++-
 3 files changed, 101 insertions(+), 27 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v5 1/4] xen/arm: Make gic-v2 code handle hip04-d01 platform
  2015-02-20  9:56 [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform Frediano Ziglio
@ 2015-02-20  9:56 ` Frediano Ziglio
  2015-02-24 16:16   ` Ian Campbell
  2015-02-20  9:56 ` [PATCH v5 2/4] xen/arm: Add support for DTBs with strange names of Hip04 GICv2 Frediano Ziglio
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-20  9:56 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini, Tim Deegan, Julien Grall,
	frediano.ziglio
  Cc: zoltan.kiss, xen-devel

The GIC in this platform is mainly compatible with the standard
GICv2 beside:
- ITARGET is extended to 16 bit to support 16 CPUs;
- SGI mask is extended to support 16 CPUs;
- maximum supported interrupt is 510.

Use nr_lines to check for maximum irq supported. hip04-d01 support less
interrupts due to some field restriction. Any value above this is already
an error.

Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
Signed-off-by: Zoltan Kiss <zoltan.kiss@huawei.com>
---
 xen/arch/arm/gic-v2.c     | 85 ++++++++++++++++++++++++++++++++++++++---------
 xen/arch/arm/gic.c        |  3 +-
 xen/include/asm-arm/gic.h |  4 ++-
 3 files changed, 75 insertions(+), 17 deletions(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 4d1924e..7f5cfcb 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -79,16 +79,25 @@ static struct gic_info gicv2_info;
  * logical CPU numbering. Let's use mapping as returned by the GIC
  * itself
  */
-static DEFINE_PER_CPU(u8, gic_cpu_id);
+static DEFINE_PER_CPU(u16, gic_cpu_id);
 
 /* Maximum cpu interface per GIC */
-#define NR_GIC_CPU_IF 8
+static unsigned int nr_gic_cpu_if = 8;
+static unsigned int gicd_sgi_target_shift = GICD_SGI_TARGET_SHIFT;
+static unsigned int gic_cpu_mask = 0xff;
+
+#define is_hip04() (nr_gic_cpu_if == 16)
 
 static inline void writeb_gicd(uint8_t val, unsigned int offset)
 {
     writeb_relaxed(val, gicv2.map_dbase + offset);
 }
 
+static inline void writew_gicd(uint16_t val, unsigned int offset)
+{
+    writew_relaxed(val, gicv2.map_dbase + offset);
+}
+
 static inline void writel_gicd(uint32_t val, unsigned int offset)
 {
     writel_relaxed(val, gicv2.map_dbase + offset);
@@ -132,7 +141,7 @@ static unsigned int gicv2_cpu_mask(const cpumask_t *cpumask)
     cpumask_and(&possible_mask, cpumask, &cpu_possible_map);
     for_each_cpu( cpu, &possible_mask )
     {
-        ASSERT(cpu < NR_GIC_CPU_IF);
+        ASSERT(cpu < nr_gic_cpu_if);
         mask |= per_cpu(gic_cpu_id, cpu);
     }
 
@@ -203,6 +212,15 @@ static unsigned int gicv2_read_irq(void)
     return (readl_gicc(GICC_IAR) & GICC_IA_IRQ);
 }
 
+/* Set target CPU mask (RAZ/WI on uniprocessor) */
+static void gicv2_set_irq_mask(int irq, unsigned int mask)
+{
+    if ( is_hip04() )
+        writew_gicd(mask, GICD_ITARGETSR + irq * 2);
+    else
+        writeb_gicd(mask, GICD_ITARGETSR + irq);
+}
+
 /*
  * needs to be called with a valid cpu_mask, ie each cpu in the mask has
  * already called gic_cpu_init
@@ -230,7 +248,7 @@ static void gicv2_set_irq_properties(struct irq_desc *desc,
     writel_gicd(cfg, GICD_ICFGR + (irq / 16) * 4);
 
     /* Set target CPU mask (RAZ/WI on uniprocessor) */
-    writeb_gicd(mask, GICD_ITARGETSR + irq);
+    gicv2_set_irq_mask(irq, mask);
     /* Set priority */
     writeb_gicd(priority, GICD_IPRIORITYR + irq);
 
@@ -244,16 +262,21 @@ static void __init gicv2_dist_init(void)
     uint32_t gic_cpus;
     int i;
 
-    cpumask = readl_gicd(GICD_ITARGETSR) & 0xff;
-    cpumask |= cpumask << 8;
-    cpumask |= cpumask << 16;
+    cpumask = readl_gicd(GICD_ITARGETSR) & gic_cpu_mask;
 
     /* Disable the distributor */
     writel_gicd(0, GICD_CTLR);
 
     type = readl_gicd(GICD_TYPER);
     gicv2_info.nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
-    gic_cpus = 1 + ((type & GICD_TYPE_CPUS) >> 5);
+    if ( is_hip04() )
+    {
+        gic_cpus = 16;
+    }
+    else
+    {
+        gic_cpus = 1 + ((type & GICD_TYPE_CPUS) >> 5);
+    }
     printk("GICv2: %d lines, %d cpu%s%s (IID %8.8x).\n",
            gicv2_info.nr_lines, gic_cpus, (gic_cpus == 1) ? "" : "s",
            (type & GICD_TYPE_SEC) ? ", secure" : "",
@@ -264,8 +287,19 @@ static void __init gicv2_dist_init(void)
         writel_gicd(0x0, GICD_ICFGR + (i / 16) * 4);
 
     /* Route all global IRQs to this CPU */
-    for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
-        writel_gicd(cpumask, GICD_ITARGETSR + (i / 4) * 4);
+    if ( is_hip04() )
+    {
+        cpumask |= cpumask << 16;
+        for ( i = 32; i < gicv2_info.nr_lines; i += 2 )
+            writel_gicd(cpumask, GICD_ITARGETSR + (i / 2) * 4);
+    }
+    else
+    {
+        cpumask |= cpumask << 8;
+        cpumask |= cpumask << 16;
+        for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
+            writel_gicd(cpumask, GICD_ITARGETSR + (i / 4) * 4);
+    }
 
     /* Default priority for global interrupts */
     for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
@@ -285,7 +319,7 @@ static void __cpuinit gicv2_cpu_init(void)
 {
     int i;
 
-    this_cpu(gic_cpu_id) = readl_gicd(GICD_ITARGETSR) & 0xff;
+    this_cpu(gic_cpu_id) = readl_gicd(GICD_ITARGETSR) & gic_cpu_mask;
 
     /* The first 32 interrupts (PPI and SGI) are banked per-cpu, so
      * even though they are controlled with GICD registers, they must
@@ -366,7 +400,7 @@ static void gicv2_send_SGI(enum gic_sgi sgi, enum gic_sgi_mode irqmode,
         cpumask_and(&online_mask, cpu_mask, &cpu_online_map);
         mask = gicv2_cpu_mask(&online_mask);
         writel_gicd(GICD_SGI_TARGET_LIST |
-                    (mask << GICD_SGI_TARGET_SHIFT) | sgi,
+                    (mask << gicd_sgi_target_shift) | sgi,
                     GICD_SGIR);
         break;
     default:
@@ -579,7 +613,7 @@ static void gicv2_irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_m
     mask = gicv2_cpu_mask(cpu_mask);
 
     /* Set target CPU mask (RAZ/WI on uniprocessor) */
-    writeb_gicd(mask, GICD_ITARGETSR + desc->irq);
+    gicv2_set_irq_mask(desc->irq, mask);
 
     spin_unlock(&gicv2.lock);
 }
@@ -682,7 +716,7 @@ const static struct gic_hw_operations gicv2_ops = {
 };
 
 /* Set up the GIC */
-static int __init gicv2_init(struct dt_device_node *node, const void *data)
+static int __init gicv2_init_default(struct dt_device_node *node, const void *data)
 {
     int res;
 
@@ -762,6 +796,15 @@ static int __init gicv2_init(struct dt_device_node *node, const void *data)
     return 0;
 }
 
+static int __init hip04_gicv2_init(struct dt_device_node *node, const void *data)
+{
+    nr_gic_cpu_if = 16;
+    gicd_sgi_target_shift = 8;
+    gic_cpu_mask = 0xffff;
+
+    return gicv2_init_default(node, data);
+}
+
 static const char * const gicv2_dt_compat[] __initconst =
 {
     DT_COMPAT_GIC_CORTEX_A15,
@@ -772,9 +815,21 @@ static const char * const gicv2_dt_compat[] __initconst =
 
 DT_DEVICE_START(gicv2, "GICv2:", DEVICE_GIC)
         .compatible = gicv2_dt_compat,
-        .init = gicv2_init,
+        .init = gicv2_init_default,
 DT_DEVICE_END
 
+static const char * const hip04_gicv2_dt_compat[] __initconst =
+{
+    DT_COMPAT_GIC_HIP04,
+    NULL
+};
+
+DT_DEVICE_START(hip04_gicv2, "GICv2:", DEVICE_GIC)
+        .compatible = hip04_gicv2_dt_compat,
+        .init = hip04_gicv2_init,
+DT_DEVICE_END
+
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 390c8b0..e4512a8 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -565,12 +565,13 @@ static void do_sgi(struct cpu_user_regs *regs, enum gic_sgi sgi)
 void gic_interrupt(struct cpu_user_regs *regs, int is_fiq)
 {
     unsigned int irq;
+    unsigned int max_irq = gic_hw_ops->info->nr_lines;
 
     do  {
         /* Reading IRQ will ACK it */
         irq = gic_hw_ops->read_irq();
 
-        if ( likely(irq >= 16 && irq < 1021) )
+        if ( likely(irq >= 16 && irq < max_irq) )
         {
             local_irq_enable();
             do_IRQ(regs, irq, is_fiq);
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 187dc46..c880867 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -155,10 +155,12 @@
 #define DT_COMPAT_GIC_400            "arm,gic-400"
 #define DT_COMPAT_GIC_CORTEX_A15     "arm,cortex-a15-gic"
 #define DT_COMPAT_GIC_CORTEX_A7      "arm,cortex-a7-gic"
+#define DT_COMPAT_GIC_HIP04          "hisilicon,hip04-intc"
 
 #define DT_MATCH_GIC_V2 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A15), \
                         DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A7), \
-                        DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_400)
+                        DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_400), \
+                        DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_HIP04)
 
 #define DT_COMPAT_GIC_V3             "arm,gic-v3"
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5 2/4] xen/arm: Add support for DTBs with strange names of Hip04 GICv2
  2015-02-20  9:56 [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform Frediano Ziglio
  2015-02-20  9:56 ` [PATCH v5 1/4] xen/arm: Make gic-v2 code handle " Frediano Ziglio
@ 2015-02-20  9:56 ` Frediano Ziglio
  2015-02-24 14:19   ` Julien Grall
  2015-02-24 14:19   ` Julien Grall
  2015-02-20  9:56 ` [PATCH v5 3/4] xen/arm: handle GICH register changes for hip04-d01 platform Frediano Ziglio
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-20  9:56 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini, Tim Deegan, Julien Grall,
	frediano.ziglio
  Cc: zoltan.kiss, xen-devel

This name can appear in some Linux kernel repos. Not very fortunate,
but to avoid others spending an hour to spot that few characters
difference it worth to work around it.

Signed-off-by: Zoltan Kiss <zoltan.kiss@huawei.com>
---
 xen/arch/arm/gic-v2.c     | 1 +
 xen/include/asm-arm/gic.h | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 7f5cfcb..13c038e 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -821,6 +821,7 @@ DT_DEVICE_END
 static const char * const hip04_gicv2_dt_compat[] __initconst =
 {
     DT_COMPAT_GIC_HIP04,
+    DT_COMPAT_GIC_HIP04_2,
     NULL
 };
 
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index c880867..f8ba227 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -156,11 +156,13 @@
 #define DT_COMPAT_GIC_CORTEX_A15     "arm,cortex-a15-gic"
 #define DT_COMPAT_GIC_CORTEX_A7      "arm,cortex-a7-gic"
 #define DT_COMPAT_GIC_HIP04          "hisilicon,hip04-intc"
+#define DT_COMPAT_GIC_HIP04_2        "hisilicon,hip04-gic"
 
 #define DT_MATCH_GIC_V2 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A15), \
                         DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A7), \
                         DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_400), \
-                        DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_HIP04)
+                        DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_HIP04), \
+                        DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_HIP04_2)
 
 #define DT_COMPAT_GIC_V3             "arm,gic-v3"
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5 3/4] xen/arm: handle GICH register changes for hip04-d01 platform
  2015-02-20  9:56 [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform Frediano Ziglio
  2015-02-20  9:56 ` [PATCH v5 1/4] xen/arm: Make gic-v2 code handle " Frediano Ziglio
  2015-02-20  9:56 ` [PATCH v5 2/4] xen/arm: Add support for DTBs with strange names of Hip04 GICv2 Frediano Ziglio
@ 2015-02-20  9:56 ` Frediano Ziglio
  2015-02-20  9:56 ` [PATCH v5 4/4] xen/arm: Force dom0 to use normal GICv2 driver on Hip04 platform Frediano Ziglio
  2015-02-25 16:43 ` [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform Ian Campbell
  4 siblings, 0 replies; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-20  9:56 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini, Tim Deegan, Julien Grall,
	frediano.ziglio
  Cc: zoltan.kiss, xen-devel

The GICH in this platform is mainly compatible with the standard
GICv2 beside APR and LR register offsets.

Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
---
 xen/arch/arm/gic-v2.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 13c038e..7f2040c 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -61,6 +61,9 @@
 #define GICH_V2_VMCR_PRIORITY_MASK   0x1f
 #define GICH_V2_VMCR_PRIORITY_SHIFT  27
 
+#define HIP04_GICH_APR  0x70
+#define HIP04_GICH_LR   0x80
+
 /* Global state */
 static struct {
     paddr_t dbase;            /* Address of distributor registers */
@@ -85,6 +88,8 @@ static DEFINE_PER_CPU(u16, gic_cpu_id);
 static unsigned int nr_gic_cpu_if = 8;
 static unsigned int gicd_sgi_target_shift = GICD_SGI_TARGET_SHIFT;
 static unsigned int gic_cpu_mask = 0xff;
+static unsigned int gich_apr = GICH_APR;
+static unsigned int gich_lr = GICH_LR;
 
 #define is_hip04() (nr_gic_cpu_if == 16)
 
@@ -157,9 +162,9 @@ static void gicv2_save_state(struct vcpu *v)
      * accessed simultaneously by another pCPU.
      */
     for ( i = 0; i < gicv2_info.nr_lrs; i++ )
-        v->arch.gic.v2.lr[i] = readl_gich(GICH_LR + i * 4);
+        v->arch.gic.v2.lr[i] = readl_gich(gich_lr + i * 4);
 
-    v->arch.gic.v2.apr = readl_gich(GICH_APR);
+    v->arch.gic.v2.apr = readl_gich(gich_apr);
     v->arch.gic.v2.vmcr = readl_gich(GICH_VMCR);
     /* Disable until next VCPU scheduled */
     writel_gich(0, GICH_HCR);
@@ -170,9 +175,9 @@ static void gicv2_restore_state(const struct vcpu *v)
     int i;
 
     for ( i = 0; i < gicv2_info.nr_lrs; i++ )
-        writel_gich(v->arch.gic.v2.lr[i], GICH_LR + i * 4);
+        writel_gich(v->arch.gic.v2.lr[i], gich_lr + i * 4);
 
-    writel_gich(v->arch.gic.v2.apr, GICH_APR);
+    writel_gich(v->arch.gic.v2.apr, gich_apr);
     writel_gich(v->arch.gic.v2.vmcr, GICH_VMCR);
     writel_gich(GICH_HCR_EN, GICH_HCR);
 }
@@ -185,7 +190,7 @@ static void gicv2_dump_state(const struct vcpu *v)
     {
         for ( i = 0; i < gicv2_info.nr_lrs; i++ )
             printk("   HW_LR[%d]=%x\n", i,
-                   readl_gich(GICH_LR + i * 4));
+                   readl_gich(gich_lr + i * 4));
     }
     else
     {
@@ -439,12 +444,12 @@ static void gicv2_update_lr(int lr, const struct pending_irq *p,
                             << GICH_V2_LR_PHYSICAL_SHIFT);
     }
 
-    writel_gich(lr_reg, GICH_LR + lr * 4);
+    writel_gich(lr_reg, gich_lr + lr * 4);
 }
 
 static void gicv2_clear_lr(int lr)
 {
-    writel_gich(0, GICH_LR + lr * 4);
+    writel_gich(0, gich_lr + lr * 4);
 }
 
 static int gicv2v_setup(struct domain *d)
@@ -492,7 +497,7 @@ static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
 {
     uint32_t lrv;
 
-    lrv          = readl_gich(GICH_LR + lr * 4);
+    lrv          = readl_gich(gich_lr + lr * 4);
     lr_reg->pirq = (lrv >> GICH_V2_LR_PHYSICAL_SHIFT) & GICH_V2_LR_PHYSICAL_MASK;
     lr_reg->virq = (lrv >> GICH_V2_LR_VIRTUAL_SHIFT) & GICH_V2_LR_VIRTUAL_MASK;
     lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) & GICH_V2_LR_PRIORITY_MASK;
@@ -515,7 +520,7 @@ static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg)
                                        << GICH_V2_LR_HW_SHIFT)  |
           ((uint32_t)(lr_reg->grp & GICH_V2_LR_GRP_MASK) << GICH_V2_LR_GRP_SHIFT) );
 
-    writel_gich(lrv, GICH_LR + lr * 4);
+    writel_gich(lrv, gich_lr + lr * 4);
 }
 
 static void gicv2_hcr_status(uint32_t flag, bool_t status)
@@ -538,7 +543,7 @@ static unsigned int gicv2_read_vmcr_priority(void)
 
 static unsigned int gicv2_read_apr(int apr_reg)
 {
-   return readl_gich(GICH_APR);
+   return readl_gich(gich_apr);
 }
 
 static void gicv2_irq_enable(struct irq_desc *desc)
@@ -801,6 +806,8 @@ static int __init hip04_gicv2_init(struct dt_device_node *node, const void *data
     nr_gic_cpu_if = 16;
     gicd_sgi_target_shift = 8;
     gic_cpu_mask = 0xffff;
+    gich_apr = HIP04_GICH_APR;
+    gich_lr = HIP04_GICH_LR;
 
     return gicv2_init_default(node, data);
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5 4/4] xen/arm: Force dom0 to use normal GICv2 driver on Hip04 platform
  2015-02-20  9:56 [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform Frediano Ziglio
                   ` (2 preceding siblings ...)
  2015-02-20  9:56 ` [PATCH v5 3/4] xen/arm: handle GICH register changes for hip04-d01 platform Frediano Ziglio
@ 2015-02-20  9:56 ` Frediano Ziglio
  2015-02-25 16:43 ` [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform Ian Campbell
  4 siblings, 0 replies; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-20  9:56 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini, Tim Deegan, Julien Grall,
	frediano.ziglio
  Cc: zoltan.kiss, xen-devel

Until vGIC support is not implemented and tested, this will prevent
guest kernels to use their Hip04 driver, or crash when they don't
have any.

Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
---
 xen/arch/arm/gic-v2.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 7f2040c..05dc729 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -639,6 +639,12 @@ static int gicv2_make_dt_node(const struct domain *d,
         return -FDT_ERR_XEN(ENOENT);
     }
 
+    if ( is_hip04() )
+    {
+        compatible = DT_COMPAT_GIC_CORTEX_A15;
+        len = strlen((char*) compatible) + 1;
+    }
+
     res = fdt_begin_node(fdt, "interrupt-controller");
     if ( res )
         return res;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/4] xen/arm: Add support for DTBs with strange names of Hip04 GICv2
  2015-02-20  9:56 ` [PATCH v5 2/4] xen/arm: Add support for DTBs with strange names of Hip04 GICv2 Frediano Ziglio
@ 2015-02-24 14:19   ` Julien Grall
  2015-02-24 16:13     ` Ian Campbell
  2015-02-24 14:19   ` Julien Grall
  1 sibling, 1 reply; 32+ messages in thread
From: Julien Grall @ 2015-02-24 14:19 UTC (permalink / raw)
  To: Frediano Ziglio, Ian Campbell, Stefano Stabellini, Tim Deegan
  Cc: zoltan.kiss, xen-devel

Hi Frediano,

On 20/02/15 09:56, Frediano Ziglio wrote:
> This name can appear in some Linux kernel repos. Not very fortunate,
> but to avoid others spending an hour to spot that few characters
> difference it worth to work around it.

As Zoltan said on a previous mail [1], this is only used on your
internal kernel.

Furthermore, as the bindings is not official, nothing prevent someone to
re-use the bindings for another purpose (such as GICv3 on hisilicon).

Xen upstream aims to support only official bindings for a this reason.
So I don't think we should take this patch.

BTW, it would have been nice add a link to the discussion [1]. It would
have avoid us to get back on the archives to find the reason of this patch.

Regards,

[1] http://lists.xen.org/archives/html/xen-devel/2014-11/msg00507.html

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/4] xen/arm: Add support for DTBs with strange names of Hip04 GICv2
  2015-02-20  9:56 ` [PATCH v5 2/4] xen/arm: Add support for DTBs with strange names of Hip04 GICv2 Frediano Ziglio
  2015-02-24 14:19   ` Julien Grall
@ 2015-02-24 14:19   ` Julien Grall
  1 sibling, 0 replies; 32+ messages in thread
From: Julien Grall @ 2015-02-24 14:19 UTC (permalink / raw)
  To: Frediano Ziglio, Ian Campbell, Stefano Stabellini, Tim Deegan
  Cc: zoltan.kiss, xen-devel

Hi Frediano,

On 20/02/15 09:56, Frediano Ziglio wrote:
> This name can appear in some Linux kernel repos. Not very fortunate,
> but to avoid others spending an hour to spot that few characters
> difference it worth to work around it.

As Zoltan said on a previous mail [1], this is only used on your
internal kernel.

Furthermore, as the bindings is not official, nothing prevent someone to
re-use the bindings for another purpose (such as GICv3 on hisilicon).

Xen upstream aims to support only official bindings for a this reason.
So I don't think we should take this patch.

BTW, it would have been nice add a link to the discussion [1]. It would
have avoid us to get back on the archives to find the reason of this patch.

Regards,

[1] http://lists.xen.org/archives/html/xen-devel/2014-11/msg00507.html

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/4] xen/arm: Add support for DTBs with strange names of Hip04 GICv2
  2015-02-24 14:19   ` Julien Grall
@ 2015-02-24 16:13     ` Ian Campbell
  2015-02-24 16:38       ` Frediano Ziglio
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Campbell @ 2015-02-24 16:13 UTC (permalink / raw)
  To: Julien Grall
  Cc: Frediano Ziglio, Tim Deegan, xen-devel, Stefano Stabellini, zoltan.kiss

On Tue, 2015-02-24 at 14:19 +0000, Julien Grall wrote:
> Hi Frediano,
> 
> On 20/02/15 09:56, Frediano Ziglio wrote:
> > This name can appear in some Linux kernel repos. Not very fortunate,
> > but to avoid others spending an hour to spot that few characters
> > difference it worth to work around it.
> 
> As Zoltan said on a previous mail [1], this is only used on your
> internal kernel.
> 
> Furthermore, as the bindings is not official, nothing prevent someone to
> re-use the bindings for another purpose (such as GICv3 on hisilicon).
> 
> Xen upstream aims to support only official bindings for a this reason.
> So I don't think we should take this patch.

Neither do I. Nacked-by: Ian Campbell <ian.campbell@citrix.com>.

If/when these bindings are described in the official bindings document
then please let us know.

Ian.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/4] xen/arm: Make gic-v2 code handle hip04-d01 platform
  2015-02-20  9:56 ` [PATCH v5 1/4] xen/arm: Make gic-v2 code handle " Frediano Ziglio
@ 2015-02-24 16:16   ` Ian Campbell
  2015-02-25 15:28     ` [PATCH v5.99.1 RFC 0/4] xen/arm: Add support for Huawei " Frediano Ziglio
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Campbell @ 2015-02-24 16:16 UTC (permalink / raw)
  To: Frediano Ziglio
  Cc: Tim Deegan, Julien Grall, Stefano Stabellini, zoltan.kiss, xen-devel

On Fri, 2015-02-20 at 09:56 +0000, Frediano Ziglio wrote:
> The GIC in this platform is mainly compatible with the standard
> GICv2 beside:
> - ITARGET is extended to 16 bit to support 16 CPUs;
> - SGI mask is extended to support 16 CPUs;
> - maximum supported interrupt is 510.

I'm afraid I'm still not keen on mangling our spec compliant gic v2
driver in this way.

I'd be far happier if this was done as a new driver for your
gicv2-a-like interrupt controller.

> Use nr_lines to check for maximum irq supported. hip04-d01 support less
> interrupts due to some field restriction. Any value above this is already
> an error.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> Signed-off-by: Zoltan Kiss <zoltan.kiss@huawei.com>
> ---
>  xen/arch/arm/gic-v2.c     | 85 ++++++++++++++++++++++++++++++++++++++---------
>  xen/arch/arm/gic.c        |  3 +-
>  xen/include/asm-arm/gic.h |  4 ++-
>  3 files changed, 75 insertions(+), 17 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 4d1924e..7f5cfcb 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -79,16 +79,25 @@ static struct gic_info gicv2_info;
>   * logical CPU numbering. Let's use mapping as returned by the GIC
>   * itself
>   */
> -static DEFINE_PER_CPU(u8, gic_cpu_id);
> +static DEFINE_PER_CPU(u16, gic_cpu_id);
>  
>  /* Maximum cpu interface per GIC */
> -#define NR_GIC_CPU_IF 8
> +static unsigned int nr_gic_cpu_if = 8;
> +static unsigned int gicd_sgi_target_shift = GICD_SGI_TARGET_SHIFT;
> +static unsigned int gic_cpu_mask = 0xff;
> +
> +#define is_hip04() (nr_gic_cpu_if == 16)
>  
>  static inline void writeb_gicd(uint8_t val, unsigned int offset)
>  {
>      writeb_relaxed(val, gicv2.map_dbase + offset);
>  }
>  
> +static inline void writew_gicd(uint16_t val, unsigned int offset)
> +{
> +    writew_relaxed(val, gicv2.map_dbase + offset);
> +}
> +
>  static inline void writel_gicd(uint32_t val, unsigned int offset)
>  {
>      writel_relaxed(val, gicv2.map_dbase + offset);
> @@ -132,7 +141,7 @@ static unsigned int gicv2_cpu_mask(const cpumask_t *cpumask)
>      cpumask_and(&possible_mask, cpumask, &cpu_possible_map);
>      for_each_cpu( cpu, &possible_mask )
>      {
> -        ASSERT(cpu < NR_GIC_CPU_IF);
> +        ASSERT(cpu < nr_gic_cpu_if);
>          mask |= per_cpu(gic_cpu_id, cpu);
>      }
>  
> @@ -203,6 +212,15 @@ static unsigned int gicv2_read_irq(void)
>      return (readl_gicc(GICC_IAR) & GICC_IA_IRQ);
>  }
>  
> +/* Set target CPU mask (RAZ/WI on uniprocessor) */
> +static void gicv2_set_irq_mask(int irq, unsigned int mask)
> +{
> +    if ( is_hip04() )
> +        writew_gicd(mask, GICD_ITARGETSR + irq * 2);
> +    else
> +        writeb_gicd(mask, GICD_ITARGETSR + irq);
> +}
> +
>  /*
>   * needs to be called with a valid cpu_mask, ie each cpu in the mask has
>   * already called gic_cpu_init
> @@ -230,7 +248,7 @@ static void gicv2_set_irq_properties(struct irq_desc *desc,
>      writel_gicd(cfg, GICD_ICFGR + (irq / 16) * 4);
>  
>      /* Set target CPU mask (RAZ/WI on uniprocessor) */
> -    writeb_gicd(mask, GICD_ITARGETSR + irq);
> +    gicv2_set_irq_mask(irq, mask);
>      /* Set priority */
>      writeb_gicd(priority, GICD_IPRIORITYR + irq);
>  
> @@ -244,16 +262,21 @@ static void __init gicv2_dist_init(void)
>      uint32_t gic_cpus;
>      int i;
>  
> -    cpumask = readl_gicd(GICD_ITARGETSR) & 0xff;
> -    cpumask |= cpumask << 8;
> -    cpumask |= cpumask << 16;
> +    cpumask = readl_gicd(GICD_ITARGETSR) & gic_cpu_mask;
>  
>      /* Disable the distributor */
>      writel_gicd(0, GICD_CTLR);
>  
>      type = readl_gicd(GICD_TYPER);
>      gicv2_info.nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
> -    gic_cpus = 1 + ((type & GICD_TYPE_CPUS) >> 5);
> +    if ( is_hip04() )
> +    {
> +        gic_cpus = 16;
> +    }
> +    else
> +    {
> +        gic_cpus = 1 + ((type & GICD_TYPE_CPUS) >> 5);
> +    }
>      printk("GICv2: %d lines, %d cpu%s%s (IID %8.8x).\n",
>             gicv2_info.nr_lines, gic_cpus, (gic_cpus == 1) ? "" : "s",
>             (type & GICD_TYPE_SEC) ? ", secure" : "",
> @@ -264,8 +287,19 @@ static void __init gicv2_dist_init(void)
>          writel_gicd(0x0, GICD_ICFGR + (i / 16) * 4);
>  
>      /* Route all global IRQs to this CPU */
> -    for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
> -        writel_gicd(cpumask, GICD_ITARGETSR + (i / 4) * 4);
> +    if ( is_hip04() )
> +    {
> +        cpumask |= cpumask << 16;
> +        for ( i = 32; i < gicv2_info.nr_lines; i += 2 )
> +            writel_gicd(cpumask, GICD_ITARGETSR + (i / 2) * 4);
> +    }
> +    else
> +    {
> +        cpumask |= cpumask << 8;
> +        cpumask |= cpumask << 16;
> +        for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
> +            writel_gicd(cpumask, GICD_ITARGETSR + (i / 4) * 4);
> +    }
>  
>      /* Default priority for global interrupts */
>      for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
> @@ -285,7 +319,7 @@ static void __cpuinit gicv2_cpu_init(void)
>  {
>      int i;
>  
> -    this_cpu(gic_cpu_id) = readl_gicd(GICD_ITARGETSR) & 0xff;
> +    this_cpu(gic_cpu_id) = readl_gicd(GICD_ITARGETSR) & gic_cpu_mask;
>  
>      /* The first 32 interrupts (PPI and SGI) are banked per-cpu, so
>       * even though they are controlled with GICD registers, they must
> @@ -366,7 +400,7 @@ static void gicv2_send_SGI(enum gic_sgi sgi, enum gic_sgi_mode irqmode,
>          cpumask_and(&online_mask, cpu_mask, &cpu_online_map);
>          mask = gicv2_cpu_mask(&online_mask);
>          writel_gicd(GICD_SGI_TARGET_LIST |
> -                    (mask << GICD_SGI_TARGET_SHIFT) | sgi,
> +                    (mask << gicd_sgi_target_shift) | sgi,
>                      GICD_SGIR);
>          break;
>      default:
> @@ -579,7 +613,7 @@ static void gicv2_irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_m
>      mask = gicv2_cpu_mask(cpu_mask);
>  
>      /* Set target CPU mask (RAZ/WI on uniprocessor) */
> -    writeb_gicd(mask, GICD_ITARGETSR + desc->irq);
> +    gicv2_set_irq_mask(desc->irq, mask);
>  
>      spin_unlock(&gicv2.lock);
>  }
> @@ -682,7 +716,7 @@ const static struct gic_hw_operations gicv2_ops = {
>  };
>  
>  /* Set up the GIC */
> -static int __init gicv2_init(struct dt_device_node *node, const void *data)
> +static int __init gicv2_init_default(struct dt_device_node *node, const void *data)
>  {
>      int res;
>  
> @@ -762,6 +796,15 @@ static int __init gicv2_init(struct dt_device_node *node, const void *data)
>      return 0;
>  }
>  
> +static int __init hip04_gicv2_init(struct dt_device_node *node, const void *data)
> +{
> +    nr_gic_cpu_if = 16;
> +    gicd_sgi_target_shift = 8;
> +    gic_cpu_mask = 0xffff;
> +
> +    return gicv2_init_default(node, data);
> +}
> +
>  static const char * const gicv2_dt_compat[] __initconst =
>  {
>      DT_COMPAT_GIC_CORTEX_A15,
> @@ -772,9 +815,21 @@ static const char * const gicv2_dt_compat[] __initconst =
>  
>  DT_DEVICE_START(gicv2, "GICv2:", DEVICE_GIC)
>          .compatible = gicv2_dt_compat,
> -        .init = gicv2_init,
> +        .init = gicv2_init_default,
>  DT_DEVICE_END
>  
> +static const char * const hip04_gicv2_dt_compat[] __initconst =
> +{
> +    DT_COMPAT_GIC_HIP04,
> +    NULL
> +};
> +
> +DT_DEVICE_START(hip04_gicv2, "GICv2:", DEVICE_GIC)
> +        .compatible = hip04_gicv2_dt_compat,
> +        .init = hip04_gicv2_init,
> +DT_DEVICE_END
> +
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 390c8b0..e4512a8 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -565,12 +565,13 @@ static void do_sgi(struct cpu_user_regs *regs, enum gic_sgi sgi)
>  void gic_interrupt(struct cpu_user_regs *regs, int is_fiq)
>  {
>      unsigned int irq;
> +    unsigned int max_irq = gic_hw_ops->info->nr_lines;
>  
>      do  {
>          /* Reading IRQ will ACK it */
>          irq = gic_hw_ops->read_irq();
>  
> -        if ( likely(irq >= 16 && irq < 1021) )
> +        if ( likely(irq >= 16 && irq < max_irq) )
>          {
>              local_irq_enable();
>              do_IRQ(regs, irq, is_fiq);
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 187dc46..c880867 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -155,10 +155,12 @@
>  #define DT_COMPAT_GIC_400            "arm,gic-400"
>  #define DT_COMPAT_GIC_CORTEX_A15     "arm,cortex-a15-gic"
>  #define DT_COMPAT_GIC_CORTEX_A7      "arm,cortex-a7-gic"
> +#define DT_COMPAT_GIC_HIP04          "hisilicon,hip04-intc"
>  
>  #define DT_MATCH_GIC_V2 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A15), \
>                          DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A7), \
> -                        DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_400)
> +                        DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_400), \
> +                        DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_HIP04)
>  
>  #define DT_COMPAT_GIC_V3             "arm,gic-v3"
>  

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/4] xen/arm: Add support for DTBs with strange names of Hip04 GICv2
  2015-02-24 16:13     ` Ian Campbell
@ 2015-02-24 16:38       ` Frediano Ziglio
  0 siblings, 0 replies; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-24 16:38 UTC (permalink / raw)
  To: Ian Campbell
  Cc: zoltan.kiss, Julien Grall, Tim Deegan, xen-devel,
	Frediano Ziglio, Stefano Stabellini


[-- Attachment #1.1: Type: text/plain, Size: 1102 bytes --]

Il 24/Feb/2015 16:24 "Ian Campbell" <ian.campbell@citrix.com> ha scritto:
>
> On Tue, 2015-02-24 at 14:19 +0000, Julien Grall wrote:
> > Hi Frediano,
> >
> > On 20/02/15 09:56, Frediano Ziglio wrote:
> > > This name can appear in some Linux kernel repos. Not very fortunate,
> > > but to avoid others spending an hour to spot that few characters
> > > difference it worth to work around it.
> >
> > As Zoltan said on a previous mail [1], this is only used on your
> > internal kernel.
> >
> > Furthermore, as the bindings is not official, nothing prevent someone to
> > re-use the bindings for another purpose (such as GICv3 on hisilicon).
> >
> > Xen upstream aims to support only official bindings for a this reason.
> > So I don't think we should take this patch.
>
> Neither do I. Nacked-by: Ian Campbell <ian.campbell@citrix.com>.
>
> If/when these bindings are described in the official bindings document
> then please let us know.
>
> Ian.
>

I currently ack the nack. As this dtb is only used by xen just updating it
make everybody happy. Dom0 and other domains will use standard gic

Frediano

[-- Attachment #1.2: Type: text/html, Size: 1506 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v5.99.1 RFC 0/4] xen/arm: Add support for Huawei hip04-d01 platform
  2015-02-24 16:16   ` Ian Campbell
@ 2015-02-25 15:28     ` Frediano Ziglio
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version Frediano Ziglio
                         ` (3 more replies)
  0 siblings, 4 replies; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-25 15:28 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini, Tim Deegan, Julien Grall,
	frediano.ziglio
  Cc: zoltan.kiss, xen-devel

This set of patches add Xen support for hip04-d01 platform (see https://wiki.linaro.org/Boards/D01 for details).

Changes from v5:
- do not change gic-v2.c code but use a copy.

To be considered RFC, to see if better to use copy or other techniques.

Changes from v4:
- rebased to new version;
- removed patch for computing GIC addresses as it apply to all platforms;
- removed patches to platform (cpu and system operations) as now they can
  use a bootwrapper which provide them.

Changes from v3:
- change the way regs property is computed for GICv2 (Julien Grall);
- revert order of compaible names for GIC (Julien Grall).

Changes from v2:
- rewrote DTS fix patch (Ian Campbell);
- use is_hip04 macro instead of doing explicit test (Julien Grall);
- do not use quirks to distinguish this platform (Ian Cambell);
- move some GIC constants to C files instead of header (Julien Grall);
- minor changes (Julien Grall).

Changes from v1:
- style (Julien Grall);
- make gicv2_send_SGI faster (Julien Grall);
- cleanup correctly if hip04_smp_init fails (Julien Grall);
- remove quirks using compatibility (Ian Campbell);
- other minor suggestions by Julien Grall.

Frediano Ziglio (4):
  xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  xen/arm: Make gic-v2 code handle hip04-d01 platform
  xen/arm: handle GICH register changes for hip04-d01 platform
  xen/arm: Force dom0 to use normal GICv2 driver on Hip04 platform

 xen/arch/arm/Makefile       |   2 +-
 xen/arch/arm/domain_build.c |   1 +
 xen/arch/arm/gic-hip04.c    | 796 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/gic.c          |   3 +-
 xen/include/asm-arm/gic.h   |   3 +
 5 files changed, 803 insertions(+), 2 deletions(-)
 create mode 100644 xen/arch/arm/gic-hip04.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  2015-02-25 15:28     ` [PATCH v5.99.1 RFC 0/4] xen/arm: Add support for Huawei " Frediano Ziglio
@ 2015-02-25 15:28       ` Frediano Ziglio
  2015-02-25 15:34         ` Julien Grall
  2015-02-25 16:34         ` Stefano Stabellini
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 2/4] xen/arm: Make gic-v2 code handle hip04-d01 platform Frediano Ziglio
                         ` (2 subsequent siblings)
  3 siblings, 2 replies; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-25 15:28 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini, Tim Deegan, Julien Grall,
	frediano.ziglio
  Cc: zoltan.kiss, xen-devel

HiSilison Hip04 platform use a slightly different version

Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
---
 xen/arch/arm/Makefile    |   2 +-
 xen/arch/arm/gic-hip04.c | 788 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 789 insertions(+), 1 deletion(-)
 create mode 100644 xen/arch/arm/gic-hip04.c

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 41aba2e..6fb8ba9 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -11,7 +11,7 @@ obj-y += vpsci.o
 obj-y += domctl.o
 obj-y += sysctl.o
 obj-y += domain_build.o
-obj-y += gic.o gic-v2.o
+obj-y += gic.o gic-v2.o gic-hip04.o
 obj-$(CONFIG_ARM_64) += gic-v3.o
 obj-y += io.o
 obj-y += irq.o
diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
new file mode 100644
index 0000000..a401e3f
--- /dev/null
+++ b/xen/arch/arm/gic-hip04.c
@@ -0,0 +1,788 @@
+/*
+ * xen/arch/arm/gic-v2.c
+ *
+ * ARM Generic Interrupt Controller support v2
+ *
+ * Tim Deegan <tim@xen.org>
+ * Copyright (c) 2011 Citrix Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <xen/config.h>
+#include <xen/lib.h>
+#include <xen/init.h>
+#include <xen/mm.h>
+#include <xen/irq.h>
+#include <xen/sched.h>
+#include <xen/errno.h>
+#include <xen/softirq.h>
+#include <xen/list.h>
+#include <xen/device_tree.h>
+#include <xen/libfdt/libfdt.h>
+#include <asm/p2m.h>
+#include <asm/domain.h>
+#include <asm/platform.h>
+#include <asm/device.h>
+
+#include <asm/io.h>
+#include <asm/gic.h>
+
+/*
+ * LR register definitions are GIC v2 specific.
+ * Moved these definitions from header file to here
+ */
+#define GICH_V2_LR_VIRTUAL_MASK    0x3ff
+#define GICH_V2_LR_VIRTUAL_SHIFT   0
+#define GICH_V2_LR_PHYSICAL_MASK   0x3ff
+#define GICH_V2_LR_PHYSICAL_SHIFT  10
+#define GICH_V2_LR_STATE_MASK      0x3
+#define GICH_V2_LR_STATE_SHIFT     28
+#define GICH_V2_LR_PRIORITY_SHIFT  23
+#define GICH_V2_LR_PRIORITY_MASK   0x1f
+#define GICH_V2_LR_HW_SHIFT        31
+#define GICH_V2_LR_HW_MASK         0x1
+#define GICH_V2_LR_GRP_SHIFT       30
+#define GICH_V2_LR_GRP_MASK        0x1
+#define GICH_V2_LR_MAINTENANCE_IRQ (1<<19)
+#define GICH_V2_LR_GRP1            (1<<30)
+#define GICH_V2_LR_HW              (1<<31)
+#define GICH_V2_LR_CPUID_SHIFT     9
+#define GICH_V2_VTR_NRLRGS         0x3f
+
+#define GICH_V2_VMCR_PRIORITY_MASK   0x1f
+#define GICH_V2_VMCR_PRIORITY_SHIFT  27
+
+/* Global state */
+static struct {
+    paddr_t dbase;            /* Address of distributor registers */
+    void __iomem * map_dbase; /* IO mapped Address of distributor registers */
+    paddr_t cbase;            /* Address of CPU interface registers */
+    void __iomem * map_cbase[2]; /* IO mapped Address of CPU interface registers */
+    paddr_t hbase;            /* Address of virtual interface registers */
+    void __iomem * map_hbase; /* IO Address of virtual interface registers */
+    paddr_t vbase;            /* Address of virtual cpu interface registers */
+    spinlock_t lock;
+} gicv2;
+
+static struct gic_info gicv2_info;
+
+/* The GIC mapping of CPU interfaces does not necessarily match the
+ * logical CPU numbering. Let's use mapping as returned by the GIC
+ * itself
+ */
+static DEFINE_PER_CPU(u8, gic_cpu_id);
+
+/* Maximum cpu interface per GIC */
+#define NR_GIC_CPU_IF 8
+
+static inline void writeb_gicd(uint8_t val, unsigned int offset)
+{
+    writeb_relaxed(val, gicv2.map_dbase + offset);
+}
+
+static inline void writel_gicd(uint32_t val, unsigned int offset)
+{
+    writel_relaxed(val, gicv2.map_dbase + offset);
+}
+
+static inline uint32_t readl_gicd(unsigned int offset)
+{
+    return readl_relaxed(gicv2.map_dbase + offset);
+}
+
+static inline void writel_gicc(uint32_t val, unsigned int offset)
+{
+    unsigned int page = offset >> PAGE_SHIFT;
+    offset &= ~PAGE_MASK;
+    writel_relaxed(val, gicv2.map_cbase[page] + offset);
+}
+
+static inline uint32_t readl_gicc(unsigned int offset)
+{
+    unsigned int page = offset >> PAGE_SHIFT;
+    offset &= ~PAGE_MASK;
+    return readl_relaxed(gicv2.map_cbase[page] + offset);
+}
+
+static inline void writel_gich(uint32_t val, unsigned int offset)
+{
+    writel_relaxed(val, gicv2.map_hbase + offset);
+}
+
+static inline uint32_t readl_gich(int unsigned offset)
+{
+    return readl_relaxed(gicv2.map_hbase + offset);
+}
+
+static unsigned int gicv2_cpu_mask(const cpumask_t *cpumask)
+{
+    unsigned int cpu;
+    unsigned int mask = 0;
+    cpumask_t possible_mask;
+
+    cpumask_and(&possible_mask, cpumask, &cpu_possible_map);
+    for_each_cpu( cpu, &possible_mask )
+    {
+        ASSERT(cpu < NR_GIC_CPU_IF);
+        mask |= per_cpu(gic_cpu_id, cpu);
+    }
+
+    return mask;
+}
+
+static void gicv2_save_state(struct vcpu *v)
+{
+    int i;
+
+    /* No need for spinlocks here because interrupts are disabled around
+     * this call and it only accesses struct vcpu fields that cannot be
+     * accessed simultaneously by another pCPU.
+     */
+    for ( i = 0; i < gicv2_info.nr_lrs; i++ )
+        v->arch.gic.v2.lr[i] = readl_gich(GICH_LR + i * 4);
+
+    v->arch.gic.v2.apr = readl_gich(GICH_APR);
+    v->arch.gic.v2.vmcr = readl_gich(GICH_VMCR);
+    /* Disable until next VCPU scheduled */
+    writel_gich(0, GICH_HCR);
+}
+
+static void gicv2_restore_state(const struct vcpu *v)
+{
+    int i;
+
+    for ( i = 0; i < gicv2_info.nr_lrs; i++ )
+        writel_gich(v->arch.gic.v2.lr[i], GICH_LR + i * 4);
+
+    writel_gich(v->arch.gic.v2.apr, GICH_APR);
+    writel_gich(v->arch.gic.v2.vmcr, GICH_VMCR);
+    writel_gich(GICH_HCR_EN, GICH_HCR);
+}
+
+static void gicv2_dump_state(const struct vcpu *v)
+{
+    int i;
+
+    if ( v == current )
+    {
+        for ( i = 0; i < gicv2_info.nr_lrs; i++ )
+            printk("   HW_LR[%d]=%x\n", i,
+                   readl_gich(GICH_LR + i * 4));
+    }
+    else
+    {
+        for ( i = 0; i < gicv2_info.nr_lrs; i++ )
+            printk("   VCPU_LR[%d]=%x\n", i, v->arch.gic.v2.lr[i]);
+    }
+}
+
+static void gicv2_eoi_irq(struct irq_desc *irqd)
+{
+    int irq = irqd->irq;
+    /* Lower the priority */
+    writel_gicc(irq, GICC_EOIR);
+}
+
+static void gicv2_dir_irq(struct irq_desc *irqd)
+{
+    /* Deactivate */
+    writel_gicc(irqd->irq, GICC_DIR);
+}
+
+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)
+{
+    uint32_t cfg, edgebit;
+    unsigned int mask = gicv2_cpu_mask(cpu_mask);
+    unsigned int irq = desc->irq;
+    unsigned int type = desc->arch.type;
+
+    ASSERT(type != DT_IRQ_TYPE_INVALID);
+    ASSERT(spin_is_locked(&desc->lock));
+
+    spin_lock(&gicv2.lock);
+    /* Set edge / level */
+    cfg = readl_gicd(GICD_ICFGR + (irq / 16) * 4);
+    edgebit = 2u << (2 * (irq % 16));
+    if ( type & DT_IRQ_TYPE_LEVEL_MASK )
+        cfg &= ~edgebit;
+    else if ( type & DT_IRQ_TYPE_EDGE_BOTH )
+        cfg |= edgebit;
+    writel_gicd(cfg, GICD_ICFGR + (irq / 16) * 4);
+
+    /* Set target CPU mask (RAZ/WI on uniprocessor) */
+    writeb_gicd(mask, GICD_ITARGETSR + irq);
+    /* Set priority */
+    writeb_gicd(priority, GICD_IPRIORITYR + irq);
+
+    spin_unlock(&gicv2.lock);
+}
+
+static void __init gicv2_dist_init(void)
+{
+    uint32_t type;
+    uint32_t cpumask;
+    uint32_t gic_cpus;
+    int i;
+
+    cpumask = readl_gicd(GICD_ITARGETSR) & 0xff;
+    cpumask |= cpumask << 8;
+    cpumask |= cpumask << 16;
+
+    /* Disable the distributor */
+    writel_gicd(0, GICD_CTLR);
+
+    type = readl_gicd(GICD_TYPER);
+    gicv2_info.nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
+    gic_cpus = 1 + ((type & GICD_TYPE_CPUS) >> 5);
+    printk("GICv2: %d lines, %d cpu%s%s (IID %8.8x).\n",
+           gicv2_info.nr_lines, gic_cpus, (gic_cpus == 1) ? "" : "s",
+           (type & GICD_TYPE_SEC) ? ", secure" : "",
+           readl_gicd(GICD_IIDR));
+
+    /* Default all global IRQs to level, active low */
+    for ( i = 32; i < gicv2_info.nr_lines; i += 16 )
+        writel_gicd(0x0, GICD_ICFGR + (i / 16) * 4);
+
+    /* Route all global IRQs to this CPU */
+    for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
+        writel_gicd(cpumask, GICD_ITARGETSR + (i / 4) * 4);
+
+    /* Default priority for global interrupts */
+    for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
+        writel_gicd(GIC_PRI_IRQ << 24 | GIC_PRI_IRQ << 16 |
+                    GIC_PRI_IRQ << 8 | GIC_PRI_IRQ,
+                    GICD_IPRIORITYR + (i / 4) * 4);
+
+    /* Disable all global interrupts */
+    for ( i = 32; i < gicv2_info.nr_lines; i += 32 )
+        writel_gicd(~0x0, GICD_ICENABLER + (i / 32) * 4);
+
+    /* Turn on the distributor */
+    writel_gicd(GICD_CTL_ENABLE, GICD_CTLR);
+}
+
+static void __cpuinit gicv2_cpu_init(void)
+{
+    int i;
+
+    this_cpu(gic_cpu_id) = readl_gicd(GICD_ITARGETSR) & 0xff;
+
+    /* The first 32 interrupts (PPI and SGI) are banked per-cpu, so
+     * even though they are controlled with GICD registers, they must
+     * be set up here with the other per-cpu state. */
+    writel_gicd(0xffff0000, GICD_ICENABLER); /* Disable all PPI */
+    writel_gicd(0x0000ffff, GICD_ISENABLER); /* Enable all SGI */
+
+    /* Set SGI priorities */
+    for ( i = 0; i < 16; i += 4 )
+        writel_gicd(GIC_PRI_IPI << 24 | GIC_PRI_IPI << 16 |
+                    GIC_PRI_IPI << 8 | GIC_PRI_IPI,
+                    GICD_IPRIORITYR + (i / 4) * 4);
+
+    /* Set PPI priorities */
+    for ( i = 16; i < 32; i += 4 )
+        writel_gicd(GIC_PRI_IRQ << 24 | GIC_PRI_IRQ << 16 |
+                    GIC_PRI_IRQ << 8 | GIC_PRI_IRQ,
+                    GICD_IPRIORITYR + (i / 4) * 4);
+
+    /* Local settings: interface controller */
+    /* Don't mask by priority */
+    writel_gicc(0xff, GICC_PMR);
+    /* Finest granularity of priority */
+    writel_gicc(0x0, GICC_BPR);
+    /* Turn on delivery */
+    writel_gicc(GICC_CTL_ENABLE|GICC_CTL_EOI, GICC_CTLR);
+}
+
+static void gicv2_cpu_disable(void)
+{
+    writel_gicc(0x0, GICC_CTLR);
+}
+
+static void __cpuinit gicv2_hyp_init(void)
+{
+    uint32_t vtr;
+    uint8_t nr_lrs;
+
+    vtr = readl_gich(GICH_VTR);
+    nr_lrs  = (vtr & GICH_V2_VTR_NRLRGS) + 1;
+    gicv2_info.nr_lrs = nr_lrs;
+
+    writel_gich(GICH_MISR_EOI, GICH_MISR);
+}
+
+static void __cpuinit gicv2_hyp_disable(void)
+{
+    writel_gich(0, GICH_HCR);
+}
+
+static int gicv2_secondary_cpu_init(void)
+{
+    spin_lock(&gicv2.lock);
+
+    gicv2_cpu_init();
+    gicv2_hyp_init();
+
+    spin_unlock(&gicv2.lock);
+
+    return 0;
+}
+
+static void gicv2_send_SGI(enum gic_sgi sgi, enum gic_sgi_mode irqmode,
+                           const cpumask_t *cpu_mask)
+{
+    unsigned int mask = 0;
+    cpumask_t online_mask;
+
+    switch ( irqmode )
+    {
+    case SGI_TARGET_OTHERS:
+        writel_gicd(GICD_SGI_TARGET_OTHERS | sgi, GICD_SGIR);
+        break;
+    case SGI_TARGET_SELF:
+        writel_gicd(GICD_SGI_TARGET_SELF | sgi, GICD_SGIR);
+        break;
+    case SGI_TARGET_LIST:
+        cpumask_and(&online_mask, cpu_mask, &cpu_online_map);
+        mask = gicv2_cpu_mask(&online_mask);
+        writel_gicd(GICD_SGI_TARGET_LIST |
+                    (mask << GICD_SGI_TARGET_SHIFT) | sgi,
+                    GICD_SGIR);
+        break;
+    default:
+        BUG();
+    }
+}
+
+/* Shut down the per-CPU GIC interface */
+static void gicv2_disable_interface(void)
+{
+    spin_lock(&gicv2.lock);
+    gicv2_cpu_disable();
+    gicv2_hyp_disable();
+    spin_unlock(&gicv2.lock);
+}
+
+static void gicv2_update_lr(int lr, const struct pending_irq *p,
+                            unsigned int state)
+{
+    uint32_t lr_reg;
+
+    BUG_ON(lr >= gicv2_info.nr_lrs);
+    BUG_ON(lr < 0);
+
+    lr_reg = (((state & GICH_V2_LR_STATE_MASK) << GICH_V2_LR_STATE_SHIFT)  |
+              ((GIC_PRI_TO_GUEST(p->priority) & GICH_V2_LR_PRIORITY_MASK)
+                                             << GICH_V2_LR_PRIORITY_SHIFT) |
+              ((p->irq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT));
+
+    if ( p->desc != NULL )
+    {
+        if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) )
+            lr_reg |= GICH_V2_LR_MAINTENANCE_IRQ;
+        else
+            lr_reg |= GICH_V2_LR_HW | ((p->desc->irq & GICH_V2_LR_PHYSICAL_MASK )
+                            << GICH_V2_LR_PHYSICAL_SHIFT);
+    }
+
+    writel_gich(lr_reg, GICH_LR + lr * 4);
+}
+
+static void gicv2_clear_lr(int lr)
+{
+    writel_gich(0, GICH_LR + lr * 4);
+}
+
+static int gicv2v_setup(struct domain *d)
+{
+    int ret;
+
+    /*
+     * The hardware domain gets the hardware address.
+     * Guests get the virtual platform layout.
+     */
+    if ( is_hardware_domain(d) )
+    {
+        d->arch.vgic.dbase = gicv2.dbase;
+        d->arch.vgic.cbase = gicv2.cbase;
+    }
+    else
+    {
+        d->arch.vgic.dbase = GUEST_GICD_BASE;
+        d->arch.vgic.cbase = GUEST_GICC_BASE;
+    }
+
+    /*
+     * Map the gic virtual cpu interface in the gic cpu interface
+     * region of the guest.
+     *
+     * The second page is always mapped at +4K irrespective of the
+     * GIC_64K_STRIDE quirk. The DTB passed to the guest reflects this.
+     */
+    ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase), 1,
+                            paddr_to_pfn(gicv2.vbase));
+    if ( ret )
+        return ret;
+
+    if ( !platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) )
+        ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + PAGE_SIZE),
+                               2, paddr_to_pfn(gicv2.vbase + PAGE_SIZE));
+    else
+        ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + PAGE_SIZE),
+                               2, paddr_to_pfn(gicv2.vbase + 16*PAGE_SIZE));
+
+    return ret;
+}
+
+static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
+{
+    uint32_t lrv;
+
+    lrv          = readl_gich(GICH_LR + lr * 4);
+    lr_reg->pirq = (lrv >> GICH_V2_LR_PHYSICAL_SHIFT) & GICH_V2_LR_PHYSICAL_MASK;
+    lr_reg->virq = (lrv >> GICH_V2_LR_VIRTUAL_SHIFT) & GICH_V2_LR_VIRTUAL_MASK;
+    lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) & GICH_V2_LR_PRIORITY_MASK;
+    lr_reg->state     = (lrv >> GICH_V2_LR_STATE_SHIFT) & GICH_V2_LR_STATE_MASK;
+    lr_reg->hw_status = (lrv >> GICH_V2_LR_HW_SHIFT) & GICH_V2_LR_HW_MASK;
+    lr_reg->grp       = (lrv >> GICH_V2_LR_GRP_SHIFT) & GICH_V2_LR_GRP_MASK;
+}
+
+static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg)
+{
+    uint32_t lrv = 0;
+
+    lrv = ( ((lr_reg->pirq & GICH_V2_LR_PHYSICAL_MASK) << GICH_V2_LR_PHYSICAL_SHIFT) |
+          ((lr_reg->virq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT)   |
+          ((uint32_t)(lr_reg->priority & GICH_V2_LR_PRIORITY_MASK)
+                                      << GICH_V2_LR_PRIORITY_SHIFT) |
+          ((uint32_t)(lr_reg->state & GICH_V2_LR_STATE_MASK)
+                                   << GICH_V2_LR_STATE_SHIFT) |
+          ((uint32_t)(lr_reg->hw_status & GICH_V2_LR_HW_MASK)
+                                       << GICH_V2_LR_HW_SHIFT)  |
+          ((uint32_t)(lr_reg->grp & GICH_V2_LR_GRP_MASK) << GICH_V2_LR_GRP_SHIFT) );
+
+    writel_gich(lrv, GICH_LR + lr * 4);
+}
+
+static void gicv2_hcr_status(uint32_t flag, bool_t status)
+{
+    uint32_t hcr = readl_gich(GICH_HCR);
+
+    if ( status )
+        hcr |= flag;
+    else
+        hcr &= (~flag);
+
+    writel_gich(hcr, GICH_HCR);
+}
+
+static unsigned int gicv2_read_vmcr_priority(void)
+{
+   return ((readl_gich(GICH_VMCR) >> GICH_V2_VMCR_PRIORITY_SHIFT)
+           & GICH_V2_VMCR_PRIORITY_MASK);
+}
+
+static unsigned int gicv2_read_apr(int apr_reg)
+{
+   return readl_gich(GICH_APR);
+}
+
+static void gicv2_irq_enable(struct irq_desc *desc)
+{
+    unsigned long flags;
+    int irq = desc->irq;
+
+    ASSERT(spin_is_locked(&desc->lock));
+
+    spin_lock_irqsave(&gicv2.lock, flags);
+    clear_bit(_IRQ_DISABLED, &desc->status);
+    dsb(sy);
+    /* Enable routing */
+    writel_gicd((1u << (irq % 32)), GICD_ISENABLER + (irq / 32) * 4);
+    spin_unlock_irqrestore(&gicv2.lock, flags);
+}
+
+static void gicv2_irq_disable(struct irq_desc *desc)
+{
+    unsigned long flags;
+    int irq = desc->irq;
+
+    ASSERT(spin_is_locked(&desc->lock));
+
+    spin_lock_irqsave(&gicv2.lock, flags);
+    /* Disable routing */
+    writel_gicd(1u << (irq % 32), GICD_ICENABLER + (irq / 32) * 4);
+    set_bit(_IRQ_DISABLED, &desc->status);
+    spin_unlock_irqrestore(&gicv2.lock, flags);
+}
+
+static unsigned int gicv2_irq_startup(struct irq_desc *desc)
+{
+    gicv2_irq_enable(desc);
+
+    return 0;
+}
+
+static void gicv2_irq_shutdown(struct irq_desc *desc)
+{
+    gicv2_irq_disable(desc);
+}
+
+static void gicv2_irq_ack(struct irq_desc *desc)
+{
+    /* No ACK -- reading IAR has done this for us */
+}
+
+static void gicv2_host_irq_end(struct irq_desc *desc)
+{
+    /* Lower the priority */
+    gicv2_eoi_irq(desc);
+    /* Deactivate */
+    gicv2_dir_irq(desc);
+}
+
+static void gicv2_guest_irq_end(struct irq_desc *desc)
+{
+    /* Lower the priority of the IRQ */
+    gicv2_eoi_irq(desc);
+    /* Deactivation happens in maintenance interrupt / via GICV */
+}
+
+static void gicv2_irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask)
+{
+    unsigned int mask;
+
+    ASSERT(!cpumask_empty(cpu_mask));
+
+    spin_lock(&gicv2.lock);
+
+    mask = gicv2_cpu_mask(cpu_mask);
+
+    /* Set target CPU mask (RAZ/WI on uniprocessor) */
+    writeb_gicd(mask, GICD_ITARGETSR + desc->irq);
+
+    spin_unlock(&gicv2.lock);
+}
+
+static int gicv2_make_dt_node(const struct domain *d,
+                              const struct dt_device_node *node, void *fdt)
+{
+    const struct dt_device_node *gic = dt_interrupt_controller;
+    const void *compatible = NULL;
+    u32 len;
+    const __be32 *regs;
+    int res = 0;
+
+    compatible = dt_get_property(gic, "compatible", &len);
+    if ( !compatible )
+    {
+        dprintk(XENLOG_ERR, "Can't find compatible property for the gic node\n");
+        return -FDT_ERR_XEN(ENOENT);
+    }
+
+    res = fdt_begin_node(fdt, "interrupt-controller");
+    if ( res )
+        return res;
+
+    res = fdt_property(fdt, "compatible", compatible, len);
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "#interrupt-cells", 3);
+    if ( res )
+        return res;
+
+    res = fdt_property(fdt, "interrupt-controller", NULL, 0);
+
+    if ( res )
+        return res;
+
+    /*
+     * DTB provides up to 4 regions to handle virtualization
+     * however dom0 just needs GICC and GICD provided by Xen.
+     */
+    regs = dt_get_property(gic, "reg", &len);
+    if ( !regs )
+    {
+        dprintk(XENLOG_ERR, "Can't find reg property for the gic node\n");
+        return -FDT_ERR_XEN(ENOENT);
+    }
+
+    len = dt_cells_to_size(dt_n_addr_cells(node) + dt_n_size_cells(node));
+    len *= 2;
+
+    res = fdt_property(fdt, "reg", regs, len);
+
+    return res;
+}
+
+/* XXX different for level vs edge */
+static hw_irq_controller gicv2_host_irq_type = {
+    .typename     = "gic-v2",
+    .startup      = gicv2_irq_startup,
+    .shutdown     = gicv2_irq_shutdown,
+    .enable       = gicv2_irq_enable,
+    .disable      = gicv2_irq_disable,
+    .ack          = gicv2_irq_ack,
+    .end          = gicv2_host_irq_end,
+    .set_affinity = gicv2_irq_set_affinity,
+};
+
+static hw_irq_controller gicv2_guest_irq_type = {
+    .typename     = "gic-v2",
+    .startup      = gicv2_irq_startup,
+    .shutdown     = gicv2_irq_shutdown,
+    .enable       = gicv2_irq_enable,
+    .disable      = gicv2_irq_disable,
+    .ack          = gicv2_irq_ack,
+    .end          = gicv2_guest_irq_end,
+    .set_affinity = gicv2_irq_set_affinity,
+};
+
+const static struct gic_hw_operations gicv2_ops = {
+    .info                = &gicv2_info,
+    .secondary_init      = gicv2_secondary_cpu_init,
+    .save_state          = gicv2_save_state,
+    .restore_state       = gicv2_restore_state,
+    .dump_state          = gicv2_dump_state,
+    .gicv_setup          = gicv2v_setup,
+    .gic_host_irq_type   = &gicv2_host_irq_type,
+    .gic_guest_irq_type  = &gicv2_guest_irq_type,
+    .eoi_irq             = gicv2_eoi_irq,
+    .deactivate_irq      = gicv2_dir_irq,
+    .read_irq            = gicv2_read_irq,
+    .set_irq_properties  = gicv2_set_irq_properties,
+    .send_SGI            = gicv2_send_SGI,
+    .disable_interface   = gicv2_disable_interface,
+    .update_lr           = gicv2_update_lr,
+    .update_hcr_status   = gicv2_hcr_status,
+    .clear_lr            = gicv2_clear_lr,
+    .read_lr             = gicv2_read_lr,
+    .write_lr            = gicv2_write_lr,
+    .read_vmcr_priority  = gicv2_read_vmcr_priority,
+    .read_apr            = gicv2_read_apr,
+    .make_dt_node        = gicv2_make_dt_node,
+};
+
+/* Set up the GIC */
+static int __init gicv2_init(struct dt_device_node *node, const void *data)
+{
+    int res;
+
+    dt_device_set_used_by(node, DOMID_XEN);
+
+    res = dt_device_get_address(node, 0, &gicv2.dbase, NULL);
+    if ( res || !gicv2.dbase || (gicv2.dbase & ~PAGE_MASK) )
+        panic("GICv2: Cannot find a valid address for the distributor");
+
+    res = dt_device_get_address(node, 1, &gicv2.cbase, NULL);
+    if ( res || !gicv2.cbase || (gicv2.cbase & ~PAGE_MASK) )
+        panic("GICv2: Cannot find a valid address for the CPU");
+
+    res = dt_device_get_address(node, 2, &gicv2.hbase, NULL);
+    if ( res || !gicv2.hbase || (gicv2.hbase & ~PAGE_MASK) )
+        panic("GICv2: Cannot find a valid address for the hypervisor");
+
+    res = dt_device_get_address(node, 3, &gicv2.vbase, NULL);
+    if ( res || !gicv2.vbase || (gicv2.vbase & ~PAGE_MASK) )
+        panic("GICv2: Cannot find a valid address for the virtual CPU");
+
+    res = platform_get_irq(node, 0);
+    if ( res < 0 )
+        panic("GICv2: Cannot find the maintenance IRQ");
+    gicv2_info.maintenance_irq = res;
+
+    /* Set the GIC as the primary interrupt controller */
+    dt_interrupt_controller = node;
+
+    /* TODO: Add check on distributor, cpu size */
+
+    printk("GICv2 initialization:\n"
+              "        gic_dist_addr=%"PRIpaddr"\n"
+              "        gic_cpu_addr=%"PRIpaddr"\n"
+              "        gic_hyp_addr=%"PRIpaddr"\n"
+              "        gic_vcpu_addr=%"PRIpaddr"\n"
+              "        gic_maintenance_irq=%u\n",
+              gicv2.dbase, gicv2.cbase, gicv2.hbase, gicv2.vbase,
+              gicv2_info.maintenance_irq);
+
+    if ( (gicv2.dbase & ~PAGE_MASK) || (gicv2.cbase & ~PAGE_MASK) ||
+         (gicv2.hbase & ~PAGE_MASK) || (gicv2.vbase & ~PAGE_MASK) )
+        panic("GICv2 interfaces not page aligned");
+
+    gicv2.map_dbase = ioremap_nocache(gicv2.dbase, PAGE_SIZE);
+    if ( !gicv2.map_dbase )
+        panic("GICv2: Failed to ioremap for GIC distributor\n");
+
+    gicv2.map_cbase[0] = ioremap_nocache(gicv2.cbase, PAGE_SIZE);
+
+    if ( platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) )
+        gicv2.map_cbase[1] = ioremap_nocache(gicv2.cbase + PAGE_SIZE * 0x10,
+                                           PAGE_SIZE);
+    else
+        gicv2.map_cbase[1] = ioremap_nocache(gicv2.cbase + PAGE_SIZE, PAGE_SIZE);
+
+    if ( !gicv2.map_cbase[0] || !gicv2.map_cbase[1] )
+        panic("GICv2: Failed to ioremap for GIC CPU interface\n");
+
+    gicv2.map_hbase = ioremap_nocache(gicv2.hbase, PAGE_SIZE);
+    if ( !gicv2.map_hbase )
+        panic("GICv2: Failed to ioremap for GIC Virtual interface\n");
+
+    /* Global settings: interrupt distributor */
+    spin_lock_init(&gicv2.lock);
+    spin_lock(&gicv2.lock);
+
+    gicv2_dist_init();
+    gicv2_cpu_init();
+    gicv2_hyp_init();
+
+    spin_unlock(&gicv2.lock);
+
+    gicv2_info.hw_version = GIC_V2;
+    register_gic_ops(&gicv2_ops);
+
+    return 0;
+}
+
+static const char * const gicv2_dt_compat[] __initconst =
+{
+    DT_COMPAT_GIC_CORTEX_A15,
+    DT_COMPAT_GIC_CORTEX_A7,
+    DT_COMPAT_GIC_400,
+    NULL
+};
+
+DT_DEVICE_START(gicv2, "GICv2:", DEVICE_GIC)
+        .compatible = gicv2_dt_compat,
+        .init = gicv2_init,
+DT_DEVICE_END
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5.99.1 RFC 2/4] xen/arm: Make gic-v2 code handle hip04-d01 platform
  2015-02-25 15:28     ` [PATCH v5.99.1 RFC 0/4] xen/arm: Add support for Huawei " Frediano Ziglio
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version Frediano Ziglio
@ 2015-02-25 15:28       ` Frediano Ziglio
  2015-02-25 16:53         ` Stefano Stabellini
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 3/4] xen/arm: handle GICH register changes for " Frediano Ziglio
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 4/4] xen/arm: Force dom0 to use normal GICv2 driver on Hip04 platform Frediano Ziglio
  3 siblings, 1 reply; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-25 15:28 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini, Tim Deegan, Julien Grall,
	frediano.ziglio
  Cc: zoltan.kiss, xen-devel

The GIC in this platform is mainly compatible with the standard
GICv2 beside:
- ITARGET is extended to 16 bit to support 16 CPUs;
- SGI mask is extended to support 16 CPUs;
- maximum supported interrupt is 510.

Use nr_lines to check for maximum irq supported. hip04-d01 support less
interrupts due to some field restriction. Any value above this is already
an error.

Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
Signed-off-by: Zoltan Kiss <zoltan.kiss@huawei.com>
---
 xen/arch/arm/domain_build.c |  1 +
 xen/arch/arm/gic-hip04.c    | 43 +++++++++++++++++++++++++------------------
 xen/arch/arm/gic.c          |  3 ++-
 xen/include/asm-arm/gic.h   |  3 +++
 4 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index c2dcb49..0834053 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1038,6 +1038,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
     static const struct dt_device_match gic_matches[] __initconst =
     {
         DT_MATCH_GIC_V2,
+        DT_MATCH_GIC_HIP04,
         DT_MATCH_GIC_V3,
         { /* sentinel */ },
     };
diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
index a401e3f..9a7ed46 100644
--- a/xen/arch/arm/gic-hip04.c
+++ b/xen/arch/arm/gic-hip04.c
@@ -1,7 +1,8 @@
 /*
- * xen/arch/arm/gic-v2.c
+ * xen/arch/arm/gic-hip04.c
  *
- * ARM Generic Interrupt Controller support v2
+ * ARM Generic Interrupt Controller support for HiSilicon Hip04 platform
+ * Based heavily from gic-v2.c
  *
  * Tim Deegan <tim@xen.org>
  * Copyright (c) 2011 Citrix Systems.
@@ -79,16 +80,24 @@ static struct gic_info gicv2_info;
  * logical CPU numbering. Let's use mapping as returned by the GIC
  * itself
  */
-static DEFINE_PER_CPU(u8, gic_cpu_id);
+static DEFINE_PER_CPU(u16, gic_cpu_id);
 
 /* Maximum cpu interface per GIC */
-#define NR_GIC_CPU_IF 8
+#define NR_GIC_CPU_IF 16
+
+#undef GICD_SGI_TARGET_SHIFT
+#define GICD_SGI_TARGET_SHIFT 8
 
 static inline void writeb_gicd(uint8_t val, unsigned int offset)
 {
     writeb_relaxed(val, gicv2.map_dbase + offset);
 }
 
+static inline void writew_gicd(uint16_t val, unsigned int offset)
+{
+    writew_relaxed(val, gicv2.map_dbase + offset);
+}
+
 static inline void writel_gicd(uint32_t val, unsigned int offset)
 {
     writel_relaxed(val, gicv2.map_dbase + offset);
@@ -230,7 +239,7 @@ static void gicv2_set_irq_properties(struct irq_desc *desc,
     writel_gicd(cfg, GICD_ICFGR + (irq / 16) * 4);
 
     /* Set target CPU mask (RAZ/WI on uniprocessor) */
-    writeb_gicd(mask, GICD_ITARGETSR + irq);
+    writew_gicd(mask, GICD_ITARGETSR + irq * 2);
     /* Set priority */
     writeb_gicd(priority, GICD_IPRIORITYR + irq);
 
@@ -244,8 +253,7 @@ static void __init gicv2_dist_init(void)
     uint32_t gic_cpus;
     int i;
 
-    cpumask = readl_gicd(GICD_ITARGETSR) & 0xff;
-    cpumask |= cpumask << 8;
+    cpumask = readl_gicd(GICD_ITARGETSR) & 0xffff;
     cpumask |= cpumask << 16;
 
     /* Disable the distributor */
@@ -253,7 +261,7 @@ static void __init gicv2_dist_init(void)
 
     type = readl_gicd(GICD_TYPER);
     gicv2_info.nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
-    gic_cpus = 1 + ((type & GICD_TYPE_CPUS) >> 5);
+    gic_cpus = 16;
     printk("GICv2: %d lines, %d cpu%s%s (IID %8.8x).\n",
            gicv2_info.nr_lines, gic_cpus, (gic_cpus == 1) ? "" : "s",
            (type & GICD_TYPE_SEC) ? ", secure" : "",
@@ -264,8 +272,8 @@ static void __init gicv2_dist_init(void)
         writel_gicd(0x0, GICD_ICFGR + (i / 16) * 4);
 
     /* Route all global IRQs to this CPU */
-    for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
-        writel_gicd(cpumask, GICD_ITARGETSR + (i / 4) * 4);
+    for ( i = 32; i < gicv2_info.nr_lines; i += 2 )
+        writel_gicd(cpumask, GICD_ITARGETSR + (i / 2) * 4);
 
     /* Default priority for global interrupts */
     for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
@@ -285,7 +293,7 @@ static void __cpuinit gicv2_cpu_init(void)
 {
     int i;
 
-    this_cpu(gic_cpu_id) = readl_gicd(GICD_ITARGETSR) & 0xff;
+    this_cpu(gic_cpu_id) = readl_gicd(GICD_ITARGETSR) & 0xffff;
 
     /* The first 32 interrupts (PPI and SGI) are banked per-cpu, so
      * even though they are controlled with GICD registers, they must
@@ -579,7 +587,7 @@ static void gicv2_irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_m
     mask = gicv2_cpu_mask(cpu_mask);
 
     /* Set target CPU mask (RAZ/WI on uniprocessor) */
-    writeb_gicd(mask, GICD_ITARGETSR + desc->irq);
+    writew_gicd(mask, GICD_ITARGETSR + desc->irq * 2);
 
     spin_unlock(&gicv2.lock);
 }
@@ -765,19 +773,18 @@ static int __init gicv2_init(struct dt_device_node *node, const void *data)
     return 0;
 }
 
-static const char * const gicv2_dt_compat[] __initconst =
+static const char * const hip04_gicv2_dt_compat[] __initconst =
 {
-    DT_COMPAT_GIC_CORTEX_A15,
-    DT_COMPAT_GIC_CORTEX_A7,
-    DT_COMPAT_GIC_400,
+    DT_COMPAT_GIC_HIP04,
     NULL
 };
 
-DT_DEVICE_START(gicv2, "GICv2:", DEVICE_GIC)
-        .compatible = gicv2_dt_compat,
+DT_DEVICE_START(hip04_gicv2, "GICv2:", DEVICE_GIC)
+        .compatible = hip04_gicv2_dt_compat,
         .init = gicv2_init,
 DT_DEVICE_END
 
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 390c8b0..e4512a8 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -565,12 +565,13 @@ static void do_sgi(struct cpu_user_regs *regs, enum gic_sgi sgi)
 void gic_interrupt(struct cpu_user_regs *regs, int is_fiq)
 {
     unsigned int irq;
+    unsigned int max_irq = gic_hw_ops->info->nr_lines;
 
     do  {
         /* Reading IRQ will ACK it */
         irq = gic_hw_ops->read_irq();
 
-        if ( likely(irq >= 16 && irq < 1021) )
+        if ( likely(irq >= 16 && irq < max_irq) )
         {
             local_irq_enable();
             do_IRQ(regs, irq, is_fiq);
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 187dc46..f245c50 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -155,11 +155,14 @@
 #define DT_COMPAT_GIC_400            "arm,gic-400"
 #define DT_COMPAT_GIC_CORTEX_A15     "arm,cortex-a15-gic"
 #define DT_COMPAT_GIC_CORTEX_A7      "arm,cortex-a7-gic"
+#define DT_COMPAT_GIC_HIP04          "hisilicon,hip04-intc"
 
 #define DT_MATCH_GIC_V2 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A15), \
                         DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A7), \
                         DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_400)
 
+#define DT_MATCH_GIC_HIP04 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_HIP04)
+
 #define DT_COMPAT_GIC_V3             "arm,gic-v3"
 
 #define DT_MATCH_GIC_V3 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_V3)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5.99.1 RFC 3/4] xen/arm: handle GICH register changes for hip04-d01 platform
  2015-02-25 15:28     ` [PATCH v5.99.1 RFC 0/4] xen/arm: Add support for Huawei " Frediano Ziglio
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version Frediano Ziglio
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 2/4] xen/arm: Make gic-v2 code handle hip04-d01 platform Frediano Ziglio
@ 2015-02-25 15:28       ` Frediano Ziglio
  2015-02-25 16:53         ` Stefano Stabellini
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 4/4] xen/arm: Force dom0 to use normal GICv2 driver on Hip04 platform Frediano Ziglio
  3 siblings, 1 reply; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-25 15:28 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini, Tim Deegan, Julien Grall,
	frediano.ziglio
  Cc: zoltan.kiss, xen-devel

The GICH in this platform is mainly compatible with the standard
GICv2 beside APR and LR register offsets.

Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
---
 xen/arch/arm/gic-hip04.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
index 9a7ed46..a1ae520 100644
--- a/xen/arch/arm/gic-hip04.c
+++ b/xen/arch/arm/gic-hip04.c
@@ -88,6 +88,11 @@ static DEFINE_PER_CPU(u16, gic_cpu_id);
 #undef GICD_SGI_TARGET_SHIFT
 #define GICD_SGI_TARGET_SHIFT 8
 
+#undef GICH_APR
+#undef GICH_LR
+#define GICH_APR   0x70
+#define GICH_LR    0x80
+
 static inline void writeb_gicd(uint8_t val, unsigned int offset)
 {
     writeb_relaxed(val, gicv2.map_dbase + offset);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5.99.1 RFC 4/4] xen/arm: Force dom0 to use normal GICv2 driver on Hip04 platform
  2015-02-25 15:28     ` [PATCH v5.99.1 RFC 0/4] xen/arm: Add support for Huawei " Frediano Ziglio
                         ` (2 preceding siblings ...)
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 3/4] xen/arm: handle GICH register changes for " Frediano Ziglio
@ 2015-02-25 15:28       ` Frediano Ziglio
  2015-02-25 16:54         ` Stefano Stabellini
  3 siblings, 1 reply; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-25 15:28 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini, Tim Deegan, Julien Grall,
	frediano.ziglio
  Cc: zoltan.kiss, xen-devel

Until vGIC support is not implemented and tested, this will prevent
guest kernels to use their Hip04 driver, or crash when they don't
have any.

Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
---
 xen/arch/arm/gic-hip04.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
index a1ae520..afd2622 100644
--- a/xen/arch/arm/gic-hip04.c
+++ b/xen/arch/arm/gic-hip04.c
@@ -601,17 +601,13 @@ static int gicv2_make_dt_node(const struct domain *d,
                               const struct dt_device_node *node, void *fdt)
 {
     const struct dt_device_node *gic = dt_interrupt_controller;
-    const void *compatible = NULL;
+    const void *compatible;
     u32 len;
     const __be32 *regs;
     int res = 0;
 
-    compatible = dt_get_property(gic, "compatible", &len);
-    if ( !compatible )
-    {
-        dprintk(XENLOG_ERR, "Can't find compatible property for the gic node\n");
-        return -FDT_ERR_XEN(ENOENT);
-    }
+    compatible = DT_COMPAT_GIC_CORTEX_A15;
+    len = strlen((char*) compatible) + 1;
 
     res = fdt_begin_node(fdt, "interrupt-controller");
     if ( res )
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version Frediano Ziglio
@ 2015-02-25 15:34         ` Julien Grall
  2015-02-25 15:42           ` Frediano Ziglio
                             ` (2 more replies)
  2015-02-25 16:34         ` Stefano Stabellini
  1 sibling, 3 replies; 32+ messages in thread
From: Julien Grall @ 2015-02-25 15:34 UTC (permalink / raw)
  To: Frediano Ziglio, Ian Campbell, Stefano Stabellini, Tim Deegan
  Cc: zoltan.kiss, xen-devel

Hi Frediano,

On 25/02/15 15:28, Frediano Ziglio wrote:
> HiSilison Hip04 platform use a slightly different version

I honestly don't like the idea to copy the whole GIC-v2 drivers. It will
require more maintenance for us.

Is there any way to introduce callbacks in the GICv2 code where the
hisilicon hardware differs?

> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> ---
>  xen/arch/arm/Makefile    |   2 +-
>  xen/arch/arm/gic-hip04.c | 788 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 789 insertions(+), 1 deletion(-)
>  create mode 100644 xen/arch/arm/gic-hip04.c
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 41aba2e..6fb8ba9 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -11,7 +11,7 @@ obj-y += vpsci.o
>  obj-y += domctl.o
>  obj-y += sysctl.o
>  obj-y += domain_build.o
> -obj-y += gic.o gic-v2.o
> +obj-y += gic.o gic-v2.o gic-hip04.o

As you do a verbatim copy, you should enable to compilation on the next
patch.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  2015-02-25 15:34         ` Julien Grall
@ 2015-02-25 15:42           ` Frediano Ziglio
  2015-02-25 15:46           ` Ian Campbell
  2015-02-25 15:47           ` Frediano Ziglio
  2 siblings, 0 replies; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-25 15:42 UTC (permalink / raw)
  To: Julien Grall, Ian Campbell, Stefano Stabellini, Tim Deegan
  Cc: Zoltan Kiss, xen-devel

> 
> Hi Frediano,
> 
> On 25/02/15 15:28, Frediano Ziglio wrote:
> > HiSilison Hip04 platform use a slightly different version
> 
> I honestly don't like the idea to copy the whole GIC-v2 drivers. It
> will require more maintenance for us.
> 
> Is there any way to introduce callbacks in the GICv2 code where the
> hisilicon hardware differs?
> 

This is why I introduced RFC back. I proposed two technique, one is copy (this), the other is templating. I'll send the other implementation (once done). If I understood function callback (that require quite a lot of changes and slow down execution) was not an option.

> > Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> > ---
> >  xen/arch/arm/Makefile    |   2 +-
> >  xen/arch/arm/gic-hip04.c | 788
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 789 insertions(+), 1 deletion(-)  create mode
> 100644
> > xen/arch/arm/gic-hip04.c
> >
> > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index
> > 41aba2e..6fb8ba9 100644
> > --- a/xen/arch/arm/Makefile
> > +++ b/xen/arch/arm/Makefile
> > @@ -11,7 +11,7 @@ obj-y += vpsci.o
> >  obj-y += domctl.o
> >  obj-y += sysctl.o
> >  obj-y += domain_build.o
> > -obj-y += gic.o gic-v2.o
> > +obj-y += gic.o gic-v2.o gic-hip04.o
> 
> As you do a verbatim copy, you should enable to compilation on the next
> patch.
> 

No, this is done on purpose as git does not support copy.

This make easier to understand the changes from the base which now is present as single commit.

Frediano

> Regards,
> 
> --
> Julien Grall

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  2015-02-25 15:34         ` Julien Grall
  2015-02-25 15:42           ` Frediano Ziglio
@ 2015-02-25 15:46           ` Ian Campbell
  2015-02-25 15:47           ` Frediano Ziglio
  2 siblings, 0 replies; 32+ messages in thread
From: Ian Campbell @ 2015-02-25 15:46 UTC (permalink / raw)
  To: Julien Grall
  Cc: Frediano Ziglio, Tim Deegan, xen-devel, Stefano Stabellini, zoltan.kiss

On Wed, 2015-02-25 at 15:34 +0000, Julien Grall wrote:
> Hi Frediano,
> 
> On 25/02/15 15:28, Frediano Ziglio wrote:
> > HiSilison Hip04 platform use a slightly different version
> 
> I honestly don't like the idea to copy the whole GIC-v2 drivers. It will
> require more maintenance for us.

Will it? This puts the hip driver in its own file where I presume
Frediano et al will take care of it.

> Is there any way to introduce callbacks in the GICv2 code where the
> hisilicon hardware differs?

That's even worse than the first approach (if (hip()) in gic-v2.c) IMHO.

The if-s (or hooks) would be a maintenance burden in the gic-v2.c
driver, making maintenance harder for all the other platforms which
actually followed the spec.

> 
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> > ---
> >  xen/arch/arm/Makefile    |   2 +-
> >  xen/arch/arm/gic-hip04.c | 788 +++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 789 insertions(+), 1 deletion(-)
> >  create mode 100644 xen/arch/arm/gic-hip04.c
> > 
> > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> > index 41aba2e..6fb8ba9 100644
> > --- a/xen/arch/arm/Makefile
> > +++ b/xen/arch/arm/Makefile
> > @@ -11,7 +11,7 @@ obj-y += vpsci.o
> >  obj-y += domctl.o
> >  obj-y += sysctl.o
> >  obj-y += domain_build.o
> > -obj-y += gic.o gic-v2.o
> > +obj-y += gic.o gic-v2.o gic-hip04.o
> 
> As you do a verbatim copy, you should enable to compilation on the next
> patch.
> 
> Regards,
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  2015-02-25 15:34         ` Julien Grall
  2015-02-25 15:42           ` Frediano Ziglio
  2015-02-25 15:46           ` Ian Campbell
@ 2015-02-25 15:47           ` Frediano Ziglio
  2 siblings, 0 replies; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-25 15:47 UTC (permalink / raw)
  To: Julien Grall, Ian Campbell, Stefano Stabellini, Tim Deegan
  Cc: Zoltan Kiss, xen-devel

> 
> Hi Frediano,
> 
> On 25/02/15 15:28, Frediano Ziglio wrote:
> > HiSilison Hip04 platform use a slightly different version
> 
> I honestly don't like the idea to copy the whole GIC-v2 drivers. It
> will require more maintenance for us.
> 
> Is there any way to introduce callbacks in the GICv2 code where the
> hisilicon hardware differs?
> 
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> > ---
> >  xen/arch/arm/Makefile    |   2 +-
> >  xen/arch/arm/gic-hip04.c | 788
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 789 insertions(+), 1 deletion(-)  create mode
> 100644
> > xen/arch/arm/gic-hip04.c
> >
> > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index
> > 41aba2e..6fb8ba9 100644
> > --- a/xen/arch/arm/Makefile
> > +++ b/xen/arch/arm/Makefile
> > @@ -11,7 +11,7 @@ obj-y += vpsci.o
> >  obj-y += domctl.o
> >  obj-y += sysctl.o
> >  obj-y += domain_build.o
> > -obj-y += gic.o gic-v2.o
> > +obj-y += gic.o gic-v2.o gic-hip04.o
> 
> As you do a verbatim copy, you should enable to compilation on the next
> patch.
> 

I understood now what you where saying. Yes, you are right, single patch should just contains the copy

Frediano

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version Frediano Ziglio
  2015-02-25 15:34         ` Julien Grall
@ 2015-02-25 16:34         ` Stefano Stabellini
  2015-02-25 16:59           ` Ian Campbell
  2015-02-26 17:39           ` Ian Campbell
  1 sibling, 2 replies; 32+ messages in thread
From: Stefano Stabellini @ 2015-02-25 16:34 UTC (permalink / raw)
  To: Frediano Ziglio
  Cc: zoltan.kiss, Ian Campbell, Julien Grall, Tim Deegan, xen-devel,
	Stefano Stabellini

On Wed, 25 Feb 2015, Frediano Ziglio wrote:
> HiSilison Hip04 platform use a slightly different version
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>

I think that this is preferable to the previous approach of modifying
the existing gic-v2 driver, after all the hip04 interrupt controller is
not a gicv2.



>  xen/arch/arm/Makefile    |   2 +-
>  xen/arch/arm/gic-hip04.c | 788 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 789 insertions(+), 1 deletion(-)
>  create mode 100644 xen/arch/arm/gic-hip04.c

I am concerned about the increased size of the Xen binary as a result of
the introduction of this driver.

I think we should disable the build of all drivers in Xen by default,
except for the ARM standard compliant ones (for aarch64 the SBSA is a
nice summary of what is considered compliant), to keep the size of the
binary small.

Could you please introduce a Xen build time option in
xen/arch/arm/Rules.mk, called HAS_NON_STANDARD_DRIVERS, that by default
is n, and gate the build of gic-hip04.c on it?



> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 41aba2e..6fb8ba9 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -11,7 +11,7 @@ obj-y += vpsci.o
>  obj-y += domctl.o
>  obj-y += sysctl.o
>  obj-y += domain_build.o
> -obj-y += gic.o gic-v2.o
> +obj-y += gic.o gic-v2.o gic-hip04.o
>  obj-$(CONFIG_ARM_64) += gic-v3.o
>  obj-y += io.o
>  obj-y += irq.o
> diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
> new file mode 100644
> index 0000000..a401e3f
> --- /dev/null
> +++ b/xen/arch/arm/gic-hip04.c
> @@ -0,0 +1,788 @@
> +/*
> + * xen/arch/arm/gic-v2.c

gic-hip04.c


> + * ARM Generic Interrupt Controller support v2

This is not an ARM Generic Interrupt Controller v2


> + * Tim Deegan <tim@xen.org>
> + * Copyright (c) 2011 Citrix Systems.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <xen/config.h>
> +#include <xen/lib.h>
> +#include <xen/init.h>
> +#include <xen/mm.h>
> +#include <xen/irq.h>
> +#include <xen/sched.h>
> +#include <xen/errno.h>
> +#include <xen/softirq.h>
> +#include <xen/list.h>
> +#include <xen/device_tree.h>
> +#include <xen/libfdt/libfdt.h>
> +#include <asm/p2m.h>
> +#include <asm/domain.h>
> +#include <asm/platform.h>
> +#include <asm/device.h>
> +
> +#include <asm/io.h>
> +#include <asm/gic.h>
> +
> +/*
> + * LR register definitions are GIC v2 specific.
> + * Moved these definitions from header file to here
> + */
> +#define GICH_V2_LR_VIRTUAL_MASK    0x3ff
> +#define GICH_V2_LR_VIRTUAL_SHIFT   0
> +#define GICH_V2_LR_PHYSICAL_MASK   0x3ff
> +#define GICH_V2_LR_PHYSICAL_SHIFT  10
> +#define GICH_V2_LR_STATE_MASK      0x3
> +#define GICH_V2_LR_STATE_SHIFT     28
> +#define GICH_V2_LR_PRIORITY_SHIFT  23
> +#define GICH_V2_LR_PRIORITY_MASK   0x1f
> +#define GICH_V2_LR_HW_SHIFT        31
> +#define GICH_V2_LR_HW_MASK         0x1
> +#define GICH_V2_LR_GRP_SHIFT       30
> +#define GICH_V2_LR_GRP_MASK        0x1
> +#define GICH_V2_LR_MAINTENANCE_IRQ (1<<19)
> +#define GICH_V2_LR_GRP1            (1<<30)
> +#define GICH_V2_LR_HW              (1<<31)
> +#define GICH_V2_LR_CPUID_SHIFT     9
> +#define GICH_V2_VTR_NRLRGS         0x3f
> +
> +#define GICH_V2_VMCR_PRIORITY_MASK   0x1f
> +#define GICH_V2_VMCR_PRIORITY_SHIFT  27

Please rename all constants, definitions and function names to something
else to make it clear that this driver is not for a gicv2.
Replacing gicv2 with hip04-gic would be a good start.


> +/* Global state */
> +static struct {
> +    paddr_t dbase;            /* Address of distributor registers */
> +    void __iomem * map_dbase; /* IO mapped Address of distributor registers */
> +    paddr_t cbase;            /* Address of CPU interface registers */
> +    void __iomem * map_cbase[2]; /* IO mapped Address of CPU interface registers */
> +    paddr_t hbase;            /* Address of virtual interface registers */
> +    void __iomem * map_hbase; /* IO Address of virtual interface registers */
> +    paddr_t vbase;            /* Address of virtual cpu interface registers */
> +    spinlock_t lock;
> +} gicv2;
> +
> +static struct gic_info gicv2_info;
> +
> +/* The GIC mapping of CPU interfaces does not necessarily match the
> + * logical CPU numbering. Let's use mapping as returned by the GIC
> + * itself
> + */
> +static DEFINE_PER_CPU(u8, gic_cpu_id);
> +
> +/* Maximum cpu interface per GIC */
> +#define NR_GIC_CPU_IF 8
> +
> +static inline void writeb_gicd(uint8_t val, unsigned int offset)
> +{
> +    writeb_relaxed(val, gicv2.map_dbase + offset);
> +}
> +
> +static inline void writel_gicd(uint32_t val, unsigned int offset)
> +{
> +    writel_relaxed(val, gicv2.map_dbase + offset);
> +}
> +
> +static inline uint32_t readl_gicd(unsigned int offset)
> +{
> +    return readl_relaxed(gicv2.map_dbase + offset);
> +}
> +
> +static inline void writel_gicc(uint32_t val, unsigned int offset)
> +{
> +    unsigned int page = offset >> PAGE_SHIFT;
> +    offset &= ~PAGE_MASK;
> +    writel_relaxed(val, gicv2.map_cbase[page] + offset);
> +}
> +
> +static inline uint32_t readl_gicc(unsigned int offset)
> +{
> +    unsigned int page = offset >> PAGE_SHIFT;
> +    offset &= ~PAGE_MASK;
> +    return readl_relaxed(gicv2.map_cbase[page] + offset);
> +}
> +
> +static inline void writel_gich(uint32_t val, unsigned int offset)
> +{
> +    writel_relaxed(val, gicv2.map_hbase + offset);
> +}
> +
> +static inline uint32_t readl_gich(int unsigned offset)
> +{
> +    return readl_relaxed(gicv2.map_hbase + offset);
> +}
> +
> +static unsigned int gicv2_cpu_mask(const cpumask_t *cpumask)
> +{
> +    unsigned int cpu;
> +    unsigned int mask = 0;
> +    cpumask_t possible_mask;
> +
> +    cpumask_and(&possible_mask, cpumask, &cpu_possible_map);
> +    for_each_cpu( cpu, &possible_mask )
> +    {
> +        ASSERT(cpu < NR_GIC_CPU_IF);
> +        mask |= per_cpu(gic_cpu_id, cpu);
> +    }
> +
> +    return mask;
> +}
> +
> +static void gicv2_save_state(struct vcpu *v)
> +{
> +    int i;
> +
> +    /* No need for spinlocks here because interrupts are disabled around
> +     * this call and it only accesses struct vcpu fields that cannot be
> +     * accessed simultaneously by another pCPU.
> +     */
> +    for ( i = 0; i < gicv2_info.nr_lrs; i++ )
> +        v->arch.gic.v2.lr[i] = readl_gich(GICH_LR + i * 4);
> +
> +    v->arch.gic.v2.apr = readl_gich(GICH_APR);
> +    v->arch.gic.v2.vmcr = readl_gich(GICH_VMCR);
> +    /* Disable until next VCPU scheduled */
> +    writel_gich(0, GICH_HCR);
> +}
> +
> +static void gicv2_restore_state(const struct vcpu *v)
> +{
> +    int i;
> +
> +    for ( i = 0; i < gicv2_info.nr_lrs; i++ )
> +        writel_gich(v->arch.gic.v2.lr[i], GICH_LR + i * 4);
> +
> +    writel_gich(v->arch.gic.v2.apr, GICH_APR);
> +    writel_gich(v->arch.gic.v2.vmcr, GICH_VMCR);
> +    writel_gich(GICH_HCR_EN, GICH_HCR);
> +}
> +
> +static void gicv2_dump_state(const struct vcpu *v)
> +{
> +    int i;
> +
> +    if ( v == current )
> +    {
> +        for ( i = 0; i < gicv2_info.nr_lrs; i++ )
> +            printk("   HW_LR[%d]=%x\n", i,
> +                   readl_gich(GICH_LR + i * 4));
> +    }
> +    else
> +    {
> +        for ( i = 0; i < gicv2_info.nr_lrs; i++ )
> +            printk("   VCPU_LR[%d]=%x\n", i, v->arch.gic.v2.lr[i]);
> +    }
> +}
> +
> +static void gicv2_eoi_irq(struct irq_desc *irqd)
> +{
> +    int irq = irqd->irq;
> +    /* Lower the priority */
> +    writel_gicc(irq, GICC_EOIR);
> +}
> +
> +static void gicv2_dir_irq(struct irq_desc *irqd)
> +{
> +    /* Deactivate */
> +    writel_gicc(irqd->irq, GICC_DIR);
> +}
> +
> +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)
> +{
> +    uint32_t cfg, edgebit;
> +    unsigned int mask = gicv2_cpu_mask(cpu_mask);
> +    unsigned int irq = desc->irq;
> +    unsigned int type = desc->arch.type;
> +
> +    ASSERT(type != DT_IRQ_TYPE_INVALID);
> +    ASSERT(spin_is_locked(&desc->lock));
> +
> +    spin_lock(&gicv2.lock);
> +    /* Set edge / level */
> +    cfg = readl_gicd(GICD_ICFGR + (irq / 16) * 4);
> +    edgebit = 2u << (2 * (irq % 16));
> +    if ( type & DT_IRQ_TYPE_LEVEL_MASK )
> +        cfg &= ~edgebit;
> +    else if ( type & DT_IRQ_TYPE_EDGE_BOTH )
> +        cfg |= edgebit;
> +    writel_gicd(cfg, GICD_ICFGR + (irq / 16) * 4);
> +
> +    /* Set target CPU mask (RAZ/WI on uniprocessor) */
> +    writeb_gicd(mask, GICD_ITARGETSR + irq);
> +    /* Set priority */
> +    writeb_gicd(priority, GICD_IPRIORITYR + irq);
> +
> +    spin_unlock(&gicv2.lock);
> +}
> +
> +static void __init gicv2_dist_init(void)
> +{
> +    uint32_t type;
> +    uint32_t cpumask;
> +    uint32_t gic_cpus;
> +    int i;
> +
> +    cpumask = readl_gicd(GICD_ITARGETSR) & 0xff;
> +    cpumask |= cpumask << 8;
> +    cpumask |= cpumask << 16;
> +
> +    /* Disable the distributor */
> +    writel_gicd(0, GICD_CTLR);
> +
> +    type = readl_gicd(GICD_TYPER);
> +    gicv2_info.nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
> +    gic_cpus = 1 + ((type & GICD_TYPE_CPUS) >> 5);
> +    printk("GICv2: %d lines, %d cpu%s%s (IID %8.8x).\n",
> +           gicv2_info.nr_lines, gic_cpus, (gic_cpus == 1) ? "" : "s",
> +           (type & GICD_TYPE_SEC) ? ", secure" : "",
> +           readl_gicd(GICD_IIDR));
> +
> +    /* Default all global IRQs to level, active low */
> +    for ( i = 32; i < gicv2_info.nr_lines; i += 16 )
> +        writel_gicd(0x0, GICD_ICFGR + (i / 16) * 4);
> +
> +    /* Route all global IRQs to this CPU */
> +    for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
> +        writel_gicd(cpumask, GICD_ITARGETSR + (i / 4) * 4);
> +
> +    /* Default priority for global interrupts */
> +    for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
> +        writel_gicd(GIC_PRI_IRQ << 24 | GIC_PRI_IRQ << 16 |
> +                    GIC_PRI_IRQ << 8 | GIC_PRI_IRQ,
> +                    GICD_IPRIORITYR + (i / 4) * 4);
> +
> +    /* Disable all global interrupts */
> +    for ( i = 32; i < gicv2_info.nr_lines; i += 32 )
> +        writel_gicd(~0x0, GICD_ICENABLER + (i / 32) * 4);
> +
> +    /* Turn on the distributor */
> +    writel_gicd(GICD_CTL_ENABLE, GICD_CTLR);
> +}
> +
> +static void __cpuinit gicv2_cpu_init(void)
> +{
> +    int i;
> +
> +    this_cpu(gic_cpu_id) = readl_gicd(GICD_ITARGETSR) & 0xff;
> +
> +    /* The first 32 interrupts (PPI and SGI) are banked per-cpu, so
> +     * even though they are controlled with GICD registers, they must
> +     * be set up here with the other per-cpu state. */
> +    writel_gicd(0xffff0000, GICD_ICENABLER); /* Disable all PPI */
> +    writel_gicd(0x0000ffff, GICD_ISENABLER); /* Enable all SGI */
> +
> +    /* Set SGI priorities */
> +    for ( i = 0; i < 16; i += 4 )
> +        writel_gicd(GIC_PRI_IPI << 24 | GIC_PRI_IPI << 16 |
> +                    GIC_PRI_IPI << 8 | GIC_PRI_IPI,
> +                    GICD_IPRIORITYR + (i / 4) * 4);
> +
> +    /* Set PPI priorities */
> +    for ( i = 16; i < 32; i += 4 )
> +        writel_gicd(GIC_PRI_IRQ << 24 | GIC_PRI_IRQ << 16 |
> +                    GIC_PRI_IRQ << 8 | GIC_PRI_IRQ,
> +                    GICD_IPRIORITYR + (i / 4) * 4);
> +
> +    /* Local settings: interface controller */
> +    /* Don't mask by priority */
> +    writel_gicc(0xff, GICC_PMR);
> +    /* Finest granularity of priority */
> +    writel_gicc(0x0, GICC_BPR);
> +    /* Turn on delivery */
> +    writel_gicc(GICC_CTL_ENABLE|GICC_CTL_EOI, GICC_CTLR);
> +}
> +
> +static void gicv2_cpu_disable(void)
> +{
> +    writel_gicc(0x0, GICC_CTLR);
> +}
> +
> +static void __cpuinit gicv2_hyp_init(void)
> +{
> +    uint32_t vtr;
> +    uint8_t nr_lrs;
> +
> +    vtr = readl_gich(GICH_VTR);
> +    nr_lrs  = (vtr & GICH_V2_VTR_NRLRGS) + 1;
> +    gicv2_info.nr_lrs = nr_lrs;
> +
> +    writel_gich(GICH_MISR_EOI, GICH_MISR);
> +}
> +
> +static void __cpuinit gicv2_hyp_disable(void)
> +{
> +    writel_gich(0, GICH_HCR);
> +}
> +
> +static int gicv2_secondary_cpu_init(void)
> +{
> +    spin_lock(&gicv2.lock);
> +
> +    gicv2_cpu_init();
> +    gicv2_hyp_init();
> +
> +    spin_unlock(&gicv2.lock);
> +
> +    return 0;
> +}
> +
> +static void gicv2_send_SGI(enum gic_sgi sgi, enum gic_sgi_mode irqmode,
> +                           const cpumask_t *cpu_mask)
> +{
> +    unsigned int mask = 0;
> +    cpumask_t online_mask;
> +
> +    switch ( irqmode )
> +    {
> +    case SGI_TARGET_OTHERS:
> +        writel_gicd(GICD_SGI_TARGET_OTHERS | sgi, GICD_SGIR);
> +        break;
> +    case SGI_TARGET_SELF:
> +        writel_gicd(GICD_SGI_TARGET_SELF | sgi, GICD_SGIR);
> +        break;
> +    case SGI_TARGET_LIST:
> +        cpumask_and(&online_mask, cpu_mask, &cpu_online_map);
> +        mask = gicv2_cpu_mask(&online_mask);
> +        writel_gicd(GICD_SGI_TARGET_LIST |
> +                    (mask << GICD_SGI_TARGET_SHIFT) | sgi,
> +                    GICD_SGIR);
> +        break;
> +    default:
> +        BUG();
> +    }
> +}
> +
> +/* Shut down the per-CPU GIC interface */
> +static void gicv2_disable_interface(void)
> +{
> +    spin_lock(&gicv2.lock);
> +    gicv2_cpu_disable();
> +    gicv2_hyp_disable();
> +    spin_unlock(&gicv2.lock);
> +}
> +
> +static void gicv2_update_lr(int lr, const struct pending_irq *p,
> +                            unsigned int state)
> +{
> +    uint32_t lr_reg;
> +
> +    BUG_ON(lr >= gicv2_info.nr_lrs);
> +    BUG_ON(lr < 0);
> +
> +    lr_reg = (((state & GICH_V2_LR_STATE_MASK) << GICH_V2_LR_STATE_SHIFT)  |
> +              ((GIC_PRI_TO_GUEST(p->priority) & GICH_V2_LR_PRIORITY_MASK)
> +                                             << GICH_V2_LR_PRIORITY_SHIFT) |
> +              ((p->irq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT));
> +
> +    if ( p->desc != NULL )
> +    {
> +        if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) )
> +            lr_reg |= GICH_V2_LR_MAINTENANCE_IRQ;
> +        else
> +            lr_reg |= GICH_V2_LR_HW | ((p->desc->irq & GICH_V2_LR_PHYSICAL_MASK )
> +                            << GICH_V2_LR_PHYSICAL_SHIFT);
> +    }
> +
> +    writel_gich(lr_reg, GICH_LR + lr * 4);
> +}
> +
> +static void gicv2_clear_lr(int lr)
> +{
> +    writel_gich(0, GICH_LR + lr * 4);
> +}
> +
> +static int gicv2v_setup(struct domain *d)
> +{
> +    int ret;
> +
> +    /*
> +     * The hardware domain gets the hardware address.
> +     * Guests get the virtual platform layout.
> +     */
> +    if ( is_hardware_domain(d) )
> +    {
> +        d->arch.vgic.dbase = gicv2.dbase;
> +        d->arch.vgic.cbase = gicv2.cbase;
> +    }
> +    else
> +    {
> +        d->arch.vgic.dbase = GUEST_GICD_BASE;
> +        d->arch.vgic.cbase = GUEST_GICC_BASE;
> +    }
> +
> +    /*
> +     * Map the gic virtual cpu interface in the gic cpu interface
> +     * region of the guest.
> +     *
> +     * The second page is always mapped at +4K irrespective of the
> +     * GIC_64K_STRIDE quirk. The DTB passed to the guest reflects this.
> +     */
> +    ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase), 1,
> +                            paddr_to_pfn(gicv2.vbase));
> +    if ( ret )
> +        return ret;
> +
> +    if ( !platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) )
> +        ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + PAGE_SIZE),
> +                               2, paddr_to_pfn(gicv2.vbase + PAGE_SIZE));
> +    else
> +        ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + PAGE_SIZE),
> +                               2, paddr_to_pfn(gicv2.vbase + 16*PAGE_SIZE));
> +
> +    return ret;
> +}
> +
> +static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
> +{
> +    uint32_t lrv;
> +
> +    lrv          = readl_gich(GICH_LR + lr * 4);
> +    lr_reg->pirq = (lrv >> GICH_V2_LR_PHYSICAL_SHIFT) & GICH_V2_LR_PHYSICAL_MASK;
> +    lr_reg->virq = (lrv >> GICH_V2_LR_VIRTUAL_SHIFT) & GICH_V2_LR_VIRTUAL_MASK;
> +    lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) & GICH_V2_LR_PRIORITY_MASK;
> +    lr_reg->state     = (lrv >> GICH_V2_LR_STATE_SHIFT) & GICH_V2_LR_STATE_MASK;
> +    lr_reg->hw_status = (lrv >> GICH_V2_LR_HW_SHIFT) & GICH_V2_LR_HW_MASK;
> +    lr_reg->grp       = (lrv >> GICH_V2_LR_GRP_SHIFT) & GICH_V2_LR_GRP_MASK;
> +}
> +
> +static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg)
> +{
> +    uint32_t lrv = 0;
> +
> +    lrv = ( ((lr_reg->pirq & GICH_V2_LR_PHYSICAL_MASK) << GICH_V2_LR_PHYSICAL_SHIFT) |
> +          ((lr_reg->virq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT)   |
> +          ((uint32_t)(lr_reg->priority & GICH_V2_LR_PRIORITY_MASK)
> +                                      << GICH_V2_LR_PRIORITY_SHIFT) |
> +          ((uint32_t)(lr_reg->state & GICH_V2_LR_STATE_MASK)
> +                                   << GICH_V2_LR_STATE_SHIFT) |
> +          ((uint32_t)(lr_reg->hw_status & GICH_V2_LR_HW_MASK)
> +                                       << GICH_V2_LR_HW_SHIFT)  |
> +          ((uint32_t)(lr_reg->grp & GICH_V2_LR_GRP_MASK) << GICH_V2_LR_GRP_SHIFT) );
> +
> +    writel_gich(lrv, GICH_LR + lr * 4);
> +}
> +
> +static void gicv2_hcr_status(uint32_t flag, bool_t status)
> +{
> +    uint32_t hcr = readl_gich(GICH_HCR);
> +
> +    if ( status )
> +        hcr |= flag;
> +    else
> +        hcr &= (~flag);
> +
> +    writel_gich(hcr, GICH_HCR);
> +}
> +
> +static unsigned int gicv2_read_vmcr_priority(void)
> +{
> +   return ((readl_gich(GICH_VMCR) >> GICH_V2_VMCR_PRIORITY_SHIFT)
> +           & GICH_V2_VMCR_PRIORITY_MASK);
> +}
> +
> +static unsigned int gicv2_read_apr(int apr_reg)
> +{
> +   return readl_gich(GICH_APR);
> +}
> +
> +static void gicv2_irq_enable(struct irq_desc *desc)
> +{
> +    unsigned long flags;
> +    int irq = desc->irq;
> +
> +    ASSERT(spin_is_locked(&desc->lock));
> +
> +    spin_lock_irqsave(&gicv2.lock, flags);
> +    clear_bit(_IRQ_DISABLED, &desc->status);
> +    dsb(sy);
> +    /* Enable routing */
> +    writel_gicd((1u << (irq % 32)), GICD_ISENABLER + (irq / 32) * 4);
> +    spin_unlock_irqrestore(&gicv2.lock, flags);
> +}
> +
> +static void gicv2_irq_disable(struct irq_desc *desc)
> +{
> +    unsigned long flags;
> +    int irq = desc->irq;
> +
> +    ASSERT(spin_is_locked(&desc->lock));
> +
> +    spin_lock_irqsave(&gicv2.lock, flags);
> +    /* Disable routing */
> +    writel_gicd(1u << (irq % 32), GICD_ICENABLER + (irq / 32) * 4);
> +    set_bit(_IRQ_DISABLED, &desc->status);
> +    spin_unlock_irqrestore(&gicv2.lock, flags);
> +}
> +
> +static unsigned int gicv2_irq_startup(struct irq_desc *desc)
> +{
> +    gicv2_irq_enable(desc);
> +
> +    return 0;
> +}
> +
> +static void gicv2_irq_shutdown(struct irq_desc *desc)
> +{
> +    gicv2_irq_disable(desc);
> +}
> +
> +static void gicv2_irq_ack(struct irq_desc *desc)
> +{
> +    /* No ACK -- reading IAR has done this for us */
> +}
> +
> +static void gicv2_host_irq_end(struct irq_desc *desc)
> +{
> +    /* Lower the priority */
> +    gicv2_eoi_irq(desc);
> +    /* Deactivate */
> +    gicv2_dir_irq(desc);
> +}
> +
> +static void gicv2_guest_irq_end(struct irq_desc *desc)
> +{
> +    /* Lower the priority of the IRQ */
> +    gicv2_eoi_irq(desc);
> +    /* Deactivation happens in maintenance interrupt / via GICV */
> +}
> +
> +static void gicv2_irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask)
> +{
> +    unsigned int mask;
> +
> +    ASSERT(!cpumask_empty(cpu_mask));
> +
> +    spin_lock(&gicv2.lock);
> +
> +    mask = gicv2_cpu_mask(cpu_mask);
> +
> +    /* Set target CPU mask (RAZ/WI on uniprocessor) */
> +    writeb_gicd(mask, GICD_ITARGETSR + desc->irq);
> +
> +    spin_unlock(&gicv2.lock);
> +}
> +
> +static int gicv2_make_dt_node(const struct domain *d,
> +                              const struct dt_device_node *node, void *fdt)
> +{
> +    const struct dt_device_node *gic = dt_interrupt_controller;
> +    const void *compatible = NULL;
> +    u32 len;
> +    const __be32 *regs;
> +    int res = 0;
> +
> +    compatible = dt_get_property(gic, "compatible", &len);
> +    if ( !compatible )
> +    {
> +        dprintk(XENLOG_ERR, "Can't find compatible property for the gic node\n");
> +        return -FDT_ERR_XEN(ENOENT);
> +    }
> +
> +    res = fdt_begin_node(fdt, "interrupt-controller");
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property(fdt, "compatible", compatible, len);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "#interrupt-cells", 3);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property(fdt, "interrupt-controller", NULL, 0);
> +
> +    if ( res )
> +        return res;
> +
> +    /*
> +     * DTB provides up to 4 regions to handle virtualization
> +     * however dom0 just needs GICC and GICD provided by Xen.
> +     */
> +    regs = dt_get_property(gic, "reg", &len);
> +    if ( !regs )
> +    {
> +        dprintk(XENLOG_ERR, "Can't find reg property for the gic node\n");
> +        return -FDT_ERR_XEN(ENOENT);
> +    }
> +
> +    len = dt_cells_to_size(dt_n_addr_cells(node) + dt_n_size_cells(node));
> +    len *= 2;
> +
> +    res = fdt_property(fdt, "reg", regs, len);
> +
> +    return res;
> +}
> +
> +/* XXX different for level vs edge */
> +static hw_irq_controller gicv2_host_irq_type = {
> +    .typename     = "gic-v2",
> +    .startup      = gicv2_irq_startup,
> +    .shutdown     = gicv2_irq_shutdown,
> +    .enable       = gicv2_irq_enable,
> +    .disable      = gicv2_irq_disable,
> +    .ack          = gicv2_irq_ack,
> +    .end          = gicv2_host_irq_end,
> +    .set_affinity = gicv2_irq_set_affinity,
> +};
> +
> +static hw_irq_controller gicv2_guest_irq_type = {
> +    .typename     = "gic-v2",
> +    .startup      = gicv2_irq_startup,
> +    .shutdown     = gicv2_irq_shutdown,
> +    .enable       = gicv2_irq_enable,
> +    .disable      = gicv2_irq_disable,
> +    .ack          = gicv2_irq_ack,
> +    .end          = gicv2_guest_irq_end,
> +    .set_affinity = gicv2_irq_set_affinity,
> +};
> +
> +const static struct gic_hw_operations gicv2_ops = {
> +    .info                = &gicv2_info,
> +    .secondary_init      = gicv2_secondary_cpu_init,
> +    .save_state          = gicv2_save_state,
> +    .restore_state       = gicv2_restore_state,
> +    .dump_state          = gicv2_dump_state,
> +    .gicv_setup          = gicv2v_setup,
> +    .gic_host_irq_type   = &gicv2_host_irq_type,
> +    .gic_guest_irq_type  = &gicv2_guest_irq_type,
> +    .eoi_irq             = gicv2_eoi_irq,
> +    .deactivate_irq      = gicv2_dir_irq,
> +    .read_irq            = gicv2_read_irq,
> +    .set_irq_properties  = gicv2_set_irq_properties,
> +    .send_SGI            = gicv2_send_SGI,
> +    .disable_interface   = gicv2_disable_interface,
> +    .update_lr           = gicv2_update_lr,
> +    .update_hcr_status   = gicv2_hcr_status,
> +    .clear_lr            = gicv2_clear_lr,
> +    .read_lr             = gicv2_read_lr,
> +    .write_lr            = gicv2_write_lr,
> +    .read_vmcr_priority  = gicv2_read_vmcr_priority,
> +    .read_apr            = gicv2_read_apr,
> +    .make_dt_node        = gicv2_make_dt_node,
> +};
> +
> +/* Set up the GIC */
> +static int __init gicv2_init(struct dt_device_node *node, const void *data)
> +{
> +    int res;
> +
> +    dt_device_set_used_by(node, DOMID_XEN);
> +
> +    res = dt_device_get_address(node, 0, &gicv2.dbase, NULL);
> +    if ( res || !gicv2.dbase || (gicv2.dbase & ~PAGE_MASK) )
> +        panic("GICv2: Cannot find a valid address for the distributor");
> +
> +    res = dt_device_get_address(node, 1, &gicv2.cbase, NULL);
> +    if ( res || !gicv2.cbase || (gicv2.cbase & ~PAGE_MASK) )
> +        panic("GICv2: Cannot find a valid address for the CPU");
> +
> +    res = dt_device_get_address(node, 2, &gicv2.hbase, NULL);
> +    if ( res || !gicv2.hbase || (gicv2.hbase & ~PAGE_MASK) )
> +        panic("GICv2: Cannot find a valid address for the hypervisor");
> +
> +    res = dt_device_get_address(node, 3, &gicv2.vbase, NULL);
> +    if ( res || !gicv2.vbase || (gicv2.vbase & ~PAGE_MASK) )
> +        panic("GICv2: Cannot find a valid address for the virtual CPU");
> +
> +    res = platform_get_irq(node, 0);
> +    if ( res < 0 )
> +        panic("GICv2: Cannot find the maintenance IRQ");
> +    gicv2_info.maintenance_irq = res;
> +
> +    /* Set the GIC as the primary interrupt controller */
> +    dt_interrupt_controller = node;
> +
> +    /* TODO: Add check on distributor, cpu size */
> +
> +    printk("GICv2 initialization:\n"
> +              "        gic_dist_addr=%"PRIpaddr"\n"
> +              "        gic_cpu_addr=%"PRIpaddr"\n"
> +              "        gic_hyp_addr=%"PRIpaddr"\n"
> +              "        gic_vcpu_addr=%"PRIpaddr"\n"
> +              "        gic_maintenance_irq=%u\n",
> +              gicv2.dbase, gicv2.cbase, gicv2.hbase, gicv2.vbase,
> +              gicv2_info.maintenance_irq);
> +
> +    if ( (gicv2.dbase & ~PAGE_MASK) || (gicv2.cbase & ~PAGE_MASK) ||
> +         (gicv2.hbase & ~PAGE_MASK) || (gicv2.vbase & ~PAGE_MASK) )
> +        panic("GICv2 interfaces not page aligned");
> +
> +    gicv2.map_dbase = ioremap_nocache(gicv2.dbase, PAGE_SIZE);
> +    if ( !gicv2.map_dbase )
> +        panic("GICv2: Failed to ioremap for GIC distributor\n");
> +
> +    gicv2.map_cbase[0] = ioremap_nocache(gicv2.cbase, PAGE_SIZE);
> +
> +    if ( platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) )
> +        gicv2.map_cbase[1] = ioremap_nocache(gicv2.cbase + PAGE_SIZE * 0x10,
> +                                           PAGE_SIZE);
> +    else
> +        gicv2.map_cbase[1] = ioremap_nocache(gicv2.cbase + PAGE_SIZE, PAGE_SIZE);
> +
> +    if ( !gicv2.map_cbase[0] || !gicv2.map_cbase[1] )
> +        panic("GICv2: Failed to ioremap for GIC CPU interface\n");
> +
> +    gicv2.map_hbase = ioremap_nocache(gicv2.hbase, PAGE_SIZE);
> +    if ( !gicv2.map_hbase )
> +        panic("GICv2: Failed to ioremap for GIC Virtual interface\n");
> +
> +    /* Global settings: interrupt distributor */
> +    spin_lock_init(&gicv2.lock);
> +    spin_lock(&gicv2.lock);
> +
> +    gicv2_dist_init();
> +    gicv2_cpu_init();
> +    gicv2_hyp_init();
> +
> +    spin_unlock(&gicv2.lock);
> +
> +    gicv2_info.hw_version = GIC_V2;
> +    register_gic_ops(&gicv2_ops);
> +
> +    return 0;
> +}
> +
> +static const char * const gicv2_dt_compat[] __initconst =
> +{
> +    DT_COMPAT_GIC_CORTEX_A15,
> +    DT_COMPAT_GIC_CORTEX_A7,
> +    DT_COMPAT_GIC_400,
> +    NULL
> +};
> +
> +DT_DEVICE_START(gicv2, "GICv2:", DEVICE_GIC)
> +        .compatible = gicv2_dt_compat,
> +        .init = gicv2_init,
> +DT_DEVICE_END
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> -- 
> 1.9.1
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform
  2015-02-20  9:56 [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform Frediano Ziglio
                   ` (3 preceding siblings ...)
  2015-02-20  9:56 ` [PATCH v5 4/4] xen/arm: Force dom0 to use normal GICv2 driver on Hip04 platform Frediano Ziglio
@ 2015-02-25 16:43 ` Ian Campbell
  2015-02-25 17:10   ` Frediano Ziglio
  4 siblings, 1 reply; 32+ messages in thread
From: Ian Campbell @ 2015-02-25 16:43 UTC (permalink / raw)
  To: Frediano Ziglio
  Cc: Tim Deegan, Julien Grall, Stefano Stabellini, zoltan.kiss, xen-devel

On Fri, 2015-02-20 at 09:56 +0000, Frediano Ziglio wrote:
> This set of patches add Xen support for hip04-d01 platform (see
> https://wiki.linaro.org/Boards/D01 for details).

When (or where) would the general public be able to purchase one of
these systems?

Ian.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 2/4] xen/arm: Make gic-v2 code handle hip04-d01 platform
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 2/4] xen/arm: Make gic-v2 code handle hip04-d01 platform Frediano Ziglio
@ 2015-02-25 16:53         ` Stefano Stabellini
  2015-02-26  9:54           ` Frediano Ziglio
  0 siblings, 1 reply; 32+ messages in thread
From: Stefano Stabellini @ 2015-02-25 16:53 UTC (permalink / raw)
  To: Frediano Ziglio
  Cc: zoltan.kiss, Ian Campbell, Julien Grall, Tim Deegan, xen-devel,
	Stefano Stabellini

On Wed, 25 Feb 2015, Frediano Ziglio wrote:
> The GIC in this platform is mainly compatible with the standard
> GICv2 beside:
> - ITARGET is extended to 16 bit to support 16 CPUs;
> - SGI mask is extended to support 16 CPUs;
> - maximum supported interrupt is 510.
> 
> Use nr_lines to check for maximum irq supported. hip04-d01 support less
> interrupts due to some field restriction. Any value above this is already
> an error.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> Signed-off-by: Zoltan Kiss <zoltan.kiss@huawei.com>
> ---
>  xen/arch/arm/domain_build.c |  1 +
>  xen/arch/arm/gic-hip04.c    | 43 +++++++++++++++++++++++++------------------
>  xen/arch/arm/gic.c          |  3 ++-
>  xen/include/asm-arm/gic.h   |  3 +++
>  4 files changed, 31 insertions(+), 19 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index c2dcb49..0834053 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1038,6 +1038,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>      static const struct dt_device_match gic_matches[] __initconst =
>      {
>          DT_MATCH_GIC_V2,
> +        DT_MATCH_GIC_HIP04,
>          DT_MATCH_GIC_V3,
>          { /* sentinel */ },
>      };
> diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
> index a401e3f..9a7ed46 100644
> --- a/xen/arch/arm/gic-hip04.c
> +++ b/xen/arch/arm/gic-hip04.c
> @@ -1,7 +1,8 @@
>  /*
> - * xen/arch/arm/gic-v2.c
> + * xen/arch/arm/gic-hip04.c
>   *
> - * ARM Generic Interrupt Controller support v2
> + * ARM Generic Interrupt Controller support for HiSilicon Hip04 platform
> + * Based heavily from gic-v2.c
>   *
>   * Tim Deegan <tim@xen.org>
>   * Copyright (c) 2011 Citrix Systems.
> @@ -79,16 +80,24 @@ static struct gic_info gicv2_info;
>   * logical CPU numbering. Let's use mapping as returned by the GIC
>   * itself
>   */
> -static DEFINE_PER_CPU(u8, gic_cpu_id);
> +static DEFINE_PER_CPU(u16, gic_cpu_id);
>  
>  /* Maximum cpu interface per GIC */
> -#define NR_GIC_CPU_IF 8
> +#define NR_GIC_CPU_IF 16
> +
> +#undef GICD_SGI_TARGET_SHIFT
> +#define GICD_SGI_TARGET_SHIFT 8
>  
>  static inline void writeb_gicd(uint8_t val, unsigned int offset)
>  {
>      writeb_relaxed(val, gicv2.map_dbase + offset);
>  }
>  
> +static inline void writew_gicd(uint16_t val, unsigned int offset)
> +{
> +    writew_relaxed(val, gicv2.map_dbase + offset);
> +}
> +
>  static inline void writel_gicd(uint32_t val, unsigned int offset)
>  {
>      writel_relaxed(val, gicv2.map_dbase + offset);
> @@ -230,7 +239,7 @@ static void gicv2_set_irq_properties(struct irq_desc *desc,
>      writel_gicd(cfg, GICD_ICFGR + (irq / 16) * 4);
>  
>      /* Set target CPU mask (RAZ/WI on uniprocessor) */
> -    writeb_gicd(mask, GICD_ITARGETSR + irq);
> +    writew_gicd(mask, GICD_ITARGETSR + irq * 2);
>      /* Set priority */
>      writeb_gicd(priority, GICD_IPRIORITYR + irq);
>  
> @@ -244,8 +253,7 @@ static void __init gicv2_dist_init(void)
>      uint32_t gic_cpus;
>      int i;
>  
> -    cpumask = readl_gicd(GICD_ITARGETSR) & 0xff;
> -    cpumask |= cpumask << 8;
> +    cpumask = readl_gicd(GICD_ITARGETSR) & 0xffff;
>      cpumask |= cpumask << 16;
>  
>      /* Disable the distributor */
> @@ -253,7 +261,7 @@ static void __init gicv2_dist_init(void)
>  
>      type = readl_gicd(GICD_TYPER);
>      gicv2_info.nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
> -    gic_cpus = 1 + ((type & GICD_TYPE_CPUS) >> 5);
> +    gic_cpus = 16;
>      printk("GICv2: %d lines, %d cpu%s%s (IID %8.8x).\n",
>             gicv2_info.nr_lines, gic_cpus, (gic_cpus == 1) ? "" : "s",
>             (type & GICD_TYPE_SEC) ? ", secure" : "",
> @@ -264,8 +272,8 @@ static void __init gicv2_dist_init(void)
>          writel_gicd(0x0, GICD_ICFGR + (i / 16) * 4);
>  
>      /* Route all global IRQs to this CPU */
> -    for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
> -        writel_gicd(cpumask, GICD_ITARGETSR + (i / 4) * 4);
> +    for ( i = 32; i < gicv2_info.nr_lines; i += 2 )
> +        writel_gicd(cpumask, GICD_ITARGETSR + (i / 2) * 4);
>  
>      /* Default priority for global interrupts */
>      for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
> @@ -285,7 +293,7 @@ static void __cpuinit gicv2_cpu_init(void)
>  {
>      int i;
>  
> -    this_cpu(gic_cpu_id) = readl_gicd(GICD_ITARGETSR) & 0xff;
> +    this_cpu(gic_cpu_id) = readl_gicd(GICD_ITARGETSR) & 0xffff;
>  
>      /* The first 32 interrupts (PPI and SGI) are banked per-cpu, so
>       * even though they are controlled with GICD registers, they must
> @@ -579,7 +587,7 @@ static void gicv2_irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_m
>      mask = gicv2_cpu_mask(cpu_mask);
>  
>      /* Set target CPU mask (RAZ/WI on uniprocessor) */
> -    writeb_gicd(mask, GICD_ITARGETSR + desc->irq);
> +    writew_gicd(mask, GICD_ITARGETSR + desc->irq * 2);
>  
>      spin_unlock(&gicv2.lock);
>  }
> @@ -765,19 +773,18 @@ static int __init gicv2_init(struct dt_device_node *node, const void *data)
>      return 0;
>  }
>  
> -static const char * const gicv2_dt_compat[] __initconst =
> +static const char * const hip04_gicv2_dt_compat[] __initconst =
>  {
> -    DT_COMPAT_GIC_CORTEX_A15,
> -    DT_COMPAT_GIC_CORTEX_A7,
> -    DT_COMPAT_GIC_400,
> +    DT_COMPAT_GIC_HIP04,
>      NULL
>  };
>  
> -DT_DEVICE_START(gicv2, "GICv2:", DEVICE_GIC)
> -        .compatible = gicv2_dt_compat,
> +DT_DEVICE_START(hip04_gicv2, "GICv2:", DEVICE_GIC)
> +        .compatible = hip04_gicv2_dt_compat,
>          .init = gicv2_init,
>  DT_DEVICE_END
>  
> +
>  /*
>   * Local variables:
>   * mode: C

Please merge these changes in the previous patch


> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 390c8b0..e4512a8 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -565,12 +565,13 @@ static void do_sgi(struct cpu_user_regs *regs, enum gic_sgi sgi)
>  void gic_interrupt(struct cpu_user_regs *regs, int is_fiq)
>  {
>      unsigned int irq;
> +    unsigned int max_irq = gic_hw_ops->info->nr_lines;
>  
>      do  {
>          /* Reading IRQ will ACK it */
>          irq = gic_hw_ops->read_irq();
>  
> -        if ( likely(irq >= 16 && irq < 1021) )
> +        if ( likely(irq >= 16 && irq < max_irq) )
>          {
>              local_irq_enable();
>              do_IRQ(regs, irq, is_fiq);
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 187dc46..f245c50 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -155,11 +155,14 @@
>  #define DT_COMPAT_GIC_400            "arm,gic-400"
>  #define DT_COMPAT_GIC_CORTEX_A15     "arm,cortex-a15-gic"
>  #define DT_COMPAT_GIC_CORTEX_A7      "arm,cortex-a7-gic"
> +#define DT_COMPAT_GIC_HIP04          "hisilicon,hip04-intc"
>  
>  #define DT_MATCH_GIC_V2 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A15), \
>                          DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A7), \
>                          DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_400)
>  
> +#define DT_MATCH_GIC_HIP04 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_HIP04)
> +
>  #define DT_COMPAT_GIC_V3             "arm,gic-v3"
>  
>  #define DT_MATCH_GIC_V3 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_V3)
> -- 
> 1.9.1
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 3/4] xen/arm: handle GICH register changes for hip04-d01 platform
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 3/4] xen/arm: handle GICH register changes for " Frediano Ziglio
@ 2015-02-25 16:53         ` Stefano Stabellini
  0 siblings, 0 replies; 32+ messages in thread
From: Stefano Stabellini @ 2015-02-25 16:53 UTC (permalink / raw)
  To: Frediano Ziglio
  Cc: zoltan.kiss, Ian Campbell, Julien Grall, Tim Deegan, xen-devel,
	Stefano Stabellini

On Wed, 25 Feb 2015, Frediano Ziglio wrote:
> The GICH in this platform is mainly compatible with the standard
> GICv2 beside APR and LR register offsets.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> ---
>  xen/arch/arm/gic-hip04.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
> index 9a7ed46..a1ae520 100644
> --- a/xen/arch/arm/gic-hip04.c
> +++ b/xen/arch/arm/gic-hip04.c
> @@ -88,6 +88,11 @@ static DEFINE_PER_CPU(u16, gic_cpu_id);
>  #undef GICD_SGI_TARGET_SHIFT
>  #define GICD_SGI_TARGET_SHIFT 8
>  
> +#undef GICH_APR
> +#undef GICH_LR
> +#define GICH_APR   0x70
> +#define GICH_LR    0x80
> +
>  static inline void writeb_gicd(uint8_t val, unsigned int offset)
>  {
>      writeb_relaxed(val, gicv2.map_dbase + offset);

You can fold these changes in the first patch. Please use a clear naming
scheme that doesn't collide with GICv2 names.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 4/4] xen/arm: Force dom0 to use normal GICv2 driver on Hip04 platform
  2015-02-25 15:28       ` [PATCH v5.99.1 RFC 4/4] xen/arm: Force dom0 to use normal GICv2 driver on Hip04 platform Frediano Ziglio
@ 2015-02-25 16:54         ` Stefano Stabellini
  0 siblings, 0 replies; 32+ messages in thread
From: Stefano Stabellini @ 2015-02-25 16:54 UTC (permalink / raw)
  To: Frediano Ziglio
  Cc: zoltan.kiss, Ian Campbell, Julien Grall, Tim Deegan, xen-devel,
	Stefano Stabellini

On Wed, 25 Feb 2015, Frediano Ziglio wrote:
> Until vGIC support is not implemented and tested, this will prevent
> guest kernels to use their Hip04 driver, or crash when they don't
> have any.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> ---
>  xen/arch/arm/gic-hip04.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
> index a1ae520..afd2622 100644
> --- a/xen/arch/arm/gic-hip04.c
> +++ b/xen/arch/arm/gic-hip04.c
> @@ -601,17 +601,13 @@ static int gicv2_make_dt_node(const struct domain *d,
>                                const struct dt_device_node *node, void *fdt)
>  {
>      const struct dt_device_node *gic = dt_interrupt_controller;
> -    const void *compatible = NULL;
> +    const void *compatible;
>      u32 len;
>      const __be32 *regs;
>      int res = 0;
>  
> -    compatible = dt_get_property(gic, "compatible", &len);
> -    if ( !compatible )
> -    {
> -        dprintk(XENLOG_ERR, "Can't find compatible property for the gic node\n");
> -        return -FDT_ERR_XEN(ENOENT);
> -    }
> +    compatible = DT_COMPAT_GIC_CORTEX_A15;
> +    len = strlen((char*) compatible) + 1;
>  
>      res = fdt_begin_node(fdt, "interrupt-controller");
>      if ( res )

It would be nice to have an in-code comment about this too.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  2015-02-25 16:34         ` Stefano Stabellini
@ 2015-02-25 16:59           ` Ian Campbell
  2015-02-26 12:44             ` Ian Campbell
  2015-02-26 17:39           ` Ian Campbell
  1 sibling, 1 reply; 32+ messages in thread
From: Ian Campbell @ 2015-02-25 16:59 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: zoltan.kiss, Julien Grall, Tim Deegan, xen-devel,
	Frediano Ziglio, Stefano Stabellini

On Wed, 2015-02-25 at 16:34 +0000, Stefano Stabellini wrote:
> I am concerned about the increased size of the Xen binary as a result of
> the introduction of this driver.

I'm also somewhat concerned about the ongoing maintenance of a
proliferation of (subtly or otherwise) different interrupt controllers.
(I remember too well the morass which Linux/ARM was in a few years ago
with all the h/w variations, which they have spent many years getting on
top of.)

Speaking only for myself I am (obviously) happy to be part of the
maintenance effort for the architecturally defined(/compliant, whatever)
ones and of the generic infrastructure.

But I think it is reasonable to expect the interested community members
to pick up the burden for anything else they would like to add, rather
than expecting the core maintainers to do it. (which BTW implies this
patch should include a hunk touching MAINTAINERS)

I also think it would be reasonable for us to drop support for hardware
which is not being adequately maintained. (Not that I expect that to
happen here, but it is inevitable that eventually we will see such
drivers I think). "adequately maintained" would mean things like timely
responses to core infrastructure updates and not bit-rotting.

> I think we should disable the build of all drivers in Xen by default,
> except for the ARM standard compliant ones (for aarch64 the SBSA is a
> nice summary of what is considered compliant), to keep the size of the
> binary small.

I don't think the SBSA is necessarily the best reference here, since it
only defines what is standardised within the scope of "server
systems" (whatever you take that to mean) and there are things which do
not fall under that umbrella.

That said I'm not sure what better reference there is.

Maybe even on non-server systems the set of hardware which Xen has to
drive itself is limited to things which are covered by the SBSA in
practice, by the nature of the fact that the majority of the wacky stuff
is driven from hardware domains.

So maybe SBSA is OK then...

> Could you please introduce a Xen build time option in
> xen/arch/arm/Rules.mk, called HAS_NON_STANDARD_DRIVERS, that by default
> is n, and gate the build of gic-hip04.c on it?

I'm rather wary of creating a "two tier" system like this, but I cannot
think of a better compromise :-(

Ian.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform
  2015-02-25 16:43 ` [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform Ian Campbell
@ 2015-02-25 17:10   ` Frediano Ziglio
  0 siblings, 0 replies; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-25 17:10 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Tim Deegan, Julien Grall, Stefano Stabellini, Zoltan Kiss, xen-devel

> On Fri, 2015-02-20 at 09:56 +0000, Frediano Ziglio wrote:
> > This set of patches add Xen support for hip04-d01 platform (see
> > https://wiki.linaro.org/Boards/D01 for details).
> 
> When (or where) would the general public be able to purchase one of
> these systems?
> 
> Ian.

These exact board is already in the LAVA laboratory at Linaro.

About purchasing there will be some product based on this board. I actually do not remember if some product is already available (I'll ask).

Frediano

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 2/4] xen/arm: Make gic-v2 code handle hip04-d01 platform
  2015-02-25 16:53         ` Stefano Stabellini
@ 2015-02-26  9:54           ` Frediano Ziglio
  0 siblings, 0 replies; 32+ messages in thread
From: Frediano Ziglio @ 2015-02-26  9:54 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Zoltan Kiss, Ian Campbell, Julien Grall, Tim Deegan, Xen-devel,
	Frediano Ziglio, Stefano Stabellini

2015-02-25 16:53 GMT+00:00 Stefano Stabellini
<stefano.stabellini@eu.citrix.com>:
> On Wed, 25 Feb 2015, Frediano Ziglio wrote:
>> The GIC in this platform is mainly compatible with the standard
>> GICv2 beside:
>> - ITARGET is extended to 16 bit to support 16 CPUs;
>> - SGI mask is extended to support 16 CPUs;
>> - maximum supported interrupt is 510.
>>
>> Use nr_lines to check for maximum irq supported. hip04-d01 support less
>> interrupts due to some field restriction. Any value above this is already
>> an error.
>>
>> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
>> Signed-off-by: Zoltan Kiss <zoltan.kiss@huawei.com>
>> ---
>>  xen/arch/arm/domain_build.c |  1 +
>>  xen/arch/arm/gic-hip04.c    | 43 +++++++++++++++++++++++++------------------
>>  xen/arch/arm/gic.c          |  3 ++-
>>  xen/include/asm-arm/gic.h   |  3 +++
>>  4 files changed, 31 insertions(+), 19 deletions(-)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index c2dcb49..0834053 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -1038,6 +1038,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>>      static const struct dt_device_match gic_matches[] __initconst =
>>      {
>>          DT_MATCH_GIC_V2,
>> +        DT_MATCH_GIC_HIP04,
>>          DT_MATCH_GIC_V3,
>>          { /* sentinel */ },
>>      };
>> diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
>> index a401e3f..9a7ed46 100644
>> --- a/xen/arch/arm/gic-hip04.c
>> +++ b/xen/arch/arm/gic-hip04.c
>> @@ -1,7 +1,8 @@
>>  /*
>> - * xen/arch/arm/gic-v2.c
>> + * xen/arch/arm/gic-hip04.c
>>   *
>> - * ARM Generic Interrupt Controller support v2
>> + * ARM Generic Interrupt Controller support for HiSilicon Hip04 platform
>> + * Based heavily from gic-v2.c
>>   *
>>   * Tim Deegan <tim@xen.org>
>>   * Copyright (c) 2011 Citrix Systems.
>> @@ -79,16 +80,24 @@ static struct gic_info gicv2_info;
>>   * logical CPU numbering. Let's use mapping as returned by the GIC
>>   * itself
>>   */
>> -static DEFINE_PER_CPU(u8, gic_cpu_id);
>> +static DEFINE_PER_CPU(u16, gic_cpu_id);
>>
>>  /* Maximum cpu interface per GIC */
>> -#define NR_GIC_CPU_IF 8
>> +#define NR_GIC_CPU_IF 16
>> +
>> +#undef GICD_SGI_TARGET_SHIFT
>> +#define GICD_SGI_TARGET_SHIFT 8
>>
>>  static inline void writeb_gicd(uint8_t val, unsigned int offset)
>>  {
>>      writeb_relaxed(val, gicv2.map_dbase + offset);
>>  }
>>
>> +static inline void writew_gicd(uint16_t val, unsigned int offset)
>> +{
>> +    writew_relaxed(val, gicv2.map_dbase + offset);
>> +}
>> +
>>  static inline void writel_gicd(uint32_t val, unsigned int offset)
>>  {
>>      writel_relaxed(val, gicv2.map_dbase + offset);
>> @@ -230,7 +239,7 @@ static void gicv2_set_irq_properties(struct irq_desc *desc,
>>      writel_gicd(cfg, GICD_ICFGR + (irq / 16) * 4);
>>
>>      /* Set target CPU mask (RAZ/WI on uniprocessor) */
>> -    writeb_gicd(mask, GICD_ITARGETSR + irq);
>> +    writew_gicd(mask, GICD_ITARGETSR + irq * 2);
>>      /* Set priority */
>>      writeb_gicd(priority, GICD_IPRIORITYR + irq);
>>
>> @@ -244,8 +253,7 @@ static void __init gicv2_dist_init(void)
>>      uint32_t gic_cpus;
>>      int i;
>>
>> -    cpumask = readl_gicd(GICD_ITARGETSR) & 0xff;
>> -    cpumask |= cpumask << 8;
>> +    cpumask = readl_gicd(GICD_ITARGETSR) & 0xffff;
>>      cpumask |= cpumask << 16;
>>
>>      /* Disable the distributor */
>> @@ -253,7 +261,7 @@ static void __init gicv2_dist_init(void)
>>
>>      type = readl_gicd(GICD_TYPER);
>>      gicv2_info.nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
>> -    gic_cpus = 1 + ((type & GICD_TYPE_CPUS) >> 5);
>> +    gic_cpus = 16;
>>      printk("GICv2: %d lines, %d cpu%s%s (IID %8.8x).\n",
>>             gicv2_info.nr_lines, gic_cpus, (gic_cpus == 1) ? "" : "s",
>>             (type & GICD_TYPE_SEC) ? ", secure" : "",
>> @@ -264,8 +272,8 @@ static void __init gicv2_dist_init(void)
>>          writel_gicd(0x0, GICD_ICFGR + (i / 16) * 4);
>>
>>      /* Route all global IRQs to this CPU */
>> -    for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
>> -        writel_gicd(cpumask, GICD_ITARGETSR + (i / 4) * 4);
>> +    for ( i = 32; i < gicv2_info.nr_lines; i += 2 )
>> +        writel_gicd(cpumask, GICD_ITARGETSR + (i / 2) * 4);
>>
>>      /* Default priority for global interrupts */
>>      for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
>> @@ -285,7 +293,7 @@ static void __cpuinit gicv2_cpu_init(void)
>>  {
>>      int i;
>>
>> -    this_cpu(gic_cpu_id) = readl_gicd(GICD_ITARGETSR) & 0xff;
>> +    this_cpu(gic_cpu_id) = readl_gicd(GICD_ITARGETSR) & 0xffff;
>>
>>      /* The first 32 interrupts (PPI and SGI) are banked per-cpu, so
>>       * even though they are controlled with GICD registers, they must
>> @@ -579,7 +587,7 @@ static void gicv2_irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_m
>>      mask = gicv2_cpu_mask(cpu_mask);
>>
>>      /* Set target CPU mask (RAZ/WI on uniprocessor) */
>> -    writeb_gicd(mask, GICD_ITARGETSR + desc->irq);
>> +    writew_gicd(mask, GICD_ITARGETSR + desc->irq * 2);
>>
>>      spin_unlock(&gicv2.lock);
>>  }
>> @@ -765,19 +773,18 @@ static int __init gicv2_init(struct dt_device_node *node, const void *data)
>>      return 0;
>>  }
>>
>> -static const char * const gicv2_dt_compat[] __initconst =
>> +static const char * const hip04_gicv2_dt_compat[] __initconst =
>>  {
>> -    DT_COMPAT_GIC_CORTEX_A15,
>> -    DT_COMPAT_GIC_CORTEX_A7,
>> -    DT_COMPAT_GIC_400,
>> +    DT_COMPAT_GIC_HIP04,
>>      NULL
>>  };
>>
>> -DT_DEVICE_START(gicv2, "GICv2:", DEVICE_GIC)
>> -        .compatible = gicv2_dt_compat,
>> +DT_DEVICE_START(hip04_gicv2, "GICv2:", DEVICE_GIC)
>> +        .compatible = hip04_gicv2_dt_compat,
>>          .init = gicv2_init,
>>  DT_DEVICE_END
>>
>> +
>>  /*
>>   * Local variables:
>>   * mode: C
>
> Please merge these changes in the previous patch
>

First patch was meant to contains only the copy of file (not Makefile
change). This to have a commit that helps to see future changes made
to this file and gic-v2.c. This as a workaround to git not having full
support for copy.

Frediano

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  2015-02-25 16:59           ` Ian Campbell
@ 2015-02-26 12:44             ` Ian Campbell
  2015-02-26 13:54               ` Julien Grall
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Campbell @ 2015-02-26 12:44 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: zoltan.kiss, Julien Grall, Tim Deegan, xen-devel,
	Frediano Ziglio, Stefano Stabellini

On Wed, 2015-02-25 at 16:59 +0000, Ian Campbell wrote:
> > I think we should disable the build of all drivers in Xen by default,
> > except for the ARM standard compliant ones (for aarch64 the SBSA is a
> > nice summary of what is considered compliant), to keep the size of the
> > binary small.
> 
> I don't think the SBSA is necessarily the best reference here, since it
> only defines what is standardised within the scope of "server
> systems" (whatever you take that to mean) and there are things which do
> not fall under that umbrella.
> 
> That said I'm not sure what better reference there is.
> 
> Maybe even on non-server systems the set of hardware which Xen has to
> drive itself is limited to things which are covered by the SBSA in
> practice, by the nature of the fact that the majority of the wacky stuff
> is driven from hardware domains.
> 
> So maybe SBSA is OK then...

Having thought about this overnight I'm now not so sure of this again, I
don't think SBSA is the definition we want.

Lets take a step back...

The set of hardware which Xen needs to be able to drive (rather than
give to a hardware domain) is:

      * CPUs
      * Host interrupt controllers
      * Host timers
      * A UART
      * Second-state MMUs
      * Second-stage SMMUs/IOMMU (First-stage used by domains)
      * PCI host bridges (in the near future)

(did I forget any?)

NB: I'm only considering host level stuff here. Our virtualised hardware
as exposed to the guest is well defined right now and any conversation
about deviating from the set of hardware (e.g. providing a guest view to
a non-GIC compliant virtual interrupt controller) would be part of a
separate larger conversation about "hvm" style guests (and I'd, as you
might imagine, be very reluctant to code to Xen itself to support
non-standard vGICs in particular).

>From a "what does 'standards compliant' mean" PoV we have:

CPUs:

        Specified in the ARM ARM (v7=ARM DDI 0406, v8=ARM DDI 0487).
        
        Uncontroversial, I hope!
        
Host interrupt controllers:

        Defined in "ARM Generic Interrupt Controller Architecture
        Specification" (v2=ARM IHI 0048B, v3=???).
        
        Referenced from ARMv8 ARM (but not required AFAICT) but
        nonetheless this is what we consider when we talk about the
        "standard interrupt controller".
        
Host timers:

        Defined in the ARMv8 ARM "Generic Timers" chapter.
        
        Defined as an extension to ARMv7 (don't have doc ref handy). For
        our purposes such extensions are considered standardized[*].

UARTS:

        SBSA defines some (pl011 only?), but there are lots, including
        8250-alike ones (ns16550 etc) which is a well established
        standard (from x86).
        
        Given that UART drivers are generally small and pretty trivial I
        think we can tolerate "non-standard" (i.e. non-SBSA, non-8250)
        ones, so long as they are able to support our vuart interface.
        
        I think the non-{pl011,8250} ones should be subject to non-core
        (i.e. community) maintenance as I mentioned previously, i.e
        should be listed in MAINTAINERS other than under the core ARM
        entry. If we decide to go ahead with this approach I'll ask the
        appropriate people to update MAINTAINERS.

Second stage MMU:

        Defined in the ARMv8 ARM, or the ARMv7 LPAE extensions[*, **].
        
Second stage SMMU/IOMMU:

        Defined in "ARM System Memory Management Unit Architecture
        Specification" (ARM IHI 0062D?)
        
PCI host bridges:

        We don't actually (properly) support PCI yet, but see my mails
        in the "Enhance platform support for PCI" thread this morning,
        for what we think the general shape might be taking.
        
        The meaning of the PCI CFG (CAM, ECAM etc), MMIO and IO (MMIO
        mapped) regions are, I think, all pretty well defined by the
        (various?) PCI spec(s).
        
        What's not quite so clear cut is the discovery of where the
        controllers actually live (based on the fact that
        Documentation/devicetree/bindings/pci/ has more than one entry
        in it...).
        
        SBSA doesn't really cover this either, it says "The base address
        of each ECAM region within the system memory map is
        IMPLEMENTATION DEFINED and is expected to be discoverable from
        system firmware data."
        
        However I think it will be the case that most "pci host bridge
        drivers" for Xen will end up being fairly trivial affairs, i.e.
        parse the DT, perhaps a little bit of basic setup and register
        the resulting regions with the PCI core and perhaps provide some
        simple accessors.
        
        So, I think we can say that for PCI controllers which export a
        set of PCI standard CFG, MMIO, IO space regions (all as MMIO
        mapped regions) and just require a specific driver for discovery
        of the host bridge and small amounts of start of day setup
        should be considered "standard" for our purposes.

So, based on the above I think we overall have a pretty good idea what
"standardized" means for the hardware components which we care about.

I think the above is a workable definition of what it is reasonable to
expect the core Xen/ARM maintainers to look after (with that particular
hat on) vs. what it should be expected for interested members of the
community to step up and maintain (where the people who happen to be
core Xen/ARM maintainers may occasionally chose to have such a hat too.)

I think it also provides a workable definition for the line for
Stefano's HAS_NON_STANDARD_DRIVERS idea as well, if we want to go down
that path.

Ian.

[*] For ARMv7 I think we can consider things like the Generic Timers and
LPAE extension specs to be morally equivalent to the bits of the ARMv8
ARM spec, i.e. if it was merged into the ARMv8 ARM then we treat it as
part of the ARMv7 ARM from a "is it standardized" PoV.

[**] The LPAE extensions include/are mixed with the hyp mode page table
format, so we pretty certainly need it.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  2015-02-26 12:44             ` Ian Campbell
@ 2015-02-26 13:54               ` Julien Grall
  2015-02-26 15:28                 ` Ian Campbell
  0 siblings, 1 reply; 32+ messages in thread
From: Julien Grall @ 2015-02-26 13:54 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini
  Cc: Frediano Ziglio, Tim Deegan, zoltan.kiss, Stefano Stabellini, xen-devel

Hi Ian,

On 26/02/15 12:44, Ian Campbell wrote:
> The set of hardware which Xen needs to be able to drive (rather than
> give to a hardware domain) is:
> 
>       * CPUs
>       * Host interrupt controllers
>       * Host timers
>       * A UART
>       * Second-state MMUs
>       * Second-stage SMMUs/IOMMU (First-stage used by domains)
>       * PCI host bridges (in the near future)
> 
> (did I forget any?)

I think everything is there.

> NB: I'm only considering host level stuff here. Our virtualised hardware
> as exposed to the guest is well defined right now and any conversation
> about deviating from the set of hardware (e.g. providing a guest view to
> a non-GIC compliant virtual interrupt controller) would be part of a
> separate larger conversation about "hvm" style guests (and I'd, as you
> might imagine, be very reluctant to code to Xen itself to support
> non-standard vGICs in particular).

That would mean on platform such as Hisilicon, Guest (including DOM0)
won't be able to use more than 8 CPUs. But I guess this is a fair trade
for having a GIC which differs from the spec.

> From a "what does 'standards compliant' mean" PoV we have:
> 
> CPUs:
> 
>         Specified in the ARM ARM (v7=ARM DDI 0406, v8=ARM DDI 0487).
>         
>         Uncontroversial, I hope!
>         
> Host interrupt controllers:
> 
>         Defined in "ARM Generic Interrupt Controller Architecture
>         Specification" (v2=ARM IHI 0048B, v3=???).

AFAICT, for GICv3 there is a hardware spec available (though not
publicly) but no developer spec.

>         Referenced from ARMv8 ARM (but not required AFAICT) but
>         nonetheless this is what we consider when we talk about the
>         "standard interrupt controller".
>         
> Host timers:
> 
>         Defined in the ARMv8 ARM "Generic Timers" chapter.
>         
>         Defined as an extension to ARMv7 (don't have doc ref handy). For
>         our purposes such extensions are considered standardized[*].

It's worth to mention that we don't support generic memory mapped timer
for now. I don't know if we aim to support it.

> UARTS:
> 
>         SBSA defines some (pl011 only?), but there are lots, including
>         8250-alike ones (ns16550 etc) which is a well established
>         standard (from x86).
>         
>         Given that UART drivers are generally small and pretty trivial I
>         think we can tolerate "non-standard" (i.e. non-SBSA, non-8250)
>         ones, so long as they are able to support our vuart interface.
>         
>         I think the non-{pl011,8250} ones should be subject to non-core
>         (i.e. community) maintenance as I mentioned previously, i.e
>         should be listed in MAINTAINERS other than under the core ARM
>         entry. If we decide to go ahead with this approach I'll ask the
>         appropriate people to update MAINTAINERS.

At that time we have 3 "non-compliant" UART in Xen: exynos4210, scif and
omap.

Having maintainers per non-compliant UART would make some generic more
complicate to upstream. Indeed, it would require all the ack.

> 
> Second stage MMU:
> 
>         Defined in the ARMv8 ARM, or the ARMv7 LPAE extensions[*, **].
>         
> Second stage SMMU/IOMMU:
> 
>         Defined in "ARM System Memory Management Unit Architecture
>         Specification" (ARM IHI 0062D?)

The D is the revision, so this would be ARM IHI 0062 for all the SMMUs.

[..]

> I think the above is a workable definition of what it is reasonable to
> expect the core Xen/ARM maintainers to look after (with that particular
> hat on) vs. what it should be expected for interested members of the
> community to step up and maintain (where the people who happen to be
> core Xen/ARM maintainers may occasionally chose to have such a hat too.)

I got few questions about it:
	-  What happen if the maintainers of a specific driver (UART/IRQ
controller) doesn't answer?
	- How do we handle possible security issue related to a specific
driver? Is it even considered as a security one?
	- As a new drivers would tight to a new set of maintainers, how do we
decide that a new drivers is accepted in Xen?

Given the governance spec [1], we may decide to reject a maintainers for
some reason. Does it mean the drivers is rejected too?

Overall, I think we should clearly define the condition of
acceptance/maintenance of a specific driver.

[..]

> [**] The LPAE extensions include/are mixed with the hyp mode page table
> format, so we pretty certainly need it.

Rigth, the ARM spec required LPAE extensions when virtualization is
supported.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  2015-02-26 13:54               ` Julien Grall
@ 2015-02-26 15:28                 ` Ian Campbell
  0 siblings, 0 replies; 32+ messages in thread
From: Ian Campbell @ 2015-02-26 15:28 UTC (permalink / raw)
  To: Julien Grall
  Cc: zoltan.kiss, Stefano Stabellini, Tim Deegan, xen-devel,
	Frediano Ziglio, Stefano Stabellini

On Thu, 2015-02-26 at 13:54 +0000, Julien Grall wrote:
> > NB: I'm only considering host level stuff here. Our virtualised hardware
> > as exposed to the guest is well defined right now and any conversation
> > about deviating from the set of hardware (e.g. providing a guest view to
> > a non-GIC compliant virtual interrupt controller) would be part of a
> > separate larger conversation about "hvm" style guests (and I'd, as you
> > might imagine, be very reluctant to code to Xen itself to support
> > non-standard vGICs in particular).
> 
> That would mean on platform such as Hisilicon, Guest (including DOM0)
> won't be able to use more than 8 CPUs. But I guess this is a fair trade
> for having a GIC which differs from the spec.

Correct.

> 
> > From a "what does 'standards compliant' mean" PoV we have:
> > 
> > CPUs:
> > 
> >         Specified in the ARM ARM (v7=ARM DDI 0406, v8=ARM DDI 0487).
> >         
> >         Uncontroversial, I hope!
> >         
> > Host interrupt controllers:
> > 
> >         Defined in "ARM Generic Interrupt Controller Architecture
> >         Specification" (v2=ARM IHI 0048B, v3=???).
> 
> AFAICT, for GICv3 there is a hardware spec available (though not
> publicly) but no developer spec.


The "Architecture Specification" is the one we want, I don't know if
that is what you meant by hardware spec, I have one although as you say
I don't think it is public yet.

> 
> >         Referenced from ARMv8 ARM (but not required AFAICT) but
> >         nonetheless this is what we consider when we talk about the
> >         "standard interrupt controller".
> >         
> > Host timers:
> > 
> >         Defined in the ARMv8 ARM "Generic Timers" chapter.
> >         
> >         Defined as an extension to ARMv7 (don't have doc ref handy). For
> >         our purposes such extensions are considered standardized[*].
> 
> It's worth to mention that we don't support generic memory mapped timer
> for now. I don't know if we aim to support it.

I don't know either, yet. For now we don't, that's correct.

> > UARTS:
> > 
> >         SBSA defines some (pl011 only?), but there are lots, including
> >         8250-alike ones (ns16550 etc) which is a well established
> >         standard (from x86).
> >         
> >         Given that UART drivers are generally small and pretty trivial I
> >         think we can tolerate "non-standard" (i.e. non-SBSA, non-8250)
> >         ones, so long as they are able to support our vuart interface.
> >         
> >         I think the non-{pl011,8250} ones should be subject to non-core
> >         (i.e. community) maintenance as I mentioned previously, i.e
> >         should be listed in MAINTAINERS other than under the core ARM
> >         entry. If we decide to go ahead with this approach I'll ask the
> >         appropriate people to update MAINTAINERS.
> 
> At that time we have 3 "non-compliant" UART in Xen: exynos4210, scif and
> omap.
> 
> Having maintainers per non-compliant UART would make some generic more
> complicate to upstream.

In reality by a negligible amount, I expect.

>  Indeed, it would require all the ack.

I don't think that's true, an update to core which requires updates to
all drivers shouldn't be blocked by non-responsive maintainer. If they
don't respond then their driver might break.

This all works fine for much larger projects. Take Linux for example:
you don't see them getting stalled on core infrastructure updates
because the author of some niche serial driver isn't responding to his
mail. They do the sensible thing and get on with it.

> [..]
> 
> > I think the above is a workable definition of what it is reasonable to
> > expect the core Xen/ARM maintainers to look after (with that particular
> > hat on) vs. what it should be expected for interested members of the
> > community to step up and maintain (where the people who happen to be
> > core Xen/ARM maintainers may occasionally chose to have such a hat too.)
> 
> I got few questions about it:
> 	-  What happen if the maintainers of a specific driver (UART/IRQ
> controller) doesn't answer?

Then their driver might break or bitrot, and eventually be removed.

> 	- How do we handle possible security issue related to a specific
> driver? Is it even considered as a security one?

In the same way we do today with any security issue, which is to say the
security team will deal with it, bringing in people as they feel
appropriate (and the discoverer agrees). This is no different to a bug
in any other bit of Xen who's maintainer is not on the security team.

> 	- As a new drivers would tight to a new set of maintainers, how do we
> decide that a new drivers is accepted in Xen?

In the normal way.

> Given the governance spec [1], we may decide to reject a maintainers for
> some reason. Does it mean the drivers is rejected too?

If someone writes a driver for a h/w component and wants to be the
maintainer then there is no normal reason to reject them IMHO.

To put it another way, if we don't want to accept them as maintainer for
the driver which they have written then why would we want to accept the
driver itself.

> Overall, I think we should clearly define the condition of
> acceptance/maintenance of a specific driver.

This will follow the normal development process and patch acceptance
criteria, I don't think we need to make any of this more complicated
than that.

TBH, I think you are worrying about the process stuff unnecessarily,
this will all just work like it already does.

> 
> [..]
> 
> > [**] The LPAE extensions include/are mixed with the hyp mode page table
> > format, so we pretty certainly need it.
> 
> Rigth, the ARM spec required LPAE extensions when virtualization is
> supported.
> 
> Regards,
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  2015-02-25 16:34         ` Stefano Stabellini
  2015-02-25 16:59           ` Ian Campbell
@ 2015-02-26 17:39           ` Ian Campbell
  2015-02-26 17:59             ` Stefano Stabellini
  1 sibling, 1 reply; 32+ messages in thread
From: Ian Campbell @ 2015-02-26 17:39 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: zoltan.kiss, Julien Grall, Tim Deegan, xen-devel,
	Frediano Ziglio, Stefano Stabellini

On Wed, 2015-02-25 at 16:34 +0000, Stefano Stabellini wrote:
> I think we should disable the build of all drivers in Xen by default,
> except for the ARM standard compliant ones (for aarch64 the SBSA is a
> nice summary of what is considered compliant), to keep the size of the
> binary small.

I think this last statement was based on information that the gic-v2
driver was of the order of 70-100K in size, but I think that information
was wrong (I suspect it was the raw .o size, which includes debug info
and other extraneous bits). Here I see:

$ du -h xen/arch/arm/gic-v2.o
148K	xen/arch/arm/gic-v2.o
$ aarch64-linux-gnu-size xen/arch/arm/gic-v2.o 
   text	   data	    bss	    dec	    hex	filename
   6619	      0	     97	   6716	   1a3c	xen/arch/arm/gic-v2.o

IOW the actual binary size is on the order of 6K (gic-v3.o is around the
same). This is arm64, I can't be bothered to rebuild for arm32, it'll be
similar.

Given that then I really don't think it is worth introducing a two tier
build over it.

If we really cared about these sorts of savings we would arrange to
discard all of the unused GIC/SMMU/UART driver's .text/.data/.bss after
boot (easy enough to achieve by putting each in a dedicated segment).

But I don't think we have enough such drivers to start worrying about
doing that just now. We have that opportunity in our back pocket if we
ever get to that point, which is good enough I think.

> Could you please introduce a Xen build time option in
> xen/arch/arm/Rules.mk, called HAS_NON_STANDARD_DRIVERS, that by default
> is n, and gate the build of gic-hip04.c on it?

Frediano, I see you've already done so in v6, thanks for that. Sorry to
go back on it.

Assuming the rest of the series in v6 is OK (gets acked and whatever)
then I expect I can just skip that one patch when applying and fixup the
Makefile in the obvious way (approx s/HAS_NON.../CONFIG_ARM32/) in the
dependent patch.

Ian.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
  2015-02-26 17:39           ` Ian Campbell
@ 2015-02-26 17:59             ` Stefano Stabellini
  0 siblings, 0 replies; 32+ messages in thread
From: Stefano Stabellini @ 2015-02-26 17:59 UTC (permalink / raw)
  To: Ian Campbell
  Cc: zoltan.kiss, Stefano Stabellini, Julien Grall, Tim Deegan,
	xen-devel, Frediano Ziglio, Stefano Stabellini

On Thu, 26 Feb 2015, Ian Campbell wrote:
> On Wed, 2015-02-25 at 16:34 +0000, Stefano Stabellini wrote:
> > I think we should disable the build of all drivers in Xen by default,
> > except for the ARM standard compliant ones (for aarch64 the SBSA is a
> > nice summary of what is considered compliant), to keep the size of the
> > binary small.
> 
> I think this last statement was based on information that the gic-v2
> driver was of the order of 70-100K in size, but I think that information
> was wrong (I suspect it was the raw .o size, which includes debug info
> and other extraneous bits). Here I see:
> 
> $ du -h xen/arch/arm/gic-v2.o
> 148K	xen/arch/arm/gic-v2.o
> $ aarch64-linux-gnu-size xen/arch/arm/gic-v2.o 
>    text	   data	    bss	    dec	    hex	filename
>    6619	      0	     97	   6716	   1a3c	xen/arch/arm/gic-v2.o
> 
> IOW the actual binary size is on the order of 6K (gic-v3.o is around the
> same). This is arm64, I can't be bothered to rebuild for arm32, it'll be
> similar.
> 
> Given that then I really don't think it is worth introducing a two tier
> build over it.
> 
> If we really cared about these sorts of savings we would arrange to
> discard all of the unused GIC/SMMU/UART driver's .text/.data/.bss after
> boot (easy enough to achieve by putting each in a dedicated segment).
> 
> But I don't think we have enough such drivers to start worrying about
> doing that just now. We have that opportunity in our back pocket if we
> ever get to that point, which is good enough I think.
> 
> > Could you please introduce a Xen build time option in
> > xen/arch/arm/Rules.mk, called HAS_NON_STANDARD_DRIVERS, that by default
> > is n, and gate the build of gic-hip04.c on it?
> 
> Frediano, I see you've already done so in v6, thanks for that. Sorry to
> go back on it.
> 
> Assuming the rest of the series in v6 is OK (gets acked and whatever)
> then I expect I can just skip that one patch when applying and fixup the
> Makefile in the obvious way (approx s/HAS_NON.../CONFIG_ARM32/) in the
> dependent patch.

v6 is fine from my POV, you can add my Acked-by to all patches.
I am OK with dropping HAS_NON_STANDARD_DRIVERS.

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2015-02-26 17:59 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-20  9:56 [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform Frediano Ziglio
2015-02-20  9:56 ` [PATCH v5 1/4] xen/arm: Make gic-v2 code handle " Frediano Ziglio
2015-02-24 16:16   ` Ian Campbell
2015-02-25 15:28     ` [PATCH v5.99.1 RFC 0/4] xen/arm: Add support for Huawei " Frediano Ziglio
2015-02-25 15:28       ` [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version Frediano Ziglio
2015-02-25 15:34         ` Julien Grall
2015-02-25 15:42           ` Frediano Ziglio
2015-02-25 15:46           ` Ian Campbell
2015-02-25 15:47           ` Frediano Ziglio
2015-02-25 16:34         ` Stefano Stabellini
2015-02-25 16:59           ` Ian Campbell
2015-02-26 12:44             ` Ian Campbell
2015-02-26 13:54               ` Julien Grall
2015-02-26 15:28                 ` Ian Campbell
2015-02-26 17:39           ` Ian Campbell
2015-02-26 17:59             ` Stefano Stabellini
2015-02-25 15:28       ` [PATCH v5.99.1 RFC 2/4] xen/arm: Make gic-v2 code handle hip04-d01 platform Frediano Ziglio
2015-02-25 16:53         ` Stefano Stabellini
2015-02-26  9:54           ` Frediano Ziglio
2015-02-25 15:28       ` [PATCH v5.99.1 RFC 3/4] xen/arm: handle GICH register changes for " Frediano Ziglio
2015-02-25 16:53         ` Stefano Stabellini
2015-02-25 15:28       ` [PATCH v5.99.1 RFC 4/4] xen/arm: Force dom0 to use normal GICv2 driver on Hip04 platform Frediano Ziglio
2015-02-25 16:54         ` Stefano Stabellini
2015-02-20  9:56 ` [PATCH v5 2/4] xen/arm: Add support for DTBs with strange names of Hip04 GICv2 Frediano Ziglio
2015-02-24 14:19   ` Julien Grall
2015-02-24 16:13     ` Ian Campbell
2015-02-24 16:38       ` Frediano Ziglio
2015-02-24 14:19   ` Julien Grall
2015-02-20  9:56 ` [PATCH v5 3/4] xen/arm: handle GICH register changes for hip04-d01 platform Frediano Ziglio
2015-02-20  9:56 ` [PATCH v5 4/4] xen/arm: Force dom0 to use normal GICv2 driver on Hip04 platform Frediano Ziglio
2015-02-25 16:43 ` [PATCH v5] xen/arm: Add support for Huawei hip04-d01 platform Ian Campbell
2015-02-25 17:10   ` Frediano Ziglio

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.