Re: [PATCH v3 6/9] xen/arm64: bpi: Add missing code symbol annotations

["Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxx>]

On Tue, May 7, 2024 at 11:57 AM Julien Grall <julien@xxxxxxx> wrote:
>
> Hi,
>
> On 06/05/2024 13:54, Edgar E. Iglesias wrote:
> > On Sat, May 4, 2024 at 2:14 AM Stefano Stabellini
> > <sstabellini@xxxxxxxxxx> wrote:
> >>
> >> On Wed, 1 May 2024, Edgar E. Iglesias wrote:
> >>> From: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxx>
> >>>
> >>> Use the generic xen/linkage.h macros to annotate code symbols
> >>> and add missing annotations.
> >>>
> >>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xxxxxxx>
> >>> ---
> >>>   xen/arch/arm/arm64/bpi.S | 20 ++++++++++++--------
> >>>   1 file changed, 12 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
> >>> index 4e63825220..b16e4d1e29 100644
> >>> --- a/xen/arch/arm/arm64/bpi.S
> >>> +++ b/xen/arch/arm/arm64/bpi.S
> >>> @@ -52,14 +52,15 @@
> >>>    * micro-architectures in a system.
> >>>    */
> >>>       .align   11
> >>> -ENTRY(__bp_harden_hyp_vecs_start)
> >>> +FUNC(__bp_harden_hyp_vecs_start)
> >>>       .rept 4
> >>>       vectors hyp_traps_vector
> >>>       .endr
> >>> -ENTRY(__bp_harden_hyp_vecs_end)
> >>> +GLOBAL(__bp_harden_hyp_vecs_end)
> >>> +END(__bp_harden_hyp_vecs_start)
> >>
> >> Shouldn't GLOBAL be changed to FUNC as well?
> >>
> >
> > I was a bit unsure but went for GLOBAL since the _end labels point to
> > addresses after and outside of the code sequence.
> > But I don't have a strong opinion and am happy to change them to FUNC
> > if you feel that's better.
>
> I don't think it should be FUNC as this is not meant to be called
> directly. I am also under the impression, we were planning to get rid of
> GLOBAL() as well.
>
> Furthermore, __bp_harden_hyp_vec_start is not a function per say. It is
> a pointer to the vector table.
>
>  From the brief look, the same remarks would apply to the rest of bpi.S.
> So I think we want to switch all the ENTRY() to LABEL().

Hi Julien,

The reason I choose FUNC for the start of the symbol is because these
symbols contain
executable code (not only a table of pointers to code somewhere else)
and the ELF spec
says that STT_FUNC means the symbol contains functions or other executable
code (not only callable functions IIUC):

"STT_FUNC The symbol is associated with a function or other executable code."
https://refspecs.linuxbase.org/elf/elf.pdf
(Symbol Table 1-20).

I think using LABEL instead of GLOBAL for the _end labels of these
code sequences makes sense.
I'm happy to change the _start labels to LABEL too if you guys feel
that's better.

Cheers,
Edgar