linux-kernel - Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 27 Apr 2016 00:34:07 -0700
From:	Stefan Agner <stefan@...er.ch>
To:	Dong Aisheng <dongas86@...il.com>
Cc:	Shawn Guo <shawnguo@...nel.org>,
	Lucas Stach <l.stach@...gutronix.de>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...eaurora.org>,
	linux-kernel@...r.kernel.org, mingo@...hat.com,
	kernel@...gutronix.de, tglx@...utronix.de,
	linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

On 2016-04-26 19:57, Dong Aisheng wrote:
> On Wed, Apr 27, 2016 at 9:58 AM, Shawn Guo <shawnguo@...nel.org> wrote:
>> On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote:
>>> Shawn,
>>> What's your suggestion?
>>
>> I think this needs more discussion, and I just dropped Stefan's patch
>> from my tree.
>>
>> We need to firstly understand why this is happening.  The .prepare hook
>> is defined to be non-atomic context, and so that we call sleep function
>> in there.  We did everything right.  Why are we getting the warning?  If
>> I'm correct, this warning only happens on i.MX7D.  Why is that?
>>
> 
> Why Stefan's patch works (checking irqs_disabled()) is because during kernel
> time init, the irq is still not enabled. It fixes the issue indirectly.
> See:
> asmlinkage __visible void __init start_kernel(void)
> {
>         /*
>          * Set up the scheduler prior starting any interrupts (such as the
>          * timer interrupt). Full topology setup happens at smp_init()
>          * time - but meanwhile we still have a functioning scheduler.
>          */
>         sched_init();
>         .............
>         time_init();
>         ..............
>         WARN(!irqs_disabled(), "Interrupts were enabled early\n");
>         early_boot_irqs_disabled = false;
>         local_irq_enable();
> }
> 
> The issue can only happen when PLL enable causes a schedule during
> imx_clock_init().
> Not all PLL has this issue.
> The issue happens on MX7D pll_audio_main_clk/pll_video_main_clk
> which requires more delay time and cause usleep.
> Because clk framework does not support MX7D clock types (operation requires
> parents on), we simply enable all clocks in imx7d_clocks_init().
> 
> If apply my this patch series:
> https://lkml.org/lkml/2016/4/20/199
> The issue can also be gone.

Oh ok, it does make sense to enable as few clocks as possible.

However, even if we do not enable lots of clocks at that time, and this
helps to avoid the problem for now, it could still be that
someone/something requests a clock early during boot, leading to a PLL
enable... Shouldn't we make sure that those base clocks can be enabled
even during early boot..?

--
Stefan