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

[Sumit Garg <sumit.garg@kernel.org>]

Hi Amir,

On Thu, May 28, 2026 at 10:45:32AM +1000, Amirreza Zarrabi wrote:
> Hi Sumit,
> 
> On 5/22/2026 9:59 PM, Sumit Garg wrote:
> > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > 
> > 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
> > using S-EL2 with 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.
> > 
> > Reviewed-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > Tested-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> # Lemans
> > Reviewed-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
> > Tested-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com> # IPQ9650
> > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > ---
> >  drivers/firmware/qcom/Kconfig          |   8 +
> >  drivers/firmware/qcom/Makefile         |   1 +
> >  drivers/firmware/qcom/qcom_pas.c       | 291 +++++++++++++++++++++++++
> >  drivers/firmware/qcom/qcom_pas.h       |  50 +++++
> >  include/linux/firmware/qcom/qcom_pas.h |  43 ++++
> >  5 files changed, 393 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..9f66cc774508 100644
> > --- a/drivers/firmware/qcom/Kconfig
> > +++ b/drivers/firmware/qcom/Kconfig
> > @@ -6,6 +6,14 @@
> >  
> >  menu "Qualcomm firmware drivers"
> >  
> > +config QCOM_PAS
> > +	tristate "Qualcomm generic PAS interface driver"
> > +	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

<snip>

> > diff --git a/include/linux/firmware/qcom/qcom_pas.h b/include/linux/firmware/qcom/qcom_pas.h
> > new file mode 100644
> > index 000000000000..65b1c9564458
> > --- /dev/null
> > +++ b/include/linux/firmware/qcom/qcom_pas.h
> > @@ -0,0 +1,43 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (c) 2010-2015, 2018-2019 The Linux Foundation. All rights reserved.
> > + * Copyright (C) 2015 Linaro Ltd.
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
> > +
> > +#ifndef __QCOM_PAS_H
> > +#define __QCOM_PAS_H
> > +
> > +#include <linux/err.h>
> > +#include <linux/types.h>
> > +
> > +struct qcom_pas_context {
> > +	struct device *dev;
> > +	u32 pas_id;
> > +	phys_addr_t mem_phys;
> > +	size_t mem_size;
> > +	void *ptr;
> > +	dma_addr_t phys;
> > +	ssize_t size;
> > +	bool use_tzmem;
> > +};
> > +
> > +bool qcom_pas_is_available(void);
> > +struct qcom_pas_context *devm_qcom_pas_context_alloc(struct device *dev,
> > +						     u32 pas_id,
> > +						     phys_addr_t mem_phys,
> > +						     size_t mem_size);
> > +int qcom_pas_init_image(u32 pas_id, const void *metadata, size_t size,
> > +			struct qcom_pas_context *ctx);
> > +struct resource_table *qcom_pas_get_rsc_table(struct qcom_pas_context *ctx,
> > +					      void *input_rt, size_t input_rt_size,
> > +					      size_t *output_rt_size);
> > +int qcom_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size);
> > +int qcom_pas_auth_and_reset(u32 pas_id);
> > +int qcom_pas_prepare_and_auth_reset(struct qcom_pas_context *ctx);
> > +int qcom_pas_set_remote_state(u32 state, u32 pas_id);
> > +int qcom_pas_shutdown(u32 pas_id);
> > +bool qcom_pas_supported(u32 pas_id);
> > +void qcom_pas_metadata_release(struct qcom_pas_context *ctx);
> > +
> > +#endif /* __QCOM_PAS_H */
> 
> I have a question about the shape of the generic PAS abstraction.
> 
> Looking at the current interface, it seems that pas_id is still treated as
> the primary identity for most PAS operations, while struct qcom_pas_context
> is used as optional extended state for the operations that need it. I can see
> how this maps well to the existing SCM PAS interface and keeps the transition
> simple.
> 
> However, I wonder if this makes the abstraction less generic in the long term.
> Some callbacks in struct qcom_pas_ops receive only pas_id, while others
> receive struct qcom_pas_context *ctx. This works as long as pas_id is
> sufficient for those operations. But if a backend needs per-peripheral
> private state for operations such as auth_and_reset,
> shutdown, or set_remote_state, it would need to reconstruct that state from
> pas_id or maintain a separate pas_id to backend-context mapping.
> 
> Would it be cleaner to make struct qcom_pas_context the common
> per-peripheral object and pass it consistently to all per-peripheral callbacks?
> Existing backends could still use ctx->pas_id, but future backends would not
> need to perform a separate lookup only because the callback was passed a raw
> pas_id.

The common evolution for a generic abstraction comes when it's needed.
So once the client driver or the backend comes up with certain
requirements then the generic PAS layer can be extended as needed. It
should be possible for more APIs to migrate to using PAS context but
there should be corresponding use-case requirements.

> 
> I also wonder whether struct qcom_pas_context is exposing some
> implementation-specific state. Fields such as ptr, phys, size, and
> use_tzmem seem to describe how the current SCM/QTEE implementations manage
> metadata memory, rather than generic PAS state. For another backend, the
> per-operation state might be a tee_shm, an FF-A memory handle, shared or lent
> memory state, or something else transport-specific.
> 
> Would it make sense to keep only the common PAS fields in
> struct qcom_pas_context, such as dev, pas_id, and possibly the firmware
> memory address/size if those are truly generic, and add a backend-private
> pointer for implementation-specific state?

Currently, the 2 backends can re-use parts of the PAS context but I
agree more abstraction can be done as needed for future backends.

> 
> I may be missing a reason why the pas_id-only callbacks are preferred, but if
> this is intended to be the long-term generic PAS layer rather than mainly a
> shim over the existing SCM API shape, using the context consistently and keeping
> backend-specific state private seems easier to extend.

As I mentioned above, the generic PAS layer isn't fixed but it will
surely go through evolution as needed. Current abstraction handles the
existing client driver and backend requirments.

Moreover the subsystem maintainers are already complaining about this
series being too large for the merge to happen.

-Sumit