[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH V4 19/24] xen/arm: io: Abstract sign-extension
On Tue, 12 Jan 2021, Oleksandr Tyshchenko wrote: > 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> > [On Arm only] > Tested-by: Wei Chen <Wei.Chen@xxxxxxx> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx> > --- > 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 > > Changes V3 -> V4: > - no changes here, but in new patch: > "xen/arm: io: Harden sign extension check" > --- > 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 9814481..307c521 100644 > --- a/xen/arch/arm/io.c > +++ b/xen/arch/arm/io.c > @@ -24,6 +24,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" > @@ -40,26 +41,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 3c4a24d..40b9e59 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; > + } > + > + return r; > +} > + > #endif /* __ASM_ARM_TRAPS__ */ > /* > * Local variables: > -- > 2.7.4 >
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |