diff mbox series

[net-next] octeon_ep: Add control plane host and firmware versions.

Message ID 20230629084227.98848-1-sedara@marvell.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net-next] octeon_ep: Add control plane host and firmware versions. | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
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: 8 this patch: 8
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang fail Errors and warnings before: 36 this patch: 31
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 fail Errors and warnings before: 10 this patch: 21
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Sathesh B Edara June 29, 2023, 8:42 a.m. UTC
Implement control plane mailbox versions for host and firmware.
Versions are published in info area of control mailbox bar4
memory structure.Firmware will publish minimum and maximum
supported versions.Host will validate itself against the
firmware version range before using the control plane mailbox.
Control plane mailbox apis will check for firmware version
before sending any control commands to firmware.Notifications
from firmware will similarly be checked for host version
compatibility.

Signed-off-by: Sathesh Edara <sedara@marvell.com>
---
 .../marvell/octeon_ep/octep_ctrl_mbox.c       |  9 ++++-
 .../marvell/octeon_ep/octep_ctrl_mbox.h       |  6 +++
 .../marvell/octeon_ep/octep_ctrl_net.c        | 39 ++++++++++++++++++-
 .../marvell/octeon_ep/octep_ctrl_net.h        | 16 ++++++++
 4 files changed, 67 insertions(+), 3 deletions(-)

Comments

kernel test robot June 29, 2023, 12:35 p.m. UTC | #1
Hi Sathesh,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Sathesh-Edara/octeon_ep-Add-control-plane-host-and-firmware-versions/20230629-164335
base:   net-next/main
patch link:    https://lore.kernel.org/r/20230629084227.98848-1-sedara%40marvell.com
patch subject: [net-next PATCH] octeon_ep: Add control plane host and firmware versions.
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20230629/202306292050.kg5Y1BXr-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230629/202306292050.kg5Y1BXr-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306292050.kg5Y1BXr-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/net/ethernet/marvell/octeon_ep/octep_main.c:18:
>> drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h:10:10: fatal error: octep_cp_version.h: No such file or directory
      10 | #include "octep_cp_version.h"
         |          ^~~~~~~~~~~~~~~~~~~~
   compilation terminated.


vim +10 drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h

     9	
  > 10	#include "octep_cp_version.h"
    11
Andrew Lunn June 29, 2023, 1:40 p.m. UTC | #2
>  int octep_ctrl_net_init(struct octep_device *oct)
>  {
>  	struct octep_ctrl_mbox *ctrl_mbox;
> @@ -84,12 +101,22 @@ int octep_ctrl_net_init(struct octep_device *oct)
>  
>  	/* Initialize control mbox */
>  	ctrl_mbox = &oct->ctrl_mbox;
> +	ctrl_mbox->version = OCTEP_CP_VERSION_CURRENT;
>  	ctrl_mbox->barmem = CFG_GET_CTRL_MBOX_MEM_ADDR(oct->conf);
>  	ret = octep_ctrl_mbox_init(ctrl_mbox);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to initialize control mbox\n");
>  		return ret;
>  	}
> +	dev_info(&pdev->dev, "Control plane versions host: %llx, firmware: %x:%x\n",
> +		 ctrl_mbox->version, ctrl_mbox->min_fw_version,
> +		 ctrl_mbox->max_fw_version);

Please consider exporting this information via devlink.

> +	ret = validate_fw_version(ctrl_mbox);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Control plane version mismatch\n");
> +		octep_ctrl_mbox_uninit(ctrl_mbox);
> +		return -EINVAL;
> +	}

If i'm reading this correct, a mismatch is fatal, the driver probe
will error out. That sort of thing is generally not liked. The driver
worked so far with mismatched firmware. It should keep working, but
not offer the features which require matching firmware.

    Andrew
Sathesh B Edara Aug. 6, 2023, 4:54 p.m. UTC | #3
Hi Andrew,
 Apologies for late response.

-----Original Message-----
From: Andrew Lunn <andrew@lunn.ch> 
Sent: Thursday, June 29, 2023 7:10 PM
To: Sathesh B Edara <sedara@marvell.com>
Cc: linux-kernel@vger.kernel.org; Satananda Burla <sburla@marvell.com>; Veerasenareddy Burru <vburru@marvell.com>; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; Haseeb Gani <hgani@marvell.com>
Subject:  Re: [net-next PATCH] octeon_ep: Add control plane host and firmware versions.

>  int octep_ctrl_net_init(struct octep_device *oct)  {
>  	struct octep_ctrl_mbox *ctrl_mbox;
> @@ -84,12 +101,22 @@ int octep_ctrl_net_init(struct octep_device *oct)
>  
>  	/* Initialize control mbox */
>  	ctrl_mbox = &oct->ctrl_mbox;
> +	ctrl_mbox->version = OCTEP_CP_VERSION_CURRENT;
>  	ctrl_mbox->barmem = CFG_GET_CTRL_MBOX_MEM_ADDR(oct->conf);
>  	ret = octep_ctrl_mbox_init(ctrl_mbox);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to initialize control mbox\n");
>  		return ret;
>  	}
> +	dev_info(&pdev->dev, "Control plane versions host: %llx, firmware: %x:%x\n",
> +		 ctrl_mbox->version, ctrl_mbox->min_fw_version,
> +		 ctrl_mbox->max_fw_version);

> Please consider exporting this information via devlink.

Sure I agree. This will be implemented as separate patch.

> +	ret = validate_fw_version(ctrl_mbox);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Control plane version mismatch\n");
> +		octep_ctrl_mbox_uninit(ctrl_mbox);
> +		return -EINVAL;
> +	}

> If i'm reading this correct, a mismatch is fatal, the driver probe will error out. That sort of thing is generally not liked. The driver worked so far with mismatched firmware. It should keep working, but not offer the features which require matching firmware.

Yes. You are correct this check is not required and I will submit the V2 patch by excluding this firmware version check. 

 >  Andrew
Thanks,
Sathesh
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_mbox.c b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_mbox.c
index 035ead7935c7..c46179b5de6f 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_mbox.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_mbox.c
@@ -37,7 +37,9 @@ 
 
 #define OCTEP_CTRL_MBOX_INFO_MAGIC_NUM(m)	(m)
 #define OCTEP_CTRL_MBOX_INFO_BARMEM_SZ(m)	((m) + 8)
+#define OCTEP_CTRL_MBOX_INFO_HOST_VERSION(m)   ((m) + 16)
 #define OCTEP_CTRL_MBOX_INFO_HOST_STATUS(m)	((m) + 24)
+#define OCTEP_CTRL_MBOX_INFO_FW_VERSION(m)     ((m) + 136)
 #define OCTEP_CTRL_MBOX_INFO_FW_STATUS(m)	((m) + 144)
 
 #define OCTEP_CTRL_MBOX_H2FQ_INFO(m)	((m) + OCTEP_CTRL_MBOX_INFO_SZ)
@@ -71,7 +73,7 @@  static u32 octep_ctrl_mbox_circq_depth(u32 pi, u32 ci, u32 sz)
 
 int octep_ctrl_mbox_init(struct octep_ctrl_mbox *mbox)
 {
-	u64 magic_num, status;
+	u64 magic_num, status, fw_versions;
 
 	if (!mbox)
 		return -EINVAL;
@@ -93,6 +95,9 @@  int octep_ctrl_mbox_init(struct octep_ctrl_mbox *mbox)
 		return -EINVAL;
 	}
 
+	fw_versions = readq(OCTEP_CTRL_MBOX_INFO_FW_VERSION(mbox->barmem));
+	mbox->min_fw_version = ((fw_versions & 0xffffffff00000000ull) >> 32);
+	mbox->max_fw_version = (fw_versions & 0xffffffff);
 	mbox->barmem_sz = readl(OCTEP_CTRL_MBOX_INFO_BARMEM_SZ(mbox->barmem));
 
 	writeq(OCTEP_CTRL_MBOX_STATUS_INIT,
@@ -110,6 +115,7 @@  int octep_ctrl_mbox_init(struct octep_ctrl_mbox *mbox)
 			  OCTEP_CTRL_MBOX_TOTAL_INFO_SZ +
 			  mbox->h2fq.sz;
 
+	writeq(mbox->version, OCTEP_CTRL_MBOX_INFO_HOST_VERSION(mbox->barmem));
 	/* ensure ready state is seen after everything is initialized */
 	wmb();
 	writeq(OCTEP_CTRL_MBOX_STATUS_READY,
@@ -255,6 +261,7 @@  int octep_ctrl_mbox_uninit(struct octep_ctrl_mbox *mbox)
 	if (!mbox->barmem)
 		return -EINVAL;
 
+	writeq(0, OCTEP_CTRL_MBOX_INFO_HOST_VERSION(mbox->barmem));
 	writeq(OCTEP_CTRL_MBOX_STATUS_INVALID,
 	       OCTEP_CTRL_MBOX_INFO_HOST_STATUS(mbox->barmem));
 	/* ensure uninit state is written before uninitialization */
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_mbox.h b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_mbox.h
index 9c4ff0fba6a0..7f8135788efc 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_mbox.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_mbox.h
@@ -121,6 +121,8 @@  struct octep_ctrl_mbox_q {
 };
 
 struct octep_ctrl_mbox {
+	/* control plane version */
+	u64 version;
 	/* size of bar memory */
 	u32 barmem_sz;
 	/* pointer to BAR memory */
@@ -133,6 +135,10 @@  struct octep_ctrl_mbox {
 	struct mutex h2fq_lock;
 	/* lock for f2hq */
 	struct mutex f2hq_lock;
+	/* Min control plane version supported by firmware */
+	u32 min_fw_version;
+	/* Max control plane version supported by firmware */
+	u32 max_fw_version;
 };
 
 /* Initialize control mbox.
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c
index 1cc6af2feb38..ee5ea4d2bda1 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c
@@ -14,6 +14,9 @@ 
 #include "octep_main.h"
 #include "octep_ctrl_net.h"
 
+/* Control plane version */
+#define OCTEP_CP_VERSION_CURRENT	OCTEP_CP_VERSION(1, 0, 0)
+
 static const u32 req_hdr_sz = sizeof(union octep_ctrl_net_req_hdr);
 static const u32 mtu_sz = sizeof(struct octep_ctrl_net_h2f_req_cmd_mtu);
 static const u32 mac_sz = sizeof(struct octep_ctrl_net_h2f_req_cmd_mac);
@@ -41,7 +44,13 @@  static int octep_send_mbox_req(struct octep_device *oct,
 			       struct octep_ctrl_net_wait_data *d,
 			       bool wait_for_response)
 {
-	int err, ret;
+	int err, ret, cmd;
+
+	/* check if firmware is compatible for this request */
+	cmd = d->data.req.hdr.s.cmd;
+	if (octep_ctrl_net_h2f_cmd_versions[cmd] > oct->ctrl_mbox.max_fw_version ||
+	    octep_ctrl_net_h2f_cmd_versions[cmd] < oct->ctrl_mbox.min_fw_version)
+		return -EOPNOTSUPP;
 
 	err = octep_ctrl_mbox_send(&oct->ctrl_mbox, &d->msg);
 	if (err < 0)
@@ -73,6 +82,14 @@  static int octep_send_mbox_req(struct octep_device *oct,
 	return 0;
 }
 
+static int validate_fw_version(struct octep_ctrl_mbox *ctrl_mbox)
+{
+	if (ctrl_mbox->version < ctrl_mbox->min_fw_version ||
+	    ctrl_mbox->version > ctrl_mbox->max_fw_version)
+		return -EINVAL;
+	return 0;
+}
+
 int octep_ctrl_net_init(struct octep_device *oct)
 {
 	struct octep_ctrl_mbox *ctrl_mbox;
@@ -84,12 +101,22 @@  int octep_ctrl_net_init(struct octep_device *oct)
 
 	/* Initialize control mbox */
 	ctrl_mbox = &oct->ctrl_mbox;
+	ctrl_mbox->version = OCTEP_CP_VERSION_CURRENT;
 	ctrl_mbox->barmem = CFG_GET_CTRL_MBOX_MEM_ADDR(oct->conf);
 	ret = octep_ctrl_mbox_init(ctrl_mbox);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to initialize control mbox\n");
 		return ret;
 	}
+	dev_info(&pdev->dev, "Control plane versions host: %llx, firmware: %x:%x\n",
+		 ctrl_mbox->version, ctrl_mbox->min_fw_version,
+		 ctrl_mbox->max_fw_version);
+	ret = validate_fw_version(ctrl_mbox);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Control plane version mismatch\n");
+		octep_ctrl_mbox_uninit(ctrl_mbox);
+		return -EINVAL;
+	}
 	oct->ctrl_mbox_ifstats_offset = ctrl_mbox->barmem_sz;
 
 	return 0;
@@ -273,9 +300,17 @@  static int process_mbox_notify(struct octep_device *oct,
 {
 	struct net_device *netdev = oct->netdev;
 	struct octep_ctrl_net_f2h_req *req;
+	int cmd;
 
 	req = (struct octep_ctrl_net_f2h_req *)msg->sg_list[0].msg;
-	switch (req->hdr.s.cmd) {
+	cmd = req->hdr.s.cmd;
+
+	/* check if we support this command */
+	if (octep_ctrl_net_f2h_cmd_versions[cmd] > OCTEP_CP_VERSION_CURRENT ||
+	    octep_ctrl_net_f2h_cmd_versions[cmd] < OCTEP_CP_VERSION_CURRENT)
+		return -EOPNOTSUPP;
+
+	switch (cmd) {
 	case OCTEP_CTRL_NET_F2H_CMD_LINK_STATUS:
 		if (netif_running(netdev)) {
 			if (req->link.state) {
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h
index 37880dd79116..dd4f055fa8da 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h
@@ -7,6 +7,8 @@ 
 #ifndef __OCTEP_CTRL_NET_H__
 #define __OCTEP_CTRL_NET_H__
 
+#include "octep_cp_version.h"
+
 #define OCTEP_CTRL_NET_INVALID_VFID	(-1)
 
 /* Supported commands */
@@ -39,12 +41,26 @@  enum octep_ctrl_net_h2f_cmd {
 	OCTEP_CTRL_NET_H2F_CMD_LINK_STATUS,
 	OCTEP_CTRL_NET_H2F_CMD_RX_STATE,
 	OCTEP_CTRL_NET_H2F_CMD_LINK_INFO,
+	OCTEP_CTRL_NET_H2F_CMD_MAX
+};
+
+/* Control plane version in which OCTEP_CTRL_NET_H2F_CMD was added */
+static const u32 octep_ctrl_net_h2f_cmd_versions[OCTEP_CTRL_NET_H2F_CMD_MAX] = {
+	[OCTEP_CTRL_NET_H2F_CMD_INVALID ... OCTEP_CTRL_NET_H2F_CMD_LINK_INFO] =
+	 OCTEP_CP_VERSION(1, 0, 0)
 };
 
 /* Supported fw to host commands */
 enum octep_ctrl_net_f2h_cmd {
 	OCTEP_CTRL_NET_F2H_CMD_INVALID = 0,
 	OCTEP_CTRL_NET_F2H_CMD_LINK_STATUS,
+	OCTEP_CTRL_NET_F2H_CMD_MAX
+};
+
+/* Control plane version in which OCTEP_CTRL_NET_F2H_CMD was added */
+static const u32 octep_ctrl_net_f2h_cmd_versions[OCTEP_CTRL_NET_F2H_CMD_MAX] = {
+	[OCTEP_CTRL_NET_F2H_CMD_INVALID ... OCTEP_CTRL_NET_F2H_CMD_LINK_STATUS] =
+	 OCTEP_CP_VERSION(1, 0, 0)
 };
 
 union octep_ctrl_net_req_hdr {