Re: [PATCH v2 2/2] x86/xen: allow privcmd hypercalls to be preempted

["H. Peter Anvin" <hpa@zytor.com>]

On 12/10/2014 03:34 PM, Luis R. Rodriguez wrote:
> diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
> index 344b63f..40b5c0c 100644
> --- a/arch/x86/kernel/entry_32.S
> +++ b/arch/x86/kernel/entry_32.S
> @@ -982,7 +982,28 @@ ENTRY(xen_hypervisor_callback)
>  ENTRY(xen_do_upcall)
>  1:	mov %esp, %eax
>  	call xen_evtchn_do_upcall
> +#ifdef CONFIG_PREEMPT
>  	jmp  ret_from_intr
> +#else
> +	GET_THREAD_INFO(%ebp)
> +#ifdef CONFIG_VM86
> +	movl PT_EFLAGS(%esp), %eax	# mix EFLAGS and CS
> +	movb PT_CS(%esp), %al
> +	andl $(X86_EFLAGS_VM | SEGMENT_RPL_MASK), %eax
> +#else
> +	movl PT_CS(%esp), %eax
> +	andl $SEGMENT_RPL_MASK, %eax
> +#endif
> +	cmpl $USER_RPL, %eax
> +	jae resume_userspace		# returning to v8086 or userspace
> +	DISABLE_INTERRUPTS(CLBR_ANY)
> +	cmpb $0,PER_CPU_VAR(xen_in_preemptible_hcall)
> +	jz resume_kernel
> +	movb $0,PER_CPU_VAR(xen_in_preemptible_hcall)
> +	call cond_resched_irq
> +	movb $1,PER_CPU_VAR(xen_in_preemptible_hcall)
> +	jmp resume_kernel
> +#endif /* CONFIG_PREEMPT */
>  	CFI_ENDPROC
>  ENDPROC(xen_hypervisor_callback)
>  
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index c0226ab..0ccdd06 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -1170,7 +1170,23 @@ ENTRY(xen_do_hypervisor_callback)   # do_hypervisor_callback(struct *pt_regs)
>  	popq %rsp
>  	CFI_DEF_CFA_REGISTER rsp
>  	decl PER_CPU_VAR(irq_count)
> +#ifdef CONFIG_PREEMPT
>  	jmp  error_exit
> +#else
> +	movl %ebx, %eax
> +	RESTORE_REST
> +	DISABLE_INTERRUPTS(CLBR_NONE)
> +	TRACE_IRQS_OFF
> +	GET_THREAD_INFO(%rcx)
> +	testl %eax, %eax
> +	je error_exit_user
> +	cmpb $0,PER_CPU_VAR(xen_in_preemptible_hcall)
> +	jz retint_kernel
> +	movb $0,PER_CPU_VAR(xen_in_preemptible_hcall)
> +	call cond_resched_irq
> +	movb $1,PER_CPU_VAR(xen_in_preemptible_hcall)
> +	jmp retint_kernel
> +#endif /* CONFIG_PREEMPT */
>  	CFI_ENDPROC
>  END(xen_do_hypervisor_callback)
>  
> @@ -1398,6 +1414,7 @@ ENTRY(error_exit)
>  	GET_THREAD_INFO(%rcx)
>  	testl %eax,%eax
>  	jne retint_kernel
> +error_exit_user:
>  	LOCKDEP_SYS_EXIT_IRQ
>  	movl TI_flags(%rcx),%edx
>  	movl $_TIF_WORK_MASK,%edi

You're adding a bunch of code for the *non*-preemptive case here... why?

	-hpa