linux-kernel - Re: [RFC PATCH 2/2] regulator: add boot protection flag

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: <5D111CFC-F2DD-4372-B66F-9F2E90877A03@linaro.org>
Date:	Tue, 26 Apr 2016 19:46:07 +0800
From:	Pingbo Wen <pingbo.wen@...aro.org>
To:	Mark Brown <broonie@...nel.org>
Cc:	Pingbo Wen <pingbo.wen@...aro.org>, linux-kernel@...r.kernel.org,
	lgirdwood@...il.com, Vincent Guittot <vincent.guittot@...aro.org>,
	stephen.boyd@...aro.org
Subject: Re: [RFC PATCH 2/2] regulator: add boot protection flag

Hi, Mark

> 在 2016年4月25日，06:51，Mark Brown <broonie@...nel.org> 写道：
> 
> On Sat, Apr 23, 2016 at 03:11:06PM +0800, WEN Pingbo wrote:
> 
>> This patch try to add a boot_protection flag in regulator constraints.
>> So the regulator core will prevent the specified operation during kernel
>> booting.
> 
>> The boot_protection flag only work before late_initicall. And as other
>> constraints liked, you can specify this flag in a board file, or in
>> dts file. By default, all operations of this regulator will be rejected
>> during kernel booting, if you add this flag in a regulator. But you
>> still have a chance to change this, by modifying boot_valid_ops_mask.
> 
> This is still a complete hack which is going to break as soon as things
> are built modular, it's definitely *not* something that should ever
> appear in DT since it depends so heavily on implementation details.  If
> you need some driver to start early work on getting that sorted.
> 

I think this patch can handle the case you mentioned. I have add a
regulator_has_booted flag, and it will set in regulator_init_complete()
late_initcall hook. The regulator_ops_is_valid() will ignore boot
protection if this flag is set.

> This is also going to interact badly with any other drivers that are
> trying to configure things at runtime, if they've done enables and
> disables (or especially an enable without a matching disable) their
> refcounts are going to be wrong and if they've tried to do anything with
> setting voltages we'll have completely ignored whatever they asked for
> or told them that they can't change voltages.  If we were doing anything
> like this it would need to be a lot more transparent to other
> regulators sharing the supplies (which are presumably what's causing
> problems here).

Ok, I have to admit that the boot_protection didn’t cover this. If other
consumer try to configure during booting, it will get some error code.
And the consumers need to re-configure the regulator state after
late_initcall.

If we need to hold the state of other consumer, I prefer using a
dummy-consumer to hold this. And this is my next try.

> 
>> @@ -868,7 +877,7 @@ static void print_constraints(struct regulator_dev *rdev)
>> 	rdev_dbg(rdev, "%s\n", buf);
>> 
>> 	if ((constraints->min_uV != constraints->max_uV) &&
>> -	    !regulator_ops_is_valid(rdev, REGULATOR_CHANGE_VOLTAGE))
>> +	    !(constraints->valid_ops_mask & REGULATOR_CHANGE_VOLTAGE))
>> 		rdev_warn(rdev,
>> 			  "Voltage range but no REGULATOR_CHANGE_VOLTAGE\n");
>> }
> 
> This appears to be unrelated?
> 
>> +	if (constraints->boot_protection) {
>> +		if (of_property_read_bool(np, "boot-allow-set-voltage"))
>> +			constraints->boot_valid_ops_mask |=
>> +				REGULATOR_CHANGE_VOLTAGE;
> 
> We were factoring things out a minute ago…

Actually, the two part are only want to check the regulator operation
capacity, if we include the boot_protect checking, it will get error.

Pingbo