[PATCHv1 0/3]: xen: voluntary preemption for privcmd hypercalls

[PATCHv1 0/3]: xen: voluntary preemption for privcmd hypercalls

[David Vrabel]

This series adds a voluntary preemption point into hypercalls issued
by privcmd.  Without this, long running hypercalls will prevent the
task from being scheduled (potentially for several seconds) which may
trigger the kernel's soft lockup detector.

I've added a stub is_preemptible_hypercall() for ARM but would
appreciate a working implementation.  Checking whether the PC is
within the privcmd_hypercall function would probably do.

David

[PATCH 1/3] x86/xen: allow for privcmd hypercalls to be preempted

[David Vrabel]

From: Andrew Cooper <andrew.cooper3@citrix.com>

Hypercalls submitted by user space tools via the privcmd driver can
take a long time (potentially many 10s of seconds) if the hypercall
has many sub-operations.

A fully preemptible kernel may deschedule such as task in any upcall
called from a hypercall continuation.

However, in a kernel with only voluntary preemption, hypercall
continuations in Xen allow event handlers to be run but the task
issuing the hypercall will not be descheduled until the hypercall is
complete and the ioctl returns to user space.  These long running
tasks may also trigger the kernel's soft lockup detection.

There needs to be a voluntary preemption point (cond_resched()) at the
end of an upcall, but only if the interrupted task had issued a
hypercall via the privcmd driver.  Add is_preemptible_hypercall()
which may be used in a upcall to determine this.

Implement is_premptible_hypercall() by adding a second hypercall page
(preemptible_hypercall_page, copied from hypercall_page).  Calls made
via the new page may be preempted.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 arch/x86/include/asm/xen/hypercall.h |   14 ++++++++++++++
 arch/x86/xen/enlighten.c             |    7 +++++++
 arch/x86/xen/xen-head.S              |   18 +++++++++++++++++-
 3 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
index e709884..4658a75 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
@@ -83,6 +83,16 @@
  */
 
 extern struct { char _entry[32]; } hypercall_page[];
+#ifndef CONFIG_PREEMPT
+extern struct { char _entry[32]; } preemptible_hypercall_page[];
+
+static inline bool is_preemptible_hypercall(struct pt_regs *regs)
+{
+	return !user_mode_vm(regs) &&
+		regs->ip >= (unsigned long)preemptible_hypercall_page &&
+		regs->ip < (unsigned long)preemptible_hypercall_page + PAGE_SIZE;
+}
+#endif
 
 #define __HYPERCALL		"call hypercall_page+%c[offset]"
 #define __HYPERCALL_ENTRY(x)						\
@@ -215,7 +225,11 @@ privcmd_call(unsigned call,
 
 	asm volatile("call *%[call]"
 		     : __HYPERCALL_5PARAM
+#ifndef CONFIG_PREEMPT
+		     : [call] "a" (&preemptible_hypercall_page[call])
+#else
 		     : [call] "a" (&hypercall_page[call])
+#endif
 		     : __HYPERCALL_CLOBBER5);
 
 	return (long)__res;
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index a4d7b64..7320fa8 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -84,6 +84,9 @@
 #include "multicalls.h"
 
 EXPORT_SYMBOL_GPL(hypercall_page);
+#ifndef CONFIG_PREEMPT
+EXPORT_SYMBOL_GPL(preemptible_hypercall_page);
+#endif
 
 /*
  * Pointer to the xen_vcpu_info structure or
@@ -1517,6 +1520,10 @@ asmlinkage void __init xen_start_kernel(void)
 	xen_pvh_early_guest_init();
 	xen_setup_machphys_mapping();
 
+#ifndef CONFIG_PREEMPT
+	copy_page(preemptible_hypercall_page, hypercall_page);
+#endif
+
 	/* Install Xen paravirt ops */
 	pv_info = xen_info;
 	pv_init_ops = xen_init_ops;
diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
index 485b695..0407d48 100644
--- a/arch/x86/xen/xen-head.S
+++ b/arch/x86/xen/xen-head.S
@@ -50,9 +50,18 @@ ENTRY(startup_xen)
 .pushsection .text
 	.balign PAGE_SIZE
 ENTRY(hypercall_page)
+
+#ifdef CONFIG_PREEMPT
+#  define PREEMPT_HYPERCALL_ENTRY(x)
+#else
+#  define PREEMPT_HYPERCALL_ENTRY(x) \
+	.global xen_hypercall_##x ## _p ASM_NL \
+	.set preemptible_xen_hypercall_##x, xen_hypercall_##x + PAGE_SIZE ASM_NL
+#endif
 #define NEXT_HYPERCALL(x) \
 	ENTRY(xen_hypercall_##x) \
-	.skip 32
+	.skip 32 ASM_NL \
+	PREEMPT_HYPERCALL_ENTRY(x)
 
 NEXT_HYPERCALL(set_trap_table)
 NEXT_HYPERCALL(mmu_update)
@@ -103,6 +112,13 @@ NEXT_HYPERCALL(arch_4)
 NEXT_HYPERCALL(arch_5)
 NEXT_HYPERCALL(arch_6)
 	.balign PAGE_SIZE
+
+#ifndef CONFIG_PREEMPT
+ENTRY(preemptible_hypercall_page)
+	.skip PAGE_SIZE
+#endif /* CONFIG_PREEMPT */
+
+#undef NEXT_HYPERCALL
 .popsection
 
 	ELFNOTE(Xen, XEN_ELFNOTE_GUEST_OS,       .asciz "linux")
-- 
1.7.2.5

Re: [PATCH 1/3] x86/xen: allow for privcmd hypercalls to be preempted

[Ian Campbell]

On Tue, 2014-02-11 at 19:19 +0000, David Vrabel wrote:
> Implement is_premptible_hypercall() by adding a second hypercall page
> (preemptible_hypercall_page, copied from hypercall_page).  Calls made
> via the new page may be preempted.

Wouldn't a per-cpu flag variable set for the duration of privcmd_call do
the job just as well without requiring per-arch knowledge?

Ian.

Re: [PATCH 1/3] x86/xen: allow for privcmd hypercalls to be preempted

[Andrew Cooper]

On 12/02/14 09:38, Ian Campbell wrote:
> On Tue, 2014-02-11 at 19:19 +0000, David Vrabel wrote:
>> Implement is_premptible_hypercall() by adding a second hypercall page
>> (preemptible_hypercall_page, copied from hypercall_page).  Calls made
>> via the new page may be preempted.
> Wouldn't a per-cpu flag variable set for the duration of privcmd_call do
> the job just as well without requiring per-arch knowledge?
>
> Ian.
>

Why should privcmd_call be the only preemptible calls?  This code allows
anyone to voluntarily use the preemptible_hypercall_page.  I would not
be surprised if modules like gntdev could do with some preemption, but
it is the long-running toolstack hypercalls which are currently killing
any loaded XenServer system.

~Andrew

Re: [PATCH 1/3] x86/xen: allow for privcmd hypercalls to be preempted

[Ian Campbell]

On Wed, 2014-02-12 at 10:10 +0000, Andrew Cooper wrote:
> On 12/02/14 09:38, Ian Campbell wrote:
> > On Tue, 2014-02-11 at 19:19 +0000, David Vrabel wrote:
> >> Implement is_premptible_hypercall() by adding a second hypercall page
> >> (preemptible_hypercall_page, copied from hypercall_page).  Calls made
> >> via the new page may be preempted.
> > Wouldn't a per-cpu flag variable set for the duration of privcmd_call do
> > the job just as well without requiring per-arch knowledge?
> >
> > Ian.
> >
> 
> Why should privcmd_call be the only preemptible calls?  This code allows
> anyone to voluntarily use the preemptible_hypercall_page.  I would not
> be surprised if modules like gntdev could do with some preemption, but
> it is the long-running toolstack hypercalls which are currently killing
> any loaded XenServer system.

Other sites could equally well use the flag, you could even wrap it up
in xen_preemptible_hypercall_{start,end}.

Ian.

[PATCH 2/3] arm/xen: add stub is_preemptible_hypercall()

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

is_preemptible_hypercall() may be called from an upcall to determine
if the interrupted task was in a preemptible hypercall.

Provide a stub implementation that always returns false.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 arch/arm/include/asm/xen/hypercall.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/xen/hypercall.h b/arch/arm/include/asm/xen/hypercall.h
index 7704e28..4b988fe 100644
--- a/arch/arm/include/asm/xen/hypercall.h
+++ b/arch/arm/include/asm/xen/hypercall.h
@@ -68,4 +68,12 @@ HYPERVISOR_multicall(void *call_list, int nr_calls)
 {
 	BUG();
 }
+
+#ifndef CONFIG_PREEMPT
+static inline bool is_preemptible_hypercall(struct pt_regs *regs)
+{
+	return false;
+}
+#endif
+
 #endif /* _ASM_ARM_XEN_HYPERCALL_H */
-- 
1.7.2.5

[PATCH 3/3] xen/events: schedule if the interrupted task is in a preemptible hypercall

[David Vrabel]

From: Andrew Cooper <andrew.cooper3@citrix.com>

In evtchn_do_upcall(), if the interrupted task was in a preemptible
hypercall add a conditional schedule point.

This allows tasks in long running preemptible hypercalls to be
descheduled.  Allowing other tasks to run and prevents long running
hypercalls issued via the privcmd driver from triggering soft lockups.

Signed-off-by: Andrew Cooper <andrew.cooper3.citrix.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 drivers/xen/events/events_base.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 4672e00..0b5df37 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1254,6 +1254,12 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
 
 	irq_exit();
 	set_irq_regs(old_regs);
+
+#ifndef CONFIG_PREEMPT
+	if ( __this_cpu_read(xed_nesting_count) == 0
+	     && is_preemptible_hypercall(regs) )
+		_cond_resched();
+#endif
 }
 
 void xen_hvm_evtchn_do_upcall(void)
-- 
1.7.2.5

Re: [PATCH 3/3] xen/events: schedule if the interrupted task is in a preemptible hypercall

[Jan Beulich]

>>> On 11.02.14 at 20:19, David Vrabel <david.vrabel@citrix.com> wrote:
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -1254,6 +1254,12 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
>  
>  	irq_exit();
>  	set_irq_regs(old_regs);
> +
> +#ifndef CONFIG_PREEMPT
> +	if ( __this_cpu_read(xed_nesting_count) == 0
> +	     && is_preemptible_hypercall(regs) )
> +		_cond_resched();
> +#endif

I don't think this can be done here - a 64-bit x86 kernel would
generally be on the IRQ stack, and I don't think scheduling
should be done in this state.

Jan

Re: [PATCH 3/3] xen/events: schedule if the interrupted task is in a preemptible hypercall

[David Vrabel]

On 12/02/14 11:59, Jan Beulich wrote:
>>>> On 11.02.14 at 20:19, David Vrabel <david.vrabel@citrix.com> wrote:
>> --- a/drivers/xen/events/events_base.c
>> +++ b/drivers/xen/events/events_base.c
>> @@ -1254,6 +1254,12 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
>>  
>>  	irq_exit();
>>  	set_irq_regs(old_regs);
>> +
>> +#ifndef CONFIG_PREEMPT
>> +	if ( __this_cpu_read(xed_nesting_count) == 0
>> +	     && is_preemptible_hypercall(regs) )
>> +		_cond_resched();
>> +#endif
> 
> I don't think this can be done here - a 64-bit x86 kernel would
> generally be on the IRQ stack, and I don't think scheduling
> should be done in this state.

_cond_resched() doesn't look that different from preempt_schedule_irq()
which is explicitly callable from irq context.

David

Re: [PATCH 3/3] xen/events: schedule if the interrupted task is in a preemptible hypercall

[Jan Beulich]

>>> On 12.02.14 at 13:54, David Vrabel <david.vrabel@citrix.com> wrote:
> On 12/02/14 11:59, Jan Beulich wrote:
>>>>> On 11.02.14 at 20:19, David Vrabel <david.vrabel@citrix.com> wrote:
>>> --- a/drivers/xen/events/events_base.c
>>> +++ b/drivers/xen/events/events_base.c
>>> @@ -1254,6 +1254,12 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
>>>  
>>>  	irq_exit();
>>>  	set_irq_regs(old_regs);
>>> +
>>> +#ifndef CONFIG_PREEMPT
>>> +	if ( __this_cpu_read(xed_nesting_count) == 0
>>> +	     && is_preemptible_hypercall(regs) )
>>> +		_cond_resched();
>>> +#endif
>> 
>> I don't think this can be done here - a 64-bit x86 kernel would
>> generally be on the IRQ stack, and I don't think scheduling
>> should be done in this state.
> 
> _cond_resched() doesn't look that different from preempt_schedule_irq()
> which is explicitly callable from irq context.

But IRQ context and running on the IRQ stack isn't the same. All
current callers of that function are in assembly code, where one
would hope people know what they're doing (and in particular
_when_ they do so).

Jan

Re: [PATCH 3/3] xen/events: schedule if the interrupted task is in a preemptible hypercall

[David Vrabel]

On 12/02/14 13:56, Jan Beulich wrote:
>>>> On 12.02.14 at 13:54, David Vrabel <david.vrabel@citrix.com> wrote:
>> On 12/02/14 11:59, Jan Beulich wrote:
>>>>>> On 11.02.14 at 20:19, David Vrabel <david.vrabel@citrix.com> wrote:
>>>> --- a/drivers/xen/events/events_base.c
>>>> +++ b/drivers/xen/events/events_base.c
>>>> @@ -1254,6 +1254,12 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
>>>>  
>>>>  	irq_exit();
>>>>  	set_irq_regs(old_regs);
>>>> +
>>>> +#ifndef CONFIG_PREEMPT
>>>> +	if ( __this_cpu_read(xed_nesting_count) == 0
>>>> +	     && is_preemptible_hypercall(regs) )
>>>> +		_cond_resched();
>>>> +#endif
>>>
>>> I don't think this can be done here - a 64-bit x86 kernel would
>>> generally be on the IRQ stack, and I don't think scheduling
>>> should be done in this state.
>>
>> _cond_resched() doesn't look that different from preempt_schedule_irq()
>> which is explicitly callable from irq context.
> 
> But IRQ context and running on the IRQ stack isn't the same. All
> current callers of that function are in assembly code, where one
> would hope people know what they're doing (and in particular
> _when_ they do so).

Ok.

I'm not sure I can claim I know what I'm doing, but I think this does
the right thing now.

8<--------------------------------------
>From 3094ed5851697b8bffe1227d32c1f1022e553191 Mon Sep 17 00:00:00 2001
From: David Vrabel <david.vrabel@citrix.com>
Date: Tue, 11 Feb 2014 15:41:03 +0000
Subject: [PATCH] xen: allow privcmd hypercalls to be preempted

Hypercalls submitted by user space tools via the privcmd driver can
take a long time (potentially many 10s of seconds) if the hypercall
has many sub-operations.

A fully preemptible kernel may deschedule such a task in any upcall
called from a hypercall continuation.

However, in a kernel with voluntary or no preemption, hypercall
continuations in Xen allow event handlers to be run but the task
issuing the hypercall will not be descheduled until the hypercall is
complete and the ioctl returns to user space.  These long running
tasks may also trigger the kernel's soft lockup detection.

Add xen_preemptible_hcall_begin() and xen_preemptible_hcall_end() to
bracket hypercalls that may be preempted.  Use these in the privcmd
driver.

When returning from an upcall, call preempt_schedule_irq() if the
current task was within a preemptible hypercall.

Since preempt_schedule_irq() can move the task to a different CPU,
clear and set xen_in_preemptible_hcall around the call.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 arch/x86/kernel/entry_64.S       |   23 ++++++++++++++++++++++-
 drivers/xen/Makefile             |    2 +-
 drivers/xen/events/events_base.c |    1 +
 drivers/xen/preempt.c            |   16 ++++++++++++++++
 drivers/xen/privcmd.c            |    2 ++
 include/xen/xen-ops.h            |   27 +++++++++++++++++++++++++++
 6 files changed, 69 insertions(+), 2 deletions(-)
 create mode 100644 drivers/xen/preempt.c

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 1e96c36..e614aaa 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -1404,7 +1404,7 @@ ENTRY(xen_do_hypervisor_callback)   #
do_hypervisor_callback(struct *pt_regs)
 	popq %rsp
 	CFI_DEF_CFA_REGISTER rsp
 	decl PER_CPU_VAR(irq_count)
-	jmp  error_exit
+	jmp  xen_error_exit
 	CFI_ENDPROC
 END(xen_do_hypervisor_callback)

@@ -1470,6 +1470,26 @@ END(xen_failsafe_callback)
 apicinterrupt3 HYPERVISOR_CALLBACK_VECTOR \
 	xen_hvm_callback_vector xen_evtchn_do_upcall

+ENTRY(xen_error_exit)
+	DEFAULT_FRAME
+	movl %ebx,%eax
+	RESTORE_REST
+	DISABLE_INTERRUPTS(CLBR_NONE)
+	TRACE_IRQS_OFF
+	GET_THREAD_INFO(%rcx)
+	testl %eax,%eax
+	je error_exit_user
+#ifndef CONFIG_PREEMPT
+	testb $0, PER_CPU_VAR(xen_in_preemptible_hcall)
+	je retint_kernel
+	movb $0, PER_CPU_VAR(xen_in_preemptible_hcall)
+	call preempt_schedule_irq
+	movb $1, PER_CPU_VAR(xen_in_preemptible_hcall)
+#endif
+	jmp retint_kernel
+	CFI_ENDPROC
+END(xen_error_exit)
+
 #endif /* CONFIG_XEN */

 #if IS_ENABLED(CONFIG_HYPERV)
@@ -1629,6 +1649,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
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index d75c811..f8c7e04 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -2,7 +2,7 @@ ifeq ($(filter y, $(CONFIG_ARM) $(CONFIG_ARM64)),)
 obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
 endif
 obj-$(CONFIG_X86)			+= fallback.o
-obj-y	+= grant-table.o features.o balloon.o manage.o
+obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o
 obj-y	+= events/
 obj-y	+= xenbus/

diff --git a/drivers/xen/events/events_base.c
b/drivers/xen/events/events_base.c
index 4672e00..db9584a 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -32,6 +32,7 @@
 #include <linux/slab.h>
 #include <linux/irqnr.h>
 #include <linux/pci.h>
+#include <linux/sched.h>

 #ifdef CONFIG_X86
 #include <asm/desc.h>
diff --git a/drivers/xen/preempt.c b/drivers/xen/preempt.c
new file mode 100644
index 0000000..3275ffe
--- /dev/null
+++ b/drivers/xen/preempt.c
@@ -0,0 +1,16 @@
+/*
+ * Preemptible hypercalls
+ *
+ * Copyright (C) 2014 Citrix Systems R&D ltd.
+ *
+ * This source code is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ */
+
+#include <xen/xen-ops.h>
+
+#ifndef CONFIG_PREEMPT
+DEFINE_PER_CPU(bool, xen_in_preemptible_hcall);
+#endif
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 569a13b..59ac71c 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -56,10 +56,12 @@ static long privcmd_ioctl_hypercall(void __user *udata)
 	if (copy_from_user(&hypercall, udata, sizeof(hypercall)))
 		return -EFAULT;

+	xen_preemptible_hcall_begin();
 	ret = privcmd_call(hypercall.op,
 			   hypercall.arg[0], hypercall.arg[1],
 			   hypercall.arg[2], hypercall.arg[3],
 			   hypercall.arg[4]);
+	xen_preemptible_hcall_end();

 	return ret;
 }
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index fb2ea8f..6d8c042 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -35,4 +35,31 @@ int xen_unmap_domain_mfn_range(struct vm_area_struct
*vma,
 			       int numpgs, struct page **pages);

 bool xen_running_on_version_or_later(unsigned int major, unsigned int
minor);
+
+#ifdef CONFIG_PREEMPT
+
+static inline void xen_preemptible_hcall_begin(void)
+{
+}
+
+static inline void xen_preemptible_hcall_end(void)
+{
+}
+
+#else
+
+DECLARE_PER_CPU(bool, xen_in_preemptible_hcall);
+
+static inline void xen_preemptible_hcall_begin(void)
+{
+	__this_cpu_write(xen_in_preemptible_hcall, true);
+}
+
+static inline void xen_preemptible_hcall_end(void)
+{
+	__this_cpu_write(xen_in_preemptible_hcall, false);
+}
+
+#endif /* CONFIG_PREEMPT */
+
 #endif /* INCLUDE_XEN_OPS_H */
-- 
1.7.2.5

Re: [PATCH 3/3] xen/events: schedule if the interrupted task is in a preemptible hypercall

[Jan Beulich]

>>> On 12.02.14 at 17:35, David Vrabel <david.vrabel@citrix.com> wrote:
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -1404,7 +1404,7 @@ ENTRY(xen_do_hypervisor_callback)   #
> do_hypervisor_callback(struct *pt_regs)
>  	popq %rsp
>  	CFI_DEF_CFA_REGISTER rsp
>  	decl PER_CPU_VAR(irq_count)
> -	jmp  error_exit
> +	jmp  xen_error_exit

Any reason not to put all the new code right here, instead of the
jmp?

>  	CFI_ENDPROC
>  END(xen_do_hypervisor_callback)
> 
> @@ -1470,6 +1470,26 @@ END(xen_failsafe_callback)
>  apicinterrupt3 HYPERVISOR_CALLBACK_VECTOR \
>  	xen_hvm_callback_vector xen_evtchn_do_upcall
> 
> +ENTRY(xen_error_exit)
> +	DEFAULT_FRAME
> +	movl %ebx,%eax
> +	RESTORE_REST
> +	DISABLE_INTERRUPTS(CLBR_NONE)
> +	TRACE_IRQS_OFF
> +	GET_THREAD_INFO(%rcx)
> +	testl %eax,%eax
> +	je error_exit_user
> +#ifndef CONFIG_PREEMPT
> +	testb $0, PER_CPU_VAR(xen_in_preemptible_hcall)
> +	je retint_kernel

This is effectively an unconditional branch now. You either want
cmpb instead of testb or $0xff instead of $0.

Jan

> +	movb $0, PER_CPU_VAR(xen_in_preemptible_hcall)
> +	call preempt_schedule_irq
> +	movb $1, PER_CPU_VAR(xen_in_preemptible_hcall)
> +#endif
> +	jmp retint_kernel
> +	CFI_ENDPROC
> +END(xen_error_exit)
> +
>  #endif /* CONFIG_XEN */
> 
>  #if IS_ENABLED(CONFIG_HYPERV)