Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Josh Poimboeuf]

On Fri, May 19, 2017 at 11:17:24AM +0200, Jiri Slaby wrote:
> On 05/17/2017, 03:23 PM, Jiri Slaby wrote:
> >> So the initial CFI state is different between the two types of
> >> "functions".  And there are a lot of other differences.  C-type
> >> functions have to follow frame pointer conventions, for example.  So
> >> your FUNC_START macro (and objtool) would have to somehow figure out a
> >> way to make a distinction between the two.  So it would probably work
> >> out better if we kept the distinction between C-type functions and other
> >> code.
> > 
> > Ok, that makes a lot of sense.
> 
> A quick question:
> Do you consider these to be C-type functions?
> 
>   ENTRY(function_hook)
> ret
>   END(function_hook)
> 
> or this?
> 
>   ENTRY(native_load_gs_index)
> pushfq
> DISABLE_INTERRUPTS(CLBR_ANY & ~CLBR_RDI)
> SWAPGS
> movl%edi, %gs
> SWAPGS
> popfq
> ret
>   END(native_load_gs_index)
> 
> Both are called from C, but they do not setup frame pointer etc.

Yeah, those are valid C-type functions.  Setting up the frame pointer is
optional for leaf functions (i.e.  functions which don't call other
functions).

-- 
Josh

Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Josh Poimboeuf]

On Fri, May 19, 2017 at 11:17:24AM +0200, Jiri Slaby wrote:
> On 05/17/2017, 03:23 PM, Jiri Slaby wrote:
> >> So the initial CFI state is different between the two types of
> >> "functions".  And there are a lot of other differences.  C-type
> >> functions have to follow frame pointer conventions, for example.  So
> >> your FUNC_START macro (and objtool) would have to somehow figure out a
> >> way to make a distinction between the two.  So it would probably work
> >> out better if we kept the distinction between C-type functions and other
> >> code.
> > 
> > Ok, that makes a lot of sense.
> 
> A quick question:
> Do you consider these to be C-type functions?
> 
>   ENTRY(function_hook)
> ret
>   END(function_hook)
> 
> or this?
> 
>   ENTRY(native_load_gs_index)
> pushfq
> DISABLE_INTERRUPTS(CLBR_ANY & ~CLBR_RDI)
> SWAPGS
> movl%edi, %gs
> SWAPGS
> popfq
> ret
>   END(native_load_gs_index)
> 
> Both are called from C, but they do not setup frame pointer etc.

Yeah, those are valid C-type functions.  Setting up the frame pointer is
optional for leaf functions (i.e.  functions which don't call other
functions).

-- 
Josh

Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Jiri Slaby]

On 05/17/2017, 03:23 PM, Jiri Slaby wrote:
>> So the initial CFI state is different between the two types of
>> "functions".  And there are a lot of other differences.  C-type
>> functions have to follow frame pointer conventions, for example.  So
>> your FUNC_START macro (and objtool) would have to somehow figure out a
>> way to make a distinction between the two.  So it would probably work
>> out better if we kept the distinction between C-type functions and other
>> code.
> 
> Ok, that makes a lot of sense.

A quick question:
Do you consider these to be C-type functions?

  ENTRY(function_hook)
ret
  END(function_hook)

or this?

  ENTRY(native_load_gs_index)
pushfq
DISABLE_INTERRUPTS(CLBR_ANY & ~CLBR_RDI)
SWAPGS
movl%edi, %gs
SWAPGS
popfq
ret
  END(native_load_gs_index)

Both are called from C, but they do not setup frame pointer etc.

thanks,
-- 
js
suse labs

Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Jiri Slaby]

On 05/17/2017, 03:23 PM, Jiri Slaby wrote:
>> So the initial CFI state is different between the two types of
>> "functions".  And there are a lot of other differences.  C-type
>> functions have to follow frame pointer conventions, for example.  So
>> your FUNC_START macro (and objtool) would have to somehow figure out a
>> way to make a distinction between the two.  So it would probably work
>> out better if we kept the distinction between C-type functions and other
>> code.
> 
> Ok, that makes a lot of sense.

A quick question:
Do you consider these to be C-type functions?

  ENTRY(function_hook)
ret
  END(function_hook)

or this?

  ENTRY(native_load_gs_index)
pushfq
DISABLE_INTERRUPTS(CLBR_ANY & ~CLBR_RDI)
SWAPGS
movl%edi, %gs
SWAPGS
popfq
ret
  END(native_load_gs_index)

Both are called from C, but they do not setup frame pointer etc.

thanks,
-- 
js
suse labs

Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Jiri Slaby]

On 05/13/2017, 12:15 AM, Josh Poimboeuf wrote:
>> Similarly, I have OBJTOOL(START_FUNC) and OBJTOOL(END_FUNC) emitted with
>> each FUNC_START/FUNC_END. So far, when manually expanded for simplicity,
>> it looks like this:
> 
> I like the idea of making objtool smart enough to read the entry code,
> and of combining automated annotations (where possible) with manual
> annotations (where necessary).  And it does make sense for objtool to
> automate every rsp-related push/pop/sub/add annotation.  That will make
> the entry code quite a bit cleaner since we don't need 'push_cfi' and
> friends anymore.
> 
> However, I think trying to force the entry code snippets into being
> normal functions would be awkward.  For example, C-type functions all
> start off with the following initial CFI state:
> 
>  LOC   CFA  ra
>    rsp+8c-8
> 
> That means the previous frame's stack pointer was at rsp+8 and the
> return instruction pointer is (rsp).  But those assumptions don't hold
> for non-C-type functions, which usually start with pt_regs or iret regs
> on the stack, or a blank slate.
> 
> So the initial CFI state is different between the two types of
> "functions".  And there are a lot of other differences.  C-type
> functions have to follow frame pointer conventions, for example.  So
> your FUNC_START macro (and objtool) would have to somehow figure out a
> way to make a distinction between the two.  So it would probably work
> out better if we kept the distinction between C-type functions and other
> code.

Ok, that makes a lot of sense.

> I think ENDPROC (or FUNC_START/FUNC_END) should mean "this function is
> 100% standardized to the C ABI and its debuginfo can be completely
> automated".  And any code outside of that would be "this code is special
> and needs a mix of automated and manual debuginfo annotations."

I only hesitate how to call the others. I assume, SYM_FUNC_START and
SYM_FUNC_END were agreed upon for the C-func-like functions.

For the others, what about simply:
  SYM_FUNC_START_SPECIAL/SYM_FUNC_END_SPECIAL
or
  SYM_CODE_START/SYM_CODE_END
or
  SOMETHING_ELSE
?

> I'm also not sure we need the objtool-specific macros.  It might be
> simpler to have macros which just output the cfi instead.  I guess this
> goes back to our previous discussions about whether objtool's CFI access
> should be read/write or write-only.  I don't remember, did we ever to
> come to a conclusion with that?

Correct, exactly to avoid r-w on dwarfinfo in objtool, I introduced the
special objtool macros. They would just put the same cfis into the
.discard section for objtool to combine them with the automatic injected
annotations and put them to the correct place. For -- almost -- free.

Our last discussion on this topic ended up with w-only for objtool at
the moment. I originally wanted r-w to support inline assembly in C, but
you suggested r-only is quite easier, therefore we should start with it.
So the r-w extension is doable, but the question is whether we want the
complexity now.

> Either way, from looking at the entry code, we may be able to get away
> with only the following .macros:
> 
> - DWARF_EMPTY_FRAME signal=0
> 
>   Mark all registers as undefined and potentially mark the frame as a
>   signal frame.
> 
> - DWARF_SET_CFA base=rsp offset=0 c_regs=0 extra_regs=0 iret_regs=0
> 
>   Set the CFA value.  Set c_regs, extra_regs, and/or iret_regs to
>   indicate which regs (if any) are stored just below the CFA.
> 
> - DWARF_SET_INDIRECT_CFA base=rsp offset=0 val_offset=0
> 
>   Set CFA = *(base + offset) + val_offset.  I only saw a few places
>   where this is needed, where it switches to the irq stack.  We might be
>   able to figure out a way to simplify the code in a non-intrusive way
>   to get rid of the need for this one.

Correct, it corresponds with what I had locally to make DWARF unwinder
working through interrupts, in terms of CFI's:
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -463,6 +463,7 @@ SYM_FUNC_END(irq_entries_start)
ALLOC_PT_GPREGS_ON_STACK
SAVE_C_REGS
SAVE_EXTRA_REGS
+   DW_CFI(.cfi_rel_offset rbp, RBP+8)
ENCODE_FRAME_POINTER

testb   $3, CS(%rsp)
@@ -497,7 +498,17 @@ SYM_FUNC_END(irq_entries_start)
movq%rsp, %rdi
inclPER_CPU_VAR(irq_count)
cmovzq  PER_CPU_VAR(irq_stack_ptr), %rsp
+   DW_CFI(.cfi_def_cfa_register rdi)
+
pushq   %rdi
+   DW_CFI(.cfi_escape 0x0f /* DW_CFA_def_cfa_expression */, 6 /*
block len */, \
+   0x77 /* DW_OP_breg7 (rsp) */, 0 /* offset */, \
+   0x06 /* DW_OP_deref */, \
+   0x08 /* DW_OP_const1u */, SIZEOF_PTREGS, \
+   0x22 /* DW_OP_plus */)
+   DW_CFI(.cfi_offset rsp, -2*8)
+   DW_CFI(.cfi_offset rip, -5*8)
+
/* We entered an interrupt context - irqs are off: */
TRACE_IRQS_OFF

@@ -654,9 +665,15 @@ SYM_FUNC_END(common_interrupt)
  * APIC interrupts.
  

Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Jiri Slaby]

On 05/13/2017, 12:15 AM, Josh Poimboeuf wrote:
>> Similarly, I have OBJTOOL(START_FUNC) and OBJTOOL(END_FUNC) emitted with
>> each FUNC_START/FUNC_END. So far, when manually expanded for simplicity,
>> it looks like this:
> 
> I like the idea of making objtool smart enough to read the entry code,
> and of combining automated annotations (where possible) with manual
> annotations (where necessary).  And it does make sense for objtool to
> automate every rsp-related push/pop/sub/add annotation.  That will make
> the entry code quite a bit cleaner since we don't need 'push_cfi' and
> friends anymore.
> 
> However, I think trying to force the entry code snippets into being
> normal functions would be awkward.  For example, C-type functions all
> start off with the following initial CFI state:
> 
>  LOC   CFA  ra
>    rsp+8c-8
> 
> That means the previous frame's stack pointer was at rsp+8 and the
> return instruction pointer is (rsp).  But those assumptions don't hold
> for non-C-type functions, which usually start with pt_regs or iret regs
> on the stack, or a blank slate.
> 
> So the initial CFI state is different between the two types of
> "functions".  And there are a lot of other differences.  C-type
> functions have to follow frame pointer conventions, for example.  So
> your FUNC_START macro (and objtool) would have to somehow figure out a
> way to make a distinction between the two.  So it would probably work
> out better if we kept the distinction between C-type functions and other
> code.

Ok, that makes a lot of sense.

> I think ENDPROC (or FUNC_START/FUNC_END) should mean "this function is
> 100% standardized to the C ABI and its debuginfo can be completely
> automated".  And any code outside of that would be "this code is special
> and needs a mix of automated and manual debuginfo annotations."

I only hesitate how to call the others. I assume, SYM_FUNC_START and
SYM_FUNC_END were agreed upon for the C-func-like functions.

For the others, what about simply:
  SYM_FUNC_START_SPECIAL/SYM_FUNC_END_SPECIAL
or
  SYM_CODE_START/SYM_CODE_END
or
  SOMETHING_ELSE
?

> I'm also not sure we need the objtool-specific macros.  It might be
> simpler to have macros which just output the cfi instead.  I guess this
> goes back to our previous discussions about whether objtool's CFI access
> should be read/write or write-only.  I don't remember, did we ever to
> come to a conclusion with that?

Correct, exactly to avoid r-w on dwarfinfo in objtool, I introduced the
special objtool macros. They would just put the same cfis into the
.discard section for objtool to combine them with the automatic injected
annotations and put them to the correct place. For -- almost -- free.

Our last discussion on this topic ended up with w-only for objtool at
the moment. I originally wanted r-w to support inline assembly in C, but
you suggested r-only is quite easier, therefore we should start with it.
So the r-w extension is doable, but the question is whether we want the
complexity now.

> Either way, from looking at the entry code, we may be able to get away
> with only the following .macros:
> 
> - DWARF_EMPTY_FRAME signal=0
> 
>   Mark all registers as undefined and potentially mark the frame as a
>   signal frame.
> 
> - DWARF_SET_CFA base=rsp offset=0 c_regs=0 extra_regs=0 iret_regs=0
> 
>   Set the CFA value.  Set c_regs, extra_regs, and/or iret_regs to
>   indicate which regs (if any) are stored just below the CFA.
> 
> - DWARF_SET_INDIRECT_CFA base=rsp offset=0 val_offset=0
> 
>   Set CFA = *(base + offset) + val_offset.  I only saw a few places
>   where this is needed, where it switches to the irq stack.  We might be
>   able to figure out a way to simplify the code in a non-intrusive way
>   to get rid of the need for this one.

Correct, it corresponds with what I had locally to make DWARF unwinder
working through interrupts, in terms of CFI's:
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -463,6 +463,7 @@ SYM_FUNC_END(irq_entries_start)
ALLOC_PT_GPREGS_ON_STACK
SAVE_C_REGS
SAVE_EXTRA_REGS
+   DW_CFI(.cfi_rel_offset rbp, RBP+8)
ENCODE_FRAME_POINTER

testb   $3, CS(%rsp)
@@ -497,7 +498,17 @@ SYM_FUNC_END(irq_entries_start)
movq%rsp, %rdi
inclPER_CPU_VAR(irq_count)
cmovzq  PER_CPU_VAR(irq_stack_ptr), %rsp
+   DW_CFI(.cfi_def_cfa_register rdi)
+
pushq   %rdi
+   DW_CFI(.cfi_escape 0x0f /* DW_CFA_def_cfa_expression */, 6 /*
block len */, \
+   0x77 /* DW_OP_breg7 (rsp) */, 0 /* offset */, \
+   0x06 /* DW_OP_deref */, \
+   0x08 /* DW_OP_const1u */, SIZEOF_PTREGS, \
+   0x22 /* DW_OP_plus */)
+   DW_CFI(.cfi_offset rsp, -2*8)
+   DW_CFI(.cfi_offset rip, -5*8)
+
/* We entered an interrupt context - irqs are off: */
TRACE_IRQS_OFF

@@ -654,9 +665,15 @@ SYM_FUNC_END(common_interrupt)
  * APIC interrupts.
  

Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Josh Poimboeuf]

On Fri, May 12, 2017 at 09:53:48AM +0200, Jiri Slaby wrote:
> On 04/26/2017, 03:42 AM, Josh Poimboeuf wrote:
> >> @@ -323,7 +323,7 @@ ENTRY(resume_userspace)
> >>movl%esp, %eax
> >>callprepare_exit_to_usermode
> >>jmp restore_all
> >> -END(ret_from_exception)
> >> +ENDPROC(ret_from_exception)
> > 
> > What exactly is the motivation of this patch?  It would be good to
> > describe that in the commit message.
> > 
> > Is the point to allow objtool to generate CFI for it?  If so, I don't
> > really see how that would work.  Today, objtool considers ENDPROC to
> > annotate a *callable* function which conforms to the C calling ABI and
> > can be called by another function.  The stack is in a known state at
> > function entry, and so the CFI (or frame pointer info) can be reliably
> > determined.
> 
> Ugh, I haven't checked this in 100 % of cases, but this looks pretty
> fragile to me. From reading the code, the use of END or ENDPROC is
> rather random -- depending on mood and who wrote the code.

Yes, it would be fragile, but objtool has a fix for that.  It looks at
every instruction in the object file and warns if it finds a return
instruction outside of an ENDPROC function.  That works because all
callable instructions have return instructions (except when they have
sibling calls, but objtool detects those too).  So objtool will flag any
C-type functions that forgot to use ENDPROC.

> > But entry code is different.  In most cases, the global symbols aren't
> > actually called, and they don't follow any conventions.  The code is
> > spaghetti-esque, with HW handlers and jumps everywhere.  The state of
> > the stack at symbol entry varies per "function".  That's why objtool
> > ignores these files.
> 
> Unfortunately, this is true.
> 
> > For special cases (like entry code), I was thinking we'd need manual CFI
> > annotations, like we had before.  Or maybe there's another way, like
> > some new macros which tell objtool about the HW entry points and the
> > state of the registers there.
> > 
> > But I'm having trouble seeing how marking these code snippets with
> > ENTRY/ENDPROC would help objtool make any sense of the code and where
> > things are on the stack.
> 
> Ok, my intention was to have every line of assembly code in between of
> FUNC_START/FUNC_END. That way, every rsp related push/pop/sub/add can be
> annotated very easily. For the C-like functions this is all what needs
> to be done.
> 
> Then there is the spaghetti code. And I was thinking about manual
> annotations like:
> 
>   # skip the frame pointer checking between START+END here
>   OBJTOOL(SKIP_CHECKING)
> 
>   # this fn has unusual frame (like interrupts have),
> and you can find return RIP stored at fp + 0x20
>   OBJTOOL(RIP_IS_AT, 0x20)
> 
>   # put this raw CFI for this location into eh_frame
>   OBJTOOL(RAW_CFI, 0x00, 0x00, 0x00)
> 
> 
> Similarly, I have OBJTOOL(START_FUNC) and OBJTOOL(END_FUNC) emitted with
> each FUNC_START/FUNC_END. So far, when manually expanded for simplicity,
> it looks like this:

I like the idea of making objtool smart enough to read the entry code,
and of combining automated annotations (where possible) with manual
annotations (where necessary).  And it does make sense for objtool to
automate every rsp-related push/pop/sub/add annotation.  That will make
the entry code quite a bit cleaner since we don't need 'push_cfi' and
friends anymore.

However, I think trying to force the entry code snippets into being
normal functions would be awkward.  For example, C-type functions all
start off with the following initial CFI state:

 LOC   CFA  ra
   rsp+8c-8

That means the previous frame's stack pointer was at rsp+8 and the
return instruction pointer is (rsp).  But those assumptions don't hold
for non-C-type functions, which usually start with pt_regs or iret regs
on the stack, or a blank slate.

So the initial CFI state is different between the two types of
"functions".  And there are a lot of other differences.  C-type
functions have to follow frame pointer conventions, for example.  So
your FUNC_START macro (and objtool) would have to somehow figure out a
way to make a distinction between the two.  So it would probably work
out better if we kept the distinction between C-type functions and other
code.

I think ENDPROC (or FUNC_START/FUNC_END) should mean "this function is
100% standardized to the C ABI and its debuginfo can be completely
automated".  And any code outside of that would be "this code is special
and needs a mix of automated and manual debuginfo annotations."

I'm also not sure we need the objtool-specific macros.  It might be
simpler to have macros which just output the cfi instead.  I guess this
goes back to our previous discussions about whether objtool's CFI access
should be read/write or write-only.  I don't remember, did we ever to
come to a conclusion with that?

Either way, from looking at the entry code, we may be able to get 

Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Josh Poimboeuf]

On Fri, May 12, 2017 at 09:53:48AM +0200, Jiri Slaby wrote:
> On 04/26/2017, 03:42 AM, Josh Poimboeuf wrote:
> >> @@ -323,7 +323,7 @@ ENTRY(resume_userspace)
> >>movl%esp, %eax
> >>callprepare_exit_to_usermode
> >>jmp restore_all
> >> -END(ret_from_exception)
> >> +ENDPROC(ret_from_exception)
> > 
> > What exactly is the motivation of this patch?  It would be good to
> > describe that in the commit message.
> > 
> > Is the point to allow objtool to generate CFI for it?  If so, I don't
> > really see how that would work.  Today, objtool considers ENDPROC to
> > annotate a *callable* function which conforms to the C calling ABI and
> > can be called by another function.  The stack is in a known state at
> > function entry, and so the CFI (or frame pointer info) can be reliably
> > determined.
> 
> Ugh, I haven't checked this in 100 % of cases, but this looks pretty
> fragile to me. From reading the code, the use of END or ENDPROC is
> rather random -- depending on mood and who wrote the code.

Yes, it would be fragile, but objtool has a fix for that.  It looks at
every instruction in the object file and warns if it finds a return
instruction outside of an ENDPROC function.  That works because all
callable instructions have return instructions (except when they have
sibling calls, but objtool detects those too).  So objtool will flag any
C-type functions that forgot to use ENDPROC.

> > But entry code is different.  In most cases, the global symbols aren't
> > actually called, and they don't follow any conventions.  The code is
> > spaghetti-esque, with HW handlers and jumps everywhere.  The state of
> > the stack at symbol entry varies per "function".  That's why objtool
> > ignores these files.
> 
> Unfortunately, this is true.
> 
> > For special cases (like entry code), I was thinking we'd need manual CFI
> > annotations, like we had before.  Or maybe there's another way, like
> > some new macros which tell objtool about the HW entry points and the
> > state of the registers there.
> > 
> > But I'm having trouble seeing how marking these code snippets with
> > ENTRY/ENDPROC would help objtool make any sense of the code and where
> > things are on the stack.
> 
> Ok, my intention was to have every line of assembly code in between of
> FUNC_START/FUNC_END. That way, every rsp related push/pop/sub/add can be
> annotated very easily. For the C-like functions this is all what needs
> to be done.
> 
> Then there is the spaghetti code. And I was thinking about manual
> annotations like:
> 
>   # skip the frame pointer checking between START+END here
>   OBJTOOL(SKIP_CHECKING)
> 
>   # this fn has unusual frame (like interrupts have),
> and you can find return RIP stored at fp + 0x20
>   OBJTOOL(RIP_IS_AT, 0x20)
> 
>   # put this raw CFI for this location into eh_frame
>   OBJTOOL(RAW_CFI, 0x00, 0x00, 0x00)
> 
> 
> Similarly, I have OBJTOOL(START_FUNC) and OBJTOOL(END_FUNC) emitted with
> each FUNC_START/FUNC_END. So far, when manually expanded for simplicity,
> it looks like this:

I like the idea of making objtool smart enough to read the entry code,
and of combining automated annotations (where possible) with manual
annotations (where necessary).  And it does make sense for objtool to
automate every rsp-related push/pop/sub/add annotation.  That will make
the entry code quite a bit cleaner since we don't need 'push_cfi' and
friends anymore.

However, I think trying to force the entry code snippets into being
normal functions would be awkward.  For example, C-type functions all
start off with the following initial CFI state:

 LOC   CFA  ra
   rsp+8c-8

That means the previous frame's stack pointer was at rsp+8 and the
return instruction pointer is (rsp).  But those assumptions don't hold
for non-C-type functions, which usually start with pt_regs or iret regs
on the stack, or a blank slate.

So the initial CFI state is different between the two types of
"functions".  And there are a lot of other differences.  C-type
functions have to follow frame pointer conventions, for example.  So
your FUNC_START macro (and objtool) would have to somehow figure out a
way to make a distinction between the two.  So it would probably work
out better if we kept the distinction between C-type functions and other
code.

I think ENDPROC (or FUNC_START/FUNC_END) should mean "this function is
100% standardized to the C ABI and its debuginfo can be completely
automated".  And any code outside of that would be "this code is special
and needs a mix of automated and manual debuginfo annotations."

I'm also not sure we need the objtool-specific macros.  It might be
simpler to have macros which just output the cfi instead.  I guess this
goes back to our previous discussions about whether objtool's CFI access
should be read/write or write-only.  I don't remember, did we ever to
come to a conclusion with that?

Either way, from looking at the entry code, we may be able to get 

Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Jiri Slaby]

On 04/26/2017, 03:42 AM, Josh Poimboeuf wrote:
>> @@ -323,7 +323,7 @@ ENTRY(resume_userspace)
>>  movl%esp, %eax
>>  callprepare_exit_to_usermode
>>  jmp restore_all
>> -END(ret_from_exception)
>> +ENDPROC(ret_from_exception)
> 
> What exactly is the motivation of this patch?  It would be good to
> describe that in the commit message.
> 
> Is the point to allow objtool to generate CFI for it?  If so, I don't
> really see how that would work.  Today, objtool considers ENDPROC to
> annotate a *callable* function which conforms to the C calling ABI and
> can be called by another function.  The stack is in a known state at
> function entry, and so the CFI (or frame pointer info) can be reliably
> determined.

Ugh, I haven't checked this in 100 % of cases, but this looks pretty
fragile to me. From reading the code, the use of END or ENDPROC is
rather random -- depending on mood and who wrote the code.

> But entry code is different.  In most cases, the global symbols aren't
> actually called, and they don't follow any conventions.  The code is
> spaghetti-esque, with HW handlers and jumps everywhere.  The state of
> the stack at symbol entry varies per "function".  That's why objtool
> ignores these files.

Unfortunately, this is true.

> For special cases (like entry code), I was thinking we'd need manual CFI
> annotations, like we had before.  Or maybe there's another way, like
> some new macros which tell objtool about the HW entry points and the
> state of the registers there.
> 
> But I'm having trouble seeing how marking these code snippets with
> ENTRY/ENDPROC would help objtool make any sense of the code and where
> things are on the stack.

Ok, my intention was to have every line of assembly code in between of
FUNC_START/FUNC_END. That way, every rsp related push/pop/sub/add can be
annotated very easily. For the C-like functions this is all what needs
to be done.

Then there is the spaghetti code. And I was thinking about manual
annotations like:

  # skip the frame pointer checking between START+END here
  OBJTOOL(SKIP_CHECKING)

  # this fn has unusual frame (like interrupts have),
and you can find return RIP stored at fp + 0x20
  OBJTOOL(RIP_IS_AT, 0x20)

  # put this raw CFI for this location into eh_frame
  OBJTOOL(RAW_CFI, 0x00, 0x00, 0x00)


Similarly, I have OBJTOOL(START_FUNC) and OBJTOOL(END_FUNC) emitted with
each FUNC_START/FUNC_END. So far, when manually expanded for simplicity,
it looks like this:

#define OBJTOOL_START_FUNC  \
.pushsection .discard.asmfunctions ASM_NL   \
.long 0xfd11 ASM_NL \
.long 1f - . ASM_NL \
.popsection ASM_NL  \
1:

#define OBJTOOL_END_FUNC\
.pushsection .discard.asmfunctions ASM_NL   \
.long 0xfe11 ASM_NL \
.long 1f - . ASM_NL \
.popsection ASM_NL  \
1:

0xfd11, 0xfe11 are "opcodes" for objtool meaning
START_FUNC/END_FUNC. Similar would be SKIP_CHECKING, RIP_IS_AT, and
RAW_CFI from the above.

So on the objtool side, it looks like:
switch (data->magic) {
case 0xfd11:
pc_begin = rela->addend;
break;
case 0xfe11:
ret = dwarf_annotate_func(dwarf, rela->sym->sec,
pc_begin, rela->addend - pc_begin);
if (ret < 0)
return -1;

break;

So this was my idea -- having all code marked as function and manually
annotate those which are different.

thanks,
-- 
js
suse labs

Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Jiri Slaby]

On 04/26/2017, 03:42 AM, Josh Poimboeuf wrote:
>> @@ -323,7 +323,7 @@ ENTRY(resume_userspace)
>>  movl%esp, %eax
>>  callprepare_exit_to_usermode
>>  jmp restore_all
>> -END(ret_from_exception)
>> +ENDPROC(ret_from_exception)
> 
> What exactly is the motivation of this patch?  It would be good to
> describe that in the commit message.
> 
> Is the point to allow objtool to generate CFI for it?  If so, I don't
> really see how that would work.  Today, objtool considers ENDPROC to
> annotate a *callable* function which conforms to the C calling ABI and
> can be called by another function.  The stack is in a known state at
> function entry, and so the CFI (or frame pointer info) can be reliably
> determined.

Ugh, I haven't checked this in 100 % of cases, but this looks pretty
fragile to me. From reading the code, the use of END or ENDPROC is
rather random -- depending on mood and who wrote the code.

> But entry code is different.  In most cases, the global symbols aren't
> actually called, and they don't follow any conventions.  The code is
> spaghetti-esque, with HW handlers and jumps everywhere.  The state of
> the stack at symbol entry varies per "function".  That's why objtool
> ignores these files.

Unfortunately, this is true.

> For special cases (like entry code), I was thinking we'd need manual CFI
> annotations, like we had before.  Or maybe there's another way, like
> some new macros which tell objtool about the HW entry points and the
> state of the registers there.
> 
> But I'm having trouble seeing how marking these code snippets with
> ENTRY/ENDPROC would help objtool make any sense of the code and where
> things are on the stack.

Ok, my intention was to have every line of assembly code in between of
FUNC_START/FUNC_END. That way, every rsp related push/pop/sub/add can be
annotated very easily. For the C-like functions this is all what needs
to be done.

Then there is the spaghetti code. And I was thinking about manual
annotations like:

  # skip the frame pointer checking between START+END here
  OBJTOOL(SKIP_CHECKING)

  # this fn has unusual frame (like interrupts have),
and you can find return RIP stored at fp + 0x20
  OBJTOOL(RIP_IS_AT, 0x20)

  # put this raw CFI for this location into eh_frame
  OBJTOOL(RAW_CFI, 0x00, 0x00, 0x00)


Similarly, I have OBJTOOL(START_FUNC) and OBJTOOL(END_FUNC) emitted with
each FUNC_START/FUNC_END. So far, when manually expanded for simplicity,
it looks like this:

#define OBJTOOL_START_FUNC  \
.pushsection .discard.asmfunctions ASM_NL   \
.long 0xfd11 ASM_NL \
.long 1f - . ASM_NL \
.popsection ASM_NL  \
1:

#define OBJTOOL_END_FUNC\
.pushsection .discard.asmfunctions ASM_NL   \
.long 0xfe11 ASM_NL \
.long 1f - . ASM_NL \
.popsection ASM_NL  \
1:

0xfd11, 0xfe11 are "opcodes" for objtool meaning
START_FUNC/END_FUNC. Similar would be SKIP_CHECKING, RIP_IS_AT, and
RAW_CFI from the above.

So on the objtool side, it looks like:
switch (data->magic) {
case 0xfd11:
pc_begin = rela->addend;
break;
case 0xfe11:
ret = dwarf_annotate_func(dwarf, rela->sym->sec,
pc_begin, rela->addend - pc_begin);
if (ret < 0)
return -1;

break;

So this was my idea -- having all code marked as function and manually
annotate those which are different.

thanks,
-- 
js
suse labs

Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Josh Poimboeuf]

On Fri, Apr 21, 2017 at 04:12:40PM +0200, Jiri Slaby wrote:
> Somewhere END was used to end a function. It is not intended to be used
> for functions, because it does not mark the actual symbols as functions.
> Use ENDPROC in such cases which does the right job.
> 
> Signed-off-by: Jiri Slaby 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: "H. Peter Anvin" 
> Cc: 
> Cc: Boris Ostrovsky 
> Cc: Juergen Gross 
> Reviewed-by: Juergen Gross  [xen parts]
> Cc: 
> ---
>  arch/x86/entry/entry_32.S| 58 
> 
>  arch/x86/entry/entry_64.S| 40 +--
>  arch/x86/entry/entry_64_compat.S |  4 +--
>  arch/x86/kernel/ftrace_32.S  |  8 +++---
>  arch/x86/kernel/ftrace_64.S  | 10 +++
>  arch/x86/xen/xen-pvh.S   |  2 +-
>  6 files changed, 61 insertions(+), 61 deletions(-)
> 
> diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
> index 50bc26949e9e..a546b84aec01 100644
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -249,7 +249,7 @@ ENTRY(__switch_to_asm)
>   popl%ebp
>  
>   jmp __switch_to
> -END(__switch_to_asm)
> +ENDPROC(__switch_to_asm)
>  
>  /*
>   * A newly forked process directly context switches into this address.
> @@ -289,7 +289,7 @@ ENTRY(ret_from_fork)
>*/
>   movl$0, PT_EAX(%esp)
>   jmp 2b
> -END(ret_from_fork)
> +ENDPROC(ret_from_fork)
>  
>  /*
>   * Return to user mode is not as complex as all this looks,
> @@ -323,7 +323,7 @@ ENTRY(resume_userspace)
>   movl%esp, %eax
>   callprepare_exit_to_usermode
>   jmp restore_all
> -END(ret_from_exception)
> +ENDPROC(ret_from_exception)

What exactly is the motivation of this patch?  It would be good to
describe that in the commit message.

Is the point to allow objtool to generate CFI for it?  If so, I don't
really see how that would work.  Today, objtool considers ENDPROC to
annotate a *callable* function which conforms to the C calling ABI and
can be called by another function.  The stack is in a known state at
function entry, and so the CFI (or frame pointer info) can be reliably
determined.

But entry code is different.  In most cases, the global symbols aren't
actually called, and they don't follow any conventions.  The code is
spaghetti-esque, with HW handlers and jumps everywhere.  The state of
the stack at symbol entry varies per "function".  That's why objtool
ignores these files.

For special cases (like entry code), I was thinking we'd need manual CFI
annotations, like we had before.  Or maybe there's another way, like
some new macros which tell objtool about the HW entry points and the
state of the registers there.

But I'm having trouble seeing how marking these code snippets with
ENTRY/ENDPROC would help objtool make any sense of the code and where
things are on the stack.

-- 
Josh

Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Josh Poimboeuf]

On Fri, Apr 21, 2017 at 04:12:40PM +0200, Jiri Slaby wrote:
> Somewhere END was used to end a function. It is not intended to be used
> for functions, because it does not mark the actual symbols as functions.
> Use ENDPROC in such cases which does the right job.
> 
> Signed-off-by: Jiri Slaby 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: "H. Peter Anvin" 
> Cc: 
> Cc: Boris Ostrovsky 
> Cc: Juergen Gross 
> Reviewed-by: Juergen Gross  [xen parts]
> Cc: 
> ---
>  arch/x86/entry/entry_32.S| 58 
> 
>  arch/x86/entry/entry_64.S| 40 +--
>  arch/x86/entry/entry_64_compat.S |  4 +--
>  arch/x86/kernel/ftrace_32.S  |  8 +++---
>  arch/x86/kernel/ftrace_64.S  | 10 +++
>  arch/x86/xen/xen-pvh.S   |  2 +-
>  6 files changed, 61 insertions(+), 61 deletions(-)
> 
> diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
> index 50bc26949e9e..a546b84aec01 100644
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -249,7 +249,7 @@ ENTRY(__switch_to_asm)
>   popl%ebp
>  
>   jmp __switch_to
> -END(__switch_to_asm)
> +ENDPROC(__switch_to_asm)
>  
>  /*
>   * A newly forked process directly context switches into this address.
> @@ -289,7 +289,7 @@ ENTRY(ret_from_fork)
>*/
>   movl$0, PT_EAX(%esp)
>   jmp 2b
> -END(ret_from_fork)
> +ENDPROC(ret_from_fork)
>  
>  /*
>   * Return to user mode is not as complex as all this looks,
> @@ -323,7 +323,7 @@ ENTRY(resume_userspace)
>   movl%esp, %eax
>   callprepare_exit_to_usermode
>   jmp restore_all
> -END(ret_from_exception)
> +ENDPROC(ret_from_exception)

What exactly is the motivation of this patch?  It would be good to
describe that in the commit message.

Is the point to allow objtool to generate CFI for it?  If so, I don't
really see how that would work.  Today, objtool considers ENDPROC to
annotate a *callable* function which conforms to the C calling ABI and
can be called by another function.  The stack is in a known state at
function entry, and so the CFI (or frame pointer info) can be reliably
determined.

But entry code is different.  In most cases, the global symbols aren't
actually called, and they don't follow any conventions.  The code is
spaghetti-esque, with HW handlers and jumps everywhere.  The state of
the stack at symbol entry varies per "function".  That's why objtool
ignores these files.

For special cases (like entry code), I was thinking we'd need manual CFI
annotations, like we had before.  Or maybe there's another way, like
some new macros which tell objtool about the HW entry points and the
state of the registers there.

But I'm having trouble seeing how marking these code snippets with
ENTRY/ENDPROC would help objtool make any sense of the code and where
things are on the stack.

-- 
Josh

[PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Jiri Slaby]

Somewhere END was used to end a function. It is not intended to be used
for functions, because it does not mark the actual symbols as functions.
Use ENDPROC in such cases which does the right job.

Signed-off-by: Jiri Slaby 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: "H. Peter Anvin" 
Cc: 
Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Reviewed-by: Juergen Gross  [xen parts]
Cc: 
---
 arch/x86/entry/entry_32.S| 58 
 arch/x86/entry/entry_64.S| 40 +--
 arch/x86/entry/entry_64_compat.S |  4 +--
 arch/x86/kernel/ftrace_32.S  |  8 +++---
 arch/x86/kernel/ftrace_64.S  | 10 +++
 arch/x86/xen/xen-pvh.S   |  2 +-
 6 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 50bc26949e9e..a546b84aec01 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -249,7 +249,7 @@ ENTRY(__switch_to_asm)
popl%ebp
 
jmp __switch_to
-END(__switch_to_asm)
+ENDPROC(__switch_to_asm)
 
 /*
  * A newly forked process directly context switches into this address.
@@ -289,7 +289,7 @@ ENTRY(ret_from_fork)
 */
movl$0, PT_EAX(%esp)
jmp 2b
-END(ret_from_fork)
+ENDPROC(ret_from_fork)
 
 /*
  * Return to user mode is not as complex as all this looks,
@@ -323,7 +323,7 @@ ENTRY(resume_userspace)
movl%esp, %eax
callprepare_exit_to_usermode
jmp restore_all
-END(ret_from_exception)
+ENDPROC(ret_from_exception)
 
 #ifdef CONFIG_PREEMPT
 ENTRY(resume_kernel)
@@ -335,7 +335,7 @@ ENTRY(resume_kernel)
jz  restore_all
callpreempt_schedule_irq
jmp .Lneed_resched
-END(resume_kernel)
+ENDPROC(resume_kernel)
 #endif
 
 GLOBAL(__begin_SYSENTER_singlestep_region)
@@ -635,7 +635,7 @@ ENTRY(irq_entries_start)
jmp common_interrupt
.align  8
 .endr
-END(irq_entries_start)
+ENDPROC(irq_entries_start)
 
 /*
  * the CPU automatically disables interrupts when executing an IRQ vector,
@@ -684,7 +684,7 @@ ENTRY(coprocessor_error)
pushl   $0
pushl   $do_coprocessor_error
jmp common_exception
-END(coprocessor_error)
+ENDPROC(coprocessor_error)
 
 ENTRY(simd_coprocessor_error)
ASM_CLAC
@@ -698,20 +698,20 @@ ENTRY(simd_coprocessor_error)
pushl   $do_simd_coprocessor_error
 #endif
jmp common_exception
-END(simd_coprocessor_error)
+ENDPROC(simd_coprocessor_error)
 
 ENTRY(device_not_available)
ASM_CLAC
pushl   $-1 # mark this as an int
pushl   $do_device_not_available
jmp common_exception
-END(device_not_available)
+ENDPROC(device_not_available)
 
 #ifdef CONFIG_PARAVIRT
 ENTRY(native_iret)
iret
_ASM_EXTABLE(native_iret, iret_exc)
-END(native_iret)
+ENDPROC(native_iret)
 #endif
 
 ENTRY(overflow)
@@ -719,59 +719,59 @@ ENTRY(overflow)
pushl   $0
pushl   $do_overflow
jmp common_exception
-END(overflow)
+ENDPROC(overflow)
 
 ENTRY(bounds)
ASM_CLAC
pushl   $0
pushl   $do_bounds
jmp common_exception
-END(bounds)
+ENDPROC(bounds)
 
 ENTRY(invalid_op)
ASM_CLAC
pushl   $0
pushl   $do_invalid_op
jmp common_exception
-END(invalid_op)
+ENDPROC(invalid_op)
 
 ENTRY(coprocessor_segment_overrun)
ASM_CLAC
pushl   $0
pushl   $do_coprocessor_segment_overrun
jmp common_exception
-END(coprocessor_segment_overrun)
+ENDPROC(coprocessor_segment_overrun)
 
 ENTRY(invalid_TSS)
ASM_CLAC
pushl   $do_invalid_TSS
jmp common_exception
-END(invalid_TSS)
+ENDPROC(invalid_TSS)
 
 ENTRY(segment_not_present)
ASM_CLAC
pushl   $do_segment_not_present
jmp common_exception
-END(segment_not_present)
+ENDPROC(segment_not_present)
 
 ENTRY(stack_segment)
ASM_CLAC
pushl   $do_stack_segment
jmp common_exception
-END(stack_segment)
+ENDPROC(stack_segment)
 
 ENTRY(alignment_check)
ASM_CLAC
pushl   $do_alignment_check
jmp common_exception
-END(alignment_check)
+ENDPROC(alignment_check)
 
 ENTRY(divide_error)
ASM_CLAC
pushl   $0  # no error code
pushl   $do_divide_error
jmp common_exception
-END(divide_error)
+ENDPROC(divide_error)
 
 #ifdef CONFIG_X86_MCE
 ENTRY(machine_check)
@@ -779,7 +779,7 @@ ENTRY(machine_check)
pushl   $0
pushl   machine_check_vector
jmp common_exception
-END(machine_check)
+ENDPROC(machine_check)
 #endif
 
 ENTRY(spurious_interrupt_bug)
@@ -787,7 +787,7 @@ ENTRY(spurious_interrupt_bug)
pushl   $0
pushl   

[PATCH v3 04/29] x86: assembly, use ENDPROC for functions

[Jiri Slaby]

Somewhere END was used to end a function. It is not intended to be used
for functions, because it does not mark the actual symbols as functions.
Use ENDPROC in such cases which does the right job.

Signed-off-by: Jiri Slaby 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: "H. Peter Anvin" 
Cc: 
Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Reviewed-by: Juergen Gross  [xen parts]
Cc: 
---
 arch/x86/entry/entry_32.S| 58 
 arch/x86/entry/entry_64.S| 40 +--
 arch/x86/entry/entry_64_compat.S |  4 +--
 arch/x86/kernel/ftrace_32.S  |  8 +++---
 arch/x86/kernel/ftrace_64.S  | 10 +++
 arch/x86/xen/xen-pvh.S   |  2 +-
 6 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 50bc26949e9e..a546b84aec01 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -249,7 +249,7 @@ ENTRY(__switch_to_asm)
popl%ebp
 
jmp __switch_to
-END(__switch_to_asm)
+ENDPROC(__switch_to_asm)
 
 /*
  * A newly forked process directly context switches into this address.
@@ -289,7 +289,7 @@ ENTRY(ret_from_fork)
 */
movl$0, PT_EAX(%esp)
jmp 2b
-END(ret_from_fork)
+ENDPROC(ret_from_fork)
 
 /*
  * Return to user mode is not as complex as all this looks,
@@ -323,7 +323,7 @@ ENTRY(resume_userspace)
movl%esp, %eax
callprepare_exit_to_usermode
jmp restore_all
-END(ret_from_exception)
+ENDPROC(ret_from_exception)
 
 #ifdef CONFIG_PREEMPT
 ENTRY(resume_kernel)
@@ -335,7 +335,7 @@ ENTRY(resume_kernel)
jz  restore_all
callpreempt_schedule_irq
jmp .Lneed_resched
-END(resume_kernel)
+ENDPROC(resume_kernel)
 #endif
 
 GLOBAL(__begin_SYSENTER_singlestep_region)
@@ -635,7 +635,7 @@ ENTRY(irq_entries_start)
jmp common_interrupt
.align  8
 .endr
-END(irq_entries_start)
+ENDPROC(irq_entries_start)
 
 /*
  * the CPU automatically disables interrupts when executing an IRQ vector,
@@ -684,7 +684,7 @@ ENTRY(coprocessor_error)
pushl   $0
pushl   $do_coprocessor_error
jmp common_exception
-END(coprocessor_error)
+ENDPROC(coprocessor_error)
 
 ENTRY(simd_coprocessor_error)
ASM_CLAC
@@ -698,20 +698,20 @@ ENTRY(simd_coprocessor_error)
pushl   $do_simd_coprocessor_error
 #endif
jmp common_exception
-END(simd_coprocessor_error)
+ENDPROC(simd_coprocessor_error)
 
 ENTRY(device_not_available)
ASM_CLAC
pushl   $-1 # mark this as an int
pushl   $do_device_not_available
jmp common_exception
-END(device_not_available)
+ENDPROC(device_not_available)
 
 #ifdef CONFIG_PARAVIRT
 ENTRY(native_iret)
iret
_ASM_EXTABLE(native_iret, iret_exc)
-END(native_iret)
+ENDPROC(native_iret)
 #endif
 
 ENTRY(overflow)
@@ -719,59 +719,59 @@ ENTRY(overflow)
pushl   $0
pushl   $do_overflow
jmp common_exception
-END(overflow)
+ENDPROC(overflow)
 
 ENTRY(bounds)
ASM_CLAC
pushl   $0
pushl   $do_bounds
jmp common_exception
-END(bounds)
+ENDPROC(bounds)
 
 ENTRY(invalid_op)
ASM_CLAC
pushl   $0
pushl   $do_invalid_op
jmp common_exception
-END(invalid_op)
+ENDPROC(invalid_op)
 
 ENTRY(coprocessor_segment_overrun)
ASM_CLAC
pushl   $0
pushl   $do_coprocessor_segment_overrun
jmp common_exception
-END(coprocessor_segment_overrun)
+ENDPROC(coprocessor_segment_overrun)
 
 ENTRY(invalid_TSS)
ASM_CLAC
pushl   $do_invalid_TSS
jmp common_exception
-END(invalid_TSS)
+ENDPROC(invalid_TSS)
 
 ENTRY(segment_not_present)
ASM_CLAC
pushl   $do_segment_not_present
jmp common_exception
-END(segment_not_present)
+ENDPROC(segment_not_present)
 
 ENTRY(stack_segment)
ASM_CLAC
pushl   $do_stack_segment
jmp common_exception
-END(stack_segment)
+ENDPROC(stack_segment)
 
 ENTRY(alignment_check)
ASM_CLAC
pushl   $do_alignment_check
jmp common_exception
-END(alignment_check)
+ENDPROC(alignment_check)
 
 ENTRY(divide_error)
ASM_CLAC
pushl   $0  # no error code
pushl   $do_divide_error
jmp common_exception
-END(divide_error)
+ENDPROC(divide_error)
 
 #ifdef CONFIG_X86_MCE
 ENTRY(machine_check)
@@ -779,7 +779,7 @@ ENTRY(machine_check)
pushl   $0
pushl   machine_check_vector
jmp common_exception
-END(machine_check)
+ENDPROC(machine_check)
 #endif
 
 ENTRY(spurious_interrupt_bug)
@@ -787,7 +787,7 @@ ENTRY(spurious_interrupt_bug)
pushl   $0
pushl   $do_spurious_interrupt_bug
jmp common_exception
-END(spurious_interrupt_bug)
+ENDPROC(spurious_interrupt_bug)
 
 #ifdef CONFIG_XEN
 ENTRY(xen_hypervisor_callback)
@@ -888,7