Re: [PATCH 2/2] Xen/x86: Improve information from domain_crash_synchronous

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 05/09/13 09:15, Jan Beulich wrote:
>>>> On 04.09.13 at 20:18, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> As it currently stands, the string "domain_crash_sync called from entry.S" is
>> not helpful at identifying why the domain was crashed, and a debug build of
>> Xen doesn't help the matter
>>
>> This patch improves the information printed, by pointing to where the crash
>> decision was made.
> Looks quite useful.
>
>> --- a/xen/arch/x86/x86_64/compat/entry.S
>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>> @@ -263,6 +263,7 @@ ENTRY(compat_int80_direct_trap)
>>  /*   {[ERRCODE,] EIP, CS, EFLAGS, [ESP, SS]}                             */
>>  /* %rdx: trap_bounce, %rbx: struct vcpu                                  */
>>  /* On return only %rbx and %rdx are guaranteed non-clobbered.            */
>> +.globl compat_create_bounce_frame
>>  compat_create_bounce_frame:
> Is the addition above a left-over? I don't see any use of the
> label outside of this file.

This is for the benifit of print_symbol(), which will now give
create_bounce_frame()+some rather than trap_nop()+loads.

>
>> @@ -329,7 +330,12 @@ UNLIKELY_END(compat_bounce_failsafe)
>>          movzwl TRAPBOUNCE_cs(%rdx),%eax
>>          /* Null selectors (0-3) are not allowed. */
>>          testl $~3,%eax
>> -        jz    domain_crash_synchronous
>> +.Lcompat_bounce_null_selector:
>> +UNLIKELY_START(z, compat_bounce_null_selector)
>> +        lea   .Lcompat_bounce_null_selector(%rip), %rdi
>> +        jmp   asm_domain_crash_synchronous
>> +        ud2a
>> +UNLIKELY_END(compat_bounce_null_selector)
> Here and further down you don't really need the label at the
> start of the unlikely section - the place can as well be identified
> by using
>
>         lea   (%rip), %rdi
>
> inside that section (the place is still unique, just outside the
> original code stream, i.e. just slightly more difficult to
> re-associate).
>
> Jan
>

But in an unlikely section, %rip is shifted quite a lot from %rip of the
code immediately before.  This is also for the benefit of print_symbol()
which will pick up the {compat_,}create_bounce_frame rather than the
global symbol surrounding the unlikely section.

~Andrew