[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] Ping: [PATCH v2] x86/EFI: meet further spec requirements for runtime calls
On Tue, Nov 22, 2016 at 03:09:37AM -0700, Jan Beulich wrote: > >>> On 15.11.16 at 17:06, <JBeulich@xxxxxxxx> wrote: > >>>> On 15.11.16 at 16:47, <andrew.cooper3@xxxxxxxxxx> wrote: > >> On 14/11/16 10:32, 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 making use of -mpreferred-stack-boundary=3, as mentioned in > >>> the comment, wouldn't help to reduce the needed alignment: The compiler > >>> would then be free to align the stack of the function with the aligned > >>> object, but would be permitted to place an odd number of 8-byte objects > >>> there, resulting in the callee to still run on an unaligned stack. > >>> > >>> (The only working alternative to the approach chosen here would be to > >>> use -mincoming-stack-boundary=3, but that would affect all functions in > >>> runtime.c, not just the ones actually making runtime services calls. > >>> And it would still require the manual alignment logic here to be used > >>> with gcc 5.2 and earlier - not permitting that command line option -, > >>> just that then the alignment amount would become conditional.) > >>> > >>> Hence enforce the needed alignment by making efi_rs_enter() return a > >>> suitably aligned structure, which the caller then necessarily has to > >>> store in a suitably aligned local variable, the address of which then > >>> gets passed to efi_rs_leave(). Also (to limit exposure) move the > >>> function declarations to where they belong: They're local to runtime.c, > >>> and shared only with compat.c (by the latter including the former). > >> > >> Why does this guarantee alignment? What prevents the compiler from > >> reordering the items in its stack layout? > > > > The compiler will always allocate stack variables such that called > > functions will see an ABI-compliant stack. Without variables of > > bigger alignment, it does this by implying that the current function > > also has an aligned stack. Since we start out with a stack frame on > > an 8 mod 16 boundary, said compiler behavior propagates > > this through all call hierarchies. With variables of bigger alignment > > the compiler arranges for the current frame to be suitably > > expanded, and it will of course continue to guarantee that all > > callees get to see a 16-byte aligned stack. IOW all we need to do > > is break this 8 mod 16 thing once. > > Ping? We have a problem to solve here, so I think I can expect that > either the proposed solution (even if not covering all theoretical > cases of possibly compiler behavior) is accepted, or an alternative > proposal is put up. I'd really like to avoid seeing 4.8 go out with the > problem un-addressed. (From a strictly formal perspective, me being > the only maintainer of EFI code, I could put the patch in without any > ack [other than Wei's release one], but I'd like to avoid that if at all > possible.) > I can't judge the technical correctness of this patch. I think it would be good to fix this, so: Release-acked-by: Wei Liu <wei.liu2@xxxxxxxxxx> > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |