linux-kernel - Re: [PATCH v2 2/3] ARM: at91: pm: add per soc validation of pm modes

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: <5c00c0f9-dc73-fb8d-aef8-88f4bc923295@microchip.com>
Date:   Tue, 4 Aug 2020 15:45:40 +0000
From:   <Claudiu.Beznea@...rochip.com>
To:     <alexandre.belloni@...tlin.com>
CC:     <Nicolas.Ferre@...rochip.com>, <Ludovic.Desroches@...rochip.com>,
        <wenyou.yang@...el.com>, <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/3] ARM: at91: pm: add per soc validation of pm modes



On 04.08.2020 18:08, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 04/08/2020 15:00:38+0000, Claudiu.Beznea@...rochip.com wrote:
>>
>>
>> On 04.08.2020 14:42, Alexandre Belloni wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hello,
>>>
>>> On 04/08/2020 14:07:37+0300, Claudiu Beznea wrote:
>>>>  void __init at91rm9200_pm_init(void)
>>>>  {
>>>> +     static const int modes[] __initconst = {
>>>
>>> You don't need that to be static as it is now local to the function.
>>>
>>>> +             AT91_PM_STANDBY, AT91_PM_ULP0
>>>> +     };
>>>> +
>>>>       if (!IS_ENABLED(CONFIG_SOC_AT91RM9200))
>>>>               return;
>>>>
>>>> +     at91_pm_modes_validate(modes, ARRAY_SIZE(modes));
>>>
>>> For rm9200 and at91sam9, I would not allow changing the pm_modes and
>>> simply enforce standby_mode = AT91_PM_STANDBY and suspend_mode =
>>> AT91_PM_ULP0.I don't think you have any user that ever changed that
>>> behaviour also that avoids increasing the boot time for those slow SoCs.
>>
>> OK, but bootargs is parsed at a moment when there is no information about
>> the machine that is running the code. And enforcing this in *_pm_init()
>> functions for rm9200 and at91sam9 may change suspend and standby mode that
>> user selected. If there is no user up to this moment there is still the
>> possibility of being one in the future.
>>
> 
> So let's prevent users from doing that. Unused arguments are silently
> ignored which is exactly what we want to do.

Can you share what are you thinking about? You want to not parse
atmel.pm_modes for this machines?

> You won't make me believe
> there is actually a use case for swapping the standby and suspend
> meanings.
What i want to say is this:
bootargs contains atmel.pm_modes=ulp0,standby

this leads to
standby_mode=ulp0
suspend_mode=standby

But you want in code to force
standby_mode=standby
suspend_mode=ulp0

The question is: is this what you are thinking this should be done?

> 
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>