xen/arm: vgic: Optimize the way to store GICD_IPRIORITYR in the rank

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>

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index dcbba0fff97d3ee31f8c460f48d57ba77fdb4f8f..ad1bb15eb898b93c43490f491ba7efa386cec043 100644 (file)
--- 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 6de7f00af4dffaecbed5ed2f46e6df70b38369c6..92a3ccfb9492974616f65fa990deb54cda6243d4 100644 (file)
--- 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 a6835a806cd2a1b3a87313281cafc544b38f3b97..2128d2934e28b0804b8ec6ba36c32225f2784045 100644 (file)
--- 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 354c0d4bc6eb24c20ab2f5914e5498d4ea2125cc..ff989139477b8c6b628db2bf1c4911122ef001e9 100644 (file)
--- 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);