Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Xen-devel] [PATCH v3 0/2] x86/xen: Make idle tasks reliable
@ 2020-03-26  9:26 Miroslav Benes
  2020-03-26  9:26 ` [Xen-devel] [PATCH v3 1/2] x86/xen: Make the boot CPU idle task reliable Miroslav Benes
  2020-03-26  9:26 ` [Xen-devel] [PATCH v3 2/2] x86/xen: Make the secondary CPU idle tasks reliable Miroslav Benes
  0 siblings, 2 replies; 5+ messages in thread
From: Miroslav Benes @ 2020-03-26  9:26 UTC (permalink / raw)
  To: boris.ostrovsky, jgross, sstabellini, tglx, mingo, bp, hpa, jpoimboe
  Cc: andrew.cooper3, x86, linux-kernel, jbeulich, live-patching,
	xen-devel, Miroslav Benes, jslaby

The unwinder reports idle tasks' stack on XEN PV as unreliable which
complicates things for at least live patching. The two patches in the
series try to amend that by using similar approach as non-XEN x86 does.

v2->v3:
- change prototype of asm_cpu_bringup_and_idle()
- replace %_ASM_SP with %rsp and %esp respectively
- fix build for !CONFIG_XEN_PV_SMP

v1->v2:
- call instruction used instead of push+jmp
- initial_stack used directly

v1 https://lore.kernel.org/live-patching/20200312142007.11488-1-mbenes@suse.cz/
v2 https://lore.kernel.org/live-patching/20200319095606.23627-1-mbenes@suse.cz/

Miroslav Benes (2):
  x86/xen: Make the boot CPU idle task reliable
  x86/xen: Make the secondary CPU idle tasks reliable

 arch/x86/xen/smp_pv.c   |  3 ++-
 arch/x86/xen/xen-head.S | 18 ++++++++++++++++--
 2 files changed, 18 insertions(+), 3 deletions(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Xen-devel] [PATCH v3 1/2] x86/xen: Make the boot CPU idle task reliable
  2020-03-26  9:26 [Xen-devel] [PATCH v3 0/2] x86/xen: Make idle tasks reliable Miroslav Benes
@ 2020-03-26  9:26 ` Miroslav Benes
  2020-03-26  9:29   ` Jürgen Groß
  2020-03-26  9:26 ` [Xen-devel] [PATCH v3 2/2] x86/xen: Make the secondary CPU idle tasks reliable Miroslav Benes
  1 sibling, 1 reply; 5+ messages in thread
From: Miroslav Benes @ 2020-03-26  9:26 UTC (permalink / raw)
  To: boris.ostrovsky, jgross, sstabellini, tglx, mingo, bp, hpa, jpoimboe
  Cc: andrew.cooper3, x86, linux-kernel, jbeulich, live-patching,
	xen-devel, Miroslav Benes, jslaby

The unwinder reports the boot CPU idle task's stack on XEN PV as
unreliable, which affects at least live patching. There are two reasons
for this. First, the task does not follow the x86 convention that its
stack starts at the offset right below saved pt_regs. It allows the
unwinder to easily detect the end of the stack and verify it. Second,
startup_xen() function does not store the return address before jumping
to xen_start_kernel() which confuses the unwinder.

Amend both issues by moving the starting point of initial stack in
startup_xen() and storing the return address before the jump, which is
exactly what call instruction does.

Signed-off-by: Miroslav Benes <mbenes@suse.cz>
---
 arch/x86/xen/xen-head.S | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
index 1d0cee3163e4..d63806e1ff7a 100644
--- a/arch/x86/xen/xen-head.S
+++ b/arch/x86/xen/xen-head.S
@@ -35,7 +35,11 @@ SYM_CODE_START(startup_xen)
 	rep __ASM_SIZE(stos)
 
 	mov %_ASM_SI, xen_start_info
-	mov $init_thread_union+THREAD_SIZE, %_ASM_SP
+#ifdef CONFIG_X86_64
+	mov initial_stack(%rip), %rsp
+#else
+	mov pa(initial_stack), %esp
+#endif
 
 #ifdef CONFIG_X86_64
 	/* Set up %gs.
@@ -51,7 +55,7 @@ SYM_CODE_START(startup_xen)
 	wrmsr
 #endif
 
-	jmp xen_start_kernel
+	call xen_start_kernel
 SYM_CODE_END(startup_xen)
 	__FINIT
 #endif
-- 
2.25.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Xen-devel] [PATCH v3 2/2] x86/xen: Make the secondary CPU idle tasks reliable
  2020-03-26  9:26 [Xen-devel] [PATCH v3 0/2] x86/xen: Make idle tasks reliable Miroslav Benes
  2020-03-26  9:26 ` [Xen-devel] [PATCH v3 1/2] x86/xen: Make the boot CPU idle task reliable Miroslav Benes
@ 2020-03-26  9:26 ` Miroslav Benes
  2020-03-26  9:30   ` Jürgen Groß
  1 sibling, 1 reply; 5+ messages in thread
From: Miroslav Benes @ 2020-03-26  9:26 UTC (permalink / raw)
  To: boris.ostrovsky, jgross, sstabellini, tglx, mingo, bp, hpa, jpoimboe
  Cc: andrew.cooper3, x86, linux-kernel, jbeulich, live-patching,
	xen-devel, Miroslav Benes, jslaby

The unwinder reports the secondary CPU idle tasks' stack on XEN PV as
unreliable, which affects at least live patching.
cpu_initialize_context() sets up the context of the CPU through
VCPUOP_initialise hypercall. After it is woken up, the idle task starts
in cpu_bringup_and_idle() function and its stack starts at the offset
right below pt_regs. The unwinder correctly detects the end of stack
there but it is confused by NULL return address in the last frame.

Introduce a wrapper in assembly, which just calls
cpu_bringup_and_idle(). The return address is thus pushed on the stack
and the wrapper contains the annotation hint for the unwinder regarding
the stack state.

Signed-off-by: Miroslav Benes <mbenes@suse.cz>
---
 arch/x86/xen/smp_pv.c   |  3 ++-
 arch/x86/xen/xen-head.S | 10 ++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c
index 802ee5bba66c..8fb8a50a28b4 100644
--- a/arch/x86/xen/smp_pv.c
+++ b/arch/x86/xen/smp_pv.c
@@ -53,6 +53,7 @@ static DEFINE_PER_CPU(struct xen_common_irq, xen_irq_work) = { .irq = -1 };
 static DEFINE_PER_CPU(struct xen_common_irq, xen_pmu_irq) = { .irq = -1 };
 
 static irqreturn_t xen_irq_work_interrupt(int irq, void *dev_id);
+void asm_cpu_bringup_and_idle(void);
 
 static void cpu_bringup(void)
 {
@@ -309,7 +310,7 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle)
 	 * pointing just below where pt_regs would be if it were a normal
 	 * kernel entry.
 	 */
-	ctxt->user_regs.eip = (unsigned long)cpu_bringup_and_idle;
+	ctxt->user_regs.eip = (unsigned long)asm_cpu_bringup_and_idle;
 	ctxt->flags = VGCF_IN_KERNEL;
 	ctxt->user_regs.eflags = 0x1000; /* IOPL_RING1 */
 	ctxt->user_regs.ds = __USER_DS;
diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
index d63806e1ff7a..7d1c4fcbe8f7 100644
--- a/arch/x86/xen/xen-head.S
+++ b/arch/x86/xen/xen-head.S
@@ -58,6 +58,16 @@ SYM_CODE_START(startup_xen)
 	call xen_start_kernel
 SYM_CODE_END(startup_xen)
 	__FINIT
+
+#ifdef CONFIG_XEN_PV_SMP
+.pushsection .text
+SYM_CODE_START(asm_cpu_bringup_and_idle)
+	UNWIND_HINT_EMPTY
+
+	call cpu_bringup_and_idle
+SYM_CODE_END(asm_cpu_bringup_and_idle)
+.popsection
+#endif
 #endif
 
 .pushsection .text
-- 
2.25.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Xen-devel] [PATCH v3 1/2] x86/xen: Make the boot CPU idle task reliable
  2020-03-26  9:26 ` [Xen-devel] [PATCH v3 1/2] x86/xen: Make the boot CPU idle task reliable Miroslav Benes
@ 2020-03-26  9:29   ` Jürgen Groß
  0 siblings, 0 replies; 5+ messages in thread
From: Jürgen Groß @ 2020-03-26  9:29 UTC (permalink / raw)
  To: Miroslav Benes, boris.ostrovsky, sstabellini, tglx, mingo, bp,
	hpa, jpoimboe
  Cc: andrew.cooper3, x86, linux-kernel, jbeulich, live-patching,
	xen-devel, jslaby

On 26.03.20 10:26, Miroslav Benes wrote:
> The unwinder reports the boot CPU idle task's stack on XEN PV as
> unreliable, which affects at least live patching. There are two reasons
> for this. First, the task does not follow the x86 convention that its
> stack starts at the offset right below saved pt_regs. It allows the
> unwinder to easily detect the end of the stack and verify it. Second,
> startup_xen() function does not store the return address before jumping
> to xen_start_kernel() which confuses the unwinder.
> 
> Amend both issues by moving the starting point of initial stack in
> startup_xen() and storing the return address before the jump, which is
> exactly what call instruction does.
> 
> Signed-off-by: Miroslav Benes <mbenes@suse.cz>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Xen-devel] [PATCH v3 2/2] x86/xen: Make the secondary CPU idle tasks reliable
  2020-03-26  9:26 ` [Xen-devel] [PATCH v3 2/2] x86/xen: Make the secondary CPU idle tasks reliable Miroslav Benes
@ 2020-03-26  9:30   ` Jürgen Groß
  0 siblings, 0 replies; 5+ messages in thread
From: Jürgen Groß @ 2020-03-26  9:30 UTC (permalink / raw)
  To: Miroslav Benes, boris.ostrovsky, sstabellini, tglx, mingo, bp,
	hpa, jpoimboe
  Cc: andrew.cooper3, x86, linux-kernel, jbeulich, live-patching,
	xen-devel, jslaby

On 26.03.20 10:26, Miroslav Benes wrote:
> The unwinder reports the secondary CPU idle tasks' stack on XEN PV as
> unreliable, which affects at least live patching.
> cpu_initialize_context() sets up the context of the CPU through
> VCPUOP_initialise hypercall. After it is woken up, the idle task starts
> in cpu_bringup_and_idle() function and its stack starts at the offset
> right below pt_regs. The unwinder correctly detects the end of stack
> there but it is confused by NULL return address in the last frame.
> 
> Introduce a wrapper in assembly, which just calls
> cpu_bringup_and_idle(). The return address is thus pushed on the stack
> and the wrapper contains the annotation hint for the unwinder regarding
> the stack state.
> 
> Signed-off-by: Miroslav Benes <mbenes@suse.cz>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26  9:26 [Xen-devel] [PATCH v3 0/2] x86/xen: Make idle tasks reliable Miroslav Benes
2020-03-26  9:26 ` [Xen-devel] [PATCH v3 1/2] x86/xen: Make the boot CPU idle task reliable Miroslav Benes
2020-03-26  9:29   ` Jürgen Groß
2020-03-26  9:26 ` [Xen-devel] [PATCH v3 2/2] x86/xen: Make the secondary CPU idle tasks reliable Miroslav Benes
2020-03-26  9:30   ` Jürgen Groß

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git