From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
In order to avoid code duplication (both handle_read() and
handle_ioserv() contain the same code for the sign-extension)
put this code to a common helper to be used for both.
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>
---
Please note, this is a split/cleanup/hardening of Julien's PoC:
"Add support for Guest IO forwarding to a device emulator"
Changes V1 -> V2:
- new patch
Changes V2 -> V3:
- no changes
---
---
xen/arch/arm/io.c | 18 ++----------------
xen/arch/arm/ioreq.c | 17 +----------------
xen/include/asm-arm/traps.h | 24 ++++++++++++++++++++++++
3 files changed, 27 insertions(+), 32 deletions(-)
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index f44cfd4..8d6ec6c 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -23,6 +23,7 @@
#include <asm/cpuerrata.h>
#include <asm/current.h>
#include <asm/mmio.h>
+#include <asm/traps.h>
#include <asm/hvm/ioreq.h>
#include "decode.h"
@@ -39,26 +40,11 @@ static enum io_state handle_read(const struct mmio_handler
*handler,
* setting r).
*/
register_t r = 0;
- uint8_t size = (1 << dabt.size) * 8;
if ( !handler->ops->read(v, info, &r, handler->priv) )
return IO_ABORT;
- /*
- * Sign extend if required.
- * Note that we expect the read handler to have zeroed the bits
- * outside the requested access size.
- */
- if ( 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;
- }
+ r = sign_extend(dabt, r);
set_user_reg(regs, dabt.reg, r);
diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c
index f08190c..2f39289 100644
--- a/xen/arch/arm/ioreq.c
+++ b/xen/arch/arm/ioreq.c
@@ -28,7 +28,6 @@ enum io_state handle_ioserv(struct cpu_user_regs *regs,
struct vcpu *v)
const union hsr hsr = { .bits = regs->hsr };
const struct hsr_dabt dabt = hsr.dabt;
/* Code is similar to handle_read */
- uint8_t size = (1 << dabt.size) * 8;
register_t r = v->io.req.data;
/* We are done with the IO */
@@ -37,21 +36,7 @@ enum io_state handle_ioserv(struct cpu_user_regs *regs,
struct vcpu *v)
if ( dabt.write )
return IO_HANDLED;
- /*
- * Sign extend if required.
- * Note that we expect the read handler to have zeroed the bits
- * outside the requested access size.
- */
- if ( 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;
- }
+ r = sign_extend(dabt, r);
set_user_reg(regs, dabt.reg, r);
diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
index 997c378..e301c44 100644
--- a/xen/include/asm-arm/traps.h
+++ b/xen/include/asm-arm/traps.h
@@ -83,6 +83,30 @@ static inline bool VABORT_GEN_BY_GUEST(const struct
cpu_user_regs *regs)
(unsigned long)abort_guest_exit_end == regs->pc;
}
+/* Check whether the sign extension is required and perform it */
+static inline register_t sign_extend(const struct hsr_dabt dabt, register_t r)
+{
+ uint8_t size = (1 << dabt.size) * 8;
+
+ /*
+ * Sign extend if required.
+ * Note that we expect the read handler to have zeroed the bits
+ * outside the requested access size.
+ */
+ if ( 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;
