Boot hang on Origen with (!SMP && CPU_IDLE)

Boot hang on Origen with (!SMP && CPU_IDLE)

[Tushar Behera]

Hi,

We are getting boot-time system hang on Exynos4210-based Origen board
if the kernel (right now testing v3.13-rc6) is built using
exynos_defconfig, disabling SMP support and enabling CPU_IDLE support.
The boot log can be found here[1].

Git bisect points to following commit.

commit 87107d89052bcec1fe91b309631de4ed294a5171
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Wed Jun 19 01:36:52 2013 +0900

    ARM: EXYNOS: Remove legacy L2X0 initialization

    Since Exynos is now supporting only DT-based boot, the old L2X0
    initialization code is not needed anymore, so exynos4_l2x0_cache_init()
    can be greatly simplified.

    Signed-off-by: Arnd Bergmann <arnd@arndb.de>
    Signed-off-by: Tomasz Figa <t.figa@samsung.com>
    Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
    Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>

Reverting the changes, the kernel boots up.

Any idea what else we might be missing?

[1] http://pastebin.com/0mP6ML4y
-- 
Tushar Behera

Re: Boot hang on Origen with (!SMP && CPU_IDLE)

[Arnd Bergmann]

On Friday 03 January 2014, Tushar Behera wrote:
> Hi,
> 
> We are getting boot-time system hang on Exynos4210-based Origen board
> if the kernel (right now testing v3.13-rc6) is built using
> exynos_defconfig, disabling SMP support and enabling CPU_IDLE support.
> The boot log can be found here[1].
> 
> Git bisect points to following commit.
> 
> commit 87107d89052bcec1fe91b309631de4ed294a5171
> Author: Arnd Bergmann <arnd@arndb.de>
> Date:   Wed Jun 19 01:36:52 2013 +0900
> 
>     ARM: EXYNOS: Remove legacy L2X0 initialization
> 
>     Since Exynos is now supporting only DT-based boot, the old L2X0
>     initialization code is not needed anymore, so exynos4_l2x0_cache_init()
>     can be greatly simplified.
> 
>     Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>     Signed-off-by: Tomasz Figa <t.figa@samsung.com>
>     Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>     Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> 
> Reverting the changes, the kernel boots up.
> 
> Any idea what else we might be missing?
> 
> [1] http://pastebin.com/0mP6ML4y

Hmm, the boot log contains no message about the l2 cache controller getting
initialized, which means that l2x0_of_init probably failed before calling
l2x0_init. It also seems that the dts files distributed with the kernel
are lacking nodes for the l2x0 device, which is indeed a perfectly good
explanation although it doesn't explain at all why it ever worked on
any system with my patch.

Can you check if there is a correct cache controller node in your device
tree, and whether it works when you add one? If so, we should probably
add a couple of stable backport patches to the dts files. It would also
be a good time to get rid of the L2_AUX_VAL and L2_AUX_MASK defines and
just read the respective settings from DT.

	Arnd

Re: Boot hang on Origen with (!SMP && CPU_IDLE)

[Tomasz Figa]

On Friday 03 of January 2014 14:37:10 Arnd Bergmann wrote:
> On Friday 03 January 2014, Tushar Behera wrote:
> > Hi,
> > 
> > We are getting boot-time system hang on Exynos4210-based Origen board
> > if the kernel (right now testing v3.13-rc6) is built using
> > exynos_defconfig, disabling SMP support and enabling CPU_IDLE support.
> > The boot log can be found here[1].
> > 
> > Git bisect points to following commit.
> > 
> > commit 87107d89052bcec1fe91b309631de4ed294a5171
> > Author: Arnd Bergmann <arnd@arndb.de>
> > Date:   Wed Jun 19 01:36:52 2013 +0900
> > 
> >     ARM: EXYNOS: Remove legacy L2X0 initialization
> > 
> >     Since Exynos is now supporting only DT-based boot, the old L2X0
> >     initialization code is not needed anymore, so exynos4_l2x0_cache_init()
> >     can be greatly simplified.
> > 
> >     Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >     Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> >     Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >     Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> > 
> > Reverting the changes, the kernel boots up.
> > 
> > Any idea what else we might be missing?
> > 
> > [1] http://pastebin.com/0mP6ML4y
> 
> Hmm, the boot log contains no message about the l2 cache controller getting
> initialized, which means that l2x0_of_init probably failed before calling
> l2x0_init. It also seems that the dts files distributed with the kernel
> are lacking nodes for the l2x0 device, which is indeed a perfectly good
> explanation although it doesn't explain at all why it ever worked on
> any system with my patch.
> 
> Can you check if there is a correct cache controller node in your device
> tree, and whether it works when you add one? If so, we should probably
> add a couple of stable backport patches to the dts files. It would also
> be a good time to get rid of the L2_AUX_VAL and L2_AUX_MASK defines and
> just read the respective settings from DT.

Tushar, do you maybe also have CONFIG_CACHE_L2X0 enabled?

Generally I can see two different issues here:

1) Broken handling of L2 cache in plat-samsung/sleep.S. It assumes that
whenever CONFIG_CACHE_L2X0 is enabled, L2 cache is enabled too.

This needs to go away. On normal resume from sleep, there is no need to
do anything with L2X0 at such early stage. This reinitialization can be
safely done later, by calling generic outer_resume().

The problem shows up when you look at AFTR and LPA idle modes. They keep
L2 cache contents, but L2X0 registers must be restored to let the cache
operate normally. This must happen early enough to keep cache data
consistent. Still, the code doing this must consider whether the cache
was enabled before entering AFTR/LPA and whether secure firmware is used
(see below), so this is a bit tricky.

2) There is no L2 cache controller node in Exynos4*.dtsi.

It should be added, but L2 cache can't be enabled on all boards yet,
since on boards where secure firmware is enabled, special configuration
involving SMC calls is required. Patches for this are queued on my work
queue, but it's quite tricky due to 1), which needs to consider whether
secure firmware is enabled or not.

Best regards,
Tomasz

Re: Boot hang on Origen with (!SMP && CPU_IDLE)

[Tushar Behera]

On 3 January 2014 19:28, Tomasz Figa <t.figa@samsung.com> wrote:
> On Friday 03 of January 2014 14:37:10 Arnd Bergmann wrote:
>> On Friday 03 January 2014, Tushar Behera wrote:

[ ... ]

>> Hmm, the boot log contains no message about the l2 cache controller getting
>> initialized, which means that l2x0_of_init probably failed before calling
>> l2x0_init. It also seems that the dts files distributed with the kernel
>> are lacking nodes for the l2x0 device, which is indeed a perfectly good
>> explanation although it doesn't explain at all why it ever worked on
>> any system with my patch.
>>
>> Can you check if there is a correct cache controller node in your device
>> tree, and whether it works when you add one? If so, we should probably
>> add a couple of stable backport patches to the dts files. It would also

The device tree node for l2x0 device was missing. After adding a node
as below I can start booting Origen board.

diff --git a/arch/arm/boot/dts/exynos4210-origen.dts
b/arch/arm/boot/dts/exynos4210-origen.dts
index 1a12fb2..675f323 100644
--- a/arch/arm/boot/dts/exynos4210-origen.dts
+++ b/arch/arm/boot/dts/exynos4210-origen.dts
@@ -32,6 +32,13 @@

+       l2-cache-controller@10502000 {
+               compatible = "arm,pl310-cache";
+               reg = <0x10502000 0x1000>;
+               cache-unified;
+               cache-level = <2>;
+       };
+

>> be a good time to get rid of the L2_AUX_VAL and L2_AUX_MASK defines and
>> just read the respective settings from DT.

Ok.

>
> Tushar, do you maybe also have CONFIG_CACHE_L2X0 enabled?
>

Yes, this config option is enabled. If this is disabled, I don't see
this problem.

> Generally I can see two different issues here:
>
> 1) Broken handling of L2 cache in plat-samsung/sleep.S. It assumes that
> whenever CONFIG_CACHE_L2X0 is enabled, L2 cache is enabled too.
>
> This needs to go away. On normal resume from sleep, there is no need to
> do anything with L2X0 at such early stage. This reinitialization can be
> safely done later, by calling generic outer_resume().
>
> The problem shows up when you look at AFTR and LPA idle modes. They keep
> L2 cache contents, but L2X0 registers must be restored to let the cache
> operate normally. This must happen early enough to keep cache data
> consistent. Still, the code doing this must consider whether the cache
> was enabled before entering AFTR/LPA and whether secure firmware is used
> (see below), so this is a bit tricky.
>
> 2) There is no L2 cache controller node in Exynos4*.dtsi.
>
> It should be added, but L2 cache can't be enabled on all boards yet,
> since on boards where secure firmware is enabled, special configuration
> involving SMC calls is required. Patches for this are queued on my work
> queue, but it's quite tricky due to 1), which needs to consider whether
> secure firmware is enabled or not.
>

In that case, would it be ok to add the node for Origen board only?

-- 
Tushar Behera

Re: Boot hang on Origen with (!SMP && CPU_IDLE)

[Arnd Bergmann]

On Monday 06 January 2014, Tushar Behera wrote:
> The device tree node for l2x0 device was missing. After adding a node
> as below I can start booting Origen board.
> 
> diff --git a/arch/arm/boot/dts/exynos4210-origen.dts
> b/arch/arm/boot/dts/exynos4210-origen.dts
> index 1a12fb2..675f323 100644
> --- a/arch/arm/boot/dts/exynos4210-origen.dts
> +++ b/arch/arm/boot/dts/exynos4210-origen.dts
> @@ -32,6 +32,13 @@
> 
> +       l2-cache-controller@10502000 {
> +               compatible = "arm,pl310-cache";
> +               reg = <0x10502000 0x1000>;
> +               cache-unified;
> +               cache-level = <2>;
> +       };
> +

Ok, very good!

> >> be a good time to get rid of the L2_AUX_VAL and L2_AUX_MASK defines and
> >> just read the respective settings from DT.
> 
> Ok.

Does the node you list above have the right settings for this?

> > 2) There is no L2 cache controller node in Exynos4*.dtsi.
> >
> > It should be added, but L2 cache can't be enabled on all boards yet,
> > since on boards where secure firmware is enabled, special configuration
> > involving SMC calls is required. Patches for this are queued on my work
> > queue, but it's quite tricky due to 1), which needs to consider whether
> > secure firmware is enabled or not.
> >
> 
> In that case, would it be ok to add the node for Origen board only?

Wouldn't that leave other systems still broken? I'm particularly worried
about what patch to backport to linux-stable. We should definitely add
the node for Origen, but we may also have to revert my broken patch
in all affected kernel versions.

Are there any systems that may or may not have secure firmware enabled
depending on the boot loader, or do we always know whether secure firmware
is there or not?

	Arnd

Re: Boot hang on Origen with (!SMP && CPU_IDLE)

[Tomasz Figa]

On Monday 06 of January 2014 16:30:56 Arnd Bergmann wrote:
> On Monday 06 January 2014, Tushar Behera wrote:
> > The device tree node for l2x0 device was missing. After adding a node
> > as below I can start booting Origen board.
> > 
> > diff --git a/arch/arm/boot/dts/exynos4210-origen.dts
> > b/arch/arm/boot/dts/exynos4210-origen.dts
> > index 1a12fb2..675f323 100644
> > --- a/arch/arm/boot/dts/exynos4210-origen.dts
> > +++ b/arch/arm/boot/dts/exynos4210-origen.dts
> > @@ -32,6 +32,13 @@
> > 
> > +       l2-cache-controller@10502000 {
> > +               compatible = "arm,pl310-cache";
> > +               reg = <0x10502000 0x1000>;
> > +               cache-unified;
> > +               cache-level = <2>;
> > +       };
> > +
> 
> Ok, very good!
> 
> > >> be a good time to get rid of the L2_AUX_VAL and L2_AUX_MASK defines and
> > >> just read the respective settings from DT.
> > 
> > Ok.
> 
> Does the node you list above have the right settings for this?
> 
> > > 2) There is no L2 cache controller node in Exynos4*.dtsi.
> > >
> > > It should be added, but L2 cache can't be enabled on all boards yet,
> > > since on boards where secure firmware is enabled, special configuration
> > > involving SMC calls is required. Patches for this are queued on my work
> > > queue, but it's quite tricky due to 1), which needs to consider whether
> > > secure firmware is enabled or not.
> > >
> > 
> > In that case, would it be ok to add the node for Origen board only?

Better solution would be to add the node in SoC-level dtsi, keep it
disabled and override the status to okay at board level. Still...

> 
> Wouldn't that leave other systems still broken? I'm particularly worried
> about what patch to backport to linux-stable. We should definitely add
> the node for Origen, but we may also have to revert my broken patch
> in all affected kernel versions.

...Origen is not the only board that is affected by this, most likely any
Exynos4-based board not running under secure firmware is.

> 
> Are there any systems that may or may not have secure firmware enabled
> depending on the boot loader, or do we always know whether secure firmware
> is there or not?

It always depends on the boot loader, but fortunately it's unlikely to
happen that one board will have both secure-enabled and normal bootloaders
available and in use, at least for Exynos4-based boards, so it's quite
safe to assume presence of secure firmware on per board basis and so you
have the secure firmware device tree node only in dts files of boards
that are known to use secure firmware.

Aynway, from what I can see, support for the only two Exynos4 boards using
for secure firmware was added in 3.12. This means that we can revert the
offending patch for 3.11, but for 3.12 and newer we can't, because this
will break such boards with CONFIG_L2X0 enabled.

Instead, wouldn't it be better to fix the issue at its cause? This means
s5p-sleep.S re-enabling L2X0 only if it was really enabled before entering
low power mode. This could be achieved by checking if l2x0_regs_phys isn't
0 for example (which isn't a valid physical RAM address on Exynos).

What do you think?

Best regards,
Tomasz

Re: Boot hang on Origen with (!SMP && CPU_IDLE)

[Tushar Behera]

On 6 January 2014 22:11, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On Monday 06 of January 2014 16:30:56 Arnd Bergmann wrote:
>> On Monday 06 January 2014, Tushar Behera wrote:
>> > The device tree node for l2x0 device was missing. After adding a node
>> > as below I can start booting Origen board.
>> >
>> > diff --git a/arch/arm/boot/dts/exynos4210-origen.dts
>> > b/arch/arm/boot/dts/exynos4210-origen.dts
>> > index 1a12fb2..675f323 100644
>> > --- a/arch/arm/boot/dts/exynos4210-origen.dts
>> > +++ b/arch/arm/boot/dts/exynos4210-origen.dts
>> > @@ -32,6 +32,13 @@
>> >
>> > +       l2-cache-controller@10502000 {
>> > +               compatible = "arm,pl310-cache";
>> > +               reg = <0x10502000 0x1000>;
>> > +               cache-unified;
>> > +               cache-level = <2>;
>> > +       };
>> > +
>>
>> Ok, very good!
>>
>> > >> be a good time to get rid of the L2_AUX_VAL and L2_AUX_MASK defines and
>> > >> just read the respective settings from DT.
>> >
>> > Ok.
>>
>> Does the node you list above have the right settings for this?
>>

I will add another patch to remove the usage of L2_AUX_VAL and
L2_AUX_MASK defines for regular mode, but we might still be needing
them for secure firmware booting,

>> > > 2) There is no L2 cache controller node in Exynos4*.dtsi.
>> > >
>> > > It should be added, but L2 cache can't be enabled on all boards yet,
>> > > since on boards where secure firmware is enabled, special configuration
>> > > involving SMC calls is required. Patches for this are queued on my work
>> > > queue, but it's quite tricky due to 1), which needs to consider whether
>> > > secure firmware is enabled or not.
>> > >
>> >
>> > In that case, would it be ok to add the node for Origen board only?
>
> Better solution would be to add the node in SoC-level dtsi, keep it
> disabled and override the status to okay at board level. Still...
>
>>
>> Wouldn't that leave other systems still broken? I'm particularly worried
>> about what patch to backport to linux-stable. We should definitely add
>> the node for Origen, but we may also have to revert my broken patch
>> in all affected kernel versions.
>
> ...Origen is not the only board that is affected by this, most likely any
> Exynos4-based board not running under secure firmware is.
>
>>
>> Are there any systems that may or may not have secure firmware enabled
>> depending on the boot loader, or do we always know whether secure firmware
>> is there or not?
>
> It always depends on the boot loader, but fortunately it's unlikely to
> happen that one board will have both secure-enabled and normal bootloaders
> available and in use, at least for Exynos4-based boards, so it's quite
> safe to assume presence of secure firmware on per board basis and so you
> have the secure firmware device tree node only in dts files of boards
> that are known to use secure firmware.
>

Ok. I will add the node in Exynos4.dtsi, keep it disabled and enable
on all Exynos4-based boards that are not using secure-firmware node.

> Aynway, from what I can see, support for the only two Exynos4 boards using
> for secure firmware was added in 3.12. This means that we can revert the
> offending patch for 3.11, but for 3.12 and newer we can't, because this
> will break such boards with CONFIG_L2X0 enabled.
>
> Instead, wouldn't it be better to fix the issue at its cause? This means
> s5p-sleep.S re-enabling L2X0 only if it was really enabled before entering
> low power mode. This could be achieved by checking if l2x0_regs_phys isn't
> 0 for example (which isn't a valid physical RAM address on Exynos).
>
> What do you think?
>
> Best regards,
> Tomasz
>

Tested successfully on Origen board with following code snippet.

diff --git a/arch/arm/plat-samsung/s5p-sleep.S
b/arch/arm/plat-samsung/s5p-sleep.S
index a030e73..d1753c5 100644
--- a/arch/arm/plat-samsung/s5p-sleep.S
+++ b/arch/arm/plat-samsung/s5p-sleep.S
@@ -62,6 +62,8 @@ ENTRY(s3c_cpu_resume)
        bne     resume_l2on
        adr     r0, l2x0_regs_phys
        ldr     r0, [r0]
+       cmp     r0, #0
+       beq     resume_l2on
        ldr     r1, [r0, #L2X0_R_PHY_BASE]
        ldr     r2, [r1, #L2X0_CTRL]
        tst     r2, #0x1

-- 
Tushar Behera

Re: Boot hang on Origen with (!SMP && CPU_IDLE)

[Arnd Bergmann]

On Tuesday 07 January 2014, Tushar Behera wrote:
> >> > >> be a good time to get rid of the L2_AUX_VAL and L2_AUX_MASK defines and
> >> > >> just read the respective settings from DT.
> >> >
> >> > Ok.
> >>
> >> Does the node you list above have the right settings for this?
> >>
> 
> I will add another patch to remove the usage of L2_AUX_VAL and
> L2_AUX_MASK defines for regular mode, but we might still be needing
> them for secure firmware booting,

Well, the point was that all the settings from these arguments are
supposed to come from DT properties these days. If you need to set
or clear a bit that doesn't have a property yet, I'd prefer if you
extend the binding and the pl310_of_setup() implementation.

	Arnd