From ac0ca6895b654d2d80b2e894183e253e0423f4e6 Mon Sep 17 00:00:00 2001 From: Julien Grall Date: Wed, 7 Oct 2015 15:41:05 +0100 Subject: [PATCH] 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 Acked-by: Ian Campbell --- xen/arch/arm/io.c | 29 ++++++++++++++++++++++++++++- xen/arch/arm/vgic-v2.c | 10 +++++----- xen/arch/arm/vgic-v3.c | 4 ++-- xen/include/asm-arm/vgic.h | 8 +++----- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c index ffe7c21595..de5765aa9d 100644 --- a/xen/arch/arm/io.c +++ b/xen/arch/arm/io.c @@ -23,6 +23,33 @@ #include #include +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, diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c index dc2c2d28aa..dcbba0fff9 100644 --- a/xen/arch/arm/vgic-v2.c +++ b/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; } diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c index 2a09f86256..6de7f00af4 100644 --- a/xen/arch/arm/vgic-v3.c +++ b/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; } diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h index 96839f0202..354c0d4bc6 100644 --- a/xen/include/asm-arm/vgic.h +++ b/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; } -- 2.30.2