Re: [PATCH v7 09/11] Arm32: use new-style entry annotations for entry code

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.11.2024 21:25, Julien Grall wrote:
> On 01/10/2024 16:17, Jan Beulich wrote:
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> Should the GLOBAL()s also be replaced?
> 
> What would you replace them with?

LABEL() or, perhaps better, DATA()/END() pairs.

>> @@ -362,13 +362,13 @@ trap_irq:
>>   trap_fiq:
>>           vector fiq
>>   
>> -return_from_trap:
>> +LABEL_LOCAL(return_from_trap)
> 
> OOI, why do we need to annotate return_from_trap?

We don't _need_ to; it's for consistency. Please advise whether you want me
to drop that change (it's the only use of LABEL_LOCAL() throughout the series).
But please be aware that then END(return_from_trap) also needs dropping (i.e.
leaving the range uncovered as far as plain ELF metadata goes).

>>           /*
>>            * Restore the stack pointer from r11. It was saved on exception
>>            * entry (see __DEFINE_TRAP_ENTRY).
>>            */
>>           mov sp, r11
>> -ENTRY(return_to_new_vcpu32)
>> +LABEL(return_to_new_vcpu32)
> 
> I am a bit confused why this is a LABEL rather than a FUNC. But I wonder 
> if either of them are corrrect? The code above is meant to fall into 
> this one.

My take is that function (or object) ranges shouldn't overlap. That's an
abstract reason. There's a practical reason, too: With "common: honor
CONFIG_CC_SPLIT_SECTIONS also for assembly functions" FUNC() will change
sections when CC_SPLIT_SECTIONS=y (which LABEL() won't). Section changes
clearly may not happen when fall-through is intended.

> So I think at least the alignment should be 0.

That would be a change compared to the prior use of ENTRY(). I can certainly
do so, but without having sufficient context my goal was to leave existing
alignment unaltered as far as possible (on the assumption that there's a
reason for it).

Jan