Re: [Xen-devel] [PATCH v5 01/13] x86: reduce general stack alignment to 8

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Nov 08, 2018 at 09:05:45AM -0700, Jan Beulich wrote:
> We don't need bigger alignment except when calling EFI boot or runtime
> services functions (and we don't guarantee that either, as explained
> close to the top of xen/common/efi/runtime.c in the struct efi_rs_state
> declaration). Hence if the compiler supports reducing stack alignment
> from the ABI compatible 16 bytes (gcc 7 and newer), do so wherever
> possible.
> 
> The EFI case itself is largely dealt with already (actually forcing
> 32-byte alignment) as a result of commit f6b7fedc89 ("x86/EFI: meet
> further spec requirements for runtime calls"). However, as explained in
> the description of that earlier change, without using
> -mincoming-stack-boundary=3 (which we don't want) we still have to make
> the compiler assume 16-byte stack boundaries for CUs making EFI calls in
> order to keep the compiler from aligning the stack, but then placing an
> odd number of 8-byte objects on it, resulting in a mis-aligned outgoing
> stack.
> 
> This as a side effect yields some code size reduction, since for a
> number of sufficiently simple non-leaf functions the stack adjustment
> (by 8, when there are no local stack variables at all) gets dropped
> altogether. I notice exceptions though, for example in guest_cpuid(),
> where in a release build gcc 8.2 now decides to set up a frame pointer
> (without ever using %rbp); I consider this a compiler quirk which we
> should leave to the compiler folks to address eventually.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v5: New.
> 
> --- a/xen/arch/x86/Rules.mk
> +++ b/xen/arch/x86/Rules.mk
> @@ -51,6 +51,11 @@ CFLAGS += -DCONFIG_INDIRECT_THUNK
>  export CONFIG_INDIRECT_THUNK=y
>  endif
>  
> +# If supported by the compiler, reduce stack alignment to 8 bytes. But allow
> +# this to be overridden elsewhere.
> +$(call cc-option-add,CFLAGS-stack-boundary,CC,-mpreferred-stack-boundary=3)
> +CFLAGS += $(CFLAGS-stack-boundary)
> +
>  # Set up the assembler include path properly for older toolchains.
>  CFLAGS += -Wa,-I$(BASEDIR)/include
>  
> --- a/xen/arch/x86/efi/Makefile
> +++ b/xen/arch/x86/efi/Makefile
> @@ -5,7 +5,11 @@ CFLAGS += -fshort-wchar
>  
>  boot.init.o: buildid.o
>  
> +EFIOBJ := boot.init.o compat.o runtime.o
> +
> +$(EFIOBJ): CFLAGS-stack-boundary := -mpreferred-stack-boundary=4

From gcc's manual on -mincoming-stack-boundary:

"Thus calling a function compiled with a higher preferred stack boundary
from a function compiled with a lower preferred stack boundary most
likely misaligns the stack." 

I notice runtime.o now has stack alignment of 2^4 while the rest of xen
has 2^3.

There is at least one example (efi_get_time) that could misalign the
stack. Is that okay?

Wei.

> +
>  obj-y := stub.o
> -obj-$(XEN_BUILD_EFI) := boot.init.o compat.o relocs-dummy.o runtime.o
> +obj-$(XEN_BUILD_EFI) := $(EFIOBJ) relocs-dummy.o
>  extra-$(XEN_BUILD_EFI) += buildid.o
>  nocov-$(XEN_BUILD_EFI) += stub.o
> 
> 
> 
> 

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