linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv1] firmware: be compatible with older version of RSU firmware
@ 2019-10-30 20:44 richard.gong
  2019-11-02 11:16 ` kbuild test robot
  0 siblings, 1 reply; 2+ messages in thread
From: richard.gong @ 2019-10-30 20:44 UTC (permalink / raw)
  To: gregkh; +Cc: linux-kernel, dinguyen, richard.gong, Richard Gong

From: Richard Gong <richard.gong@intel.com>

The older versions of RSU firmware don't support retry and notify
features then the kernel module dies when it queries the RSU retry
counter or performs notify operation.

Update the service layer and RSU drivers to be compatible with all
versions of RSU firmware.

Reported-by: Radu Barcau <radu.bacrau@intel.com>
Signed-off-by: Richard Gong <richard.gong@intel.com>
---
 drivers/firmware/stratix10-rsu.c                   | 40 +++++++++-------------
 drivers/firmware/stratix10-svc.c                   | 18 +++++++++-
 .../linux/firmware/intel/stratix10-svc-client.h    |  8 +++++
 3 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/drivers/firmware/stratix10-rsu.c b/drivers/firmware/stratix10-rsu.c
index bb008c0..f9e1851 100644
--- a/drivers/firmware/stratix10-rsu.c
+++ b/drivers/firmware/stratix10-rsu.c
@@ -20,7 +20,6 @@
 #define RSU_VERSION_MASK		GENMASK_ULL(63, 32)
 #define RSU_ERROR_LOCATION_MASK		GENMASK_ULL(31, 0)
 #define RSU_ERROR_DETAIL_MASK		GENMASK_ULL(63, 32)
-#define RSU_FW_VERSION_MASK		GENMASK_ULL(15, 0)
 
 #define RSU_TIMEOUT	(msecs_to_jiffies(SVC_RSU_REQUEST_TIMEOUT_MS))
 
@@ -109,9 +108,12 @@ static void rsu_command_callback(struct stratix10_svc_client *client,
 {
 	struct stratix10_rsu_priv *priv = client->priv;
 
-	if (data->status != BIT(SVC_STATUS_RSU_OK))
-		dev_err(client->dev, "RSU returned status is %i\n",
-			data->status);
+	if (data->status == BIT(SVC_STATUS_RSU_NO_SUPPORT))
+		dev_warn(client->dev, "Secure FW doesn't support notify\n");
+	else if (data->status == BIT(SVC_STATUS_RSU_ERROR))
+		dev_err(client->dev, "Failure, returned status is %i\n",
+			BIT(data->status));
+
 	complete(&priv->completion);
 }
 
@@ -133,9 +135,11 @@ static void rsu_retry_callback(struct stratix10_svc_client *client,
 
 	if (data->status == BIT(SVC_STATUS_RSU_OK))
 		priv->retry_counter = *counter;
+	else if (data->status == BIT(SVC_STATUS_RSU_NO_SUPPORT))
+		dev_warn(client->dev, "Secure FW doesn't support retry\n");
 	else
 		dev_err(client->dev, "Failed to get retry counter %i\n",
-			data->status);
+			BIT(data->status));
 
 	complete(&priv->completion);
 }
@@ -333,15 +337,10 @@ static ssize_t notify_store(struct device *dev,
 		return ret;
 	}
 
-	/* only 19.3 or late version FW supports retry counter feature */
-	if (FIELD_GET(RSU_FW_VERSION_MASK, priv->status.version)) {
-		ret = rsu_send_msg(priv, COMMAND_RSU_RETRY,
-				   0, rsu_retry_callback);
-		if (ret) {
-			dev_err(dev,
-				"Error, getting RSU retry %i\n", ret);
-			return ret;
-		}
+	ret = rsu_send_msg(priv, COMMAND_RSU_RETRY, 0, rsu_retry_callback);
+	if (ret) {
+		dev_err(dev, "Error, getting RSU retry %i\n", ret);
+		return ret;
 	}
 
 	return count;
@@ -413,15 +412,10 @@ static int stratix10_rsu_probe(struct platform_device *pdev)
 		stratix10_svc_free_channel(priv->chan);
 	}
 
-	/* only 19.3 or late version FW supports retry counter feature */
-	if (FIELD_GET(RSU_FW_VERSION_MASK, priv->status.version)) {
-		ret = rsu_send_msg(priv, COMMAND_RSU_RETRY, 0,
-				   rsu_retry_callback);
-		if (ret) {
-			dev_err(dev,
-				"Error, getting RSU retry %i\n", ret);
-			stratix10_svc_free_channel(priv->chan);
-		}
+	ret = rsu_send_msg(priv, COMMAND_RSU_RETRY, 0, rsu_retry_callback);
+	if (ret) {
+		dev_err(dev, "Error, getting RSU retry %i\n", ret);
+		stratix10_svc_free_channel(priv->chan);
 	}
 
 	return ret;
diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index b4853211..c6c3140 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -493,8 +493,24 @@ static int svc_normal_to_secure_thread(void *data)
 			pdata->chan->scl->receive_cb(pdata->chan->scl, cbdata);
 			break;
 		default:
-			pr_warn("it shouldn't happen\n");
+			pr_warn("Secure firmware doesn't support...\n");
+
+			/*
+			 * be compatible with older version firmware which
+			 * doesn't support RSU notify or retry
+			 */
+			if ((pdata->command == COMMAND_RSU_RETRY) ||
+				(pdata->command == COMMAND_RSU_NOTIFY)) {
+				cbdata->status =
+					BIT(SVC_STATUS_RSU_NO_SUPPORT);
+				cbdata->kaddr1 = NULL;
+				cbdata->kaddr2 = NULL;
+				cbdata->kaddr3 = NULL;
+				pdata->chan->scl->receive_cb(
+					pdata->chan->scl, cbdata);
+			}
 			break;
+
 		}
 	};
 
diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h
index b6c4302..59bc6e2 100644
--- a/include/linux/firmware/intel/stratix10-svc-client.h
+++ b/include/linux/firmware/intel/stratix10-svc-client.h
@@ -41,6 +41,12 @@
  *
  * SVC_STATUS_RSU_OK:
  * Secure firmware accepts the request of remote status update (RSU).
+ *
+ * SVC_STATUS_RSU_ERROR:
+ * Error encountered during remote system update.
+ *
+ * SVC_STATUS_RSU_NO_SUPPORT:
+ * Secure firmware doesn't support RSU retry or notify feature.
  */
 #define SVC_STATUS_RECONFIG_REQUEST_OK		0
 #define SVC_STATUS_RECONFIG_BUFFER_SUBMITTED	1
@@ -50,6 +56,8 @@
 #define SVC_STATUS_RECONFIG_ERROR		5
 #define SVC_STATUS_RSU_OK			6
 #define SVC_STATUS_RSU_ERROR			7
+#define SVC_STATUS_RSU_NO_SUPPORT		8
+
 /**
  * Flag bit for COMMAND_RECONFIG
  *
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCHv1] firmware: be compatible with older version of RSU firmware
  2019-10-30 20:44 [PATCHv1] firmware: be compatible with older version of RSU firmware richard.gong
@ 2019-11-02 11:16 ` kbuild test robot
  0 siblings, 0 replies; 2+ messages in thread
From: kbuild test robot @ 2019-11-02 11:16 UTC (permalink / raw)
  To: richard.gong
  Cc: kbuild-all, gregkh, linux-kernel, dinguyen, richard.gong, Richard Gong

[-- Attachment #1: Type: text/plain, Size: 6954 bytes --]

Hi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.4-rc5 next-20191031]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/richard-gong-linux-intel-com/firmware-be-compatible-with-older-version-of-RSU-firmware/20191102-064734
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 31408fbe33d1d5e6209fa89fa5b45459197b8970
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/of_platform.h:9:0,
                    from drivers/firmware/stratix10-rsu.c:13:
   drivers/firmware/stratix10-rsu.c: In function 'rsu_command_callback':
>> drivers/firmware/stratix10-rsu.c:114:24: warning: format '%i' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Wformat=]
      dev_err(client->dev, "Failure, returned status is %i\n",
                           ^
   include/linux/device.h:1658:22: note: in definition of macro 'dev_fmt'
    #define dev_fmt(fmt) fmt
                         ^~~
>> drivers/firmware/stratix10-rsu.c:114:3: note: in expansion of macro 'dev_err'
      dev_err(client->dev, "Failure, returned status is %i\n",
      ^~~~~~~
   drivers/firmware/stratix10-rsu.c: In function 'rsu_retry_callback':
   drivers/firmware/stratix10-rsu.c:141:24: warning: format '%i' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Wformat=]
      dev_err(client->dev, "Failed to get retry counter %i\n",
                           ^
   include/linux/device.h:1658:22: note: in definition of macro 'dev_fmt'
    #define dev_fmt(fmt) fmt
                         ^~~
   drivers/firmware/stratix10-rsu.c:141:3: note: in expansion of macro 'dev_err'
      dev_err(client->dev, "Failed to get retry counter %i\n",
      ^~~~~~~

vim +114 drivers/firmware/stratix10-rsu.c

  > 13	#include <linux/of_platform.h>
    14	#include <linux/platform_device.h>
    15	#include <linux/firmware/intel/stratix10-svc-client.h>
    16	#include <linux/string.h>
    17	#include <linux/sysfs.h>
    18	
    19	#define RSU_STATE_MASK			GENMASK_ULL(31, 0)
    20	#define RSU_VERSION_MASK		GENMASK_ULL(63, 32)
    21	#define RSU_ERROR_LOCATION_MASK		GENMASK_ULL(31, 0)
    22	#define RSU_ERROR_DETAIL_MASK		GENMASK_ULL(63, 32)
    23	
    24	#define RSU_TIMEOUT	(msecs_to_jiffies(SVC_RSU_REQUEST_TIMEOUT_MS))
    25	
    26	#define INVALID_RETRY_COUNTER		0xFFFFFFFF
    27	
    28	typedef void (*rsu_callback)(struct stratix10_svc_client *client,
    29				     struct stratix10_svc_cb_data *data);
    30	/**
    31	 * struct stratix10_rsu_priv - rsu data structure
    32	 * @chan: pointer to the allocated service channel
    33	 * @client: active service client
    34	 * @completion: state for callback completion
    35	 * @lock: a mutex to protect callback completion state
    36	 * @status.current_image: address of image currently running in flash
    37	 * @status.fail_image: address of failed image in flash
    38	 * @status.version: the version number of RSU firmware
    39	 * @status.state: the state of RSU system
    40	 * @status.error_details: error code
    41	 * @status.error_location: the error offset inside the image that failed
    42	 * @retry_counter: the current image's retry counter
    43	 */
    44	struct stratix10_rsu_priv {
    45		struct stratix10_svc_chan *chan;
    46		struct stratix10_svc_client client;
    47		struct completion completion;
    48		struct mutex lock;
    49		struct {
    50			unsigned long current_image;
    51			unsigned long fail_image;
    52			unsigned int version;
    53			unsigned int state;
    54			unsigned int error_details;
    55			unsigned int error_location;
    56		} status;
    57		unsigned int retry_counter;
    58	};
    59	
    60	/**
    61	 * rsu_status_callback() - Status callback from Intel Service Layer
    62	 * @client: pointer to service client
    63	 * @data: pointer to callback data structure
    64	 *
    65	 * Callback from Intel service layer for RSU status request. Status is
    66	 * only updated after a system reboot, so a get updated status call is
    67	 * made during driver probe.
    68	 */
    69	static void rsu_status_callback(struct stratix10_svc_client *client,
    70					struct stratix10_svc_cb_data *data)
    71	{
    72		struct stratix10_rsu_priv *priv = client->priv;
    73		struct arm_smccc_res *res = (struct arm_smccc_res *)data->kaddr1;
    74	
    75		if (data->status == BIT(SVC_STATUS_RSU_OK)) {
    76			priv->status.version = FIELD_GET(RSU_VERSION_MASK,
    77							 res->a2);
    78			priv->status.state = FIELD_GET(RSU_STATE_MASK, res->a2);
    79			priv->status.fail_image = res->a1;
    80			priv->status.current_image = res->a0;
    81			priv->status.error_location =
    82				FIELD_GET(RSU_ERROR_LOCATION_MASK, res->a3);
    83			priv->status.error_details =
    84				FIELD_GET(RSU_ERROR_DETAIL_MASK, res->a3);
    85		} else {
    86			dev_err(client->dev, "COMMAND_RSU_STATUS returned 0x%lX\n",
    87				res->a0);
    88			priv->status.version = 0;
    89			priv->status.state = 0;
    90			priv->status.fail_image = 0;
    91			priv->status.current_image = 0;
    92			priv->status.error_location = 0;
    93			priv->status.error_details = 0;
    94		}
    95	
    96		complete(&priv->completion);
    97	}
    98	
    99	/**
   100	 * rsu_command_callback() - Update callback from Intel Service Layer
   101	 * @client: pointer to client
   102	 * @data: pointer to callback data structure
   103	 *
   104	 * Callback from Intel service layer for RSU commands.
   105	 */
   106	static void rsu_command_callback(struct stratix10_svc_client *client,
   107					 struct stratix10_svc_cb_data *data)
   108	{
   109		struct stratix10_rsu_priv *priv = client->priv;
   110	
   111		if (data->status == BIT(SVC_STATUS_RSU_NO_SUPPORT))
   112			dev_warn(client->dev, "Secure FW doesn't support notify\n");
   113		else if (data->status == BIT(SVC_STATUS_RSU_ERROR))
 > 114			dev_err(client->dev, "Failure, returned status is %i\n",
   115				BIT(data->status));
   116	
   117		complete(&priv->completion);
   118	}
   119	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67231 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-11-02 11:17 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-30 20:44 [PATCHv1] firmware: be compatible with older version of RSU firmware richard.gong
2019-11-02 11:16 ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).