* [PATCH v2 01/15] xen/arm: Gate GICv3 change with HAS_GICV3 rather than CONFIG_ARM_64...
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-30 12:41 ` Ian Campbell
2015-06-26 9:34 ` [PATCH v2 02/15] xen/arm: gic: Rename the callback make_dt_node in make_hwdom_dt_node Julien Grall
` (14 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 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 is HAS_GICV3.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
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.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 02/15] xen/arm: gic: Rename the callback make_dt_node in make_hwdom_dt_node
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
2015-06-26 9:34 ` [PATCH v2 01/15] xen/arm: Gate GICv3 change with HAS_GICV3 rather than CONFIG_ARM_64 Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-30 12:43 ` Ian Campbell
2015-06-26 9:34 ` [PATCH v2 03/15] xen/arm: vGIC: Export vgic_vN ops rather than add an indirection Julien Grall
` (13 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, stefano.stabellini, ian.campbell
Make 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>
---
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.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 03/15] xen/arm: vGIC: Export vgic_vN ops rather than add an indirection
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
2015-06-26 9:34 ` [PATCH v2 01/15] xen/arm: Gate GICv3 change with HAS_GICV3 rather than CONFIG_ARM_64 Julien Grall
2015-06-26 9:34 ` [PATCH v2 02/15] xen/arm: gic: Rename the callback make_dt_node in make_hwdom_dt_node Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-30 12:45 ` Ian Campbell
2015-06-26 9:34 ` [PATCH v2 04/15] xen/arm: vGIC: Check return of the domain_init callback Julien Grall
` (12 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, stefano.stabellini, ian.campbell
The function vgic_vN_init only calls register_vgic_ops. As it will never
contain anything else, domain initialization code should be in the
callback domain_init, remove them and directly use the VGIC ops in the
commmon vGIC code.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
Changes in v2:
- Gate the vgic v3 ops with HAS_GICV3
- Didn't retain Ian's ack for this change
---
xen/arch/arm/vgic-v2.c | 9 +--------
xen/arch/arm/vgic-v3.c | 9 +--------
xen/arch/arm/vgic.c | 11 ++---------
xen/include/asm-arm/vgic.h | 11 ++++++++---
4 files changed, 12 insertions(+), 28 deletions(-)
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index b5a8f29..9eecabc 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -539,20 +539,13 @@ static int vgic_v2_domain_init(struct domain *d)
return 0;
}
-static const struct vgic_ops vgic_v2_ops = {
+const struct vgic_ops vgic_v2_ops = {
.vcpu_init = vgic_v2_vcpu_init,
.domain_init = vgic_v2_domain_init,
.get_irq_priority = vgic_v2_get_irq_priority,
.get_target_vcpu = vgic_v2_get_target_vcpu,
};
-int vgic_v2_init(struct domain *d)
-{
- register_vgic_ops(d, &vgic_v2_ops);
-
- return 0;
-}
-
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 4af5a84..2b9501c 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1144,7 +1144,7 @@ static int vgic_v3_domain_init(struct domain *d)
return 0;
}
-static const struct vgic_ops v3_ops = {
+const struct vgic_ops vgic_v3_ops = {
.vcpu_init = vgic_v3_vcpu_init,
.domain_init = vgic_v3_domain_init,
.get_irq_priority = vgic_v3_get_irq_priority,
@@ -1152,13 +1152,6 @@ static const struct vgic_ops v3_ops = {
.emulate_sysreg = vgic_v3_emulate_sysreg,
};
-int vgic_v3_init(struct domain *d)
-{
- register_vgic_ops(d, &v3_ops);
-
- return 0;
-}
-
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index dfd959a..c9676e0 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -84,13 +84,11 @@ int domain_vgic_init(struct domain *d, unsigned int nr_spis)
{
#ifdef HAS_GICV3
case GIC_V3:
- if ( vgic_v3_init(d) )
- return -ENODEV;
+ d->arch.vgic.handler = &vgic_v3_ops;
break;
#endif
case GIC_V2:
- if ( vgic_v2_init(d) )
- return -ENODEV;
+ d->arch.vgic.handler = &vgic_v2_ops;
break;
default:
return -ENODEV;
@@ -128,11 +126,6 @@ int domain_vgic_init(struct domain *d, unsigned int nr_spis)
return 0;
}
-void register_vgic_ops(struct domain *d, const struct vgic_ops *ops)
-{
- d->arch.vgic.handler = ops;
-}
-
void domain_vgic_free(struct domain *d)
{
int i;
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 8d22532..4422a24 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -191,9 +191,14 @@ extern struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned int irq);
extern int vgic_emulate(struct cpu_user_regs *regs, union hsr hsr);
extern void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n);
extern void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n);
-extern void register_vgic_ops(struct domain *d, const struct vgic_ops *ops);
-int vgic_v2_init(struct domain *d);
-int vgic_v3_init(struct domain *d);
+
+#define DEFINE_VGIC_OPS(version) \
+ extern const struct vgic_ops vgic_v##version##_ops;
+DEFINE_VGIC_OPS(2)
+#ifdef HAS_GICV3
+DEFINE_VGIC_OPS(3)
+#endif
+#undef DEFINE_VGIC_OPS
extern int vcpu_vgic_free(struct vcpu *v);
extern int vgic_to_sgi(struct vcpu *v, register_t sgir,
--
2.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 03/15] xen/arm: vGIC: Export vgic_vN ops rather than add an indirection
2015-06-26 9:34 ` [PATCH v2 03/15] xen/arm: vGIC: Export vgic_vN ops rather than add an indirection Julien Grall
@ 2015-06-30 12:45 ` Ian Campbell
2015-06-30 13:24 ` Julien Grall
0 siblings, 1 reply; 37+ messages in thread
From: Ian Campbell @ 2015-06-30 12:45 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini
On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote:
> The function vgic_vN_init only calls register_vgic_ops. As it will never
> contain anything else, domain initialization code should be in the
> callback domain_init, remove them and directly use the VGIC ops in the
> commmon vGIC code.
Too many m's in common.
> +#define DEFINE_VGIC_OPS(version) \
> + extern const struct vgic_ops vgic_v##version##_ops;
> +DEFINE_VGIC_OPS(2)
> +#ifdef HAS_GICV3
> +DEFINE_VGIC_OPS(3)
> +#endif
> +#undef DEFINE_VGIC_OPS
I think the macro is a bit unnecessary, to externs would have been just
fine.
Either way:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 03/15] xen/arm: vGIC: Export vgic_vN ops rather than add an indirection
2015-06-30 12:45 ` Ian Campbell
@ 2015-06-30 13:24 ` Julien Grall
0 siblings, 0 replies; 37+ messages in thread
From: Julien Grall @ 2015-06-30 13:24 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, stefano.stabellini
Hi Ian,
On 30/06/15 13:45, Ian Campbell wrote:
> On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote:
>> The function vgic_vN_init only calls register_vgic_ops. As it will never
>> contain anything else, domain initialization code should be in the
>> callback domain_init, remove them and directly use the VGIC ops in the
>> commmon vGIC code.
>
> Too many m's in common.
>
>> +#define DEFINE_VGIC_OPS(version) \
>> + extern const struct vgic_ops vgic_v##version##_ops;
>> +DEFINE_VGIC_OPS(2)
>> +#ifdef HAS_GICV3
>> +DEFINE_VGIC_OPS(3)
>> +#endif
>> +#undef DEFINE_VGIC_OPS
>
> I think the macro is a bit unnecessary, to externs would have been just
> fine.
I will drop the macro.
>
> Either way:
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 04/15] xen/arm: vGIC: Check return of the domain_init callback
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (2 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 03/15] xen/arm: vGIC: Export vgic_vN ops rather than add an indirection Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-26 9:34 ` [PATCH v2 05/15] xen/arm: gic-v3: Fix the distributor region to 64kB Julien Grall
` (11 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 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 c9676e0..23ea58d 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;
@@ -112,7 +113,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.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 05/15] xen/arm: gic-v3: Fix the distributor region to 64kB
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (3 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 04/15] xen/arm: vGIC: Check return of the domain_init callback Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-26 9:34 ` [PATCH v2 06/15] xen/arm: gic-v3: Use the domain redistributor information to make the DT node Julien Grall
` (10 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 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 2b9501c..cfaedc1 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.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 06/15] xen/arm: gic-v3: Use the domain redistributor information to make the DT node
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (4 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 05/15] xen/arm: gic-v3: Fix the distributor region to 64kB Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-30 12:46 ` Ian Campbell
2015-06-26 9:34 ` [PATCH v2 07/15] xen/arm: gic-v3: Rework the print message at initialization Julien Grall
` (9 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, stefano.stabellini, ian.campbell
It's not necessary to get again from the hardware DT the redistributor
informations. 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>
---
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.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 07/15] xen/arm: gic-v3: Rework the print message at initialization
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (5 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 06/15] xen/arm: gic-v3: Use the domain redistributor information to make the DT node Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-30 12:49 ` Ian Campbell
2015-06-26 9:34 ` [PATCH v2 08/15] xen/arm: gic-{v2, hip04}: Remove redundant check in {gicv2, hip04gic}_init Julien Grall
` (8 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 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
differentiation from decimal
- Re-order informations 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 use like as it is).
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
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.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 07/15] xen/arm: gic-v3: Rework the print message at initialization
2015-06-26 9:34 ` [PATCH v2 07/15] xen/arm: gic-v3: Rework the print message at initialization Julien Grall
@ 2015-06-30 12:49 ` Ian Campbell
0 siblings, 0 replies; 37+ messages in thread
From: Ian Campbell @ 2015-06-30 12:49 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini
On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote:
Subject: "Rework the messages printed at initialization"
> - Print all the redistributor regions rather than only the first
> one...
> - Add # in the format to print 0x for hexadecimal. It's easier to
> differentiation from decimal
"differentiate"
> - Re-order informations printed
"information".
> - 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 use like as it is).
"used"
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
With those fixed:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 08/15] xen/arm: gic-{v2, hip04}: Remove redundant check in {gicv2, hip04gic}_init
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (6 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 07/15] xen/arm: gic-v3: Rework the print message at initialization Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-26 9:34 ` [PATCH v2 09/15] xen/arm: gic-{v2, hip04}: Use SZ_64K rather than our custom value Julien Grall
` (7 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 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.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 09/15] xen/arm: gic-{v2, hip04}: Use SZ_64K rather than our custom value
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (7 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 08/15] xen/arm: gic-{v2, hip04}: Remove redundant check in {gicv2, hip04gic}_init Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-26 9:34 ` [PATCH v2 10/15] xen/arm: gic: Allow the base address to be 0 Julien Grall
` (6 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 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.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 10/15] xen/arm: gic: Allow the base address to be 0
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (8 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 09/15] xen/arm: gic-{v2, hip04}: Use SZ_64K rather than our custom value Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-26 9:34 ` [PATCH v2 11/15] xen/arm: gic-{v2, hip04}: Remove hbase from the global state Julien Grall
` (5 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 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 acked
- 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.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 11/15] xen/arm: gic-{v2, hip04}: Remove hbase from the global state
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (9 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 10/15] xen/arm: gic: Allow the base address to be 0 Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-26 9:34 ` [PATCH v2 12/15] xen/arm: gic: Store the necessary HW information per vGIC Julien Grall
` (4 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 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 acked
- 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.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 12/15] xen/arm: gic: Store the necessary HW information per vGIC ...
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (10 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 11/15] xen/arm: gic-{v2, hip04}: Remove hbase from the global state Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-30 12:56 ` Ian Campbell
2015-06-26 9:34 ` [PATCH v2 13/15] xen/arm: Merge gicv_setup with vgic_domain_init Julien Grall
` (3 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, stefano.stabellini, ian.campbell
... in order to decouple the vGIC driver from the GIC driver.
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 to setup correctly
the vGIC. The GIC will have to call them to announce the support of this
specific version.
Also drop fields that become unecessary in each global state.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
Changes in v2:
- 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/arch/arm/gic-hip04.c | 37 +++++++++++++--------------
xen/arch/arm/gic-v2.c | 37 +++++++++++++--------------
xen/arch/arm/gic-v3.c | 31 +++++++++++------------
xen/arch/arm/vgic-v2.c | 2 ++
xen/arch/arm/vgic-v3.c | 2 ++
xen/arch/arm/vgic.c | 16 ++++++++++++
xen/include/asm-arm/gic_v3_defs.h | 7 ++++++
xen/include/asm-arm/vgic.h | 53 +++++++++++++++++++++++++++++++++++++++
8 files changed, 130 insertions(+), 55 deletions(-)
diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
index 984726a..259c6d6 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;
@@ -444,8 +441,8 @@ static int hip04gicv_setup(struct domain *d)
*/
if ( is_hardware_domain(d) )
{
- d->arch.vgic.dbase = gicv2.dbase;
- d->arch.vgic.cbase = gicv2.cbase;
+ d->arch.vgic.dbase = vgic_v2_hw.dbase;
+ d->arch.vgic.cbase = vgic_v2_hw.cbase;
}
else
{
@@ -461,16 +458,16 @@ static int hip04gicv_setup(struct domain *d)
* 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));
+ 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(gicv2.vbase + 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(gicv2.vbase + SZ_64K));
+ 2, paddr_to_pfn(vgic_v2_hw.vbase + SZ_64K));
return ret;
}
@@ -676,14 +673,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 +688,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 +705,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 +730,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);
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 6ec6602..4d94335 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;
@@ -434,8 +431,8 @@ static int gicv2v_setup(struct domain *d)
*/
if ( is_hardware_domain(d) )
{
- d->arch.vgic.dbase = gicv2.dbase;
- d->arch.vgic.cbase = gicv2.cbase;
+ d->arch.vgic.dbase = vgic_v2_hw.dbase;
+ d->arch.vgic.cbase = vgic_v2_hw.cbase;
}
else
{
@@ -451,16 +448,16 @@ static int gicv2v_setup(struct domain *d)
* 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));
+ 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(gicv2.vbase + 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(gicv2.vbase + SZ_64K));
+ 2, paddr_to_pfn(vgic_v2_hw.vbase + SZ_64K));
return ret;
}
@@ -662,14 +659,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 +674,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 +691,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 +716,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);
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 1c38c02..73ea700 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;
@@ -936,9 +929,9 @@ static int gicv_v3_init(struct domain *d)
{
unsigned int first_cpu = 0;
- d->arch.vgic.dbase = gicv3.dbase;
+ d->arch.vgic.dbase = vgic_v3_hw.dbase;
- d->arch.vgic.rdist_stride = gicv3.rdist_stride;
+ 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
@@ -949,9 +942,9 @@ static int gicv_v3_init(struct domain *d)
for ( i = 0; i < gicv3.rdist_count; i++ )
{
- paddr_t size = gicv3.rdist_regions[i].size;
+ paddr_t size = vgic_v3_hw.regions[i].size;
- d->arch.vgic.rdist_regions[i].base = gicv3.rdist_regions[i].base;
+ 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 */
@@ -959,7 +952,7 @@ static int gicv_v3_init(struct domain *d)
first_cpu += size / d->arch.vgic.rdist_stride;
}
- d->arch.vgic.nr_regions = gicv3.rdist_count;
+ d->arch.vgic.nr_regions = vgic_v3_hw.nr_rdist_regions;
}
else
{
@@ -1190,6 +1183,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 +1191,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 +1260,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 +1271,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);
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 9eecabc..029305a 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -31,6 +31,8 @@
#include <asm/gic.h>
#include <asm/vgic.h>
+struct vgic_v2_hw_config vgic_v2_hw;
+
static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
{
struct hsr_dabt dabt = info->dabt;
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index cfaedc1..5d295e4 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -50,6 +50,8 @@
*/
#define VGICD_CTLR_DEFAULT (GICD_CTLR_ARE_NS)
+struct vgic_v3_hw_config vgic_v3_hw;
+
static struct vcpu *vgic_v3_irouter_to_vcpu(struct domain *d, uint64_t irouter)
{
unsigned int vcpu_id;
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 23ea58d..08c5e45 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -85,13 +85,29 @@ int domain_vgic_init(struct domain *d, unsigned int nr_spis)
{
#ifdef HAS_GICV3
case GIC_V3:
+ if ( !vgic_v3_hw.enabled )
+ {
+ printk(XENLOG_G_ERR
+ "d%d: vGICv3 is not supported on this platform\n",
+ d->domain_id);
+ return -ENODEV;
+ }
d->arch.vgic.handler = &vgic_v3_ops;
break;
#endif
case GIC_V2:
+ if ( !vgic_v2_hw.enabled )
+ {
+ printk(XENLOG_G_ERR
+ "d%d: vGICv2 is not supported on this platform\n",
+ d->domain_id);
+ return -ENODEV;
+ }
d->arch.vgic.handler = &vgic_v2_ops;
break;
default:
+ printk(XENLOG_G_ERR "d%d: Unknown vGIC version %u\n",
+ d->domain_id, d->arch.vgic.version);
return -ENODEV;
}
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 4422a24..e107a42 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -20,6 +20,10 @@
#include <xen/bitops.h>
+#ifdef HAS_GICV3
+#include <asm/gic_v3_defs.h>
+#endif
+
struct pending_irq
{
/*
@@ -228,6 +232,55 @@ static inline int vgic_allocate_spi(struct domain *d)
extern void vgic_free_virq(struct domain *d, unsigned int virq);
+struct vgic_v2_hw_config
+{
+ bool_t enabled;
+ /* Distributor interface address */
+ paddr_t dbase;
+ /* CPU interface address */
+ paddr_t cbase;
+ /* Virtual CPU interface address */
+ paddr_t vbase;
+};
+
+extern struct vgic_v2_hw_config vgic_v2_hw;
+
+static inline 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;
+}
+
+#ifdef HAS_GICV3
+struct vgic_v3_hw_config
+{
+ 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 */
+};
+
+extern struct vgic_v3_hw_config vgic_v3_hw;
+
+static inline 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;
+}
+#endif
+
#endif /* __ASM_ARM_VGIC_H__ */
/*
--
2.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 12/15] xen/arm: gic: Store the necessary HW information per vGIC ...
2015-06-26 9:34 ` [PATCH v2 12/15] xen/arm: gic: Store the necessary HW information per vGIC Julien Grall
@ 2015-06-30 12:56 ` Ian Campbell
2015-06-30 13:38 ` Julien Grall
0 siblings, 1 reply; 37+ messages in thread
From: Ian Campbell @ 2015-06-30 12:56 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini
On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote:
> ... in order to decouple the vGIC driver from the GIC driver.
>
> 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 to setup correctly
> the vGIC. The GIC will have to call them to announce the support of this
> specific version.
>
"...to help the GIC correctly setup the vGIC"
"...to announce support for this specific version"
> Also drop fields that become unecessary in each global state.
"unnecessary"
> @@ -228,6 +232,55 @@ static inline int vgic_allocate_spi(struct domain *d)
>
> extern void vgic_free_virq(struct domain *d, unsigned int virq);
>
> +struct vgic_v2_hw_config
> +{
> + bool_t enabled;
> + /* Distributor interface address */
> + paddr_t dbase;
> + /* CPU interface address */
> + paddr_t cbase;
> + /* Virtual CPU interface address */
> + paddr_t vbase;
> +};
> +
> +extern struct vgic_v2_hw_config vgic_v2_hw;
My inclination is to call this either vgic_v2_hwdom(_config) (since it
is vgic config for the hw dom) or to call it gic_v2_hw_config (since it
contains config info of the physical gic which we happen to be going to
use for vgic).
I think given the expected usage the former makes more sense.
> +
> +static inline 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;
> +}
If you were to move this out of line into vgic-v2.c would that mean that
vgic_v2_hw_config etc could be static to that file?
(same two comments for the v3 bits)
Ian.
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v2 12/15] xen/arm: gic: Store the necessary HW information per vGIC ...
2015-06-30 12:56 ` Ian Campbell
@ 2015-06-30 13:38 ` Julien Grall
2015-06-30 14:00 ` Ian Campbell
0 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-30 13:38 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, stefano.stabellini
Hi Ian,
On 30/06/15 13:56, Ian Campbell wrote:
> On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote:
>> ... in order to decouple the vGIC driver from the GIC driver.
>>
>> 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 to setup correctly
>> the vGIC. The GIC will have to call them to announce the support of this
>> specific version.
>>
>
> "...to help the GIC correctly setup the vGIC"
>
> "...to announce support for this specific version"
>
>
>> Also drop fields that become unecessary in each global state.
>
> "unnecessary"
I will fix it in the next version.
>> @@ -228,6 +232,55 @@ static inline int vgic_allocate_spi(struct domain *d)
>>
>> extern void vgic_free_virq(struct domain *d, unsigned int virq);
>>
>> +struct vgic_v2_hw_config
>> +{
>> + bool_t enabled;
>> + /* Distributor interface address */
>> + paddr_t dbase;
>> + /* CPU interface address */
>> + paddr_t cbase;
>> + /* Virtual CPU interface address */
>> + paddr_t vbase;
>> +};
>> +
>> +extern struct vgic_v2_hw_config vgic_v2_hw;
>
> My inclination is to call this either vgic_v2_hwdom(_config) (since it
> is vgic config for the hw dom) or to call it gic_v2_hw_config (since it
> contains config info of the physical gic which we happen to be going to
> use for vgic).
>
> I think given the expected usage the former makes more sense.
I think that the 2 names don't work for the usage of the structure:
- vgic_v2_hwdom: vbase is used to map the guest CPU interface into the
virtual CPU interface
- gic_v2_hw_config: it gives the impression to be physical GIC specific
rather the virtual GIC.
I would prefer to stick in vgic_v2_hw_config which show that we use it
for the virtual GIC.
>> +
>> +static inline 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;
>> +}
>
> If you were to move this out of line into vgic-v2.c would that mean that
> vgic_v2_hw_config etc could be static to that file?
No, we have to access the field enabled in domain_vgic_init to verify
the GIC is supporting the version of the vGIC.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v2 12/15] xen/arm: gic: Store the necessary HW information per vGIC ...
2015-06-30 13:38 ` Julien Grall
@ 2015-06-30 14:00 ` Ian Campbell
2015-06-30 14:11 ` Julien Grall
0 siblings, 1 reply; 37+ messages in thread
From: Ian Campbell @ 2015-06-30 14:00 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini
On Tue, 2015-06-30 at 14:38 +0100, Julien Grall wrote:
> Hi Ian,
>
> On 30/06/15 13:56, Ian Campbell wrote:
> > On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote:
> >> ... in order to decouple the vGIC driver from the GIC driver.
> >>
> >> 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 to setup correctly
> >> the vGIC. The GIC will have to call them to announce the support of this
> >> specific version.
> >>
> >
> > "...to help the GIC correctly setup the vGIC"
> >
> > "...to announce support for this specific version"
> >
> >
> >> Also drop fields that become unecessary in each global state.
> >
> > "unnecessary"
>
> I will fix it in the next version.
>
> >> @@ -228,6 +232,55 @@ static inline int vgic_allocate_spi(struct domain *d)
> >>
> >> extern void vgic_free_virq(struct domain *d, unsigned int virq);
> >>
> >> +struct vgic_v2_hw_config
> >> +{
> >> + bool_t enabled;
> >> + /* Distributor interface address */
> >> + paddr_t dbase;
> >> + /* CPU interface address */
> >> + paddr_t cbase;
> >> + /* Virtual CPU interface address */
> >> + paddr_t vbase;
> >> +};
> >> +
> >> +extern struct vgic_v2_hw_config vgic_v2_hw;
> >
> > My inclination is to call this either vgic_v2_hwdom(_config) (since it
> > is vgic config for the hw dom) or to call it gic_v2_hw_config (since it
> > contains config info of the physical gic which we happen to be going to
> > use for vgic).
> >
> > I think given the expected usage the former makes more sense.
>
> I think that the 2 names don't work for the usage of the structure:
> - vgic_v2_hwdom: vbase is used to map the guest CPU interface into the
> virtual CPU interface
> - gic_v2_hw_config: it gives the impression to be physical GIC specific
> rather the virtual GIC.
>
> I would prefer to stick in vgic_v2_hw_config which show that we use it
> for the virtual GIC.
OK.
> >> +
> >> +static inline 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;
> >> +}
> >
> > If you were to move this out of line into vgic-v2.c would that mean that
> > vgic_v2_hw_config etc could be static to that file?
>
> No, we have to access the field enabled in domain_vgic_init to verify
> the GIC is supporting the version of the vGIC.
That's a shame.
vgic_vN_init would have been the ideal place to test for this, which
would have kept everything in one place, but you've just nuked that and
I suppose don't want it coming back.
It was the inclusion of <asm/gic_v3_defs.h> into vgic.h which took me
down the line of thinking of wanting to make this self contained, and in
particular exposing struct rdist_region.
Ian.
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v2 12/15] xen/arm: gic: Store the necessary HW information per vGIC ...
2015-06-30 14:00 ` Ian Campbell
@ 2015-06-30 14:11 ` Julien Grall
2015-06-30 14:18 ` Ian Campbell
0 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-30 14:11 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, stefano.stabellini
On 30/06/15 15:00, Ian Campbell wrote:
>>>> +static inline 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;
>>>> +}
>>>
>>> If you were to move this out of line into vgic-v2.c would that mean that
>>> vgic_v2_hw_config etc could be static to that file?
>>
>> No, we have to access the field enabled in domain_vgic_init to verify
>> the GIC is supporting the version of the vGIC.
>
> That's a shame.
>
> vgic_vN_init would have been the ideal place to test for this, which
> would have kept everything in one place, but you've just nuked that and
> I suppose don't want it coming back.
I dropped vgic_vN_init because it was only setting the ops. I don't mind
to remove the patch #3 and move all the structure in vgic-v*.c.
It will be a lot cleaner.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v2 12/15] xen/arm: gic: Store the necessary HW information per vGIC ...
2015-06-30 14:11 ` Julien Grall
@ 2015-06-30 14:18 ` Ian Campbell
0 siblings, 0 replies; 37+ messages in thread
From: Ian Campbell @ 2015-06-30 14:18 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini
On Tue, 2015-06-30 at 15:11 +0100, Julien Grall wrote:
> On 30/06/15 15:00, Ian Campbell wrote:
> >>>> +static inline 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;
> >>>> +}
> >>>
> >>> If you were to move this out of line into vgic-v2.c would that mean that
> >>> vgic_v2_hw_config etc could be static to that file?
> >>
> >> No, we have to access the field enabled in domain_vgic_init to verify
> >> the GIC is supporting the version of the vGIC.
> >
> > That's a shame.
> >
> > vgic_vN_init would have been the ideal place to test for this, which
> > would have kept everything in one place, but you've just nuked that and
> > I suppose don't want it coming back.
>
> I dropped vgic_vN_init because it was only setting the ops. I don't mind
> to remove the patch #3 and move all the structure in vgic-v*.c.
> It will be a lot cleaner.
That sounds good, thanks.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 13/15] xen/arm: Merge gicv_setup with vgic_domain_init
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (11 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 12/15] xen/arm: gic: Store the necessary HW information per vGIC Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-30 12:59 ` Ian Campbell
2015-06-26 9:34 ` [PATCH v2 14/15] arm: Allow the user to specify the GIC version Julien Grall
` (2 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, Zoltan Kiss, 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.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
Cc: Zoltan Kiss <zoltan.kiss@huawei.com>
---
Changes in v2:
- 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 | 42 ----------------------------------
xen/arch/arm/gic-v2.c | 42 ----------------------------------
xen/arch/arm/gic-v3.c | 58 -----------------------------------------------
xen/arch/arm/gic.c | 10 ++++----
xen/arch/arm/vgic-v2.c | 43 ++++++++++++++++++++++++++++++++---
xen/arch/arm/vgic-v3.c | 54 +++++++++++++++++++++++++++++++++++++++++--
xen/include/asm-arm/gic.h | 4 ++--
8 files changed, 99 insertions(+), 157 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 259c6d6..c5ed545 100644
--- a/xen/arch/arm/gic-hip04.c
+++ b/xen/arch/arm/gic-hip04.c
@@ -431,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 = 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));
-
- return ret;
-}
-
static void hip04gic_read_lr(int lr, struct gic_lr *lr_reg)
{
uint32_t lrv;
@@ -752,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 4d94335..596126d 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -421,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 = 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));
-
- return ret;
-}
-
static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
{
uint32_t lrv;
@@ -738,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 73ea700..337fbb9 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -917,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 = 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 < gicv3.rdist_count; 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;
- }
-
- return 0;
-}
-
static void gicv3_hcr_status(uint32_t flag, bool_t status)
{
uint32_t hcr;
@@ -1293,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..3c09c3e 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -67,6 +67,11 @@ unsigned int gic_number_lines(void)
return gic_hw_ops->info->nr_lines;
}
+const struct gic_info *gic_info(void)
+{
+ return gic_hw_ops->info;
+}
+
void gic_save_state(struct vcpu *v)
{
ASSERT(!local_irq_is_enabled());
@@ -661,11 +666,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 029305a..0cae6f4 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -24,11 +24,12 @@
#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>
struct vgic_v2_hw_config vgic_v2_hw;
@@ -527,14 +528,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);
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 5d295e4..ab6a9ac 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 */
@@ -1121,13 +1120,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);
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 71f4813..49d2b52 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -300,6 +300,8 @@ struct gic_info {
const struct dt_device_node *node;
};
+const struct gic_info *gic_info(void);
+
struct gic_hw_operations {
/* Hold GIC HW information */
const struct gic_info *info;
@@ -311,8 +313,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;
--
2.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 13/15] xen/arm: Merge gicv_setup with vgic_domain_init
2015-06-26 9:34 ` [PATCH v2 13/15] xen/arm: Merge gicv_setup with vgic_domain_init Julien Grall
@ 2015-06-30 12:59 ` Ian Campbell
2015-06-30 13:51 ` Julien Grall
0 siblings, 1 reply; 37+ messages in thread
From: Ian Campbell @ 2015-06-30 12:59 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini, Zoltan Kiss
On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote:
> 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.
>
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> Cc: Zoltan Kiss <zoltan.kiss@huawei.com>
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 341b6df..3c09c3e 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -67,6 +67,11 @@ unsigned int gic_number_lines(void)
> return gic_hw_ops->info->nr_lines;
> }
>
> +const struct gic_info *gic_info(void)
> +{
> + return gic_hw_ops->info;
This doesn't seem to be used here. Is it a remnant of the previous
approach?
> + if ( !platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) )
Perhaps the stride should be in the info struct?
Ian.
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v2 13/15] xen/arm: Merge gicv_setup with vgic_domain_init
2015-06-30 12:59 ` Ian Campbell
@ 2015-06-30 13:51 ` Julien Grall
2015-06-30 13:56 ` Ian Campbell
0 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-30 13:51 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, stefano.stabellini, Zoltan Kiss
Hi Ian,
On 30/06/15 13:59, Ian Campbell wrote:
> On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote:
>> 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.
>>
>> Signed-off-by: Julien Grall <julien.grall@citrix.com>
>> Cc: Zoltan Kiss <zoltan.kiss@huawei.com>
>> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
>> index 341b6df..3c09c3e 100644
>> --- a/xen/arch/arm/gic.c
>> +++ b/xen/arch/arm/gic.c
>> @@ -67,6 +67,11 @@ unsigned int gic_number_lines(void)
>> return gic_hw_ops->info->nr_lines;
>> }
>>
>> +const struct gic_info *gic_info(void)
>> +{
>> + return gic_hw_ops->info;
>
> This doesn't seem to be used here. Is it a remnant of the previous
> approach?
Right, I forgot to remove it.
>> + if ( !platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) )
>
> Perhaps the stride should be in the info struct?
This is not related to any GICvN configuration but an hardware bug on
some platform (currently only xgene). And we are not immune to a similar
bug on GICv3.
So I would prefer to keep the check in vgic_domain_init which is more
self contained than spread it in different place.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v2 13/15] xen/arm: Merge gicv_setup with vgic_domain_init
2015-06-30 13:51 ` Julien Grall
@ 2015-06-30 13:56 ` Ian Campbell
0 siblings, 0 replies; 37+ messages in thread
From: Ian Campbell @ 2015-06-30 13:56 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini, Zoltan Kiss
On Tue, 2015-06-30 at 14:51 +0100, Julien Grall wrote:
> >> + if ( !platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) )
> >
> > Perhaps the stride should be in the info struct?
>
> This is not related to any GICvN configuration but an hardware bug on
> some platform (currently only xgene). And we are not immune to a similar
> bug on GICv3.
>
> So I would prefer to keep the check in vgic_domain_init which is more
> self contained than spread it in different place.
OK.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 14/15] arm: Allow the user to specify the GIC version
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (12 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 13/15] xen/arm: Merge gicv_setup with vgic_domain_init Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-30 13:06 ` Ian Campbell
2015-06-26 9:34 ` [PATCH v2 15/15] xen/arm: gic-v3: Add support of vGICv2 when available Julien Grall
2015-06-30 8:05 ` [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Chen Baozi
15 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 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 on 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 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 | 27 ++++++++++++++++++++++++++
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 | 2 +-
xen/include/asm-arm/domain.h | 2 ++
8 files changed, 100 insertions(+), 20 deletions(-)
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index a3e0e2e..663eb2d 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1688,6 +1688,33 @@ 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 GICv2 compatibility is not supported.
+
+=back
+
+Although, all the versions may not be supported on the host.
+
+=back
+
=head1 SEE ALSO
=over 4
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 0a7913b..68bc954 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 gic_version field.
+ */
+#define LIBXL_HAVE_BUILDINFO_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..5b637e9 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_2:
+ xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_V2;
+ break;
+ case LIBXL_GIC_VERSION_3:
+ xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
+ break;
+ default:
+ LOG(ERROR, "Unkwnon 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 23f27d4..30f69ea 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -366,6 +366,12 @@ libxl_vnode_info = Struct("vnode_info", [
("vcpus", libxl_bitmap), # vcpus in this node
])
+libxl_gic_version = Enumeration("gic_version", [
+ (0, "DEFAULT"),
+ (1, "2"),
+ (2, "3")
+ ], init_val = "LIBXL_GIC_VERSION_DEFAULT")
+
libxl_domain_build_info = Struct("domain_build_info",[
("max_vcpus", integer),
("avail_vcpus", libxl_bitmap),
@@ -477,6 +483,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..dcf1963 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)) {
+ if (!strcmp(buf, "v2"))
+ b_info->arch_arm.gic_version = LIBXL_GIC_VERSION_2;
+ else if (!strcmp(buf, "v3"))
+ b_info->arch_arm.gic_version = LIBXL_GIC_VERSION_3;
+ else {
+ fprintf(stderr,
+ "Uknown gic_version \"%s\" specified\n", buf);
+ exit(1);
+ }
+ }
+
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 08c5e45..f237914 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:
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.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 14/15] arm: Allow the user to specify the GIC version
2015-06-26 9:34 ` [PATCH v2 14/15] arm: Allow the user to specify the GIC version Julien Grall
@ 2015-06-30 13:06 ` Ian Campbell
2015-06-30 14:23 ` Julien Grall
0 siblings, 1 reply; 37+ messages in thread
From: Ian Campbell @ 2015-06-30 13:06 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini, Ian Jackson, Wei Liu
On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote:
> 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 on GICv2 and GICv3 are supported).
^ly?
>
> 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 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 | 27 ++++++++++++++++++++++++++
> 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 | 2 +-
> xen/include/asm-arm/domain.h | 2 ++
> 8 files changed, 100 insertions(+), 20 deletions(-)
>
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index a3e0e2e..663eb2d 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1688,6 +1688,33 @@ 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 GICv2 compatibility is not supported.
"Note that the emulated GIC does not support the GICv2 compatibility
mode". (To avoid confusion with such compatibility used to provide the
other option to the guest)
> +
> +=back
> +
> +Although, all the versions may not be supported on the host.
"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 0a7913b..68bc954 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 gic_version field.
> + */
> +#define LIBXL_HAVE_BUILDINFO_GIC_VERSION 1
Mention ARM in the comment and/or name?
> +
> +/*
> * 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..5b637e9 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_2:
> + xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_V2;
> + break;
> + case LIBXL_GIC_VERSION_3:
> + xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
> + break;
> + default:
> + LOG(ERROR, "Unkwnon GIC version %s\n",
"Unknown"
>
> +libxl_gic_version = Enumeration("gic_version", [
> + (0, "DEFAULT"),
> + (1, "2"),
> + (2, "3")
If you call these "V2" and "V3" then you can use
libxl_gic_version_from_string to make the parsing much easier. If you
subsequently add e.g. "v2m" then the parsing code in xl will also just
work,
Better to line up the values with the gic version if possible too. Enums
don't need to be contiguous. Or maybe use 0x20, 0x30 etc for the value,
leaving space for gicv2m in 0x21 etc.
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index c858068..dcf1963 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)) {
> + if (!strcmp(buf, "v2"))
> + b_info->arch_arm.gic_version = LIBXL_GIC_VERSION_2;
> + else if (!strcmp(buf, "v3"))
> + b_info->arch_arm.gic_version = LIBXL_GIC_VERSION_3;
> + else {
> + fprintf(stderr,
> + "Uknown gic_version \"%s\" specified\n", buf);
"Unknown".
Ian.
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v2 14/15] arm: Allow the user to specify the GIC version
2015-06-30 13:06 ` Ian Campbell
@ 2015-06-30 14:23 ` Julien Grall
0 siblings, 0 replies; 37+ messages in thread
From: Julien Grall @ 2015-06-30 14:23 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, stefano.stabellini, Ian Jackson, Wei Liu
Hi Ian,
On 30/06/15 14:06, Ian Campbell wrote:
> On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote:
>> 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 on GICv2 and GICv3 are supported).
> ^ly?
Yes. I will fixed on the next version.
[..]
>> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
>> index a3e0e2e..663eb2d 100644
>> --- a/docs/man/xl.cfg.pod.5
>> +++ b/docs/man/xl.cfg.pod.5
>> @@ -1688,6 +1688,33 @@ 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 GICv2 compatibility is not supported.
>
> "Note that the emulated GIC does not support the GICv2 compatibility
> mode". (To avoid confusion with such compatibility used to provide the
> other option to the guest)
Good idea. I will use it.
>
>> +
>> +=back
>> +
>> +Although, all the versions may not be supported on the host.
>
> "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 0a7913b..68bc954 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 gic_version field.
>> + */
>> +#define LIBXL_HAVE_BUILDINFO_GIC_VERSION 1
>
> Mention ARM in the comment and/or name?
Yes, I forgot to update it when I move the field within the "arm" structure.
[..]
>>
>> +libxl_gic_version = Enumeration("gic_version", [
>> + (0, "DEFAULT"),
>> + (1, "2"),
>> + (2, "3")
>
> If you call these "V2" and "V3" then you can use
> libxl_gic_version_from_string to make the parsing much easier. If you
> subsequently add e.g. "v2m" then the parsing code in xl will also just
> work,
Will do.
> Better to line up the values with the gic version if possible too. Enums
> don't need to be contiguous. Or maybe use 0x20, 0x30 etc for the value,
> leaving space for gicv2m in 0x21 etc.
I will use 0x20, 0x30...
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 15/15] xen/arm: gic-v3: Add support of vGICv2 when available
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (13 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 14/15] arm: Allow the user to specify the GIC version Julien Grall
@ 2015-06-26 9:34 ` Julien Grall
2015-06-30 13:16 ` Ian Campbell
2015-06-30 8:05 ` [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Chen Baozi
15 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-26 9:34 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>
---
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 v2:
- Use vgic_v2_hw_setup to notify that GICv3 supports GICv2
- Revert changes in comment
---
xen/arch/arm/gic-v3.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 52 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 337fbb9..876a56b 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,20 @@ 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);
+ isb();
+
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 +880,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 +1140,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 +1264,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.4.3
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 15/15] xen/arm: gic-v3: Add support of vGICv2 when available
2015-06-26 9:34 ` [PATCH v2 15/15] xen/arm: gic-v3: Add support of vGICv2 when available Julien Grall
@ 2015-06-30 13:16 ` Ian Campbell
2015-06-30 17:29 ` Julien Grall
0 siblings, 1 reply; 37+ messages in thread
From: Ian Campbell @ 2015-06-30 13:16 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini
On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote:
> * 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>
>
> ---
> 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
I suppose I can live with this.
>
> Changes in v2:
> - Use vgic_v2_hw_setup to notify that GICv3 supports GICv2
> - Revert changes in comment
> ---
> xen/arch/arm/gic-v3.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 337fbb9..876a56b 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,20 @@ 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
I suppose we can assume that v4, v5 etc will want to expose these (i.e.
the condition needn't be inverted to be don't give when not using v3)
> + */
> + 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);
> + isb();
Is the isb strictly needed? I suppose we are already using rather too
many, perhaps a more complete audit is in order.
> +
> 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 +880,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
Unless the guest actually asked for a FIQ (not sure how that works).
Anyway, I don't think you need to support that case here...
> + */
> + 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 +1140,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.
I wonder how one specifies without GICC, given that GICH comes after
it... Oh well!
Despite all the comments I don't think there are any blockers:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v2 15/15] xen/arm: gic-v3: Add support of vGICv2 when available
2015-06-30 13:16 ` Ian Campbell
@ 2015-06-30 17:29 ` Julien Grall
2015-07-01 8:12 ` Ian Campbell
0 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2015-06-30 17:29 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, stefano.stabellini
Hi Ian,
On 30/06/15 14:16, Ian Campbell wrote:
> On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote:
>> * 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>
>>
>> ---
>> 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
>
> I suppose I can live with this.
>
>>
>> Changes in v2:
>> - Use vgic_v2_hw_setup to notify that GICv3 supports GICv2
>> - Revert changes in comment
>> ---
>> xen/arch/arm/gic-v3.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 52 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index 337fbb9..876a56b 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,20 @@ 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
>
> I suppose we can assume that v4, v5 etc will want to expose these (i.e.
> the condition needn't be inverted to be don't give when not using v3)
GICv3 and onwards are always using System Registers (i.e SRE = 1). On
platform which don't require to be backward compatible with GICv2, this
bit may be RAO/WI.
This is true for v4, but I don't know for v5 as I don't have the spec.
It may even be a completely different driver.
>
>> + */
>> + 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);
>> + isb();
>
> Is the isb strictly needed? I suppose we are already using rather too
> many, perhaps a more complete audit is in order.
AFAICT no, the ENEL1 doesn't gate any access to EL1 systems register in EL2.
There is an isb in the caller (gic_restore_state) but I find it
confusing because we rely on the caller doing the right thing for us.
I'm thinking to push the isb within the callee for more clarify.
It can be part of a bigger audit.
>
>> +
>> 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 +880,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
>
> Unless the guest actually asked for a FIQ (not sure how that works).
> Anyway, I don't think you need to support that case here...
Well, I don't even think that FIQ has ever working with guest and Xen.
AFAICT, supporting FIQ would require to have 2 groups (GRP0 and GRP1).
Although both our vGICv2 and vGICv3 driver only support a single group
(GRP0 for GICv2 and GRP1 for GICv3).
So I would expect to see more work than this small change.
>
>> + */
>> + 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 +1140,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.
>
>
> I wonder how one specifies without GICC, given that GICH comes after
> it... Oh well!
GICv3 doesn't use the GICH to control the GIC hypervisor interface but
the system register.
> Despite all the comments I don't think there are any blockers:
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Thank you.
--
Julien Grall
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v2 15/15] xen/arm: gic-v3: Add support of vGICv2 when available
2015-06-30 17:29 ` Julien Grall
@ 2015-07-01 8:12 ` Ian Campbell
2015-07-01 10:21 ` Julien Grall
0 siblings, 1 reply; 37+ messages in thread
From: Ian Campbell @ 2015-07-01 8:12 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini
On Tue, 2015-06-30 at 18:29 +0100, Julien Grall wrote:
> >
> >> + */
> >> + 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);
> >> + isb();
> >
> > Is the isb strictly needed? I suppose we are already using rather too
> > many, perhaps a more complete audit is in order.
>
> AFAICT no, the ENEL1 doesn't gate any access to EL1 systems register in EL2.
>
> There is an isb in the caller (gic_restore_state) but I find it
> confusing because we rely on the caller doing the right thing for us.
> I'm thinking to push the isb within the callee for more clarify.
isb's aren't free though.
> It can be part of a bigger audit.
Indeed, this isn't one for now/.
Really there should probably be a single isb at the end of ctxt
save/restore and very little otherwise except where there is an absolute
need for some sort of ordering between steps.
> >> + /*
> >> + * 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
> >
> > Unless the guest actually asked for a FIQ (not sure how that works).
> > Anyway, I don't think you need to support that case here...
>
> Well, I don't even think that FIQ has ever working with guest and Xen.
Yes, you don't need to anything, I was just idly wondering, sorry for
not being clear about that.
> So I would expect to see more work than this small change.
Absolutely,
> > I wonder how one specifies without GICC, given that GICH comes after
> > it... Oh well!
>
> GICv3 doesn't use the GICH to control the GIC hypervisor interface but
> the system register.
Oh of course, I'd forgotten.
> > Despite all the comments I don't think there are any blockers:
> >
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> Thank you.
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 15/15] xen/arm: gic-v3: Add support of vGICv2 when available
2015-07-01 8:12 ` Ian Campbell
@ 2015-07-01 10:21 ` Julien Grall
0 siblings, 0 replies; 37+ messages in thread
From: Julien Grall @ 2015-07-01 10:21 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, stefano.stabellini
Hi Ian,
On 01/07/15 09:12, Ian Campbell wrote:
> On Tue, 2015-06-30 at 18:29 +0100, Julien Grall wrote:
>>>
>>>> + */
>>>> + 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);
>>>> + isb();
>>>
>>> Is the isb strictly needed? I suppose we are already using rather too
>>> many, perhaps a more complete audit is in order.
>>
>> AFAICT no, the ENEL1 doesn't gate any access to EL1 systems register in EL2.
>>
>> There is an isb in the caller (gic_restore_state) but I find it
>> confusing because we rely on the caller doing the right thing for us.
>> I'm thinking to push the isb within the callee for more clarify.
>
> isb's aren't free though.
>
>> It can be part of a bigger audit.
>
> Indeed, this isn't one for now/.
>
> Really there should probably be a single isb at the end of ctxt
> save/restore and very little otherwise except where there is an absolute
> need for some sort of ordering between steps.
I will drop this one in the next version.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3
2015-06-26 9:34 [PATCH v2 00/15] xen/arm: Add support for GICv2 on GICv3 Julien Grall
` (14 preceding siblings ...)
2015-06-26 9:34 ` [PATCH v2 15/15] xen/arm: gic-v3: Add support of vGICv2 when available Julien Grall
@ 2015-06-30 8:05 ` Chen Baozi
15 siblings, 0 replies; 37+ messages in thread
From: Chen Baozi @ 2015-06-30 8:05 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini, ian.campbell
Hi Julien,
On Fri, Jun 26, 2015 at 10:34:14AM +0100, Julien Grall wrote:
> 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). That will allow us to re-use same
> vGIC drivers.
>
> 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 for 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-v2
>
> For all the changes see in each patch.
I have rebased the 8+ CPU patch series onto this version. It looks fine for me.
You may add "Tested-by: Chen Baozi <baozich@gmail.com>".
Cheers,
Baozi.
^ permalink raw reply [flat|nested] 37+ messages in thread