Hi Julien, > On 18 Apr 2021, at 19:03, Julien Grall <[email protected]> wrote: > > From: Julien Grall <[email protected]> > > Some CPUs can speculate past a RET instruction and potentially perform > speculative accesses to memory before processing the return. > > There is no known gadget available after the RET instruction today. > However some of the registers (such as in check_pending_guest_serror()) > may contain a value provided by the guest. > > In order to harden the code, it would be better to add a speculation > barrier after each RET instruction. The performance impact is meant to > be negligeable as the speculation barrier is not meant to be > architecturally executed. > > Rather than manually inserting a speculation barrier, use a macro > which overrides the mnemonic RET and replace with RET + SB. We need to > use the opcode for RET to prevent any macro recursion. > > This patch is only covering the assembly code. C code would need to be > covered separately using the compiler support. > > Note that the definition of the macros sb needs to be moved earlier in > asm-arm/macros.h so it can be used by the new macro. > > This is part of the work to mitigate straight-line speculation. > > Signed-off-by: Julien Grall <[email protected]> Reviewed-by: Bertrand Marquis <[email protected]>
Cheers Bertrand > > --- > > It is not clear to me whether Armv7 (we don't officially support 32-bit > hypervisor on Armv8) is also affected by straight-line speculation. > The LLVM website suggests it is: https://reviews.llvm.org/D92395 > > For now only focus on arm64. > > Changes in v4: > - Remove Bertand's reviewed-by > - Use /* ... */ rather than // for comments > - Remove arm32 changes > - Explain why the macro sb is moved around > > Changes in v3: > - Add Bertrand's reviewed-by > > Changes in v2: > - Use a macro rather than inserting the speculation barrier > manually > - Remove mitigation for arm32 > --- > xen/include/asm-arm/arm64/macros.h | 6 ++++++ > xen/include/asm-arm/macros.h | 18 +++++++++--------- > 2 files changed, 15 insertions(+), 9 deletions(-) > > diff --git a/xen/include/asm-arm/arm64/macros.h > b/xen/include/asm-arm/arm64/macros.h > index f981b4f43e84..5ad66efd6ba4 100644 > --- a/xen/include/asm-arm/arm64/macros.h > +++ b/xen/include/asm-arm/arm64/macros.h > @@ -21,6 +21,12 @@ > ldr \dst, [\dst, \tmp] > .endm > > + .macro ret > + /* ret opcode */ > + .inst 0xd65f03c0 > + sb > + .endm > + > /* > * Register aliases. > */ > diff --git a/xen/include/asm-arm/macros.h b/xen/include/asm-arm/macros.h > index 4833671f4ced..1aa373760f98 100644 > --- a/xen/include/asm-arm/macros.h > +++ b/xen/include/asm-arm/macros.h > @@ -5,6 +5,15 @@ > # error "This file should only be included in assembly file" > #endif > > + /* > + * Speculative barrier > + * XXX: Add support for the 'sb' instruction > + */ > + .macro sb > + dsb nsh > + isb > + .endm > + > #if defined (CONFIG_ARM_32) > # include <asm/arm32/macros.h> > #elif defined(CONFIG_ARM_64) > @@ -20,13 +29,4 @@ > .endr > .endm > > - /* > - * Speculative barrier > - * XXX: Add support for the 'sb' instruction > - */ > - .macro sb > - dsb nsh > - isb > - .endm > - > #endif /* __ASM_ARM_MACROS_H */ > -- > 2.17.1 >
