xen/arm: io: Support sign-extension for every read access

The guest may try to load data from the emulated MMIO region using
instructions with Sign-Extension (i.e ldrs*). Any use of one those,
will set the SSE bit (Syndrome Sign Extend) in the ISS (see B3-1433
in DDI 0406C.b).

Note that the bit can only be set for access size smaller than the
register size (i.e byte/half-word for aarch32, byte/half-word/word for
aarch32). So we don't have to worry about undefined C behavior.

Until now, the support of sign-extension was limited for byte access in
vGIC emulation. Although there is no reason to not have it generically.

So move the support just after we get the data from the MMIO emulation.

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

xen/arch/arm/io.c

@@ -23,6 +23,33 @@
 #include <asm/current.h>
 #include <asm/mmio.h>
 
+static int handle_read(const struct mmio_handler *handler, struct vcpu *v,
+                       mmio_info_t *info, register_t *r)
+{
+    uint8_t size = (1 << info->dabt.size) * 8;
+
+    if ( !handler->ops->read(v, info, r, handler->priv) )
+        return 0;
+
+    /*
+     * Sign extend if required.
+     * Note that we expect the read handler to have zeroed the bits
+     * outside the requested access size.
+     */
+    if ( info->dabt.sign && (*r & (1UL << (size - 1)) ))
+    {
+        /*
+         * We are relying on register_t using the same as
+         * an unsigned long in order to keep the 32-bit assembly
+         * code smaller.
+         */
+        BUILD_BUG_ON(sizeof(register_t) != sizeof(unsigned long));
+        *r |= (~0UL) << size;
+    }
+
+    return 1;
+}
+
 int handle_mmio(mmio_info_t *info)
 {
     struct vcpu *v = current;
@@ -48,7 +75,7 @@ int handle_mmio(mmio_info_t *info)
     if ( info->dabt.write )
         return handler->ops->write(v, info, *r, handler->priv);
     else
-        return handler->ops->read(v, info, r, handler->priv);
+        return handle_read(handler, v, info, r);
 }
 
 void register_mmio_handler(struct domain *d,

xen/arch/arm/vgic-v2.c

@@ -129,7 +129,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
         *r = rank->v2.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);
+            *r = vgic_byte_read(*r, gicd_reg);
         vgic_unlock_rank(v, rank, flags);
         return 1;
 
@@ -142,7 +142,7 @@ static int vgic_v2_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, gicd_reg);
+            *r = vgic_byte_read(*r, gicd_reg);
         vgic_unlock_rank(v, rank, flags);
         return 1;
 
@@ -377,7 +377,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
 
             new_target = i % 8;
             old_target_mask = vgic_byte_read(rank->v2.itargets[REG_RANK_INDEX(8,
-                                             gicd_reg - GICD_ITARGETSR, DABT_WORD)], 0, i/8);
+                                             gicd_reg - GICD_ITARGETSR, DABT_WORD)], i/8);
             old_target = find_first_bit(&old_target_mask, 8);
 
             if ( new_target != old_target )
@@ -503,7 +503,7 @@ static struct vcpu *vgic_v2_get_target_vcpu(struct vcpu *v, unsigned int irq)
     ASSERT(spin_is_locked(&rank->lock));
 
     target = vgic_byte_read(rank->v2.itargets[REG_RANK_INDEX(8,
-                                              irq, DABT_WORD)], 0, irq & 0x3);
+                                              irq, DABT_WORD)], irq & 0x3);
 
     /* 1-N SPI should be delivered as pending to all the vcpus in the
      * mask, but here we just return the first vcpu for simplicity and
@@ -521,7 +521,7 @@ static int vgic_v2_get_irq_priority(struct vcpu *v, unsigned int irq)
 
     ASSERT(spin_is_locked(&rank->lock));
     priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8,
-                                              irq, DABT_WORD)], 0, irq & 0x3);
+                                              irq, DABT_WORD)], irq & 0x3);
 
     return priority;
 }

xen/arch/arm/vgic-v3.c

@@ -336,7 +336,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
         *r = rank->ipriority[REG_RANK_INDEX(8, reg - GICD_IPRIORITYR,
                                             DABT_WORD)];
         if ( dabt.size == DABT_BYTE )
-            *r = vgic_byte_read(*r, dabt.sign, reg);
+            *r = vgic_byte_read(*r, reg);
         vgic_unlock_rank(v, rank, flags);
         return 1;
     case GICD_ICFGR ... GICD_ICFGRN:
@@ -1042,7 +1042,7 @@ static int vgic_v3_get_irq_priority(struct vcpu *v, unsigned int irq)
 
     ASSERT(spin_is_locked(&rank->lock));
     priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8,
-                                              irq, DABT_WORD)], 0, irq & 0x3);
+                                              irq, DABT_WORD)], irq & 0x3);
 
     return priority;
 }

xen/include/asm-arm/vgic.h

@@ -158,15 +158,13 @@ static inline int REG_RANK_NR(int b, uint32_t n)
     }
 }
 
-static inline uint32_t vgic_byte_read(uint32_t val, int sign, int offset)
+static inline uint32_t vgic_byte_read(uint32_t val, int offset)
 {
     int byte = offset & 0x3;
 
     val = val >> (8*byte);
-    if ( sign && (val & 0x80) )
-        val |= 0xffffff00;
-    else
-        val &= 0x000000ff;
+    val &= 0x000000ff;
+
     return val;
 }