Re: [PATCH v5 2/3] firmware: stratix10-svc: add FCS crypto-service commands for Agilex 5

[Dinh Nguyen]

Hi Hang Suan,

On 9/15/26 03:25, hang.suan.wang@xxxxxxxxxx wrote:
> From: Hang Suan Wang <hang.suan.wang@xxxxxxxxxx>
>
> The Agilex 5 Secure Device Manager (SDM 1.5) exposes an FPGA Crypto
> Service (FCS) over the existing SIP SMC mailbox: a session-based
> interface for crypto primitives such as SDOS (Secure Data Object
> Service) encrypt/decrypt. The service layer has no command to drive it
> yet.
>
> Configure stratix10-svc about this interface so an in-kernel FCS client
> can use it:
>
>    - add the client command codes COMMAND_FCS_CRYPTO_OPEN_SESSION,
>      COMMAND_FCS_CRYPTO_CLOSE_SESSION and COMMAND_FCS_SDOS_DATA_EXT (all
>      asynchronous)
>
>    - add the matching asynchronous SIP SMC function IDs
>      (INTEL_SIP_SMC_ASYNC_FCS_OPEN_CS_SESSION,
>      INTEL_SIP_SMC_ASYNC_FCS_CLOSE_CS_SESSION and
>      INTEL_SIP_SMC_ASYNC_FCS_CRYPTION_EXT) with their register-usage
>      documentation;
>
>    - match "intel,agilex5-svc" and register a "stratix10-fcs" child
>      platform device, mirroring the existing RSU child, so an FCS client
>      driver can bind without a dedicated device-tree node;
>
>    - dispatch the new commands in the asynchronous send and response
>      paths; for the SDOS data command, translate the source and
>      destination buffers (allocated from the service-layer gen_pool) to
>      physical addresses and pass them, together with the session/context
>      IDs and owner ID, to the SDM.
>
> The transport is unchanged: Agilex 5 reuses the SIP SMC calling
> convention and async mailbox ABI the driver already implements, so no
> new transport mechanism is required.
>
> The SDOS SMMU-remapped address slots currently carry the buffer
> physical addresses; SMMU remapping support is added in a follow-up
> series.
>
> This is a prerequisite for the SoCFPGA FCS driver, the first in-tree
> consumer of these commands.
>
> Signed-off-by: Hang Suan Wang <hang.suan.wang@xxxxxxxxxx>
> Reviewed-by: Dinh Nguyen <dinguyen@xxxxxxxxxx>
> ---
>   drivers/firmware/stratix10-svc.c              | 59 +++++++++++++++--
>   include/linux/firmware/intel/stratix10-smc.h  | 64 +++++++++++++++++++
>   .../firmware/intel/stratix10-svc-client.h     | 16 +++++
>   3 files changed, 134 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
> index 07345efeef0c..8ead4a3c4a1b 100644
> --- a/drivers/firmware/stratix10-svc.c
> +++ b/drivers/firmware/stratix10-svc.c
> @@ -46,6 +46,7 @@
>   
>   /* stratix10 service layer clients */
>   #define STRATIX10_RSU				"stratix10-rsu"
> +#define STRATIX10_FCS				"stratix10-fcs"
>   #define SOCFPGA_HWMON				"socfpga-hwmon"
>   
>   /* Maximum number of SDM client IDs. */
> @@ -106,10 +107,12 @@ struct stratix10_svc_chan;
>   /**
>    * struct stratix10_svc - svc private data
>    * @stratix10_svc_rsu: pointer to stratix10 RSU device
> + * @stratix10_svc_fcs: pointer to stratix10 FCS device
>    * @stratix10_svc_hwmon: pointer to stratix10 HWMON device
>    */
>   struct stratix10_svc {
>   	struct platform_device *stratix10_svc_rsu;
> +	struct platform_device *stratix10_svc_fcs;
>   	struct platform_device *stratix10_svc_hwmon;
>   };
>   
> @@ -1398,6 +1401,30 @@ int stratix10_svc_async_send(struct stratix10_svc_chan *chan, void *msg,
>   		STRATIX10_SIP_SMC_SET_TRANSACTIONID_X1(handle->transaction_id);
>   
>   	switch (p_msg->command) {
> +	case COMMAND_FCS_CRYPTO_OPEN_SESSION:
> +		args.a0 = INTEL_SIP_SMC_ASYNC_FCS_OPEN_CS_SESSION;
> +		break;
> +	case COMMAND_FCS_CRYPTO_CLOSE_SESSION:
> +		args.a0 = INTEL_SIP_SMC_ASYNC_FCS_CLOSE_CS_SESSION;
> +		args.a2 = p_msg->arg[0];
> +		break;
> +	case COMMAND_FCS_SDOS_DATA_EXT:
> +		args.a0 = INTEL_SIP_SMC_ASYNC_FCS_CRYPTION_EXT;
> +		args.a2 = p_msg->arg[0];
> +		args.a3 = p_msg->arg[1];
> +		args.a4 = p_msg->arg[2];
> +		/* payloads are allocated from the svc gen_pool; pass phys addr */
> +		args.a5 = gen_pool_virt_to_phys(ctrl->genpool,
> +						(unsigned long)p_msg->payload);
> +		args.a6 = p_msg->payload_length;
> +		args.a7 = gen_pool_virt_to_phys(ctrl->genpool,
> +						(unsigned long)p_msg->payload_output);

These calls to gen_pool_virt_to_phys() can fail.

<snip>

>    * Sync call to request temperature
> diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h
> index 9bb46c3cb0f8..ffc1ac7c9785 100644
> --- a/include/linux/firmware/intel/stratix10-svc-client.h
> +++ b/include/linux/firmware/intel/stratix10-svc-client.h
> @@ -7,6 +7,8 @@
>   #ifndef __STRATIX10_SVC_CLIENT_H
>   #define __STRATIX10_SVC_CLIENT_H
>   
> +#include <linux/types.h>

I don't think you need the above include.


Dinh