2021-03-30 14:15:40

by Richard Gong

[permalink] [raw]
Subject: [PATCH] firmware: stratix10-svc: extend SVC driver to get the firmware version

From: Richard Gong <[email protected]>

Extend Intel service layer driver to get the firmware version running at
FPGA device. Therefore FPGA manager driver, one of Intel service layer
driver's client, can decide whether to handle the newly added bitstream
authentication function based on the retrieved firmware version.

Signed-off-by: Richard Gong <[email protected]>
Acked-by: Moritz Fischr <[email protected]>
---
drivers/firmware/stratix10-svc.c | 12 ++++++++++--
include/linux/firmware/intel/stratix10-smc.h | 21 +++++++++++++++++++--
include/linux/firmware/intel/stratix10-svc-client.h | 4 ++++
3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index 3aa489d..1443bbd 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -306,6 +306,7 @@ static void svc_thread_recv_status_ok(struct stratix10_svc_data *p_data,
break;
case COMMAND_RSU_RETRY:
case COMMAND_RSU_MAX_RETRY:
+ case COMMAND_FIRMWARE_VERSION:
cb_data->status = BIT(SVC_STATUS_OK);
cb_data->kaddr1 = &res.a1;
break;
@@ -422,6 +423,11 @@ static int svc_normal_to_secure_thread(void *data)
a1 = 0;
a2 = 0;
break;
+ case COMMAND_FIRMWARE_VERSION:
+ a0 = INTEL_SIP_SMC_FIRMWARE_VERSION;
+ a1 = 0;
+ a2 = 0;
+ break;
default:
pr_warn("it shouldn't happen\n");
break;
@@ -487,11 +493,13 @@ static int svc_normal_to_secure_thread(void *data)

/*
* be compatible with older version firmware which
- * doesn't support RSU notify or retry
+ * doesn't support RSU notify, retry or bitstream
+ * authentication.
*/
if ((pdata->command == COMMAND_RSU_RETRY) ||
(pdata->command == COMMAND_RSU_MAX_RETRY) ||
- (pdata->command == COMMAND_RSU_NOTIFY)) {
+ (pdata->command == COMMAND_RSU_NOTIFY) ||
+ (pdata->command == COMMAND_FIRMWARE_VERSION)) {
cbdata->status =
BIT(SVC_STATUS_NO_SUPPORT);
cbdata->kaddr1 = NULL;
diff --git a/include/linux/firmware/intel/stratix10-smc.h b/include/linux/firmware/intel/stratix10-smc.h
index c3e5ab0..505fcca 100644
--- a/include/linux/firmware/intel/stratix10-smc.h
+++ b/include/linux/firmware/intel/stratix10-smc.h
@@ -321,8 +321,6 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
#define INTEL_SIP_SMC_ECC_DBE \
INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_ECC_DBE)

-#endif
-
/**
* Request INTEL_SIP_SMC_RSU_NOTIFY
*
@@ -404,3 +402,22 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
#define INTEL_SIP_SMC_FUNCID_RSU_MAX_RETRY 18
#define INTEL_SIP_SMC_RSU_MAX_RETRY \
INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_RSU_MAX_RETRY)
+
+/**
+ * Request INTEL_SIP_SMC_FIRMWARE_VERSION
+ *
+ * Sync call used to query the version of running firmware
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_FIRMWARE_VERSION
+ * a1-a7 not used
+ *
+ * Return status:
+ * a0 INTEL_SIP_SMC_STATUS_OK or INTEL_SIP_SMC_STATUS_ERROR
+ * a1 running firmware version
+ */
+#define INTEL_SIP_SMC_FUNCID_FIRMWARE_VERSION 31
+#define INTEL_SIP_SMC_FIRMWARE_VERSION \
+ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FIRMWARE_VERSION)
+
+#endif
diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h
index 19781b0f..18c1841 100644
--- a/include/linux/firmware/intel/stratix10-svc-client.h
+++ b/include/linux/firmware/intel/stratix10-svc-client.h
@@ -104,6 +104,9 @@ struct stratix10_svc_chan;
*
* @COMMAND_RSU_DCMF_VERSION: query firmware for the DCMF version, return status
* is SVC_STATUS_OK or SVC_STATUS_ERROR
+ *
+ * @COMMAND_FIRMWARE_VERSION: query running firmware version, return status
+ * is SVC_STATUS_OK or SVC_STATUS_ERROR
*/
enum stratix10_svc_command_code {
COMMAND_NOOP = 0,
@@ -117,6 +120,7 @@ enum stratix10_svc_command_code {
COMMAND_RSU_RETRY,
COMMAND_RSU_MAX_RETRY,
COMMAND_RSU_DCMF_VERSION,
+ COMMAND_FIRMWARE_VERSION,
};

/**
--
2.7.4


2021-03-30 14:23:33

by David Laight

[permalink] [raw]
Subject: RE: [PATCH] firmware: stratix10-svc: extend SVC driver to get the firmware version

From: [email protected]
> Sent: 30 March 2021 15:33
>
> Extend Intel service layer driver to get the firmware version running at
> FPGA device. Therefore FPGA manager driver, one of Intel service layer
> driver's client, can decide whether to handle the newly added bitstream
> authentication function based on the retrieved firmware version.

Using the version number to detect features is just plain wrong.

You should use something like a bitmap of supported features.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

2021-03-30 15:46:57

by Richard Gong

[permalink] [raw]
Subject: Re: [PATCH] firmware: stratix10-svc: extend SVC driver to get the firmware version


Hi David,

On 3/30/21 9:19 AM, David Laight wrote:
> From: [email protected]
>> Sent: 30 March 2021 15:33
>>
>> Extend Intel service layer driver to get the firmware version running at
>> FPGA device. Therefore FPGA manager driver, one of Intel service layer
>> driver's client, can decide whether to handle the newly added bitstream
>> authentication function based on the retrieved firmware version.
>
> Using the version number to detect features is just plain wrong.
>
> You should use something like a bitmap of supported features.

Firmware, which runs at EL3, returns the version to Intel service layer
driver in a 64-bit value at the register. Intel service layer driver
runs at EL1.

Regards,
Richard

>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

2021-03-30 16:19:43

by Moritz Fischer

[permalink] [raw]
Subject: Re: [PATCH] firmware: stratix10-svc: extend SVC driver to get the firmware version

Hi Richard,

On Tue, Mar 30, 2021 at 09:33:05AM -0500, [email protected] wrote:
> From: Richard Gong <[email protected]>
>
> Extend Intel service layer driver to get the firmware version running at
> FPGA device. Therefore FPGA manager driver, one of Intel service layer
> driver's client, can decide whether to handle the newly added bitstream
> authentication function based on the retrieved firmware version.
>
> Signed-off-by: Richard Gong <[email protected]>
> Acked-by: Moritz Fischr <[email protected]>
> ---
> drivers/firmware/stratix10-svc.c | 12 ++++++++++--
> include/linux/firmware/intel/stratix10-smc.h | 21 +++++++++++++++++++--
> include/linux/firmware/intel/stratix10-svc-client.h | 4 ++++
> 3 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
> index 3aa489d..1443bbd 100644
> --- a/drivers/firmware/stratix10-svc.c
> +++ b/drivers/firmware/stratix10-svc.c
> @@ -306,6 +306,7 @@ static void svc_thread_recv_status_ok(struct stratix10_svc_data *p_data,
> break;
> case COMMAND_RSU_RETRY:
> case COMMAND_RSU_MAX_RETRY:
> + case COMMAND_FIRMWARE_VERSION:
> cb_data->status = BIT(SVC_STATUS_OK);
> cb_data->kaddr1 = &res.a1;
> break;
> @@ -422,6 +423,11 @@ static int svc_normal_to_secure_thread(void *data)
> a1 = 0;
> a2 = 0;
> break;
> + case COMMAND_FIRMWARE_VERSION:
> + a0 = INTEL_SIP_SMC_FIRMWARE_VERSION;
> + a1 = 0;
> + a2 = 0;
> + break;
> default:
> pr_warn("it shouldn't happen\n");
> break;
> @@ -487,11 +493,13 @@ static int svc_normal_to_secure_thread(void *data)
>
> /*
> * be compatible with older version firmware which
> - * doesn't support RSU notify or retry
> + * doesn't support RSU notify, retry or bitstream
> + * authentication.
> */
> if ((pdata->command == COMMAND_RSU_RETRY) ||
> (pdata->command == COMMAND_RSU_MAX_RETRY) ||
> - (pdata->command == COMMAND_RSU_NOTIFY)) {
> + (pdata->command == COMMAND_RSU_NOTIFY) ||
> + (pdata->command == COMMAND_FIRMWARE_VERSION)) {
> cbdata->status =
> BIT(SVC_STATUS_NO_SUPPORT);
> cbdata->kaddr1 = NULL;
> diff --git a/include/linux/firmware/intel/stratix10-smc.h b/include/linux/firmware/intel/stratix10-smc.h
> index c3e5ab0..505fcca 100644
> --- a/include/linux/firmware/intel/stratix10-smc.h
> +++ b/include/linux/firmware/intel/stratix10-smc.h
> @@ -321,8 +321,6 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
> #define INTEL_SIP_SMC_ECC_DBE \
> INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_ECC_DBE)
>
> -#endif
> -
> /**
> * Request INTEL_SIP_SMC_RSU_NOTIFY
> *
> @@ -404,3 +402,22 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
> #define INTEL_SIP_SMC_FUNCID_RSU_MAX_RETRY 18
> #define INTEL_SIP_SMC_RSU_MAX_RETRY \
> INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_RSU_MAX_RETRY)
> +
> +/**
> + * Request INTEL_SIP_SMC_FIRMWARE_VERSION
> + *
> + * Sync call used to query the version of running firmware
> + *
> + * Call register usage:
> + * a0 INTEL_SIP_SMC_FIRMWARE_VERSION
> + * a1-a7 not used
> + *
> + * Return status:
> + * a0 INTEL_SIP_SMC_STATUS_OK or INTEL_SIP_SMC_STATUS_ERROR
> + * a1 running firmware version
> + */
> +#define INTEL_SIP_SMC_FUNCID_FIRMWARE_VERSION 31
> +#define INTEL_SIP_SMC_FIRMWARE_VERSION \
> + INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FIRMWARE_VERSION)
> +
> +#endif
> diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h
> index 19781b0f..18c1841 100644
> --- a/include/linux/firmware/intel/stratix10-svc-client.h
> +++ b/include/linux/firmware/intel/stratix10-svc-client.h
> @@ -104,6 +104,9 @@ struct stratix10_svc_chan;
> *
> * @COMMAND_RSU_DCMF_VERSION: query firmware for the DCMF version, return status
> * is SVC_STATUS_OK or SVC_STATUS_ERROR
> + *
> + * @COMMAND_FIRMWARE_VERSION: query running firmware version, return status
> + * is SVC_STATUS_OK or SVC_STATUS_ERROR
> */
> enum stratix10_svc_command_code {
> COMMAND_NOOP = 0,
> @@ -117,6 +120,7 @@ enum stratix10_svc_command_code {
> COMMAND_RSU_RETRY,
> COMMAND_RSU_MAX_RETRY,
> COMMAND_RSU_DCMF_VERSION,
> + COMMAND_FIRMWARE_VERSION,
> };
>
> /**
> --
> 2.7.4
>

Let's hold off on this patch until we have sorted the rest of this patch
series out. As it stands it doesn't have a in-tree user.

Thanks,
Moritz

2021-03-30 17:18:46

by Richard Gong

[permalink] [raw]
Subject: Re: [PATCH] firmware: stratix10-svc: extend SVC driver to get the firmware version


Hi Moritz,

On 3/30/21 11:15 AM, Moritz Fischer wrote:
> Hi Richard,
>
> On Tue, Mar 30, 2021 at 09:33:05AM -0500, [email protected] wrote:
>> From: Richard Gong <[email protected]>
>>
>> Extend Intel service layer driver to get the firmware version running at
>> FPGA device. Therefore FPGA manager driver, one of Intel service layer
>> driver's client, can decide whether to handle the newly added bitstream
>> authentication function based on the retrieved firmware version.
>>
>> Signed-off-by: Richard Gong <[email protected]>
>> Acked-by: Moritz Fischr <[email protected]>
>> ---
>> drivers/firmware/stratix10-svc.c | 12 ++++++++++--
>> include/linux/firmware/intel/stratix10-smc.h | 21 +++++++++++++++++++--
>> include/linux/firmware/intel/stratix10-svc-client.h | 4 ++++
>> 3 files changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
>> index 3aa489d..1443bbd 100644
>> --- a/drivers/firmware/stratix10-svc.c
>> +++ b/drivers/firmware/stratix10-svc.c
>> @@ -306,6 +306,7 @@ static void svc_thread_recv_status_ok(struct stratix10_svc_data *p_data,
>> break;
>> case COMMAND_RSU_RETRY:
>> case COMMAND_RSU_MAX_RETRY:
>> + case COMMAND_FIRMWARE_VERSION:
>> cb_data->status = BIT(SVC_STATUS_OK);
>> cb_data->kaddr1 = &res.a1;
>> break;
>> @@ -422,6 +423,11 @@ static int svc_normal_to_secure_thread(void *data)
>> a1 = 0;
>> a2 = 0;
>> break;
>> + case COMMAND_FIRMWARE_VERSION:
>> + a0 = INTEL_SIP_SMC_FIRMWARE_VERSION;
>> + a1 = 0;
>> + a2 = 0;
>> + break;
>> default:
>> pr_warn("it shouldn't happen\n");
>> break;
>> @@ -487,11 +493,13 @@ static int svc_normal_to_secure_thread(void *data)
>>
>> /*
>> * be compatible with older version firmware which
>> - * doesn't support RSU notify or retry
>> + * doesn't support RSU notify, retry or bitstream
>> + * authentication.
>> */
>> if ((pdata->command == COMMAND_RSU_RETRY) ||
>> (pdata->command == COMMAND_RSU_MAX_RETRY) ||
>> - (pdata->command == COMMAND_RSU_NOTIFY)) {
>> + (pdata->command == COMMAND_RSU_NOTIFY) ||
>> + (pdata->command == COMMAND_FIRMWARE_VERSION)) {
>> cbdata->status =
>> BIT(SVC_STATUS_NO_SUPPORT);
>> cbdata->kaddr1 = NULL;
>> diff --git a/include/linux/firmware/intel/stratix10-smc.h b/include/linux/firmware/intel/stratix10-smc.h
>> index c3e5ab0..505fcca 100644
>> --- a/include/linux/firmware/intel/stratix10-smc.h
>> +++ b/include/linux/firmware/intel/stratix10-smc.h
>> @@ -321,8 +321,6 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
>> #define INTEL_SIP_SMC_ECC_DBE \
>> INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_ECC_DBE)
>>
>> -#endif
>> -
>> /**
>> * Request INTEL_SIP_SMC_RSU_NOTIFY
>> *
>> @@ -404,3 +402,22 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
>> #define INTEL_SIP_SMC_FUNCID_RSU_MAX_RETRY 18
>> #define INTEL_SIP_SMC_RSU_MAX_RETRY \
>> INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_RSU_MAX_RETRY)
>> +
>> +/**
>> + * Request INTEL_SIP_SMC_FIRMWARE_VERSION
>> + *
>> + * Sync call used to query the version of running firmware
>> + *
>> + * Call register usage:
>> + * a0 INTEL_SIP_SMC_FIRMWARE_VERSION
>> + * a1-a7 not used
>> + *
>> + * Return status:
>> + * a0 INTEL_SIP_SMC_STATUS_OK or INTEL_SIP_SMC_STATUS_ERROR
>> + * a1 running firmware version
>> + */
>> +#define INTEL_SIP_SMC_FUNCID_FIRMWARE_VERSION 31
>> +#define INTEL_SIP_SMC_FIRMWARE_VERSION \
>> + INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FIRMWARE_VERSION)
>> +
>> +#endif
>> diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h
>> index 19781b0f..18c1841 100644
>> --- a/include/linux/firmware/intel/stratix10-svc-client.h
>> +++ b/include/linux/firmware/intel/stratix10-svc-client.h
>> @@ -104,6 +104,9 @@ struct stratix10_svc_chan;
>> *
>> * @COMMAND_RSU_DCMF_VERSION: query firmware for the DCMF version, return status
>> * is SVC_STATUS_OK or SVC_STATUS_ERROR
>> + *
>> + * @COMMAND_FIRMWARE_VERSION: query running firmware version, return status
>> + * is SVC_STATUS_OK or SVC_STATUS_ERROR
>> */
>> enum stratix10_svc_command_code {
>> COMMAND_NOOP = 0,
>> @@ -117,6 +120,7 @@ enum stratix10_svc_command_code {
>> COMMAND_RSU_RETRY,
>> COMMAND_RSU_MAX_RETRY,
>> COMMAND_RSU_DCMF_VERSION,
>> + COMMAND_FIRMWARE_VERSION,
>> };
>>
>> /**
>> --
>> 2.7.4
>>
>
> Let's hold off on this patch until we have sorted the rest of this patch
> series out. As it stands it doesn't have a in-tree user.
> OK,

Regards,
Richard

> Thanks,
> Moritz
>