linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
Date: Mon, 13 Mar 2017 14:47:16 -0700	[thread overview]
Message-ID: <20170313214716.GY20572@atomide.com> (raw)
In-Reply-To: <c6d992fc-5468-8124-f1ee-558889147b72@ti.com>

* Andrew F. Davis <afd@ti.com> [170313 14:30]:
> On 03/13/2017 03:52 PM, Tony Lindgren wrote:
> > So let's fix the issue reported by Andrew by making CPU1 reset conditional
> > and only do it if CPU1 is configured to boot at an address that is within
> > the booting kernel. In these cases we know for sure that we just overwrote
> > the configured boot_secondary() during booting and that it's invalid.
> 
> One thing that worries me is the false negatives, what happens if the
> boot address register (WAKEUP_NS_PA_ADDR) is pointing outside the new
> kernel (which will most likely be the case as it probably will be
> pointing to the old kernel), but the core is not correctly parked (lets
> say it is in WFI idle loop in old kernel), we will fail to reboot it
> when in this case we should.

Yup this won't handle all the cases for sure. The case you're describing
always happens with u-boot, I think it parks CPU1 to the end of DDR.
And then we just blindly assume CPU1 is properly configured.

For kexec booting, the old parked address will be almost certainly
overlapping with the new kernel and the code gets overwritten. I don't
think the kexec case of WAKEUP_NS_PA_ADDR being outside the booted
kernel can happen without patched kernel. But it probably can with
crashkernel configurations, and properly parking the kernel is the
real fix there.

Or do you have some specific check in mind we could add?

> I think the best way to handle this is to set WAKEUP_NS_PA_ADDR to zero
> when we park it, for all other cases we can assume it is not parked. As
> long as we remember in the bootloader and kernel to zero that register
> when parking it, then we shouldn't have any false positives or any
> failures to reset when we should have.

Yeah makes sense. So for CPU1 hot-unplug, we also need to bring up
CPU1 from idle and park it to bootrom to also prevent CPU1 waking
from idle to a wrong address.

Regards,

Tony

  reply	other threads:[~2017-03-13 21:47 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-13 20:52 [PATCH] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot Tony Lindgren
2017-03-13 21:28 ` Andrew F. Davis
2017-03-13 21:47   ` Tony Lindgren [this message]
2017-03-14  7:30 ` Tero Kristo
2017-03-14 15:17   ` Tony Lindgren
2017-03-14 16:02     ` Andrew F. Davis
2017-03-14 16:41       ` Tony Lindgren
2017-03-14 17:57         ` Andrew F. Davis
2017-03-14 18:14           ` Tony Lindgren
2017-03-15 17:22             ` Tony Lindgren
2017-03-16 15:29               ` Tony Lindgren
2017-03-17  9:24                 ` Russell King - ARM Linux
2017-03-17 13:57                   ` Tony Lindgren
2017-03-17 16:25                     ` Andrew F. Davis
2017-03-22 17:57                       ` Tony Lindgren
  -- strict thread matches above, loose matches on Subject: below --
2017-02-13 21:50 [PATCH] ARM: omap2+: Revert omap-smp.c changes resetting cpu1 " Tony Lindgren
2017-02-14 19:36 ` Tony Lindgren
2017-02-15 18:39   ` Tony Lindgren
2017-02-15 19:12     ` Tony Lindgren
2017-02-15 22:13       ` Andrew F. Davis
2017-02-15 22:27         ` Tony Lindgren
2017-02-16 16:10           ` Tony Lindgren
2017-02-16 16:21             ` Tony Lindgren
2017-02-16 16:29             ` Andrew F. Davis
2017-02-16 16:54               ` Tony Lindgren
2017-02-16 19:07                 ` Tony Lindgren
2017-02-17 15:55                   ` Tony Lindgren
2017-02-17 20:27                     ` Andrew F. Davis
2017-02-17 21:09                       ` Tony Lindgren

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=20170313214716.GY20572@atomide.com \
    --to=tony@atomide.com \
    --cc=linux-arm-kernel@lists.infradead.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).