Re: [PATCH v3 1/2] xen/build: put image header into a separate section

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.03.2022 15:18, Roger Pau Monné wrote:
> On Tue, Mar 08, 2022 at 02:57:23PM +0100, Jan Beulich wrote:
>> On 08.03.2022 14:49, Roger Pau Monne wrote:
>>> So it can be explicitly placed ahead of the rest of the .text content
>>> in the linker script (and thus the resulting image). This is a
>>> prerequisite for further work that will add a catch-all to the text
>>> section (.text.*).
>>>
>>> Note that placement of the sections inside of .text is also slightly
>>> adjusted to be more similar to the position found in the default GNU
>>> ld linker script.
>>>
>>> The special handling of the object file containing the header data as
>>> the first object file passed to the linker command line can also be
>>> removed.
>>>
>>> While there also remove the special handling of efi/ on x86. There's
>>> no need for the resulting object file to be passed in any special
>>> order to the linker.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>
>> Looks good to me, but I have one question before feeling ready to
>> offer R-b:
>>
>>> @@ -86,8 +84,13 @@ SECTIONS
>>>         *(.text.kexec)          /* Page aligned in the object file. */
>>>         kexec_reloc_end = .;
>>>  
>>> -       *(.text.cold)
>>> -       *(.text.unlikely)
>>> +       *(.text.cold .text.cold.*)
>>> +       *(.text.unlikely .text.*_unlikely .text.unlikely.*)
>>
>> What generates .text.*_unlikely? And if anything really does, why
>> would .text.cold not have a similar equivalent?
> 
> That matches what I saw in the default linker script from my version
> of GNU ld:
> 
>     *(.text.unlikely .text.*_unlikely .text.unlikely.*)
> 
> I really don't know what could generate .text.*_unlikely, but since
> it's part of the default linker script I assumed it was better to just
> add it.

I've checked - gcc up to 4.5.x would generate .text.*_unlikely; from
4.6.x. onwards it would be .text.unlikely.*.

As to the dissimilarity with .text.cold: I wonder why we have that in
the first place. It matches our __cold attribute, just that we don't
use that anywhere afaics.

In any event:
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
albeit preferably with .text.cold.* dropped again.

Jan