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

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

[Dinh Nguyen]

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

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.

Acked-by: Moritz Fischr <mdf@kernel.org>
Signed-off-by: Richard Gong <richard.gong@intel.com>
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 drivers/firmware/stratix10-svc.c              |  9 +++++++-
 include/linux/firmware/intel/stratix10-smc.h  | 21 +++++++++++++++++--
 .../firmware/intel/stratix10-svc-client.h     |  4 ++++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index 29c0a616b317..4bd57a908efe 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;
@@ -491,7 +497,8 @@ static int svc_normal_to_secure_thread(void *data)
 			 */
 			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 c3e5ab014caf..aad497a9ad8b 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 19781b0f6429..18c1841fdb1f 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.25.1

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

[Xu Yilun]

On Tue, Jan 25, 2022 at 01:28:53PM -0600, Dinh Nguyen wrote:
> From: Richard Gong <richard.gong@intel.com>
> 
> 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.
> 
> Acked-by: Moritz Fischr <mdf@kernel.org>

This is already acked and no more change, is it? So please just ping in
the original mail or add the [RESEND PATCH] prefix.

The patch is good to me.

Acked-by: Xu Yilun <yilun.xu@intel.com>

> Signed-off-by: Richard Gong <richard.gong@intel.com>
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
>  drivers/firmware/stratix10-svc.c              |  9 +++++++-
>  include/linux/firmware/intel/stratix10-smc.h  | 21 +++++++++++++++++--
>  .../firmware/intel/stratix10-svc-client.h     |  4 ++++
>  3 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
> index 29c0a616b317..4bd57a908efe 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;
> @@ -491,7 +497,8 @@ static int svc_normal_to_secure_thread(void *data)
>  			 */
>  			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 c3e5ab014caf..aad497a9ad8b 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 19781b0f6429..18c1841fdb1f 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.25.1

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

[Dinh Nguyen]

On 1/25/22 19:07, Xu Yilun wrote:
> On Tue, Jan 25, 2022 at 01:28:53PM -0600, Dinh Nguyen wrote:
>> From: Richard Gong <richard.gong@intel.com>
>>
>> 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.
>>
>> Acked-by: Moritz Fischr <mdf@kernel.org>
> 
> This is already acked and no more change, is it? So please just ping in
> the original mail or add the [RESEND PATCH] prefix.
> 
> The patch is good to me.
> 
> Acked-by: Xu Yilun <yilun.xu@intel.com>
> 

Thanks, I added Moritz Acked-by: is the only change.

Dinh

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

[Moritz Fischer]

Dinh,

On Tue, Jan 25, 2022 at 10:31:06PM -0600, Dinh Nguyen wrote:
> 
> 
> On 1/25/22 19:07, Xu Yilun wrote:
> > On Tue, Jan 25, 2022 at 01:28:53PM -0600, Dinh Nguyen wrote:
> > > From: Richard Gong <richard.gong@intel.com>
> > > 
> > > 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.
> > > 
> > > Acked-by: Moritz Fischr <mdf@kernel.org>
> > 
> > This is already acked and no more change, is it? So please just ping in
> > the original mail or add the [RESEND PATCH] prefix.
> > 
> > The patch is good to me.
> > 
> > Acked-by: Xu Yilun <yilun.xu@intel.com>
> > 
> 
> Thanks, I added Moritz Acked-by: is the only change.
> 
> Dinh

Did we traditionally take those through FPGA tree, did Greg pick those
up?

Cheers,
Moritz

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

[Dinh Nguyen]

Hi Moritz,

On 1/25/22 22:54, Moritz Fischer wrote:
> Dinh,
> 
> On Tue, Jan 25, 2022 at 10:31:06PM -0600, Dinh Nguyen wrote:
>>
>>
>> On 1/25/22 19:07, Xu Yilun wrote:
>>> On Tue, Jan 25, 2022 at 01:28:53PM -0600, Dinh Nguyen wrote:
>>>> From: Richard Gong <richard.gong@intel.com>
>>>>
>>>> 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.
>>>>
>>>> Acked-by: Moritz Fischr <mdf@kernel.org>
>>>
>>> This is already acked and no more change, is it? So please just ping in
>>> the original mail or add the [RESEND PATCH] prefix.
>>>
>>> The patch is good to me.
>>>
>>> Acked-by: Xu Yilun <yilun.xu@intel.com>
>>>
>>
>> Thanks, I added Moritz Acked-by: is the only change.
>>
>> Dinh
> 
> Did we traditionally take those through FPGA tree, did Greg pick those
> up?
> 

I don't know for sure.

Dinh

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

[Xu Yilun]

On Tue, Jan 25, 2022 at 08:54:48PM -0800, Moritz Fischer wrote:
> Dinh,
> 
> On Tue, Jan 25, 2022 at 10:31:06PM -0600, Dinh Nguyen wrote:
> > 
> > 
> > On 1/25/22 19:07, Xu Yilun wrote:
> > > On Tue, Jan 25, 2022 at 01:28:53PM -0600, Dinh Nguyen wrote:
> > > > From: Richard Gong <richard.gong@intel.com>
> > > > 
> > > > 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.
> > > > 
> > > > Acked-by: Moritz Fischr <mdf@kernel.org>
> > > 
> > > This is already acked and no more change, is it? So please just ping in
> > > the original mail or add the [RESEND PATCH] prefix.
> > > 
> > > The patch is good to me.
> > > 
> > > Acked-by: Xu Yilun <yilun.xu@intel.com>
> > > 
> > 
> > Thanks, I added Moritz Acked-by: is the only change.
> > 
> > Dinh
> 
> Did we traditionally take those through FPGA tree, did Greg pick those
> up?

This patch was the 3rd one of a patchset. It is acked but the patchset
is not, so we didn't take the single one in FPGA tree. This is the last
conversation.

https://lore.kernel.org/linux-fpga/YGDsl3VlIAnyJDzd@epycbox.lan/

Thanks,
Yilun

> 
> Cheers,
> Moritz

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

[Dinh Nguyen]

On 1/26/22 18:39, Xu Yilun wrote:
> On Tue, Jan 25, 2022 at 08:54:48PM -0800, Moritz Fischer wrote:
>> Dinh,
>>
>> On Tue, Jan 25, 2022 at 10:31:06PM -0600, Dinh Nguyen wrote:
>>>
>>>
>>> On 1/25/22 19:07, Xu Yilun wrote:
>>>> On Tue, Jan 25, 2022 at 01:28:53PM -0600, Dinh Nguyen wrote:
>>>>> From: Richard Gong <richard.gong@intel.com>
>>>>>
>>>>> 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.
>>>>>
>>>>> Acked-by: Moritz Fischr <mdf@kernel.org>
>>>>
>>>> This is already acked and no more change, is it? So please just ping in
>>>> the original mail or add the [RESEND PATCH] prefix.
>>>>
>>>> The patch is good to me.
>>>>
>>>> Acked-by: Xu Yilun <yilun.xu@intel.com>
>>>>
>>>
>>> Thanks, I added Moritz Acked-by: is the only change.
>>>
>>> Dinh
>>
>> Did we traditionally take those through FPGA tree, did Greg pick those
>> up?
> 
> This patch was the 3rd one of a patchset. It is acked but the patchset
> is not, so we didn't take the single one in FPGA tree. This is the last
> conversation.
> 
> https://lore.kernel.org/linux-fpga/YGDsl3VlIAnyJDzd@epycbox.lan/
> 

Right, Richard has left Intel. So what I've done is gone back and try to 
re-send patches that have been Acked, like this one, and split out the 
other patches to a smaller series to restart the process.

Dinh

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

[Dinh Nguyen]

On 1/25/22 22:54, Moritz Fischer wrote:
> Dinh,
> 
> On Tue, Jan 25, 2022 at 10:31:06PM -0600, Dinh Nguyen wrote:
>>
>>
>> On 1/25/22 19:07, Xu Yilun wrote:
>>> On Tue, Jan 25, 2022 at 01:28:53PM -0600, Dinh Nguyen wrote:
>>>> From: Richard Gong <richard.gong@intel.com>
>>>>
>>>> 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.
>>>>
>>>> Acked-by: Moritz Fischr <mdf@kernel.org>
>>>
>>> This is already acked and no more change, is it? So please just ping in
>>> the original mail or add the [RESEND PATCH] prefix.
>>>
>>> The patch is good to me.
>>>
>>> Acked-by: Xu Yilun <yilun.xu@intel.com>
>>>
>>
>> Thanks, I added Moritz Acked-by: is the only change.
>>
>> Dinh
> 
> Did we traditionally take those through FPGA tree, did Greg pick those
> up?
> 


I'll take these patches through my repo and will send it up to Greg.

Dinh

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

[Richard Gong]

Hi Moritz,

On 3/30/21 11:15 AM, Moritz Fischer wrote:
> Hi Richard,
> 
> On Tue, Mar 30, 2021 at 09:33:05AM -0500, richard.gong@linux.intel.com wrote:
>> From: Richard Gong <richard.gong@intel.com>
>>
>> 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 <richard.gong@intel.com>
>> Acked-by: Moritz Fischr <mdf@kernel.org>
>> ---
>>   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
> 

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

[Moritz Fischer]

Hi Richard,

On Tue, Mar 30, 2021 at 09:33:05AM -0500, richard.gong@linux.intel.com wrote:
> From: Richard Gong <richard.gong@intel.com>
> 
> 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 <richard.gong@intel.com>
> Acked-by: Moritz Fischr <mdf@kernel.org>
> ---
>  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

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

[Richard Gong]

Hi David,

On 3/30/21 9:19 AM, David Laight wrote:
> From: richard.gong@linux.intel.com
>> 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)
> 

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

[richard.gong]

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

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 <richard.gong@intel.com>
Acked-by: Moritz Fischr <mdf@kernel.org>
---
 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

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

[David Laight]

From: richard.gong@linux.intel.com
> 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)