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

H. Peter Anvin
2014-12-11T00:30:16Z

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