Message-ID: <20200721135626.GC2205@nanopsycho>
Date:   Tue, 21 Jul 2020 15:56:26 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jacob Keller <jacob.e.keller@...el.com>
Cc:     Jakub Kicinski <kubakici@...pl>, netdev@...r.kernel.org,
        Tom Herbert <tom@...bertland.com>,
        Jiri Pirko <jiri@...lanox.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        Michael Chan <michael.chan@...adcom.com>,
        Bin Luo <luobin9@...wei.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Leon Romanovsky <leon@...nel.org>,
        Ido Schimmel <idosch@...lanox.com>,
        Danielle Ratson <danieller@...lanox.com>
Subject: Re: [RFC PATCH net-next v2 6/6] devlink: add overwrite mode to flash
 update

Mon, Jul 20, 2020 at 08:52:58PM CEST, jacob.e.keller@...el.com wrote:
>
>
>On 7/20/2020 8:51 AM, Jakub Kicinski wrote:
>> On Mon, 20 Jul 2020 12:09:53 +0200 Jiri Pirko wrote:
>>> This looks odd. You have a single image yet you somehow divide it
>>> into "program" and "config" areas. We already have infra in place to
>>> take care of this. See DEVLINK_ATTR_FLASH_UPDATE_COMPONENT.
>>> You should have 2 components:
>>> 1) "program"
>>> 2) "config"
>>>
>
>First off, unfortunately at least for ice, the "main" section of NVM
>contains both the management firmware as well as config settings. I
>don't really have a way to split it up.

You don't have to split it up. Just for component "x" you push binary
"A" and flash part of it and for comonent "y" you push the same binary
"A" and flash different part of it.

Consider the component as a "mask" in your case.


>
>This series includes support for updating the main NVM section
>containing the management firmware (and some config) "fw.mgmt", as well
>as "fw.undi" which contains the OptionROM, and "fw.netlist" which
>contains additional configuration TLVs.
>
>The firmware interface allows me to separate the three components, but
>does not let me separate the "fw binary" from the "config settings" that
>are stored within the main NVM bank. (These fields include other data
>like the device MAC address and VPD area of the device too, so using
>"config" is a bit of a misnomer).
>
>>> Then it is up to the user what he decides to flash.
>> 
>> 99.9% of the time users want to flash "all". To achieve "don't flash
>> config" with current infra users would have to flash each component 
>> one by one and then omit the one(s) which is config (guessing which 
>> one that is based on the name).
>> 
>> Wouldn't this be quite inconvenient?
>> 
>
>I also agree here, I'd like to be able to make the "update with the
>complete file" just work in the most straight forward  way (i.e. without
>erasing stuff by accident) be the default.
>
>The option I'm proposing here is to enable allowing tools to optionally
>specify handling this type of overwrite. The goal is to support these
>use cases:
>
>a) (default) just update the image, but keep the config and vital data
>the same as before the update.
>
>b) overwrite config fields, but keep vital fields the same. Intended to
>allow returning configuration to "image defaults". This mostly is
>intended in case regular update caused some issues like if somehow the
>config preservation didn't work properly.
>
>c) overwrite all fields. The intention here is to allow programming a
>customized image during initial setup that would contain new IDs etc. It
>is not expected to be used in general, as this does overwrite vital data
>like the MAC addresses and such.
>
>So the problem is that the vital data, config data, and firmware
>binaries are stored in the same section, without a good way to separate
>between them. We program all of these together as one chunk to the
>"secondary NVM bank"  and then ask firmware to update. It reads through
>and based on our "preservation" setting will update the binaries and
>merge the configuration sections.
>
>> In case of MLX is PSID considered config?
>> 

Powered by blists - more mailing lists