linux-kernel - Re: [RESEND RFC 2/3] nvmem: Add 'nvmem-blob' driver

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: <CAHQ1cqFB4vBxfCZ_40UPmX5dheSx5+70umZsAx=Nmg6xJi9C7w@mail.gmail.com>
Date:	Wed, 9 Mar 2016 09:04:23 -0800
From:	Andrey Smirnov <andrew.smirnov@...il.com>
To:	Maxime Ripard <maxime.ripard@...e-electrons.com>
Cc:	Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
	linux-kernel@...r.kernel.org,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Trent Piepho <tpiepho@...etacorp.com>
Subject: Re: [RESEND RFC 2/3] nvmem: Add 'nvmem-blob' driver

On Wed, Mar 9, 2016 at 1:58 AM, Maxime Ripard
<maxime.ripard@...e-electrons.com> wrote:
> On Tue, Mar 08, 2016 at 02:46:52PM -0800, Andrey Smirnov wrote:
>> >>
>> >> I don't think I understand what you mean, could you give me an example
>> >> of how I'd use local-mac-address property for that use case? AFAIK,
>> >> local-mac-address is just an array of bytes embedded into device tree,
>> >
>> > Well, yeah, but the nvmem-blob is also just an array of bytes embedded
>> > into the DT, right?
>>
>> One is accessible via "nvmem" API and the other one isn't.
>
> But there's no point in accessing it with nvmem in the first
> place. You want to provide some information to the ethernet driver to
> give it his mac address, that's what you want to do.

No, that's not what I want to do. I want to be able to deal with data
layout and composition of a "nvmem" cell at the data producer site and
not add custom hacks to the consumer code.

> And we already have a property that does just that, and is supported by all the
> ethernet drivers. Why would you want to add a special case to
> something that is doing exactly the same thing?

That's not what I am trying to add. The intent of "nvmem-blob" is to
be use as a part of a "nvmem-composite" cell in order to be able to
inject constants. It has nothing to do with storing complete
MAC-addresses, the fact that it can be used as a complete replacement
of "local-mac-address" is just a side-effect.

Sidenote: now that we are on the subject of using this code as
equivalent of "local-mac-address", I'd argue that if it were possible
to ignore backwards compatibility, removing "local-mac-address" from
all the drivers and .dts's and replacing it with something to the
effect of "nvmem-blob" would be a better solution since you'd be able
to handle the cases of MAC address being a part of DT and being stored
in NVM using exactly the same code-path.

>
>> >> how would it get populated with data from OTP memory of SoC?
>> >
>> > In the bootloader, or Linux, read the OTP, patch the DT to add that
>> > node, done.
>>
>> No, it's not really "done", because if you read my previous messages,
>> "read the OTP, patch the DT" is exactly the problem I am trying to
>> solve. The overall goal is to be able to read a certain "nvmem" cell
>> and patch DT with that data as MAC address,
>
> But *why* do you need to store that using nvmem in the first place?

Because I want to combine that data with other nvmem blobs using
"nvmem-composite"

> You really have two solutions here: make your ethernet driver read the
> EEPROM using nvmem directly or do it in the bootloader and set
> local-mac-address (note that you can do both).

Which would lead to per-board/per-SoC custom code. I've had that kind
of solution before I started this discussion. I am hoping that we all
can come up with more elegant solution and that's why I started this
thread.

>
>> however in it's current incarnation "nvmem" doesn't have provisions
>> to make cells that are just combination of other cells (patch #3)
>
> This use case is valid.
>
>> and to embed certain data in DT and then access it as "nvmem" cell
>> (patch #2)
>
> And this one is redundant.

How is it redundant? What kernel code allows one to create a constant
"nvmem" cell?

And why do you think #3 valid? Your solution for #2, namely:

> make your ethernet driver read the
> EEPROM using nvmem directly or do it in the bootloader and set
> local-mac-address (note that you can do both)

applies in the case of #3 as well. I can read and shuffle the data in
either consumer side (in this particular case the Ethernet driver) or
in the bootloader.