Re: [PATCH v2 02/15] firmware: qcom: Add a generic PAS service

[Krzysztof Kozlowski]

On 23/03/2026 14:22, Sumit Garg wrote:
> On Mon, Mar 16, 2026 at 08:51:16AM +0100, Krzysztof Kozlowski wrote:
>> On 12/03/2026 07:27, Sumit Garg wrote:
>>> From: Sumit Garg <sumit.garg@xxxxxxxxxxxxxxxx>
>>>
>>> Qcom platforms has the legacy of using non-standard SCM calls
>>> splintered over the various kernel drivers. These SCM calls aren't
>>> compliant with the standard SMC calling conventions which is a
>>> prerequisite to enable migration to the FF-A specifications from Arm.
>>>
>>> OP-TEE as an alternative trusted OS to Qualcomm TEE (QTEE) can't
>>> support these non-standard SCM calls. And even for newer architectures
>>> with S-EL2 and Hafnium support, QTEE won't be able to support SCM
>>> calls either with FF-A requirements coming in. And with both OP-TEE
>>> and QTEE drivers well integrated in the TEE subsystem, it makes further
>>> sense to reuse the TEE bus client drivers infrastructure.
>>>
>>> The added benefit of TEE bus infrastructure is that there is support
>>> for discoverable/enumerable services. With that client drivers don't
>>> have to manually invoke a special SCM call to know the service status.
>>>
>>> So enable the generic Peripheral Authentication Service (PAS) provided
>>> by the firmware. It acts as the common layer with different TZ
>>> backends plugged in whether it's an SCM implementation or a proper
>>> TEE bus based PAS service implementation.
>>>
>>> Signed-off-by: Sumit Garg <sumit.garg@xxxxxxxxxxxxxxxx>
>>> ---
>>>  drivers/firmware/qcom/Kconfig          |   8 +
>>>  drivers/firmware/qcom/Makefile         |   1 +
>>>  drivers/firmware/qcom/qcom_pas.c       | 298 +++++++++++++++++++++++++
>>>  drivers/firmware/qcom/qcom_pas.h       |  53 +++++
>>>  include/linux/firmware/qcom/qcom_pas.h |  41 ++++
>>>  5 files changed, 401 insertions(+)
>>>  create mode 100644 drivers/firmware/qcom/qcom_pas.c
>>>  create mode 100644 drivers/firmware/qcom/qcom_pas.h
>>>  create mode 100644 include/linux/firmware/qcom/qcom_pas.h
>>>
>>> diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
>>> index b477d54b495a..8653639d06db 100644
>>> --- a/drivers/firmware/qcom/Kconfig
>>> +++ b/drivers/firmware/qcom/Kconfig
>>> @@ -6,6 +6,14 @@
>>>  
>>>  menu "Qualcomm firmware drivers"
>>>  
>>> +config QCOM_PAS
>>> +	tristate
>>> +	help
>>> +	  Enable the generic Peripheral Authentication Service (PAS) provided
>>> +	  by the firmware. It acts as the common layer with different TZ
>>> +	  backends plugged in whether it's an SCM implementation or a proper
>>> +	  TEE bus based PAS service implementation.
>>> +
>>>  config QCOM_SCM
>>>  	select QCOM_TZMEM
>>>  	tristate
>>> diff --git a/drivers/firmware/qcom/Makefile b/drivers/firmware/qcom/Makefile
>>> index 0be40a1abc13..dc5ab45f906a 100644
>>> --- a/drivers/firmware/qcom/Makefile
>>> +++ b/drivers/firmware/qcom/Makefile
>>> @@ -8,3 +8,4 @@ qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
>>>  obj-$(CONFIG_QCOM_TZMEM)	+= qcom_tzmem.o
>>>  obj-$(CONFIG_QCOM_QSEECOM)	+= qcom_qseecom.o
>>>  obj-$(CONFIG_QCOM_QSEECOM_UEFISECAPP) += qcom_qseecom_uefisecapp.o
>>> +obj-$(CONFIG_QCOM_PAS)		+= qcom_pas.o
>>> diff --git a/drivers/firmware/qcom/qcom_pas.c b/drivers/firmware/qcom/qcom_pas.c
>>> new file mode 100644
>>> index 000000000000..beb1bae55546
>>> --- /dev/null
>>> +++ b/drivers/firmware/qcom/qcom_pas.c
>>> @@ -0,0 +1,298 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>>> + */
>>> +
>>> +#include <linux/device/devres.h>
>>> +#include <linux/firmware/qcom/qcom_pas.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +
>>> +#include "qcom_pas.h"
>>> +
>>> +struct qcom_pas_ops *ops_ptr;
>>
>> Same comment as before. Don't create singletons. And for sure not global
>> ones.
> 
> This pattern has been carried from the PAS API contract among kernel
> clients and the SCM PAS service earlier. The clients don't hold a
> reference to the PAS data like underlying platform or TEE device etc.
> Hence the need to have a global data pointer to hold reference to the
> ops data structure registered by drivers having different lifetime of
> devices. Also, the PAS APIs can be called from very different client
> driver contexts.
> 
> Surely, avoiding global data is always better given a better alternative
> is there. Do you have any better alternative proposal here?

Why it cannot be part of the context?

Look at your API, e.g.:
qcom_pas_init_image(). It takes struct qcom_pas_context which should
contain the ops.

Best regards,
Krzysztof