diff mbox series

[RFC,net-next,7/9] ethtool: cmis_cdb: Add a layer for supporting CDB commands

Message ID 20240122084530.32451-8-danieller@nvidia.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add ability to flash modules' firmware | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl fail Generated files up to date; build failed; build has 8 warnings/errors; GEN HAS DIFF 2 files changed, 227 insertions(+);
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1079 this patch: 1079
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang fail Errors and warnings before: 1096 this patch: 1100
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1097 this patch: 1097
netdev/checkpatch warning CHECK: Alignment should match open parenthesis CHECK: Unnecessary parentheses around 'rpl->state' WARNING: Missing a blank line after declarations WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 82 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline fail Was 0 now: 4

Commit Message

Danielle Ratson Jan. 22, 2024, 8:45 a.m. UTC
CDB (Command Data Block Message Communication) reads and writes are
performed on memory map pages 9Fh-AFh according to the CMIS standard,
section 8.20 of revision 5.2.
Page 9Fh is used to specify the CDB command to be executed and also
provides an area for a local payload (LPL).

According to the CMIS standard, the firmware update process is done using
a CDB commands sequence that will be implemented in the next patch.

The kernel interface that will implement the firmware update using CDB
command will include 2 layers that will be added under ethtool:

* The upper layer that will be triggered from the module layer, is
  cmis_fw_update.
* The lower one is cmis_cdb.

In the future there might be more operations to implement using CDB
commands. Therefore, the idea is to keep the CDB interface clean and the
cmis_fw_update specific to the CDB commands handling it.

These two layers will communicate using the API the consists of three
functions:

- struct ethtool_cmis_cdb *
  ethtool_cmis_cdb_init(struct net_device *dev,
			struct ethtool_module_fw_flash_params *params);
- void ethtool_cmis_cdb_fini(struct ethtool_cmis_cdb *cdb);
- int ethtool_cmis_cdb_execute_cmd(struct net_device *dev,
				   struct ethtool_cmis_cdb_cmd_args *args);

Add the CDB layer to support initializing, finishing and executing CDB
commands:

* The initialization process will include creating of an ethtool_cmis_cdb
  instance, querying the module CDB support, entering and validating the
  password from user space (CMD 0x0000) and querying the module features
  (CMD 0x0040).

* The finishing API will simply free the ethtool_cmis_cdb instance.

* The executing process will write the CDB command to EEPROM using
  set_module_eeprom_by_page() that was presented earlier, and will
  process the reply from EEPROM.

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---
 net/ethtool/Makefile    |   2 +-
 net/ethtool/cmis.h      | 112 ++++++++
 net/ethtool/cmis_cdb.c  | 563 ++++++++++++++++++++++++++++++++++++++++
 net/ethtool/module_fw.h |  12 +
 4 files changed, 688 insertions(+), 1 deletion(-)
 create mode 100644 net/ethtool/cmis.h
 create mode 100644 net/ethtool/cmis_cdb.c

Comments

Simon Horman Jan. 22, 2024, 10:31 a.m. UTC | #1
On Mon, Jan 22, 2024 at 10:45:28AM +0200, Danielle Ratson wrote:

...

> +/**
> + * struct ethtool_cmis_cdb_request - CDB commands request fields as decribed in
> + *				the CMIS standard
> + * @id: Command ID.
> + * @epl_len: EPL memory length.
> + * @lpl_len: LPL memory length.
> + * @chk_code: Check code for the previous field and the payload.
> + * @resv1: Added to match the CMIS standard request continuity.
> + * @resv2: Added to match the CMIS standard request continuity.
> + * @payload: Payload for the CDB commands.
> + */
> +struct ethtool_cmis_cdb_request {
> +	__be16 id;
> +	struct_group(body,
> +		u16 epl_len;
> +		u8 lpl_len;
> +		u8 chk_code;
> +		u8 resv1;
> +		u8 resv2;
> +		u8 payload[ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH];
> +	);
> +};
> +
> +#define CDB_F_COMPLETION_VALID		BIT(0)
> +#define CDB_F_STATUS_VALID		BIT(1)
> +
> +/**
> + * struct ethtool_cmis_cdb_cmd_args - CDB commands execution arguments
> + * @req: CDB command fields as described in the CMIS standard.
> + * @max_duration: Maximum duration time for command completion in msec.
> + * @read_write_len_ext: Allowable additional number of byte octets to the LPL
> + *			in a READ or a WRITE commands.
> + * @rpl_exp_len: Expected reply length in bytes.
> + * @flags: Validation flags for CDB commands.
> + */
> +struct ethtool_cmis_cdb_cmd_args {
> +	struct ethtool_cmis_cdb_request req;
> +	u16				max_duration;
> +	u8				read_write_len_ext;
> +	u8                              rpl_exp_len;
> +	u8				flags;
> +};

...

> +int ethtool_cmis_page_init(struct ethtool_module_eeprom *page_data,
> +			   u8 page, u32 offset, u32 length)
> +{
> +	page_data->page = page;
> +	page_data->offset = offset;
> +	page_data->length = length;
> +	page_data->i2c_address = ETHTOOL_CMIS_CDB_PAGE_I2C_ADDR;
> +	page_data->data = kmalloc(page_data->length, GFP_KERNEL);
> +	if (!page_data->data)
> +		return -ENOMEM;
> +
> +	return 0;
> +}

...

> +static int
> +__ethtool_cmis_cdb_execute_cmd(struct net_device *dev,
> +			       struct ethtool_module_eeprom *page_data,
> +			       u32 offset, u32 length, void *data)
> +{
> +	const struct ethtool_ops *ops = dev->ethtool_ops;
> +	struct netlink_ext_ack extack = {};
> +	int err;
> +
> +	page_data->offset = offset;
> +	page_data->length = length;
> +
> +	memset(page_data->data, 0, ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH);
> +	memcpy(page_data->data, data, page_data->length);
> +
> +	err = ops->set_module_eeprom_by_page(dev, page_data, &extack);
> +	if (err < 0) {
> +		if (extack._msg)
> +			netdev_err(dev, "%s\n", extack._msg);
> +	}
> +
> +	return err;
> +}

...

> +int ethtool_cmis_cdb_execute_cmd(struct net_device *dev,
> +				 struct ethtool_cmis_cdb_cmd_args *args)
> +{
> +	struct ethtool_module_eeprom page_data = {};
> +	u32 offset;
> +	int err;
> +
> +	args->req.chk_code =
> +		cmis_cdb_calc_checksum(&args->req, sizeof(args->req));
> +
> +	if (args->req.lpl_len > args->read_write_len_ext) {
> +		ethnl_module_fw_flash_ntf_err(dev,
> +					      "LPL length is longer than CDB read write length extension allows");
> +		return -EINVAL;
> +	}
> +
> +	err = ethtool_cmis_page_init(&page_data, ETHTOOL_CMIS_CDB_CMD_PAGE, 0,
> +				     ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH);

ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH is passed as the length argument
of ethtool_cmis_page_init, which will allocate that many
bytes for page_data->data.

> +	if (err < 0)
> +		return err;
> +
> +	/* According to the CMIS standard, there are two options to trigger the
> +	 * CDB commands. The default option is triggering the command by writing
> +	 * the CMDID bytes. Therefore, the command will be split to 2 calls:
> +	 * First, with everything except the CMDID field and then the CMDID
> +	 * field.
> +	 */
> +	offset = CMIS_CDB_CMD_ID_OFFSET +
> +		offsetof(struct ethtool_cmis_cdb_request, body);
> +	err = __ethtool_cmis_cdb_execute_cmd(dev, &page_data, offset,
> +					     sizeof(args->req.body),
> +					     &args->req.body);

Hi Danielle,

However, here sizeof(args->req.body) is passed as the length
argument of __ethtool_cmis_cdb_execute_cmd() which will:

1. Zero ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH bytes of page_data->data
2. Copy sizeof(args->req.body) bytes into page_data->data

args->req.body includes several fields, one of which is
ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH bytes long. So,
args->req.body > ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH
and it seems that step 2 above causes a buffer overrun.

Flagged by clang-17 W=1 build

 In file included from net/ethtool/cmis_cdb.c:3:
 In file included from ./include/linux/ethtool.h:16:
 In file included from ./include/linux/bitmap.h:12:
 In file included from ./include/linux/string.h:295:
 ./include/linux/fortify-string.h:579:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
   579 |                         __write_overflow_field(p_size_field, size);
       |                         ^
 ./include/linux/fortify-string.h:579:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
 ./include/linux/fortify-string.h:579:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]


> +	if (err < 0)
> +		goto out;
> +
> +	offset = CMIS_CDB_CMD_ID_OFFSET +
> +		offsetof(struct ethtool_cmis_cdb_request, id);
> +	err = __ethtool_cmis_cdb_execute_cmd(dev, &page_data, offset,
> +					     sizeof(args->req.id),
> +					     &args->req.id);
> +	if (err < 0)
> +		goto out;
> +
> +	err = cmis_cdb_wait_for_completion(dev, args);
> +	if (err < 0)
> +		goto out;
> +
> +	err = cmis_cdb_wait_for_status(dev, args);
> +	if (err < 0)
> +		goto out;
> +
> +	err = cmis_cdb_process_reply(dev, &page_data, args);
> +
> +out:
> +	ethtool_cmis_page_fini(&page_data);
> +	return err;
> +}

...
Danielle Ratson Jan. 22, 2024, 2:35 p.m. UTC | #2
> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: Monday, 22 January 2024 12:31
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: netdev@vger.kernel.org; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; corbet@lwn.net;
> linux@armlinux.org.uk; sdf@google.com; kory.maincent@bootlin.com;
> maxime.chevallier@bootlin.com; vladimir.oltean@nxp.com;
> przemyslaw.kitszel@intel.com; ahmed.zaki@intel.com;
> richardcochran@gmail.com; shayagr@amazon.com;
> paul.greenwalt@intel.com; jiri@resnulli.us; linux-doc@vger.kernel.org; linux-
> kernel@vger.kernel.org; mlxsw <mlxsw@nvidia.com>; Petr Machata
> <petrm@nvidia.com>; Ido Schimmel <idosch@nvidia.com>
> Subject: Re: [RFC PATCH net-next 7/9] ethtool: cmis_cdb: Add a layer for
> supporting CDB commands
> 
> On Mon, Jan 22, 2024 at 10:45:28AM +0200, Danielle Ratson wrote:
> 
> ...
> 
> > +/**
> > + * struct ethtool_cmis_cdb_request - CDB commands request fields as
> decribed in
> > + *				the CMIS standard
> > + * @id: Command ID.
> > + * @epl_len: EPL memory length.
> > + * @lpl_len: LPL memory length.
> > + * @chk_code: Check code for the previous field and the payload.
> > + * @resv1: Added to match the CMIS standard request continuity.
> > + * @resv2: Added to match the CMIS standard request continuity.
> > + * @payload: Payload for the CDB commands.
> > + */
> > +struct ethtool_cmis_cdb_request {
> > +	__be16 id;
> > +	struct_group(body,
> > +		u16 epl_len;
> > +		u8 lpl_len;
> > +		u8 chk_code;
> > +		u8 resv1;
> > +		u8 resv2;
> > +		u8 payload[ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH];
> > +	);
> > +};
> > +
> > +#define CDB_F_COMPLETION_VALID		BIT(0)
> > +#define CDB_F_STATUS_VALID		BIT(1)
> > +
> > +/**
> > + * struct ethtool_cmis_cdb_cmd_args - CDB commands execution
> > +arguments
> > + * @req: CDB command fields as described in the CMIS standard.
> > + * @max_duration: Maximum duration time for command completion in
> msec.
> > + * @read_write_len_ext: Allowable additional number of byte octets to the
> LPL
> > + *			in a READ or a WRITE commands.
> > + * @rpl_exp_len: Expected reply length in bytes.
> > + * @flags: Validation flags for CDB commands.
> > + */
> > +struct ethtool_cmis_cdb_cmd_args {
> > +	struct ethtool_cmis_cdb_request req;
> > +	u16				max_duration;
> > +	u8				read_write_len_ext;
> > +	u8                              rpl_exp_len;
> > +	u8				flags;
> > +};
> 
> ...
> 
> > +int ethtool_cmis_page_init(struct ethtool_module_eeprom *page_data,
> > +			   u8 page, u32 offset, u32 length) {
> > +	page_data->page = page;
> > +	page_data->offset = offset;
> > +	page_data->length = length;
> > +	page_data->i2c_address = ETHTOOL_CMIS_CDB_PAGE_I2C_ADDR;
> > +	page_data->data = kmalloc(page_data->length, GFP_KERNEL);
> > +	if (!page_data->data)
> > +		return -ENOMEM;
> > +
> > +	return 0;
> > +}
> 
> ...
> 
> > +static int
> > +__ethtool_cmis_cdb_execute_cmd(struct net_device *dev,
> > +			       struct ethtool_module_eeprom *page_data,
> > +			       u32 offset, u32 length, void *data) {
> > +	const struct ethtool_ops *ops = dev->ethtool_ops;
> > +	struct netlink_ext_ack extack = {};
> > +	int err;
> > +
> > +	page_data->offset = offset;
> > +	page_data->length = length;
> > +
> > +	memset(page_data->data, 0,
> ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH);
> > +	memcpy(page_data->data, data, page_data->length);
> > +
> > +	err = ops->set_module_eeprom_by_page(dev, page_data, &extack);
> > +	if (err < 0) {
> > +		if (extack._msg)
> > +			netdev_err(dev, "%s\n", extack._msg);
> > +	}
> > +
> > +	return err;
> > +}
> 
> ...
> 
> > +int ethtool_cmis_cdb_execute_cmd(struct net_device *dev,
> > +				 struct ethtool_cmis_cdb_cmd_args *args) {
> > +	struct ethtool_module_eeprom page_data = {};
> > +	u32 offset;
> > +	int err;
> > +
> > +	args->req.chk_code =
> > +		cmis_cdb_calc_checksum(&args->req, sizeof(args->req));
> > +
> > +	if (args->req.lpl_len > args->read_write_len_ext) {
> > +		ethnl_module_fw_flash_ntf_err(dev,
> > +					      "LPL length is longer than CDB read
> write length extension allows");
> > +		return -EINVAL;
> > +	}
> > +
> > +	err = ethtool_cmis_page_init(&page_data,
> ETHTOOL_CMIS_CDB_CMD_PAGE, 0,
> > +
> ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH);
> 
> ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH is passed as the length argument
> of ethtool_cmis_page_init, which will allocate that many bytes for page_data-
> >data.
> 
> > +	if (err < 0)
> > +		return err;
> > +
> > +	/* According to the CMIS standard, there are two options to trigger
> the
> > +	 * CDB commands. The default option is triggering the command by
> writing
> > +	 * the CMDID bytes. Therefore, the command will be split to 2 calls:
> > +	 * First, with everything except the CMDID field and then the CMDID
> > +	 * field.
> > +	 */
> > +	offset = CMIS_CDB_CMD_ID_OFFSET +
> > +		offsetof(struct ethtool_cmis_cdb_request, body);
> > +	err = __ethtool_cmis_cdb_execute_cmd(dev, &page_data, offset,
> > +					     sizeof(args->req.body),
> > +					     &args->req.body);
> 
> Hi Danielle,
> 
> However, here sizeof(args->req.body) is passed as the length argument of
> __ethtool_cmis_cdb_execute_cmd() which will:
> 
> 1. Zero ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH bytes of page_data->data
> 2. Copy sizeof(args->req.body) bytes into page_data->data
> 
> args->req.body includes several fields, one of which is
> ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH bytes long. So,
> args->req.body > ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH
> and it seems that step 2 above causes a buffer overrun.
> 
> Flagged by clang-17 W=1 build
> 
>  In file included from net/ethtool/cmis_cdb.c:3:
>  In file included from ./include/linux/ethtool.h:16:
>  In file included from ./include/linux/bitmap.h:12:
>  In file included from ./include/linux/string.h:295:
>  ./include/linux/fortify-string.h:579:4: error: call to '__write_overflow_field'
> declared with 'warning' attribute: detected write beyond size of field (1st
> parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
>    579 |                         __write_overflow_field(p_size_field, size);
>        |                         ^
>  ./include/linux/fortify-string.h:579:4: error: call to '__write_overflow_field'
> declared with 'warning' attribute: detected write beyond size of field (1st
> parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
>  ./include/linux/fortify-string.h:579:4: error: call to '__write_overflow_field'
> declared with 'warning' attribute: detected write beyond size of field (1st
> parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
> 

Hi Simon,

Thanks for the feedback.

Ill fix that, something like:

diff --git a/net/ethtool/cmis_cdb.c b/net/ethtool/cmis_cdb.c
index b27e12871816..888b02e6eade 100644
--- a/net/ethtool/cmis_cdb.c
+++ b/net/ethtool/cmis_cdb.c
@@ -521,7 +521,7 @@ int ethtool_cmis_cdb_execute_cmd(struct net_device *dev,
        }

        err = ethtool_cmis_page_init(&page_data, ETHTOOL_CMIS_CDB_CMD_PAGE, 0,
-                                    ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH);
+                                    sizeof(args->req.body));
        if (err < 0)
                return err;

Danielle

> 
> > +	if (err < 0)
> > +		goto out;
> > +
> > +	offset = CMIS_CDB_CMD_ID_OFFSET +
> > +		offsetof(struct ethtool_cmis_cdb_request, id);
> > +	err = __ethtool_cmis_cdb_execute_cmd(dev, &page_data, offset,
> > +					     sizeof(args->req.id),
> > +					     &args->req.id);
> > +	if (err < 0)
> > +		goto out;
> > +
> > +	err = cmis_cdb_wait_for_completion(dev, args);
> > +	if (err < 0)
> > +		goto out;
> > +
> > +	err = cmis_cdb_wait_for_status(dev, args);
> > +	if (err < 0)
> > +		goto out;
> > +
> > +	err = cmis_cdb_process_reply(dev, &page_data, args);
> > +
> > +out:
> > +	ethtool_cmis_page_fini(&page_data);
> > +	return err;
> > +}
> 
> ...
Simon Horman Jan. 22, 2024, 8:03 p.m. UTC | #3
On Mon, Jan 22, 2024 at 02:35:59PM +0000, Danielle Ratson wrote:

...

> Hi Simon,
> 
> Thanks for the feedback.
> 
> Ill fix that, something like:
> 
> diff --git a/net/ethtool/cmis_cdb.c b/net/ethtool/cmis_cdb.c
> index b27e12871816..888b02e6eade 100644
> --- a/net/ethtool/cmis_cdb.c
> +++ b/net/ethtool/cmis_cdb.c
> @@ -521,7 +521,7 @@ int ethtool_cmis_cdb_execute_cmd(struct net_device *dev,
>         }
> 
>         err = ethtool_cmis_page_init(&page_data, ETHTOOL_CMIS_CDB_CMD_PAGE, 0,
> -                                    ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH);
> +                                    sizeof(args->req.body));
>         if (err < 0)
>                 return err;

Thanks Danielle,

I also think something like that should fix the problem.
Russell King (Oracle) Jan. 23, 2024, 5:17 p.m. UTC | #4
On Mon, Jan 22, 2024 at 10:45:28AM +0200, Danielle Ratson wrote:
> +int ethtool_cmis_page_init(struct ethtool_module_eeprom *page_data,
> +			   u8 page, u32 offset, u32 length)
> +{
> +	page_data->page = page;
> +	page_data->offset = offset;
> +	page_data->length = length;
> +	page_data->i2c_address = ETHTOOL_CMIS_CDB_PAGE_I2C_ADDR;
> +	page_data->data = kmalloc(page_data->length, GFP_KERNEL);
> +	if (!page_data->data)
> +		return -ENOMEM;

Hmm, so every use is forced to use kmalloc() even when it's just one
byte? That seems rather wasteful.

> +/* See section 9.4.1 "CMD 0040h: Module Features" in CMIS standard revision 5.2.
> + * struct cmis_cdb_module_features_rpl is structured layout of the flat
> + * array, ethtool_cmis_cdb_rpl::payload.
> + */
> +struct cmis_cdb_module_features_rpl {
> +	u8	resv1[CMIS_CDB_MODULE_FEATURES_RESV_DATA];
> +	__be16	max_completion_time;
> +};

Does this structure need to be packed? I would suggest it does to
ensure that the __be16 is correctly placed after the 34 bytes of u8.

Overall, I think the idea of always kmalloc()ing the data is a bad idea
at the moment. We have no implementations that DMA to/from this buffer,
and it means extra cycles spent, and an extra failure point each time
we want to do a CMIS command.

It also introduces extra complexity, where we could just be passing
a pointer to a function local variable or function local structure.

Unless we decide that the data pointer should be DMA-able from (in
which case, that needs documenting as such) then I would suggest
getting rid of the extra kmalloc()...kfree() bits.

Thanks.
Danielle Ratson Jan. 30, 2024, 7:55 a.m. UTC | #5
> > +int ethtool_cmis_page_init(struct ethtool_module_eeprom *page_data,
> > +			   u8 page, u32 offset, u32 length) {
> > +	page_data->page = page;
> > +	page_data->offset = offset;
> > +	page_data->length = length;
> > +	page_data->i2c_address = ETHTOOL_CMIS_CDB_PAGE_I2C_ADDR;
> > +	page_data->data = kmalloc(page_data->length, GFP_KERNEL);
> > +	if (!page_data->data)
> > +		return -ENOMEM;
> 
> Hmm, so every use is forced to use kmalloc() even when it's just one byte?
> That seems rather wasteful.
> 
> > +/* See section 9.4.1 "CMD 0040h: Module Features" in CMIS standard
> revision 5.2.
> > + * struct cmis_cdb_module_features_rpl is structured layout of the
> > +flat
> > + * array, ethtool_cmis_cdb_rpl::payload.
> > + */
> > +struct cmis_cdb_module_features_rpl {
> > +	u8	resv1[CMIS_CDB_MODULE_FEATURES_RESV_DATA];
> > +	__be16	max_completion_time;
> > +};
> 
> Does this structure need to be packed? I would suggest it does to ensure that
> the __be16 is correctly placed after the 34 bytes of u8.
> 
> Overall, I think the idea of always kmalloc()ing the data is a bad idea at the
> moment. We have no implementations that DMA to/from this buffer, and it
> means extra cycles spent, and an extra failure point each time we want to do a
> CMIS command.
> 
> It also introduces extra complexity, where we could just be passing a pointer
> to a function local variable or function local structure.
> 
> Unless we decide that the data pointer should be DMA-able from (in which
> case, that needs documenting as such) then I would suggest getting rid of the
> extra kmalloc()...kfree() bits.
> 
> Thanks.
> 

Will fix, thanks!

> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
diff mbox series

Patch

diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 504f954a1b28..38806b3ecf83 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -8,4 +8,4 @@  ethtool_nl-y	:= netlink.o bitset.o strset.o linkinfo.o linkmodes.o rss.o \
 		   linkstate.o debug.o wol.o features.o privflags.o rings.o \
 		   channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \
 		   tunnels.o fec.o eeprom.o stats.o phc_vclocks.o mm.o \
-		   module.o pse-pd.o plca.o mm.o
+		   module.o cmis_cdb.o pse-pd.o plca.o mm.o
diff --git a/net/ethtool/cmis.h b/net/ethtool/cmis.h
new file mode 100644
index 000000000000..a8d6f96ed26f
--- /dev/null
+++ b/net/ethtool/cmis.h
@@ -0,0 +1,112 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#define ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH		120
+#define ETHTOOL_CMIS_CDB_CMD_PAGE			0x9F
+#define ETHTOOL_CMIS_CDB_PAGE_I2C_ADDR			0x50
+
+/**
+ * struct ethtool_cmis_cdb - CDB commands parameters
+ * @cmis_rev: CMIS revision major.
+ * @read_write_len_ext: Allowable additional number of byte octets to the LPL
+ *			in a READ or a WRITE CDB commands.
+ * @max_completion_time:  Maximum CDB command completion time in msec.
+ */
+struct ethtool_cmis_cdb {
+	u8	cmis_rev;
+	u8      read_write_len_ext;
+	u16     max_completion_time;
+};
+
+enum ethtool_cmis_cdb_cmd_id {
+	ETHTOOL_CMIS_CDB_CMD_QUERY_STATUS		= 0x0000,
+	ETHTOOL_CMIS_CDB_CMD_MODULE_FEATURES		= 0x0040,
+};
+
+/**
+ * struct ethtool_cmis_cdb_request - CDB commands request fields as decribed in
+ *				the CMIS standard
+ * @id: Command ID.
+ * @epl_len: EPL memory length.
+ * @lpl_len: LPL memory length.
+ * @chk_code: Check code for the previous field and the payload.
+ * @resv1: Added to match the CMIS standard request continuity.
+ * @resv2: Added to match the CMIS standard request continuity.
+ * @payload: Payload for the CDB commands.
+ */
+struct ethtool_cmis_cdb_request {
+	__be16 id;
+	struct_group(body,
+		u16 epl_len;
+		u8 lpl_len;
+		u8 chk_code;
+		u8 resv1;
+		u8 resv2;
+		u8 payload[ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH];
+	);
+};
+
+#define CDB_F_COMPLETION_VALID		BIT(0)
+#define CDB_F_STATUS_VALID		BIT(1)
+
+/**
+ * struct ethtool_cmis_cdb_cmd_args - CDB commands execution arguments
+ * @req: CDB command fields as described in the CMIS standard.
+ * @max_duration: Maximum duration time for command completion in msec.
+ * @read_write_len_ext: Allowable additional number of byte octets to the LPL
+ *			in a READ or a WRITE commands.
+ * @rpl_exp_len: Expected reply length in bytes.
+ * @flags: Validation flags for CDB commands.
+ */
+struct ethtool_cmis_cdb_cmd_args {
+	struct ethtool_cmis_cdb_request req;
+	u16				max_duration;
+	u8				read_write_len_ext;
+	u8                              rpl_exp_len;
+	u8				flags;
+};
+
+/**
+ * struct ethtool_cmis_cdb_rpl_hdr - CDB commands reply header arguments
+ * @rpl_len: Reply length.
+ * @rpl_chk_code: Reply check code.
+ */
+struct ethtool_cmis_cdb_rpl_hdr {
+	u8 rpl_len;
+	u8 rpl_chk_code;
+};
+
+/**
+ * struct ethtool_cmis_cdb_rpl - CDB commands reply arguments
+ * @hdr: CDB commands reply header arguments.
+ * @payload: Payload for the CDB commands reply.
+ */
+struct ethtool_cmis_cdb_rpl {
+	struct ethtool_cmis_cdb_rpl_hdr hdr;
+	u8 payload[ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH];
+};
+
+u32 ethtool_cmis_get_max_payload_size(u8 num_of_byte_octs);
+
+void ethtool_cmis_cdb_compose_args(struct ethtool_cmis_cdb_cmd_args *args,
+				   enum ethtool_cmis_cdb_cmd_id cmd, u8 *pl,
+				   u8 lpl_len, u16 max_duration,
+				   u8 read_write_len_ext, u8 rpl_exp_len,
+				   u8 flags);
+
+void ethtool_cmis_cdb_check_completion_flag(u8 cmis_rev, u8 *flags);
+
+int ethtool_cmis_page_init(struct ethtool_module_eeprom *page_data,
+			   u8 page, u32 offset, u32 length);
+void ethtool_cmis_page_fini(struct ethtool_module_eeprom *page_data);
+
+struct ethtool_cmis_cdb *
+ethtool_cmis_cdb_init(struct net_device *dev,
+		      const struct ethtool_module_fw_flash_params *params);
+void ethtool_cmis_cdb_fini(struct ethtool_cmis_cdb *cdb);
+
+int ethtool_cmis_wait_for_cond(struct net_device *dev, u8 flags, u8 flag,
+			       u16 max_duration, u32 offset,
+			       bool (*cond_success)(u8), bool (*cond_fail)(u8));
+
+int ethtool_cmis_cdb_execute_cmd(struct net_device *dev,
+				 struct ethtool_cmis_cdb_cmd_args *args);
diff --git a/net/ethtool/cmis_cdb.c b/net/ethtool/cmis_cdb.c
new file mode 100644
index 000000000000..b27e12871816
--- /dev/null
+++ b/net/ethtool/cmis_cdb.c
@@ -0,0 +1,563 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/ethtool.h>
+#include <linux/jiffies.h>
+
+#include "common.h"
+#include "module_fw.h"
+#include "cmis.h"
+
+/* For accessing the LPL field on page 9Fh, the allowable length extension is
+ * min(i, 15) byte octets where i specifies the allowable additional number of
+ * byte octets in a READ or a WRITE.
+ */
+u32 ethtool_cmis_get_max_payload_size(u8 num_of_byte_octs)
+{
+	return 8 * (1 + min_t(u8, num_of_byte_octs, 15));
+}
+
+void ethtool_cmis_cdb_compose_args(struct ethtool_cmis_cdb_cmd_args *args,
+				   enum ethtool_cmis_cdb_cmd_id cmd, u8 *pl,
+				   u8 lpl_len, u16 max_duration,
+				   u8 read_write_len_ext, u8 rpl_exp_len,
+				   u8 flags)
+{
+	args->req.id = cpu_to_be16(cmd);
+	args->req.lpl_len = lpl_len;
+	if (pl)
+		memcpy(args->req.payload, pl, args->req.lpl_len);
+
+	args->max_duration = max_duration;
+	args->read_write_len_ext =
+		ethtool_cmis_get_max_payload_size(read_write_len_ext);
+	args->rpl_exp_len = rpl_exp_len;
+	args->flags = flags;
+}
+
+int ethtool_cmis_page_init(struct ethtool_module_eeprom *page_data,
+			   u8 page, u32 offset, u32 length)
+{
+	page_data->page = page;
+	page_data->offset = offset;
+	page_data->length = length;
+	page_data->i2c_address = ETHTOOL_CMIS_CDB_PAGE_I2C_ADDR;
+	page_data->data = kmalloc(page_data->length, GFP_KERNEL);
+	if (!page_data->data)
+		return -ENOMEM;
+
+	return 0;
+}
+
+void ethtool_cmis_page_fini(struct ethtool_module_eeprom *page_data)
+{
+	kfree(page_data->data);
+}
+
+#define CMIS_REVISION_PAGE	0x00
+#define CMIS_REVISION_OFFSET	0x01
+
+struct cmis_rev_rpl {
+	u8 rev;
+};
+
+static inline u8
+cmis_rev_rpl_major(struct cmis_rev_rpl *rpl)
+{
+	return rpl->rev >> 4;
+}
+
+static int cmis_rev_major_get(struct net_device *dev, u8 *rev_major)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	struct ethtool_module_eeprom page_data = {};
+	struct netlink_ext_ack extack = {};
+	struct cmis_rev_rpl *rpl;
+	int err;
+
+	err = ethtool_cmis_page_init(&page_data, CMIS_REVISION_PAGE,
+				     CMIS_REVISION_OFFSET, sizeof(*rpl));
+	if (err < 0)
+		return err;
+
+	err = ops->get_module_eeprom_by_page(dev, &page_data, &extack);
+	if (err < 0) {
+		if (extack._msg)
+			netdev_err(dev, "%s\n", extack._msg);
+		goto out;
+	}
+
+	rpl = (struct cmis_rev_rpl *)page_data.data;
+	*rev_major = cmis_rev_rpl_major(rpl);
+
+out:
+	ethtool_cmis_page_fini(&page_data);
+	return err;
+}
+
+#define CMIS_CDB_ADVERTISEMENT_PAGE	0x01
+#define CMIS_CDB_ADVERTISEMENT_OFFSET	0xA3
+
+/* Based on section 8.4.11 "CDB Messaging Support Advertisement" in CMIS
+ * standard revision 5.2.
+ */
+struct cmis_cdb_advert_rpl {
+	u8	inst_supported;
+	u8	read_write_len_ext;
+	u8	resv1;
+	u8	resv2;
+};
+
+static inline u8
+cmis_cdb_advert_rpl_inst_supported(struct cmis_cdb_advert_rpl *rpl)
+{
+	return rpl->inst_supported >> 6;
+}
+
+static int cmis_cdb_advertisement_get(struct ethtool_cmis_cdb *cdb,
+				      struct net_device *dev)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	struct ethtool_module_eeprom page_data = {};
+	struct netlink_ext_ack extack = {};
+	struct cmis_cdb_advert_rpl *rpl;
+	int err;
+
+	err = ethtool_cmis_page_init(&page_data, CMIS_CDB_ADVERTISEMENT_PAGE,
+				     CMIS_CDB_ADVERTISEMENT_OFFSET,
+				     sizeof(*rpl));
+	if (err < 0)
+		return err;
+
+	err = ops->get_module_eeprom_by_page(dev, &page_data, &extack);
+	if (err < 0) {
+		if (extack._msg)
+			netdev_err(dev, "%s\n", extack._msg);
+		goto out;
+	}
+
+	rpl = (struct cmis_cdb_advert_rpl *)page_data.data;
+	if (!cmis_cdb_advert_rpl_inst_supported(rpl)) {
+		err = -EOPNOTSUPP;
+		goto out;
+	}
+
+	cdb->read_write_len_ext = rpl->read_write_len_ext;
+
+out:
+	ethtool_cmis_page_fini(&page_data);
+	return err;
+}
+
+#define CMIS_PASSWORD_ENTRY_PAGE	0x00
+#define CMIS_PASSWORD_ENTRY_OFFSET	0x7A
+
+struct cmis_password_entry_pl {
+	u32 password;
+};
+
+/* See section 9.3.1 "CMD 0000h: Query Status" in CMIS standard revision 5.2.
+ * struct cmis_cdb_query_status_pl and struct cmis_cdb_query_status_rpl are
+ * structured layouts of the flat arrays,
+ * struct ethtool_cmis_cdb_request::payload and
+ * struct ethtool_cmis_cdb_rpl::payload respectively.
+ */
+struct cmis_cdb_query_status_pl {
+	u16 response_delay;
+};
+
+struct cmis_cdb_query_status_rpl {
+	u8 length;
+	u8 status;
+};
+
+static int
+cmis_cdb_validate_password(struct ethtool_cmis_cdb *cdb,
+			   struct net_device *dev,
+			   const struct ethtool_module_fw_flash_params *params)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	struct cmis_cdb_query_status_pl qs_pl = {0};
+	struct ethtool_module_eeprom page_data = {};
+	struct cmis_cdb_query_status_rpl *rpl;
+	struct ethtool_cmis_cdb_cmd_args args;
+	struct cmis_password_entry_pl *pe_pl;
+	struct netlink_ext_ack extack = {};
+	int err;
+
+	err = ethtool_cmis_page_init(&page_data, CMIS_PASSWORD_ENTRY_PAGE,
+				     CMIS_PASSWORD_ENTRY_OFFSET,
+				     sizeof(*pe_pl));
+	if (err < 0)
+		return err;
+
+	pe_pl = (struct cmis_password_entry_pl *)page_data.data;
+	pe_pl->password = params->password;
+	err = ops->set_module_eeprom_by_page(dev, &page_data, &extack);
+	if (err < 0) {
+		if (extack._msg)
+			netdev_err(dev, "%s\n", extack._msg);
+		goto out;
+	}
+
+	ethtool_cmis_cdb_compose_args(&args, ETHTOOL_CMIS_CDB_CMD_QUERY_STATUS,
+				      (u8 *)&qs_pl, sizeof(qs_pl), 0,
+				      cdb->read_write_len_ext, sizeof(*rpl),
+				      CDB_F_COMPLETION_VALID | CDB_F_STATUS_VALID);
+
+	err = ethtool_cmis_cdb_execute_cmd(dev, &args);
+	if (err < 0) {
+		ethnl_module_fw_flash_ntf_err(dev,
+					      "Query Status command failed");
+		goto out;
+	}
+
+	rpl = (struct cmis_cdb_query_status_rpl *)args.req.payload;
+	if (!rpl->length || !rpl->status) {
+		ethnl_module_fw_flash_ntf_err(dev, "Password was not accepted");
+		err = -EINVAL;
+	}
+
+out:
+	ethtool_cmis_page_fini(&page_data);
+	return err;
+}
+
+/* Some CDB commands asserts the CDB completion flag only from CMIS
+ * revision 5. Therefore, check the relevant validity flag only when
+ * the revision supports it.
+ */
+inline void ethtool_cmis_cdb_check_completion_flag(u8 cmis_rev, u8 *flags)
+{
+	*flags |= cmis_rev >= 5 ? CDB_F_COMPLETION_VALID : 0;
+}
+
+#define CMIS_CDB_MODULE_FEATURES_RESV_DATA	34
+
+/* See section 9.4.1 "CMD 0040h: Module Features" in CMIS standard revision 5.2.
+ * struct cmis_cdb_module_features_rpl is structured layout of the flat
+ * array, ethtool_cmis_cdb_rpl::payload.
+ */
+struct cmis_cdb_module_features_rpl {
+	u8	resv1[CMIS_CDB_MODULE_FEATURES_RESV_DATA];
+	__be16	max_completion_time;
+};
+
+static inline u16
+cmis_cdb_module_features_completion_time(struct cmis_cdb_module_features_rpl *rpl)
+{
+	return be16_to_cpu(rpl->max_completion_time);
+}
+
+static int cmis_cdb_module_features_get(struct ethtool_cmis_cdb *cdb,
+					struct net_device *dev)
+{
+	struct cmis_cdb_module_features_rpl *rpl;
+	struct ethtool_cmis_cdb_cmd_args args;
+	u8 flags = CDB_F_STATUS_VALID;
+	int err;
+
+	ethtool_cmis_cdb_check_completion_flag(cdb->cmis_rev, &flags);
+	ethtool_cmis_cdb_compose_args(&args,
+				      ETHTOOL_CMIS_CDB_CMD_MODULE_FEATURES,
+				      NULL, 0, 0, cdb->read_write_len_ext,
+				      sizeof(*rpl), flags);
+
+	err = ethtool_cmis_cdb_execute_cmd(dev, &args);
+	if (err < 0) {
+		ethnl_module_fw_flash_ntf_err(dev,
+					      "Module Features command failed");
+		return err;
+	}
+
+	rpl = (struct cmis_cdb_module_features_rpl *)args.req.payload;
+	cdb->max_completion_time =
+		cmis_cdb_module_features_completion_time(rpl);
+
+	return 0;
+}
+
+struct ethtool_cmis_cdb *
+ethtool_cmis_cdb_init(struct net_device *dev,
+		      const struct ethtool_module_fw_flash_params *params)
+{
+	struct ethtool_cmis_cdb *cdb;
+	int err;
+
+	cdb = kzalloc(sizeof(*cdb), GFP_KERNEL);
+	if (!cdb)
+		return ERR_PTR(-ENOMEM);
+
+	err = cmis_rev_major_get(dev, &cdb->cmis_rev);
+	if (err < 0)
+		goto err;
+
+	if (cdb->cmis_rev < 4) {
+		ethnl_module_fw_flash_ntf_err(dev,
+					      "CMIS revision doesn't support module firmware flashing");
+		err = -EOPNOTSUPP;
+		goto err;
+	}
+
+	err = cmis_cdb_advertisement_get(cdb, dev);
+	if (err < 0)
+		goto err;
+
+	if (params->password_valid) {
+		err = cmis_cdb_validate_password(cdb, dev, params);
+		if (err < 0)
+			goto err;
+	}
+
+	err = cmis_cdb_module_features_get(cdb, dev);
+	if (err < 0)
+		goto err;
+
+	return cdb;
+
+err:
+	ethtool_cmis_cdb_fini(cdb);
+	return ERR_PTR(err);
+}
+
+void ethtool_cmis_cdb_fini(struct ethtool_cmis_cdb *cdb)
+{
+	kfree(cdb);
+}
+
+static bool is_completed(u8 data)
+{
+	return data;
+}
+
+#define CMIS_CDB_STATUS_SUCCESS	0x01
+
+static bool status_success(u8 data)
+{
+	return data == CMIS_CDB_STATUS_SUCCESS;
+}
+
+#define CMIS_CDB_STATUS_FAIL	0x40
+
+static bool status_fail(u8 data)
+{
+	return data & CMIS_CDB_STATUS_FAIL;
+}
+
+#define CMIS_LOWER_PAGE		0x00
+#define CMIS_BYTE_LENGTH	1
+
+struct cmis_wait_for_cond_rpl {
+	u8 state;
+};
+
+int ethtool_cmis_wait_for_cond(struct net_device *dev, u8 flags, u8 flag,
+			       u16 max_duration, u32 offset,
+			       bool (*cond_success)(u8), bool (*cond_fail)(u8))
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	struct ethtool_module_eeprom page_data = {};
+	struct cmis_wait_for_cond_rpl *rpl;
+	struct netlink_ext_ack extack = {};
+	unsigned long end;
+	int err;
+
+	if (!(flags & flag))
+		return 0;
+
+	err = ethtool_cmis_page_init(&page_data, CMIS_LOWER_PAGE, offset,
+				     CMIS_BYTE_LENGTH);
+	if (err < 0)
+		return err;
+
+	if (max_duration == 0)
+		max_duration = U16_MAX;
+
+	end = jiffies + msecs_to_jiffies(max_duration);
+	do {
+		err = ops->get_module_eeprom_by_page(dev, &page_data, &extack);
+		if (err < 0) {
+			if (extack._msg)
+				netdev_err(dev, "%s\n", extack._msg);
+			continue;
+		}
+
+		rpl = (struct cmis_wait_for_cond_rpl *)page_data.data;
+		if ((*cond_success)(rpl->state))
+			goto out;
+
+		if (*cond_fail && (*cond_fail)(rpl->state))
+			break;
+
+	} while (time_before(jiffies, end));
+
+	err = -EBUSY;
+
+out:
+	ethtool_cmis_page_fini(&page_data);
+	return err;
+}
+
+#define CMIS_CDB_COMPLETION_FLAG_OFFSET	0x08
+
+static int cmis_cdb_wait_for_completion(struct net_device *dev,
+					struct ethtool_cmis_cdb_cmd_args *args)
+{
+	return ethtool_cmis_wait_for_cond(dev, args->flags,
+					  CDB_F_COMPLETION_VALID,
+					  args->max_duration,
+					  CMIS_CDB_COMPLETION_FLAG_OFFSET,
+					  is_completed, NULL);
+}
+
+#define CMIS_CDB_STATUS_OFFSET	0x25
+
+static int cmis_cdb_wait_for_status(struct net_device *dev,
+				    struct ethtool_cmis_cdb_cmd_args *args)
+{
+	return ethtool_cmis_wait_for_cond(dev, args->flags, CDB_F_STATUS_VALID,
+					  args->max_duration,
+					  CMIS_CDB_STATUS_OFFSET,
+					  status_success, status_fail);
+}
+
+#define CMIS_CDB_REPLY_OFFSET	0x86
+
+static int cmis_cdb_get_reply(struct net_device *dev,
+			      struct ethtool_module_eeprom *page_data,
+			      u8 rpl_exp_len, struct netlink_ext_ack *extack)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+
+	page_data->offset = CMIS_CDB_REPLY_OFFSET;
+	page_data->length = rpl_exp_len;
+	page_data->i2c_address = ETHTOOL_CMIS_CDB_PAGE_I2C_ADDR;
+	page_data->page = ETHTOOL_CMIS_CDB_CMD_PAGE;
+
+	return ops->get_module_eeprom_by_page(dev, page_data, extack);
+}
+
+static int cmis_cdb_process_reply(struct net_device *dev,
+				  struct ethtool_module_eeprom *page_data,
+				  struct ethtool_cmis_cdb_cmd_args *args)
+{
+	u8 rpl_hdr_len = sizeof(struct ethtool_cmis_cdb_rpl_hdr);
+	struct netlink_ext_ack extack = {};
+	struct ethtool_cmis_cdb_rpl *rpl;
+	int err;
+
+	if (!args->rpl_exp_len)
+		return 0;
+
+	err = cmis_cdb_get_reply(dev, page_data,
+				 args->rpl_exp_len + rpl_hdr_len, &extack);
+	if (err < 0) {
+		if (extack._msg)
+			netdev_err(dev, "%s\n", extack._msg);
+		return err;
+	}
+
+	rpl = (struct ethtool_cmis_cdb_rpl *)page_data->data;
+	if ((args->rpl_exp_len > rpl->hdr.rpl_len + rpl_hdr_len) ||
+	    !rpl->hdr.rpl_chk_code)
+		return -EIO;
+
+	args->req.lpl_len = rpl->hdr.rpl_len;
+	memcpy(args->req.payload, rpl->payload, args->req.lpl_len);
+
+	return 0;
+}
+
+static int
+__ethtool_cmis_cdb_execute_cmd(struct net_device *dev,
+			       struct ethtool_module_eeprom *page_data,
+			       u32 offset, u32 length, void *data)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	struct netlink_ext_ack extack = {};
+	int err;
+
+	page_data->offset = offset;
+	page_data->length = length;
+
+	memset(page_data->data, 0, ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH);
+	memcpy(page_data->data, data, page_data->length);
+
+	err = ops->set_module_eeprom_by_page(dev, page_data, &extack);
+	if (err < 0) {
+		if (extack._msg)
+			netdev_err(dev, "%s\n", extack._msg);
+	}
+
+	return err;
+}
+
+static u8 cmis_cdb_calc_checksum(const void *data, size_t size)
+{
+	const u8 *bytes = (const u8 *)data;
+	u8 checksum = 0;
+
+	for (size_t i = 0; i < size; i++)
+		checksum += bytes[i];
+
+	return ~checksum;
+}
+
+#define CMIS_CDB_CMD_ID_OFFSET	0x80
+
+int ethtool_cmis_cdb_execute_cmd(struct net_device *dev,
+				 struct ethtool_cmis_cdb_cmd_args *args)
+{
+	struct ethtool_module_eeprom page_data = {};
+	u32 offset;
+	int err;
+
+	args->req.chk_code =
+		cmis_cdb_calc_checksum(&args->req, sizeof(args->req));
+
+	if (args->req.lpl_len > args->read_write_len_ext) {
+		ethnl_module_fw_flash_ntf_err(dev,
+					      "LPL length is longer than CDB read write length extension allows");
+		return -EINVAL;
+	}
+
+	err = ethtool_cmis_page_init(&page_data, ETHTOOL_CMIS_CDB_CMD_PAGE, 0,
+				     ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH);
+	if (err < 0)
+		return err;
+
+	/* According to the CMIS standard, there are two options to trigger the
+	 * CDB commands. The default option is triggering the command by writing
+	 * the CMDID bytes. Therefore, the command will be split to 2 calls:
+	 * First, with everything except the CMDID field and then the CMDID
+	 * field.
+	 */
+	offset = CMIS_CDB_CMD_ID_OFFSET +
+		offsetof(struct ethtool_cmis_cdb_request, body);
+	err = __ethtool_cmis_cdb_execute_cmd(dev, &page_data, offset,
+					     sizeof(args->req.body),
+					     &args->req.body);
+	if (err < 0)
+		goto out;
+
+	offset = CMIS_CDB_CMD_ID_OFFSET +
+		offsetof(struct ethtool_cmis_cdb_request, id);
+	err = __ethtool_cmis_cdb_execute_cmd(dev, &page_data, offset,
+					     sizeof(args->req.id),
+					     &args->req.id);
+	if (err < 0)
+		goto out;
+
+	err = cmis_cdb_wait_for_completion(dev, args);
+	if (err < 0)
+		goto out;
+
+	err = cmis_cdb_wait_for_status(dev, args);
+	if (err < 0)
+		goto out;
+
+	err = cmis_cdb_process_reply(dev, &page_data, args);
+
+out:
+	ethtool_cmis_page_fini(&page_data);
+	return err;
+}
diff --git a/net/ethtool/module_fw.h b/net/ethtool/module_fw.h
index 36cb0bc85526..a4d4220db35d 100644
--- a/net/ethtool/module_fw.h
+++ b/net/ethtool/module_fw.h
@@ -8,3 +8,15 @@  void ethnl_module_fw_flash_ntf_start(struct net_device *dev);
 void ethnl_module_fw_flash_ntf_complete(struct net_device *dev);
 void ethnl_module_fw_flash_ntf_in_progress(struct net_device *dev, u64 done,
 					   u64 total);
+
+/**
+ * struct ethtool_module_fw_flash_params - module firmware flashing parameters
+ * @file_name: Firmware image file name.
+ * @password: Module password. Only valid when @pass_valid is set.
+ * @password_valid: Whether the module password is valid or not.
+ */
+struct ethtool_module_fw_flash_params {
+	const char *file_name;
+	u32 password;
+	u8 password_valid:1;
+};