* [PATCH v8a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h
2014-07-03 8:34 [PATCH v8a 0/6] GIC and VGIC code refactoring vijay.kilari
@ 2014-07-03 8:34 ` vijay.kilari
2014-07-03 9:12 ` Jan Beulich
2014-07-03 12:49 ` Julien Grall
2014-07-03 8:34 ` [PATCH v8a 2/6] xen/arm: move pending_irq structure to vgic header file vijay.kilari
` (4 subsequent siblings)
5 siblings, 2 replies; 19+ messages in thread
From: vijay.kilari @ 2014-07-03 8:34 UTC (permalink / raw)
To: Ian.Campbell, julien.grall, stefano.stabellini,
stefano.stabellini, tim, xen-devel
Cc: keir, vijay.kilari, Prasun.Kapoor, Vijaya Kumar K, george.dunlap,
jbeulich
From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
is_vcpu_running function in vgic driver is generic. So move
this to sched.h and rename it as is_vcpu_online
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
CC: jbeulich@suse.com
CC: keir@xen.org
CC: george.dunlap@citrix.com
---
v8a:is_vcpu_online() is modified to take only vcpu as
parameter and validity checks on vcpu is done in
caller
v7a:Changed vcpuid parameter from int to unsigned int
Removed check for test_bit. Return !test_bit()
---
xen/arch/arm/vgic.c | 22 ++++------------------
xen/include/xen/sched.h | 5 +++++
2 files changed, 9 insertions(+), 18 deletions(-)
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 96bd7c1..4abc682 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -405,22 +405,6 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
}
}
-static inline int is_vcpu_running(struct domain *d, int vcpuid)
-{
- struct vcpu *v;
-
- if ( vcpuid >= d->max_vcpus )
- return 0;
-
- v = d->vcpu[vcpuid];
- if ( v == NULL )
- return 0;
- if (test_bit(_VPF_down, &v->pause_flags) )
- return 0;
-
- return 1;
-}
-
static int vgic_to_sgi(struct vcpu *v, register_t sgir)
{
struct domain *d = v->domain;
@@ -444,7 +428,8 @@ static int vgic_to_sgi(struct vcpu *v, register_t sgir)
case GICD_SGI_TARGET_OTHERS:
for ( i = 0; i < d->max_vcpus; i++ )
{
- if ( i != current->vcpu_id && is_vcpu_running(d, i) )
+ if ( i != current->vcpu_id && d->vcpu != NULL &&
+ d->vcpu[i] != NULL && is_vcpu_online(d->vcpu[i]) )
set_bit(i, &vcpu_mask);
}
break;
@@ -459,7 +444,8 @@ static int vgic_to_sgi(struct vcpu *v, register_t sgir)
for_each_set_bit( vcpuid, &vcpu_mask, d->max_vcpus )
{
- if ( !is_vcpu_running(d, vcpuid) )
+ if ( d->vcpu != NULL && d->vcpu[vcpuid] != NULL &&
+ !is_vcpu_online(d->vcpu[vcpuid]) )
{
gdprintk(XENLOG_WARNING, "vGICD: GICD_SGIR write r=%"PRIregister" vcpu_mask=%lx, wrong CPUTargetList\n",
sgir, vcpu_mask);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index f920e1a..99e6c4f 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -829,6 +829,11 @@ void watchdog_domain_destroy(struct domain *d);
#define need_iommu(d) (0)
#endif
+static inline bool_t is_vcpu_online(const struct vcpu *v)
+{
+ return !test_bit(_VPF_down, &v->pause_flags);
+}
+
void set_vcpu_migration_delay(unsigned int delay);
unsigned int get_vcpu_migration_delay(void);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v8a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h
2014-07-03 8:34 ` [PATCH v8a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h vijay.kilari
@ 2014-07-03 9:12 ` Jan Beulich
2014-07-03 9:23 ` Ian Campbell
2014-07-03 12:49 ` Julien Grall
1 sibling, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2014-07-03 9:12 UTC (permalink / raw)
To: vijay.kilari
Cc: keir, Ian.Campbell, stefano.stabellini, Prasun.Kapoor,
Vijaya Kumar K, julien.grall, tim, george.dunlap, xen-devel,
stefano.stabellini
>>> On 03.07.14 at 10:34, <vijay.kilari@gmail.com> wrote:
> +static inline bool_t is_vcpu_online(const struct vcpu *v)
> +{
> + return !test_bit(_VPF_down, &v->pause_flags);
> +}
Which now becomes a bit questionable a helper - it's not really helping
much to wrap this ting test_bit() into an inline function. But if others
like it, I'm not going to stand in the way.
Jan
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v8a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h
2014-07-03 9:12 ` Jan Beulich
@ 2014-07-03 9:23 ` Ian Campbell
0 siblings, 0 replies; 19+ messages in thread
From: Ian Campbell @ 2014-07-03 9:23 UTC (permalink / raw)
To: Jan Beulich
Cc: keir, vijay.kilari, stefano.stabellini, Prasun.Kapoor,
Vijaya Kumar K, julien.grall, tim, george.dunlap, xen-devel,
stefano.stabellini
On Thu, 2014-07-03 at 10:12 +0100, Jan Beulich wrote:
> >>> On 03.07.14 at 10:34, <vijay.kilari@gmail.com> wrote:
> > +static inline bool_t is_vcpu_online(const struct vcpu *v)
> > +{
> > + return !test_bit(_VPF_down, &v->pause_flags);
> > +}
>
> Which now becomes a bit questionable a helper - it's not really helping
> much to wrap this ting test_bit() into an inline function. But if others
> like it, I'm not going to stand in the way.
I think it's ok to have these sorts of accessors, they make the
resulting calling if's a bit easier to reason about IMHO.
Ian.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h
2014-07-03 8:34 ` [PATCH v8a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h vijay.kilari
2014-07-03 9:12 ` Jan Beulich
@ 2014-07-03 12:49 ` Julien Grall
2014-07-03 13:24 ` Jan Beulich
1 sibling, 1 reply; 19+ messages in thread
From: Julien Grall @ 2014-07-03 12:49 UTC (permalink / raw)
To: vijay.kilari, Ian.Campbell, stefano.stabellini,
stefano.stabellini, tim, xen-devel
Cc: Prasun.Kapoor, vijaya.kumar, keir, george.dunlap, jbeulich
Hi Vijay,
On 07/03/2014 09:34 AM, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>
> is_vcpu_running function in vgic driver is generic. So move
> this to sched.h and rename it as is_vcpu_online
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> CC: jbeulich@suse.com
> CC: keir@xen.org
> CC: george.dunlap@citrix.com
> ---
> v8a:is_vcpu_online() is modified to take only vcpu as
> parameter and validity checks on vcpu is done in
> caller
>
> v7a:Changed vcpuid parameter from int to unsigned int
> Removed check for test_bit. Return !test_bit()
> ---
> xen/arch/arm/vgic.c | 22 ++++------------------
> xen/include/xen/sched.h | 5 +++++
> 2 files changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 96bd7c1..4abc682 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -405,22 +405,6 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
> }
> }
>
> -static inline int is_vcpu_running(struct domain *d, int vcpuid)
> -{
> - struct vcpu *v;
> -
> - if ( vcpuid >= d->max_vcpus )
> - return 0;
> -
> - v = d->vcpu[vcpuid];
> - if ( v == NULL )
> - return 0;
> - if (test_bit(_VPF_down, &v->pause_flags) )
> - return 0;
> -
> - return 1;
> -}
> -
> static int vgic_to_sgi(struct vcpu *v, register_t sgir)
> {
> struct domain *d = v->domain;
> @@ -444,7 +428,8 @@ static int vgic_to_sgi(struct vcpu *v, register_t sgir)
> case GICD_SGI_TARGET_OTHERS:
> for ( i = 0; i < d->max_vcpus; i++ )
> {
> - if ( i != current->vcpu_id && is_vcpu_running(d, i) )
> + if ( i != current->vcpu_id && d->vcpu != NULL &&
d->vcpu can't be NULL at this point. Otherwise you won't be able to run
VCPU0 of the domain :).
Although, I may have miss something during the previous version, why
don't you check that the VCPU is NULL in is_vcpu_online?
> + d->vcpu[i] != NULL && is_vcpu_online(d->vcpu[i]) )
> set_bit(i, &vcpu_mask);
> }
> break;
> @@ -459,7 +444,8 @@ static int vgic_to_sgi(struct vcpu *v, register_t sgir)
>
> for_each_set_bit( vcpuid, &vcpu_mask, d->max_vcpus )
> {
> - if ( !is_vcpu_running(d, vcpuid) )
> + if ( d->vcpu != NULL && d->vcpu[vcpuid] != NULL &&
Same here.
> + !is_vcpu_online(d->vcpu[vcpuid]) )
> {
> gdprintk(XENLOG_WARNING, "vGICD: GICD_SGIR write r=%"PRIregister" vcpu_mask=%lx, wrong CPUTargetList\n",
> sgir, vcpu_mask);
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v8a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h
2014-07-03 12:49 ` Julien Grall
@ 2014-07-03 13:24 ` Jan Beulich
0 siblings, 0 replies; 19+ messages in thread
From: Jan Beulich @ 2014-07-03 13:24 UTC (permalink / raw)
To: Julien Grall
Cc: keir, Ian.Campbell, vijay.kilari, stefano.stabellini,
Prasun.Kapoor, vijaya.kumar, tim, george.dunlap, xen-devel,
stefano.stabellini
>>> On 03.07.14 at 14:49, <julien.grall@linaro.org> wrote:
> On 07/03/2014 09:34 AM, vijay.kilari@gmail.com wrote:
>> @@ -444,7 +428,8 @@ static int vgic_to_sgi(struct vcpu *v, register_t sgir)
>> case GICD_SGI_TARGET_OTHERS:
>> for ( i = 0; i < d->max_vcpus; i++ )
>> {
>> - if ( i != current->vcpu_id && is_vcpu_running(d, i) )
>> + if ( i != current->vcpu_id && d->vcpu != NULL &&
>
> d->vcpu can't be NULL at this point. Otherwise you won't be able to run
> VCPU0 of the domain :).
>
> Although, I may have miss something during the previous version, why
> don't you check that the VCPU is NULL in is_vcpu_online?
Functions like this generally assume to be called with valid arguments.
Eventual future users may have done the validation already, and
hence would then be (slightly) penalized by redundant checking.
Jan
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v8a 2/6] xen/arm: move pending_irq structure to vgic header file
2014-07-03 8:34 [PATCH v8a 0/6] GIC and VGIC code refactoring vijay.kilari
2014-07-03 8:34 ` [PATCH v8a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h vijay.kilari
@ 2014-07-03 8:34 ` vijay.kilari
2014-07-03 8:34 ` [PATCH v8a 3/6] xen/arm: calculate vgic irq rank based on register size vijay.kilari
` (3 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: vijay.kilari @ 2014-07-03 8:34 UTC (permalink / raw)
To: Ian.Campbell, julien.grall, stefano.stabellini,
stefano.stabellini, tim, xen-devel
Cc: Prasun.Kapoor, Vijaya Kumar K, vijay.kilari
From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
gic.h requires definition of pending_irq structure defined
in domain.h and domain.h requires gic_state structure defined
in gic.h and hence there is inter-dependency between domain.h
and gic.h files.
So move pending_irq to vgic.h which is relevant place for this
structure and break domain.h and gic.h interdependency
By this move irq_to_pending function declaration from gic.h
to vgic.h
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
xen/include/asm-arm/domain.h | 57 -----------------------------------------
xen/include/asm-arm/gic.h | 2 +-
xen/include/asm-arm/vgic.h | 58 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 58 deletions(-)
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 077ac1e..2a1e976 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -13,63 +13,6 @@
#include <xen/serial.h>
#include <xen/hvm/iommu.h>
-struct pending_irq
-{
- /*
- * The following two states track the lifecycle of the guest irq.
- * However because we are not sure and we don't want to track
- * whether an irq added to an LR register is PENDING or ACTIVE, the
- * following states are just an approximation.
- *
- * GIC_IRQ_GUEST_QUEUED: the irq is asserted and queued for
- * injection into the guest's LRs.
- *
- * GIC_IRQ_GUEST_VISIBLE: the irq has been added to an LR register,
- * therefore the guest is aware of it. From the guest point of view
- * the irq can be pending (if the guest has not acked the irq yet)
- * or active (after acking the irq).
- *
- * In order for the state machine to be fully accurate, for level
- * interrupts, we should keep the interrupt's pending state until
- * the guest deactivates the irq. However because we are not sure
- * when that happens, we instead track whether there is an interrupt
- * queued using GIC_IRQ_GUEST_QUEUED. We clear it when we add it to
- * an LR register. We set it when we receive another interrupt
- * notification. Therefore it is possible to set
- * GIC_IRQ_GUEST_QUEUED while the irq is GIC_IRQ_GUEST_VISIBLE. We
- * could also change the state of the guest irq in the LR register
- * from active to active and pending, but for simplicity we simply
- * inject a second irq after the guest EOIs the first one.
- *
- *
- * An additional state is used to keep track of whether the guest
- * irq is enabled at the vgicd level:
- *
- * GIC_IRQ_GUEST_ENABLED: the guest IRQ is enabled at the VGICD
- * level (GICD_ICENABLER/GICD_ISENABLER).
- *
- */
-#define GIC_IRQ_GUEST_QUEUED 0
-#define GIC_IRQ_GUEST_ACTIVE 1
-#define GIC_IRQ_GUEST_VISIBLE 2
-#define GIC_IRQ_GUEST_ENABLED 3
- unsigned long status;
- struct irq_desc *desc; /* only set it the irq corresponds to a physical irq */
- int irq;
-#define GIC_INVALID_LR ~(uint8_t)0
- uint8_t lr;
- uint8_t priority;
- /* inflight is used to append instances of pending_irq to
- * vgic.inflight_irqs */
- struct list_head inflight;
- /* lr_queue is used to append instances of pending_irq to
- * lr_pending. lr_pending is a per vcpu queue, therefore lr_queue
- * accesses are protected with the vgic lock.
- * TODO: when implementing irq migration, taking only the current
- * vgic lock is not going to be enough. */
- struct list_head lr_queue;
-};
-
struct hvm_domain
{
uint64_t params[HVM_NR_PARAMS];
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index ed610cb..875729e 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -143,6 +143,7 @@
#ifndef __ASSEMBLY__
#include <xen/device_tree.h>
#include <xen/irq.h>
+#include <asm-arm/vgic.h>
#define DT_COMPAT_GIC_CORTEX_A15 "arm,cortex-a15-gic"
#define DT_COMPAT_GIC_CORTEX_A7 "arm,cortex-a7-gic"
@@ -188,7 +189,6 @@ enum gic_version {
};
extern enum gic_version gic_hw_version(void);
-extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq);
/* Program the GIC to route an interrupt */
extern void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 92f1e86..003c3e9 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -20,6 +20,63 @@
#include <xen/bitops.h>
+struct pending_irq
+{
+ /*
+ * The following two states track the lifecycle of the guest irq.
+ * However because we are not sure and we don't want to track
+ * whether an irq added to an LR register is PENDING or ACTIVE, the
+ * following states are just an approximation.
+ *
+ * GIC_IRQ_GUEST_QUEUED: the irq is asserted and queued for
+ * injection into the guest's LRs.
+ *
+ * GIC_IRQ_GUEST_VISIBLE: the irq has been added to an LR register,
+ * therefore the guest is aware of it. From the guest point of view
+ * the irq can be pending (if the guest has not acked the irq yet)
+ * or active (after acking the irq).
+ *
+ * In order for the state machine to be fully accurate, for level
+ * interrupts, we should keep the interrupt's pending state until
+ * the guest deactivates the irq. However because we are not sure
+ * when that happens, we instead track whether there is an interrupt
+ * queued using GIC_IRQ_GUEST_QUEUED. We clear it when we add it to
+ * an LR register. We set it when we receive another interrupt
+ * notification. Therefore it is possible to set
+ * GIC_IRQ_GUEST_QUEUED while the irq is GIC_IRQ_GUEST_VISIBLE. We
+ * could also change the state of the guest irq in the LR register
+ * from active to active and pending, but for simplicity we simply
+ * inject a second irq after the guest EOIs the first one.
+ *
+ *
+ * An additional state is used to keep track of whether the guest
+ * irq is enabled at the vgicd level:
+ *
+ * GIC_IRQ_GUEST_ENABLED: the guest IRQ is enabled at the VGICD
+ * level (GICD_ICENABLER/GICD_ISENABLER).
+ *
+ */
+#define GIC_IRQ_GUEST_QUEUED 0
+#define GIC_IRQ_GUEST_ACTIVE 1
+#define GIC_IRQ_GUEST_VISIBLE 2
+#define GIC_IRQ_GUEST_ENABLED 3
+ unsigned long status;
+ struct irq_desc *desc; /* only set it the irq corresponds to a physical irq */
+ int irq;
+#define GIC_INVALID_LR ~(uint8_t)0
+ uint8_t lr;
+ uint8_t priority;
+ /* inflight is used to append instances of pending_irq to
+ * vgic.inflight_irqs */
+ struct list_head inflight;
+ /* lr_queue is used to append instances of pending_irq to
+ * lr_pending. lr_pending is a per vcpu queue, therefore lr_queue
+ * accesses are protected with the vgic lock.
+ * TODO: when implementing irq migration, taking only the current
+ * vgic lock is not going to be enough. */
+ struct list_head lr_queue;
+};
+
/* Represents state corresponding to a block of 32 interrupts */
struct vgic_irq_rank {
spinlock_t lock; /* Covers access to all other members of this struct */
@@ -87,6 +144,7 @@ extern void domain_vgic_free(struct domain *d);
extern int vcpu_vgic_init(struct vcpu *v);
extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq);
extern void vgic_clear_pending_irqs(struct vcpu *v);
+extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq);
extern int vcpu_vgic_free(struct vcpu *v);
#endif /* __ASM_ARM_VGIC_H__ */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v8a 3/6] xen/arm: calculate vgic irq rank based on register size
2014-07-03 8:34 [PATCH v8a 0/6] GIC and VGIC code refactoring vijay.kilari
2014-07-03 8:34 ` [PATCH v8a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h vijay.kilari
2014-07-03 8:34 ` [PATCH v8a 2/6] xen/arm: move pending_irq structure to vgic header file vijay.kilari
@ 2014-07-03 8:34 ` vijay.kilari
2014-07-03 8:34 ` [PATCH v8a 4/6] xen/arm: Remove REG macro in vgic driver vijay.kilari
` (2 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: vijay.kilari @ 2014-07-03 8:34 UTC (permalink / raw)
To: Ian.Campbell, julien.grall, stefano.stabellini,
stefano.stabellini, tim, xen-devel
Cc: Prasun.Kapoor, Vijaya Kumar K, vijay.kilari
From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
vGIC irq rank was computed assuming the register offset is byte
size.Use the HSR abort address size in calculating register size.
So, with this patch following are achieved
(1) In the code 'dabt.size != number' this number is always
BYTE/HALF_WORD/WORD/DOUBLE defined by HSR register.
Instead of checking for hard coded values use HSR abort
address size values.
(2) The vgic_rank_offset also depends on register size to
compute the rank offset. Though there is no direct relation
between rank offset computation and HSR dabt.size the same
values are used to calculate irq rank.
This make vgic_rank_offset generic as it takes register
size as parameter to calculate irq rank instead of hard coding to
value 2 in previous patches
Also, output of REG_RANK_INDEX macro is modulo by 32 to make
sure register index is always within irq rank
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/vgic.c | 139 ++++++++++++++++++++-------------------
xen/include/asm-arm/processor.h | 8 +++
xen/include/asm-arm/vgic.h | 4 +-
3 files changed, 83 insertions(+), 68 deletions(-)
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 4abc682..c45c243 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -40,9 +40,10 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info);
* Returns rank corresponding to a GICD_<FOO><n> register for
* GICD_<FOO> with <b>-bits-per-interrupt.
*/
-static struct vgic_irq_rank *vgic_rank_offset(struct vcpu *v, int b, int n)
+static struct vgic_irq_rank *vgic_rank_offset(struct vcpu *v, int b, int n,
+ int s)
{
- int rank = REG_RANK_NR(b, (n >> 2));
+ int rank = REG_RANK_NR(b, (n >> s));
if ( rank == 0 )
return v->arch.vgic.private_irqs;
@@ -54,7 +55,7 @@ static struct vgic_irq_rank *vgic_rank_offset(struct vcpu *v, int b, int n)
static struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned int irq)
{
- return vgic_rank_offset(v, 8, irq);
+ return vgic_rank_offset(v, 8, irq, DABT_WORD);
}
static const struct mmio_handler_ops vgic_distr_mmio_handler = {
@@ -161,13 +162,13 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
switch ( gicd_reg )
{
case GICD_CTLR:
- if ( dabt.size != 2 ) goto bad_width;
+ if ( dabt.size != DABT_WORD ) goto bad_width;
vgic_lock(v);
*r = v->domain->arch.vgic.ctlr;
vgic_unlock(v);
return 1;
case GICD_TYPER:
- if ( dabt.size != 2 ) goto bad_width;
+ if ( dabt.size != DABT_WORD ) goto bad_width;
/* No secure world support for guests. */
vgic_lock(v);
*r = ( (v->domain->max_vcpus<<5) & GICD_TYPE_CPUS )
@@ -175,7 +176,7 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
vgic_unlock(v);
return 1;
case GICD_IIDR:
- if ( dabt.size != 2 ) goto bad_width;
+ if ( dabt.size != DABT_WORD ) goto bad_width;
/*
* XXX Do we need a JEP106 manufacturer ID?
* Just use the physical h/w value for now
@@ -192,8 +193,8 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
goto read_as_zero;
case GICD_ISENABLER ... GICD_ISENABLERN:
- if ( dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISENABLER);
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISENABLER, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
*r = rank->ienable;
@@ -201,8 +202,8 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
return 1;
case GICD_ICENABLER ... GICD_ICENABLERN:
- if ( dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICENABLER);
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICENABLER, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
*r = rank->ienable;
@@ -210,8 +211,8 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
return 1;
case GICD_ISPENDR ... GICD_ISPENDRN:
- if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISPENDR);
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISPENDR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
*r = vgic_byte_read(rank->ipend, dabt.sign, offset);
@@ -219,8 +220,8 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
return 1;
case GICD_ICPENDR ... GICD_ICPENDRN:
- if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICPENDR);
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICPENDR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
*r = vgic_byte_read(rank->ipend, dabt.sign, offset);
@@ -228,8 +229,8 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
return 1;
case GICD_ISACTIVER ... GICD_ISACTIVERN:
- if ( dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISACTIVER);
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISACTIVER, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
*r = rank->iactive;
@@ -237,8 +238,8 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
return 1;
case GICD_ICACTIVER ... GICD_ICACTIVERN:
- if ( dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICACTIVER);
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICACTIVER, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
*r = rank->iactive;
@@ -246,35 +247,37 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
return 1;
case GICD_ITARGETSR ... GICD_ITARGETSRN:
- if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR);
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
- *r = rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR)];
- if ( dabt.size == 0 )
+ *r = rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR,
+ DABT_WORD)];
+ if ( dabt.size == DABT_BYTE )
*r = vgic_byte_read(*r, dabt.sign, offset);
vgic_unlock_rank(v, rank);
return 1;
case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
- if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 8, gicd_reg - GICD_IPRIORITYR);
+ if ( dabt.size != 0 && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 8, gicd_reg - GICD_IPRIORITYR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
- *r = rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR)];
- if ( dabt.size == 0 )
+ *r = rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
+ DABT_WORD)];
+ if ( dabt.size == DABT_BYTE )
*r = vgic_byte_read(*r, dabt.sign, offset);
vgic_unlock_rank(v, rank);
return 1;
case GICD_ICFGR ... GICD_ICFGRN:
- if ( dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR);
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
- *r = rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR)];
+ *r = rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)];
vgic_unlock_rank(v, rank);
return 1;
@@ -283,14 +286,14 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
goto read_as_zero;
case GICD_SGIR:
- if ( dabt.size != 2 ) goto bad_width;
+ if ( dabt.size != DABT_WORD ) goto bad_width;
/* Write only -- read unknown */
*r = 0xdeadbeef;
return 1;
case GICD_CPENDSGIR ... GICD_CPENDSGIRN:
- if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_CPENDSGIR);
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_CPENDSGIR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
*r = vgic_byte_read(rank->pendsgi, dabt.sign, offset);
@@ -298,8 +301,8 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
return 1;
case GICD_SPENDSGIR ... GICD_SPENDSGIRN:
- if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_SPENDSGIR);
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_SPENDSGIR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
*r = vgic_byte_read(rank->pendsgi, dabt.sign, offset);
@@ -311,7 +314,7 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
goto read_as_zero;
case GICD_ICPIDR2:
- if ( dabt.size != 2 ) goto bad_width;
+ if ( dabt.size != DABT_WORD ) goto bad_width;
printk("vGICD: unhandled read from ICPIDR2\n");
return 0;
@@ -341,7 +344,7 @@ bad_width:
return 0;
read_as_zero:
- if ( dabt.size != 2 ) goto bad_width;
+ if ( dabt.size != DABT_WORD ) goto bad_width;
*r = 0;
return 1;
}
@@ -469,7 +472,7 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
switch ( gicd_reg )
{
case GICD_CTLR:
- if ( dabt.size != 2 ) goto bad_width;
+ if ( dabt.size != DABT_WORD ) goto bad_width;
/* Ignore all but the enable bit */
v->domain->arch.vgic.ctlr = (*r) & GICD_CTL_ENABLE;
return 1;
@@ -488,42 +491,44 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
goto write_ignore;
case GICD_ISENABLER ... GICD_ISENABLERN:
- if ( dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISENABLER);
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISENABLER, DABT_WORD);
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank);
tr = rank->ienable;
rank->ienable |= *r;
vgic_unlock_rank(v, rank);
- vgic_enable_irqs(v, (*r) & (~tr), (gicd_reg - GICD_ISENABLER) >> 2);
+ vgic_enable_irqs(v, (*r) & (~tr),
+ (gicd_reg - GICD_ISENABLER) >> DABT_WORD);
return 1;
case GICD_ICENABLER ... GICD_ICENABLERN:
- if ( dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICENABLER);
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICENABLER, DABT_WORD);
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank);
tr = rank->ienable;
rank->ienable &= ~*r;
vgic_unlock_rank(v, rank);
- vgic_disable_irqs(v, (*r) & tr, (gicd_reg - GICD_ICENABLER) >> 2);
+ vgic_disable_irqs(v, (*r) & tr,
+ (gicd_reg - GICD_ICENABLER) >> DABT_WORD);
return 1;
case GICD_ISPENDR ... GICD_ISPENDRN:
- if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
printk("vGICD: unhandled %s write %#"PRIregister" to ISPENDR%d\n",
dabt.size ? "word" : "byte", *r, gicd_reg - GICD_ISPENDR);
return 0;
case GICD_ICPENDR ... GICD_ICPENDRN:
- if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
printk("vGICD: unhandled %s write %#"PRIregister" to ICPENDR%d\n",
dabt.size ? "word" : "byte", *r, gicd_reg - GICD_ICPENDR);
return 0;
case GICD_ISACTIVER ... GICD_ISACTIVERN:
- if ( dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISACTIVER);
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISACTIVER, DABT_WORD);
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank);
rank->iactive &= ~*r;
@@ -531,8 +536,8 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
return 1;
case GICD_ICACTIVER ... GICD_ICACTIVERN:
- if ( dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICACTIVER);
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICACTIVER, DABT_WORD);
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank);
rank->iactive &= ~*r;
@@ -544,30 +549,32 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
goto write_ignore;
case GICD_ITARGETSR + 8 ... GICD_ITARGETSRN:
- if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR);
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank);
- if ( dabt.size == 2 )
- rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR)] = *r;
+ if ( dabt.size == DABT_WORD )
+ rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR,
+ DABT_WORD)] = *r;
else
{
- tr = REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR);
+ tr = REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
vgic_byte_write(&rank->itargets[tr], *r, offset);
}
vgic_unlock_rank(v, rank);
return 1;
case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
- if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 8, gicd_reg - GICD_IPRIORITYR);
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 8, gicd_reg - GICD_IPRIORITYR, DABT_WORD);
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank);
- if ( dabt.size == 2 )
- rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR)] = *r;
+ if ( dabt.size == DABT_WORD )
+ rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
+ DABT_WORD)] = *r;
else
{
- tr = REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR);
+ tr = REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR, DABT_WORD);
vgic_byte_write(&rank->ipriority[tr], *r, offset);
}
vgic_unlock_rank(v, rank);
@@ -579,11 +586,11 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
/* It is implementation defined if these are writeable. We chose not */
goto write_ignore;
case GICD_ICFGR + 2 ... GICD_ICFGRN: /* SPIs */
- if ( dabt.size != 2 ) goto bad_width;
- rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR);
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank);
- rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR)] = *r;
+ rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)] = *r;
vgic_unlock_rank(v, rank);
return 1;
@@ -597,13 +604,13 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
return vgic_to_sgi(v, *r);
case GICD_CPENDSGIR ... GICD_CPENDSGIRN:
- if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
printk("vGICD: unhandled %s write %#"PRIregister" to ICPENDSGIR%d\n",
dabt.size ? "word" : "byte", *r, gicd_reg - GICD_CPENDSGIR);
return 0;
case GICD_SPENDSGIR ... GICD_SPENDSGIRN:
- if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
printk("vGICD: unhandled %s write %#"PRIregister" to ISPENDSGIR%d\n",
dabt.size ? "word" : "byte", *r, gicd_reg - GICD_SPENDSGIR);
return 0;
@@ -642,7 +649,7 @@ bad_width:
return 0;
write_ignore:
- if ( dabt.size != 2 ) goto bad_width;
+ if ( dabt.size != DABT_WORD ) goto bad_width;
return 1;
}
@@ -694,7 +701,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq)
return;
}
- priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8, irq)], 0, irq & 0x3);
+ priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8, irq, DABT_WORD)], 0, irq & 0x3);
n->irq = irq;
set_bit(GIC_IRQ_GUEST_QUEUED, &n->status);
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 3be86f1..bdfff4e 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -251,6 +251,14 @@ extern struct cpuinfo_arm cpu_data[];
extern u32 __cpu_logical_map[];
#define cpu_logical_map(cpu) __cpu_logical_map[cpu]
+/* HSR data abort size definition */
+enum dabt_size {
+ DABT_BYTE = 0,
+ DABT_HALF_WORD = 1,
+ DABT_WORD = 2,
+ DABT_DOUBLE_WORD = 3,
+};
+
union hsr {
uint32_t bits;
struct {
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 003c3e9..7c71d16 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -134,10 +134,10 @@ static inline void vgic_byte_write(uint32_t *reg, uint32_t var, int offset)
}
/*
- * Offset of GICD_<FOO><n> with its rank, for GICD_<FOO> with
+ * Offset of GICD_<FOO><n> with its rank, for GICD_<FOO> size <s> with
* <b>-bits-per-interrupt.
*/
-#define REG_RANK_INDEX(b, n) (((n) >> 2) & ((b)-1))
+#define REG_RANK_INDEX(b, n, s) ((((n) >> s) & ((b)-1)) % 32)
extern int domain_vgic_init(struct domain *d);
extern void domain_vgic_free(struct domain *d);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v8a 4/6] xen/arm: Remove REG macro in vgic driver
2014-07-03 8:34 [PATCH v8a 0/6] GIC and VGIC code refactoring vijay.kilari
` (2 preceding siblings ...)
2014-07-03 8:34 ` [PATCH v8a 3/6] xen/arm: calculate vgic irq rank based on register size vijay.kilari
@ 2014-07-03 8:34 ` vijay.kilari
2014-07-03 8:34 ` [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver vijay.kilari
2014-07-03 8:34 ` [PATCH v8a 6/6] xen/arm: Restrict saving of gic register for idle domain vijay.kilari
5 siblings, 0 replies; 19+ messages in thread
From: vijay.kilari @ 2014-07-03 8:34 UTC (permalink / raw)
To: Ian.Campbell, julien.grall, stefano.stabellini,
stefano.stabellini, tim, xen-devel
Cc: Prasun.Kapoor, Vijaya Kumar K, vijay.kilari
From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
REG macro does not compute any value and offset
variable is no more required. Hence removed
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/vgic.c | 68 ++++++++++++++++++++++++---------------------------
1 file changed, 32 insertions(+), 36 deletions(-)
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index c45c243..ed21f62 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -31,8 +31,6 @@
#include <asm/gic.h>
#include <asm/vgic.h>
-#define REG(n) (n)
-
static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info);
static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info);
@@ -156,8 +154,7 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
struct cpu_user_regs *regs = guest_cpu_user_regs();
register_t *r = select_user_reg(regs, dabt.reg);
struct vgic_irq_rank *rank;
- int offset = (int)(info->gpa - v->domain->arch.vgic.dbase);
- int gicd_reg = REG(offset);
+ int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
switch ( gicd_reg )
{
@@ -185,7 +182,7 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
return 1;
/* Implementation defined -- read as zero */
- case REG(0x020) ... REG(0x03c):
+ case 0x020 ... 0x03c:
goto read_as_zero;
case GICD_IGROUPR ... GICD_IGROUPRN:
@@ -215,7 +212,7 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISPENDR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
- *r = vgic_byte_read(rank->ipend, dabt.sign, offset);
+ *r = vgic_byte_read(rank->ipend, dabt.sign, gicd_reg);
vgic_unlock_rank(v, rank);
return 1;
@@ -224,7 +221,7 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICPENDR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
- *r = vgic_byte_read(rank->ipend, dabt.sign, offset);
+ *r = vgic_byte_read(rank->ipend, dabt.sign, gicd_reg);
vgic_unlock_rank(v, rank);
return 1;
@@ -255,7 +252,7 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
*r = rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR,
DABT_WORD)];
if ( dabt.size == DABT_BYTE )
- *r = vgic_byte_read(*r, dabt.sign, offset);
+ *r = vgic_byte_read(*r, dabt.sign, gicd_reg);
vgic_unlock_rank(v, rank);
return 1;
@@ -268,7 +265,7 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
*r = rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
DABT_WORD)];
if ( dabt.size == DABT_BYTE )
- *r = vgic_byte_read(*r, dabt.sign, offset);
+ *r = vgic_byte_read(*r, dabt.sign, gicd_reg);
vgic_unlock_rank(v, rank);
return 1;
@@ -296,7 +293,7 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
rank = vgic_rank_offset(v, 1, gicd_reg - GICD_CPENDSGIR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
- *r = vgic_byte_read(rank->pendsgi, dabt.sign, offset);
+ *r = vgic_byte_read(rank->pendsgi, dabt.sign, gicd_reg);
vgic_unlock_rank(v, rank);
return 1;
@@ -305,12 +302,12 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
rank = vgic_rank_offset(v, 1, gicd_reg - GICD_SPENDSGIR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank);
- *r = vgic_byte_read(rank->pendsgi, dabt.sign, offset);
+ *r = vgic_byte_read(rank->pendsgi, dabt.sign, gicd_reg);
vgic_unlock_rank(v, rank);
return 1;
/* Implementation defined -- read as zero */
- case REG(0xfd0) ... REG(0xfe4):
+ case 0xfd0 ... 0xfe4:
goto read_as_zero;
case GICD_ICPIDR2:
@@ -319,27 +316,27 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
return 0;
/* Implementation defined -- read as zero */
- case REG(0xfec) ... REG(0xffc):
+ case 0xfec ... 0xffc:
goto read_as_zero;
/* Reserved -- read as zero */
- case REG(0x00c) ... REG(0x01c):
- case REG(0x040) ... REG(0x07c):
- case REG(0x7fc):
- case REG(0xbfc):
- case REG(0xf04) ... REG(0xf0c):
- case REG(0xf30) ... REG(0xfcc):
+ case 0x00c ... 0x01c:
+ case 0x040 ... 0x07c:
+ case 0x7fc:
+ case 0xbfc:
+ case 0xf04 ... 0xf0c:
+ case 0xf30 ... 0xfcc:
goto read_as_zero;
default:
printk("vGICD: unhandled read r%d offset %#08x\n",
- dabt.reg, offset);
+ dabt.reg, gicd_reg);
return 0;
}
bad_width:
printk("vGICD: bad read width %d r%d offset %#08x\n",
- dabt.size, dabt.reg, offset);
+ dabt.size, dabt.reg, gicd_reg);
domain_crash_synchronous();
return 0;
@@ -465,8 +462,7 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
struct cpu_user_regs *regs = guest_cpu_user_regs();
register_t *r = select_user_reg(regs, dabt.reg);
struct vgic_irq_rank *rank;
- int offset = (int)(info->gpa - v->domain->arch.vgic.dbase);
- int gicd_reg = REG(offset);
+ int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
uint32_t tr;
switch ( gicd_reg )
@@ -483,7 +479,7 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
goto write_ignore;
/* Implementation defined -- write ignored */
- case REG(0x020) ... REG(0x03c):
+ case 0x020 ... 0x03c:
goto write_ignore;
case GICD_IGROUPR ... GICD_IGROUPRN:
@@ -559,7 +555,7 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
else
{
tr = REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
- vgic_byte_write(&rank->itargets[tr], *r, offset);
+ vgic_byte_write(&rank->itargets[tr], *r, gicd_reg);
}
vgic_unlock_rank(v, rank);
return 1;
@@ -575,7 +571,7 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
else
{
tr = REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR, DABT_WORD);
- vgic_byte_write(&rank->ipriority[tr], *r, offset);
+ vgic_byte_write(&rank->ipriority[tr], *r, gicd_reg);
}
vgic_unlock_rank(v, rank);
return 1;
@@ -616,7 +612,7 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
return 0;
/* Implementation defined -- write ignored */
- case REG(0xfd0) ... REG(0xfe4):
+ case 0xfd0 ... 0xfe4:
goto write_ignore;
/* R/O -- write ignore */
@@ -624,27 +620,27 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
goto write_ignore;
/* Implementation defined -- write ignored */
- case REG(0xfec) ... REG(0xffc):
+ case 0xfec ... 0xffc:
goto write_ignore;
/* Reserved -- write ignored */
- case REG(0x00c) ... REG(0x01c):
- case REG(0x040) ... REG(0x07c):
- case REG(0x7fc):
- case REG(0xbfc):
- case REG(0xf04) ... REG(0xf0c):
- case REG(0xf30) ... REG(0xfcc):
+ case 0x00c ... 0x01c:
+ case 0x040 ... 0x07c:
+ case 0x7fc:
+ case 0xbfc:
+ case 0xf04 ... 0xf0c:
+ case 0xf30 ... 0xfcc:
goto write_ignore;
default:
printk("vGICD: unhandled write r%d=%"PRIregister" offset %#08x\n",
- dabt.reg, *r, offset);
+ dabt.reg, *r, gicd_reg);
return 0;
}
bad_width:
printk("vGICD: bad write width %d r%d=%"PRIregister" offset %#08x\n",
- dabt.size, dabt.reg, *r, offset);
+ dabt.size, dabt.reg, *r, gicd_reg);
domain_crash_synchronous();
return 0;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver
2014-07-03 8:34 [PATCH v8a 0/6] GIC and VGIC code refactoring vijay.kilari
` (3 preceding siblings ...)
2014-07-03 8:34 ` [PATCH v8a 4/6] xen/arm: Remove REG macro in vgic driver vijay.kilari
@ 2014-07-03 8:34 ` vijay.kilari
2014-07-03 12:57 ` Julien Grall
2014-07-03 8:34 ` [PATCH v8a 6/6] xen/arm: Restrict saving of gic register for idle domain vijay.kilari
5 siblings, 1 reply; 19+ messages in thread
From: vijay.kilari @ 2014-07-03 8:34 UTC (permalink / raw)
To: Ian.Campbell, julien.grall, stefano.stabellini,
stefano.stabellini, tim, xen-devel
Cc: Prasun.Kapoor, Vijaya Kumar K, vijay.kilari
From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
Existing vGIC driver has both generic code and hw specific
code. Segregate vGIC low level driver into vgic-v2.c and
keep generic code in existing vgic.c file.
Some static generic functions in vgic.c is made as non-static
so that these generic functions can be used in vGIC v2 driver.
vGIC v2 driver registers required callbacks to generic vGIC
driver. This helps to plug in next version of vGIC drivers
like vGIC v3. These callbacks are registered per domain
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
---
v8a: set vcpu_mask to 0 for SGI_TARGET_{OTHERS,SELF} modes
made vgic-v2 mmio handler static
---
xen/arch/arm/Makefile | 2 +-
xen/arch/arm/vgic-v2.c | 507 ++++++++++++++++++++++++++++++++++++++++++
xen/arch/arm/vgic.c | 499 +++++------------------------------------
xen/include/asm-arm/domain.h | 2 +
xen/include/asm-arm/gic.h | 3 +
xen/include/asm-arm/vgic.h | 20 ++
6 files changed, 592 insertions(+), 441 deletions(-)
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 969ee52..20f59f4 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -26,7 +26,7 @@ obj-y += smpboot.o
obj-y += smp.o
obj-y += shutdown.o
obj-y += traps.o
-obj-y += vgic.o
+obj-y += vgic.o vgic-v2.o
obj-y += vtimer.o
obj-y += vuart.o
obj-y += hvm.o
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
new file mode 100644
index 0000000..5959ca7
--- /dev/null
+++ b/xen/arch/arm/vgic-v2.c
@@ -0,0 +1,507 @@
+/*
+ * xen/arch/arm/vgic-v2.c
+ *
+ * ARM Virtual Generic Interrupt Controller support v2
+ *
+ * Ian Campbell <ian.campbell@citrix.com>
+ * Copyright (c) 2011 Citrix Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <xen/bitops.h>
+#include <xen/config.h>
+#include <xen/lib.h>
+#include <xen/init.h>
+#include <xen/softirq.h>
+#include <xen/irq.h>
+#include <xen/sched.h>
+
+#include <asm/current.h>
+#include <asm/device.h>
+
+#include <asm/mmio.h>
+#include <asm/gic.h>
+#include <asm/vgic.h>
+
+static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
+{
+ struct hsr_dabt dabt = info->dabt;
+ struct cpu_user_regs *regs = guest_cpu_user_regs();
+ register_t *r = select_user_reg(regs, dabt.reg);
+ struct vgic_irq_rank *rank;
+ int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
+
+ switch ( gicd_reg )
+ {
+ case GICD_CTLR:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ vgic_lock(v);
+ *r = v->domain->arch.vgic.ctlr;
+ vgic_unlock(v);
+ return 1;
+ case GICD_TYPER:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ /* No secure world support for guests. */
+ vgic_lock(v);
+ *r = ( (v->domain->max_vcpus << 5) & GICD_TYPE_CPUS )
+ |( ((v->domain->arch.vgic.nr_lines / 32)) & GICD_TYPE_LINES );
+ vgic_unlock(v);
+ return 1;
+ case GICD_IIDR:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ /*
+ * XXX Do we need a JEP106 manufacturer ID?
+ * Just use the physical h/w value for now
+ */
+ *r = 0x0000043b;
+ return 1;
+
+ /* Implementation defined -- read as zero */
+ case 0x020 ... 0x03c:
+ goto read_as_zero;
+
+ case GICD_IGROUPR ... GICD_IGROUPRN:
+ /* We do not implement security extensions for guests, read zero */
+ goto read_as_zero;
+
+ case GICD_ISENABLER ... GICD_ISENABLERN:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISENABLER, DABT_WORD);
+ if ( rank == NULL) goto read_as_zero;
+ vgic_lock_rank(v, rank);
+ *r = rank->ienable;
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_ICENABLER ... GICD_ICENABLERN:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICENABLER, DABT_WORD);
+ if ( rank == NULL) goto read_as_zero;
+ vgic_lock_rank(v, rank);
+ *r = rank->ienable;
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_ISPENDR ... GICD_ISPENDRN:
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISPENDR, DABT_WORD);
+ if ( rank == NULL) goto read_as_zero;
+ vgic_lock_rank(v, rank);
+ *r = vgic_byte_read(rank->ipend, dabt.sign, gicd_reg);
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_ICPENDR ... GICD_ICPENDRN:
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICPENDR, DABT_WORD);
+ if ( rank == NULL) goto read_as_zero;
+ vgic_lock_rank(v, rank);
+ *r = vgic_byte_read(rank->ipend, dabt.sign, gicd_reg);
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_ISACTIVER ... GICD_ISACTIVERN:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISACTIVER, DABT_WORD);
+ if ( rank == NULL) goto read_as_zero;
+ vgic_lock_rank(v, rank);
+ *r = rank->iactive;
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_ICACTIVER ... GICD_ICACTIVERN:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICACTIVER, DABT_WORD);
+ if ( rank == NULL) goto read_as_zero;
+ vgic_lock_rank(v, rank);
+ *r = rank->iactive;
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_ITARGETSR ... GICD_ITARGETSRN:
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
+ if ( rank == NULL) goto read_as_zero;
+
+ vgic_lock_rank(v, rank);
+ *r = rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR,
+ DABT_WORD)];
+ if ( dabt.size == DABT_BYTE )
+ *r = vgic_byte_read(*r, dabt.sign, gicd_reg);
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 8, gicd_reg - GICD_IPRIORITYR, DABT_WORD);
+ if ( rank == NULL) goto read_as_zero;
+
+ vgic_lock_rank(v, rank);
+ *r = rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
+ DABT_WORD)];
+ if ( dabt.size == DABT_BYTE )
+ *r = vgic_byte_read(*r, dabt.sign, gicd_reg);
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_ICFGR ... GICD_ICFGRN:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
+ if ( rank == NULL) goto read_as_zero;
+ vgic_lock_rank(v, rank);
+ *r = rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)];
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_NSACR ... GICD_NSACRN:
+ /* We do not implement security extensions for guests, read zero */
+ goto read_as_zero;
+
+ case GICD_SGIR:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ /* Write only -- read unknown */
+ *r = 0xdeadbeef;
+ return 1;
+
+ case GICD_CPENDSGIR ... GICD_CPENDSGIRN:
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_CPENDSGIR, DABT_WORD);
+ if ( rank == NULL) goto read_as_zero;
+ vgic_lock_rank(v, rank);
+ *r = vgic_byte_read(rank->pendsgi, dabt.sign, gicd_reg);
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_SPENDSGIR ... GICD_SPENDSGIRN:
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_SPENDSGIR, DABT_WORD);
+ if ( rank == NULL) goto read_as_zero;
+ vgic_lock_rank(v, rank);
+ *r = vgic_byte_read(rank->pendsgi, dabt.sign, gicd_reg);
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ /* Implementation defined -- read as zero */
+ case 0xfd0 ... 0xfe4:
+ goto read_as_zero;
+
+ case GICD_ICPIDR2:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ printk("vGICD: unhandled read from ICPIDR2\n");
+ return 0;
+
+ /* Implementation defined -- read as zero */
+ case 0xfec ... 0xffc:
+ goto read_as_zero;
+
+ /* Reserved -- read as zero */
+ case 0x00c ... 0x01c:
+ case 0x040 ... 0x07c:
+ case 0x7fc:
+ case 0xbfc:
+ case 0xf04 ... 0xf0c:
+ case 0xf30 ... 0xfcc:
+ goto read_as_zero;
+
+ default:
+ printk("vGICD: unhandled read r%d offset %#08x\n",
+ dabt.reg, gicd_reg);
+ return 0;
+ }
+
+bad_width:
+ printk("vGICD: bad read width %d r%d offset %#08x\n",
+ dabt.size, dabt.reg, gicd_reg);
+ domain_crash_synchronous();
+ return 0;
+
+read_as_zero:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ *r = 0;
+ return 1;
+}
+
+static int vgic_v2_to_sgi(struct vcpu *v, register_t sgir)
+{
+
+ int virq;
+ int irqmode;
+ enum gic_sgi_mode sgi_mode;
+ unsigned long vcpu_mask = 0;
+
+ irqmode = (sgir & GICD_SGI_TARGET_LIST_MASK) >> GICD_SGI_TARGET_LIST_SHIFT;
+ virq = (sgir & GICD_SGI_INTID_MASK);
+ vcpu_mask = (sgir & GICD_SGI_TARGET_MASK) >> GICD_SGI_TARGET_SHIFT;
+
+ /* Map GIC sgi value to enum value */
+ switch ( irqmode )
+ {
+ case GICD_SGI_TARGET_LIST_VAL:
+ sgi_mode = SGI_TARGET_LIST;
+ break;
+ case GICD_SGI_TARGET_OTHERS_VAL:
+ sgi_mode = SGI_TARGET_OTHERS;
+ break;
+ case GICD_SGI_TARGET_SELF_VAL:
+ sgi_mode = SGI_TARGET_SELF;
+ break;
+ default:
+ BUG();
+ }
+
+ return vgic_to_sgi(v, sgir, sgi_mode, virq, vcpu_mask);
+}
+
+static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
+{
+ struct hsr_dabt dabt = info->dabt;
+ struct cpu_user_regs *regs = guest_cpu_user_regs();
+ register_t *r = select_user_reg(regs, dabt.reg);
+ struct vgic_irq_rank *rank;
+ int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
+ uint32_t tr;
+
+ switch ( gicd_reg )
+ {
+ case GICD_CTLR:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ /* Ignore all but the enable bit */
+ v->domain->arch.vgic.ctlr = (*r) & GICD_CTL_ENABLE;
+ return 1;
+
+ /* R/O -- write ignored */
+ case GICD_TYPER:
+ case GICD_IIDR:
+ goto write_ignore;
+
+ /* Implementation defined -- write ignored */
+ case 0x020 ... 0x03c:
+ goto write_ignore;
+
+ case GICD_IGROUPR ... GICD_IGROUPRN:
+ /* We do not implement security extensions for guests, write ignore */
+ goto write_ignore;
+
+ case GICD_ISENABLER ... GICD_ISENABLERN:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISENABLER, DABT_WORD);
+ if ( rank == NULL) goto write_ignore;
+ vgic_lock_rank(v, rank);
+ tr = rank->ienable;
+ rank->ienable |= *r;
+ vgic_unlock_rank(v, rank);
+ /* The virtual irq is derived from register offset.
+ * The register difference is word difference. So divide by 2(DABT_WORD)
+ * to get Virtual irq number */
+ vgic_enable_irqs(v, (*r) & (~tr),
+ (gicd_reg - GICD_ISENABLER) >> DABT_WORD);
+ return 1;
+
+ case GICD_ICENABLER ... GICD_ICENABLERN:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICENABLER, DABT_WORD);
+ if ( rank == NULL) goto write_ignore;
+ vgic_lock_rank(v, rank);
+ tr = rank->ienable;
+ rank->ienable &= ~*r;
+ vgic_unlock_rank(v, rank);
+ /* The virtual irq is derived from register offset.
+ * The register difference is word difference. So divide by 2(DABT_WORD)
+ * to get Virtual irq number */
+ vgic_disable_irqs(v, (*r) & tr,
+ (gicd_reg - GICD_ICENABLER) >> DABT_WORD);
+ return 1;
+
+ case GICD_ISPENDR ... GICD_ISPENDRN:
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ printk("vGICD: unhandled %s write %#"PRIregister" to ISPENDR%d\n",
+ dabt.size ? "word" : "byte", *r, gicd_reg - GICD_ISPENDR);
+ return 0;
+
+ case GICD_ICPENDR ... GICD_ICPENDRN:
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ printk("vGICD: unhandled %s write %#"PRIregister" to ICPENDR%d\n",
+ dabt.size ? "word" : "byte", *r, gicd_reg - GICD_ICPENDR);
+ return 0;
+
+ case GICD_ISACTIVER ... GICD_ISACTIVERN:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISACTIVER, DABT_WORD);
+ if ( rank == NULL) goto write_ignore;
+ vgic_lock_rank(v, rank);
+ rank->iactive &= ~*r;
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_ICACTIVER ... GICD_ICACTIVERN:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICACTIVER, DABT_WORD);
+ if ( rank == NULL) goto write_ignore;
+ vgic_lock_rank(v, rank);
+ rank->iactive &= ~*r;
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_ITARGETSR ... GICD_ITARGETSR + 7:
+ /* SGI/PPI target is read only */
+ goto write_ignore;
+
+ case GICD_ITARGETSR + 8 ... GICD_ITARGETSRN:
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
+ if ( rank == NULL) goto write_ignore;
+ vgic_lock_rank(v, rank);
+ if ( dabt.size == DABT_WORD )
+ rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR,
+ DABT_WORD)] = *r;
+ else
+ vgic_byte_write(&rank->itargets[REG_RANK_INDEX(8,
+ gicd_reg - GICD_ITARGETSR, DABT_WORD)], *r, gicd_reg);
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 8, gicd_reg - GICD_IPRIORITYR, DABT_WORD);
+ if ( rank == NULL) goto write_ignore;
+ vgic_lock_rank(v, rank);
+ if ( dabt.size == DABT_WORD )
+ rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
+ DABT_WORD)] = *r;
+ else
+ vgic_byte_write(&rank->ipriority[REG_RANK_INDEX(8,
+ gicd_reg - GICD_IPRIORITYR, DABT_WORD)], *r, gicd_reg);
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_ICFGR: /* SGIs */
+ goto write_ignore;
+ case GICD_ICFGR + 1: /* PPIs */
+ /* It is implementation defined if these are writeable. We chose not */
+ goto write_ignore;
+ case GICD_ICFGR + 2 ... GICD_ICFGRN: /* SPIs */
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
+ if ( rank == NULL) goto write_ignore;
+ vgic_lock_rank(v, rank);
+ rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)] = *r;
+ vgic_unlock_rank(v, rank);
+ return 1;
+
+ case GICD_NSACR ... GICD_NSACRN:
+ /* We do not implement security extensions for guests, write ignore */
+ goto write_ignore;
+
+ case GICD_SGIR:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ return vgic_send_sgi(v, *r);
+
+ case GICD_CPENDSGIR ... GICD_CPENDSGIRN:
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ printk("vGICD: unhandled %s write %#"PRIregister" to ICPENDSGIR%d\n",
+ dabt.size ? "word" : "byte", *r, gicd_reg - GICD_CPENDSGIR);
+ return 0;
+
+ case GICD_SPENDSGIR ... GICD_SPENDSGIRN:
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
+ printk("vGICD: unhandled %s write %#"PRIregister" to ISPENDSGIR%d\n",
+ dabt.size ? "word" : "byte", *r, gicd_reg - GICD_SPENDSGIR);
+ return 0;
+
+ /* Implementation defined -- write ignored */
+ case 0xfd0 ... 0xfe4:
+ goto write_ignore;
+
+ /* R/O -- write ignore */
+ case GICD_ICPIDR2:
+ goto write_ignore;
+
+ /* Implementation defined -- write ignored */
+ case 0xfec ... 0xffc:
+ goto write_ignore;
+
+ /* Reserved -- write ignored */
+ case 0x00c ... 0x01c:
+ case 0x040 ... 0x07c:
+ case 0x7fc:
+ case 0xbfc:
+ case 0xf04 ... 0xf0c:
+ case 0xf30 ... 0xfcc:
+ goto write_ignore;
+
+ default:
+ printk("vGICD: unhandled write r%d=%"PRIregister" offset %#08x\n",
+ dabt.reg, *r, gicd_reg);
+ return 0;
+ }
+
+bad_width:
+ printk("vGICD: bad write width %d r%d=%"PRIregister" offset %#08x\n",
+ dabt.size, dabt.reg, *r, gicd_reg);
+ domain_crash_synchronous();
+ return 0;
+
+write_ignore:
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+ return 1;
+}
+
+static const struct mmio_handler_ops vgic_v2_distr_mmio_handler = {
+ .read_handler = vgic_v2_distr_mmio_read,
+ .write_handler = vgic_v2_distr_mmio_write,
+};
+
+static int vgic_v2_vcpu_init(struct vcpu *v)
+{
+ int i;
+
+ /* For SGI and PPI the target is always this CPU */
+ for ( i = 0 ; i < 8 ; i++ )
+ v->arch.vgic.private_irqs->itargets[i] =
+ (1<<(v->vcpu_id+0))
+ | (1<<(v->vcpu_id+8))
+ | (1<<(v->vcpu_id+16))
+ | (1<<(v->vcpu_id+24));
+
+ return 0;
+}
+
+static int vgic_v2_domain_init(struct domain *d)
+{
+ /* 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);
+
+ return 0;
+}
+
+static const struct vgic_ops vgic_v2_ops = {
+ .vcpu_init = vgic_v2_vcpu_init,
+ .domain_init = vgic_v2_domain_init,
+ .send_sgi = vgic_v2_to_sgi,
+};
+
+int vgic_v2_init(struct domain *d)
+{
+ register_vgic_ops(d, &vgic_v2_ops);
+
+ return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index ed21f62..3ccabf4 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -31,14 +31,11 @@
#include <asm/gic.h>
#include <asm/vgic.h>
-static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info);
-static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info);
-
/*
* Returns rank corresponding to a GICD_<FOO><n> register for
* GICD_<FOO> with <b>-bits-per-interrupt.
*/
-static struct vgic_irq_rank *vgic_rank_offset(struct vcpu *v, int b, int n,
+struct vgic_irq_rank *vgic_rank_offset(struct vcpu *v, int b, int n,
int s)
{
int rank = REG_RANK_NR(b, (n >> s));
@@ -56,11 +53,6 @@ static struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned int irq)
return vgic_rank_offset(v, 8, irq, DABT_WORD);
}
-static const struct mmio_handler_ops vgic_distr_mmio_handler = {
- .read_handler = vgic_distr_mmio_read,
- .write_handler = vgic_distr_mmio_write,
-};
-
int domain_vgic_init(struct domain *d)
{
int i;
@@ -75,6 +67,15 @@ int domain_vgic_init(struct domain *d)
else
d->arch.vgic.nr_lines = 0; /* We don't need SPIs for the guest */
+ switch ( gic_hw_version() )
+ {
+ case GIC_V2:
+ vgic_v2_init(d);
+ break;
+ default:
+ return -ENODEV;
+ }
+
d->arch.vgic.shared_irqs =
xzalloc_array(struct vgic_irq_rank, DOMAIN_NR_RANKS(d));
if ( d->arch.vgic.shared_irqs == NULL )
@@ -96,15 +97,16 @@ int domain_vgic_init(struct domain *d)
for (i=0; i<DOMAIN_NR_RANKS(d); i++)
spin_lock_init(&d->arch.vgic.shared_irqs[i].lock);
- /*
- * We rely on gicv_setup() to initialize dbase(vGIC distributor base)
- */
- register_mmio_handler(d, &vgic_distr_mmio_handler,
- d->arch.vgic.dbase, PAGE_SIZE);
+ d->arch.vgic.handler->domain_init(d);
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)
{
xfree(d->arch.vgic.shared_irqs);
@@ -121,6 +123,8 @@ int vcpu_vgic_init(struct vcpu *v)
spin_lock_init(&v->arch.vgic.private_irqs->lock);
+ v->domain->arch.vgic.handler->vcpu_init(v);
+
memset(&v->arch.vgic.pending_irqs, 0, sizeof(v->arch.vgic.pending_irqs));
for (i = 0; i < 32; i++)
{
@@ -128,13 +132,6 @@ int vcpu_vgic_init(struct vcpu *v)
INIT_LIST_HEAD(&v->arch.vgic.pending_irqs[i].lr_queue);
}
- /* For SGI and PPI the target is always this CPU */
- for ( i = 0 ; i < 8 ; i++ )
- v->arch.vgic.private_irqs->itargets[i] =
- (1<<(v->vcpu_id+0))
- | (1<<(v->vcpu_id+8))
- | (1<<(v->vcpu_id+16))
- | (1<<(v->vcpu_id+24));
INIT_LIST_HEAD(&v->arch.vgic.inflight_irqs);
INIT_LIST_HEAD(&v->arch.vgic.lr_pending);
spin_lock_init(&v->arch.vgic.lock);
@@ -148,205 +145,7 @@ int vcpu_vgic_free(struct vcpu *v)
return 0;
}
-static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
-{
- struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
- struct vgic_irq_rank *rank;
- int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
-
- switch ( gicd_reg )
- {
- case GICD_CTLR:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- vgic_lock(v);
- *r = v->domain->arch.vgic.ctlr;
- vgic_unlock(v);
- return 1;
- case GICD_TYPER:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- /* No secure world support for guests. */
- vgic_lock(v);
- *r = ( (v->domain->max_vcpus<<5) & GICD_TYPE_CPUS )
- |( ((v->domain->arch.vgic.nr_lines/32)) & GICD_TYPE_LINES );
- vgic_unlock(v);
- return 1;
- case GICD_IIDR:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- /*
- * XXX Do we need a JEP106 manufacturer ID?
- * Just use the physical h/w value for now
- */
- *r = 0x0000043b;
- return 1;
-
- /* Implementation defined -- read as zero */
- case 0x020 ... 0x03c:
- goto read_as_zero;
-
- case GICD_IGROUPR ... GICD_IGROUPRN:
- /* We do not implement security extensions for guests, read zero */
- goto read_as_zero;
-
- case GICD_ISENABLER ... GICD_ISENABLERN:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISENABLER, DABT_WORD);
- if ( rank == NULL) goto read_as_zero;
- vgic_lock_rank(v, rank);
- *r = rank->ienable;
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_ICENABLER ... GICD_ICENABLERN:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICENABLER, DABT_WORD);
- if ( rank == NULL) goto read_as_zero;
- vgic_lock_rank(v, rank);
- *r = rank->ienable;
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_ISPENDR ... GICD_ISPENDRN:
- if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISPENDR, DABT_WORD);
- if ( rank == NULL) goto read_as_zero;
- vgic_lock_rank(v, rank);
- *r = vgic_byte_read(rank->ipend, dabt.sign, gicd_reg);
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_ICPENDR ... GICD_ICPENDRN:
- if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICPENDR, DABT_WORD);
- if ( rank == NULL) goto read_as_zero;
- vgic_lock_rank(v, rank);
- *r = vgic_byte_read(rank->ipend, dabt.sign, gicd_reg);
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_ISACTIVER ... GICD_ISACTIVERN:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISACTIVER, DABT_WORD);
- if ( rank == NULL) goto read_as_zero;
- vgic_lock_rank(v, rank);
- *r = rank->iactive;
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_ICACTIVER ... GICD_ICACTIVERN:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICACTIVER, DABT_WORD);
- if ( rank == NULL) goto read_as_zero;
- vgic_lock_rank(v, rank);
- *r = rank->iactive;
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_ITARGETSR ... GICD_ITARGETSRN:
- if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
- if ( rank == NULL) goto read_as_zero;
-
- vgic_lock_rank(v, rank);
- *r = rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR,
- DABT_WORD)];
- if ( dabt.size == DABT_BYTE )
- *r = vgic_byte_read(*r, dabt.sign, gicd_reg);
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
- if ( dabt.size != 0 && dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 8, gicd_reg - GICD_IPRIORITYR, DABT_WORD);
- if ( rank == NULL) goto read_as_zero;
-
- vgic_lock_rank(v, rank);
- *r = rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
- DABT_WORD)];
- if ( dabt.size == DABT_BYTE )
- *r = vgic_byte_read(*r, dabt.sign, gicd_reg);
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_ICFGR ... GICD_ICFGRN:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
- if ( rank == NULL) goto read_as_zero;
- vgic_lock_rank(v, rank);
- *r = rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)];
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_NSACR ... GICD_NSACRN:
- /* We do not implement security extensions for guests, read zero */
- goto read_as_zero;
-
- case GICD_SGIR:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- /* Write only -- read unknown */
- *r = 0xdeadbeef;
- return 1;
-
- case GICD_CPENDSGIR ... GICD_CPENDSGIRN:
- if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_CPENDSGIR, DABT_WORD);
- if ( rank == NULL) goto read_as_zero;
- vgic_lock_rank(v, rank);
- *r = vgic_byte_read(rank->pendsgi, dabt.sign, gicd_reg);
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_SPENDSGIR ... GICD_SPENDSGIRN:
- if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_SPENDSGIR, DABT_WORD);
- if ( rank == NULL) goto read_as_zero;
- vgic_lock_rank(v, rank);
- *r = vgic_byte_read(rank->pendsgi, dabt.sign, gicd_reg);
- vgic_unlock_rank(v, rank);
- return 1;
-
- /* Implementation defined -- read as zero */
- case 0xfd0 ... 0xfe4:
- goto read_as_zero;
-
- case GICD_ICPIDR2:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- printk("vGICD: unhandled read from ICPIDR2\n");
- return 0;
-
- /* Implementation defined -- read as zero */
- case 0xfec ... 0xffc:
- goto read_as_zero;
-
- /* Reserved -- read as zero */
- case 0x00c ... 0x01c:
- case 0x040 ... 0x07c:
- case 0x7fc:
- case 0xbfc:
- case 0xf04 ... 0xf0c:
- case 0xf30 ... 0xfcc:
- goto read_as_zero;
-
- default:
- printk("vGICD: unhandled read r%d offset %#08x\n",
- dabt.reg, gicd_reg);
- return 0;
- }
-
-bad_width:
- printk("vGICD: bad read width %d r%d offset %#08x\n",
- dabt.size, dabt.reg, gicd_reg);
- domain_crash_synchronous();
- return 0;
-
-read_as_zero:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- *r = 0;
- return 1;
-}
-
-static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
+void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
{
const unsigned long mask = r;
struct pending_irq *p;
@@ -369,7 +168,7 @@ static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
}
}
-static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
+void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
{
const unsigned long mask = r;
struct pending_irq *p;
@@ -405,41 +204,48 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
}
}
-static int vgic_to_sgi(struct vcpu *v, register_t sgir)
+/* TODO: unsigned long is used to fit vcpu_mask.*/
+int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, int virq,
+ unsigned long vcpu_mask)
{
struct domain *d = v->domain;
- int virtual_irq;
- int filter;
int vcpuid;
int i;
- unsigned long vcpu_mask = 0;
ASSERT(d->max_vcpus < 8*sizeof(vcpu_mask));
- filter = (sgir & GICD_SGI_TARGET_LIST_MASK);
- virtual_irq = (sgir & GICD_SGI_INTID_MASK);
- ASSERT( virtual_irq < 16 );
+ ASSERT( virq < 16 );
- switch ( filter )
+ switch ( irqmode )
{
- case GICD_SGI_TARGET_LIST:
- vcpu_mask = (sgir & GICD_SGI_TARGET_MASK) >> GICD_SGI_TARGET_SHIFT;
- break;
- case GICD_SGI_TARGET_OTHERS:
- for ( i = 0; i < d->max_vcpus; i++ )
- {
- if ( i != current->vcpu_id && d->vcpu != NULL &&
- d->vcpu[i] != NULL && is_vcpu_online(d->vcpu[i]) )
- set_bit(i, &vcpu_mask);
- }
- break;
- case GICD_SGI_TARGET_SELF:
- set_bit(current->vcpu_id, &vcpu_mask);
- break;
- default:
- gdprintk(XENLOG_WARNING, "vGICD: unhandled GICD_SGIR write %"PRIregister" with wrong TargetListFilter field\n",
- sgir);
- return 0;
+ case SGI_TARGET_LIST:
+ break;
+ case SGI_TARGET_OTHERS:
+ /*
+ * We expect vcpu_mask to be 0 for SGI_TARGET_OTHERS and
+ * SGI_TARGET_SELF mode. So Force vcpu_mask to 0
+ */
+ vcpu_mask = 0;
+ for ( i = 0; i < d->max_vcpus; i++ )
+ {
+ if ( i != current->vcpu_id && d->vcpu != NULL &&
+ d->vcpu[i] != NULL && is_vcpu_online(d->vcpu[i]) )
+ set_bit(i, &vcpu_mask);
+ }
+ break;
+ case SGI_TARGET_SELF:
+ /*
+ * We expect vcpu_mask to be 0 for SGI_TARGET_OTHERS and
+ * SGI_TARGET_SELF mode. So Force vcpu_mask to 0
+ */
+ vcpu_mask = 0;
+ set_bit(current->vcpu_id, &vcpu_mask);
+ break;
+ default:
+ gdprintk(XENLOG_WARNING,
+ "vGICD:unhandled GICD_SGIR write %"PRIregister" \
+ with wrong mode\n", sgir);
+ return 0;
}
for_each_set_bit( vcpuid, &vcpu_mask, d->max_vcpus )
@@ -447,206 +253,19 @@ static int vgic_to_sgi(struct vcpu *v, register_t sgir)
if ( d->vcpu != NULL && d->vcpu[vcpuid] != NULL &&
!is_vcpu_online(d->vcpu[vcpuid]) )
{
- gdprintk(XENLOG_WARNING, "vGICD: GICD_SGIR write r=%"PRIregister" vcpu_mask=%lx, wrong CPUTargetList\n",
- sgir, vcpu_mask);
+ gdprintk(XENLOG_WARNING, "VGIC: write r=%"PRIregister" \
+ vcpu_mask=%lx, wrong CPUTargetList\n", sgir, vcpu_mask);
continue;
}
- vgic_vcpu_inject_irq(d->vcpu[vcpuid], virtual_irq);
+ vgic_vcpu_inject_irq(d->vcpu[vcpuid], virq);
}
return 1;
}
-static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
+int vgic_send_sgi(struct vcpu *v, register_t sgir)
{
- struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
- struct vgic_irq_rank *rank;
- int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
- uint32_t tr;
-
- switch ( gicd_reg )
- {
- case GICD_CTLR:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- /* Ignore all but the enable bit */
- v->domain->arch.vgic.ctlr = (*r) & GICD_CTL_ENABLE;
- return 1;
-
- /* R/O -- write ignored */
- case GICD_TYPER:
- case GICD_IIDR:
- goto write_ignore;
-
- /* Implementation defined -- write ignored */
- case 0x020 ... 0x03c:
- goto write_ignore;
-
- case GICD_IGROUPR ... GICD_IGROUPRN:
- /* We do not implement security extensions for guests, write ignore */
- goto write_ignore;
-
- case GICD_ISENABLER ... GICD_ISENABLERN:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISENABLER, DABT_WORD);
- if ( rank == NULL) goto write_ignore;
- vgic_lock_rank(v, rank);
- tr = rank->ienable;
- rank->ienable |= *r;
- vgic_unlock_rank(v, rank);
- vgic_enable_irqs(v, (*r) & (~tr),
- (gicd_reg - GICD_ISENABLER) >> DABT_WORD);
- return 1;
-
- case GICD_ICENABLER ... GICD_ICENABLERN:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICENABLER, DABT_WORD);
- if ( rank == NULL) goto write_ignore;
- vgic_lock_rank(v, rank);
- tr = rank->ienable;
- rank->ienable &= ~*r;
- vgic_unlock_rank(v, rank);
- vgic_disable_irqs(v, (*r) & tr,
- (gicd_reg - GICD_ICENABLER) >> DABT_WORD);
- return 1;
-
- case GICD_ISPENDR ... GICD_ISPENDRN:
- if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
- printk("vGICD: unhandled %s write %#"PRIregister" to ISPENDR%d\n",
- dabt.size ? "word" : "byte", *r, gicd_reg - GICD_ISPENDR);
- return 0;
-
- case GICD_ICPENDR ... GICD_ICPENDRN:
- if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
- printk("vGICD: unhandled %s write %#"PRIregister" to ICPENDR%d\n",
- dabt.size ? "word" : "byte", *r, gicd_reg - GICD_ICPENDR);
- return 0;
-
- case GICD_ISACTIVER ... GICD_ISACTIVERN:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISACTIVER, DABT_WORD);
- if ( rank == NULL) goto write_ignore;
- vgic_lock_rank(v, rank);
- rank->iactive &= ~*r;
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_ICACTIVER ... GICD_ICACTIVERN:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICACTIVER, DABT_WORD);
- if ( rank == NULL) goto write_ignore;
- vgic_lock_rank(v, rank);
- rank->iactive &= ~*r;
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_ITARGETSR ... GICD_ITARGETSR + 7:
- /* SGI/PPI target is read only */
- goto write_ignore;
-
- case GICD_ITARGETSR + 8 ... GICD_ITARGETSRN:
- if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
- if ( rank == NULL) goto write_ignore;
- vgic_lock_rank(v, rank);
- if ( dabt.size == DABT_WORD )
- rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR,
- DABT_WORD)] = *r;
- else
- {
- tr = REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
- vgic_byte_write(&rank->itargets[tr], *r, gicd_reg);
- }
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
- if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 8, gicd_reg - GICD_IPRIORITYR, DABT_WORD);
- if ( rank == NULL) goto write_ignore;
- vgic_lock_rank(v, rank);
- if ( dabt.size == DABT_WORD )
- rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
- DABT_WORD)] = *r;
- else
- {
- tr = REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR, DABT_WORD);
- vgic_byte_write(&rank->ipriority[tr], *r, gicd_reg);
- }
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_ICFGR: /* SGIs */
- goto write_ignore;
- case GICD_ICFGR + 1: /* PPIs */
- /* It is implementation defined if these are writeable. We chose not */
- goto write_ignore;
- case GICD_ICFGR + 2 ... GICD_ICFGRN: /* SPIs */
- if ( dabt.size != DABT_WORD ) goto bad_width;
- rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
- if ( rank == NULL) goto write_ignore;
- vgic_lock_rank(v, rank);
- rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)] = *r;
- vgic_unlock_rank(v, rank);
- return 1;
-
- case GICD_NSACR ... GICD_NSACRN:
- /* We do not implement security extensions for guests, write ignore */
- goto write_ignore;
-
- case GICD_SGIR:
- if ( dabt.size != 2 )
- goto bad_width;
- return vgic_to_sgi(v, *r);
-
- case GICD_CPENDSGIR ... GICD_CPENDSGIRN:
- if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
- printk("vGICD: unhandled %s write %#"PRIregister" to ICPENDSGIR%d\n",
- dabt.size ? "word" : "byte", *r, gicd_reg - GICD_CPENDSGIR);
- return 0;
-
- case GICD_SPENDSGIR ... GICD_SPENDSGIRN:
- if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
- printk("vGICD: unhandled %s write %#"PRIregister" to ISPENDSGIR%d\n",
- dabt.size ? "word" : "byte", *r, gicd_reg - GICD_SPENDSGIR);
- return 0;
-
- /* Implementation defined -- write ignored */
- case 0xfd0 ... 0xfe4:
- goto write_ignore;
-
- /* R/O -- write ignore */
- case GICD_ICPIDR2:
- goto write_ignore;
-
- /* Implementation defined -- write ignored */
- case 0xfec ... 0xffc:
- goto write_ignore;
-
- /* Reserved -- write ignored */
- case 0x00c ... 0x01c:
- case 0x040 ... 0x07c:
- case 0x7fc:
- case 0xbfc:
- case 0xf04 ... 0xf0c:
- case 0xf30 ... 0xfcc:
- goto write_ignore;
-
- default:
- printk("vGICD: unhandled write r%d=%"PRIregister" offset %#08x\n",
- dabt.reg, *r, gicd_reg);
- return 0;
- }
-
-bad_width:
- printk("vGICD: bad write width %d r%d=%"PRIregister" offset %#08x\n",
- dabt.size, dabt.reg, *r, gicd_reg);
- domain_crash_synchronous();
- return 0;
-
-write_ignore:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- return 1;
+ const struct domain *d = v->domain;
+ return d->arch.vgic.handler->send_sgi(v, sgir);
}
struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq)
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 2a1e976..32d0554 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -76,6 +76,8 @@ struct arch_domain
} virt_timer_base;
struct {
+ /* GIC HW version specific vGIC driver handler */
+ const struct vgic_ops *handler;
/*
* Covers access to other members of this struct _except_ for
* shared_irqs where each member contains its own locking.
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 875729e..a0c07bf 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -53,8 +53,11 @@
#define GICD_SGI_TARGET_LIST_SHIFT (24)
#define GICD_SGI_TARGET_LIST_MASK (0x3UL << GICD_SGI_TARGET_LIST_SHIFT)
#define GICD_SGI_TARGET_LIST (0UL<<GICD_SGI_TARGET_LIST_SHIFT)
+#define GICD_SGI_TARGET_LIST_VAL (0)
#define GICD_SGI_TARGET_OTHERS (1UL<<GICD_SGI_TARGET_LIST_SHIFT)
+#define GICD_SGI_TARGET_OTHERS_VAL (1)
#define GICD_SGI_TARGET_SELF (2UL<<GICD_SGI_TARGET_LIST_SHIFT)
+#define GICD_SGI_TARGET_SELF_VAL (2)
#define GICD_SGI_TARGET_SHIFT (16)
#define GICD_SGI_TARGET_MASK (0xFFUL<<GICD_SGI_TARGET_SHIFT)
#define GICD_SGI_GROUP1 (1UL<<15)
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 7c71d16..d310ca1 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -86,6 +86,15 @@ struct vgic_irq_rank {
uint32_t itargets[8];
};
+struct vgic_ops {
+ /* Initialize vGIC */
+ int (*vcpu_init)(struct vcpu *v);
+ /* Domain specific initialization of vGIC */
+ int (*domain_init)(struct domain *d);
+ /* SGI handler of vGIC */
+ int (*send_sgi)(struct vcpu *v, register_t sgir);
+};
+
/* Number of ranks of interrupt registers for a domain */
#define DOMAIN_NR_RANKS(d) (((d)->arch.vgic.nr_lines+31)/32)
@@ -133,6 +142,8 @@ static inline void vgic_byte_write(uint32_t *reg, uint32_t var, int offset)
*reg |= var;
}
+enum gic_sgi_mode;
+
/*
* Offset of GICD_<FOO><n> with its rank, for GICD_<FOO> size <s> with
* <b>-bits-per-interrupt.
@@ -145,8 +156,17 @@ extern int vcpu_vgic_init(struct vcpu *v);
extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq);
extern void vgic_clear_pending_irqs(struct vcpu *v);
extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq);
+extern struct vgic_irq_rank *vgic_rank_offset(struct vcpu *v, int b, int n, int s);
+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);
extern int vcpu_vgic_free(struct vcpu *v);
+extern int vgic_to_sgi(struct vcpu *v, register_t sgir,
+ enum gic_sgi_mode irqmode, int virq,
+ unsigned long vcpu_mask);
+extern int vgic_send_sgi(struct vcpu *v, register_t sgir);
#endif /* __ASM_ARM_VGIC_H__ */
/*
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver
2014-07-03 8:34 ` [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver vijay.kilari
@ 2014-07-03 12:57 ` Julien Grall
2014-07-03 13:02 ` Ian Campbell
0 siblings, 1 reply; 19+ messages in thread
From: Julien Grall @ 2014-07-03 12:57 UTC (permalink / raw)
To: vijay.kilari, Ian.Campbell, stefano.stabellini,
stefano.stabellini, tim, xen-devel
Cc: Prasun.Kapoor, vijaya.kumar
On 07/03/2014 09:34 AM, vijay.kilari@gmail.com wrote:
> int domain_vgic_init(struct domain *d)
> {
> int i;
> @@ -75,6 +67,15 @@ int domain_vgic_init(struct domain *d)
> else
> d->arch.vgic.nr_lines = 0; /* We don't need SPIs for the guest */
>
> + switch ( gic_hw_version() )
> + {
> + case GIC_V2:
> + vgic_v2_init(d);
4th time:
vgic_v2_init can return an error, even though it's not the case right
now. Please check the return.
[..]
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index 7c71d16..d310ca1 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
[..]
> +struct vgic_ops {
> + /* Initialize vGIC */
> + int (*vcpu_init)(struct vcpu *v);
> + /* Domain specific initialization of vGIC */
> + int (*domain_init)(struct domain *d);
> + /* SGI handler of vGIC */
> + int (*send_sgi)(struct vcpu *v, register_t sgir);
By reviewing the VGIC-v3 support, I still don't think this is the right
callback to add. You bypass the VGIC common emulation with your
vgic_emulate...
I would introduce a callback to emulate_sysreg rather than this send_sgi.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver
2014-07-03 12:57 ` Julien Grall
@ 2014-07-03 13:02 ` Ian Campbell
2014-07-03 13:25 ` Julien Grall
0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2014-07-03 13:02 UTC (permalink / raw)
To: Julien Grall
Cc: vijay.kilari, stefano.stabellini, Prasun.Kapoor, vijaya.kumar,
tim, xen-devel, stefano.stabellini
On Thu, 2014-07-03 at 13:57 +0100, Julien Grall wrote:
> > +struct vgic_ops {
> > + /* Initialize vGIC */
> > + int (*vcpu_init)(struct vcpu *v);
> > + /* Domain specific initialization of vGIC */
> > + int (*domain_init)(struct domain *d);
> > + /* SGI handler of vGIC */
> > + int (*send_sgi)(struct vcpu *v, register_t sgir);
>
> By reviewing the VGIC-v3 support, I still don't think this is the right
> callback to add. You bypass the VGIC common emulation with your
> vgic_emulate...
>
> I would introduce a callback to emulate_sysreg rather than this send_sgi.
Why? The vgic will either be v2 or v3, so either MMIO or sysreg, once
the thing has been decoded then you want to send an SGI I think, hence
the callback. Passing a register_t does seem odd though, I'd have
thought it would take an SGI number and any other flags which would then
be interpreted for either v2 or v3 as appropriate.
Although thinking about it now: Given that a guest can't mix gic v2 and
v3 why is the send sgi exposed outside of the respective files at all? I
suppose it's because it goes via the core gic code?
Ian.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver
2014-07-03 13:02 ` Ian Campbell
@ 2014-07-03 13:25 ` Julien Grall
2014-07-03 14:02 ` Ian Campbell
0 siblings, 1 reply; 19+ messages in thread
From: Julien Grall @ 2014-07-03 13:25 UTC (permalink / raw)
To: Ian Campbell
Cc: vijay.kilari, stefano.stabellini, Prasun.Kapoor, vijaya.kumar,
tim, xen-devel, stefano.stabellini
On 07/03/2014 02:02 PM, Ian Campbell wrote:
> On Thu, 2014-07-03 at 13:57 +0100, Julien Grall wrote:
>
>>> +struct vgic_ops {
>>> + /* Initialize vGIC */
>>> + int (*vcpu_init)(struct vcpu *v);
>>> + /* Domain specific initialization of vGIC */
>>> + int (*domain_init)(struct domain *d);
>>> + /* SGI handler of vGIC */
>>> + int (*send_sgi)(struct vcpu *v, register_t sgir);
>>
>> By reviewing the VGIC-v3 support, I still don't think this is the right
>> callback to add. You bypass the VGIC common emulation with your
>> vgic_emulate...
>>
>> I would introduce a callback to emulate_sysreg rather than this send_sgi.
>
> Why? The vgic will either be v2 or v3, so either MMIO or sysreg, once
> the thing has been decoded then you want to send an SGI I think, hence
> the callback. Passing a register_t does seem odd though, I'd have
> thought it would take an SGI number and any other flags which would then
> be interpreted for either v2 or v3 as appropriate.
The decoding depends on the vgic emulation. For now this function is
badly implement in vgic-v3.c.
What I was trying to say is send_sgi can be handled internally. If you
are looking to the calls of this function, it's only happen within the
file vgic-v2.c (or vgic-v3.c)
But, the sysreg emulation is called outside the vgic code. So we should
add a callaback for this.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver
2014-07-03 13:25 ` Julien Grall
@ 2014-07-03 14:02 ` Ian Campbell
2014-07-03 14:21 ` Julien Grall
0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2014-07-03 14:02 UTC (permalink / raw)
To: Julien Grall
Cc: vijay.kilari, stefano.stabellini, Prasun.Kapoor, vijaya.kumar,
tim, xen-devel, stefano.stabellini
On Thu, 2014-07-03 at 14:25 +0100, Julien Grall wrote:
> On 07/03/2014 02:02 PM, Ian Campbell wrote:
> > On Thu, 2014-07-03 at 13:57 +0100, Julien Grall wrote:
> >
> >>> +struct vgic_ops {
> >>> + /* Initialize vGIC */
> >>> + int (*vcpu_init)(struct vcpu *v);
> >>> + /* Domain specific initialization of vGIC */
> >>> + int (*domain_init)(struct domain *d);
> >>> + /* SGI handler of vGIC */
> >>> + int (*send_sgi)(struct vcpu *v, register_t sgir);
> >>
> >> By reviewing the VGIC-v3 support, I still don't think this is the right
> >> callback to add. You bypass the VGIC common emulation with your
> >> vgic_emulate...
> >>
> >> I would introduce a callback to emulate_sysreg rather than this send_sgi.
> >
> > Why? The vgic will either be v2 or v3, so either MMIO or sysreg, once
> > the thing has been decoded then you want to send an SGI I think, hence
> > the callback. Passing a register_t does seem odd though, I'd have
> > thought it would take an SGI number and any other flags which would then
> > be interpreted for either v2 or v3 as appropriate.
>
> The decoding depends on the vgic emulation. For now this function is
> badly implement in vgic-v3.c.
>
> What I was trying to say is send_sgi can be handled internally. If you
> are looking to the calls of this function, it's only happen within the
> file vgic-v2.c (or vgic-v3.c)
>
> But, the sysreg emulation is called outside the vgic code. So we should
> add a callaback for this.
So the common code would have
case HSR_SYSREG_ICC_SGI0R:
gic->handle_sysreg(esr, val)
instead of
case HSR_SYSREG_ICC_SGI0R:
gic->handle_sysreg(val);
?
That might be nicer, but TBH given that there is only one trappable gic
sysreg right now I don't think it is worth getting too worried about. We
can always rework this interface when gic v4 or v5 needs something more.
Ian.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver
2014-07-03 14:02 ` Ian Campbell
@ 2014-07-03 14:21 ` Julien Grall
2014-07-03 15:18 ` Ian Campbell
0 siblings, 1 reply; 19+ messages in thread
From: Julien Grall @ 2014-07-03 14:21 UTC (permalink / raw)
To: Ian Campbell
Cc: vijay.kilari, stefano.stabellini, Prasun.Kapoor, vijaya.kumar,
tim, xen-devel, stefano.stabellini
On 07/03/2014 03:02 PM, Ian Campbell wrote:
> On Thu, 2014-07-03 at 14:25 +0100, Julien Grall wrote:
>> On 07/03/2014 02:02 PM, Ian Campbell wrote:
>>> On Thu, 2014-07-03 at 13:57 +0100, Julien Grall wrote:
>>>
>>>>> +struct vgic_ops {
>>>>> + /* Initialize vGIC */
>>>>> + int (*vcpu_init)(struct vcpu *v);
>>>>> + /* Domain specific initialization of vGIC */
>>>>> + int (*domain_init)(struct domain *d);
>>>>> + /* SGI handler of vGIC */
>>>>> + int (*send_sgi)(struct vcpu *v, register_t sgir);
>>>>
>>>> By reviewing the VGIC-v3 support, I still don't think this is the right
>>>> callback to add. You bypass the VGIC common emulation with your
>>>> vgic_emulate...
>>>>
>>>> I would introduce a callback to emulate_sysreg rather than this send_sgi.
>>>
>>> Why? The vgic will either be v2 or v3, so either MMIO or sysreg, once
>>> the thing has been decoded then you want to send an SGI I think, hence
>>> the callback. Passing a register_t does seem odd though, I'd have
>>> thought it would take an SGI number and any other flags which would then
>>> be interpreted for either v2 or v3 as appropriate.
>>
>> The decoding depends on the vgic emulation. For now this function is
>> badly implement in vgic-v3.c.
>>
>> What I was trying to say is send_sgi can be handled internally. If you
>> are looking to the calls of this function, it's only happen within the
>> file vgic-v2.c (or vgic-v3.c)
>>
>> But, the sysreg emulation is called outside the vgic code. So we should
>> add a callaback for this.
>
> So the common code would have
> case HSR_SYSREG_ICC_SGI0R:
> gic->handle_sysreg(esr, val)
> instead of
> case HSR_SYSREG_ICC_SGI0R:
> gic->handle_sysreg(val);
> ?
The is not the actual case,
Actually, the common code is:
case HSR_SYSREG_ICC_SGI0R:
vgic_emulate(regs, hsr)
where vgic_emulate is implemented in vgic-v3.c rather than in vgic.c.
The function will decode the register and then call vgic_send_sgi.
But, as the function send_sgi is only used internaly there is no reason
to create a callback.
What I ask is to have a new callback emulate_sysreg. The common code
(i.e traps.c) will have:
case HSR_SYSREG_ICC_SGI0R:
vgic_emulate(regs, hsr).
The function vgic_emulate will be implemented in vgic.c:
vgic_emulate(...)
{
vgic->emulate_sysreg(regs, hsr);
}
The vgic-v3.c will implement the callback correctly.
> That might be nicer, but TBH given that there is only one trappable gic
> sysreg right now I don't think it is worth getting too worried about. We
> can always rework this interface when gic v4 or v5 needs something more.
I don't see why we should break the vgic common implementation as it
does on the next series:
http://www.gossamer-threads.com/lists/xen/devel/337708.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver
2014-07-03 14:21 ` Julien Grall
@ 2014-07-03 15:18 ` Ian Campbell
2014-07-04 7:01 ` Vijay Kilari
0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2014-07-03 15:18 UTC (permalink / raw)
To: Julien Grall
Cc: vijay.kilari, stefano.stabellini, Prasun.Kapoor, vijaya.kumar,
tim, xen-devel, stefano.stabellini
On Thu, 2014-07-03 at 15:21 +0100, Julien Grall wrote:
> On 07/03/2014 03:02 PM, Ian Campbell wrote:
> > On Thu, 2014-07-03 at 14:25 +0100, Julien Grall wrote:
> >> On 07/03/2014 02:02 PM, Ian Campbell wrote:
> >>> On Thu, 2014-07-03 at 13:57 +0100, Julien Grall wrote:
> >>>
> >>>>> +struct vgic_ops {
> >>>>> + /* Initialize vGIC */
> >>>>> + int (*vcpu_init)(struct vcpu *v);
> >>>>> + /* Domain specific initialization of vGIC */
> >>>>> + int (*domain_init)(struct domain *d);
> >>>>> + /* SGI handler of vGIC */
> >>>>> + int (*send_sgi)(struct vcpu *v, register_t sgir);
> >>>>
> >>>> By reviewing the VGIC-v3 support, I still don't think this is the right
> >>>> callback to add. You bypass the VGIC common emulation with your
> >>>> vgic_emulate...
> >>>>
> >>>> I would introduce a callback to emulate_sysreg rather than this send_sgi.
> >>>
> >>> Why? The vgic will either be v2 or v3, so either MMIO or sysreg, once
> >>> the thing has been decoded then you want to send an SGI I think, hence
> >>> the callback. Passing a register_t does seem odd though, I'd have
> >>> thought it would take an SGI number and any other flags which would then
> >>> be interpreted for either v2 or v3 as appropriate.
> >>
> >> The decoding depends on the vgic emulation. For now this function is
> >> badly implement in vgic-v3.c.
> >>
> >> What I was trying to say is send_sgi can be handled internally. If you
> >> are looking to the calls of this function, it's only happen within the
> >> file vgic-v2.c (or vgic-v3.c)
> >>
> >> But, the sysreg emulation is called outside the vgic code. So we should
> >> add a callaback for this.
> >
> > So the common code would have
> > case HSR_SYSREG_ICC_SGI0R:
> > gic->handle_sysreg(esr, val)
> > instead of
> > case HSR_SYSREG_ICC_SGI0R:
> > gic->handle_sysreg(val);
> > ?
>
> The is not the actual case,
>
> Actually, the common code is:
>
> case HSR_SYSREG_ICC_SGI0R:
> vgic_emulate(regs, hsr)
>
> where vgic_emulate is implemented in vgic-v3.c rather than in vgic.c.
> The function will decode the register and then call vgic_send_sgi.
>
> But, as the function send_sgi is only used internaly there is no reason
> to create a callback.
>
> What I ask is to have a new callback emulate_sysreg. The common code
> (i.e traps.c) will have:
>
> case HSR_SYSREG_ICC_SGI0R:
> vgic_emulate(regs, hsr).
>
> The function vgic_emulate will be implemented in vgic.c:
>
> vgic_emulate(...)
> {
> vgic->emulate_sysreg(regs, hsr);
> }
>
> The vgic-v3.c will implement the callback correctly.
I don't mind this but I would be equally happy with vgic_emulate being
in gic-v3.c at this stage without the unnecessary callback via
->send_sgi.
> > That might be nicer, but TBH given that there is only one trappable gic
> > sysreg right now I don't think it is worth getting too worried about. We
> > can always rework this interface when gic v4 or v5 needs something more.
>
> I don't see why we should break the vgic common implementation as it
> does on the next series:
> http://www.gossamer-threads.com/lists/xen/devel/337708.
Like I said, for this particular instance I don't think it is a big deal
right now.
IOW I'd rather see this series go in whether this aspect is perfect or
not.
Ian.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver
2014-07-03 15:18 ` Ian Campbell
@ 2014-07-04 7:01 ` Vijay Kilari
2014-07-04 10:20 ` Julien Grall
0 siblings, 1 reply; 19+ messages in thread
From: Vijay Kilari @ 2014-07-04 7:01 UTC (permalink / raw)
To: Ian Campbell
Cc: Stefano Stabellini, Prasun Kapoor, Vijaya Kumar K, Julien Grall,
Tim Deegan, xen-devel@lists.xen.org, Stefano Stabellini
On Thu, Jul 3, 2014 at 8:48 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2014-07-03 at 15:21 +0100, Julien Grall wrote:
>> On 07/03/2014 03:02 PM, Ian Campbell wrote:
>> > On Thu, 2014-07-03 at 14:25 +0100, Julien Grall wrote:
>> >> On 07/03/2014 02:02 PM, Ian Campbell wrote:
>> >>> On Thu, 2014-07-03 at 13:57 +0100, Julien Grall wrote:
>> >>>
>> >>>>> +struct vgic_ops {
>> >>>>> + /* Initialize vGIC */
>> >>>>> + int (*vcpu_init)(struct vcpu *v);
>> >>>>> + /* Domain specific initialization of vGIC */
>> >>>>> + int (*domain_init)(struct domain *d);
>> >>>>> + /* SGI handler of vGIC */
>> >>>>> + int (*send_sgi)(struct vcpu *v, register_t sgir);
>> >>>>
>> >>>> By reviewing the VGIC-v3 support, I still don't think this is the right
>> >>>> callback to add. You bypass the VGIC common emulation with your
>> >>>> vgic_emulate...
>> >>>>
>> >>>> I would introduce a callback to emulate_sysreg rather than this send_sgi.
>> >>>
>> >>> Why? The vgic will either be v2 or v3, so either MMIO or sysreg, once
>> >>> the thing has been decoded then you want to send an SGI I think, hence
>> >>> the callback. Passing a register_t does seem odd though, I'd have
>> >>> thought it would take an SGI number and any other flags which would then
>> >>> be interpreted for either v2 or v3 as appropriate.
>> >>
>> >> The decoding depends on the vgic emulation. For now this function is
>> >> badly implement in vgic-v3.c.
>> >>
>> >> What I was trying to say is send_sgi can be handled internally. If you
>> >> are looking to the calls of this function, it's only happen within the
>> >> file vgic-v2.c (or vgic-v3.c)
>> >>
>> >> But, the sysreg emulation is called outside the vgic code. So we should
>> >> add a callaback for this.
>> >
>> > So the common code would have
>> > case HSR_SYSREG_ICC_SGI0R:
>> > gic->handle_sysreg(esr, val)
>> > instead of
>> > case HSR_SYSREG_ICC_SGI0R:
>> > gic->handle_sysreg(val);
>> > ?
>>
>> The is not the actual case,
>>
>> Actually, the common code is:
>>
>> case HSR_SYSREG_ICC_SGI0R:
>> vgic_emulate(regs, hsr)
>>
>> where vgic_emulate is implemented in vgic-v3.c rather than in vgic.c.
>> The function will decode the register and then call vgic_send_sgi.
The reason why vgic_emulate is kept in vgic-v3.c is because it is
GICv3 specific
and also the traps are landing in respective drivers. Ex: for GICv2
write to GICD_SGIR
is trapped in vgic-v2.c and similarly I expect for GICv3 is should be
kept in GICv3.
If we introduce emulate_sysreg as a callback, then there is no need of
send_sgi callback
and hence the handling path of SGI trap is different for GICv2 and GICv3.
>>
>> But, as the function send_sgi is only used internaly there is no reason
>> to create a callback.
>>
>> What I ask is to have a new callback emulate_sysreg. The common code
>> (i.e traps.c) will have:
>>
>> case HSR_SYSREG_ICC_SGI0R:
>> vgic_emulate(regs, hsr).
>>
>> The function vgic_emulate will be implemented in vgic.c:
>>
>> vgic_emulate(...)
>> {
>> vgic->emulate_sysreg(regs, hsr);
>> }
>>
>> The vgic-v3.c will implement the callback correctly.
>
> I don't mind this but I would be equally happy with vgic_emulate being
> in gic-v3.c at this stage without the unnecessary callback via
> ->send_sgi.
>
>> > That might be nicer, but TBH given that there is only one trappable gic
>> > sysreg right now I don't think it is worth getting too worried about. We
>> > can always rework this interface when gic v4 or v5 needs something more.
>>
>> I don't see why we should break the vgic common implementation as it
>> does on the next series:
>> http://www.gossamer-threads.com/lists/xen/devel/337708.
>
> Like I said, for this particular instance I don't think it is a big deal
> right now.
>
> IOW I'd rather see this series go in whether this aspect is perfect or
> not.
>
> Ian.
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver
2014-07-04 7:01 ` Vijay Kilari
@ 2014-07-04 10:20 ` Julien Grall
0 siblings, 0 replies; 19+ messages in thread
From: Julien Grall @ 2014-07-04 10:20 UTC (permalink / raw)
To: Vijay Kilari, Ian Campbell
Cc: Stefano Stabellini, Prasun Kapoor, Vijaya Kumar K, Tim Deegan,
xen-devel@lists.xen.org, Stefano Stabellini
On 07/04/2014 08:01 AM, Vijay Kilari wrote:
>>> where vgic_emulate is implemented in vgic-v3.c rather than in vgic.c.
>>> The function will decode the register and then call vgic_send_sgi.
>
> The reason why vgic_emulate is kept in vgic-v3.c is because it is
> GICv3 specific
> and also the traps are landing in respective drivers. Ex: for GICv2
> write to GICD_SGIR
> is trapped in vgic-v2.c and similarly I expect for GICv3 is should be
> kept in GICv3.
Even though it's vGICv3 specific, vGICv2 will be handled sooner or later
on GICv3 platform.
Directly implement vgic_emulate in vgic-v3.c won't help to catch coding
error from Xen and may bring the hypervisor to segfault.
It could be the case if we forgot to disable system register trapping
(which I don't think you do) in the GICv3.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v8a 6/6] xen/arm: Restrict saving of gic register for idle domain
2014-07-03 8:34 [PATCH v8a 0/6] GIC and VGIC code refactoring vijay.kilari
` (4 preceding siblings ...)
2014-07-03 8:34 ` [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver vijay.kilari
@ 2014-07-03 8:34 ` vijay.kilari
5 siblings, 0 replies; 19+ messages in thread
From: vijay.kilari @ 2014-07-03 8:34 UTC (permalink / raw)
To: Ian.Campbell, julien.grall, stefano.stabellini,
stefano.stabellini, tim, xen-devel
Cc: Prasun.Kapoor, Vijaya Kumar K, vijay.kilari
From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
Check for idle domain is missing before saving gic context.
Xen stays in hypervisor mode when idle vcpu is running.
So no need to save and restore context when switched to/from
idle domain
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
---
xen/arch/arm/gic.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index e1e27b35..c0d70b8 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -70,6 +70,9 @@ void gic_save_state(struct vcpu *v)
{
ASSERT(!local_irq_is_enabled());
+ if ( is_idle_vcpu(v) )
+ return;
+
/* No need for spinlocks here because interrupts are disabled around
* this call and it only accesses struct vcpu fields that cannot be
* accessed simultaneously by another pCPU.
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread