All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9a 0/6] GIC and VGIC code refactoring
@ 2014-07-09  4:56 vijay.kilari
  2014-07-09  4:56 ` [PATCH v9a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h vijay.kilari
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: vijay.kilari @ 2014-07-09  4:56 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>

Made changes to existing gic and vgic drivers to make it
generic and added support for GIC v3 hw version.

The complete GICv3 patch series
"[PATCH v5 00/21] xen/arm: Add GICv3 support" is split
into two patch series version v6a and v6.

The first 8 patches of v6a/v7a series are merged. Hence
posting remaining 6 patches here.

Tested with ARM64 simulator with multicore core
and booted Dom0 kernel.

Major changes in v9a:
 - Added check on return value of vgic_v2_init()
 - Removed VGIC SGI handling callback
 - Removed check VCPU value before calling is_vcpu_online()

Major changes in v8a:
 - is_vcpu_online() is modified to take only vcpu as
    parameter and validity checks on vcpu is done in
    caller.
 - set vcpu_mask to 0 for SGI_TARGET_{OTHERS,SELF} modes
     made vgic-v2 mmio handler static

Major changes in v7a:
 - Made changes to send_SGI() to pass NULL as cpu_mask for
   sgi modes which does not require cpu mask
 - Renamed update_cpu_lr_mask() clear_cpu_lr_mask()
 - used writeb_relaxed() to access ITARGETR & IPRIORITYR
   registers

Major changes in v6a:
 - Changed send_SGI parameters ordering
 - coding styles and missing comments
 - New patch (17) to check for idle domain before saving GIC context
 - Introduced new callback in vgic handler for handling SGI

Major changes in v5:
 - Introduced new patch for checking platform capability for gicv3
 - Introduced more patches for vgic clean up and code movement
 - Added synchronization barriers and clean up in GICv3 driver
 - Rebase on top of master branch +
   remotes/origin/no_maintenance_interrupts-v8 patch set
 - Code base available in github
   git clone https://github.com/vijaykilari/Xen-GICv3.git
 - Fixed comments and coding style

Major changes in v4:
 - Changed io handlers to take mmio address and size as
   parameters
 - ioremap is used instead of fixmap to map GICv2 address
   space. Removed /4 in GICv2 register definitions
 - vGIC driver now uses register size to calculate IRQ rank
 - GICv2 LR register definitions are declared locally in GICv2 driver
 - GICR & GICD common register handling in vgic-v3 driver are segregated
   in one common function
 - irq_hw_controller definition is managed in GICv2 and GICv3 drivers
 - Made irq_ops const
 - GIC DT node is updated in respective drivers
 - Comments and coding style fixes
 - Rebased on remotes/origin/no_maintenance_interrupts-v8 + Julien's
   patch set

Vijaya Kumar K (6):
  xen/arm: move and rename is_vcpu_running function to sched.h
  xen/arm: move pending_irq structure to vgic header file
  xen/arm: calculate vgic irq rank based on register size
  xen/arm: Remove REG macro in vgic driver
  xen/arm: split vgic driver into generic and vgic-v2 driver
  xen/arm: Restrict saving of gic register for idle domain

 xen/arch/arm/Makefile           |    2 +-
 xen/arch/arm/gic.c              |    3 +
 xen/arch/arm/vgic-v2.c          |  506 +++++++++++++++++++++++++++++++++++++
 xen/arch/arm/vgic.c             |  522 +++++----------------------------------
 xen/include/asm-arm/domain.h    |   59 +----
 xen/include/asm-arm/gic.h       |    5 +-
 xen/include/asm-arm/processor.h |    8 +
 xen/include/asm-arm/vgic.h      |   79 +++++-
 xen/include/xen/sched.h         |    5 +
 9 files changed, 668 insertions(+), 521 deletions(-)
 create mode 100644 xen/arch/arm/vgic-v2.c

-- 
1.7.9.5

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

* [PATCH v9a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h
  2014-07-09  4:56 [PATCH v9a 0/6] GIC and VGIC code refactoring vijay.kilari
@ 2014-07-09  4:56 ` vijay.kilari
  2014-07-09 16:42   ` Julien Grall
  2014-07-09  4:56 ` [PATCH v9a 2/6] xen/arm: move pending_irq structure to vgic header file vijay.kilari
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: vijay.kilari @ 2014-07-09  4:56 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
---
v9a: Remove redundant check vcpu is NULL

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     |   21 +++------------------
 xen/include/xen/sched.h |    5 +++++
 2 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 96bd7c1..da807bd 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[i] != NULL &&
+                     is_vcpu_online(d->vcpu[i]) )
                     set_bit(i, &vcpu_mask);
             }
             break;
@@ -459,7 +444,7 @@ 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[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 d5bc461..2f876f5 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -838,6 +838,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] 11+ messages in thread

* [PATCH v9a 2/6] xen/arm: move pending_irq structure to vgic header file
  2014-07-09  4:56 [PATCH v9a 0/6] GIC and VGIC code refactoring vijay.kilari
  2014-07-09  4:56 ` [PATCH v9a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h vijay.kilari
@ 2014-07-09  4:56 ` vijay.kilari
  2014-07-09  4:56 ` [PATCH v9a 3/6] xen/arm: calculate vgic irq rank based on register size vijay.kilari
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: vijay.kilari @ 2014-07-09  4:56 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] 11+ messages in thread

* [PATCH v9a 3/6] xen/arm: calculate vgic irq rank based on register size
  2014-07-09  4:56 [PATCH v9a 0/6] GIC and VGIC code refactoring vijay.kilari
  2014-07-09  4:56 ` [PATCH v9a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h vijay.kilari
  2014-07-09  4:56 ` [PATCH v9a 2/6] xen/arm: move pending_irq structure to vgic header file vijay.kilari
@ 2014-07-09  4:56 ` vijay.kilari
  2014-07-09  4:56 ` [PATCH v9a 4/6] xen/arm: Remove REG macro in vgic driver vijay.kilari
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: vijay.kilari @ 2014-07-09  4:56 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 da807bd..48551bf 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;
 }
@@ -468,7 +471,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;
@@ -487,42 +490,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;
@@ -530,8 +535,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;
@@ -543,30 +548,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);
@@ -578,11 +585,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;
 
@@ -596,13 +603,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;
@@ -641,7 +648,7 @@ bad_width:
     return 0;
 
 write_ignore:
-    if ( dabt.size != 2 ) goto bad_width;
+    if ( dabt.size != DABT_WORD ) goto bad_width;
     return 1;
 }
 
@@ -693,7 +700,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] 11+ messages in thread

* [PATCH v9a 4/6] xen/arm: Remove REG macro in vgic driver
  2014-07-09  4:56 [PATCH v9a 0/6] GIC and VGIC code refactoring vijay.kilari
                   ` (2 preceding siblings ...)
  2014-07-09  4:56 ` [PATCH v9a 3/6] xen/arm: calculate vgic irq rank based on register size vijay.kilari
@ 2014-07-09  4:56 ` vijay.kilari
  2014-07-09  4:56 ` [PATCH v9a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver vijay.kilari
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: vijay.kilari @ 2014-07-09  4:56 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 48551bf..c8fbf9b 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;
 
@@ -464,8 +461,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 )
@@ -482,7 +478,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:
@@ -558,7 +554,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;
@@ -574,7 +570,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;
@@ -615,7 +611,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 */
@@ -623,27 +619,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] 11+ messages in thread

* [PATCH v9a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver
  2014-07-09  4:56 [PATCH v9a 0/6] GIC and VGIC code refactoring vijay.kilari
                   ` (3 preceding siblings ...)
  2014-07-09  4:56 ` [PATCH v9a 4/6] xen/arm: Remove REG macro in vgic driver vijay.kilari
@ 2014-07-09  4:56 ` vijay.kilari
  2014-07-09 16:49   ` Julien Grall
  2014-07-09  4:56 ` [PATCH v9a 6/6] xen/arm: Restrict saving of gic register for idle domain vijay.kilari
  2014-07-10 12:54 ` [PATCH v9a 0/6] GIC and VGIC code refactoring Ian Campbell
  6 siblings, 1 reply; 11+ messages in thread
From: vijay.kilari @ 2014-07-09  4:56 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>
---
v9a: Check for return value of vgic_v2_init().
     Removed vgic callback for sgi handling.

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       |  506 ++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/vgic.c          |  500 +++++------------------------------------
 xen/include/asm-arm/domain.h |    2 +
 xen/include/asm-arm/gic.h    |    3 +
 xen/include/asm-arm/vgic.h   |   17 ++
 6 files changed, 586 insertions(+), 444 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..2102e43
--- /dev/null
+++ b/xen/arch/arm/vgic-v2.c
@@ -0,0 +1,506 @@
+/*
+ * 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_v2_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;
+}
+
+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,
+};
+
+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 c8fbf9b..d223db4 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,16 @@ 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:
+        if ( vgic_v2_init(d) )
+            return -ENODEV;
+        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 +98,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 +124,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 +133,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 +146,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 +169,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,246 +205,60 @@ 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[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[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 )
     {
         if ( 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);
-    }
-    return 1;
-}
-
-static int vgic_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);
-        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;
+        vgic_vcpu_inject_irq(d->vcpu[vcpuid], virq);
     }
-
-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;
 }
 
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..19eed7e 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -86,6 +86,13 @@ 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);
+};
+
 /* Number of ranks of interrupt registers for a domain */
 #define DOMAIN_NR_RANKS(d) (((d)->arch.vgic.nr_lines+31)/32)
 
@@ -133,6 +140,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 +154,16 @@ 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);
 #endif /* __ASM_ARM_VGIC_H__ */
 
 /*
-- 
1.7.9.5

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

* [PATCH v9a 6/6] xen/arm: Restrict saving of gic register for idle domain
  2014-07-09  4:56 [PATCH v9a 0/6] GIC and VGIC code refactoring vijay.kilari
                   ` (4 preceding siblings ...)
  2014-07-09  4:56 ` [PATCH v9a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver vijay.kilari
@ 2014-07-09  4:56 ` vijay.kilari
  2014-07-09 16:51   ` Julien Grall
  2014-07-10 12:54 ` [PATCH v9a 0/6] GIC and VGIC code refactoring Ian Campbell
  6 siblings, 1 reply; 11+ messages in thread
From: vijay.kilari @ 2014-07-09  4:56 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>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.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] 11+ messages in thread

* Re: [PATCH v9a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h
  2014-07-09  4:56 ` [PATCH v9a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h vijay.kilari
@ 2014-07-09 16:42   ` Julien Grall
  0 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2014-07-09 16:42 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/09/2014 05:56 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

For the ARM part:

Acked-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

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

* Re: [PATCH v9a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver
  2014-07-09  4:56 ` [PATCH v9a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver vijay.kilari
@ 2014-07-09 16:49   ` Julien Grall
  0 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2014-07-09 16:49 UTC (permalink / raw)
  To: vijay.kilari, Ian.Campbell, stefano.stabellini,
	stefano.stabellini, tim, xen-devel
  Cc: Prasun.Kapoor, vijaya.kumar

Hi Vijay,

On 07/09/2014 05:56 AM, vijay.kilari@gmail.com wrote:
> 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>
Acked-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

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

* Re: [PATCH v9a 6/6] xen/arm: Restrict saving of gic register for idle domain
  2014-07-09  4:56 ` [PATCH v9a 6/6] xen/arm: Restrict saving of gic register for idle domain vijay.kilari
@ 2014-07-09 16:51   ` Julien Grall
  0 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2014-07-09 16:51 UTC (permalink / raw)
  To: vijay.kilari, Ian.Campbell, stefano.stabellini,
	stefano.stabellini, tim, xen-devel
  Cc: Prasun.Kapoor, vijaya.kumar

Hi,

On 07/09/2014 05:56 AM, vijay.kilari@gmail.com wrote:
> 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

I sent yesterday a patch to avoid context switch for idle VCPU (see
https://patches.linaro.org/33164/). This patch can be dropped from your
series.

Regards,

-- 
Julien Grall

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

* Re: [PATCH v9a 0/6] GIC and VGIC code refactoring
  2014-07-09  4:56 [PATCH v9a 0/6] GIC and VGIC code refactoring vijay.kilari
                   ` (5 preceding siblings ...)
  2014-07-09  4:56 ` [PATCH v9a 6/6] xen/arm: Restrict saving of gic register for idle domain vijay.kilari
@ 2014-07-10 12:54 ` Ian Campbell
  6 siblings, 0 replies; 11+ messages in thread
From: Ian Campbell @ 2014-07-10 12:54 UTC (permalink / raw)
  To: vijay.kilari
  Cc: stefano.stabellini, Prasun.Kapoor, vijaya.kumar, julien.grall,
	tim, xen-devel, stefano.stabellini


On Wed, 2014-07-09 at 10:26 +0530, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> 
> Made changes to existing gic and vgic drivers to make it
> generic and added support for GIC v3 hw version.

I acked the few unacked patches and applied all but the last patch which
as Julien indicates isn't necessary any more after "xen/arm: Don't
save/restore context for idle VCPU" which I pushed earlier today/

Thanks!

Ian.

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

end of thread, other threads:[~2014-07-10 12:54 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-09  4:56 [PATCH v9a 0/6] GIC and VGIC code refactoring vijay.kilari
2014-07-09  4:56 ` [PATCH v9a 1/6] xen/arm: move and rename is_vcpu_running function to sched.h vijay.kilari
2014-07-09 16:42   ` Julien Grall
2014-07-09  4:56 ` [PATCH v9a 2/6] xen/arm: move pending_irq structure to vgic header file vijay.kilari
2014-07-09  4:56 ` [PATCH v9a 3/6] xen/arm: calculate vgic irq rank based on register size vijay.kilari
2014-07-09  4:56 ` [PATCH v9a 4/6] xen/arm: Remove REG macro in vgic driver vijay.kilari
2014-07-09  4:56 ` [PATCH v9a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver vijay.kilari
2014-07-09 16:49   ` Julien Grall
2014-07-09  4:56 ` [PATCH v9a 6/6] xen/arm: Restrict saving of gic register for idle domain vijay.kilari
2014-07-09 16:51   ` Julien Grall
2014-07-10 12:54 ` [PATCH v9a 0/6] GIC and VGIC code refactoring Ian Campbell

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