All of lore.kernel.org
 help / color / mirror / Atom feed
* [v3 00/13] xen/arm: Add support for GICv2 on GICv3
@ 2015-07-01 11:00 Julien Grall
  2015-07-01 11:01 ` [v3 01/13] xen/arm: Gate GICv3 change with HAS_GICV3 rather than CONFIG_ARM_64 Julien Grall
                   ` (14 more replies)
  0 siblings, 15 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Zoltan Kiss, ian.campbell, Shameerali Kolothum Thodi,
	Julien Grall, stefano.stabellini, Chen Baozi

Hi all,

This patch series adds support for GICv2 on GICv3. This feature is available
only when the GICv3 hardware is compatible with GICv2.

When it's the case, the same interface is provided in order to use a
virtualize GICv2 (i.e GICC and GICV). This will allow us to re-use the
same vGIC driver.

Currently GIC and vGIC drivers are tight because of the domain initialization
splitted between GIC and vGIC. This patch series intends to remove this
dependency in order to make the vGIC driver agnostic of the GIC driver.

It has been tested on the ARMv8 Foundation Model with GICv2 and GICv3 as
well as changing the vGIC version emulated for the guest (only on GICv3 host).

A branch with all the patches can be found here:
    git://xenbits.xen.org/people/julieng/xen-unstable.git branch gicv2-on-gicv3-v3

All the patches as been acked except #11 and #12.

For all the changes see in each patch.

Sincerely yours,

Tested-by: Chen Baozi <baozich@gmail.com>
Cc: Zoltan Kiss <zoltan.kiss@huawei.com>
Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>

Julien Grall (13):
  xen/arm: Gate GICv3 change with HAS_GICV3 rather than CONFIG_ARM_64...
  xen/arm: gic: Rename make_dt_node into make_hwdom_dt_node
  xen/arm: vGIC: Check return of the domain_init callback
  xen/arm: gic-v3: Fix the distributor region to 64kB
  xen/arm: gic-v3: Use the domain redistributor information to make the
    DT node
  xen/arm: gic-v3: Rework the messages printed at initialization
  xen/arm: gic-{v2,hip04}: Remove redundant check in
    {gicv2,hip04gic}_init
  xen/arm: gic-{v2,hip04}: Use SZ_64K rather than our custom value
  xen/arm: gic: Allow the base address to be 0
  xen/arm: gic-{v2,hip04}: Remove hbase from the global state
  xen/arm: Merge gicv_setup with vgic_domain_init
  arm: Allow the user to specify the GIC version
  xen/arm: gic-v3: Add support of vGICv2 when available

 config/arm64.mk                   |   1 +
 docs/man/xl.cfg.pod.5             |  29 ++++++
 tools/libxl/libxl.h               |   5 +
 tools/libxl/libxl_arm.c           |  16 +++-
 tools/libxl/libxl_types.idl       |  11 +++
 tools/libxl/xl_cmdimpl.c          |  12 +++
 xen/arch/arm/Makefile             |   2 +-
 xen/arch/arm/Rules.mk             |   2 +
 xen/arch/arm/domain.c             |  48 +++++-----
 xen/arch/arm/domain_build.c       |   2 +-
 xen/arch/arm/gic-hip04.c          |  90 +++++-------------
 xen/arch/arm/gic-v2.c             |  90 +++++-------------
 xen/arch/arm/gic-v3.c             | 191 +++++++++++++++++---------------------
 xen/arch/arm/gic.c                |  12 +--
 xen/arch/arm/vgic-v2.c            |  69 +++++++++++++-
 xen/arch/arm/vgic-v3.c            |  86 ++++++++++++++++-
 xen/arch/arm/vgic.c               |  11 ++-
 xen/include/asm-arm/domain.h      |   5 +-
 xen/include/asm-arm/gic.h         |  15 +--
 xen/include/asm-arm/gic_v3_defs.h |   7 ++
 xen/include/asm-arm/vgic.h        |  10 ++
 21 files changed, 426 insertions(+), 288 deletions(-)

-- 
2.1.4

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

* [v3 01/13] xen/arm: Gate GICv3 change with HAS_GICV3 rather than CONFIG_ARM_64...
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 11:01 ` [v3] xen: new maintainer for the RTDS scheduler Julien Grall
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, stefano.stabellini, ian.campbell

for clarity and it will be easier to understand some follow-up patches.

Also gate gic_v3 structure with HAS_GICV3.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v3:
        - Fix typo in commit message
        - Add Ian's ack

    Changes in v2:
        - Patch added
---
 config/arm64.mk              | 1 +
 xen/arch/arm/Makefile        | 2 +-
 xen/arch/arm/Rules.mk        | 2 ++
 xen/arch/arm/vgic.c          | 2 +-
 xen/include/asm-arm/domain.h | 2 +-
 xen/include/asm-arm/gic.h    | 4 ++++
 6 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/config/arm64.mk b/config/arm64.mk
index e24c1d1..c5deb4e 100644
--- a/config/arm64.mk
+++ b/config/arm64.mk
@@ -10,6 +10,7 @@ HAS_PL011 := y
 HAS_CADENCE_UART := y
 HAS_NS16550 := y
 HAS_MEM_ACCESS := y
+HAS_GICV3 := y
 
 # Use only if calling $(LD) directly.
 LDFLAGS_DIRECT += -EL
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 935999e..1ef39f7 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -13,7 +13,7 @@ obj-y += sysctl.o
 obj-y += domain_build.o
 obj-y += gic.o gic-v2.o
 obj-$(CONFIG_ARM_32) += gic-hip04.o
-obj-$(CONFIG_ARM_64) += gic-v3.o
+obj-$(HAS_GICV3) += gic-v3.o
 obj-y += io.o
 obj-y += irq.o
 obj-y += kernel.o
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index e27f573..b31770c 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -38,6 +38,8 @@ ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n)
 CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
 endif
 
+CFLAGS-$(HAS_GICV3) += -DHAS_GICV3
+
 EARLY_PRINTK := n
 
 ifeq ($(debug),y)
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 73a6f7e..dfd959a 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -82,7 +82,7 @@ int domain_vgic_init(struct domain *d, unsigned int nr_spis)
 
     switch ( gic_hw_version() )
     {
-#ifdef CONFIG_ARM_64
+#ifdef HAS_GICV3
     case GIC_V3:
         if ( vgic_v3_init(d) )
            return -ENODEV;
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index f1a087e..96607d5 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -101,7 +101,7 @@ struct arch_domain
         /* Base address for guest GIC */
         paddr_t dbase; /* Distributor base address */
         paddr_t cbase; /* CPU base address */
-#ifdef CONFIG_ARM_64
+#ifdef HAS_GICV3
         /* GIC V3 addressing */
         paddr_t dbase_size; /* Distributor base size */
         /* List of contiguous occupied by the redistributors */
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 9e2acb7..f0dcfa1 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -162,6 +162,7 @@
 
 #define DT_MATCH_GIC_V3 DT_MATCH_COMPATIBLE("arm,gic-v3")
 
+#ifdef HAS_GICV3
 /*
  * GICv3 registers that needs to be saved/restored
  */
@@ -171,6 +172,7 @@ struct gic_v3 {
     uint32_t apr1[4];
     uint64_t lr[16];
 };
+#endif
 
 /*
  * GICv2 register that needs to be saved/restored
@@ -188,7 +190,9 @@ struct gic_v2 {
  */
 union gic_state_data {
     struct gic_v2 v2;
+#ifdef HAS_GICV3
     struct gic_v3 v3;
+#endif
 };
 
 /*
-- 
2.1.4

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

* [v3] xen: new maintainer for the RTDS scheduler
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
  2015-07-01 11:01 ` [v3 01/13] xen/arm: Gate GICv3 change with HAS_GICV3 rather than CONFIG_ARM_64 Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 11:06   ` Julien Grall
  2015-07-01 11:01 ` [v3 02/13] xen/arm: gic: Rename make_dt_node into make_hwdom_dt_node Julien Grall
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel
  Cc: George Dunlap, Dario Faggioli, stefano.stabellini, ian.campbell,
	Meng Xu

From: Dario Faggioli <dario.faggioli@citrix.com>

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Reviewed-and-Acked-by: Meng Xu <mengxu@cis.upenn.edu>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
---
 MAINTAINERS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6b1068e..e6616d2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -282,6 +282,11 @@ F:	tools/libxl/libxl_nonetbuffer.c
 F:	tools/hotplug/Linux/remus-netbuf-setup
 F:	tools/hotplug/Linux/block-drbd-probe
 
+RTDS SCHEDULER
+M:	Dario Faggioli <dario.faggioli@citrix.com>
+S:	Supported
+F:	xen/common/sched_rt.c
+
 SCHEDULING
 M:	George Dunlap <george.dunlap@eu.citrix.com>
 S:	Supported
-- 
2.1.4

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

* [v3 02/13] xen/arm: gic: Rename make_dt_node into make_hwdom_dt_node
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
  2015-07-01 11:01 ` [v3 01/13] xen/arm: Gate GICv3 change with HAS_GICV3 rather than CONFIG_ARM_64 Julien Grall
  2015-07-01 11:01 ` [v3] xen: new maintainer for the RTDS scheduler Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 11:01 ` [v3 03/13] xen/arm: vGIC: Check return of the domain_init callback Julien Grall
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, stefano.stabellini, ian.campbell

Making it clear that the callback is only used to make the device tree node
for the hardware domain.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v3:
        - Fix typoes in commit message
        - Add Ian's ack

    Changes in v2:
        - Patch added
---
 xen/arch/arm/domain_build.c | 2 +-
 xen/arch/arm/gic-hip04.c    | 7 ++++---
 xen/arch/arm/gic-v2.c       | 7 ++++---
 xen/arch/arm/gic-v3.c       | 7 ++++---
 xen/arch/arm/gic.c          | 7 ++++---
 xen/include/asm-arm/gic.h   | 9 +++++----
 6 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index e9cb8a9..366a05a 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -863,7 +863,7 @@ static int make_gic_node(const struct domain *d, void *fdt,
     if ( res )
         return res;
 
-    res = gic_make_node(d, node, fdt);
+    res = gic_make_hwdom_dt_node(d, node, fdt);
     if ( res )
         return res;
 
diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
index ee693e7..669d043 100644
--- a/xen/arch/arm/gic-hip04.c
+++ b/xen/arch/arm/gic-hip04.c
@@ -605,8 +605,9 @@ static void hip04gic_irq_set_affinity(struct irq_desc *desc, const cpumask_t *cp
     spin_unlock(&gicv2.lock);
 }
 
-static int hip04gic_make_dt_node(const struct domain *d,
-                              const struct dt_device_node *node, void *fdt)
+static int hip04gic_make_hwdom_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;
@@ -768,7 +769,7 @@ const static struct gic_hw_operations hip04gic_ops = {
     .write_lr            = hip04gic_write_lr,
     .read_vmcr_priority  = hip04gic_read_vmcr_priority,
     .read_apr            = hip04gic_read_apr,
-    .make_dt_node        = hip04gic_make_dt_node,
+    .make_hwdom_dt_node  = hip04gic_make_hwdom_dt_node,
 };
 
 /* Set up the GIC */
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index edf659b..9e55b21 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -595,8 +595,9 @@ static void gicv2_irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_m
     spin_unlock(&gicv2.lock);
 }
 
-static int gicv2_make_dt_node(const struct domain *d,
-                              const struct dt_device_node *node, void *fdt)
+static int gicv2_make_hwdom_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;
@@ -754,7 +755,7 @@ const static struct gic_hw_operations gicv2_ops = {
     .write_lr            = gicv2_write_lr,
     .read_vmcr_priority  = gicv2_read_vmcr_priority,
     .read_apr            = gicv2_read_apr,
-    .make_dt_node        = gicv2_make_dt_node,
+    .make_hwdom_dt_node  = gicv2_make_hwdom_dt_node,
 };
 
 /* Set up the GIC */
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 30682cf..9a94f6b 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1100,8 +1100,9 @@ static void gicv3_irq_set_affinity(struct irq_desc *desc, const cpumask_t *mask)
     spin_unlock(&gicv3.lock);
 }
 
-static int gicv3_make_dt_node(const struct domain *d,
-                              const struct dt_device_node *node, void *fdt)
+static int gicv3_make_hwdom_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;
@@ -1323,7 +1324,7 @@ static const struct gic_hw_operations gicv3_ops = {
     .read_vmcr_priority  = gicv3_read_vmcr_priority,
     .read_apr            = gicv3_read_apr,
     .secondary_init      = gicv3_secondary_cpu_init,
-    .make_dt_node        = gicv3_make_dt_node,
+    .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
 };
 
 static int __init gicv3_preinit(struct dt_device_node *node, const void *data)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index c41e82e..341b6df 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -706,10 +706,11 @@ void __cpuinit init_maintenance_interrupt(void)
                 "irq-maintenance", NULL);
 }
 
-int gic_make_node(const struct domain *d,const struct dt_device_node *node,
-                   void *fdt)
+int gic_make_hwdom_dt_node(const struct domain *d,
+                           const struct dt_device_node *node,
+                           void *fdt)
 {
-    return gic_hw_ops->make_dt_node(d, node, fdt);
+    return gic_hw_ops->make_hwdom_dt_node(d, node, fdt);
 }
 
 /*
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index f0dcfa1..71f4813 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -352,13 +352,14 @@ struct gic_hw_operations {
     unsigned int (*read_apr)(int apr_reg);
     /* Secondary CPU init */
     int (*secondary_init)(void);
-    int (*make_dt_node)(const struct domain *d,
-                        const struct dt_device_node *node, void *fdt);
+    int (*make_hwdom_dt_node)(const struct domain *d,
+                              const struct dt_device_node *node, void *fdt);
 };
 
 void register_gic_ops(const struct gic_hw_operations *ops);
-int gic_make_node(const struct domain *d,const struct dt_device_node *node,
-                  void *fdt);
+int gic_make_hwdom_dt_node(const struct domain *d,
+                           const struct dt_device_node *node,
+                           void *fdt);
 
 #endif /* __ASSEMBLY__ */
 #endif
-- 
2.1.4

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

* [v3 03/13] xen/arm: vGIC: Check return of the domain_init callback
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
                   ` (2 preceding siblings ...)
  2015-07-01 11:01 ` [v3 02/13] xen/arm: gic: Rename make_dt_node into make_hwdom_dt_node Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 11:01 ` [v3 04/13] xen/arm: gic-v3: Fix the distributor region to 64kB Julien Grall
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, stefano.stabellini, ian.campbell

The domain_init callback can return error. Check it and progate the
error if necessary.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v2:
        - Add Ian's ack
---
 xen/arch/arm/vgic.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index dfd959a..fe6c7a2 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -71,6 +71,7 @@ static void vgic_init_pending_irq(struct pending_irq *p, unsigned int virq)
 int domain_vgic_init(struct domain *d, unsigned int nr_spis)
 {
     int i;
+    int ret;
 
     d->arch.vgic.ctlr = 0;
 
@@ -114,7 +115,9 @@ int domain_vgic_init(struct domain *d, unsigned int nr_spis)
     for (i=0; i<DOMAIN_NR_RANKS(d); i++)
         spin_lock_init(&d->arch.vgic.shared_irqs[i].lock);
 
-    d->arch.vgic.handler->domain_init(d);
+    ret = d->arch.vgic.handler->domain_init(d);
+    if ( ret )
+        return ret;
 
     d->arch.vgic.allocated_irqs =
         xzalloc_array(unsigned long, BITS_TO_LONGS(vgic_num_irqs(d)));
-- 
2.1.4

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

* [v3 04/13] xen/arm: gic-v3: Fix the distributor region to 64kB
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
                   ` (3 preceding siblings ...)
  2015-07-01 11:01 ` [v3 03/13] xen/arm: vGIC: Check return of the domain_init callback Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 11:01 ` [v3 05/13] xen/arm: gic-v3: Use the domain redistributor information to make the DT node Julien Grall
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, stefano.stabellini, ian.campbell

On GICv3, the default size of the distributor region is 64kB (see 5.3
in PRD03-GENC-010745 24.0). This region can be extended to provide an
implementation defined set of pages containing additional aliases for MSI.
Although, the GICv3 driver only access to register within the default
distributor region.

Furthermore, our vGIC driver implementation doesn't support the extended
distributor. Therefore there is no reason to expose it to DOM0.

Finally drop the field dbase_size which is not useful anymore.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v2:
        - Typoes
        - Add a reference to the GICv3 spec
        - Add Ian's ack
---
 xen/arch/arm/gic-v3.c        | 14 +++++---------
 xen/arch/arm/vgic-v3.c       |  2 +-
 xen/include/asm-arm/domain.h |  1 -
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 9a94f6b..d1af147 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -51,7 +51,6 @@ struct rdist_region {
 /* Global state */
 static struct {
     paddr_t dbase;            /* Address of distributor registers */
-    paddr_t dbase_size;
     void __iomem *map_dbase;  /* Mapped address of distributor registers */
     struct rdist_region *rdist_regions;
     uint32_t  rdist_stride;
@@ -938,7 +937,6 @@ static int gicv_v3_init(struct domain *d)
         unsigned int first_cpu = 0;
 
         d->arch.vgic.dbase = gicv3.dbase;
-        d->arch.vgic.dbase_size = gicv3.dbase_size;
 
         d->arch.vgic.rdist_stride = gicv3.rdist_stride;
         /*
@@ -966,7 +964,6 @@ static int gicv_v3_init(struct domain *d)
     else
     {
         d->arch.vgic.dbase = GUEST_GICV3_GICD_BASE;
-        d->arch.vgic.dbase_size = GUEST_GICV3_GICD_SIZE;
 
         /* XXX: Only one Re-distributor region mapped for the guest */
         BUILD_BUG_ON(GUEST_GICV3_RDIST_REGIONS != 1);
@@ -1153,7 +1150,7 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d,
 
     tmp = new_cells;
 
-    dt_set_range(&tmp, node, d->arch.vgic.dbase, d->arch.vgic.dbase_size);
+    dt_set_range(&tmp, node, d->arch.vgic.dbase, SZ_64K);
 
     for ( i = 0; i < d->arch.vgic.nr_regions; i++ )
         dt_set_range(&tmp, node, d->arch.vgic.rdist_regions[i].base,
@@ -1209,15 +1206,15 @@ static int __init gicv3_init(void)
         return -ENODEV;
     }
 
-    res = dt_device_get_address(node, 0, &gicv3.dbase, &gicv3.dbase_size);
+    res = dt_device_get_address(node, 0, &gicv3.dbase, NULL);
     if ( res || !gicv3.dbase )
         panic("GICv3: Cannot find a valid distributor address");
 
-    if ( (gicv3.dbase & ~PAGE_MASK) || (gicv3.dbase_size & ~PAGE_MASK) )
+    if ( (gicv3.dbase & ~PAGE_MASK) )
         panic("GICv3:  Found unaligned distributor address %"PRIpaddr"",
               gicv3.dbase);
 
-    gicv3.map_dbase = ioremap_nocache(gicv3.dbase, gicv3.dbase_size);
+    gicv3.map_dbase = ioremap_nocache(gicv3.dbase, SZ_64K);
     if ( !gicv3.map_dbase )
         panic("GICv3: Failed to ioremap for GIC distributor\n");
 
@@ -1275,7 +1272,6 @@ static int __init gicv3_init(void)
 
     printk("GICv3 initialization:\n"
            "      gic_dist_addr=%"PRIpaddr"\n"
-           "      gic_dist_size=%"PRIpaddr"\n"
            "      gic_dist_mapaddr=%p\n"
            "      gic_rdist_regions=%d\n"
            "      gic_rdist_stride=%x\n"
@@ -1283,7 +1279,7 @@ static int __init gicv3_init(void)
            "      gic_rdist_base_size=%"PRIpaddr"\n"
            "      gic_rdist_base_mapaddr=%p\n"
            "      gic_maintenance_irq=%u\n",
-           gicv3.dbase, gicv3.dbase_size, gicv3.map_dbase, gicv3.rdist_count,
+           gicv3.dbase, gicv3.map_dbase, gicv3.rdist_count,
            gicv3.rdist_stride, gicv3.rdist_regions[0].base,
            gicv3.rdist_regions[0].size, gicv3.rdist_regions[0].map_base,
            gicv3_info.maintenance_irq);
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 4af5a84..a68bebb 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1127,7 +1127,7 @@ static int vgic_v3_domain_init(struct domain *d)
     }
     /* We rely on gicv init to get dbase and size */
     register_mmio_handler(d, &vgic_distr_mmio_handler, d->arch.vgic.dbase,
-                          d->arch.vgic.dbase_size);
+                          SZ_64K);
 
     /*
      * Register mmio handler per contiguous region occupied by the
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 96607d5..ecfdc10 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -103,7 +103,6 @@ struct arch_domain
         paddr_t cbase; /* CPU base address */
 #ifdef HAS_GICV3
         /* GIC V3 addressing */
-        paddr_t dbase_size; /* Distributor base size */
         /* List of contiguous occupied by the redistributors */
         struct vgic_rdist_region {
             paddr_t base;                   /* Base address */
-- 
2.1.4

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

* [v3 05/13] xen/arm: gic-v3: Use the domain redistributor information to make the DT node
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
                   ` (4 preceding siblings ...)
  2015-07-01 11:01 ` [v3 04/13] xen/arm: gic-v3: Fix the distributor region to 64kB Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 11:01 ` [v3 06/13] xen/arm: gic-v3: Rework the messages printed at initialization Julien Grall
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, stefano.stabellini, ian.campbell

It's not necessary to get from the hardware DT the redistributor
informations again. We already have it stored in the gic_info and
the domain.

Use the latter to be consistent with the rest of the function.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v3:
        - Typoes in commit message
        - Add Ian's ack

    Changes in v2:
        - Typo in the commit message
---
 xen/arch/arm/gic-v3.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index d1af147..c8b017f 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1105,9 +1105,6 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d,
     const void *compatible = NULL;
     uint32_t len;
     __be32 *new_cells, *tmp;
-    uint32_t rd_stride = 0;
-    uint32_t rd_count = 0;
-
     int i, res = 0;
 
     compatible = dt_get_property(gic, "compatible", &len);
@@ -1121,19 +1118,13 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d,
     if ( res )
         return res;
 
-    res = dt_property_read_u32(gic, "redistributor-stride", &rd_stride);
-    if ( !res )
-        rd_stride = 0;
-
-    res = dt_property_read_u32(gic, "#redistributor-regions", &rd_count);
-    if ( !res )
-        rd_count = 1;
-
-    res = fdt_property_cell(fdt, "redistributor-stride", rd_stride);
+    res = fdt_property_cell(fdt, "redistributor-stride",
+                            d->arch.vgic.rdist_stride);
     if ( res )
         return res;
 
-    res = fdt_property_cell(fdt, "#redistributor-regions", rd_count);
+    res = fdt_property_cell(fdt, "#redistributor-regions",
+                            d->arch.vgic.nr_regions);
     if ( res )
         return res;
 
-- 
2.1.4

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

* [v3 06/13] xen/arm: gic-v3: Rework the messages printed at initialization
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
                   ` (5 preceding siblings ...)
  2015-07-01 11:01 ` [v3 05/13] xen/arm: gic-v3: Use the domain redistributor information to make the DT node Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 11:01 ` [v3 07/13] xen/arm: gic-{v2, hip04}: Remove redundant check in {gicv2, hip04gic}_init Julien Grall
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, stefano.stabellini, ian.campbell

    - Print all the redistributor regions rather than only the first
    one...
    - Add # in the format to print 0x for hexadecimal. It's easier to
    differentiate from decimal
    - Re-order information printed
    - Drop print of the virtual addresses. It makes the log more
    difficult to read and don't improve user debugging experience (the
    value can't be used like as it is).

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v3:
        - Fix typoes in commit message
        - Add Ian's ack

    Changes in v2:
        - Improve commit message
---
 xen/arch/arm/gic-v3.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index c8b017f..90cfa73 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1262,18 +1262,20 @@ static int __init gicv3_init(void)
     }
 
     printk("GICv3 initialization:\n"
-           "      gic_dist_addr=%"PRIpaddr"\n"
-           "      gic_dist_mapaddr=%p\n"
-           "      gic_rdist_regions=%d\n"
-           "      gic_rdist_stride=%x\n"
-           "      gic_rdist_base=%"PRIpaddr"\n"
-           "      gic_rdist_base_size=%"PRIpaddr"\n"
-           "      gic_rdist_base_mapaddr=%p\n"
-           "      gic_maintenance_irq=%u\n",
-           gicv3.dbase, gicv3.map_dbase, gicv3.rdist_count,
-           gicv3.rdist_stride, gicv3.rdist_regions[0].base,
-           gicv3.rdist_regions[0].size, gicv3.rdist_regions[0].map_base,
-           gicv3_info.maintenance_irq);
+           "      gic_dist_addr=%#"PRIpaddr"\n"
+           "      gic_maintenance_irq=%u\n"
+           "      gic_rdist_stride=%#x\n"
+           "      gic_rdist_regions=%d\n",
+           gicv3.dbase, gicv3_info.maintenance_irq,
+           gicv3.rdist_stride, gicv3.rdist_count);
+    printk("      redistributor regions:\n");
+    for ( i = 0; i < gicv3.rdist_count; i++ )
+    {
+        const struct rdist_region *r = &gicv3.rdist_regions[i];
+
+        printk("        - region %u: %#"PRIpaddr" - %#"PRIpaddr"\n",
+               i, r->base, r->base + r->size);
+    }
 
     spin_lock_init(&gicv3.lock);
 
-- 
2.1.4

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

* [v3 07/13] xen/arm: gic-{v2, hip04}: Remove redundant check in {gicv2, hip04gic}_init
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
                   ` (6 preceding siblings ...)
  2015-07-01 11:01 ` [v3 06/13] xen/arm: gic-v3: Rework the messages printed at initialization Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 11:01 ` [v3 08/13] xen/arm: gic-{v2, hip04}: Use SZ_64K rather than our custom value Julien Grall
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, Zoltan Kiss, stefano.stabellini, ian.campbell

There is a global check for page alignment later within the same function.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Cc: Zoltan Kiss <zoltan.kiss@huawei.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v2:
        - Add Ian's ack
        - Merge "gic-v2: Remove redundant check in gicv2_init" and
        "gic-hip04: Remove redundant check in hip04gic_init"
---
 xen/arch/arm/gic-hip04.c | 8 ++++----
 xen/arch/arm/gic-v2.c    | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
index 669d043..71cdba0 100644
--- a/xen/arch/arm/gic-hip04.c
+++ b/xen/arch/arm/gic-hip04.c
@@ -679,19 +679,19 @@ static int __init hip04gic_init(void)
     const struct dt_device_node *node = gicv2_info.node;
 
     res = dt_device_get_address(node, 0, &gicv2.dbase, NULL);
-    if ( res || !gicv2.dbase || (gicv2.dbase & ~PAGE_MASK) )
+    if ( res || !gicv2.dbase )
         panic("GIC-HIP04: 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) )
+    if ( res || !gicv2.cbase )
         panic("GIC-HIP04: 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) )
+    if ( res || !gicv2.hbase )
         panic("GIC-HIP04: 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) )
+    if ( res || !gicv2.vbase )
         panic("GIC-HIP04: Cannot find a valid address for the virtual CPU");
 
     res = platform_get_irq(node, 0);
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 9e55b21..cecb092 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -665,19 +665,19 @@ static int __init gicv2_init(void)
     const struct dt_device_node *node = gicv2_info.node;
 
     res = dt_device_get_address(node, 0, &gicv2.dbase, NULL);
-    if ( res || !gicv2.dbase || (gicv2.dbase & ~PAGE_MASK) )
+    if ( res || !gicv2.dbase )
         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) )
+    if ( res || !gicv2.cbase )
         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) )
+    if ( res || !gicv2.hbase )
         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) )
+    if ( res || !gicv2.vbase )
         panic("GICv2: Cannot find a valid address for the virtual CPU");
 
     res = platform_get_irq(node, 0);
-- 
2.1.4

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

* [v3 08/13] xen/arm: gic-{v2, hip04}: Use SZ_64K rather than our custom value
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
                   ` (7 preceding siblings ...)
  2015-07-01 11:01 ` [v3 07/13] xen/arm: gic-{v2, hip04}: Remove redundant check in {gicv2, hip04gic}_init Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 11:01 ` [v3 09/13] xen/arm: gic: Allow the base address to be 0 Julien Grall
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, Zoltan Kiss, stefano.stabellini, ian.campbell

It's not easy to understand PAGE_SIZE * 0x10 and PAGE_SIZE * 16 at the
first glance.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Zoltan Kiss <zoltan.kiss@huawei.com>

---
    Changes in v2:
        - Add Ian's ack
        - Merge "xen/arm: gic-v2: Use SZ_64K rather than our custom
        value" and "xen/arm: gic-hip04: Use SZ_64K rather than our
        custom value" in a single patch.
---
 xen/arch/arm/gic-hip04.c | 6 +++---
 xen/arch/arm/gic-v2.c    | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
index 71cdba0..0ba15d1 100644
--- a/xen/arch/arm/gic-hip04.c
+++ b/xen/arch/arm/gic-hip04.c
@@ -29,6 +29,7 @@
 #include <xen/list.h>
 #include <xen/device_tree.h>
 #include <xen/libfdt/libfdt.h>
+#include <xen/sizes.h>
 #include <asm/p2m.h>
 #include <asm/domain.h>
 #include <asm/platform.h>
@@ -470,7 +471,7 @@ static int hip04gicv_setup(struct domain *d)
                                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));
+                               2, paddr_to_pfn(gicv2.vbase + SZ_64K));
 
     return ret;
 }
@@ -721,8 +722,7 @@ static int __init hip04gic_init(void)
     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);
+        gicv2.map_cbase[1] = ioremap_nocache(gicv2.cbase + SZ_64K, PAGE_SIZE);
     else
         gicv2.map_cbase[1] = ioremap_nocache(gicv2.cbase + PAGE_SIZE, PAGE_SIZE);
 
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index cecb092..f49ecd8 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -28,6 +28,7 @@
 #include <xen/list.h>
 #include <xen/device_tree.h>
 #include <xen/libfdt/libfdt.h>
+#include <xen/sizes.h>
 #include <asm/p2m.h>
 #include <asm/domain.h>
 #include <asm/platform.h>
@@ -460,7 +461,7 @@ static int gicv2v_setup(struct domain *d)
                                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));
+                               2, paddr_to_pfn(gicv2.vbase + SZ_64K));
 
     return ret;
 }
@@ -707,8 +708,7 @@ static int __init gicv2_init(void)
     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);
+        gicv2.map_cbase[1] = ioremap_nocache(gicv2.cbase + SZ_64K, PAGE_SIZE);
     else
         gicv2.map_cbase[1] = ioremap_nocache(gicv2.cbase + PAGE_SIZE, PAGE_SIZE);
 
-- 
2.1.4

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

* [v3 09/13] xen/arm: gic: Allow the base address to be 0
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
                   ` (8 preceding siblings ...)
  2015-07-01 11:01 ` [v3 08/13] xen/arm: gic-{v2, hip04}: Use SZ_64K rather than our custom value Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 11:01 ` [v3 10/13] xen/arm: gic-{v2, hip04}: Remove hbase from the global state Julien Grall
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, Zoltan Kiss, stefano.stabellini, ian.campbell

0 is a valid physical address and dt_device_get_address would return
an error if a problem during the retrieving happen.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Zoltan Kiss <zoltan.kiss@huawei.com>

---
    Changes in v2:
        - Add Ian's ack
        - Merge "xen/arm: gic-v2: Allow the base address to be 0"
        and "xen/arm: gic-hip04: Allow the base address to be 0" in a
        single patch.
        - Remove the check in gicv3 too
---
 xen/arch/arm/gic-hip04.c | 8 ++++----
 xen/arch/arm/gic-v2.c    | 8 ++++----
 xen/arch/arm/gic-v3.c    | 4 ++--
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
index 0ba15d1..1b9d35e 100644
--- a/xen/arch/arm/gic-hip04.c
+++ b/xen/arch/arm/gic-hip04.c
@@ -680,19 +680,19 @@ static int __init hip04gic_init(void)
     const struct dt_device_node *node = gicv2_info.node;
 
     res = dt_device_get_address(node, 0, &gicv2.dbase, NULL);
-    if ( res || !gicv2.dbase )
+    if ( res )
         panic("GIC-HIP04: Cannot find a valid address for the distributor");
 
     res = dt_device_get_address(node, 1, &gicv2.cbase, NULL);
-    if ( res || !gicv2.cbase )
+    if ( res )
         panic("GIC-HIP04: Cannot find a valid address for the CPU");
 
     res = dt_device_get_address(node, 2, &gicv2.hbase, NULL);
-    if ( res || !gicv2.hbase )
+    if ( res )
         panic("GIC-HIP04: Cannot find a valid address for the hypervisor");
 
     res = dt_device_get_address(node, 3, &gicv2.vbase, NULL);
-    if ( res || !gicv2.vbase )
+    if ( res )
         panic("GIC-HIP04: Cannot find a valid address for the virtual CPU");
 
     res = platform_get_irq(node, 0);
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index f49ecd8..5d0eb83 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -666,19 +666,19 @@ static int __init gicv2_init(void)
     const struct dt_device_node *node = gicv2_info.node;
 
     res = dt_device_get_address(node, 0, &gicv2.dbase, NULL);
-    if ( res || !gicv2.dbase )
+    if ( res )
         panic("GICv2: Cannot find a valid address for the distributor");
 
     res = dt_device_get_address(node, 1, &gicv2.cbase, NULL);
-    if ( res || !gicv2.cbase )
+    if ( res )
         panic("GICv2: Cannot find a valid address for the CPU");
 
     res = dt_device_get_address(node, 2, &gicv2.hbase, NULL);
-    if ( res || !gicv2.hbase )
+    if ( res )
         panic("GICv2: Cannot find a valid address for the hypervisor");
 
     res = dt_device_get_address(node, 3, &gicv2.vbase, NULL);
-    if ( res || !gicv2.vbase )
+    if ( res )
         panic("GICv2: Cannot find a valid address for the virtual CPU");
 
     res = platform_get_irq(node, 0);
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 90cfa73..1c38c02 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1198,7 +1198,7 @@ static int __init gicv3_init(void)
     }
 
     res = dt_device_get_address(node, 0, &gicv3.dbase, NULL);
-    if ( res || !gicv3.dbase )
+    if ( res )
         panic("GICv3: Cannot find a valid distributor address");
 
     if ( (gicv3.dbase & ~PAGE_MASK) )
@@ -1230,7 +1230,7 @@ static int __init gicv3_init(void)
         uint64_t rdist_base, rdist_size;
 
         res = dt_device_get_address(node, 1 + i, &rdist_base, &rdist_size);
-        if ( res || !rdist_base )
+        if ( res )
             panic("GICv3: No rdist base found for region %d\n", i);
 
         rdist_regs[i].base = rdist_base;
-- 
2.1.4

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

* [v3 10/13] xen/arm: gic-{v2, hip04}: Remove hbase from the global state
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
                   ` (9 preceding siblings ...)
  2015-07-01 11:01 ` [v3 09/13] xen/arm: gic: Allow the base address to be 0 Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 11:01 ` [v3 11/13] xen/arm: Merge gicv_setup with vgic_domain_init Julien Grall
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, Zoltan Kiss, stefano.stabellini, ian.campbell

The driver only needs to know the base address of the hypervisor
register during the GIC initialization (see {gicv2,hip04}_init).

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Zoltan Kiss <zoltan.kiss@huawei.com>

---
    Changes in v2:
        - Add Ian's ack
        - Merge "xen/arm: gic-v2: Remove hbase from the global state"
        and "xen/arm: gic-hip04: Remove hbase from the global state"
---
 xen/arch/arm/gic-hip04.c | 10 +++++-----
 xen/arch/arm/gic-v2.c    | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
index 1b9d35e..984726a 100644
--- a/xen/arch/arm/gic-hip04.c
+++ b/xen/arch/arm/gic-hip04.c
@@ -69,7 +69,6 @@ static struct {
     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;
@@ -677,6 +676,7 @@ static hw_irq_controller hip04gic_guest_irq_type = {
 static int __init hip04gic_init(void)
 {
     int res;
+    paddr_t hbase;
     const struct dt_device_node *node = gicv2_info.node;
 
     res = dt_device_get_address(node, 0, &gicv2.dbase, NULL);
@@ -687,7 +687,7 @@ static int __init hip04gic_init(void)
     if ( res )
         panic("GIC-HIP04: Cannot find a valid address for the CPU");
 
-    res = dt_device_get_address(node, 2, &gicv2.hbase, NULL);
+    res = dt_device_get_address(node, 2, &hbase, NULL);
     if ( res )
         panic("GIC-HIP04: Cannot find a valid address for the hypervisor");
 
@@ -708,11 +708,11 @@ static int __init hip04gic_init(void)
               "        gic_hyp_addr=%"PRIpaddr"\n"
               "        gic_vcpu_addr=%"PRIpaddr"\n"
               "        gic_maintenance_irq=%u\n",
-              gicv2.dbase, gicv2.cbase, gicv2.hbase, gicv2.vbase,
+              gicv2.dbase, gicv2.cbase, hbase, gicv2.vbase,
               gicv2_info.maintenance_irq);
 
     if ( (gicv2.dbase & ~PAGE_MASK) || (gicv2.cbase & ~PAGE_MASK) ||
-         (gicv2.hbase & ~PAGE_MASK) || (gicv2.vbase & ~PAGE_MASK) )
+         (hbase & ~PAGE_MASK) || (gicv2.vbase & ~PAGE_MASK) )
         panic("GIC-HIP04 interfaces not page aligned");
 
     gicv2.map_dbase = ioremap_nocache(gicv2.dbase, PAGE_SIZE);
@@ -729,7 +729,7 @@ static int __init hip04gic_init(void)
     if ( !gicv2.map_cbase[0] || !gicv2.map_cbase[1] )
         panic("GIC-HIP04: Failed to ioremap for GIC CPU interface\n");
 
-    gicv2.map_hbase = ioremap_nocache(gicv2.hbase, PAGE_SIZE);
+    gicv2.map_hbase = ioremap_nocache(hbase, PAGE_SIZE);
     if ( !gicv2.map_hbase )
         panic("GIC-HIP04: Failed to ioremap for GIC Virtual interface\n");
 
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 5d0eb83..6ec6602 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -68,7 +68,6 @@ static struct {
     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;
@@ -663,6 +662,7 @@ static hw_irq_controller gicv2_guest_irq_type = {
 static int __init gicv2_init(void)
 {
     int res;
+    paddr_t hbase;
     const struct dt_device_node *node = gicv2_info.node;
 
     res = dt_device_get_address(node, 0, &gicv2.dbase, NULL);
@@ -673,7 +673,7 @@ static int __init gicv2_init(void)
     if ( res )
         panic("GICv2: Cannot find a valid address for the CPU");
 
-    res = dt_device_get_address(node, 2, &gicv2.hbase, NULL);
+    res = dt_device_get_address(node, 2, &hbase, NULL);
     if ( res )
         panic("GICv2: Cannot find a valid address for the hypervisor");
 
@@ -694,11 +694,11 @@ static int __init gicv2_init(void)
               "        gic_hyp_addr=%"PRIpaddr"\n"
               "        gic_vcpu_addr=%"PRIpaddr"\n"
               "        gic_maintenance_irq=%u\n",
-              gicv2.dbase, gicv2.cbase, gicv2.hbase, gicv2.vbase,
+              gicv2.dbase, gicv2.cbase, hbase, gicv2.vbase,
               gicv2_info.maintenance_irq);
 
     if ( (gicv2.dbase & ~PAGE_MASK) || (gicv2.cbase & ~PAGE_MASK) ||
-         (gicv2.hbase & ~PAGE_MASK) || (gicv2.vbase & ~PAGE_MASK) )
+         (hbase & ~PAGE_MASK) || (gicv2.vbase & ~PAGE_MASK) )
         panic("GICv2 interfaces not page aligned");
 
     gicv2.map_dbase = ioremap_nocache(gicv2.dbase, PAGE_SIZE);
@@ -715,7 +715,7 @@ static int __init gicv2_init(void)
     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);
+    gicv2.map_hbase = ioremap_nocache(hbase, PAGE_SIZE);
     if ( !gicv2.map_hbase )
         panic("GICv2: Failed to ioremap for GIC Virtual interface\n");
 
-- 
2.1.4

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

* [v3 11/13] xen/arm: Merge gicv_setup with vgic_domain_init
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
                   ` (10 preceding siblings ...)
  2015-07-01 11:01 ` [v3 10/13] xen/arm: gic-{v2, hip04}: Remove hbase from the global state Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 11:01 ` [v3 12/13] arm: Allow the user to specify the GIC version Julien Grall
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, stefano.stabellini, ian.campbell

Currently, it's hard to decide whether a part of the domain
initialization  should live in gicv_setup (part of the GIC
driver) and domain_init (part of the vGIC driver).

The code to initialize the domain for a specific vGIC version is always
the same no matter the version of the GIC.

Move all the domain initialization code for the vGIC in the respective
domain_init callback of each vGIC drivers.

New structures have been introduced to store HW information per vGIC.
Each vGIC HW structure contains a boolean to indicate if the current GIC is
able to support this specific version of virtual GIC.

Helpers have been introduced in order to help the GIC correctly setup
the vGIC. The GIC will have to call them to announce support for this
specific version.

Also drop fields that become unnecessary in each global state.

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

---
    Changes in v3:
        - Fix typoes in the commit message
        - Merge "xen/arm: Merge gicv_setup with vgic_domain_init" and
        "xen/arm: gic: Store the necessary HW information per vGIC ..."
        - Move the structure internally to each vGIC drivers and use
        vgic_vN_init to check whether the vGIC is supported or not.
        - Drop gic_info function which is not used anymore

    Changes in v2:
        * "xen/arm: gic: Store the necessary HW information per vGIC ...":
            - Patch added in replacement of "xen/arm: gic-v2: Move GICD,
            GICC and GICV base address in gic_info ..." and "xen/arm:
            gic-v3: Move Distributor and Re-Distributors info in gic_info
            ..."
        * "xen/arm: Merge gicv_setup with vgic_domain_init":
            - The code merged is slightly different. Although I haven't keep
            Ian's ack
---
 xen/arch/arm/domain.c             |  3 --
 xen/arch/arm/gic-hip04.c          | 69 ++++++--------------------------
 xen/arch/arm/gic-v2.c             | 69 ++++++--------------------------
 xen/arch/arm/gic-v3.c             | 79 +++++-------------------------------
 xen/arch/arm/gic.c                |  5 ---
 xen/arch/arm/vgic-v2.c            | 69 ++++++++++++++++++++++++++++++--
 xen/arch/arm/vgic-v3.c            | 84 ++++++++++++++++++++++++++++++++++++++-
 xen/arch/arm/vgic.c               |  2 +
 xen/include/asm-arm/gic.h         |  2 -
 xen/include/asm-arm/gic_v3_defs.h |  7 ++++
 xen/include/asm-arm/vgic.h        | 10 +++++
 11 files changed, 202 insertions(+), 197 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 8b1bf5a..21a03df 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -587,9 +587,6 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
     }
     config->gic_version = gic_version;
 
-    if ( (rc = gicv_setup(d)) != 0 )
-        goto fail;
-
     if ( (rc = domain_vgic_init(d, config->nr_spis)) != 0 )
         goto fail;
 
diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
index 984726a..c5ed545 100644
--- a/xen/arch/arm/gic-hip04.c
+++ b/xen/arch/arm/gic-hip04.c
@@ -65,12 +65,9 @@
 
 /* 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 */
     void __iomem * map_hbase; /* IO Address of virtual interface registers */
-    paddr_t vbase;            /* Address of virtual cpu interface registers */
     spinlock_t lock;
 } gicv2;
 
@@ -434,47 +431,6 @@ static void hip04gic_clear_lr(int lr)
     writel_gich(0, HIP04_GICH_LR + lr * 4);
 }
 
-static int hip04gicv_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 + SZ_64K));
-
-    return ret;
-}
-
 static void hip04gic_read_lr(int lr, struct gic_lr *lr_reg)
 {
     uint32_t lrv;
@@ -676,14 +632,14 @@ static hw_irq_controller hip04gic_guest_irq_type = {
 static int __init hip04gic_init(void)
 {
     int res;
-    paddr_t hbase;
+    paddr_t hbase, dbase, cbase, vbase;
     const struct dt_device_node *node = gicv2_info.node;
 
-    res = dt_device_get_address(node, 0, &gicv2.dbase, NULL);
+    res = dt_device_get_address(node, 0, &dbase, NULL);
     if ( res )
         panic("GIC-HIP04: Cannot find a valid address for the distributor");
 
-    res = dt_device_get_address(node, 1, &gicv2.cbase, NULL);
+    res = dt_device_get_address(node, 1, &cbase, NULL);
     if ( res )
         panic("GIC-HIP04: Cannot find a valid address for the CPU");
 
@@ -691,7 +647,7 @@ static int __init hip04gic_init(void)
     if ( res )
         panic("GIC-HIP04: Cannot find a valid address for the hypervisor");
 
-    res = dt_device_get_address(node, 3, &gicv2.vbase, NULL);
+    res = dt_device_get_address(node, 3, &vbase, NULL);
     if ( res )
         panic("GIC-HIP04: Cannot find a valid address for the virtual CPU");
 
@@ -708,23 +664,23 @@ static int __init hip04gic_init(void)
               "        gic_hyp_addr=%"PRIpaddr"\n"
               "        gic_vcpu_addr=%"PRIpaddr"\n"
               "        gic_maintenance_irq=%u\n",
-              gicv2.dbase, gicv2.cbase, hbase, gicv2.vbase,
+              dbase, cbase, hbase, vbase,
               gicv2_info.maintenance_irq);
 
-    if ( (gicv2.dbase & ~PAGE_MASK) || (gicv2.cbase & ~PAGE_MASK) ||
-         (hbase & ~PAGE_MASK) || (gicv2.vbase & ~PAGE_MASK) )
+    if ( (dbase & ~PAGE_MASK) || (cbase & ~PAGE_MASK) ||
+         (hbase & ~PAGE_MASK) || (vbase & ~PAGE_MASK) )
         panic("GIC-HIP04 interfaces not page aligned");
 
-    gicv2.map_dbase = ioremap_nocache(gicv2.dbase, PAGE_SIZE);
+    gicv2.map_dbase = ioremap_nocache(dbase, PAGE_SIZE);
     if ( !gicv2.map_dbase )
         panic("GIC-HIP04: Failed to ioremap for GIC distributor\n");
 
-    gicv2.map_cbase[0] = ioremap_nocache(gicv2.cbase, PAGE_SIZE);
+    gicv2.map_cbase[0] = ioremap_nocache(cbase, PAGE_SIZE);
 
     if ( platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) )
-        gicv2.map_cbase[1] = ioremap_nocache(gicv2.cbase + SZ_64K, PAGE_SIZE);
+        gicv2.map_cbase[1] = ioremap_nocache(cbase + SZ_64K, PAGE_SIZE);
     else
-        gicv2.map_cbase[1] = ioremap_nocache(gicv2.cbase + PAGE_SIZE, PAGE_SIZE);
+        gicv2.map_cbase[1] = ioremap_nocache(cbase + PAGE_SIZE, PAGE_SIZE);
 
     if ( !gicv2.map_cbase[0] || !gicv2.map_cbase[1] )
         panic("GIC-HIP04: Failed to ioremap for GIC CPU interface\n");
@@ -733,6 +689,8 @@ static int __init hip04gic_init(void)
     if ( !gicv2.map_hbase )
         panic("GIC-HIP04: Failed to ioremap for GIC Virtual interface\n");
 
+    vgic_v2_setup_hw(dbase, cbase, vbase);
+
     /* Global settings: interrupt distributor */
     spin_lock_init(&gicv2.lock);
     spin_lock(&gicv2.lock);
@@ -753,7 +711,6 @@ const static struct gic_hw_operations hip04gic_ops = {
     .save_state          = hip04gic_save_state,
     .restore_state       = hip04gic_restore_state,
     .dump_state          = hip04gic_dump_state,
-    .gicv_setup          = hip04gicv_setup,
     .gic_host_irq_type   = &hip04gic_host_irq_type,
     .gic_guest_irq_type  = &hip04gic_guest_irq_type,
     .eoi_irq             = hip04gic_eoi_irq,
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 6ec6602..596126d 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -64,12 +64,9 @@
 
 /* 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 */
     void __iomem * map_hbase; /* IO Address of virtual interface registers */
-    paddr_t vbase;            /* Address of virtual cpu interface registers */
     spinlock_t lock;
 } gicv2;
 
@@ -424,47 +421,6 @@ 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 + SZ_64K));
-
-    return ret;
-}
-
 static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
 {
     uint32_t lrv;
@@ -662,14 +618,14 @@ static hw_irq_controller gicv2_guest_irq_type = {
 static int __init gicv2_init(void)
 {
     int res;
-    paddr_t hbase;
+    paddr_t hbase, dbase, cbase, vbase;
     const struct dt_device_node *node = gicv2_info.node;
 
-    res = dt_device_get_address(node, 0, &gicv2.dbase, NULL);
+    res = dt_device_get_address(node, 0, &dbase, NULL);
     if ( res )
         panic("GICv2: Cannot find a valid address for the distributor");
 
-    res = dt_device_get_address(node, 1, &gicv2.cbase, NULL);
+    res = dt_device_get_address(node, 1, &cbase, NULL);
     if ( res )
         panic("GICv2: Cannot find a valid address for the CPU");
 
@@ -677,7 +633,7 @@ static int __init gicv2_init(void)
     if ( res )
         panic("GICv2: Cannot find a valid address for the hypervisor");
 
-    res = dt_device_get_address(node, 3, &gicv2.vbase, NULL);
+    res = dt_device_get_address(node, 3, &vbase, NULL);
     if ( res )
         panic("GICv2: Cannot find a valid address for the virtual CPU");
 
@@ -694,23 +650,23 @@ static int __init gicv2_init(void)
               "        gic_hyp_addr=%"PRIpaddr"\n"
               "        gic_vcpu_addr=%"PRIpaddr"\n"
               "        gic_maintenance_irq=%u\n",
-              gicv2.dbase, gicv2.cbase, hbase, gicv2.vbase,
+              dbase, cbase, hbase, vbase,
               gicv2_info.maintenance_irq);
 
-    if ( (gicv2.dbase & ~PAGE_MASK) || (gicv2.cbase & ~PAGE_MASK) ||
-         (hbase & ~PAGE_MASK) || (gicv2.vbase & ~PAGE_MASK) )
+    if ( (dbase & ~PAGE_MASK) || (cbase & ~PAGE_MASK) ||
+         (hbase & ~PAGE_MASK) || (vbase & ~PAGE_MASK) )
         panic("GICv2 interfaces not page aligned");
 
-    gicv2.map_dbase = ioremap_nocache(gicv2.dbase, PAGE_SIZE);
+    gicv2.map_dbase = ioremap_nocache(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);
+    gicv2.map_cbase[0] = ioremap_nocache(cbase, PAGE_SIZE);
 
     if ( platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) )
-        gicv2.map_cbase[1] = ioremap_nocache(gicv2.cbase + SZ_64K, PAGE_SIZE);
+        gicv2.map_cbase[1] = ioremap_nocache(cbase + SZ_64K, PAGE_SIZE);
     else
-        gicv2.map_cbase[1] = ioremap_nocache(gicv2.cbase + PAGE_SIZE, PAGE_SIZE);
+        gicv2.map_cbase[1] = ioremap_nocache(cbase + PAGE_SIZE, PAGE_SIZE);
 
     if ( !gicv2.map_cbase[0] || !gicv2.map_cbase[1] )
         panic("GICv2: Failed to ioremap for GIC CPU interface\n");
@@ -719,6 +675,8 @@ static int __init gicv2_init(void)
     if ( !gicv2.map_hbase )
         panic("GICv2: Failed to ioremap for GIC Virtual interface\n");
 
+    vgic_v2_setup_hw(dbase, cbase, vbase);
+
     /* Global settings: interrupt distributor */
     spin_lock_init(&gicv2.lock);
     spin_lock(&gicv2.lock);
@@ -739,7 +697,6 @@ const static struct gic_hw_operations gicv2_ops = {
     .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,
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 1c38c02..337fbb9 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -42,15 +42,8 @@
 #include <asm/gic_v3_defs.h>
 #include <asm/cpufeature.h>
 
-struct rdist_region {
-    paddr_t base;
-    paddr_t size;
-    void __iomem *map_base;
-};
-
 /* Global state */
 static struct {
-    paddr_t dbase;            /* Address of distributor registers */
     void __iomem *map_dbase;  /* Mapped address of distributor registers */
     struct rdist_region *rdist_regions;
     uint32_t  rdist_stride;
@@ -924,63 +917,6 @@ static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr)
     gicv3_ich_write_lr(lr_reg, lrv);
 }
 
-static int gicv_v3_init(struct domain *d)
-{
-    int i;
-
-    /*
-     * Domain 0 gets the hardware address.
-     * Guests get the virtual platform layout.
-     */
-    if ( is_hardware_domain(d) )
-    {
-        unsigned int first_cpu = 0;
-
-        d->arch.vgic.dbase = gicv3.dbase;
-
-        d->arch.vgic.rdist_stride = gicv3.rdist_stride;
-        /*
-         * If the stride is not set, the default stride for GICv3 is 2 * 64K:
-         *     - first 64k page for Control and Physical LPIs
-         *     - second 64k page for Control and Generation of SGIs
-         */
-        if ( !d->arch.vgic.rdist_stride )
-            d->arch.vgic.rdist_stride = 2 * SZ_64K;
-
-        for ( i = 0; i < gicv3.rdist_count; i++ )
-        {
-            paddr_t size = gicv3.rdist_regions[i].size;
-
-            d->arch.vgic.rdist_regions[i].base = gicv3.rdist_regions[i].base;
-            d->arch.vgic.rdist_regions[i].size = size;
-
-            /* Set the first CPU handled by this region */
-            d->arch.vgic.rdist_regions[i].first_cpu = first_cpu;
-
-            first_cpu += size / d->arch.vgic.rdist_stride;
-        }
-        d->arch.vgic.nr_regions = gicv3.rdist_count;
-    }
-    else
-    {
-        d->arch.vgic.dbase = GUEST_GICV3_GICD_BASE;
-
-        /* XXX: Only one Re-distributor region mapped for the guest */
-        BUILD_BUG_ON(GUEST_GICV3_RDIST_REGIONS != 1);
-
-        d->arch.vgic.nr_regions = GUEST_GICV3_RDIST_REGIONS;
-        d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE;
-
-        /* The first redistributor should contain enough space for all CPUs */
-        BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < MAX_VIRT_CPUS);
-        d->arch.vgic.rdist_regions[0].base = GUEST_GICV3_GICR0_BASE;
-        d->arch.vgic.rdist_regions[0].size = GUEST_GICV3_GICR0_SIZE;
-        d->arch.vgic.rdist_regions[0].first_cpu = 0;
-    }
-
-    return 0;
-}
-
 static void gicv3_hcr_status(uint32_t flag, bool_t status)
 {
     uint32_t hcr;
@@ -1190,6 +1126,7 @@ static int __init gicv3_init(void)
     int res, i;
     uint32_t reg;
     const struct dt_device_node *node = gicv3_info.node;
+    paddr_t dbase;
 
     if ( !cpu_has_gicv3 )
     {
@@ -1197,15 +1134,15 @@ static int __init gicv3_init(void)
         return -ENODEV;
     }
 
-    res = dt_device_get_address(node, 0, &gicv3.dbase, NULL);
+    res = dt_device_get_address(node, 0, &dbase, NULL);
     if ( res )
         panic("GICv3: Cannot find a valid distributor address");
 
-    if ( (gicv3.dbase & ~PAGE_MASK) )
+    if ( (dbase & ~PAGE_MASK) )
         panic("GICv3:  Found unaligned distributor address %"PRIpaddr"",
-              gicv3.dbase);
+              dbase);
 
-    gicv3.map_dbase = ioremap_nocache(gicv3.dbase, SZ_64K);
+    gicv3.map_dbase = ioremap_nocache(dbase, SZ_64K);
     if ( !gicv3.map_dbase )
         panic("GICv3: Failed to ioremap for GIC distributor\n");
 
@@ -1266,7 +1203,7 @@ static int __init gicv3_init(void)
            "      gic_maintenance_irq=%u\n"
            "      gic_rdist_stride=%#x\n"
            "      gic_rdist_regions=%d\n",
-           gicv3.dbase, gicv3_info.maintenance_irq,
+           dbase, gicv3_info.maintenance_irq,
            gicv3.rdist_stride, gicv3.rdist_count);
     printk("      redistributor regions:\n");
     for ( i = 0; i < gicv3.rdist_count; i++ )
@@ -1277,6 +1214,9 @@ static int __init gicv3_init(void)
                i, r->base, r->base + r->size);
     }
 
+    vgic_v3_setup_hw(dbase, gicv3.rdist_count, gicv3.rdist_regions,
+                     gicv3.rdist_stride);
+
     spin_lock_init(&gicv3.lock);
 
     spin_lock(&gicv3.lock);
@@ -1296,7 +1236,6 @@ static const struct gic_hw_operations gicv3_ops = {
     .save_state          = gicv3_save_state,
     .restore_state       = gicv3_restore_state,
     .dump_state          = gicv3_dump_state,
-    .gicv_setup          = gicv_v3_init,
     .gic_host_irq_type   = &gicv3_host_irq_type,
     .gic_guest_irq_type  = &gicv3_guest_irq_type,
     .eoi_irq             = gicv3_eoi_irq,
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 341b6df..1757193 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -661,11 +661,6 @@ void gic_interrupt(struct cpu_user_regs *regs, int is_fiq)
     } while (1);
 }
 
-int gicv_setup(struct domain *d)
-{
-    return gic_hw_ops->gicv_setup(d);
-}
-
 static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
 {
     /*
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index b5a8f29..b246b3c 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -24,13 +24,32 @@
 #include <xen/softirq.h>
 #include <xen/irq.h>
 #include <xen/sched.h>
+#include <xen/sizes.h>
 
 #include <asm/current.h>
 
 #include <asm/mmio.h>
-#include <asm/gic.h>
+#include <asm/platform.h>
 #include <asm/vgic.h>
 
+static struct {
+    bool_t enabled;
+    /* Distributor interface address */
+    paddr_t dbase;
+    /* CPU interface address */
+    paddr_t cbase;
+    /* Virtual CPU interface address */
+    paddr_t vbase;
+} vgic_v2_hw;
+
+void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase, paddr_t vbase)
+{
+    vgic_v2_hw.enabled = 1;
+    vgic_v2_hw.dbase = dbase;
+    vgic_v2_hw.cbase = cbase;
+    vgic_v2_hw.vbase = vbase;
+}
+
 static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
 {
     struct hsr_dabt dabt = info->dabt;
@@ -525,14 +544,50 @@ static int vgic_v2_vcpu_init(struct vcpu *v)
 
 static int vgic_v2_domain_init(struct domain *d)
 {
-    int i;
+    int i, ret;
+
+    /*
+     * The hardware domain gets the hardware address.
+     * Guests get the virtual platform layout.
+     */
+    if ( is_hardware_domain(d) )
+    {
+        d->arch.vgic.dbase = vgic_v2_hw.dbase;
+        d->arch.vgic.cbase = vgic_v2_hw.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(vgic_v2_hw.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(vgic_v2_hw.vbase + PAGE_SIZE));
+    else
+        ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + PAGE_SIZE),
+                               2, paddr_to_pfn(vgic_v2_hw.vbase + SZ_64K));
+
+    if ( ret )
+        return ret;
 
     /* By default deliver to CPU0 */
     for ( i = 0; i < DOMAIN_NR_RANKS(d); i++ )
         memset(d->arch.vgic.shared_irqs[i].v2.itargets, 0x1,
                sizeof(d->arch.vgic.shared_irqs[i].v2.itargets));
 
-    /* We rely on gicv_setup() to initialize dbase(vGIC distributor base) */
     register_mmio_handler(d, &vgic_v2_distr_mmio_handler, d->arch.vgic.dbase,
                           PAGE_SIZE);
 
@@ -548,6 +603,14 @@ static const struct vgic_ops vgic_v2_ops = {
 
 int vgic_v2_init(struct domain *d)
 {
+    if ( !vgic_v2_hw.enabled )
+    {
+        printk(XENLOG_G_ERR
+               "d%d: vGICv2 is not supported on this platform.\n",
+               d->domain_id);
+        return -ENODEV;
+    }
+
     register_vgic_ops(d, &vgic_v2_ops);
 
     return 0;
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index a68bebb..77428c5 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -29,7 +29,6 @@
 #include <asm/current.h>
 #include <asm/mmio.h>
 #include <asm/gic_v3_defs.h>
-#include <asm/gic.h>
 #include <asm/vgic.h>
 
 /* GICD_PIDRn register values for ARM implementations */
@@ -50,6 +49,28 @@
  */
 #define VGICD_CTLR_DEFAULT  (GICD_CTLR_ARE_NS)
 
+static struct {
+    bool_t enabled;
+    /* Distributor interface address */
+    paddr_t dbase;
+    /* Re-distributor regions */
+    unsigned int nr_rdist_regions;
+    const struct rdist_region *regions;
+    uint32_t rdist_stride; /* Re-distributor stride */
+} vgic_v3_hw;
+
+void vgic_v3_setup_hw(paddr_t dbase,
+                      unsigned int nr_rdist_regions,
+                      const struct rdist_region *regions,
+                      uint32_t rdist_stride)
+{
+    vgic_v3_hw.enabled = 1;
+    vgic_v3_hw.dbase = dbase;
+    vgic_v3_hw.nr_rdist_regions = nr_rdist_regions;
+    vgic_v3_hw.regions = regions;
+    vgic_v3_hw.rdist_stride = rdist_stride;
+}
+
 static struct vcpu *vgic_v3_irouter_to_vcpu(struct domain *d, uint64_t irouter)
 {
     unsigned int vcpu_id;
@@ -1119,13 +1140,64 @@ static int vgic_v3_domain_init(struct domain *d)
 {
     int i, idx;
 
+    /*
+     * Domain 0 gets the hardware address.
+     * Guests get the virtual platform layout.
+     */
+    if ( is_hardware_domain(d) )
+    {
+        unsigned int first_cpu = 0;
+
+        d->arch.vgic.dbase = vgic_v3_hw.dbase;
+
+        d->arch.vgic.rdist_stride = vgic_v3_hw.rdist_stride;
+        /*
+         * If the stride is not set, the default stride for GICv3 is 2 * 64K:
+         *     - first 64k page for Control and Physical LPIs
+         *     - second 64k page for Control and Generation of SGIs
+         */
+        if ( !d->arch.vgic.rdist_stride )
+            d->arch.vgic.rdist_stride = 2 * SZ_64K;
+
+        for ( i = 0; i < vgic_v3_hw.nr_rdist_regions; i++ )
+        {
+            paddr_t size = vgic_v3_hw.regions[i].size;
+
+            d->arch.vgic.rdist_regions[i].base = vgic_v3_hw.regions[i].base;
+            d->arch.vgic.rdist_regions[i].size = size;
+
+            /* Set the first CPU handled by this region */
+            d->arch.vgic.rdist_regions[i].first_cpu = first_cpu;
+
+            first_cpu += size / d->arch.vgic.rdist_stride;
+        }
+        d->arch.vgic.nr_regions = vgic_v3_hw.nr_rdist_regions;
+    }
+    else
+    {
+        d->arch.vgic.dbase = GUEST_GICV3_GICD_BASE;
+
+        /* XXX: Only one Re-distributor region mapped for the guest */
+        BUILD_BUG_ON(GUEST_GICV3_RDIST_REGIONS != 1);
+
+        d->arch.vgic.nr_regions = GUEST_GICV3_RDIST_REGIONS;
+        d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE;
+
+        /* The first redistributor should contain enough space for all CPUs */
+        BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < MAX_VIRT_CPUS);
+        d->arch.vgic.rdist_regions[0].base = GUEST_GICV3_GICR0_BASE;
+        d->arch.vgic.rdist_regions[0].size = GUEST_GICV3_GICR0_SIZE;
+        d->arch.vgic.rdist_regions[0].first_cpu = 0;
+    }
+
     /* By default deliver to CPU0 */
     for ( i = 0; i < DOMAIN_NR_RANKS(d); i++ )
     {
         for ( idx = 0; idx < 32; idx++ )
             d->arch.vgic.shared_irqs[i].v3.irouter[idx] = 0;
     }
-    /* We rely on gicv init to get dbase and size */
+
+    /* Register mmio handle for the Distributor */
     register_mmio_handler(d, &vgic_distr_mmio_handler, d->arch.vgic.dbase,
                           SZ_64K);
 
@@ -1154,6 +1226,14 @@ static const struct vgic_ops v3_ops = {
 
 int vgic_v3_init(struct domain *d)
 {
+    if ( !vgic_v3_hw.enabled )
+    {
+        printk(XENLOG_G_ERR
+               "d%d: vGICv3 is not supported on this platform.\n",
+               d->domain_id);
+        return -ENODEV;
+    }
+
     register_vgic_ops(d, &v3_ops);
 
     return 0;
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index fe6c7a2..c1c2579 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -94,6 +94,8 @@ int domain_vgic_init(struct domain *d, unsigned int nr_spis)
             return -ENODEV;
         break;
     default:
+        printk(XENLOG_G_ERR "d%d: Unknown vGIC version %u\n",
+               d->domain_id, gic_hw_version());
         return -ENODEV;
     }
 
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 71f4813..d343abf 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -311,8 +311,6 @@ struct gic_hw_operations {
     void (*restore_state)(const struct vcpu *);
     /* Dump GIC LR register information */
     void (*dump_state)(const struct vcpu *);
-    /* Map MMIO region of GIC */
-    int (*gicv_setup)(struct domain *);
 
     /* hw_irq_controller to enable/disable/eoi host irq */
     hw_irq_controller *gic_host_irq_type;
diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
index 556f114..42bdaa3 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -152,6 +152,13 @@
 #define ICH_SGI_IRQ_SHIFT            24
 #define ICH_SGI_IRQ_MASK             0xf
 #define ICH_SGI_TARGETLIST_MASK      0xffff
+
+struct rdist_region {
+    paddr_t base;
+    paddr_t size;
+    void __iomem *map_base;
+};
+
 #endif /* __ASM_ARM_GIC_V3_DEFS_H__ */
 
 /*
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 8d22532..947b7a5 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -223,6 +223,16 @@ static inline int vgic_allocate_spi(struct domain *d)
 
 extern void vgic_free_virq(struct domain *d, unsigned int virq);
 
+void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase, paddr_t vbase);
+
+#ifdef HAS_GICV3
+struct rdist_region;
+void vgic_v3_setup_hw(paddr_t dbase,
+                      unsigned int nr_rdist_regions,
+                      const struct rdist_region *regions,
+                      uint32_t rdist_stride);
+#endif
+
 #endif /* __ASM_ARM_VGIC_H__ */
 
 /*
-- 
2.1.4

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

* [v3 12/13] arm: Allow the user to specify the GIC version
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
                   ` (11 preceding siblings ...)
  2015-07-01 11:01 ` [v3 11/13] xen/arm: Merge gicv_setup with vgic_domain_init Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-01 13:52   ` Ian Jackson
  2015-07-01 11:01 ` [v3 13/13] xen/arm: gic-v3: Add support of vGICv2 when available Julien Grall
  2015-07-02 10:37 ` [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
  14 siblings, 1 reply; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel
  Cc: Julien Grall, Ian Jackson, stefano.stabellini, ian.campbell,
	Wei Liu

A platform may have a GIC compatible with previous version of the
device.

This is allow to virtualize an unmodified OS on new hardware if the GIC
is compatible with older version.

When a guest is created, the vGIC will emulate same version as the
hardware. Although, the user can specify in the configuration file the
preferred version (currently only GICv2 and GICv3 are supported).

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>

---
    The hypervisor will check if the GIC is able to virtualize the
    version specified by the user (via the DOMCTL createdomain).
    If it's not compatible an error will be send on the Xen console
    which will make the error not obvious for user.

    I left aside a user error reporting for a follow-up as I'm not
    sure how to notify the user which GIC versions are available. May be
    by a new mechanism similar to xen_get_caps?

    Changes in v3:
        - Rename GIC_VERSION define in LIBXL_GIC_VERSION_Vn.
        - Change the value of each define
        - Use libxl_gic_version_from_string rather than custom if/else
        - Rename LIBXL_HAVE_BUILDINFO_GIC_VERSION into
        LIBXL_HAVE_BUILDINFO_ARM_GIC_VERSION and update the comment
        - Update doc with Ian's suggestion
        - Typoes

    Changes in v2:
        - Introduce arch_arm in libxl_domain_build_info to store ARM
        specific field
        - Add docs
        - Remove code that is not necessary with the new version
---
 docs/man/xl.cfg.pod.5        | 29 ++++++++++++++++++++++++++++
 tools/libxl/libxl.h          |  5 +++++
 tools/libxl/libxl_arm.c      | 16 +++++++++++++++-
 tools/libxl/libxl_types.idl  | 11 +++++++++++
 tools/libxl/xl_cmdimpl.c     | 12 ++++++++++++
 xen/arch/arm/domain.c        | 45 ++++++++++++++++++++++++++------------------
 xen/arch/arm/vgic.c          |  4 ++--
 xen/include/asm-arm/domain.h |  2 ++
 8 files changed, 103 insertions(+), 21 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index a3e0e2e..9e66ce4 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1688,6 +1688,35 @@ The default is B<en-us>.
 
 See L<qemu(1)> for more information.
 
+=head2 Architecture Specific options
+
+=head3 ARM
+
+=over 4
+
+=item B<gic_version="vN">
+
+Version of the GIC emulated for the guest. Currently, the following
+versions are supported:
+
+=over 4
+
+=item B<v2>
+
+Emulate a GICv2 hardware
+
+=item B<v3>
+
+Emulate a GICv3 hardware. Note that the emulated GIC does not support the
+GICv2 compatibility mode.
+
+=back
+
+This requires hardware compatibility with the requested version. Either
+natively or via hardware backwards compatibility support.
+
+=back
+
 =head1 SEE ALSO
 
 =over 4
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index a1c5d15..2548480 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -200,6 +200,11 @@
 #define LIBXL_HAVE_DEVICETREE_PASSTHROUGH 1
 
 /*
+ * libxl_domain_build_info has the arm.gic_version field.
+ */
+#define LIBXL_HAVE_BUILDINFO_ARM_GIC_VERSION 1
+
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index f09c860..95a0dd1 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -61,7 +61,21 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
     xc_config->nr_spis = nr_spis;
     LOG(DEBUG, " - Allocate %u SPIs", nr_spis);
 
-    xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT;
+    switch (d_config->b_info.arch_arm.gic_version) {
+    case LIBXL_GIC_VERSION_DEFAULT:
+        xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT;
+        break;
+    case LIBXL_GIC_VERSION_V2:
+        xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_V2;
+        break;
+    case LIBXL_GIC_VERSION_V3:
+        xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
+        break;
+    default:
+        LOG(ERROR, "Unknown GIC version %s\n",
+            libxl_gic_version_to_string(d_config->b_info.arch_arm.gic_version));
+        break;
+    }
 
     return 0;
 }
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index e1632fa..11f6461 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -369,6 +369,12 @@ libxl_vnode_info = Struct("vnode_info", [
     ("vcpus", libxl_bitmap), # vcpus in this node
     ])
 
+libxl_gic_version = Enumeration("gic_version", [
+    (0, "DEFAULT"),
+    (0x20, "v2"),
+    (0x30, "v3")
+    ], init_val = "LIBXL_GIC_VERSION_DEFAULT")
+
 libxl_domain_build_info = Struct("domain_build_info",[
     ("max_vcpus",       integer),
     ("avail_vcpus",     libxl_bitmap),
@@ -480,6 +486,11 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                       ])),
                  ("invalid", None),
                  ], keyvar_init_val = "LIBXL_DOMAIN_TYPE_INVALID")),
+
+
+    ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
+                              ])),
+
     ], dir=DIR_IN
 )
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index c858068..1e44692 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2242,6 +2242,18 @@ skip_vfb:
         }
     }
 
+    if (!xlu_cfg_get_string (config, "gic_version", &buf, 1)) {
+        libxl_gic_version v;
+
+        e = libxl_gic_version_from_string(buf, &v);
+        if (e) {
+            fprintf(stderr,
+                    "Unknown gic_version \"%s\" specified\n", buf);
+            exit(-ERROR_FAIL);
+        }
+        b_info->arch_arm.gic_version = v;
+     }
+
     xlu_cfg_destroy(config);
 }
 
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 21a03df..107d58f 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -535,7 +535,6 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
                        struct xen_arch_domainconfig *config)
 {
     int rc;
-    uint8_t gic_version;
 
     d->arch.relmem = RELMEM_not_started;
 
@@ -564,28 +563,38 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
     if ( (rc = p2m_alloc_table(d)) != 0 )
         goto fail;
 
-    /*
-     * Currently the vGIC is emulating the same version of the
-     * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT
-     * is allowed. The DOMCTL will return the actual version of the
-     * GIC.
-     */
-    rc = -EOPNOTSUPP;
-    if ( config->gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT )
-        goto fail;
-
-    switch ( gic_hw_version() )
+    switch ( config->gic_version )
     {
-    case GIC_V3:
-        gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
+    case XEN_DOMCTL_CONFIG_GIC_DEFAULT:
+        switch ( gic_hw_version () )
+        {
+        case GIC_V2:
+            config->gic_version = XEN_DOMCTL_CONFIG_GIC_V2;
+            d->arch.vgic.version = GIC_V2;
+            break;
+
+        case GIC_V3:
+            config->gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
+            d->arch.vgic.version = GIC_V3;
+            break;
+
+        default:
+            BUG();
+        }
+        break;
+
+    case XEN_DOMCTL_CONFIG_GIC_V2:
+        d->arch.vgic.version = GIC_V2;
         break;
-    case GIC_V2:
-        gic_version = XEN_DOMCTL_CONFIG_GIC_V2;
+
+    case XEN_DOMCTL_CONFIG_GIC_V3:
+        d->arch.vgic.version = GIC_V3;
         break;
+
     default:
-        BUG();
+        rc = -EOPNOTSUPP;
+        goto fail;
     }
-    config->gic_version = gic_version;
 
     if ( (rc = domain_vgic_init(d, config->nr_spis)) != 0 )
         goto fail;
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index c1c2579..df792f0 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -81,7 +81,7 @@ int domain_vgic_init(struct domain *d, unsigned int nr_spis)
 
     d->arch.vgic.nr_spis = nr_spis;
 
-    switch ( gic_hw_version() )
+    switch ( d->arch.vgic.version )
     {
 #ifdef HAS_GICV3
     case GIC_V3:
@@ -95,7 +95,7 @@ int domain_vgic_init(struct domain *d, unsigned int nr_spis)
         break;
     default:
         printk(XENLOG_G_ERR "d%d: Unknown vGIC version %u\n",
-               d->domain_id, gic_hw_version());
+               d->domain_id, d->arch.vgic.version);
         return -ENODEV;
     }
 
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index ecfdc10..cde1069 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -77,6 +77,8 @@ struct arch_domain
     } virt_timer_base;
 
     struct {
+        /* Version of the vGIC */
+        enum gic_version version;
         /* GIC HW version specific vGIC driver handler */
         const struct vgic_ops *handler;
         /*
-- 
2.1.4

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

* [v3 13/13] xen/arm: gic-v3: Add support of vGICv2 when available
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
                   ` (12 preceding siblings ...)
  2015-07-01 11:01 ` [v3 12/13] arm: Allow the user to specify the GIC version Julien Grall
@ 2015-07-01 11:01 ` Julien Grall
  2015-07-02 10:37 ` [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
  14 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, stefano.stabellini, ian.campbell

* Modify the GICv3 driver to recognize a such device. I wasn't able
  to find a register which tell if GICv2 is supported on GICv3. The only
  way to find it seems to check if the DT node provides GICC and GICV.

* Disable access to ICC_SRE_EL1 to guest using vGICv2

* The LR is slightly different for vGICv2. The interrupt is always
injected with group0.

* Add a comment explaining why Group1 is used for vGICv3.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    I haven't address the request from Ian to not use the R/M/W idiom.
    Most of the usage of the idiom are for EL2 registers (i.e registers
    used by the hypervisor). They can be modified at any time by Xen,
    and not specific to the running domain. We would have to progate the
    change to each domain if we don't use the R/W/M.

    ICC_SRE_EL2 falls under the same umbrella. It would be preferable to
    stay with the same model as today i.e:
        - *_EL1: straight save/restore
        - *_EL2: R/M/W to necessary field

    Changes in v3:
        - Add Ian's ack
        - Remove the isb which was not strictly necessary

    Changes in v2:
        - Use vgic_v2_hw_setup to notify that GICv3 supports GICv2
        - Revert changes in comment
---
 xen/arch/arm/gic-v3.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 51 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 337fbb9..2033951 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -247,7 +247,7 @@ static void gicv3_enable_sre(void)
     uint32_t val;
 
     val = READ_SYSREG32(ICC_SRE_EL2);
-    val |= GICC_SRE_EL2_SRE | GICC_SRE_EL2_ENEL1;
+    val |= GICC_SRE_EL2_SRE;
 
     WRITE_SYSREG32(val, ICC_SRE_EL2);
     isb();
@@ -375,6 +375,19 @@ static void gicv3_save_state(struct vcpu *v)
 
 static void gicv3_restore_state(const struct vcpu *v)
 {
+    uint32_t val;
+
+    val = READ_SYSREG32(ICC_SRE_EL2);
+    /*
+     * Don't give access to system registers when the guest is using
+     * GICv2
+     */
+    if ( v->domain->arch.vgic.version == GIC_V2 )
+        val &= ~GICC_SRE_EL2_ENEL1;
+    else
+        val |= GICC_SRE_EL2_ENEL1;
+    WRITE_SYSREG32(val, ICC_SRE_EL2);
+
     WRITE_SYSREG32(v->arch.gic.v3.sre_el1, ICC_SRE_EL1);
     WRITE_SYSREG32(v->arch.gic.v3.vmcr, ICH_VMCR_EL2);
     restore_aprn_regs(&v->arch.gic);
@@ -866,13 +879,20 @@ static void gicv3_disable_interface(void)
 static void gicv3_update_lr(int lr, const struct pending_irq *p,
                             unsigned int state)
 {
-    uint64_t grp = GICH_LR_GRP1;
     uint64_t val = 0;
 
     BUG_ON(lr >= gicv3_info.nr_lrs);
     BUG_ON(lr < 0);
 
-    val =  (((uint64_t)state & 0x3) << GICH_LR_STATE_SHIFT) | grp;
+    val =  (((uint64_t)state & 0x3) << GICH_LR_STATE_SHIFT);
+
+    /*
+     * When the guest is GICv3, all guest IRQs are Group 1, as Group0
+     * would result in a FIQ in the guest, which it wouldn't expect
+     */
+    if ( current->domain->arch.vgic.version == GIC_V3 )
+        val |= GICH_LR_GRP1;
+
     val |= ((uint64_t)p->priority & 0xff) << GICH_LR_PRIORITY_SHIFT;
     val |= ((uint64_t)p->irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT;
 
@@ -1119,6 +1139,33 @@ static int __init cmp_rdist(const void *a, const void *b)
     return ( l->base < r->base) ? -1 : 0;
 }
 
+/* If the GICv3 supports GICv2, initialize it */
+static void __init gicv3_init_v2(const struct dt_device_node *node,
+                                 paddr_t dbase)
+{
+    int res;
+    paddr_t cbase, vbase;
+
+    /*
+     * For GICv3 supporting GICv2, GICC and GICV base address will be
+     * provided.
+     */
+    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
+                                &cbase, NULL);
+    if ( res )
+        return;
+
+    res = dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
+                                &vbase, NULL);
+    if ( res )
+        return;
+
+    printk("GICv3 compatible with GICv2 cbase %#"PRIpaddr" vbase %#"PRIpaddr"\n",
+           cbase, vbase);
+
+    vgic_v2_setup_hw(dbase, cbase, vbase);
+}
+
 /* Set up the GIC */
 static int __init gicv3_init(void)
 {
@@ -1216,6 +1263,7 @@ static int __init gicv3_init(void)
 
     vgic_v3_setup_hw(dbase, gicv3.rdist_count, gicv3.rdist_regions,
                      gicv3.rdist_stride);
+    gicv3_init_v2(node, dbase);
 
     spin_lock_init(&gicv3.lock);
 
-- 
2.1.4

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

* Re: [v3] xen: new maintainer for the RTDS scheduler
  2015-07-01 11:01 ` [v3] xen: new maintainer for the RTDS scheduler Julien Grall
@ 2015-07-01 11:06   ` Julien Grall
  2015-07-01 11:07     ` George Dunlap
  0 siblings, 1 reply; 24+ messages in thread
From: Julien Grall @ 2015-07-01 11:06 UTC (permalink / raw)
  To: xen-devel
  Cc: George Dunlap, Dario Faggioli, stefano.stabellini, ian.campbell,
	Meng Xu

Hi,

Please ignore this patch.


I did a *.patch by mistake with this patch in the directory. Sorry.

Regards,

On 01/07/15 12:01, Julien Grall wrote:
> From: Dario Faggioli <dario.faggioli@citrix.com>
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> Reviewed-and-Acked-by: Meng Xu <mengxu@cis.upenn.edu>
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
>  MAINTAINERS | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6b1068e..e6616d2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -282,6 +282,11 @@ F:	tools/libxl/libxl_nonetbuffer.c
>  F:	tools/hotplug/Linux/remus-netbuf-setup
>  F:	tools/hotplug/Linux/block-drbd-probe
>  
> +RTDS SCHEDULER
> +M:	Dario Faggioli <dario.faggioli@citrix.com>
> +S:	Supported
> +F:	xen/common/sched_rt.c
> +
>  SCHEDULING
>  M:	George Dunlap <george.dunlap@eu.citrix.com>
>  S:	Supported
> 


-- 
Julien Grall

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

* Re: [v3] xen: new maintainer for the RTDS scheduler
  2015-07-01 11:06   ` Julien Grall
@ 2015-07-01 11:07     ` George Dunlap
  0 siblings, 0 replies; 24+ messages in thread
From: George Dunlap @ 2015-07-01 11:07 UTC (permalink / raw)
  To: Julien Grall, xen-devel
  Cc: Dario Faggioli, stefano.stabellini, ian.campbell, Meng Xu

On 07/01/2015 12:06 PM, Julien Grall wrote:
> Hi,
> 
> Please ignore this patch.
> 
> 
> I did a *.patch by mistake with this patch in the directory. Sorry.

I was wondering. :-)

 -George

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

* Re: [v3 12/13] arm: Allow the user to specify the GIC version
  2015-07-01 11:01 ` [v3 12/13] arm: Allow the user to specify the GIC version Julien Grall
@ 2015-07-01 13:52   ` Ian Jackson
  2015-07-01 14:37     ` Julien Grall
  0 siblings, 1 reply; 24+ messages in thread
From: Ian Jackson @ 2015-07-01 13:52 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, stefano.stabellini, ian.campbell, Wei Liu

Julien Grall writes ("[v3 12/13] arm: Allow the user to specify the GIC version"):
> A platform may have a GIC compatible with previous version of the
> device.
...
> +=item B<gic_version="vN">
> +
> +Version of the GIC emulated for the guest. Currently, the following
> +versions are supported:

This documentation should say what happens if this is not specified.

If the default behaviour is not simply `v3' (which I think it isn't)
then there should be a way to explicitly request it.  That is, there
should not be a behaviour which is only obtainable by not setting the
config option.  Instead, there should be a config option value to
request whatever that behaviour is.  (At the xl layer.)

AFIACT the default is "offer the guest the hardware's native version".
I think having called this XEN_DOMCTL_CONFIG_GIC_DEFAULT is rather
odd.  I think "native" would be a better description.

It might be better to deal with this default-filling-in by changing
the value in the domain config struct along with the other setdefault
type things in libxl_create.c.  If you do that, then saying
`libxl_domain_create_new', and then looking at the supplied config,
will give you a config specifying the GIC version explicitly, which I
think is more correct.

Ian.

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

* Re: [v3 12/13] arm: Allow the user to specify the GIC version
  2015-07-01 13:52   ` Ian Jackson
@ 2015-07-01 14:37     ` Julien Grall
  2015-07-01 14:50       ` Ian Campbell
  0 siblings, 1 reply; 24+ messages in thread
From: Julien Grall @ 2015-07-01 14:37 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, stefano.stabellini, ian.campbell, Wei Liu

On 01/07/15 14:52, Ian Jackson wrote:
> Julien Grall writes ("[v3 12/13] arm: Allow the user to specify the GIC version"):
>> A platform may have a GIC compatible with previous version of the
>> device.
> ...
>> +=item B<gic_version="vN">
>> +
>> +Version of the GIC emulated for the guest. Currently, the following
>> +versions are supported:
> 
> This documentation should say what happens if this is not specified.
> 
> If the default behaviour is not simply `v3' (which I think it isn't)
> then there should be a way to explicitly request it.  That is, there
> should not be a behaviour which is only obtainable by not setting the
> config option.  Instead, there should be a config option value to
> request whatever that behaviour is.  (At the xl layer.)

The enum for the gic_version contains 3 options: v2, v3 and default.
If the parameter is not specified, the default option is chosen.

I will document the default behavior.

> 
> AFIACT the default is "offer the guest the hardware's native version".

This is true when the domain is firstly created. But this will be
confusing if the user decide to migrate the guest to a platform where
the native GIC is different.

This is a valid use-case, though we don't support migration yet, and the
guest will expect to get the same virtual GIC (the IRQ controller driver
can't be changed).

> I think having called this XEN_DOMCTL_CONFIG_GIC_DEFAULT is rather
> odd.  I think "native" would be a better description.

I do agree that XEN_DOMCTL_CONFIG_GIC_DEFAULT is odd. This option should
be renamed to XEN_DOMCTL_CONFIG_GIC_NATIVE as this is the behavior
adopted by the hypervisor.

Although, I think we should not expose the option "native" to the user
but "default" (see why above). When the domain is created, the
hypervisor will update the field in to specify which version is
effectively used (see xen_arch_domain_create).

> It might be better to deal with this default-filling-in by changing
> the value in the domain config struct along with the other setdefault
> type things in libxl_create.c.  If you do that, then saying
> `libxl_domain_create_new', and then looking at the supplied config,
> will give you a config specifying the GIC version explicitly, which I
> think is more correct.

Good idea. We talked about it when the XEN_DOMCTL_CONFIG_* was firstly
introduced late in the release of Xen 4.5 and forgot to revisit it. I
will see to integrate it in this patch series.

-- 
Julien Grall

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

* Re: [v3 12/13] arm: Allow the user to specify the GIC version
  2015-07-01 14:37     ` Julien Grall
@ 2015-07-01 14:50       ` Ian Campbell
  2015-07-01 15:09         ` Julien Grall
  0 siblings, 1 reply; 24+ messages in thread
From: Ian Campbell @ 2015-07-01 14:50 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, Ian Jackson, Wei Liu, stefano.stabellini

On Wed, 2015-07-01 at 15:37 +0100, Julien Grall wrote:
> > AFIACT the default is "offer the guest the hardware's native version".
> 
> This is true when the domain is firstly created. But this will be
> confusing if the user decide to migrate the guest to a platform where
> the native GIC is different.
> 
> This is a valid use-case, though we don't support migration yet, and the
> guest will expect to get the same virtual GIC (the IRQ controller driver
> can't be changed).

I think an option "native" which is documented to be latching on the
host the guest is started on is absolutely fine, and avoids needing to
faff around turning default into native at the interface and/or parsing
stage.

Upon migration the config should have been updated to reflect whatever
decision Xen made when the guest was first started.

Ian.

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

* Re: [v3 12/13] arm: Allow the user to specify the GIC version
  2015-07-01 14:50       ` Ian Campbell
@ 2015-07-01 15:09         ` Julien Grall
  0 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-01 15:09 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Ian Jackson, Wei Liu, stefano.stabellini

On 01/07/15 15:50, Ian Campbell wrote:
> On Wed, 2015-07-01 at 15:37 +0100, Julien Grall wrote:
>>> AFIACT the default is "offer the guest the hardware's native version".
>>
>> This is true when the domain is firstly created. But this will be
>> confusing if the user decide to migrate the guest to a platform where
>> the native GIC is different.
>>
>> This is a valid use-case, though we don't support migration yet, and the
>> guest will expect to get the same virtual GIC (the IRQ controller driver
>> can't be changed).
> 
> I think an option "native" which is documented to be latching on the
> host the guest is started on is absolutely fine, and avoids needing to
> faff around turning default into native at the interface and/or parsing
> stage.

I wasn't planning to introduce another option. The question was about
the naming about the default option i.e either "native" or "default".

For both naming with still have to translate the xl option into an
hypervisor value (i.e XEN_DOMCTL_CONFIG_GIC_*).

Regards,

-- 
Julien Grall

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

* Re: [v3 00/13] xen/arm: Add support for GICv2 on GICv3
  2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
                   ` (13 preceding siblings ...)
  2015-07-01 11:01 ` [v3 13/13] xen/arm: gic-v3: Add support of vGICv2 when available Julien Grall
@ 2015-07-02 10:37 ` Julien Grall
  2015-07-03 10:33   ` Ian Campbell
  14 siblings, 1 reply; 24+ messages in thread
From: Julien Grall @ 2015-07-02 10:37 UTC (permalink / raw)
  To: xen-devel, ian.campbell
  Cc: stefano.stabellini, Chen Baozi, Zoltan Kiss,
	Shameerali Kolothum Thodi

On 01/07/15 12:00, Julien Grall wrote:
> Hi all,

Hi Ian,

> This patch series adds support for GICv2 on GICv3. This feature is available
> only when the GICv3 hardware is compatible with GICv2.
> 
> When it's the case, the same interface is provided in order to use a
> virtualize GICv2 (i.e GICC and GICV). This will allow us to re-use the
> same vGIC driver.
> 
> Currently GIC and vGIC drivers are tight because of the domain initialization
> splitted between GIC and vGIC. This patch series intends to remove this
> dependency in order to make the vGIC driver agnostic of the GIC driver.
> 
> It has been tested on the ARMv8 Foundation Model with GICv2 and GICv3 as
> well as changing the vGIC version emulated for the guest (only on GICv3 host).
> 
> A branch with all the patches can be found here:
>     git://xenbits.xen.org/people/julieng/xen-unstable.git branch gicv2-on-gicv3-v3
> 
> All the patches as been acked except #11 and #12.

I was wondering if you can apply patch #1-#10 as they are already acked.
It would avoid me to resend the whole series.

If you ack #11, you could even apply #11 and #13 (#12 is independent).

Regards,

-- 
Julien Grall

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

* Re: [v3 00/13] xen/arm: Add support for GICv2 on GICv3
  2015-07-02 10:37 ` [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
@ 2015-07-03 10:33   ` Ian Campbell
  2015-07-03 10:40     ` Julien Grall
  0 siblings, 1 reply; 24+ messages in thread
From: Ian Campbell @ 2015-07-03 10:33 UTC (permalink / raw)
  To: Julien Grall
  Cc: xen-devel, stefano.stabellini, Zoltan Kiss, Chen Baozi,
	Shameerali Kolothum Thodi

On Thu, 2015-07-02 at 11:37 +0100, Julien Grall wrote:
> On 01/07/15 12:00, Julien Grall wrote:
> > Hi all,
> 
> Hi Ian,
> 
> > This patch series adds support for GICv2 on GICv3. This feature is available
> > only when the GICv3 hardware is compatible with GICv2.
> > 
> > When it's the case, the same interface is provided in order to use a
> > virtualize GICv2 (i.e GICC and GICV). This will allow us to re-use the
> > same vGIC driver.
> > 
> > Currently GIC and vGIC drivers are tight because of the domain initialization
> > splitted between GIC and vGIC. This patch series intends to remove this
> > dependency in order to make the vGIC driver agnostic of the GIC driver.
> > 
> > It has been tested on the ARMv8 Foundation Model with GICv2 and GICv3 as
> > well as changing the vGIC version emulated for the guest (only on GICv3 host).
> > 
> > A branch with all the patches can be found here:
> >     git://xenbits.xen.org/people/julieng/xen-unstable.git branch gicv2-on-gicv3-v3
> > 
> > All the patches as been acked except #11 and #12.
> 
> I was wondering if you can apply patch #1-#10 as they are already acked.
> It would avoid me to resend the whole series.

Done.

There was one wrinkle, which is that the message-id's skipped #2, for
some reason. So I have applied
<1435748473-28812-NN-git-send-email-julien.grall@citrix.com>
for NN==2 (patch 1, due to 00 being NN=1, which is normal)  and
NN=4..13 inclusive, being patches 2..11, acking patch 11.

I don't know how you convinced git send-email to do this, but I only
mention it in such detail so you can check I've applied the right
things...

> If you ack #11, you could even apply #11 and #13 (#12 is independent).

I did not apply #13 since it seemed to depend on #12:
gic-v3.c: In function 'gicv3_restore_state':
gic-v3.c:385:30: error: 'struct <anonymous>' has no member named 'version'
     if ( v->domain->arch.vgic.version == GIC_V2 )
                              ^
gic-v3.c: In function 'gicv3_update_lr':
gic-v3.c:893:36: error: 'struct <anonymous>' has no member named 'version'
     if ( current->domain->arch.vgic.version == GIC_V3 )
                                    ^
make[4]: *** [gic-v3.o] Error 1




> 
> Regards,
> 

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

* Re: [v3 00/13] xen/arm: Add support for GICv2 on GICv3
  2015-07-03 10:33   ` Ian Campbell
@ 2015-07-03 10:40     ` Julien Grall
  0 siblings, 0 replies; 24+ messages in thread
From: Julien Grall @ 2015-07-03 10:40 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel, Chen Baozi, stefano.stabellini,
	Shameerali Kolothum Thodi, Zoltan Kiss

On 03/07/15 11:33, Ian Campbell wrote:
> On Thu, 2015-07-02 at 11:37 +0100, Julien Grall wrote:
>> On 01/07/15 12:00, Julien Grall wrote:
>>> Hi all,
>>
>> Hi Ian,
>>
>>> This patch series adds support for GICv2 on GICv3. This feature is available
>>> only when the GICv3 hardware is compatible with GICv2.
>>>
>>> When it's the case, the same interface is provided in order to use a
>>> virtualize GICv2 (i.e GICC and GICV). This will allow us to re-use the
>>> same vGIC driver.
>>>
>>> Currently GIC and vGIC drivers are tight because of the domain initialization
>>> splitted between GIC and vGIC. This patch series intends to remove this
>>> dependency in order to make the vGIC driver agnostic of the GIC driver.
>>>
>>> It has been tested on the ARMv8 Foundation Model with GICv2 and GICv3 as
>>> well as changing the vGIC version emulated for the guest (only on GICv3 host).
>>>
>>> A branch with all the patches can be found here:
>>>     git://xenbits.xen.org/people/julieng/xen-unstable.git branch gicv2-on-gicv3-v3
>>>
>>> All the patches as been acked except #11 and #12.
>>
>> I was wondering if you can apply patch #1-#10 as they are already acked.
>> It would avoid me to resend the whole series.
> 
> Done.
> 
> There was one wrinkle, which is that the message-id's skipped #2, for
> some reason. So I have applied
> <1435748473-28812-NN-git-send-email-julien.grall@citrix.com>
> for NN==2 (patch 1, due to 00 being NN=1, which is normal)  and
> NN=4..13 inclusive, being patches 2..11, acking patch 11.
> 
> I don't know how you convinced git send-email to do this, but I only
> mention it in such detail so you can check I've applied the right
> things...

I usually use git-sent-email *.patch and let him doing the job. Somehow
I got a spurious patch in the directory.

The patches are correctly applied thanks!

> 
>> If you ack #11, you could even apply #11 and #13 (#12 is independent).
> 
> I did not apply #13 since it seemed to depend on #12:
> gic-v3.c: In function 'gicv3_restore_state':
> gic-v3.c:385:30: error: 'struct <anonymous>' has no member named 'version'
>      if ( v->domain->arch.vgic.version == GIC_V2 )
>                               ^
> gic-v3.c: In function 'gicv3_update_lr':
> gic-v3.c:893:36: error: 'struct <anonymous>' has no member named 'version'
>      if ( current->domain->arch.vgic.version == GIC_V3 )
>                                     ^
> make[4]: *** [gic-v3.o] Error 1

Hmmm I forgot that I was using GIC_V* in this patch. I will address the
comment on #12 and resent the 2 patches.

Regards,

-- 
Julien Grall

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

end of thread, other threads:[~2015-07-03 10:41 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-01 11:00 [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
2015-07-01 11:01 ` [v3 01/13] xen/arm: Gate GICv3 change with HAS_GICV3 rather than CONFIG_ARM_64 Julien Grall
2015-07-01 11:01 ` [v3] xen: new maintainer for the RTDS scheduler Julien Grall
2015-07-01 11:06   ` Julien Grall
2015-07-01 11:07     ` George Dunlap
2015-07-01 11:01 ` [v3 02/13] xen/arm: gic: Rename make_dt_node into make_hwdom_dt_node Julien Grall
2015-07-01 11:01 ` [v3 03/13] xen/arm: vGIC: Check return of the domain_init callback Julien Grall
2015-07-01 11:01 ` [v3 04/13] xen/arm: gic-v3: Fix the distributor region to 64kB Julien Grall
2015-07-01 11:01 ` [v3 05/13] xen/arm: gic-v3: Use the domain redistributor information to make the DT node Julien Grall
2015-07-01 11:01 ` [v3 06/13] xen/arm: gic-v3: Rework the messages printed at initialization Julien Grall
2015-07-01 11:01 ` [v3 07/13] xen/arm: gic-{v2, hip04}: Remove redundant check in {gicv2, hip04gic}_init Julien Grall
2015-07-01 11:01 ` [v3 08/13] xen/arm: gic-{v2, hip04}: Use SZ_64K rather than our custom value Julien Grall
2015-07-01 11:01 ` [v3 09/13] xen/arm: gic: Allow the base address to be 0 Julien Grall
2015-07-01 11:01 ` [v3 10/13] xen/arm: gic-{v2, hip04}: Remove hbase from the global state Julien Grall
2015-07-01 11:01 ` [v3 11/13] xen/arm: Merge gicv_setup with vgic_domain_init Julien Grall
2015-07-01 11:01 ` [v3 12/13] arm: Allow the user to specify the GIC version Julien Grall
2015-07-01 13:52   ` Ian Jackson
2015-07-01 14:37     ` Julien Grall
2015-07-01 14:50       ` Ian Campbell
2015-07-01 15:09         ` Julien Grall
2015-07-01 11:01 ` [v3 13/13] xen/arm: gic-v3: Add support of vGICv2 when available Julien Grall
2015-07-02 10:37 ` [v3 00/13] xen/arm: Add support for GICv2 on GICv3 Julien Grall
2015-07-03 10:33   ` Ian Campbell
2015-07-03 10:40     ` Julien Grall

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.