WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [RFC PATCH 11/35] Add support for Xen to entry.S.

On Wednesday 22 March 2006 07:30, Chris Wright wrote:

> +
> +/* Offsets into shared_info_t. */
> +#define evtchn_upcall_pending                /* 0 */
> +#define evtchn_upcall_mask           1
> +
> +#define sizeof_vcpu_shift            6

This should be all generated in asm-offsets.c

> +#else

This needs a comment explaining why it is different

> +restore_nocheck:
> +     testl $(VM_MASK|NMI_MASK), EFLAGS(%esp)
> +     jnz hypervisor_iret
> +     movb EVENT_MASK(%esp), %al
> +     notb %al                        # %al == ~saved_mask
> +     GET_VCPU_INFO
> +     andb evtchn_upcall_mask(%esi),%al
> +     andb $1,%al                     # %al == mask & ~saved_mask

This would be faster if you used 32bit because it wouldn't cause
partial register stalls.

> +     jnz restore_all_enable_events   #     != 0 => reenable event delivery
> +#endif
>       RESTORE_REGS
>       addl $4, %esp
>  1:   iret
>  .section .fixup,"ax"
>  iret_exc:
> -     sti
> +#ifndef CONFIG_XEN
> +     ENABLE_INTERRUPTS
> +#endif
>       pushl $0                        # no error code
>       pushl $do_iret_error
>       jmp error_code
> @@ -269,6 +317,7 @@ iret_exc:
>       .long 1b,iret_exc
>  .previous
>  
> +#ifndef CONFIG_XEN
>  ldt_ss:

So are you sure that problem this ugly piece of code tries to work around
isn't in Xen kernels too? Or do you just not care? If yes add a comment.

>       larl OLDSS(%esp), %eax
>       jnz restore_nocheck
> @@ -281,7 +330,7 @@ ldt_ss:
>        * CPUs, which we can try to work around to make
>        * dosemu and wine happy. */
>       subl $8, %esp           # reserve space for switch16 pointer
> -     cli
> +     DISABLE_INTERRUPTS
>       movl %esp, %eax
>       /* Set up the 16bit stack frame with switch32 pointer on top,
>        * and a switch16 pointer on top of the current frame. */
> @@ -293,6 +342,13 @@ ldt_ss:
>       .align 4
>       .long 1b,iret_exc
>  .previous
> +#else

Needs comment.

> +hypervisor_iret:
> +     andl $~NMI_MASK, EFLAGS(%esp)
> +     RESTORE_REGS
> +     addl $4, %esp
> +     jmp  hypercall_page + (__HYPERVISOR_iret * 32)
> +#endif
>  

>  
>  ENTRY(divide_error)
>       pushl $0                        # no error code
> @@ -462,6 +522,126 @@ error_code:
>       call *%edi
>       jmp ret_from_exception
>  
> +#ifdef CONFIG_XEN

It would be nicer to put that one into a separate file

> +# A note on the "critical region" in our callback handler.




>       pushl $do_coprocessor_error
> @@ -475,17 +655,19 @@ ENTRY(simd_coprocessor_error)
>  ENTRY(device_not_available)
>       pushl $-1                       # mark this as an int
>       SAVE_ALL
> +#ifndef CONFIG_XEN
>       movl %cr0, %eax
>       testl $0x4, %eax                # EM (math emulation bit)
> -     jne device_not_available_emulate
> -     preempt_stop
> -     call math_state_restore
> -     jmp ret_from_exception
> -device_not_available_emulate:
> +     je device_available_emulate
>       pushl $0                        # temporary storage for ORIG_EIP
>       call math_emulate
>       addl $4, %esp
>       jmp ret_from_exception
> +device_available_emulate:
> +#endif
> +     preempt_stop
> +     call math_state_restore
> +     jmp ret_from_exception

It's not quite clear why the Xen special case is needed here. Just because
of the CR0 access? If yes better put that one into a macro that just writes
0 to the register for the Xen case.

-Andi

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>