All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Osipenko <digetx@gmail.com>
To: Peter Geis <pgwipeout@gmail.com>
Cc: "Russell King" <linux@armlinux.org.uk>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Jonathan Hunter" <jonathanh@nvidia.com>,
	linux-tegra@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	"Michał Mirosław" <mirq-linux@rere.qmqm.pl>
Subject: Re: [PATCH v2 0/5] Initial support of Trusted Foundations on Tegra30
Date: Wed, 04 Jul 2018 14:25:11 +0300	[thread overview]
Message-ID: <1635370.HnqPZmo3Ue@dimapc> (raw)
In-Reply-To: <5c47ef70-b90e-6e81-39dd-fbcfad2b93e5@gmail.com>

On Monday, 2 July 2018 21:53:08 MSK Peter Geis wrote:
> On 07/02/2018 10:48 AM, Dmitry Osipenko wrote:
> > On Friday, 29 June 2018 22:37:02 MSK Peter Geis wrote:
> >> Good Afternoon,
> >> 
> >> I have tested these patches on the Ouya T3 device.
> >> They work great to enable the L2 cache controller, however they do not
> >> respect explicitly disabling the L2 cache controller via the kernel
> >> config nor device tree.
> >> 
> >> With CONFIG_CACHE_L2X0 disabled, but CONFIG_TRUSTED_FOUNDATIONS enabled,
> >> the L2 cache controller is silently enabled and allows all four cores to
> >> boot.
> > 
> > I don't see how cache could be enabled with CONFIG_CACHE_L2X0 disabled,
> > there is no code to do that. Could you elaborate please?
> > 
> > Secondary cores do not depend on the cache state, disabled cache shouldn't
> > prevent them to boot.
> 
> On the untouched mainline kernel running on a Trusted Foundations T3
> device, I observed the following indications:
> With the L2 cache controller enabled, all four processor cores were
> enabled, but it would immediately panic for writing to a secure register
> from insecure mode.
> With the L2 cache controller disabled, only the boot core is detected,
> but it successfully boots.
> This is the issue that I inquired originally to you about.
> 
> With your patch running on the same device, the following is observed:
> With the L2 controller enabled, all four cores are active, and the cache
> controller appears to function.
> With the L2 controller disabled, but trusted foundations enabled, the L2
> controller enabled kernel message is missing, however all four cores
> still enable.
> 

This is the correct behaviour.

> After looking through the code a little more deeply, I see you modified
> the reset handler for handling offline cores.
> I am wondering if you fixed an additional issue inadvertently.
> 

CPU will fail to boot if it tries to apply erratas via accessing the secure 
registers. I've changed the reset handler to skip erratas on T20/30 if trusted 
foundations present, see "Don't apply CPU erratas in insecure mode" patch. 
This is an intentional change.

> The reason I discovered this is I am working with kexec as a bootloader.
> On a device without trusted foundations, kexec works without issue.
> On a device with trusted foundations and your patch, I found that
> disabling the l2 cache controller and trusted foundations allow kexec to
> occur.
> I haven't tried an either/or scenario, though now you have me thinking I
> should.
> 

Secondary cores are dying in the reset handler in a case of disabled L2 + 
disabled TF. Looks like there is something wrong in regards to stopping 
secondary CPU cores / flushing CPU caches, the suspend-resume isn't working 
right now because of it and likely that kexec is suffering from the same 
issue.

> >> One must also disable CONFIG_TRUSTED_FOUNDATIONS to stop the L2 cache
> >> controller from spinning up.
> >> 
> >> Tested-by: Peter Geis <pgwipeout@gmail.com>

Thanks for testing!

WARNING: multiple messages have this Message-ID (diff)
From: digetx@gmail.com (Dmitry Osipenko)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 0/5] Initial support of Trusted Foundations on Tegra30
Date: Wed, 04 Jul 2018 14:25:11 +0300	[thread overview]
Message-ID: <1635370.HnqPZmo3Ue@dimapc> (raw)
In-Reply-To: <5c47ef70-b90e-6e81-39dd-fbcfad2b93e5@gmail.com>

On Monday, 2 July 2018 21:53:08 MSK Peter Geis wrote:
> On 07/02/2018 10:48 AM, Dmitry Osipenko wrote:
> > On Friday, 29 June 2018 22:37:02 MSK Peter Geis wrote:
> >> Good Afternoon,
> >> 
> >> I have tested these patches on the Ouya T3 device.
> >> They work great to enable the L2 cache controller, however they do not
> >> respect explicitly disabling the L2 cache controller via the kernel
> >> config nor device tree.
> >> 
> >> With CONFIG_CACHE_L2X0 disabled, but CONFIG_TRUSTED_FOUNDATIONS enabled,
> >> the L2 cache controller is silently enabled and allows all four cores to
> >> boot.
> > 
> > I don't see how cache could be enabled with CONFIG_CACHE_L2X0 disabled,
> > there is no code to do that. Could you elaborate please?
> > 
> > Secondary cores do not depend on the cache state, disabled cache shouldn't
> > prevent them to boot.
> 
> On the untouched mainline kernel running on a Trusted Foundations T3
> device, I observed the following indications:
> With the L2 cache controller enabled, all four processor cores were
> enabled, but it would immediately panic for writing to a secure register
> from insecure mode.
> With the L2 cache controller disabled, only the boot core is detected,
> but it successfully boots.
> This is the issue that I inquired originally to you about.
> 
> With your patch running on the same device, the following is observed:
> With the L2 controller enabled, all four cores are active, and the cache
> controller appears to function.
> With the L2 controller disabled, but trusted foundations enabled, the L2
> controller enabled kernel message is missing, however all four cores
> still enable.
> 

This is the correct behaviour.

> After looking through the code a little more deeply, I see you modified
> the reset handler for handling offline cores.
> I am wondering if you fixed an additional issue inadvertently.
> 

CPU will fail to boot if it tries to apply erratas via accessing the secure 
registers. I've changed the reset handler to skip erratas on T20/30 if trusted 
foundations present, see "Don't apply CPU erratas in insecure mode" patch. 
This is an intentional change.

> The reason I discovered this is I am working with kexec as a bootloader.
> On a device without trusted foundations, kexec works without issue.
> On a device with trusted foundations and your patch, I found that
> disabling the l2 cache controller and trusted foundations allow kexec to
> occur.
> I haven't tried an either/or scenario, though now you have me thinking I
> should.
> 

Secondary cores are dying in the reset handler in a case of disabled L2 + 
disabled TF. Looks like there is something wrong in regards to stopping 
secondary CPU cores / flushing CPU caches, the suspend-resume isn't working 
right now because of it and likely that kexec is suffering from the same 
issue.

> >> One must also disable CONFIG_TRUSTED_FOUNDATIONS to stop the L2 cache
> >> controller from spinning up.
> >> 
> >> Tested-by: Peter Geis <pgwipeout@gmail.com>

Thanks for testing!

  reply	other threads:[~2018-07-04 11:25 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-19 11:00 [PATCH v2 0/5] Initial support of Trusted Foundations on Tegra30 Dmitry Osipenko
2018-06-19 11:00 ` Dmitry Osipenko
2018-06-19 11:00 ` [PATCH v2 1/5] ARM: trusted_foundations: Implement L2 cache initialization callback Dmitry Osipenko
2018-06-19 11:00   ` Dmitry Osipenko
2018-06-19 11:00   ` Dmitry Osipenko
2018-08-14 21:19   ` Dmitry Osipenko
2018-08-14 21:19     ` Dmitry Osipenko
2018-06-19 11:00 ` [PATCH v2 2/5] ARM: trusted_foundations: Provide information about whether firmware is registered Dmitry Osipenko
2018-06-19 11:00   ` Dmitry Osipenko
2018-06-19 11:00 ` [PATCH v2 3/5] ARM: tegra: Setup L2 cache using Trusted Foundations firmware Dmitry Osipenko
2018-06-19 11:00   ` Dmitry Osipenko
2018-06-19 11:00 ` [PATCH v2 4/5] ARM: tegra: Don't apply CPU erratas in insecure mode Dmitry Osipenko
2018-06-19 11:00   ` Dmitry Osipenko
2018-06-19 11:00 ` [PATCH v2 5/5] ARM: tegra: Always boot CPU in ARM-mode Dmitry Osipenko
2018-06-19 11:00   ` Dmitry Osipenko
2018-06-29 19:37 ` [PATCH v2 0/5] Initial support of Trusted Foundations on Tegra30 Peter Geis
2018-06-29 19:37   ` Peter Geis
2018-07-02 14:48   ` Dmitry Osipenko
2018-07-02 14:48     ` Dmitry Osipenko
2018-07-02 18:53     ` Peter Geis
2018-07-02 18:53       ` Peter Geis
2018-07-04 11:25       ` Dmitry Osipenko [this message]
2018-07-04 11:25         ` Dmitry Osipenko
2018-07-06 12:01       ` Dmitry Osipenko
2018-07-06 12:01         ` Dmitry Osipenko

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=1635370.HnqPZmo3Ue@dimapc \
    --to=digetx@gmail.com \
    --cc=jonathanh@nvidia.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mirq-linux@rere.qmqm.pl \
    --cc=pgwipeout@gmail.com \
    --cc=thierry.reding@gmail.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.