mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Harshal Dev <harshal.dev@oss.qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Cc: Jens Wiklander <jens.wiklander@oss.qualcomm.com>,
	Sumit Garg <sumit.garg@kernel.org>,
	Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konradybcio@kernel.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>,
	Basant Kumar <basantk@qti.qualcomm.com>,
	Apurupa Pattapu <apurupa@qti.qualcomm.com>,
	Arun Kumar Neelakantam <aneelaka@qti.qualcomm.com>,
	op-tee@lists.trustedfirmware.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v4 6/7] firmware: qcom: Add support for TEE based EFI-var client driver
Date: Mon, 12 Oct 2026 15:06:00 +0530	[thread overview]
Message-ID: <1854d39a-9ef9-4b04-8b5b-3384b8403375@oss.qualcomm.com> (raw)
In-Reply-To: <r7gtspju4cp5iwelv4zs4xoh4d7ambeceptmjtqaf5hcjzmc5n@4msc3ap4yfhz>

Hi Dmitry,

On 06-10-2026 07:18 pm, Dmitry Baryshkov wrote:
> On Tue, Oct 06, 2026 at 05:03:27PM +0530, Harshal Dev wrote:
>> On Qualcomm SoC based platforms, UEFI stores EFI variables within the
>> Replay Protected Memory Block (RPMB) located within either the UFS,
>> eMMC or SPI-NOR storage. The RPMB key which is one-time programmed into
>> the storage controller to allow authentication of the RPMB frames is
>> generated by and only available to the Qualcomm Trusted Execution
>> Environment (QTEE).
>>
>> The legacy QSEECOM protocol used for communicating with the QTEE is
>> deprecated and replaced with the use-case agnostic SMCInvoke protocol
>> starting with the Qualcomm SM8x50 series. On platforms where the QSEECOM
>> driver still probes, it does not support a listener interface with QTEE
>> to enable writing of non-volatile EFI variables to the RPMB for UFS and
>> eMMC storage.
>> Therefore on such platforms, a TEE client driver which communicates with
>> QTEE via the SMCInvoke protocol implemented by the QCOMTEE driver (and
>> registered with the TEE subsystem) must be used to update such EFI
>> variables.
>>
>> Add support for a TEE based uefisecapp client driver which installs efivar
>> operations after obtaining an object reference to the uefisecapp service.
>> This enables the kernel/user-space to access or modify both volatile EFI
>> variables stored by the Secure Application (in-memory) and non-volatile
>> ones stored within RPMB.
>>
>> +
>> +static efi_status_t qcomtee_uefi_query_variable_info(u32 attr, u64 *storage_space,
>> +						     u64 *remaining_space,
>> +						     u64 *max_variable_size)
>> +{
>> +	int ret;
>> +	u32 out_errno;
>> +	u64 maximum_variable_storage_size;
>> +	u64 remaining_variable_storage_size;
>> +	u64 maximum_variable_size;
>> +
>> +	if (!storage_space || !remaining_space || !max_variable_size)
>> +		return EFI_INVALID_PARAMETER;
>> +
>> +	ret = qcuefi_query_variable_info(attr,
>> +					 &maximum_variable_storage_size,
>> +					 &remaining_variable_storage_size,
>> +					 &maximum_variable_size,
>> +					 &out_errno);
>> +
>> +	if (ret)
>> +		return EFI_DEVICE_ERROR;
>> +
>> +	if (!out_errno) {
>> +		*storage_space = maximum_variable_storage_size;
>> +		*remaining_space = remaining_variable_storage_size;
>> +		*max_variable_size = maximum_variable_size;
> 
> Why do we need to copy data? Can we pass pointers directly to the
> qcuefi_foo calls?
>

Ack, this function can have this optimization, no need for copying.
 
>> +	}
>> +
>> +	return uefisecapp_err_to_efi_status(out_errno);
>> +}
>> +
>> +/**
>> + * qcomtee_release_object() - Release an object returned by QTEE.
>> + *
>> + * Each object returned by QTEE repesents a secure service exposed to the
>> + * client. Whenever an secure service is opened, QTEE may allocate resources
>> + * on the client's behalf. Therefore, once the client is done accessing the
>> + * secure service, the object representing it should be explicitly released
>> + * so that QTEE can release the associated resources as well.
>> + *
>> + * @ctx: TEE context.
>> + * @object: The object to release.
>> + */
>> +static void qcomtee_release_object(struct tee_context *ctx,
>> +				   struct tee_param_objref object)
>> +{
>> +	struct tee_ioctl_object_invoke_arg inv_arg;
>> +
>> +	memset(&inv_arg, 0, sizeof(inv_arg));
>> +	SET_INVOKE_ARG(inv_arg, object.id, QCOMTEE_MSG_OBJECT_OP_RELEASE, 0);
>> +	tee_client_object_invoke_func(ctx, &inv_arg, NULL);
>> +}
>> +
>> +/**
>> + * qcomtee_get_uefisec_svc_obj() - Get a UEFI Secure App service object to
>> + * begin communication with the service.
>> + * @ctx: TEE context.
>> + * @client_env_obj: The client environment object returned earlier by QTEE.
>> + * @uefisec_svc_obj: The UEFI Secure App service object.
>> + *
>> + * Returns 0 on success.
>> + * Returns < 0 if client environment object invocation failed.
>> + * Returns > 0 if client environment invocation was success but UEFI Secure App
>> + * service object could not be returned for some other reason (represented by the
>> + * returned value)
>> + */
>> +static int qcomtee_get_uefisec_svc_obj(struct tee_context *ctx,
>> +				       struct tee_param_objref client_env_obj,
>> +				       struct tee_param_objref *uefisec_svc_obj)
>> +{
>> +	int ret;
>> +	struct tee_ioctl_object_invoke_arg inv_arg;
>> +	u64 obj_id = client_env_obj.id;
>> +	struct tee_param param[QCOMTEE_GET_UEFI_SVC_NPARAMS];
>> +	u32 uefisec_uid = QCOMTEE_UEFI_SEC_UID;
>> +
>> +	memset(&inv_arg, 0, sizeof(inv_arg));
>> +	memset(&param, 0, sizeof(param));
>> +
>> +	SET_INVOKE_ARG(inv_arg, obj_id,
>> +		       QCOMTEE_OP_CLIENT_ENV_OPEN,
>> +		       QCOMTEE_GET_UEFI_SVC_NPARAMS);
>> +	SET_TEE_PARAM_UBUF(param[0], UBUF_INPUT, TEE_PARAM_UBUF(uefisec_uid));
>> +	SET_TEE_PARAM_OBJREF(param[1], OBJREF_OUTPUT, 0, 0);
>> +
>> +	ret = tee_client_object_invoke_func(ctx, &inv_arg, param);
>> +	if (ret < 0 || inv_arg.ret != 0) {
>> +		dev_err(uefisec_app.dev, "QCOMTEE_CLIENT_ENV_OPEN invoke ret: %d, err: 0x%x\n",
>> +			ret, inv_arg.ret);
>> +		return ret ?: inv_arg.ret;
>> +	}
>> +
>> +	*uefisec_svc_obj = param[1].u.objref;
>> +	return ret;
>> +}
>> +
>> +/**
>> + * qcomtee_get_client_env_obj() - Get a client environment object to begin
>> + * object exchange with QTEE.
>> + * @ctx: TEE context.
>> + * @client_env_obj: The client environment object returned by QTEE.
>> + *
>> + * Returns 0 on success.
>> + * Returns < 0 if root object invocation failed.
>> + * Returns > 0 if root object invocation was success but client environment
>> + * object could not be returned for some other reason (represented by the
>> + * returned value)
>> + */
>> +static int qcomtee_get_client_env_obj(struct tee_context *ctx,
>> +				      struct tee_param_objref *client_env_obj)
>> +{
>> +	int ret;
>> +	struct tee_ioctl_object_invoke_arg inv_arg;
>> +	struct tee_param param[QCOMTEE_GET_CLIENT_ENV_NPARAMS];
>> +
>> +	memset(&inv_arg, 0, sizeof(inv_arg));
>> +	memset(&param, 0, sizeof(param));
>> +
>> +	SET_INVOKE_ARG(inv_arg, TEE_OBJREF_NULL,
>> +		       QCOMTEE_ROOT_OP_REG_WITH_CREDENTIALS,
>> +		       QCOMTEE_GET_CLIENT_ENV_NPARAMS);
>> +	SET_TEE_PARAM_OBJREF(param[0], OBJREF_INPUT, TEE_OBJREF_NULL, 0);
>> +	SET_TEE_PARAM_OBJREF(param[1], OBJREF_OUTPUT, 0, 0);
>> +
>> +	ret = tee_client_object_invoke_func(ctx, &inv_arg, param);
>> +	if (ret < 0 || inv_arg.ret != 0) {
>> +		dev_err(uefisec_app.dev, "QCOMTEE_ROOT_OP_REG_WITH_CREDENTIALS invoke ret: %d, err: 0x%x\n",
>> +			ret, inv_arg.ret);
>> +		return ret ?: inv_arg.ret;
>> +	}
>> +
>> +	*client_env_obj = param[1].u.objref;
>> +	return ret;
>> +}
>> +
>> +static const struct efivar_operations qcom_efivar_ops = {
>> +	.get_variable = qcomtee_uefi_get_variable,
>> +	.set_variable = qcomtee_uefi_set_variable,
>> +	.get_next_variable = qcomtee_uefi_get_next_variable,
>> +	.query_variable_info = qcomtee_uefi_query_variable_info,
>> +};
>> +
>> +static int qcomtee_ctx_match(struct tee_ioctl_version_data *ver,
>> +			     const void *data)
>> +{
>> +	return (ver->impl_id == TEE_IMPL_ID_QTEE);
>> +}
>> +
>> +static int qcomtee_uefisecapp_probe(struct tee_client_device *tee_dev)
>> +{
>> +	int ret, err;
>> +	struct tee_param_objref client_env_obj;
>> +	struct tee_param_objref uefisec_svc_obj;
>> +
>> +	uefisec_app.dev = &tee_dev->dev;
>> +	/* Open context with QCOMTEE driver */
>> +	uefisec_app.ctx = tee_client_open_context(NULL, qcomtee_ctx_match, NULL,
>> +						  NULL);
>> +	if (IS_ERR(uefisec_app.ctx))
>> +		return -ENODEV;
>> +
>> +	/* Obtain a reference to client_env object to begin object exchange
>> +	 * with QTEE
> 
> Nit: check out the preferred block comment format. also, I think these
> comments echo the code which pretty obvious here.
>

Ack, I'll just drop this comment. And update the other ones to the preferred
block comment format.

>> +	 */
>> +	ret = qcomtee_get_client_env_obj(uefisec_app.ctx, &client_env_obj);
>> +	if (ret) {
>> +		err = -EINVAL;
>> +		goto err_get_client_env;
>> +	}
>> +
>> +	/* Obtain a reference to the uefisec_svc object which provides access to
>> +	 * the EFI var storage.
>> +	 */
>> +	ret = qcomtee_get_uefisec_svc_obj(uefisec_app.ctx, client_env_obj,
>> +					  &uefisec_svc_obj);
>> +	if (ret) {
>> +		err = -EINVAL;
>> +		goto err_get_uefisec_svc;
>> +	}
>> +	uefisec_app.uefisec_svc_obj = uefisec_svc_obj;
>> +
>> +	ret = efivars_register(&uefisec_app.efivars, &qcom_efivar_ops);
>> +	if (ret) {
> 
> If we have both QSEECOM and QTEE drivers enabled (which we hopefully
> will in distro kernels) and if QSEECOM has already provided UEFI vars
> implementation, this call will print a warning and a probe error in
> kernel logs. Similarly, if this driver registers UEFI vars
> implementation first, the QSEECOM one will print out the error.
> 
> We know that there is this kind of a clash. I think it makes sense to
> handle it. Earlier you wrote that QTEE would be a better option if the
> platform supports both. Would it be possible to implement this kind of
> selection?
>

This has been on my mind since the beginning. We could have a central
hub layer which both the QCOMTEE and QSEECOM drivers register with during
their probe and pass a list of services they can communicate with. This
one for example:
https://elixir.bootlin.com/linux/v7.3-rc5/source/drivers/firmware/qcom/qcom_qseecom.c#L86

When the hub realizes (at QCOMTEE registration) that a service is available
via both QSEECOM and QCOMTEE, it could call an API such as qcomtee_check_service()
to check if the current QTEE firmware OS version supports it. If so, it would
call qseecom_client_remove() for this service:
https://elixir.bootlin.com/linux/v7.3-rc5/source/drivers/firmware/qcom/qcom_qseecom.c#L30
and then call qtee_register_service().

This is a high level view of how such a framework could work. But since
only uefisecapp currently fits this scenario it feels like an overkill to
push it along with this series. I can see that Kuldeep's series which is adding
TPM service support also fits the scenario of a service being available via both
QSEECOM and QCOMTEE:
https://lore.kernel.org/all/20261001055029.1960743-1-zeroknots@protonmail.com/

I am fine with investigating the feasibility of such a framework in more
depth if everyone here feels that it is now required. Amir's inputs on this
would be welcome since he is the maintaiiner for QCOMTEE.

And this would have to be a separate patch series after this one lands.

Regards,
Harshal 
>> +		err = ret;
>> +		goto err_efi_vars_reg;
>> +	}
>> +
> 


  reply	other threads:[~2026-10-12  9:36 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-06 11:33 [PATCH v4 0/7] Add TEE based client driver for UEFI Secure Application Harshal Dev
2026-10-06 11:33 ` [PATCH v4 1/7] tee: qcomtee: Track the object invocation context Harshal Dev
2026-10-09  1:01   ` Amirreza Zarrabi
2026-10-06 11:33 ` [PATCH v4 2/7] tee: Add kernel client object invoke helper Harshal Dev
2026-10-09  1:03   ` Amirreza Zarrabi
2026-10-06 11:33 ` [PATCH v4 3/7] tee: qcomtee: Allow object invokes from kernel clients Harshal Dev
2026-10-09  1:19   ` Amirreza Zarrabi
2026-10-12  9:38     ` Harshal Dev
2026-10-06 11:33 ` [PATCH v4 4/7] tee: Export uuidv5 generation for TEE backends Harshal Dev
2026-10-09  1:23   ` Amirreza Zarrabi
2026-10-12  9:45     ` Harshal Dev
2026-10-06 11:33 ` [PATCH v4 5/7] tee: qcomtee: Add support for registering QTEE services on TEE bus Harshal Dev
2026-10-09  2:10   ` Amirreza Zarrabi
2026-10-12  9:51     ` Harshal Dev
2026-10-06 11:33 ` [PATCH v4 6/7] firmware: qcom: Add support for TEE based EFI-var client driver Harshal Dev
2026-10-06 13:48   ` Dmitry Baryshkov
2026-10-12  9:36     ` Harshal Dev [this message]
2026-10-09  2:34   ` Amirreza Zarrabi
2026-10-12 10:34     ` Harshal Dev
2026-10-06 11:33 ` [PATCH v4 7/7] arm64: defconfig: Enable UefiSecApp TEE client driver for Qualcomm SoCs Harshal Dev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1854d39a-9ef9-4b04-8b5b-3384b8403375@oss.qualcomm.com \
    --to=harshal.dev@oss.qualcomm.com \
    --cc=amirreza.zarrabi@oss.qualcomm.com \
    --cc=andersson@kernel.org \
    --cc=aneelaka@qti.qualcomm.com \
    --cc=apurupa@qti.qualcomm.com \
    --cc=basantk@qti.qualcomm.com \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=jens.wiklander@oss.qualcomm.com \
    --cc=konradybcio@kernel.org \
    --cc=krzk@kernel.org \
    --cc=kuldeep.singh@oss.qualcomm.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=op-tee@lists.trustedfirmware.org \
    --cc=sumit.garg@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®