Re: [Xen-devel] [PATCH] x86/EFI: meet further spec requirements for runtime calls

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.11.16 at 07:48, <wei.liu2@xxxxxxxxxx> wrote:
> On Fri, Nov 11, 2016 at 03:39:26PM +0000, Andrew Cooper wrote:
>> On 10/11/16 16:06, Jan Beulich wrote:
>> > So far we didn't guarantee 16-byte alignment of the stack: While (so
>> > far) we don't tell the compiler to use smaller alignment, we also don't
>> > guarantee 16-byte alignment when establishing stack pointers for new
>> > vCPU-s. Runtime service functions using SSE instructions may end with
>> > #GP(0) without that.
>> >
>> > Note that -mpreferred-stack-boundary=3 is can be used only from gcc 4.8
>> > onwards, and -mincoming-stack-boundary=3 only from 5.3 onwards. It is
>> > for that reason that an alternative approach (using higher than
>> > necessary alignment) is being used when building with such older
>> > compilers.
>> >
>> > Furthermore we should avoid #MF to be raised on the FLDCW we do.
>> >
>> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> 
>> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> Applied.

I have to withdraw this patch (and hence revert it) - it has both an
active and a latent thinko/bug: The active one is that forcing stack
alignment in efi_rs_enter() is completely pointless. We want its
callers to have an aligned stack. The latent one is that with
-mpreferred-stack-boundary=3 the compiler is free to align the
calling function's stack, but allocate an odd number of longs on the
stack, so that the called function would still receive a misaligned
stack. The conclusion is that we shouldn't use
-mpreferred-stack-boundary=3, yet using
-mincoming-stack-boundary=3 alone would mean all functions in 
runtime.c would get their stack aligned. That might be acceptable,
but is wasteful. I think universally going the route of forcing larger
than necessary alignment (as done by the broken patch just for
older gcc) is the better route, albeit I think I should really check
that all gcc versions usable for building the EFI parts actually
honor the alignment (ISTR that very old gcc doesn't).

The alternative of always forcing an aligned stack would seem to
be quite a bit more intrusive a change, due to struct cpu_user_regs
(and the part of it actually covered by get_stack_bottom()) not
being a multiple of 16 in size. But I'll check more closely whether
this might also be a viable route.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel