From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 29B2943F4B5 for ; Tue, 7 Jul 2026 18:41:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783449705; cv=none; b=GI4YYLOqj7A173Yd1LwuCWpw7/a4Kf0EfP+4QWGtyZySLCAm0Gn/KSbpGX7/mTQZjYDd8tyxQ0576sejPsHqZG/Z+sg+5K/XqTxLC53J3SWt52zumUC6HRq+Vrtea78wJOwGLLr6ck43FdxIN7R+mAmNfhR57mRtwxCbZxbbpEY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783449705; c=relaxed/simple; bh=t6v6kQYy4sPTdvPuvmGyCltnXDF2tNP4Ih7x15A0PVA=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=YXSDYDWF2eo9eCJ6+xRdcL+uO7EEiRNDFlsfKTRZjxaz10ciQoxkKMNJvb8wJF6SzqDkjFalGfo07ByPAwBtDjIHYjgWDy3ncu8DKvlFezneVH5Uo2zj4JxonkYdLP9yNCMWLxzoEI/67FeVIZVWe1v6cVQ8HkjpGqxY9xMvZJg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=jmZbRE6M; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="jmZbRE6M" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 13EE91F000E9; Tue, 7 Jul 2026 18:41:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1783449702; bh=gUGu2pOlA+Wika87+TPTaJWlaxwQl9C5mju3OI4TEes=; h=Date:Subject:To:Cc:References:From:In-Reply-To; b=jmZbRE6M5CcXYBIc3AgYQilFeuzWB4PDFUu+oU8jlnhTKVbbKBiKkntQUk5HYVzMy L7Qe7dmdEnEAI7i70c5VcgWzx/x4qOTUm7/mo+dQz1eS/gpUZ9kDsamgMRbveqrSHe UOc3QL6AsGAiTHzf9lw1i3omuDo86cLD7Y6F04uiLy+P8MLm+ik1f3ld2cCHAHAisZ vESu9/q0NSSJnjgsgdlSEJeL+Z9EP8LO13WyNrMt6ZVBPAtJej8W3dnTONlLlwopbP BLsgIVKIXcz8AVl4PhqmmRsgD4Kp8Q+Y+8o1yKMRbaUI30c4jtNnrl+1n3UYU2kAUt ow/BfPPc+t8Cg== Message-ID: Date: Tue, 7 Jul 2026 13:41:41 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 2/2] firmware: socfpga-fcs: add Altera SoCFPGA FCS driver with SDOS Content-Language: en-US To: hang.suan.wang@altera.com, linux-kernel@vger.kernel.org, "Michael S . Tsirkin" , Huacai Chen , Florian Fainelli , Chen-Yu Tsai Cc: muhammad.nazim.amirul.nazle.asmade@altera.com, tze.yee.ng@altera.com, chee.nouk.phoon@altera.com, genevieve.chan@altera.com References: <19534fefb5b8e3a24da6790b381660b4a065aff6.1782888532.git.hang.suan.wang@altera.com> From: Dinh Nguyen In-Reply-To: <19534fefb5b8e3a24da6790b381660b4a065aff6.1782888532.git.hang.suan.wang@altera.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 7/1/26 02:39, hang.suan.wang@altera.com wrote: > From: Hang Suan Wang > > 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 > --- > 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 > F: drivers/input/mouse/alps.* > > +ALTERA FCS DRIVER > +M: Hang Suan Wang > +M: Genevieve Chan > +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 > 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 > +#include > +#include > +#include > +#include > +#include > + > +#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