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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.03.2022 17:36, Roger Pau Monné wrote:
> On Tue, Mar 08, 2022 at 04:08:53PM +0100, Jan Beulich wrote:
>> 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.
> 
> Would you mind dropping the .text.cold.* at commit? Otherwise I can
> resend.

Sure; no need to resend just for this.

Jan