All of lore.kernel.org
 help / color / mirror / Atom feed
From: Geoff Levand <geoff@infradead.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <Catalin.Marinas@arm.com>,
	Deepak Saxena <dsaxena@linaro.org>,
	Will Deacon <Will.Deacon@arm.com>,
	"kexec@lists.infradead.org" <kexec@lists.infradead.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/8] arm64: Use cpu_ops for smp_stop
Date: Tue, 13 May 2014 15:27:10 -0700	[thread overview]
Message-ID: <1400020030.11178.71.camel@smoke> (raw)
In-Reply-To: <20140509084416.GD4757@e106331-lin.cambridge.arm.com>

Hi Mark,

On Fri, 2014-05-09 at 09:44 +0100, Mark Rutland wrote:
> On Fri, May 09, 2014 at 01:48:17AM +0100, Geoff Levand wrote:
> > +	/* If we have the cup_ops use them. */
> > +
> > +	if (cpu_ops[cpu]->cpu_disable && cpu_ops[cpu]->cpu_die
> > +		&& !cpu_ops[cpu]->cpu_disable(cpu))
> > +		cpu_ops[cpu]->cpu_die(cpu);
> 
> For PSCI 0.2 support, we're going to need a cpu_kill callback which we
> can't call from the dying CPU. Specifically, we'll need to poll
> CPU_AFFINITY_INFO to ensure that secondaries have _actually_ left the
> kernel and aren't going to be adversely affected by the kernel text
> getting clobbered.
> 
> As we're going to wire that up to the cpu hotplug infrastructure it
> would be nice to perform the hotplug for kexec by reusing the generic
> hotplug infrastructure rather than calling portions of the arm64
> implementation directly.

OK, is there somewhere I can see that new code, and when do you expect
it to be merged?

> > +
> > +	/* Spin here if the cup_ops fail. */
> > +
> >  	while (1)
> >  		cpu_relax();
> 
> This seems very dodgy to me. If a CPU doesn't actually die it's going to
> be spinning in some memory that we may later clobber. At that point the
> CPU will do arbitrarily bad things when it begins executing whatever its
> currently executing instructions (or vectors) were replaced by, and you
> will waste hours trying to figure out what went wrong (See 8121cf312a19
> "ARM: 7766/1: versatile: don't mark pen as __INIT" for a similar mess).
> 
> If we fail to hotplug a CPU we at minimum need some acknowledgement that
> we failed. I would rather we failed to kexec entirely in that case.

This loop is for the non-hotplug power-off shutdown.  This whole smp_stop
support needs to be reconsidered for a hotplug spin-table re-work.

-Geoff



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: geoff@infradead.org (Geoff Levand)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/8] arm64: Use cpu_ops for smp_stop
Date: Tue, 13 May 2014 15:27:10 -0700	[thread overview]
Message-ID: <1400020030.11178.71.camel@smoke> (raw)
In-Reply-To: <20140509084416.GD4757@e106331-lin.cambridge.arm.com>

Hi Mark,

On Fri, 2014-05-09 at 09:44 +0100, Mark Rutland wrote:
> On Fri, May 09, 2014 at 01:48:17AM +0100, Geoff Levand wrote:
> > +	/* If we have the cup_ops use them. */
> > +
> > +	if (cpu_ops[cpu]->cpu_disable && cpu_ops[cpu]->cpu_die
> > +		&& !cpu_ops[cpu]->cpu_disable(cpu))
> > +		cpu_ops[cpu]->cpu_die(cpu);
> 
> For PSCI 0.2 support, we're going to need a cpu_kill callback which we
> can't call from the dying CPU. Specifically, we'll need to poll
> CPU_AFFINITY_INFO to ensure that secondaries have _actually_ left the
> kernel and aren't going to be adversely affected by the kernel text
> getting clobbered.
> 
> As we're going to wire that up to the cpu hotplug infrastructure it
> would be nice to perform the hotplug for kexec by reusing the generic
> hotplug infrastructure rather than calling portions of the arm64
> implementation directly.

OK, is there somewhere I can see that new code, and when do you expect
it to be merged?

> > +
> > +	/* Spin here if the cup_ops fail. */
> > +
> >  	while (1)
> >  		cpu_relax();
> 
> This seems very dodgy to me. If a CPU doesn't actually die it's going to
> be spinning in some memory that we may later clobber. At that point the
> CPU will do arbitrarily bad things when it begins executing whatever its
> currently executing instructions (or vectors) were replaced by, and you
> will waste hours trying to figure out what went wrong (See 8121cf312a19
> "ARM: 7766/1: versatile: don't mark pen as __INIT" for a similar mess).
> 
> If we fail to hotplug a CPU we at minimum need some acknowledgement that
> we failed. I would rather we failed to kexec entirely in that case.

This loop is for the non-hotplug power-off shutdown.  This whole smp_stop
support needs to be reconsidered for a hotplug spin-table re-work.

-Geoff

  reply	other threads:[~2014-05-13 22:27 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-09  0:48 [PATCH 0/8] arm64 kexec kernel patches Geoff Levand
2014-05-09  0:48 ` Geoff Levand
2014-05-09  0:48 ` [PATCH 8/8] arm64: Enable kexec in defconfig Geoff Levand
2014-05-09  0:48   ` Geoff Levand
2014-05-09  0:48 ` [PATCH 7/8] arm64/kexec: Add core kexec support Geoff Levand
2014-05-09  0:48   ` Geoff Levand
2014-05-09 15:36   ` Mark Rutland
2014-05-09 15:36     ` Mark Rutland
2014-05-13 22:27     ` Geoff Levand
2014-05-13 22:27       ` Geoff Levand
2014-05-16 10:26       ` Mark Rutland
2014-05-16 10:26         ` Mark Rutland
2014-05-14 10:54   ` Catalin Marinas
2014-05-14 10:54     ` Catalin Marinas
2014-05-14 23:20     ` Geoff Levand
2014-05-14 23:20       ` Geoff Levand
2014-07-07  7:33   ` Dave Young
2014-07-07  7:33     ` Dave Young
2014-07-11  9:47     ` Dave Young
2014-07-11  9:47       ` Dave Young
2014-05-09  0:48 ` [PATCH 1/8] arm64: Use cpu_ops for smp_stop Geoff Levand
2014-05-09  0:48   ` Geoff Levand
2014-05-09  8:44   ` Mark Rutland
2014-05-09  8:44     ` Mark Rutland
2014-05-13 22:27     ` Geoff Levand [this message]
2014-05-13 22:27       ` Geoff Levand
2014-05-09  0:48 ` [PATCH 5/8] arm64: Split soft_restart into two stages Geoff Levand
2014-05-09  0:48   ` Geoff Levand
2014-05-09  0:48 ` [PATCH 2/8] arm64: Make cpu_read_ops generic Geoff Levand
2014-05-09  0:48   ` Geoff Levand
2014-05-09  0:48 ` [PATCH 6/8] arm64/kexec: kexec needs cpu_die Geoff Levand
2014-05-09  0:48   ` Geoff Levand
2014-05-09  8:24   ` Mark Rutland
2014-05-09  8:24     ` Mark Rutland
2014-05-13 22:27     ` Geoff Levand
2014-05-13 22:27       ` Geoff Levand
2014-05-09  0:48 ` [PATCH 3/8] arm64: Add spin-table cpu_die Geoff Levand
2014-05-09  0:48   ` Geoff Levand
2014-05-09  8:54   ` Mark Rutland
2014-05-09  8:54     ` Mark Rutland
2014-05-09  0:48 ` [PATCH 4/8] arm64: Add smp_spin_table_set_die Geoff Levand
2014-05-09  0:48   ` Geoff Levand
2014-05-09 16:22 ` [PATCH 0/8] arm64 kexec kernel patches Mark Rutland
2014-05-09 16:22   ` Mark Rutland
2014-05-13 22:26   ` Geoff Levand
2014-05-13 22:26     ` Geoff Levand

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=1400020030.11178.71.camel@smoke \
    --to=geoff@infradead.org \
    --cc=Catalin.Marinas@arm.com \
    --cc=Will.Deacon@arm.com \
    --cc=dsaxena@linaro.org \
    --cc=kexec@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.