On 17/03/2022 16:28, Jan Beulich wrote: > On 17.03.2022 17:19, Andrew Cooper wrote: >> On 17/03/2022 09:17, Jan Beulich wrote: >>> On 16.03.2022 20:23, Andrew Cooper wrote: >>>> On 16/03/2022 08:33, Jan Beulich wrote: >>>>> On 15.03.2022 17:53, Andrew Cooper wrote: >>>>>> --- a/xen/arch/x86/xen.lds.S >>>>>> +++ b/xen/arch/x86/xen.lds.S >>>>>> @@ -215,8 +215,9 @@ SECTIONS >>>>>> } PHDR(text) >>>>>> DECL_SECTION(.init.data) { >>>>>> #endif >>>>>> + . = ALIGN(STACK_SIZE); >>>>>> + *(.init.bss.stack_aligned) >>>>> No real need for the ALIGN() here - it's the contributions to the >>>>> section which ought to come with proper alignment. Imo ALIGN() >>>>> should only ever be there ahead of a symbol definition, as otherwise >>>>> the symbol might not mark what it is intended to mark due to padding >>>>> which might be inserted. See also 01fe4da6243b ("x86: force suitable >>>>> alignment in sources rather than in linker script"). >>>>> >>>>> Really we should consider using >>>>> >>>>> *(SORT_BY_ALIGNMENT(.init.data .init.data.* .init.bss.*)) >>>>> >>>>> While I can see your point against forcing sorting by alignment >>>>> globally, this very argument doesn't apply here (at least until >>>>> there appeared a way for the section attribute and -fdata-sections >>>>> to actually interact, such that .init.* could also become per- >>>>> function/object). >>>>> >>>>> Then again - this block of zeroes doesn't need to occupy space in >>>>> the binary. >>>> It already occupies space, because of mkelf32. >>> Hmm, yes, and not just because of mkelf32: Since we munge everything >>> in a single PT_LOAD segment in the linker script, all of .init.* >>> necessarily has space allocated. >>> >>>>> It could very well live in a @nobits .init.bss in the >>>>> final ELF binary. But sadly the section isn't @nobits in the object >>>>> file, and with that there would need to be a way to make the linker >>>>> convert it to @nobits (and I'm unaware of such). What would work is >>>>> naming the section .bss.init.stack_aligned (or e.g. >>>>> .bss..init.stack_aligned to make it easier to separate it from >>>>> .bss.* in the linker script) - that'll make gcc mark it @nobits. >>>> Living in .bss would prevent it from being reclaimed. We've got several >>>> nasty bugs from shooting holes in the Xen image, and too many special >>>> cases already. >>> I didn't suggest to put it in .bss; the suggested name was merely so >>> that gcc would mark the section @nobits and we could exclude the >>> section from what makes in into .bss in the final image independent >>> of .init.* vs .bss.* ordering in the linker script. But anyway - with >>> the above this aspect is now moot anyway. >> So can I take this as an ack with the .init typo fixed? > R-b, yes, as long as the ALIGN(STACK_SIZE) is also dropped and the > other ALIGN() is retained (the latter you did already indicate you > would do).
Ah yes. Thanks. ~Andrew