linux-kernel - Re: [PATCH 1/3] cpuidle: coupled: disable interrupts after entering safe state

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]

Message-ID: <CAMbhsRSTv9ivMY1=-4YPxPV0tCTcJnHwWOEvr_F5exg9A2Z1Mw@mail.gmail.com>
Date:	Thu, 29 Aug 2013 00:12:17 -0700
From:	Colin Cross <ccross@...roid.com>
To:	Prashant Gaikwad <pgaikwad@...dia.com>
Cc:	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Neil Zhang <zhangwm@...vell.com>,
	Joseph Lo <josephl@...dia.com>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Daniel Lezcano <daniel.lezcano@...aro.org>
Subject: Re: [PATCH 1/3] cpuidle: coupled: disable interrupts after entering
 safe state

On Wed, Aug 28, 2013 at 11:50 PM, Prashant Gaikwad <pgaikwad@...dia.com> wrote:
> On Saturday 24 August 2013 01:15 AM, Colin Cross wrote:
>>
>> Calling cpuidle_enter_state is expected to return with interrupts
>> enabled, but interrupts must be disabled before starting the
>> ready loop synchronization stage.  Call local_irq_disable after
>> each call to cpuidle_enter_state for the safe state.
>>
>> CC: stable@...r.kernel.org
>> Signed-off-by: Colin Cross <ccross@...roid.com>
>> ---
>>   drivers/cpuidle/coupled.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
>> index 2a297f8..db92bcb 100644
>> --- a/drivers/cpuidle/coupled.c
>> +++ b/drivers/cpuidle/coupled.c
>> @@ -460,6 +460,7 @@ int cpuidle_enter_state_coupled(struct cpuidle_device
>> *dev,
>>                 }
>>                 entered_state = cpuidle_enter_state(dev, drv,
>>                         dev->safe_state_index);
>> +               local_irq_disable();
>
>
> Colin,
>
> There is still some window where irq remains enabled after exiting safe
> state. It may introduce some corner case.
> Instead of this can we pass a parameter to cpuidle_enter_state indicating
> that irq has to be enabled or not after exit from idle state, which would be
> false when entering safe state from coupled idle.

It's fine for irqs to be enabled when exiting the safe state, in fact
on further inspection this patch isn't strictly necessary at all - we
always enable interrupts inside cpuidle_coupled_clear_pokes soon after
cpuidle_enter_state returns, and then disable them again.  It's
probably better to disable interrupts right after cpuidle_enter_state,
it makes sure interrupts are consistently disabled when calling
cpuidle_coupled_set_waiting, cpuidle_coupled_cpus_waiting and
cpuidle_coupled_clear_pokes, although that doesn't matter in the
current implementation.

Rafael, feel free to drop the stable annotation from this patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/