Re: [PATCH 3/4] firmware: Add support for Qualcomm UEFI Secure Application

[Johan Hovold]

On Sun, Jul 24, 2022 at 12:49:48AM +0200, Maximilian Luz wrote:
> On platforms using the Qualcomm UEFI Secure Application (uefisecapp),
> EFI variables cannot be accessed via the standard interface in EFI
> runtime mode. The respective functions return EFI_UNSUPPORTED. On these
> platforms, we instead need to talk to uefisecapp. This commit provides
> support for this and registers the respective efivars operations to
> access EFI variables from the kernel.
> 
> Communication with uefisecapp follows the standard Qualcomm Trusted
> Environment (TEE or TrEE) / Secure OS conventions via the respective SCM
> call interface. This is also the reason why variable access works
> normally while boot services are active. During this time, said SCM
> interface is managed by the boot services. When calling
> ExitBootServices(), the ownership is transferred to the kernel.
> Therefore, UEFI must not use that interface itself (as multiple parties
> accessing this interface at the same time may lead to complications) and
> cannot access variables for us.
> 
> Signed-off-by: Maximilian Luz <luzmaximilian@xxxxxxxxx>
> ---

> +static int qcom_uefisecapp_probe(struct platform_device *pdev)
> +{
> +	struct qcuefi_client *qcuefi;
> +	int status;

[...]

> +	/* Set up kobject for efivars interface. */
> +	qcuefi->kobj = kobject_create_and_add("qcom_tee_uefisecapp", firmware_kobj);
> +	if (!qcuefi->kobj) {
> +		status = -ENOMEM;
> +		goto err_kobj;

When preparing some related EFI patches, I noticed that the error labels
here are named after where you jump from rather than after what they do
(as is suggested by the coding standard).

Would you mind changing that (throughout) for your v2?

> +	}
> +
> +	/* Register global reference. */
> +	platform_set_drvdata(pdev, qcuefi);
> +	status = qcuefi_set_reference(qcuefi);
> +	if (status)
> +		goto err_ref;
> +
> +	/* Register efivar ops. */
> +	status = efivars_register(&qcuefi->efivars, &qcom_efivar_ops, qcuefi->kobj);
> +	if (status)
> +		goto err_register;
> +
> +	return 0;
> +
> +err_register:
> +	qcuefi_set_reference(NULL);
> +err_ref:
> +	kobject_put(qcuefi->kobj);
> +err_kobj:
> +	qctee_dma_free(qcuefi->dev, &qcuefi->dma);
> +	return status;
> +}

Johan