[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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>

The code does what it says it does, that's all I can without having gone
through EFI spec.

Since you're the EFI maintainer, you have the final say on this matter.
Not sure if you're expecting anything else from Andrew or me.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.