Live-Patching Archive on lore.kernel.org
 help / color / Atom feed
From: Miroslav Benes <mbenes@suse.cz>
To: "Jürgen Groß" <jgross@suse.com>
Cc: boris.ostrovsky@oracle.com, sstabellini@kernel.org,
	tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
	hpa@zytor.com, jpoimboe@redhat.com, x86@kernel.org,
	xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org,
	live-patching@vger.kernel.org, jslaby@suse.cz
Subject: Re: [RFC PATCH 2/2] x86/xen: Make the secondary CPU idle tasks reliable
Date: Fri, 13 Mar 2020 10:54:12 +0100 (CET)
Message-ID: <alpine.LSU.2.21.2003131048110.30076@pobox.suse.cz> (raw)
In-Reply-To: <75224ad1-f160-802a-9d72-b092ba864fb7@suse.com>

[-- Attachment #1: Type: text/plain, Size: 2944 bytes --]

On Fri, 13 Mar 2020, Jürgen Groß wrote:

> On 12.03.20 15:20, 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.
> > 
> > RFC: I haven't found the way to teach the unwinder about the state of
> > the stack there. Thus the ugly hack using assembly. Similar to what
> > startup_xen() has got for boot CPU.
> > 
> > It introduces objtool "unreachable instruction" warning just right after
> > the jump to cpu_bringup_and_idle(). It should show the idea what needs
> > to be done though, I think. Ideas welcome.
> > 
> > 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..6b88cdcbef8f 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);
> > +extern unsigned char asm_cpu_bringup_and_idle[];
> >   
> >   static void cpu_bringup(void)
> >   {
> 
> Would adding this here work?
> 
> +	asm volatile (UNWIND_HINT(ORC_REG_UNDEFINED, 0, ORC_TYPE_CALL, 1));

I tried something similar. It did not work, because than the hint is 
"bound" to the closest next call in the function which is cr4_init() in 
this case. The unwinder would not take it into account.

In my case, I placed it at the beginning of cpu_bringup_and_idle(). I also 
open coded it and played with the offset in the orc entry, but that did 
not work for some other reason.

However, now I tried this

diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c
index 6b88cdcbef8f..39afd88309cb 100644
--- a/arch/x86/xen/smp_pv.c
+++ b/arch/x86/xen/smp_pv.c
@@ -92,6 +92,7 @@ asmlinkage __visible void cpu_bringup_and_idle(void)
 {
        cpu_bringup();
        boot_init_stack_canary();
+       asm volatile (UNWIND_HINT(ORC_REG_UNDEFINED, 0, ORC_TYPE_CALL, 1));
        cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
 }

and that seems to work. I need to properly verify and test, but the 
explanation is that as opposed to the above, cpu_startup_entry() is on the 
idle task's stack and the hint is then taken into account. The unwound 
stack seems to be complete, so it could indeed be the fix.

Thanks
Miroslav

  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-12 14:20 [PATCH 0/2] x86/xen: Make " Miroslav Benes
2020-03-12 14:20 ` [PATCH 1/2] x86/xen: Make the boot CPU idle task reliable Miroslav Benes
2020-03-12 15:04   ` [Xen-devel] " Andrew Cooper
2020-03-12 15:17     ` Miroslav Benes
2020-03-16 14:33   ` Boris Ostrovsky
2020-03-17  9:13     ` Miroslav Benes
2020-03-12 14:20 ` [RFC PATCH 2/2] x86/xen: Make the secondary CPU idle tasks reliable Miroslav Benes
2020-03-13  8:26   ` Jürgen Groß
2020-03-13  9:54     ` Miroslav Benes [this message]
2020-03-16 15:51       ` Miroslav Benes
2020-03-16 20:35         ` Josh Poimboeuf
2020-03-17  9:16           ` Miroslav Benes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LSU.2.21.2003131048110.30076@pobox.suse.cz \
    --to=mbenes@suse.cz \
    --cc=boris.ostrovsky@oracle.com \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jgross@suse.com \
    --cc=jpoimboe@redhat.com \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=sstabellini@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Live-Patching Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/live-patching/0 live-patching/git/0.git

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

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.live-patching


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