[PATCH iwl-next v1 1/3] pldmfw: selected component update

Konrad Knitter posted 3 patches 1 month ago
There is a newer version of this series
[PATCH iwl-next v1 1/3] pldmfw: selected component update
Posted by Konrad Knitter 1 month ago
Enable update of a selected component.

Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Konrad Knitter <konrad.knitter@intel.com>
---
 include/linux/pldmfw.h | 8 ++++++++
 lib/pldmfw/pldmfw.c    | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/linux/pldmfw.h b/include/linux/pldmfw.h
index 0fc831338226..f5047983004f 100644
--- a/include/linux/pldmfw.h
+++ b/include/linux/pldmfw.h
@@ -125,9 +125,17 @@ struct pldmfw_ops;
  * a pointer to their own data, used to implement the device specific
  * operations.
  */
+
+enum pldmfw_update_mode {
+	PLDMFW_UPDATE_MODE_FULL,
+	PLDMFW_UPDATE_MODE_SINGLE_COMPONENT,
+};
+
 struct pldmfw {
 	const struct pldmfw_ops *ops;
 	struct device *dev;
+	u16 component_identifier;
+	enum pldmfw_update_mode mode;
 };
 
 bool pldmfw_op_pci_match_record(struct pldmfw *context, struct pldmfw_record *record);
diff --git a/lib/pldmfw/pldmfw.c b/lib/pldmfw/pldmfw.c
index 6e1581b9a616..6264e2013f25 100644
--- a/lib/pldmfw/pldmfw.c
+++ b/lib/pldmfw/pldmfw.c
@@ -481,9 +481,17 @@ static int pldm_parse_components(struct pldmfw_priv *data)
 		component->component_data = data->fw->data + offset;
 		component->component_size = size;
 
+		if (data->context->mode == PLDMFW_UPDATE_MODE_SINGLE_COMPONENT &&
+		    data->context->component_identifier != component->identifier)
+			continue;
+
 		list_add_tail(&component->entry, &data->components);
 	}
 
+	if (data->context->mode == PLDMFW_UPDATE_MODE_SINGLE_COMPONENT &&
+	    list_empty(&data->components))
+		return -ENOENT;
+
 	header_crc_ptr = data->fw->data + data->offset;
 
 	err = pldm_move_fw_offset(data, sizeof(data->header_crc));
-- 
2.38.1
RE: [Intel-wired-lan] [PATCH iwl-next v1 1/3] pldmfw: selected component update
Posted by Pucha, HimasekharX Reddy 2 weeks, 6 days ago
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Konrad Knitter
> Sent: 23 October 2024 15:37
> To: intel-wired-lan@lists.osuosl.org
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; netdev@vger.kernel.org; jiri@resnulli.us; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; linux-kernel@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Knitter, Konrad <konrad.knitter@intel.com>; Marcin Szycik <marcin.szycik@linux.intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next v1 1/3] pldmfw: selected component update
>
> Enable update of a selected component.
>
> Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Konrad Knitter <konrad.knitter@intel.com>
> ---
>  include/linux/pldmfw.h | 8 ++++++++
>  lib/pldmfw/pldmfw.c    | 8 ++++++++
>  2 files changed, 16 insertions(+)
>

Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
RE: [PATCH iwl-next v1 1/3] pldmfw: selected component update
Posted by Keller, Jacob E 1 month ago

> -----Original Message-----
> From: Knitter, Konrad <konrad.knitter@intel.com>
> Sent: Wednesday, October 23, 2024 3:07 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; netdev@vger.kernel.org;
> jiri@resnulli.us; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; linux-kernel@vger.kernel.org; Nguyen,
> Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Knitter, Konrad <konrad.knitter@intel.com>;
> Marcin Szycik <marcin.szycik@linux.intel.com>
> Subject: [PATCH iwl-next v1 1/3] pldmfw: selected component update
> 
> Enable update of a selected component.
> 
> Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Konrad Knitter <konrad.knitter@intel.com>

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Thanks for the PLDM improvement!
Re: [Intel-wired-lan] [PATCH iwl-next v1 1/3] pldmfw: selected component update
Posted by Paul Menzel 1 month ago
Dear Konrad,


Thank you for your patch.

Am 23.10.24 um 12:07 schrieb Konrad Knitter:
> Enable update of a selected component.

It’d be great if you used that for the summary/title to make it a 
statement (by adding a verb in imperative mood).

It’d be great, if you elaborated, what that feature is, and included the 
documentation used for the implementation. Also, how can it be tested?

> Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Konrad Knitter <konrad.knitter@intel.com>
> ---
>   include/linux/pldmfw.h | 8 ++++++++
>   lib/pldmfw/pldmfw.c    | 8 ++++++++
>   2 files changed, 16 insertions(+)
> 
> diff --git a/include/linux/pldmfw.h b/include/linux/pldmfw.h
> index 0fc831338226..f5047983004f 100644
> --- a/include/linux/pldmfw.h
> +++ b/include/linux/pldmfw.h
> @@ -125,9 +125,17 @@ struct pldmfw_ops;
>    * a pointer to their own data, used to implement the device specific
>    * operations.
>    */
> +
> +enum pldmfw_update_mode {
> +	PLDMFW_UPDATE_MODE_FULL,
> +	PLDMFW_UPDATE_MODE_SINGLE_COMPONENT,
> +};
> +
>   struct pldmfw {
>   	const struct pldmfw_ops *ops;
>   	struct device *dev;
> +	u16 component_identifier;
> +	enum pldmfw_update_mode mode;
>   };
>   
>   bool pldmfw_op_pci_match_record(struct pldmfw *context, struct pldmfw_record *record);
> diff --git a/lib/pldmfw/pldmfw.c b/lib/pldmfw/pldmfw.c
> index 6e1581b9a616..6264e2013f25 100644
> --- a/lib/pldmfw/pldmfw.c
> +++ b/lib/pldmfw/pldmfw.c
> @@ -481,9 +481,17 @@ static int pldm_parse_components(struct pldmfw_priv *data)
>   		component->component_data = data->fw->data + offset;
>   		component->component_size = size;
>   
> +		if (data->context->mode == PLDMFW_UPDATE_MODE_SINGLE_COMPONENT &&
> +		    data->context->component_identifier != component->identifier)
> +			continue;
> +
>   		list_add_tail(&component->entry, &data->components);
>   	}
>   
> +	if (data->context->mode == PLDMFW_UPDATE_MODE_SINGLE_COMPONENT &&
> +	    list_empty(&data->components))
> +		return -ENOENT;
> +
>   	header_crc_ptr = data->fw->data + data->offset;
>   
>   	err = pldm_move_fw_offset(data, sizeof(data->header_crc));


Kind regards,

Paul
RE: [Intel-wired-lan] [PATCH iwl-next v1 1/3] pldmfw: selected component update
Posted by Knitter, Konrad 1 month ago
> -----Original Message-----
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Wednesday, October 23, 2024 12:07 PM
> To: Knitter, Konrad <konrad.knitter@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; Keller, Jacob E
> <jacob.e.keller@intel.com>; netdev@vger.kernel.org; jiri@resnulli.us;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; linux-kernel@vger.kernel.org; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Marcin Szycik
> <marcin.szycik@linux.intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1 1/3] pldmfw: selected
> component update
> 
> Dear Konrad,
> 
> 
> Thank you for your patch.
> 
> Am 23.10.24 um 12:07 schrieb Konrad Knitter:
> > Enable update of a selected component.
> 
> It’d be great if you used that for the summary/title to make it a
> statement (by adding a verb in imperative mood).
> 
> It’d be great, if you elaborated, what that feature is, and included the
> documentation used for the implementation. 
> 

Please comment if this would be enough:

This patch enables to update a selected component from PLDM image containing multiple components.

Example usage:

struct pldmfw;
data.mode = PLDMFW_UPDATE_MODE_SINGLE_COMPONENT;
data.compontent_identifier = DRIVER_FW_MGMT_COMPONENT_ID;

>> Also, how can it be tested?

This is enabler for ice patch, where on legacy FW versions where only "fw.mgmt" component can is 
allowed to be flashed in recovery mode.

There are no dedicated images for recovery.
Usual image contains multiple components.

This patch can be then tested together with other patches in series on Intel E810 NIC - where you can 
now execute update full card or just "fw.mgmt" component.

> > Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > Signed-off-by: Konrad Knitter <konrad.knitter@intel.com>
> > ---
> >   include/linux/pldmfw.h | 8 ++++++++
> >   lib/pldmfw/pldmfw.c    | 8 ++++++++
> >   2 files changed, 16 insertions(+)
> >
> > diff --git a/include/linux/pldmfw.h b/include/linux/pldmfw.h
> > index 0fc831338226..f5047983004f 100644
> > --- a/include/linux/pldmfw.h
> > +++ b/include/linux/pldmfw.h
> > @@ -125,9 +125,17 @@ struct pldmfw_ops;
> >    * a pointer to their own data, used to implement the device specific
> >    * operations.
> >    */
> > +
> > +enum pldmfw_update_mode {
> > +	PLDMFW_UPDATE_MODE_FULL,
> > +	PLDMFW_UPDATE_MODE_SINGLE_COMPONENT,
> > +};
> > +
> >   struct pldmfw {
> >   	const struct pldmfw_ops *ops;
> >   	struct device *dev;
> > +	u16 component_identifier;
> > +	enum pldmfw_update_mode mode;
> >   };
> >
> >   bool pldmfw_op_pci_match_record(struct pldmfw *context, struct
> pldmfw_record *record);
> > diff --git a/lib/pldmfw/pldmfw.c b/lib/pldmfw/pldmfw.c
> > index 6e1581b9a616..6264e2013f25 100644
> > --- a/lib/pldmfw/pldmfw.c
> > +++ b/lib/pldmfw/pldmfw.c
> > @@ -481,9 +481,17 @@ static int pldm_parse_components(struct
> pldmfw_priv *data)
> >   		component->component_data = data->fw->data + offset;
> >   		component->component_size = size;
> >
> > +		if (data->context->mode ==
> PLDMFW_UPDATE_MODE_SINGLE_COMPONENT &&
> > +		    data->context->component_identifier != component-
> >identifier)
> > +			continue;
> > +
> >   		list_add_tail(&component->entry, &data->components);
> >   	}
> >
> > +	if (data->context->mode ==
> PLDMFW_UPDATE_MODE_SINGLE_COMPONENT &&
> > +	    list_empty(&data->components))
> > +		return -ENOENT;
> > +
> >   	header_crc_ptr = data->fw->data + data->offset;
> >
> >   	err = pldm_move_fw_offset(data, sizeof(data->header_crc));
> 
> 
> Kind regards,
> 
> Paul