linux-kernel - Re: [PATCH] dt-bindings: nvmem: add U-Boot environment variables binding

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6679b98e-f5f0-8608-a36c-c0c41fb5ea6f@gmail.com>
Date:   Tue, 15 Feb 2022 09:57:21 -0500
From:   Sean Anderson <seanga2@...il.com>
To:     Michal Simek <michal.simek@...inx.com>,
        Rafał Miłecki <zajec5@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Tom Rini <trini@...sulko.com>, Simon Glass <sjg@...omium.org>
Cc:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Ricardo Salveti <ricardo@...ndries.io>,
        Jorge Ramirez-Ortiz <jorge@...ndries.io>,
        devicetree@...r.kernel.org, u-boot@...ts.denx.de,
        linux-kernel@...r.kernel.org,
        Rafał Miłecki <rafal@...ecki.pl>
Subject: Re: [PATCH] dt-bindings: nvmem: add U-Boot environment variables
 binding

On 2/15/22 9:02 AM, Michal Simek wrote:
> 
> 
> On 2/15/22 14:49, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@...ecki.pl>
>>
>> U-Boot uses environment variables for storing device setup data on
>> flash. That data usually needs to be accessed by a bootloader, kernel
>> and often user-space.
>>
>> This binding allows describing environment data location and its format
>> clearly. In some/many cases it should be cleaner than hardcoding &
>> duplicating that info in multiple places. Bootloader & kernel can share
>> DTS and user-space can try reading it too or just have correct data
>> exposed by a kernel.
>>
>> Signed-off-by: Rafał Miłecki <rafal@...ecki.pl>
>> ---
>>   .../devicetree/bindings/nvmem/u-boot,env.yaml | 58 +++++++++++++++++++
>>   MAINTAINERS                                   |  5 ++
>>   2 files changed, 63 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>> new file mode 100644
>> index 000000000000..a2b3a9b88eb8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>> @@ -0,0 +1,58 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/nvmem/u-boot,env.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: U-Boot environment variables
>> +
>> +description: |
>> +  U-Boot uses environment variables to store device parameters and
>> +  configuration. They may be used for booting process, setup or keeping end user
>> +  info.
>> +
>> +  Data is stored on flash in a U-Boot specific format (header and NUL separated
>> +  key-value pairs).
>> +
>> +  This binding allows specifying data location and used format.
>> +
>> +maintainers:
>> +  - Rafał Miłecki <rafal@...ecki.pl>
>> +
>> +allOf:
>> +  - $ref: nvmem.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - description: A standalone env data block
>> +        const: u-boot,env
>> +      - description: Two redundant blocks with active one flagged
>> +        const: u-boot,env-redundant-bool
>> +      - description: Two redundant blocks with active having higher counter
>> +        const: u-boot,env-redundant-count
> 
> I am not convinced that this is the best way how to do it. Because in u-boot implementation you would have to enable MTD partitions to get there.
> And the whole parsing will take a lot of time.
> 
> I think the way how I think this can be handled is.
> 
> # I don't think that discussion with Simon was finished.
> But for example (chosen or firmware node)
> chosen {
>      u-boot {
>          u-boot,env = <&qspi &part0>;
>          u-boot,env-redundant = <&qspi &part1>;
>          #or
>          u-boot,env = <&qspi 0 40000>;
>          u-boot,env-redundant = <&qspi 40000 40000>;

What about when the environment is on top of UBI?

>          #or
>          u-boot,env = <&mmc 0 0 10000>; #device/start/size - raw mode
>          u-boot,env = <&mmc 0 1>; # device/partition - as file to FS

For emmc at least you will need another cell for the hardware partition.
And of course, you can name the environment file whatever you want, so
that needs to be recorded as well.

IMO to do this properly you'd need to have a property corresponding to
each of the major configs in the env menu.

--Sean

>          #etc.
>      };
> };
> 
> 
> &qspi {
>      flash {
>          partitions {
>              compatible = "fixed-partitions";
>              #address-cells = <1>;
>              #size-cells = <1>;
> 
>              part0: partition@0 {
>                  label = "u-boot-env";
>                  reg = <0x0 0x40000>;
>              };
> 
>              part1: partition@...00 {
>                  label = "u-boot-env-redundant";
>                  reg = <0x40000 0x10000>;
>              };
>      };
> };
> 
> Thanks,
> Michal
>