Re: [Intel-wired-lan] [PATCH net v1] ice: Fix inventory failed error during flash update

From: Tony Nguyen <anthony.l.nguyen@intel.com>
To: Mateusz Palczewski <mateusz.palczewski@intel.com>,
	<intel-wired-lan@lists.osuosl.org>,
	"Keller, Jacob E" <jacob.e.keller@intel.com>
Cc: Sylwester Dziedziuch <sylwesterx.dziedziuch@intel.com>
Subject: Re: [Intel-wired-lan] [PATCH net v1] ice: Fix inventory failed error during flash update
Date: Thu, 18 Aug 2022 14:01:50 -0700	[thread overview]
Message-ID: <99098383-28c1-9999-54f2-9cb12cf06373@intel.com> (raw)
In-Reply-To: <20220811114552.1577550-1-mateusz.palczewski@intel.com>

On 8/11/2022 4:45 AM, Mateusz Palczewski wrote:
> From: Sylwester Dziedziuch <sylwesterx.dziedziuch@intel.com>
> 
> After updating flash image on E810 card with NVM update tool
> there was an error: The inventory process failed.
> 
> This was reported at bugzilla thread #2114483 and caused by the tool
> trying to read devlink parameters fw.mgmt.minsrev and fw.undi.minsrev
> but those parameters were not registered by the driver.

Pointing to an issue when using with our userspace tool is not a good 
justification of why this should be accepted into the kernel.

> The ice NVM flash has a security revision field for the main NVM bank
> and the Option ROM bank. In addition to the revision within the module,
> the device also has a minimum security revision TLV area. This minimum
> security revision field indicates the minimum value that will be
> accepted for the associated security revision when loading the NVM bank.
> 
> These parameters are permanent (i.e. stored in flash), and are used to
> indicate the minimum security revision of the associated NVM bank. If
> the image in the bank has a lower security revision, then the flash
> loader will not continue loading that flash bank.
> 
> Fix this by adding two new devlink parameters fw.mgmt.minsrev
> and fw.undi.minsrev and function to read they respective values.
> 
> This idea was proposed before with both write and read funcionality
> but was rejected by community. This patch focuses on read only.

How is this different/addresses the issues that caused it to be rejected 
initially? What makes it acceptable now?

> Fixes: 1adf7ead8204 ("ice: enable initial devlink support")

#1 this is too big for net. #2 This is not fixing a bug for a tool that 
the community is concerned about.

> Signed-off-by: Sylwester Dziedziuch <sylwesterx.dziedziuch@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
> Link: https://lore.kernel.org/netdev/20210129004332.3004826-5-anthony.l.nguyen@intel.com/
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan