Re: [Minios-devel] [UNIKRAFT RFC PATCH 3/5] arch/arm64: Don't pass -mgeneral-regs-only when CONFIG_FLOAT_POINT is y

[Julien Grall <julien@xxxxxxx>]

On 19/12/2019 14:27, Jia He wrote:
> Support CONFIG_FLOAT_POINT in Unikraft app will take some overhead during
> context switch. Hence still use -mgeneral-regs-only when
> CONFIG_FLOAT_POINT is 'n'
>
> Signed-off-by: Jia He <justin.he@xxxxxxx>
> ---
>   arch/arm/arm64/Makefile.uk | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/arm64/Makefile.uk b/arch/arm/arm64/Makefile.uk
> index eb73cac..b2c093c 100644
> --- a/arch/arm/arm64/Makefile.uk
> +++ b/arch/arm/arm64/Makefile.uk
> @@ -3,9 +3,15 @@
>   # we should not enable the FP & SIMD access for kernel. As GCC will
>   # the FP & SIMD registers to pass parameters, we use -mgeneral-regs-only
>   # flag to force GCC to use generic registers
> +ifeq ($(CONFIG_FLOAT_POINT),y)
> +ASFLAGS  += -D__ARM_64__
> +CFLAGS   += -D__ARM_64__ -fms-extensions
> +CXXFLAGS += -D__ARM_64__ -fms-extensions
> +else
The comments above clearly states that FPSIMD access for the kernel 
should not be allowed. This was introduced by commit:
commit 670fe83874523f659769aa9fdd3dda891086fb7a
Author: Wei Chen <Wei.Chen@xxxxxxx>
Date:   Fri Sep 14 07:56:41 2018 +0000

    arch/arm64: Avoid using the floating-point and Advanced SIMD registers

    On Arm64, sometimes, the GCC will use floating-point and Advanced
    SIMD registers to pass parameters. For example, the va_list will
    use the SIMD&FP registers (like q0, q1) to store parameters, no
    matter you are using floating-point or not.

    But before we include the FP & SIMD registers in context switch,
    we should not enable the FP & SIMD access for kernel. So, we use
    the GCC -mgeneral-regs-only flag to force GCC to use generic
    registers only util the floating-point and Advanced SIMD registers
    are required actually.

    Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
    Reviewed-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>

The main concern here is if you enable FPSIMD for the kernel, then this 
could be used by exception (such as interrupts) and therefore corrupt 
the state.
I really think we should keep the kernel free of FPSIMD to avoid to 
increase the size of the context to switch on exception.
>   ASFLAGS  += -D__ARM_64__ -mgeneral-regs-only
>   CFLAGS   += -D__ARM_64__ -fms-extensions -mgeneral-regs-only
>   CXXFLAGS += -D__ARM_64__ -fms-extensions -mgeneral-regs-only
> +endif
>   
>   CINCLUDES   += -I$(CONFIG_UK_BASE)/arch/arm/arm64/include
>   ASINCLUDES  += -I$(CONFIG_UK_BASE)/arch/arm/arm64/include
Cheers,

--
Julien Grall

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel