linux-kernel - Re: [PATCH V3 2/8] drivers: boot_constraint: Add boot_constraints

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: <87378aamy9.fsf@intel.com>
Date:   Tue, 29 Aug 2017 09:48:46 +0300
From:   Jani Nikula <jani.nikula@...ux.intel.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Viresh Kumar <viresh.kumar@...aro.org>
Cc:     Jonathan Corbet <corbet@....net>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Mark Brown <broonie@...nel.org>,
        Stephen Boyd <sboyd@...eaurora.org>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        Shiraz Hashim <shashim@...eaurora.org>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        robdclark@...il.com, linux-doc@...r.kernel.org
Subject: Re: [PATCH V3 2/8] drivers: boot_constraint: Add boot_constraints_disable kernel parameter

On Tue, 29 Aug 2017, Greg Kroah-Hartman <gregkh@...uxfoundation.org> wrote:
> On Tue, Aug 01, 2017 at 02:53:43PM +0530, Viresh Kumar wrote:
>> Users must be given an option to discard any constraints set by
>> bootloaders. For example, consider that a constraint is set for the LCD
>> controller's supply and the LCD driver isn't loaded by the kernel. If
>> the user doesn't need to use the LCD device, then he shouldn't be forced
>> to honour the constraint.
>> 
>> We can also think about finer control of such constraints with help of
>> some sysfs files, but a kernel parameter is fine to begin with.
>> 
>> Tested-by: Rajendra Nayak <rnayak@...eaurora.org>
>> Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt |  3 +++
>>  drivers/base/boot_constraints/core.c            | 17 +++++++++++++++++
>>  2 files changed, 20 insertions(+)
>> 
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index d9c171ce4190..0706d1b6004d 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -426,6 +426,9 @@
>>  			embedded devices based on command line input.
>>  			See Documentation/block/cmdline-partition.txt
>>  
>> +	boot_constraints_disable
>> +			Do not set any boot constraints for devices.
>
> Shouldn't that be the default?  As really, that is what the situation is
> today, why force everyone to always enable the disable value?  And
> enabling a value to disable something is usually a sign of bad naming...
>
>> +
>>  	boot_delay=	Milliseconds to delay each printk during boot.
>>  			Values larger than 10 seconds (10000) are changed to
>>  			no delay (0).
>> diff --git a/drivers/base/boot_constraints/core.c b/drivers/base/boot_constraints/core.c
>> index 366a05d6d9ba..e0c33b2b216f 100644
>> --- a/drivers/base/boot_constraints/core.c
>> +++ b/drivers/base/boot_constraints/core.c
>> @@ -24,6 +24,17 @@
>>  static LIST_HEAD(constraint_devices);
>>  static DEFINE_MUTEX(constraint_devices_mutex);
>>  
>> +static bool boot_constraints_disabled;
>
> Again, this should only be an "enable" type of option, that kicks in if
> you are using this type of bootloader/kernel interaction.  Don't force
> someone to disable it.

I might add that "disable" type options lead to annoying double
negatives. Regardless of the default, I'd generally prefer "enable" type
options that you enable/disable as needed.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center