-----Original Message-----
From: Shannon Nelson <redacted>
Sent: Monday, November 28, 2022 3:46 PM
To: Jakub Kicinski <kuba@kernel.org>
Cc: Shannon Nelson <redacted>; netdev@vger.kernel.org;
davem@davemloft.net; mst@redhat.com; jasowang@redhat.com;
virtualization@lists.linux-foundation.org; drivers@pensando.io; Keller, Jacob E
[off-list ref]
Subject: Re: [RFC PATCH net-next 06/19] pds_core: add FW update feature to
devlink

On 11/28/22 3:33 PM, Jakub Kicinski wrote:

quoted

On Mon, 28 Nov 2022 14:25:46 -0800 Shannon Nelson wrote:

quoted

I don't think Intel selects which FW image to boot, but it looks like
mlxsw and nfp use the PARAM_GENERIC_FW_LOAD_POLICY to select between
DRIVER, FLASH, or DISK.  Shall I add a couple of generic SLOT_x items to
the enum devlink_param_fw_load_policy_value and use this API?  For

example:

quoted

quoted

       DEVLINK_PARAM_FW_LOAD_POLICY_VALUE_SLOT_0,
       DEVLINK_PARAM_FW_LOAD_POLICY_VALUE_SLOT_1,
       DEVLINK_PARAM_FW_LOAD_POLICY_VALUE_SLOT_2,
       DEVLINK_PARAM_FW_LOAD_POLICY_VALUE_SLOT_3,

Not the worst idea, although I presume normal FW flashing should switch
between slots to activate the new image by default? Which means the
action of fw flashing would alter the policy set by the user. A little
awkward from an API purist standpoint.

This could potentially be handled by having DELVINK_PARAM_FW_LOAD_POLICY_FLASH be the automatic "select best version", and if a user has set a manual value then don't allow flashing until a reboot or the value is set back to POLICY_FLASH?

Yes, the action of flashing will set the new bank/slot to use on the
next boot.  However, we have the ability to select from multiple valid
images and we want to pass this flexibility to the user rather than
force them to go through a whole flash sequence just to get to the other
bank.

quoted

I'd just expose the active "bank" via netlink directly.

quoted

I could then modify the devlink dev info printed to refer to fw_slot_0,
fw.slot_1, and fw.slot_2 instead of our vendor specific names.

Jake, didn't you have a similar capability in ice?

Knowing my memory I may have acquiesced to something in another driver
already. That said - I think it's cleaner if we just list the stored
versions per bank, no?

We are listing the stored images in the devlink dev info output, just
want to let the user choose which of those valid images to use next.

Cheers,
sln

Technically I think we could do something similar in ice to switch between the banks, at least as long as there is a valid image in the bank. The big trick is that I am not sure we can verify ahead of time whether we have a valid image and if you happen to boot into an invalid or blank image. There is some recovery firmware that should activate in that case, but I think our current driver doesn't implement enough of a recovery mode to actually handle this case to allow user to switch back.

Still, I think the ability to select the bank is valuable, and finding the right way to expose it is good.

Thanks,
Jake