[PATCH v3 5/9] xen/arm: vgic: Optimize the way to store GICD_IPRIORITYR in the rank

[Julien Grall <julien.grall@citrix.com>]

Xen is currently directly storing the value of GICD_IPRIORITYR register
in the rank. This makes emulation of the register access very simple
but makes the code to get the priority for a given vIRQ more complex.

While the priority of an vIRQ is retrieved every time an vIRQ is injected
to the guest, the access to register occurs less often.

Each GICD_IPRIORITYR register stores 4 priorities associated for 4 vIRQs
(see 4.3.11 in IHI 0048B). As Xen is using little endian, we can use
an union to access directly a register or a priority for a given IRQ.

Note that the field "ipriority" has been renamed to "ipriorityr" to
match the name of the register in the GIC spec.

Finally, the implementation of the callback get_irq_priority is exactly
the same for both vGIC drivers. Consolidate the implementation in the
common vGIC code and drop the callback.

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

---

The real reason is I'd like to drop vgic_byte_* helpers in favors of more
generic access helper. Although it would make the code to retrieve the
priority more complex. So reworking the way to get the priority was the
best solution.

    Changes in v3:
        - Add Ian's acked-by

    Changes in v2:
        - Use an union to get the best of the emulation and getting the
        priority quickly
        - Remove the callback get_irq_priority
        - Update commit message

    Changes in v1:
        - Patch added
---
 xen/arch/arm/vgic-v2.c     | 23 +++++------------------
 xen/arch/arm/vgic-v3.c     | 23 +++++------------------
 xen/arch/arm/vgic.c        | 18 ++++++++++++++----
 xen/include/asm-arm/vgic.h | 17 ++++++++++++++---
 4 files changed, 38 insertions(+), 43 deletions(-)

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index dcbba0f..ad1bb15 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -139,8 +139,8 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
         if ( rank == NULL) goto read_as_zero;
 
         vgic_lock_rank(v, rank, flags);
-        *r = rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
-                                            DABT_WORD)];
+        *r = rank->ipriorityr[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
+                                             DABT_WORD)];
         if ( dabt.size == DABT_BYTE )
             *r = vgic_byte_read(*r, gicd_reg);
         vgic_unlock_rank(v, rank, flags);
@@ -405,10 +405,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
         if ( rank == NULL) goto write_ignore;
         vgic_lock_rank(v, rank, flags);
         if ( dabt.size == DABT_WORD )
-            rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
-                                           DABT_WORD)] = r;
+            rank->ipriorityr[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
+                                            DABT_WORD)] = r;
         else
-            vgic_byte_write(&rank->ipriority[REG_RANK_INDEX(8,
+            vgic_byte_write(&rank->ipriorityr[REG_RANK_INDEX(8,
                         gicd_reg - GICD_IPRIORITYR, DABT_WORD)], r, gicd_reg);
         vgic_unlock_rank(v, rank, flags);
         return 1;
@@ -514,18 +514,6 @@ static struct vcpu *vgic_v2_get_target_vcpu(struct vcpu *v, unsigned int irq)
     return v_target;
 }
 
-static int vgic_v2_get_irq_priority(struct vcpu *v, unsigned int irq)
-{
-    int priority;
-    struct vgic_irq_rank *rank = vgic_rank_irq(v, irq);
-
-    ASSERT(spin_is_locked(&rank->lock));
-    priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8,
-                                              irq, DABT_WORD)], irq & 0x3);
-
-    return priority;
-}
-
 static int vgic_v2_vcpu_init(struct vcpu *v)
 {
     int i;
@@ -597,7 +585,6 @@ static int vgic_v2_domain_init(struct domain *d)
 static const struct vgic_ops vgic_v2_ops = {
     .vcpu_init   = vgic_v2_vcpu_init,
     .domain_init = vgic_v2_domain_init,
-    .get_irq_priority = vgic_v2_get_irq_priority,
     .get_target_vcpu = vgic_v2_get_target_vcpu,
     .max_vcpus = 8,
 };
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 6de7f00..92a3ccf 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -333,8 +333,8 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
         if ( rank == NULL ) goto read_as_zero;
 
         vgic_lock_rank(v, rank, flags);
-        *r = rank->ipriority[REG_RANK_INDEX(8, reg - GICD_IPRIORITYR,
-                                            DABT_WORD)];
+        *r = rank->ipriorityr[REG_RANK_INDEX(8, reg - GICD_IPRIORITYR,
+                                             DABT_WORD)];
         if ( dabt.size == DABT_BYTE )
             *r = vgic_byte_read(*r, reg);
         vgic_unlock_rank(v, rank, flags);
@@ -435,10 +435,10 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
         if ( rank == NULL ) goto write_ignore;
         vgic_lock_rank(v, rank, flags);
         if ( dabt.size == DABT_WORD )
-            rank->ipriority[REG_RANK_INDEX(8, reg - GICD_IPRIORITYR,
-                                           DABT_WORD)] = r;
+            rank->ipriorityr[REG_RANK_INDEX(8, reg - GICD_IPRIORITYR,
+                                            DABT_WORD)] = r;
         else
-            vgic_byte_write(&rank->ipriority[REG_RANK_INDEX(8,
+            vgic_byte_write(&rank->ipriorityr[REG_RANK_INDEX(8,
                        reg - GICD_IPRIORITYR, DABT_WORD)], r, reg);
         vgic_unlock_rank(v, rank, flags);
         return 1;
@@ -1035,18 +1035,6 @@ static const struct mmio_handler_ops vgic_distr_mmio_handler = {
     .write = vgic_v3_distr_mmio_write,
 };
 
-static int vgic_v3_get_irq_priority(struct vcpu *v, unsigned int irq)
-{
-    int priority;
-    struct vgic_irq_rank *rank = vgic_rank_irq(v, irq);
-
-    ASSERT(spin_is_locked(&rank->lock));
-    priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8,
-                                              irq, DABT_WORD)], irq & 0x3);
-
-    return priority;
-}
-
 static int vgic_v3_vcpu_init(struct vcpu *v)
 {
     int i;
@@ -1196,7 +1184,6 @@ static int vgic_v3_domain_init(struct domain *d)
 static const struct vgic_ops v3_ops = {
     .vcpu_init   = vgic_v3_vcpu_init,
     .domain_init = vgic_v3_domain_init,
-    .get_irq_priority = vgic_v3_get_irq_priority,
     .get_target_vcpu  = vgic_v3_get_target_vcpu,
     .emulate_sysreg  = vgic_v3_emulate_sysreg,
     /*
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index a6835a8..2128d29 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -204,6 +204,19 @@ struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int irq)
     return v_target;
 }
 
+static int vgic_get_virq_priority(struct vcpu *v, unsigned int virq)
+{
+    struct vgic_irq_rank *rank = vgic_rank_irq(v, virq);
+    unsigned long flags;
+    int priority;
+
+    vgic_lock_rank(v, rank, flags);
+    priority = rank->priority[virq & INTERRUPT_RANK_MASK];
+    vgic_unlock_rank(v, rank, flags);
+
+    return priority;
+}
+
 void vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq)
 {
     unsigned long flags;
@@ -407,14 +420,11 @@ void vgic_clear_pending_irqs(struct vcpu *v)
 void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int virq)
 {
     uint8_t priority;
-    struct vgic_irq_rank *rank = vgic_rank_irq(v, virq);
     struct pending_irq *iter, *n = irq_to_pending(v, virq);
     unsigned long flags;
     bool_t running;
 
-    vgic_lock_rank(v, rank, flags);
-    priority = v->domain->arch.vgic.handler->get_irq_priority(v, virq);
-    vgic_unlock_rank(v, rank, flags);
+    priority = vgic_get_virq_priority(v, virq);
 
     spin_lock_irqsave(&v->arch.vgic.lock, flags);
 
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 354c0d4..ff98913 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -82,12 +82,25 @@ struct pending_irq
     struct list_head lr_queue;
 };
 
+#define NR_INTERRUPT_PER_RANK   32
+#define INTERRUPT_RANK_MASK (NR_INTERRUPT_PER_RANK - 1)
+
 /* 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 */
     uint32_t ienable;
     uint32_t icfg[2];
-    uint32_t ipriority[8];
+
+    /*
+     * Provide efficient access to the priority of an vIRQ while keeping
+     * the emulation simple.
+     * Note, this is working fine as long as Xen is using little endian.
+     */
+    union {
+        uint8_t priority[32];
+        uint32_t ipriorityr[8];
+    };
+
     union {
         struct {
             uint32_t itargets[8];
@@ -114,8 +127,6 @@ struct vgic_ops {
     int (*vcpu_init)(struct vcpu *v);
     /* Domain specific initialization of vGIC */
     int (*domain_init)(struct domain *d);
-    /* Get priority for a given irq stored in vgic structure */
-    int (*get_irq_priority)(struct vcpu *v, unsigned int irq);
     /* Get the target vcpu for a given virq. The rank lock is already taken
      * when calling this. */
     struct vcpu *(*get_target_vcpu)(struct vcpu *v, unsigned int irq);
-- 
2.1.4