| Message ID | 1610488352-18494-21-git-send-email-olekstysh@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | IOREQ feature (+ virtio-mmio) on Arm | expand |
On Tue, 12 Jan 2021, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > In the ideal world we would never get an undefined behavior when > propagating the sign bit since that 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 aarch64). > > In the real world we need to care for *possible* hardware bug such as > advertising a sign extension for either 64-bit (or 32-bit) on Arm64 > (resp. Arm32). > > So harden a bit more the code to prevent undefined behavior when > propagating the sign bit in case of buggy hardware. > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > CC: Julien Grall <julien.grall@arm.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Please note, this is a split/cleanup/hardening of Julien's PoC: > "Add support for Guest IO forwarding to a device emulator" > > Changes V3 -> V4: > - new patch > --- > xen/include/asm-arm/traps.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h > index e301c44..992d537 100644 > --- a/xen/include/asm-arm/traps.h > +++ b/xen/include/asm-arm/traps.h > @@ -93,7 +93,8 @@ static inline register_t sign_extend(const struct hsr_dabt dabt, register_t r) > * Note that we expect the read handler to have zeroed the bits > * outside the requested access size. > */ > - if ( dabt.sign && (r & (1UL << (size - 1))) ) > + if ( dabt.sign && (size < sizeof(register_t) * 8) && > + (r & (1UL << (size - 1))) ) > { > /* > * We are relying on register_t using the same as > -- > 2.7.4 >
Hi Oleksandr, Oleksandr Tyshchenko writes: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > In the ideal world we would never get an undefined behavior when > propagating the sign bit since that 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 aarch64). > > In the real world we need to care for *possible* hardware bug such as > advertising a sign extension for either 64-bit (or 32-bit) on Arm64 > (resp. Arm32). > > So harden a bit more the code to prevent undefined behavior when > propagating the sign bit in case of buggy hardware. > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > CC: Julien Grall <julien.grall@arm.com> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > > --- > Please note, this is a split/cleanup/hardening of Julien's PoC: > "Add support for Guest IO forwarding to a device emulator" > > Changes V3 -> V4: > - new patch > --- > xen/include/asm-arm/traps.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h > index e301c44..992d537 100644 > --- a/xen/include/asm-arm/traps.h > +++ b/xen/include/asm-arm/traps.h > @@ -93,7 +93,8 @@ static inline register_t sign_extend(const struct hsr_dabt dabt, register_t r) > * Note that we expect the read handler to have zeroed the bits > * outside the requested access size. > */ > - if ( dabt.sign && (r & (1UL << (size - 1))) ) > + if ( dabt.sign && (size < sizeof(register_t) * 8) && > + (r & (1UL << (size - 1))) ) > { > /* > * We are relying on register_t using the same as
diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h index e301c44..992d537 100644 --- a/xen/include/asm-arm/traps.h +++ b/xen/include/asm-arm/traps.h @@ -93,7 +93,8 @@ static inline register_t sign_extend(const struct hsr_dabt dabt, register_t r) * Note that we expect the read handler to have zeroed the bits * outside the requested access size. */ - if ( dabt.sign && (r & (1UL << (size - 1))) ) + if ( dabt.sign && (size < sizeof(register_t) * 8) && + (r & (1UL << (size - 1))) ) { /* * We are relying on register_t using the same as