Re: [PATCH for-next v2 0/2] xen/arm: Mitigate straight-line speculation

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Julien,

> On 17 Mar 2021, at 11:20, Julien Grall <julien@xxxxxxx> wrote:
> 
> 
> 
> On 16/03/2021 17:16, Bertrand Marquis wrote:
>> Hi Julien,
> 
> Hi Bertrand,
> 
>>> On 16 Mar 2021, at 15:27, Julien Grall <julien@xxxxxxx> wrote:
>>> 
>>> 
>>> 
>>> On 15/03/2021 13:32, Bertrand Marquis wrote:
>>>> Hi Julien,
>>> 
>>> Hi Bertrand,
>>> 
>>>>> On 13 Mar 2021, at 16:06, Julien Grall <julien@xxxxxxx> wrote:
>>>>> 
>>>>> From: Julien Grall <jgrall@xxxxxxxxxx>
>>>>> 
>>>>> Hi all,
>>>>> 
>>>>> Last year, Arm released a whitepaper about a new category of speculation.
>>>>> (see [1] and [2]). In short, a processor may be able to speculate past
>>>>> some of the unconditional control flow instructions (e.g eret, smc, br).
>>>>> 
>>>>> In some of the cases, the registers will contain values controlled by
>>>>> the guest. While there is no known gadget afterwards, we still want to
>>>>> prevent any leakage in the future.
>>>>> 
>>>>> The mitigation is planned in two parts:
>>>>>   1) Arm provided patches for both GCC and LLVM to add speculation barrier
>>>>>   and remove problematic code sequence.
>>>>>   2) Inspection of assembly code and call to higher level (e.g smc in our 
>>>>> case).
>>>>> 
>>>>> I still haven't looked at 1) and how to mitigate properly Arm32 (see
>>>>> patch #1) and SMC call. So this issue is not fully addressed.
>>>>> 
>>>>> Note that the ERET instruction was already addressed as part of XSA-312.
>>>> On my tests, this serie is breaking the arm64 build:
>>>> | aarch64-poky-linux-ld 
>>>> --sysroot=/home/bermar01/Development/xen-dev/build/profile-fvp-base.prj/tmp/work/fvp_base-poky-linux/xen/4.15+git1-r0/recipe-sysroot
>>>>          -EL  --fix-cortex-a53-843419 --fix-cortex-a53-843419 -r -o 
>>>> built_in.o memcpy.o memcmp.o memmove.o memset.o memchr.o clear_page.o 
>>>> bitops.o find_next_bit.o strchr.o strcmp.o strlen.o strncmp.o strnlen.o 
>>>> strrchr.o
>>> 
>>> I can't see any build failure with the following GCC:
>>> 
>>> 42sh> aarch64-linux-gnu-gcc
>>> aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0
>>> Copyright (C) 2017 Free Software Foundation, Inc.
>>> This is free software; see the source for copying conditions.  There is NO
>>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>>> 
>>> AFAICT, there is also no compilation issue reported by gitlab:
>>> 
>>> https://gitlab.com/xen-project/patchew/xen/-/pipelines/269989894
>>> 
>>> What's the version of your compiler? Do you have steps to reproduce your 
>>> setup?
>> You need to have earlyprintk enabled
>> I am using gcc 7.5.0:
>> aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0
>> one configuration triggering the issue is using the default .config with the 
>> following items added:
>> CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS=y
>> CONFIG_DEBUG_LOCK_PROFILE=y
>> CONFIG_PERF_COUNTERS=y
>> CONFIG_PERF_ARRAYS=y
>> CONFIG_DEVICE_TREE_DEBUG=y
>> CONFIG_DEBUG_TRACE=y
>> CONFIG_EARLY_PRINTK_JUNO=y
>> CONFIG_EARLY_UART_PL011=y
>> CONFIG_EARLY_PRINTK=y
>> CONFIG_EARLY_UART_BASE_ADDRESS=0x7ff80000
>> CONFIG_EARLY_UART_PL011_BAUD_RATE=115200
>> CONFIG_EARLY_UART_INIT=y
>> CONFIG_EARLY_PRINTK_INC="debug-pl011.inc”
> 
> Thanks for providing the .config. I managed to reproduce it. So I removed 
> "asm_defns.h" everywhere but forgot to include it in the "config.h" :/.
> 
> This small change fixed the error:
> 
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index 51273b9db1fc..c7b77912013e 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -192,7 +192,7 @@ extern unsigned long frametable_virt_end;
> #define watchdog_enable()  ((void)0)
> 
> #if defined(__ASSEMBLY__) && !defined(__LINKER__)
> -#include <asm/asm-offsets.h>
> +#include <asm/asm_defns.h>
> #include <asm/macros.h>
> #endif
> 
> Would you still be happy to review the series before I send a v3?

Sure,

I will restart my tests with this change now and review the v2 once passed.

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall