Re: [PATCH v1 2/2] firmware: socfpga-fcs: add Altera SoCFPGA FCS driver with SDOS

[Dinh Nguyen <dinguyen@kernel.org>]

On 7/1/26 02:39, hang.suan.wang@altera.com wrote:
> From: Hang Suan Wang <hang.suan.wang@altera.com>
> 
> Add the Altera SoCFPGA Crypto Service (FCS) driver, which exposes the
> Secure Data Object Service (SDOS) encrypt/decrypt operation and its crypto
> session lifecycle to non-secure host software.
> 
> The SDOS is the FCS feature that protects data at rest: the SDM encrypts
> and decrypts using a key derived from a device-unique SDOS root key plus an
> SDM-generated IV, so the host never handles raw key material or IVs. It
> only submits plaintext it already owns and receives authenticated
> ciphertext objects managed by the SDM. A primary use case is black key
> provisioning, where operational keys are installed without ever appearing
> in cleartext.
> 
> The driver is a standalone module and describes no hardware of its own.
> It binds by name to the "stratix10-fcs" platform device that the
> stratix10-svc driver registers in code, so no device-tree node is needed,
> and detects the SoC by matching the service-layer compatible. It exposes
> the following sysfs attributes. The SDOS requests are issued to the SDM
> through the stratix10-svc asynchronous SIP SMC. Source and destination
> buffers are taken from the service-layer memory pool so the SDM can reach
> them via physical or SMMU-remapped addresses.
> 
> For encryption the SDM returns a structured object (metadata, IV, HMAC,
> ciphertext); for decryption it validates the HMAC, recovers the parameters
> from the object header, and enforces a 64-bit owner ID so that only the
> creator of an object can decrypt it.
> 
> Signed-off-by: Hang Suan Wang <hang.suan.wang@altera.com>
> ---
>   MAINTAINERS                                |   8 +
>   drivers/firmware/Kconfig                   |  16 +
>   drivers/firmware/Makefile                  |   2 +
>   drivers/firmware/socfpga-fcs-core.c        | 589 +++++++++++++++++++++
>   drivers/firmware/socfpga-fcs.c             | 294 ++++++++++
>   include/linux/firmware/intel/socfpga-fcs.h | 134 +++++
>   6 files changed, 1043 insertions(+)
>   create mode 100644 drivers/firmware/socfpga-fcs-core.c
>   create mode 100644 drivers/firmware/socfpga-fcs.c
>   create mode 100644 include/linux/firmware/intel/socfpga-fcs.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 15011f5752a9..72b12b32f8fe 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -946,6 +946,14 @@ ALPS PS/2 TOUCHPAD DRIVER
>   R:	Pali Rohár <pali@kernel.org>
>   F:	drivers/input/mouse/alps.*
>   
> +ALTERA FCS DRIVER
> +M:	Hang Suan Wang <hang.suan.wang@altera.com>
> +M:	Genevieve Chan <genevieve.chan@altera.com>
> +L:	linux-arm-kernel@lists.infradead.org
> +S:	Maintained
> +F:	drivers/firmware/socfpga-fcs*
> +F:	include/linux/firmware/intel/socfpga-fcs*
> +
>   ALTERA MAILBOX DRIVER
>   M:	Tien Sung Ang <tiensung.ang@altera.com>
>   S:	Maintained
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index 12dc70254842..9a70def6932a 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -172,6 +172,22 @@ config INTEL_STRATIX10_RSU
>   
>   	  Say Y here if you want Intel RSU support.
>   
> +config ALTERA_SOCFPGA_FCS
> +	tristate "Altera SoCFPGA Crypto Service (FCS) configuration"
> +	depends on INTEL_STRATIX10_SERVICE
> +	default n
> +	help
> +	  Altera SoCFPGA Crypto Service (FCS) driver exposes interfaces access
> +	  through the Intel Service Layer to user space via sysfs device
> +	  attribute nodes. It exposes the crypto and key-management services
> +	  of the Secure Device Manager (SDM) to the host software stack and
> +	  requests are forwarded to Arm Trusted Firmware. The SDM then
> +	  executes or authorizes them using device-rooted security resources.
> +	  Protected key material remains within the secure firmware boundary
> +	  and is not directly exposed to non-secure host software.
> +
> +	  Say Y here if you want Altera SoCFPGA FCS support.
> +
>   config MTK_ADSP_IPC
>   	tristate "MTK ADSP IPC Protocol driver"
>   	depends on MTK_ADSP_MBOX
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> index 4ddec2820c96..e9f52f0e5f7a 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -10,6 +10,8 @@ obj-$(CONFIG_EDD)		+= edd.o
>   obj-$(CONFIG_DMIID)		+= dmi-id.o
>   obj-$(CONFIG_INTEL_STRATIX10_SERVICE) += stratix10-svc.o
>   obj-$(CONFIG_INTEL_STRATIX10_RSU)     += stratix10-rsu.o
> +obj-$(CONFIG_ALTERA_SOCFPGA_FCS) += altera-fcs.o
> +altera-fcs-y := socfpga-fcs.o socfpga-fcs-core.o
>   obj-$(CONFIG_ISCSI_IBFT_FIND)	+= iscsi_ibft_find.o
>   obj-$(CONFIG_ISCSI_IBFT)	+= iscsi_ibft.o
>   obj-$(CONFIG_FIRMWARE_MEMMAP)	+= memmap.o
> diff --git a/drivers/firmware/socfpga-fcs-core.c b/drivers/firmware/socfpga-fcs-core.c
> new file mode 100644
> index 000000000000..62460fd0e9e4
> --- /dev/null
> +++ b/drivers/firmware/socfpga-fcs-core.c
> @@ -0,0 +1,589 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2026 Altera Corporation
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +#include <linux/firmware/intel/socfpga-fcs.h>
> +#include <linux/firmware/intel/stratix10-svc-client.h>
> +
> +#define OWNER_ID_OFFSET				12
> +#define OWNER_ID_SIZE				8
> +
> +#define SDOS_DECRYPTION_REPROVISION_KEY_WARN	0x102
> +#define SDOS_DECRYPTION_NOT_LATEST_KEY_WARN	0x103
> +
> +#define MSG_RETRY				3
> +#define RETRY_SLEEP_MS				1
> +#define TIMEOUT					1000
> +
> +static struct socfpga_fcs_priv *priv;
> +
> +/**
> + * fcs_atf_version_callback() - service-layer callback for the ATF version query
> + * @client: pointer to the stratix10-svc client
> + * @data: pointer to the service-layer callback data
> + *
> + * Store the returned Arm Trusted Firmware version (or mailbox error) in @priv
> + * and signal completion to the waiting caller.
> + */
> +static void fcs_atf_version_callback(struct stratix10_svc_client *client,
> +				     struct stratix10_svc_cb_data *data)
> +{
> +	struct socfpga_fcs_priv *p = client->priv;
> +
> +	p->status = data->status;
> +	if (data->status == BIT(SVC_STATUS_OK)) {
> +		p->status = 0;
> +		p->atf_version[0] = *((unsigned int *)data->kaddr1);
> +		p->atf_version[1] = *((unsigned int *)data->kaddr2);
> +		p->atf_version[2] = *((unsigned int *)data->kaddr3);
> +	} else if (data->status == BIT(SVC_STATUS_ERROR)) {
> +		p->status = *((unsigned int *)data->kaddr1);
> +		dev_err(client->dev, "mbox_error=0x%x\n", p->status);
> +	}
> +
> +	complete(&p->completion);
> +}
> +
> +/**
> + * fcs_async_callback() - completion callback for an async service request
> + * @ptr: pointer to the completion to signal
> + */
> +static void fcs_async_callback(void *ptr)
> +{
> +	if (ptr)
> +		complete(ptr);
> +}
> +
> +/**
> + * fcs_svc_send_request() - build and send an FCS command to the service layer
> + * @command: FCS command code to dispatch
> + * @timeout: time to wait for completion, in jiffies
> + *
> + * Build the service-layer message for @command and send it through the
> + * stratix10-svc service driver, using the synchronous path for the ATF version
> + * query and the asynchronous mailbox path (with retries) for the remaining
> + * commands.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +static int fcs_svc_send_request(enum fcs_command_code command,
> +				unsigned long timeout)
> +{
> +	struct fcs_cmd_context *k_ctx = &priv->k_ctx;
> +	struct stratix10_svc_cb_data data;
> +	struct completion completion;
> +	void *handle = NULL;
> +	int status, index;
> +	int ret = 0;
> +	struct stratix10_svc_client_msg *msg = kzalloc(sizeof(*msg), GFP_KERNEL);
> +
> +	priv->status = 0;
> +	priv->resp = 0;
> +
> +	switch (command) {
> +	case FCS_DEV_CRYPTO_OPEN_SESSION:
> +		pr_debug("Sending command: COMMAND_FCS_CRYPTO_OPEN_SESSION\n");
> +		msg->command = COMMAND_FCS_CRYPTO_OPEN_SESSION;
> +		break;
> +
> +	case FCS_DEV_CRYPTO_CLOSE_SESSION:
> +		pr_debug("Sending command: COMMAND_FCS_CRYPTO_CLOSE_SESSION with session_id: 0x%x\n",
> +			 priv->session_id);
> +		msg->arg[0] = priv->session_id;
> +		msg->command = COMMAND_FCS_CRYPTO_CLOSE_SESSION;
> +		break;
> +
> +	case FCS_DEV_ATF_VERSION:
> +		pr_debug("Sending command: COMMAND_SMC_ATF_BUILD_VER\n");
> +		msg->command = COMMAND_SMC_ATF_BUILD_VER;
> +		priv->client.receive_cb = fcs_atf_version_callback;

You've set the receive_cb here for this case, what if calls to the other 
cases come in? Wouldn't it just go to fcs_atf_version_callback()?


> +		break;
> +
> +	case FCS_DEV_SDOS_DATA_EXT:
> +		pr_debug("Sending command: COMMAND_FCS_SDOS_DATA_EXT with session_id: 0x%x, context_id: 0x%x, op_mode: 0x%x, own: 0x%llx\n",
> +			 priv->session_id, k_ctx->sdos.context_id,
> +			 k_ctx->sdos.op_mode, k_ctx->sdos.own);
> +		msg->arg[0] = priv->session_id;
> +		msg->arg[1] = k_ctx->sdos.context_id;
> +		msg->arg[2] = k_ctx->sdos.op_mode;
> +		msg->arg[3] = k_ctx->sdos.own;
> +		msg->payload = k_ctx->sdos.src;
> +		msg->payload_length = k_ctx->sdos.src_size;
> +		msg->payload_output = k_ctx->sdos.dst;
> +		msg->payload_length_output = *k_ctx->sdos.dst_size;
> +		msg->command = COMMAND_FCS_SDOS_DATA_EXT;
> +		break;
> +
> +	default:
> +		pr_err("Unknown command: 0x%x\n", command);
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	if (ret) {
> +		kfree(msg);
> +		return ret;
> +	}
> +
> +	if (command == FCS_DEV_ATF_VERSION) {
> +		reinit_completion(&priv->completion);
> +
> +		ret = stratix10_svc_send(priv->chan, msg);
> +		if (ret) {
> +			pr_err("failed to send message to service channel\n");
> +			goto fun_ret;
> +		}
> +
> +		if (!wait_for_completion_timeout(&priv->completion, timeout)) {
> +			pr_err("svc timeout to get completed status\n");
> +			ret = -ETIMEDOUT;
> +		}
> +fun_ret:
> +		kfree(msg);
> +		return ret;
> +	}
> +
> +	init_completion(&completion);

You're mixing a local stack completion with a priv->completion. Doesn't 
this local declaration of completion get destroy when there's a 
timeout?And if fcs_async_callback() gets fired, your local completion is 
no longer valid.


> +
> +	for (index = 0; index < MSG_RETRY; index++) {
> +		status = stratix10_svc_async_send(priv->chan, msg, &handle,
> +						  fcs_async_callback,
> +						  &completion);
> +		if (status == 0)
> +			break;
> +		msleep(RETRY_SLEEP_MS);
> +	}
> +
> +	if (!handle || status != 0) {
> +		pr_err("Failed to send async message\n");
> +		kfree(msg);
> +		return -ETIMEDOUT;
> +	}
> +
> +	ret = wait_for_completion_io_timeout(&completion, (TIMEOUT));
> +	if (ret > 0)
> +		pr_debug("Received async interrupt\n");
> +	else
> +		pr_err("timeout occurred while waiting for async message\n");

So if a timeout happens,
> +
> +	ret = stratix10_svc_async_poll(priv->chan, handle, &data);

you fall straight into here. There's a reason why the timeout happened, 
this code ignoring that reason. Shouldn't you handle the timeout?

> +	if (ret) {
> +		pr_err("Failed to poll async message\n");
> +		goto out;
> +	}
> +
> +	priv->status = data.status;
> +
> +	if (data.kaddr1)
> +		priv->resp = *((u32 *)data.kaddr1);
> +	else
> +		priv->resp = 0;
> +
> +out:
> +	stratix10_svc_async_done(priv->chan, handle);
> +	kfree(msg);
> +
> +	return ret;
> +}
> +
> +/**
> + * fcs_session_open() - open an FCS crypto service session
> + * @k_ctx: pointer to the kernel-side FCS command context
> + *
> + * Request a new session from the SDM, generate the session UUID and copy it,
> + * together with the mailbox status, back to user space.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int fcs_session_open(struct fcs_cmd_context *const k_ctx)
> +{
> +	int ret = 0;
> +
> +	ret = fcs_svc_send_request(FCS_DEV_CRYPTO_OPEN_SESSION,
> +				   SVC_FCS_REQUEST_TIMEOUT_MS);
> +	if (ret) {
> +		pr_err("Failed to send the cmd=%d,ret=%d\n",
> +		       FCS_DEV_CRYPTO_OPEN_SESSION, ret);
> +		return ret;
> +	}
> +
> +	if (priv->status) {
> +		ret = -EIO;
> +		pr_err("Mailbox error, Failed to open session ret: %d\n", ret);
> +		goto copy_mbox_status;
> +	}
> +
> +	uuid_gen(&priv->uuid_id);
> +
> +	memcpy(&priv->session_id, &priv->resp, sizeof(priv->session_id));
> +
> +	ret = copy_to_user(k_ctx->open_session.suuid, &priv->uuid_id,
> +			   sizeof(uuid_t)) ? -EFAULT : 0;
> +	if (ret) {
> +		pr_err("Failed to copy session ID to user suuid addr: %p ret: %d\n",
> +		       k_ctx->open_session.suuid, ret);
> +		goto copy_mbox_status;

Don't need this goto.

> +	}
> +
> +copy_mbox_status:
> +	if (copy_to_user(k_ctx->error_code_addr, &priv->status,
> +			 sizeof(priv->status))) {
> +		pr_err("Failed to copy mail box status code to user\n");
> +		/* surface the copy failure only if nothing failed earlier */
> +		if (!ret)
> +			ret = -EFAULT;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
> + * fcs_session_close() - close an FCS crypto service session
> + * @k_ctx: pointer to the kernel-side FCS command context
> + *
> + * Validate the caller-supplied session UUID, ask the SDM to close the session
> + * and copy the mailbox status back to user space.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int fcs_session_close(struct fcs_cmd_context *const k_ctx)
> +{
> +	int ret = 0;
> +	struct fcs_cmd_context ctx;
> +
> +	memcpy(&ctx, k_ctx, sizeof(struct fcs_cmd_context));
> +
> +	if (!uuid_equal(&priv->uuid_id, &k_ctx->close_session.suuid)) {
> +		ret = -EINVAL;
> +		pr_err("Session UUID Mismatch ret: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = fcs_svc_send_request(FCS_DEV_CRYPTO_CLOSE_SESSION,
> +				   SVC_FCS_REQUEST_TIMEOUT_MS);
> +	if (ret) {
> +		pr_err("Failed to send the cmd=%d,ret=%d\n",
> +		       FCS_DEV_CRYPTO_CLOSE_SESSION, ret);
> +		return ret;
> +	}
> +
> +	memset(&priv->uuid_id, 0, sizeof(uuid_t));
> +	priv->session_id = 0;
> +	if (priv->status) {
> +		ret = -EIO;
> +		pr_err("Mailbox error, Failed to close session ret: %d\n", ret);
> +	}
> +
> +	if (copy_to_user(ctx.error_code_addr, &priv->status,
> +			 sizeof(priv->status))) {
> +		pr_err("Failed to copy mail box status code to user\n");
> +		/* surface the copy failure only if nothing failed earlier */
> +		if (!ret)
> +			ret = -EFAULT;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
> + * fcs_get_atf_version() - return the cached Arm Trusted Firmware version
> + * @version: array of three u32 entries to receive the major, minor and patch
> + *           version numbers
> + */
> +void fcs_get_atf_version(u32 *version)
> +{
> +	memcpy(version, priv->atf_version, sizeof(priv->atf_version));
> +}
> +
> +/**
> + * fcs_sdos_crypt() - perform an SDOS encrypt or decrypt operation
> + * @k_ctx: pointer to the kernel-side FCS command context
> + *
> + * Allocate service-layer source and destination buffers, copy the input from
> + * user space, drive the SDOS data command and copy the result and length back
> + * to user space. The operation direction is selected by @k_ctx->sdos.op_mode.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int fcs_sdos_crypt(struct fcs_cmd_context *const k_ctx)
> +{
> +	void *s_buf = NULL, *d_buf = NULL;
> +	struct fcs_cmd_context ctx;
> +	u32 output_size;
> +	u32 dst_cap;
> +	u64 owner_id;
> +	int ret = 0;
> +	char *temp;
> +
> +	memcpy(&ctx, k_ctx, sizeof(struct fcs_cmd_context));
> +
> +	if (!ctx.sdos.dst || !ctx.sdos.dst_size)
> +		return -EINVAL;
> +
> +	/* Caller-provided output buffer capacity (in/out parameter) */
> +	if (copy_from_user(&dst_cap, ctx.sdos.dst_size, sizeof(dst_cap)))
> +		return -EFAULT;
> +
> +	if (ctx.sdos.op_mode) {
> +		output_size = SDOS_ENCRYPTED_MAX_SZ;
> +		/* encrypt: input is header + plaintext */
> +		if (k_ctx->sdos.src_size < SDOS_DECRYPTED_MIN_SZ ||
> +		    k_ctx->sdos.src_size > SDOS_DECRYPTED_MAX_SZ) {

You've already copied k_ctx to a local ctx, you should consistently use 
it ctx.


> +			pr_err("Invalid SDOS src_size %u\n", k_ctx->sdos.src_size);
> +			return -EINVAL;
> +		}
> +	} else {
> +		output_size = SDOS_DECRYPTED_MAX_SZ;
> +		/* decrypt: input is header + plaintext + HMAC */
> +		if (k_ctx->sdos.src_size < SDOS_ENCRYPTED_MIN_SZ ||
> +		    k_ctx->sdos.src_size > SDOS_ENCRYPTED_MAX_SZ) {
> +			pr_err("Invalid SDOS src_size %u\n", k_ctx->sdos.src_size);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	s_buf = stratix10_svc_allocate_memory(priv->chan, k_ctx->sdos.src_size);
> +	if (IS_ERR(s_buf)) {
> +		ret = -ENOMEM;
> +		pr_err("Failed to allocate memory for SDOS input data kernel buffer ret: %d\n",
> +		       ret);
> +		return ret;
> +	}
> +
> +	k_ctx->sdos.dst_size = &output_size;

output_size is declared locally on a stack, but you're assigning the 
address to a long-living structure. That leaks the stack address and 
will leave you with a dangling pointer.


> +
> +	d_buf = stratix10_svc_allocate_memory(priv->chan, *k_ctx->sdos.dst_size);
> +	if (IS_ERR(d_buf)) {
> +		ret = -ENOMEM;
> +		pr_err("Failed to allocate memory for SDOS output kernel buffer ret: %d\n", ret);
> +		goto free_sbuf;
> +	}
> +
> +	/* Copy the user space input data to the input data kernel buffer */
> +	ret = copy_from_user(s_buf, k_ctx->sdos.src,
> +			     k_ctx->sdos.src_size) ? -EFAULT : 0;
> +	if (ret) {
> +		pr_err("Failed to copy SDOS data from user to kernel buffer ret: %d\n", ret);
> +		goto free_dbuf;
> +	}
> +
> +	/* Get Owner ID from buf */
> +	temp = (uint8_t *)s_buf;

temp is only used here, just declare as a (uint8_t *) to start with.

Dinh