> -----Original Message----- > From: Shannon Nelson <shnelson@xxxxxxx> > Sent: Monday, November 28, 2022 3:46 PM > To: Jakub Kicinski <kuba@xxxxxxxxxx> > Cc: Shannon Nelson <snelson@xxxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; > davem@xxxxxxxxxxxxx; mst@xxxxxxxxxx; jasowang@xxxxxxxxxx; > virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx; drivers@xxxxxxxxxxx; Keller, Jacob E > <jacob.e.keller@xxxxxxxxx> > 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: > > On Mon, 28 Nov 2022 14:25:46 -0800 Shannon Nelson wrote: > >> 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: > >> > >> 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. > > > > > I'd just expose the active "bank" via netlink directly. > > > >> 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 _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
