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

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

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?

> ---
> v7: New.
>
> --- a/xen/arch/arm/arm32/entry.S
> +++ b/xen/arch/arm/arm32/entry.S
> @@ -31,7 +31,7 @@
>    *  r4: Set to a non-zero value if a pending Abort exception took place.
>    *      Otherwise, it will be set to zero.
>    */
> -prepare_context_from_guest:
> +FUNC_LOCAL(prepare_context_from_guest)
>   #ifdef CONFIG_ARM32_HARDEN_BRANCH_PREDICTOR
>           /*
>            * Restore vectors table to the default as it may have been
> @@ -140,7 +140,7 @@ abort_guest_exit_end:
>   
>   skip_check:
>           b   enter_hypervisor_from_guest_preirq
> -ENDPROC(prepare_context_from_guest)
> +END(prepare_context_from_guest)
>   
>           /*
>            * Macro to define a trap entry.
> @@ -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?

>           /*
>            * 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. So I think at least the alignment should be 0.
>           ldr r11, [sp, #UREGS_cpsr]
>           and r11, #PSR_MODE_MASK
>           cmp r11, #PSR_MODE_HYP
> @@ -426,6 +426,7 @@ return_to_hypervisor:
>           clrex
>           eret
>           sb
> +END(return_from_trap)
>   
>   /*
>    * struct vcpu *__context_switch(struct vcpu *prev, struct vcpu *next)
> @@ -435,12 +436,13 @@ return_to_hypervisor:
>    *
>    * Returns prev in r0
>    */
> -ENTRY(__context_switch)
> +FUNC(__context_switch)
>           add     ip, r0, #VCPU_arch_saved_context
>           stmia   ip!, {r4 - sl, fp, sp, lr}      /* Save register state */
>   
>           add     r4, r1, #VCPU_arch_saved_context
>           ldmia   r4, {r4 - sl, fp, sp, pc}       /* Load registers and return 
> */
> +END(__context_switch)
>   
>   /*
>    * Local variables:
Cheers,

--
Julien Grall