2019-10-30 23:13:42

by Richard Gong

[permalink] [raw]
Subject: [PATCHv1] firmware: be compatible with older version of RSU firmware

From: Richard Gong <[email protected]>

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 <[email protected]>
Signed-off-by: Richard Gong <[email protected]>
---
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


2019-11-02 11:18:37

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCHv1] firmware: be compatible with older version of RSU firmware

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 <[email protected]>

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


Attachments:
(No filename) (6.95 kB)
.config.gz (65.66 kB)
Download all attachments