diff mbox series

[RFC,v1,1/2] xen/arm64: entry: Use xen/linkage.h to annotate symbols

Message ID 20240410091947.1498695-2-edgar.iglesias@gmail.com (mailing list archive)
State Superseded
Headers show
Series xen/arm: Annotate code symbols | expand

Commit Message

Edgar E. Iglesias April 10, 2024, 9:19 a.m. UTC
From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

Use the generic xen/linkage.h macros when annotating symbols.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 xen/arch/arm/arm64/entry.S | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Andrew Cooper April 10, 2024, 10:21 a.m. UTC | #1
On 10/04/2024 10:19 am, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
>
> Use the generic xen/linkage.h macros when annotating symbols.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  xen/arch/arm/arm64/entry.S | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index f963c923bb..6188dd2416 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -480,9 +480,9 @@ guest_fiq_invalid_compat:
>  guest_error_compat:
>          guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_serror
>  
> -ENTRY(return_to_new_vcpu32)
> +FUNC(return_to_new_vcpu32)
>          exit    hyp=0, compat=1

In the new world, you want an END() too, which sets the size of the symbol.

A good cross-check of this annotation stuff is:

readelf -Wa xen-syms | grep return_to_new_vcpu32

which in this case will tell you that the symbol called
return_to_new_vcpu32 still has a size of 0.

~Andrew
Edgar E. Iglesias April 10, 2024, 10:24 a.m. UTC | #2
On Wed, Apr 10, 2024 at 12:21 PM Andrew Cooper <andrew.cooper3@citrix.com>
wrote:

> On 10/04/2024 10:19 am, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> >
> > Use the generic xen/linkage.h macros when annotating symbols.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> > ---
> >  xen/arch/arm/arm64/entry.S | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> > index f963c923bb..6188dd2416 100644
> > --- a/xen/arch/arm/arm64/entry.S
> > +++ b/xen/arch/arm/arm64/entry.S
> > @@ -480,9 +480,9 @@ guest_fiq_invalid_compat:
> >  guest_error_compat:
> >          guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_serror
> >
> > -ENTRY(return_to_new_vcpu32)
> > +FUNC(return_to_new_vcpu32)
> >          exit    hyp=0, compat=1
>
> In the new world, you want an END() too, which sets the size of the symbol.
>
> A good cross-check of this annotation stuff is:
>
> readelf -Wa xen-syms | grep return_to_new_vcpu32
>
> which in this case will tell you that the symbol called
> return_to_new_vcpu32 still has a size of 0.
>

Thanks Andrew,

Patch 2/2 adds the END, I should probably have squashed them into one...

Best regards,
Edgar
Jan Beulich April 18, 2024, 6:10 a.m. UTC | #3
On 10.04.2024 12:24, Edgar E. Iglesias wrote:
> On Wed, Apr 10, 2024 at 12:21 PM Andrew Cooper <andrew.cooper3@citrix.com>
> wrote:
> 
>> On 10/04/2024 10:19 am, Edgar E. Iglesias wrote:
>>> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
>>>
>>> Use the generic xen/linkage.h macros when annotating symbols.
>>>
>>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
>>> ---
>>>  xen/arch/arm/arm64/entry.S | 12 ++++++------
>>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
>>> index f963c923bb..6188dd2416 100644
>>> --- a/xen/arch/arm/arm64/entry.S
>>> +++ b/xen/arch/arm/arm64/entry.S
>>> @@ -480,9 +480,9 @@ guest_fiq_invalid_compat:
>>>  guest_error_compat:
>>>          guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_serror
>>>
>>> -ENTRY(return_to_new_vcpu32)
>>> +FUNC(return_to_new_vcpu32)
>>>          exit    hyp=0, compat=1
>>
>> In the new world, you want an END() too, which sets the size of the symbol.
>>
>> A good cross-check of this annotation stuff is:
>>
>> readelf -Wa xen-syms | grep return_to_new_vcpu32
>>
>> which in this case will tell you that the symbol called
>> return_to_new_vcpu32 still has a size of 0.
> 
> Patch 2/2 adds the END, I should probably have squashed them into one...

Only partly afaics: return_to_new_vcpu{32,64} are still left without. And
yes, preferably the adjustments to the start annotation for a symbol
would come with an END() addition right away.

Jan
Edgar E. Iglesias April 18, 2024, 7:06 a.m. UTC | #4
On Thu, Apr 18, 2024 at 8:10 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 10.04.2024 12:24, Edgar E. Iglesias wrote:
> > On Wed, Apr 10, 2024 at 12:21 PM Andrew Cooper <andrew.cooper3@citrix.com>
> > wrote:
> >
> >> On 10/04/2024 10:19 am, Edgar E. Iglesias wrote:
> >>> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> >>>
> >>> Use the generic xen/linkage.h macros when annotating symbols.
> >>>
> >>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> >>> ---
> >>>  xen/arch/arm/arm64/entry.S | 12 ++++++------
> >>>  1 file changed, 6 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> >>> index f963c923bb..6188dd2416 100644
> >>> --- a/xen/arch/arm/arm64/entry.S
> >>> +++ b/xen/arch/arm/arm64/entry.S
> >>> @@ -480,9 +480,9 @@ guest_fiq_invalid_compat:
> >>>  guest_error_compat:
> >>>          guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_serror
> >>>
> >>> -ENTRY(return_to_new_vcpu32)
> >>> +FUNC(return_to_new_vcpu32)
> >>>          exit    hyp=0, compat=1
> >>
> >> In the new world, you want an END() too, which sets the size of the symbol.
> >>
> >> A good cross-check of this annotation stuff is:
> >>
> >> readelf -Wa xen-syms | grep return_to_new_vcpu32
> >>
> >> which in this case will tell you that the symbol called
> >> return_to_new_vcpu32 still has a size of 0.
> >
> > Patch 2/2 adds the END, I should probably have squashed them into one...
>
> Only partly afaics: return_to_new_vcpu{32,64} are still left without. And
> yes, preferably the adjustments to the start annotation for a symbol
> would come with an END() addition right away.
>

Thanks Jan,

Yes, in v2 I've squashed the patches into one to avoid confusion:
https://patchew.org/Xen/20240415231541.4140052-1-edgar.iglesias@gmail.com/

Here's the hunk in patch 2/2 of the first v1 RFC submission that added
the END's to return_to_new_vcpuXX:
https://lists.xenproject.org/archives/html/xen-devel/2024-04/msg00505.html

FUNC(return_to_new_vcpu32)
         exit    hyp=0, compat=1
+END(return_to_new_vcpu32)
+
 FUNC(return_to_new_vcpu64)
         exit    hyp=0, compat=0
+END(return_to_new_vcpu64)

Cheers,
Edgar
diff mbox series

Patch

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index f963c923bb..6188dd2416 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -480,9 +480,9 @@  guest_fiq_invalid_compat:
 guest_error_compat:
         guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_serror
 
-ENTRY(return_to_new_vcpu32)
+FUNC(return_to_new_vcpu32)
         exit    hyp=0, compat=1
-ENTRY(return_to_new_vcpu64)
+FUNC(return_to_new_vcpu64)
         exit    hyp=0, compat=0
 
 return_from_trap:
@@ -536,7 +536,7 @@  return_from_trap:
  * it. So the function will unmask SError exception for a small window and
  * then mask it again.
  */
-check_pending_guest_serror:
+FUNC_LOCAL(check_pending_guest_serror)
         /*
          * Save elr_el2 to check whether the pending SError exception takes
          * place while we are doing this sync exception.
@@ -586,7 +586,7 @@  abort_guest_exit_end:
         cset    x19, ne
 
         ret
-ENDPROC(check_pending_guest_serror)
+END(check_pending_guest_serror)
 
 /*
  * Exception vectors.
@@ -597,7 +597,7 @@  ENDPROC(check_pending_guest_serror)
         .endm
 
         .align  11
-ENTRY(hyp_traps_vector)
+FUNC(hyp_traps_vector)
         ventry  hyp_sync_invalid            /* Synchronous EL2t */
         ventry  hyp_irq_invalid             /* IRQ EL2t */
         ventry  hyp_fiq_invalid             /* FIQ EL2t */
@@ -626,7 +626,7 @@  ENTRY(hyp_traps_vector)
  *
  * Returns prev in x0
  */
-ENTRY(__context_switch)
+FUNC(__context_switch)
         add     x8, x0, #VCPU_arch_saved_context
         mov     x9, sp
         stp     x19, x20, [x8], #16         /* store callee-saved registers */