All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman@linaro.org>
To: Alexandre Courbot <gnurou@gmail.com>
Cc: Olof Johansson <olof@lixom.net>,
	Alexandre Courbot <acourbot@nvidia.com>,
	Russell King <linux@arm.linux.org.uk>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Tomasz Figa <t.figa@samsung.com>,
	Dave Martin <Dave.Martin@arm.com>, Arnd Bergmann <arnd@arndb.de>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v7 1/5] ARM: add basic support for Trusted Foundations
Date: Thu, 10 Oct 2013 14:14:41 -0700	[thread overview]
Message-ID: <87y560q14u.fsf@linaro.org> (raw)
In-Reply-To: <CAAVeFuLvHdUuDurqA_=tvxJonNP+RekotM_MuysQrJD0PUgcSQ@mail.gmail.com> (Alexandre Courbot's message of "Thu, 10 Oct 2013 13:58:24 -0700")

Alexandre Courbot <gnurou@gmail.com> writes:

> On Wed, Oct 9, 2013 at 4:47 PM, Olof Johansson <olof@lixom.net> wrote:
>> On Fri, Oct 4, 2013 at 9:37 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:

[...]

>>> +static inline void register_trusted_foundations(
>>> +                                  struct trusted_foundations_platform_data *pd)
>>> +{
>>> +       /* If we try to register TF, this means the system needs it to continue.
>>> +        * Its absence if thus a fatal error. */
>>> +       panic("No support for Trusted Foundations, stopping...\n");
>>> +}
>>> +
>>> +static inline void of_register_trusted_foundations(void)
>>> +{
>>> +       /* If we find the target should enable TF but does not support it,
>>> +        * fail as the system won't be able to do much anyway */
>>> +       if (of_find_compatible_node(NULL, NULL, "tl,trusted-foundations"))
>>> +               register_trusted_foundations(NULL);
>>> +}
>>> +#endif /* CONFIG_TRUSTED_FOUNDATIONS */
>>
>> The latter function can just be shortcut unto a local panic too.
>>
>> But do we really need to panic here? It's a PITA to debug since it'll
>> be a very early panic. I'd rather have the system log errors in dmesg
>> but continue booting with one cpu.
>
> Actually the first version of this series continued on one CPU if TF
> support was missing (as secondary CPUs would simply fail to boot), but
> IIRC Stephen advocated for the current behavior (panic) instead. I
> agree such an early panic is hard to debug, especially considering
> that most devices using TF (Tegra retail devices) do not even feature
> a serial port.
>
> On the other hand, if we continue booting in these conditions the
> system freezes as soon as cpuidle kicks in, so we won't be going very
> far anyway. However the console is up when it happens.

cpu_idle_poll_ctrl(true) should solve that problem.

Kevin

> Maybe if there is a way to disable the kernel features TF is supposed
> to manage (SMP & cpuidle at the moment) when registration fails, the
> system could continue more gracefully (even if on one leg) and warn
> the user about this condition. Stephen, can I have your thoughts on
> this?

WARNING: multiple messages have this Message-ID (diff)
From: khilman@linaro.org (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v7 1/5] ARM: add basic support for Trusted Foundations
Date: Thu, 10 Oct 2013 14:14:41 -0700	[thread overview]
Message-ID: <87y560q14u.fsf@linaro.org> (raw)
In-Reply-To: <CAAVeFuLvHdUuDurqA_=tvxJonNP+RekotM_MuysQrJD0PUgcSQ@mail.gmail.com> (Alexandre Courbot's message of "Thu, 10 Oct 2013 13:58:24 -0700")

Alexandre Courbot <gnurou@gmail.com> writes:

> On Wed, Oct 9, 2013 at 4:47 PM, Olof Johansson <olof@lixom.net> wrote:
>> On Fri, Oct 4, 2013 at 9:37 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:

[...]

>>> +static inline void register_trusted_foundations(
>>> +                                  struct trusted_foundations_platform_data *pd)
>>> +{
>>> +       /* If we try to register TF, this means the system needs it to continue.
>>> +        * Its absence if thus a fatal error. */
>>> +       panic("No support for Trusted Foundations, stopping...\n");
>>> +}
>>> +
>>> +static inline void of_register_trusted_foundations(void)
>>> +{
>>> +       /* If we find the target should enable TF but does not support it,
>>> +        * fail as the system won't be able to do much anyway */
>>> +       if (of_find_compatible_node(NULL, NULL, "tl,trusted-foundations"))
>>> +               register_trusted_foundations(NULL);
>>> +}
>>> +#endif /* CONFIG_TRUSTED_FOUNDATIONS */
>>
>> The latter function can just be shortcut unto a local panic too.
>>
>> But do we really need to panic here? It's a PITA to debug since it'll
>> be a very early panic. I'd rather have the system log errors in dmesg
>> but continue booting with one cpu.
>
> Actually the first version of this series continued on one CPU if TF
> support was missing (as secondary CPUs would simply fail to boot), but
> IIRC Stephen advocated for the current behavior (panic) instead. I
> agree such an early panic is hard to debug, especially considering
> that most devices using TF (Tegra retail devices) do not even feature
> a serial port.
>
> On the other hand, if we continue booting in these conditions the
> system freezes as soon as cpuidle kicks in, so we won't be going very
> far anyway. However the console is up when it happens.

cpu_idle_poll_ctrl(true) should solve that problem.

Kevin

> Maybe if there is a way to disable the kernel features TF is supposed
> to manage (SMP & cpuidle at the moment) when registration fails, the
> system could continue more gracefully (even if on one leg) and warn
> the user about this condition. Stephen, can I have your thoughts on
> this?

WARNING: multiple messages have this Message-ID (diff)
From: Kevin Hilman <khilman@linaro.org>
To: Alexandre Courbot <gnurou@gmail.com>
Cc: Olof Johansson <olof@lixom.net>,
	Alexandre Courbot <acourbot@nvidia.com>,
	Russell King <linux@arm.linux.org.uk>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Tomasz Figa <t.figa@samsung.com>,
	Dave Martin <Dave.Martin@arm.com>, Arnd Bergmann <arnd@arndb.de>,
	"devicetree\@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-tegra\@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	"linux-arm-kernel\@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v7 1/5] ARM: add basic support for Trusted Foundations
Date: Thu, 10 Oct 2013 14:14:41 -0700	[thread overview]
Message-ID: <87y560q14u.fsf@linaro.org> (raw)
In-Reply-To: <CAAVeFuLvHdUuDurqA_=tvxJonNP+RekotM_MuysQrJD0PUgcSQ@mail.gmail.com> (Alexandre Courbot's message of "Thu, 10 Oct 2013 13:58:24 -0700")

Alexandre Courbot <gnurou@gmail.com> writes:

> On Wed, Oct 9, 2013 at 4:47 PM, Olof Johansson <olof@lixom.net> wrote:
>> On Fri, Oct 4, 2013 at 9:37 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:

[...]

>>> +static inline void register_trusted_foundations(
>>> +                                  struct trusted_foundations_platform_data *pd)
>>> +{
>>> +       /* If we try to register TF, this means the system needs it to continue.
>>> +        * Its absence if thus a fatal error. */
>>> +       panic("No support for Trusted Foundations, stopping...\n");
>>> +}
>>> +
>>> +static inline void of_register_trusted_foundations(void)
>>> +{
>>> +       /* If we find the target should enable TF but does not support it,
>>> +        * fail as the system won't be able to do much anyway */
>>> +       if (of_find_compatible_node(NULL, NULL, "tl,trusted-foundations"))
>>> +               register_trusted_foundations(NULL);
>>> +}
>>> +#endif /* CONFIG_TRUSTED_FOUNDATIONS */
>>
>> The latter function can just be shortcut unto a local panic too.
>>
>> But do we really need to panic here? It's a PITA to debug since it'll
>> be a very early panic. I'd rather have the system log errors in dmesg
>> but continue booting with one cpu.
>
> Actually the first version of this series continued on one CPU if TF
> support was missing (as secondary CPUs would simply fail to boot), but
> IIRC Stephen advocated for the current behavior (panic) instead. I
> agree such an early panic is hard to debug, especially considering
> that most devices using TF (Tegra retail devices) do not even feature
> a serial port.
>
> On the other hand, if we continue booting in these conditions the
> system freezes as soon as cpuidle kicks in, so we won't be going very
> far anyway. However the console is up when it happens.

cpu_idle_poll_ctrl(true) should solve that problem.

Kevin

> Maybe if there is a way to disable the kernel features TF is supposed
> to manage (SMP & cpuidle at the moment) when registration fails, the
> system could continue more gracefully (even if on one leg) and warn
> the user about this condition. Stephen, can I have your thoughts on
> this?

  reply	other threads:[~2013-10-10 21:14 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-04 16:37 [PATCH v7 0/5] ARM: support for Trusted Foundations secure monito Alexandre Courbot
2013-10-04 16:37 ` Alexandre Courbot
2013-10-04 16:37 ` Alexandre Courbot
2013-10-04 16:37 ` [PATCH v7 1/5] ARM: add basic support for Trusted Foundations Alexandre Courbot
2013-10-04 16:37   ` Alexandre Courbot
2013-10-04 16:37   ` Alexandre Courbot
2013-10-08  8:17   ` Michal Simek
2013-10-08  8:17     ` Michal Simek
     [not found]     ` <5253BF8E.4050802-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
2013-10-09 23:45       ` Olof Johansson
2013-10-09 23:45         ` Olof Johansson
2013-10-09 23:45         ` Olof Johansson
     [not found]         ` <CAOesGMjtHBft71nJUEc7K+2UA_LiWBOVWpbYvJDOnF=NLepLLw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-10-10  5:18           ` Michal Simek
2013-10-10  5:18             ` Michal Simek
2013-10-10  5:18             ` Michal Simek
     [not found]   ` <1380904635-18113-2-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-10-07 18:16     ` Kevin Hilman
2013-10-07 18:16       ` Kevin Hilman
2013-10-07 18:16       ` Kevin Hilman
2013-10-07 19:28     ` Stephen Warren
2013-10-07 19:28       ` Stephen Warren
2013-10-07 19:28       ` Stephen Warren
2013-10-09 23:47     ` Olof Johansson
2013-10-09 23:47       ` Olof Johansson
2013-10-09 23:47       ` Olof Johansson
     [not found]       ` <CAOesGMj74Pui1V5O7zxEipM5Hg6nboVTUCFuMY0c0aUh=wuu3Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-10-10 20:58         ` Alexandre Courbot
2013-10-10 20:58           ` Alexandre Courbot
2013-10-10 20:58           ` Alexandre Courbot
2013-10-10 21:14           ` Kevin Hilman [this message]
2013-10-10 21:14             ` Kevin Hilman
2013-10-10 21:14             ` Kevin Hilman
     [not found]           ` <CAAVeFuLvHdUuDurqA_=tvxJonNP+RekotM_MuysQrJD0PUgcSQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-10-10 22:22             ` Stephen Warren
2013-10-10 22:22               ` Stephen Warren
2013-10-10 22:22               ` Stephen Warren
     [not found]               ` <5257288E.6000704-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-10-10 22:25                 ` Olof Johansson
2013-10-10 22:25                   ` Olof Johansson
2013-10-10 22:25                   ` Olof Johansson
2013-10-10 22:28                 ` Alexandre Courbot
2013-10-10 22:28                   ` Alexandre Courbot
2013-10-10 22:28                   ` Alexandre Courbot
2013-10-04 16:37 ` [PATCH v7 2/5] ARM: tegra: add " Alexandre Courbot
2013-10-04 16:37   ` Alexandre Courbot
2013-10-04 16:37   ` Alexandre Courbot
     [not found] ` <1380904635-18113-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-10-04 16:37   ` [PATCH v7 3/5] ARM: tegra: split setting of CPU reset handler Alexandre Courbot
2013-10-04 16:37     ` Alexandre Courbot
2013-10-04 16:37     ` Alexandre Courbot
2013-10-06 18:11   ` [PATCH v7 0/5] ARM: support for Trusted Foundations secure monito Alex Courbot
2013-10-06 18:11     ` Alex Courbot
2013-10-06 18:11     ` Alex Courbot
2013-10-04 16:37 ` [PATCH v7 4/5] ARM: tegra: set CPU reset handler with firmware op Alexandre Courbot
2013-10-04 16:37   ` Alexandre Courbot
2013-10-04 16:37   ` Alexandre Courbot
2013-10-04 16:37 ` [PATCH v7 5/5] ARM: tegra: support Trusted Foundations by default Alexandre Courbot
2013-10-04 16:37   ` Alexandre Courbot
2013-10-04 16:37   ` Alexandre Courbot

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=87y560q14u.fsf@linaro.org \
    --to=khilman@linaro.org \
    --cc=Dave.Martin@arm.com \
    --cc=acourbot@nvidia.com \
    --cc=arnd@arndb.de \
    --cc=devicetree@vger.kernel.org \
    --cc=gnurou@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=olof@lixom.net \
    --cc=swarren@wwwdotorg.org \
    --cc=t.figa@samsung.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.