[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v8 1/2] x86/boot: Align mbi2.c stack to 16 bytes
On 09.10.2024 10:04, Frediano Ziglio wrote: > Doing previous testing with an Adler Lake Intel machine the following > patch (improving MBI structure checking) started to fail. In patch descriptions please don't refer to "this patch" or "the following patch"; describe a commit in a self-contained way, with references to what's already committed mentioning commit hash and title, whereas references to what hasn't been committed using merely the title (and maybe a link to its most recent posting). I'm not sure though that the other patch really matters here beyond having exposed an issue that was there (latently) anyway. > Excluding it makes the tests succeed however there was not apparent > reason (looking at the code) for the failure. > So I instrumented code to output the structure and tested code with > this extracted data with and without the following patch and results > were the same. > Compiled assembly code from lab was also fine beside not keeping > the 16-byte alignment for the stack. > Turning on stack alignment solve the problem on Adler Lake machine. > > Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx> This really wants a Fixes: tag then. > --- a/xen/arch/x86/efi/Makefile > +++ b/xen/arch/x86/efi/Makefile > @@ -11,6 +11,8 @@ $(obj)/boot.init.o: $(obj)/buildid.o > $(call cc-option-add,cflags-stack-boundary,CC,-mpreferred-stack-boundary=4) > $(addprefix $(obj)/,$(EFIOBJ-y)): CFLAGS_stack_boundary := > $(cflags-stack-boundary) > > +$(obj)/mbi2.o: CFLAGS_stack_boundary := $(cflags-stack-boundary) > + > obj-y := common-stub.o stub.o > obj-$(XEN_BUILD_EFI) := $(filter-out %.init.o,$(EFIOBJ-y)) > obj-bin-$(XEN_BUILD_EFI) := $(filter %.init.o,$(EFIOBJ-y)) You're duplicating code, which is better to avoid when possible. Is there a reason the earlier commit didn't simply add mbi2.o to $(EFIOBJ-y)? That way the existing logic would have covered that file as well. And really I think it should have been mbi2.init.o (or else adding it into $(obj-bin-y) is wrong), which probably wants correcting at the same time (ISTR actually having requested that during an earlier review round). Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |