[PATCH 0/4] Add support for the TPM FF-A start method

[PATCH 0/4] Add support for the TPM FF-A start method

[Stuart Yoder]

These patches add support for the CRB FF-A start method defined
in the TCG ACPI specification v1.4 and the FF-A ABI defined
in the Arm TPM Service CRB over FF-A (DEN0138) specification. 
(https://developer.arm.com/documentation/den0138/latest/)

FF-A is a messaging framework for Arm-based systems and in the
context of the TPM driver is used to signal 'start' to a CRB-based
TPM service which is hosted in an FF-A secure partition running in
TrustZone.

The first patch adds an FF-A driver to handle the FF-A messaging when
communicating with a CRB-based TPM secure partition built on FF-A.
The driver is probed when the TPM secure partition is discovered by
the Linux FF-A infrastructure.

The second patch consolidates the check for idle support in the CRB
driver to one place.

The third patch defines the new ACPI start method enumeration for
CRB over FF-A.

The fourth patch adds support for the FF-A ACPI start method to
the TPM crb driver.

Stuart Yoder (4):
  tpm_crb: implement driver compliant to CRB over FF-A
  tpm_crb: refactor check for idle support into TPM into inline function
  ACPICA: add start method for Arm FF-A
  tpm_crb: add support for the Arm FF-A start method

 drivers/char/tpm/Kconfig   |   9 ++
 drivers/char/tpm/Makefile  |   1 +
 drivers/char/tpm/ffa_crb.c | 310 +++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/ffa_crb.h |  30 ++++
 drivers/char/tpm/tpm_crb.c |  81 +++++++++-
 include/acpi/actbl3.h      |   1 +
 6 files changed, 426 insertions(+), 6 deletions(-)
 create mode 100644 drivers/char/tpm/ffa_crb.c
 create mode 100644 drivers/char/tpm/ffa_crb.h

-- 
2.34.1

[PATCH 1/4] tpm_crb: implement driver compliant to CRB over FF-A

[Stuart Yoder]

The Arm specification TPM Service CRB over FF-A specification
defines the FF-A messages to interact with a CRB-based TPM
implemented as an FF-A secure partition.

Spec URL:
https://developer.arm.com/documentation/den0138/latest/

This driver is probed when a TPM Secure Partition is
discovered by the FF-A subsystem. It exposes APIs
used by the TPM CRB driver to send notifications to
the TPM.

Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>
---
 drivers/char/tpm/Kconfig   |   9 ++
 drivers/char/tpm/Makefile  |   1 +
 drivers/char/tpm/ffa_crb.c | 310 +++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/ffa_crb.h |  30 ++++
 4 files changed, 350 insertions(+)
 create mode 100644 drivers/char/tpm/ffa_crb.c
 create mode 100644 drivers/char/tpm/ffa_crb.h

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 0fc9a510e059..f02af3925758 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -210,6 +210,15 @@ config TCG_CRB
 	  from within Linux.  To compile this driver as a module, choose
 	  M here; the module will be called tpm_crb.
 
+config TCG_ARM_FFA_CRB
+	tristate "TPM CRB over Arm FF-A Transport"
+	depends on ARM_FFA_TRANSPORT
+	default y if (TCG_CRB && ARM_FFA_TRANSPORT)
+	help
+	  If the Arm FF-A transport is used to access the TPM say Yes.
+	  To compile this driver as a module, choose M here; the module
+	  will be called ffa_crb.
+
 config TCG_VTPM_PROXY
 	tristate "VTPM Proxy Interface"
 	depends on TCG_TPM
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 9bb142c75243..7c65368e18db 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -42,5 +42,6 @@ obj-$(CONFIG_TCG_IBMVTPM) += tpm_ibmvtpm.o
 obj-$(CONFIG_TCG_TIS_ST33ZP24) += st33zp24/
 obj-$(CONFIG_TCG_XEN) += xen-tpmfront.o
 obj-$(CONFIG_TCG_CRB) += tpm_crb.o
+obj-$(CONFIG_TCG_ARM_FFA_CRB) += ffa_crb.o
 obj-$(CONFIG_TCG_VTPM_PROXY) += tpm_vtpm_proxy.o
 obj-$(CONFIG_TCG_FTPM_TEE) += tpm_ftpm_tee.o
diff --git a/drivers/char/tpm/ffa_crb.c b/drivers/char/tpm/ffa_crb.c
new file mode 100644
index 000000000000..8f11e49956a8
--- /dev/null
+++ b/drivers/char/tpm/ffa_crb.c
@@ -0,0 +1,310 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 Arm Ltd.
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * This device driver implements the TPM CRB start method
+ * as defined in the TPM Service Command Response Buffer
+ * Interface Over FF-A (DEN0138).
+ */
+
+#define pr_fmt(fmt) "FFA_CRB: " fmt
+
+#include <linux/arm_ffa.h>
+#include "ffa_crb.h"
+
+/* TPM service function status codes */
+#define FFA_CRB_OK			0x05000001
+#define FFA_CRB_OK_RESULTS_RETURNED	0x05000002
+#define FFA_CRB_NOFUNC			0x8e000001
+#define FFA_CRB_NOTSUP			0x8e000002
+#define FFA_CRB_INVARG			0x8e000005
+#define FFA_CRB_INV_CRB_CTRL_DATA	0x8e000006
+#define FFA_CRB_ALREADY			0x8e000009
+#define FFA_CRB_DENIED			0x8e00000a
+#define FFA_CRB_NOMEM			0x8e00000b
+
+#define FFA_CRB_VERSION_MAJOR	1
+#define FFA_CRB_VERSION_MINOR	0
+
+/* version encoding */
+#define FFA_CRB_MAJOR_VERSION_MASK  GENMASK(30, 16)
+#define FFA_CRB_MINOR_VERSION_MASK  GENMASK(15, 0)
+#define FFA_CRB_MAJOR_VERSION(x)    ((u16)(FIELD_GET(FFA_CRB_MAJOR_VERSION_MASK, (x))))
+#define FFA_CRB_MINOR_VERSION(x)    ((u16)(FIELD_GET(FFA_CRB_MINOR_VERSION_MASK, (x))))
+
+/*
+ * Normal world sends requests with FFA_MSG_SEND_DIRECT_REQ and
+ * responses are returned with FFA_MSG_SEND_DIRECT_RESP for normal
+ * messages.
+ *
+ * All requests with FFA_MSG_SEND_DIRECT_REQ and FFA_MSG_SEND_DIRECT_RESP
+ * are using the AArch32 SMC calling convention with register usage as
+ * defined in FF-A specification:
+ * w0:    Function ID (0x8400006F or 0x84000070)
+ * w1:    Source/Destination IDs
+ * w2:    Reserved (MBZ)
+ * w3-w7: Implementation defined, free to be used below
+ */
+
+/*
+ * Returns the version of the interface that is available
+ * Call register usage:
+ * w3:    Not used (MBZ)
+ * w4:    TPM service function ID, FFA_CRB_GET_INTERFACE_VERSION
+ * w5-w7: Reserved (MBZ)
+ *
+ * Return register usage:
+ * w3:    Not used (MBZ)
+ * w4:    TPM service function status
+ * w5:    TPM service interface version
+ *        Bits[31:16]: major version
+ *        Bits[15:0]: minor version
+ * w6-w7: Reserved (MBZ)
+ *
+ * Possible function status codes in register w4:
+ *     CRB_FFA_OK_RESULTS_RETURNED: The version of the interface has been
+ *                                  returned.
+ */
+#define FFA_CRB_GET_INTERFACE_VERSION 0x0f000001
+
+/*
+ * Return information on a given feature of the TPM service
+ * Call register usage:
+ * w3:    Not used (MBZ)
+ * w4:    TPM service function ID, FFA_CRB_START
+ * w5:    Start function qualifier
+ *            Bits[31:8] (MBZ)
+ *            Bits[7:0]
+ *              0: Notifies TPM that a command is ready to be processed
+ *              1: Notifies TPM that a locality request is ready to be processed
+ * w6:    TPM locality, one of 0..4
+ *            -If the start function qualifier is 0, identifies the locality
+ *             from where the command originated.
+ *            -If the start function qualifier is 1, identifies the locality
+ *             of the locality request
+ * w6-w7: Reserved (MBZ)
+ *
+ * Return register usage:
+ * w3:    Not used (MBZ)
+ * w4:    TPM service function status
+ * w5-w7: Reserved (MBZ)
+ *
+ * Possible function status codes in register w4:
+ *     FFA_CRB_OK: the TPM service has been notified successfully
+ *     FFA_CRB_INVARG: one or more arguments are not valid
+ *     FFA_CRB_INV_CRB_CTRL_DATA: CRB control data or locality control
+ *         data at the given TPM locality is not valid
+ *     FFA_CRB_DENIED: the TPM has previously disabled locality requests and
+ *         command processing at the given locality
+ */
+#define FFA_CRB_START 0x0f000201
+
+struct ffa_crb {
+	struct ffa_device *ffa_dev;
+	u16 major_version;
+	u16 minor_version;
+	struct mutex msg_data_lock;
+	struct ffa_send_direct_data direct_msg_data;
+};
+
+static struct ffa_crb *ffa_crb;
+
+static int ffa_crb_to_linux_errno(int errno)
+{
+	int rc;
+
+	switch (errno) {
+	case FFA_CRB_OK:
+		rc = 0;
+		break;
+	case FFA_CRB_OK_RESULTS_RETURNED:
+		rc = 0;
+		break;
+	case FFA_CRB_NOFUNC:
+		rc = -ENOENT;
+		break;
+	case FFA_CRB_NOTSUP:
+		rc = -EPERM;
+		break;
+	case FFA_CRB_INVARG:
+		rc = -EINVAL;
+		break;
+	case FFA_CRB_INV_CRB_CTRL_DATA:
+		rc = -ENOEXEC;
+		break;
+	case FFA_CRB_ALREADY:
+		rc = -EEXIST;
+		break;
+	case FFA_CRB_DENIED:
+		rc = -EACCES;
+		break;
+	case FFA_CRB_NOMEM:
+		rc = -ENOMEM;
+		break;
+	default:
+		rc = -EINVAL;
+	}
+
+	return rc;
+}
+
+int ffa_crb_init(void)
+{
+	if (ffa_crb == NULL)
+		return -ENOENT;
+
+	if (IS_ERR_VALUE(ffa_crb))
+		return -ENODEV;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ffa_crb_init);
+
+static int __ffa_crb_send_recieve(unsigned long func_id,
+		unsigned long a0, unsigned long a1, unsigned long a2)
+{
+	int ret;
+	const struct ffa_msg_ops *msg_ops;
+
+	if (ffa_crb == NULL)
+		return -ENOENT;
+
+	msg_ops = ffa_crb->ffa_dev->ops->msg_ops;
+
+	memset(&ffa_crb->direct_msg_data, 0x00,
+			sizeof(struct ffa_send_direct_data));
+
+	ffa_crb->direct_msg_data.data1 = func_id;
+	ffa_crb->direct_msg_data.data2 = a0;
+	ffa_crb->direct_msg_data.data3 = a1;
+	ffa_crb->direct_msg_data.data4 = a2;
+
+	ret = msg_ops->sync_send_receive(ffa_crb->ffa_dev,
+			&ffa_crb->direct_msg_data);
+	if (!ret)
+		ret = ffa_crb_to_linux_errno(ffa_crb->direct_msg_data.data1);
+
+	return ret;
+}
+
+int ffa_crb_get_interface_version(uint16_t *major, uint16_t *minor)
+{
+	int rc;
+
+	if (ffa_crb == NULL)
+		return -ENOENT;
+
+	if (IS_ERR_VALUE(ffa_crb))
+		return -ENODEV;
+
+	if (major == NULL || minor == NULL)
+		return -EINVAL;
+
+	guard(mutex)(&ffa_crb->msg_data_lock);
+
+	rc = __ffa_crb_send_recieve(FFA_CRB_GET_INTERFACE_VERSION, 0x00, 0x00, 0x00);
+	if (!rc) {
+		*major = FFA_CRB_MAJOR_VERSION(ffa_crb->direct_msg_data.data2);
+		*minor = FFA_CRB_MINOR_VERSION(ffa_crb->direct_msg_data.data2);
+	}
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(ffa_crb_get_interface_version);
+
+int ffa_crb_start(int request_type, int locality)
+{
+	if (ffa_crb == NULL)
+		return -ENOENT;
+
+	if (IS_ERR_VALUE(ffa_crb))
+		return -ENODEV;
+
+	guard(mutex)(&ffa_crb->msg_data_lock);
+
+	return __ffa_crb_send_recieve(FFA_CRB_START, request_type, locality, 0x00);
+}
+EXPORT_SYMBOL_GPL(ffa_crb_start);
+
+static int ffa_crb_probe(struct ffa_device *ffa_dev)
+{
+	int rc;
+	struct ffa_crb *p;
+
+	/* only one instance of a TPM partition is supported */
+	if (ffa_crb && !IS_ERR_VALUE(ffa_crb))
+		return -EEXIST;
+
+	ffa_crb = ERR_PTR(-ENODEV); // set ffa_crb so we can detect probe failure
+
+	if (!ffa_partition_supports_direct_recv(ffa_dev)) {
+		pr_err("TPM partition doesn't support direct message receive.\n");
+		return -EINVAL;
+	}
+
+	p = kzalloc(sizeof(*ffa_crb), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+	ffa_crb = p;
+
+	mutex_init(&ffa_crb->msg_data_lock);
+	ffa_crb->ffa_dev = ffa_dev;
+	ffa_dev_set_drvdata(ffa_dev, ffa_crb);
+
+	/* if TPM is aarch32 use 32-bit SMCs */
+	if (!ffa_partition_check_property(ffa_dev, FFA_PARTITION_AARCH64_EXEC))
+		ffa_dev->ops->msg_ops->mode_32bit_set(ffa_dev);
+
+	/* verify compatibility of TPM service version number */
+	rc = ffa_crb_get_interface_version(&ffa_crb->major_version,
+			&ffa_crb->minor_version);
+	if (rc) {
+		pr_err("failed to get crb interface version. rc:%d", rc);
+		goto out;
+	}
+
+	pr_info("ABI version %u.%u", ffa_crb->major_version,
+		ffa_crb->minor_version);
+
+	if ((ffa_crb->major_version != FFA_CRB_VERSION_MAJOR) ||
+	    (ffa_crb->minor_version < FFA_CRB_VERSION_MINOR)) {
+		pr_err("Incompatible ABI version");
+		goto out;
+	}
+
+	return 0;
+
+out:
+	kfree(ffa_crb);
+	ffa_crb = ERR_PTR(-ENODEV);
+	return -EINVAL;
+}
+
+static void ffa_crb_remove(struct ffa_device *ffa_dev)
+{
+	kfree(ffa_crb);
+	ffa_crb = NULL;
+}
+
+static const struct ffa_device_id ffa_crb_device_id[] = {
+	/* 17b862a4-1806-4faf-86b3-089a58353861 */
+	{ UUID_INIT(0x17b862a4, 0x1806, 0x4faf,
+		    0x86, 0xb3, 0x08, 0x9a, 0x58, 0x35, 0x38, 0x61) },
+	{}
+};
+
+static struct ffa_driver ffa_crb_driver = {
+	.name = "ffa-crb",
+	.probe = ffa_crb_probe,
+	.remove = ffa_crb_remove,
+	.id_table = ffa_crb_device_id,
+};
+
+module_ffa_driver(ffa_crb_driver);
+
+MODULE_AUTHOR("Arm");
+MODULE_DESCRIPTION("FFA CRB driver");
+MODULE_VERSION("1.0");
+MODULE_LICENSE("GPL");
diff --git a/drivers/char/tpm/ffa_crb.h b/drivers/char/tpm/ffa_crb.h
new file mode 100644
index 000000000000..42327d285165
--- /dev/null
+++ b/drivers/char/tpm/ffa_crb.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2024 Arm Ltd.
+ *
+ * Authors:
+ * Stuart Yoder <stuart.yoder@arm.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * This device driver implements the TPM CRB start method
+ * as defined in the TPM Service Command Response Buffer
+ * Interface Over FF-A (DEN0138).
+ */
+#ifndef _FFA_CRB_H
+#define _FFA_CRB_H
+
+#if IS_ENABLED(CONFIG_TCG_ARM_FFA_CRB)
+int ffa_crb_init(void);
+int ffa_crb_get_interface_version(uint16_t *major, uint16_t *minor);
+int ffa_crb_start(int request_type, int locality);
+#else
+static inline int ffa_crb_init(void) { return 0; }
+static inline int ffa_crb_get_interface_version(uint16_t *major, uint16_t *minor) { return 0; }
+static inline int ffa_crb_start(int request_type, int locality) { return 0; }
+#endif
+
+#define FFA_CRB_START_TYPE_COMMAND 0
+#define FFA_CRB_START_TYPE_LOCALITY_REQUEST 1
+
+#endif
-- 
2.34.1

Re: [PATCH 1/4] tpm_crb: implement driver compliant to CRB over FF-A

[Jarkko Sakkinen]

On Mon, Feb 10, 2025 at 05:22:24PM -0600, Stuart Yoder wrote:
> The Arm specification TPM Service CRB over FF-A specification
> defines the FF-A messages to interact with a CRB-based TPM
> implemented as an FF-A secure partition.
> 
> Spec URL:
> https://developer.arm.com/documentation/den0138/latest/
> 
> This driver is probed when a TPM Secure Partition is
> discovered by the FF-A subsystem. It exposes APIs
> used by the TPM CRB driver to send notifications to
> the TPM.
> 
> Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>
> ---
>  drivers/char/tpm/Kconfig   |   9 ++
>  drivers/char/tpm/Makefile  |   1 +
>  drivers/char/tpm/ffa_crb.c | 310 +++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/ffa_crb.h |  30 ++++

Let's use tpm_ prefix for these.

BR, Jarkko

Re: [PATCH 1/4] tpm_crb: implement driver compliant to CRB over FF-A

[Stuart Yoder]

On 2/11/25 3:08 PM, Jarkko Sakkinen wrote:
> On Mon, Feb 10, 2025 at 05:22:24PM -0600, Stuart Yoder wrote:
>> The Arm specification TPM Service CRB over FF-A specification
>> defines the FF-A messages to interact with a CRB-based TPM
>> implemented as an FF-A secure partition.
>>
>> Spec URL:
>> https://developer.arm.com/documentation/den0138/latest/
>>
>> This driver is probed when a TPM Secure Partition is
>> discovered by the FF-A subsystem. It exposes APIs
>> used by the TPM CRB driver to send notifications to
>> the TPM.
>>
>> Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>
>> ---
>>   drivers/char/tpm/Kconfig   |   9 ++
>>   drivers/char/tpm/Makefile  |   1 +
>>   drivers/char/tpm/ffa_crb.c | 310 +++++++++++++++++++++++++++++++++++++
>>   drivers/char/tpm/ffa_crb.h |  30 ++++
> 
> Let's use tpm_ prefix for these.

Ok, will do.

Thanks,
Stuart

[PATCH 2/4] tpm_crb: refactor check for idle support into TPM into inline function

[Stuart Yoder]

Refactor the two checks for whether the TPM supports idle into a single
inline function.

Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>
---
 drivers/char/tpm/tpm_crb.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index ea085b14ab7c..87d69e990249 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -115,6 +115,16 @@ struct tpm2_crb_pluton {
 	u64 reply_addr;
 };
 
+static inline bool does_tpm_support_idle(u32 start_method)
+{
+	if ((start_method == ACPI_TPM2_START_METHOD) ||
+	    (start_method == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD) ||
+	    (start_method == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC))
+		return false;
+	else
+		return true;
+}
+
 static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
 				unsigned long timeout)
 {
@@ -173,9 +183,7 @@ static int __crb_go_idle(struct device *dev, struct crb_priv *priv)
 {
 	int rc;
 
-	if ((priv->sm == ACPI_TPM2_START_METHOD) ||
-	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD) ||
-	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC))
+	if (!does_tpm_support_idle(priv->sm))
 		return 0;
 
 	iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->regs_t->ctrl_req);
@@ -222,9 +230,7 @@ static int __crb_cmd_ready(struct device *dev, struct crb_priv *priv)
 {
 	int rc;
 
-	if ((priv->sm == ACPI_TPM2_START_METHOD) ||
-	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD) ||
-	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC))
+	if (!does_tpm_support_idle(priv->sm))
 		return 0;
 
 	iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->regs_t->ctrl_req);
-- 
2.34.1

[PATCH 3/4] ACPICA: add start method for Arm FF-A

[Stuart Yoder]

Add TPM start method for Arm FF-A defined in the TCG ACPI
specification v1.4.

Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>
---
 include/acpi/actbl3.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
index 5cd755143b7d..a97b1dbab975 100644
--- a/include/acpi/actbl3.h
+++ b/include/acpi/actbl3.h
@@ -466,6 +466,7 @@ struct acpi_tpm2_phy {
 #define ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC       11	/* V1.2 Rev 8 */
 #define ACPI_TPM2_RESERVED                          12
 #define ACPI_TPM2_COMMAND_BUFFER_WITH_PLUTON        13
+#define ACPI_TPM2_CRB_WITH_ARM_FFA                  15
 
 /* Optional trailer appears after any start_method subtables */
 
-- 
2.34.1

Re: [PATCH 3/4] ACPICA: add start method for Arm FF-A

[Sudeep Holla]

On Mon, Feb 10, 2025 at 05:22:26PM -0600, Stuart Yoder wrote:
> Add TPM start method for Arm FF-A defined in the TCG ACPI
> specification v1.4.
>

ACPICA changes require (at least) a pull request to be submitted to
the upstream ACPICA project on GitHub from where the changes get pulled
into the kernel along with other changes.

If such a pull request is already created, please resend the Linux patch
with a link pointing to that pull request to inform the maintainer about
the same so that it helps in the review of the patches here.

-- 
Regards,
Sudeep

Re: [PATCH 3/4] ACPICA: add start method for Arm FF-A

[Stuart Yoder]

On 2/11/25 3:57 AM, Sudeep Holla wrote:
> On Mon, Feb 10, 2025 at 05:22:26PM -0600, Stuart Yoder wrote:
>> Add TPM start method for Arm FF-A defined in the TCG ACPI
>> specification v1.4.
>>
> 
> ACPICA changes require (at least) a pull request to be submitted to
> the upstream ACPICA project on GitHub from where the changes get pulled
> into the kernel along with other changes.
> 
> If such a pull request is already created, please resend the Linux patch
> with a link pointing to that pull request to inform the maintainer about
> the same so that it helps in the review of the patches here.

I will submit the pull request to acpiaca and send the link when I send
v2 of this series.

Thanks,
Stuart

[PATCH 4/4] tpm_crb: add support for the Arm FF-A start method

[Stuart Yoder]

The TCG ACPI spec v1.4 defines a start method for the
TPMs implemented with the Arm CRB over FF-A ABI.

Add support for the FF-A start method, and use interfaces
provided by the ffa_crb driver to interact with the
FF-A based TPM.

Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>
---
 drivers/char/tpm/tpm_crb.c | 65 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 64 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 87d69e990249..8b36072916a1 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -19,6 +19,7 @@
 #ifdef CONFIG_ARM64
 #include <linux/arm-smccc.h>
 #endif
+#include "ffa_crb.h"
 #include "tpm.h"
 
 #define ACPI_SIG_TPM2 "TPM2"
@@ -100,6 +101,8 @@ struct crb_priv {
 	u32 smc_func_id;
 	u32 __iomem *pluton_start_addr;
 	u32 __iomem *pluton_reply_addr;
+	u8 ffa_flags;
+	u8 ffa_attributes;
 };
 
 struct tpm2_crb_smc {
@@ -110,6 +113,14 @@ struct tpm2_crb_smc {
 	u32 smc_func_id;
 };
 
+/* CRB over FFA start method parameters in TCG2 ACPI table */
+struct tpm2_crb_ffa {
+	u8 flags;
+	u8 attributes;
+	u16 partition_id;
+	u8 reserved[8];
+};
+
 struct tpm2_crb_pluton {
 	u64 start_addr;
 	u64 reply_addr;
@@ -119,7 +130,8 @@ static inline bool does_tpm_support_idle(u32 start_method)
 {
 	if ((start_method == ACPI_TPM2_START_METHOD) ||
 	    (start_method == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD) ||
-	    (start_method == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC))
+	    (start_method == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) ||
+	    (start_method == ACPI_TPM2_CRB_WITH_ARM_FFA))
 		return false;
 	else
 		return true;
@@ -261,6 +273,7 @@ static int crb_cmd_ready(struct tpm_chip *chip)
 static int __crb_request_locality(struct device *dev,
 				  struct crb_priv *priv, int loc)
 {
+	int rc;
 	u32 value = CRB_LOC_STATE_LOC_ASSIGNED |
 		    CRB_LOC_STATE_TPM_REG_VALID_STS;
 
@@ -268,6 +281,13 @@ static int __crb_request_locality(struct device *dev,
 		return 0;
 
 	iowrite32(CRB_LOC_CTRL_REQUEST_ACCESS, &priv->regs_h->loc_ctrl);
+
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		rc = ffa_crb_start(FFA_CRB_START_TYPE_LOCALITY_REQUEST, loc);
+		if (rc)
+			return rc;
+	}
+
 	if (!crb_wait_for_reg_32(&priv->regs_h->loc_state, value, value,
 				 TPM2_TIMEOUT_C)) {
 		dev_warn(dev, "TPM_LOC_STATE_x.requestAccess timed out\n");
@@ -287,6 +307,7 @@ static int crb_request_locality(struct tpm_chip *chip, int loc)
 static int __crb_relinquish_locality(struct device *dev,
 				     struct crb_priv *priv, int loc)
 {
+	int rc;
 	u32 mask = CRB_LOC_STATE_LOC_ASSIGNED |
 		   CRB_LOC_STATE_TPM_REG_VALID_STS;
 	u32 value = CRB_LOC_STATE_TPM_REG_VALID_STS;
@@ -295,6 +316,13 @@ static int __crb_relinquish_locality(struct device *dev,
 		return 0;
 
 	iowrite32(CRB_LOC_CTRL_RELINQUISH, &priv->regs_h->loc_ctrl);
+
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		rc = ffa_crb_start(FFA_CRB_START_TYPE_LOCALITY_REQUEST, loc);
+		if (rc)
+			return rc;
+	}
+
 	if (!crb_wait_for_reg_32(&priv->regs_h->loc_state, mask, value,
 				 TPM2_TIMEOUT_C)) {
 		dev_warn(dev, "TPM_LOC_STATE_x.Relinquish timed out\n");
@@ -443,6 +471,11 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
 		rc = tpm_crb_smc_start(&chip->dev, priv->smc_func_id);
 	}
 
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
+		rc = ffa_crb_start(FFA_CRB_START_TYPE_COMMAND, chip->locality);
+	}
+
 	if (rc)
 		return rc;
 
@@ -451,6 +484,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
 
 static void crb_cancel(struct tpm_chip *chip)
 {
+	int rc;
 	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
 
 	iowrite32(CRB_CANCEL_INVOKE, &priv->regs_t->ctrl_cancel);
@@ -459,6 +493,12 @@ static void crb_cancel(struct tpm_chip *chip)
 	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)) &&
 	     crb_do_acpi_start(chip))
 		dev_err(&chip->dev, "ACPI Start failed\n");
+
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		rc = ffa_crb_start(FFA_CRB_START_TYPE_COMMAND, chip->locality);
+		if (rc)
+			dev_err(&chip->dev, "FF-A Start failed\n");
+	}
 }
 
 static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
@@ -616,6 +656,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 	 * stuff that puts the control area outside the ACPI IO region.
 	 */
 	if ((priv->sm == ACPI_TPM2_COMMAND_BUFFER) ||
+	    (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) ||
 	    (priv->sm == ACPI_TPM2_MEMORY_MAPPED)) {
 		if (iores &&
 		    buf->control_address == iores->start +
@@ -737,6 +778,7 @@ static int crb_acpi_add(struct acpi_device *device)
 	struct tpm_chip *chip;
 	struct device *dev = &device->dev;
 	struct tpm2_crb_smc *crb_smc;
+	struct tpm2_crb_ffa *crb_ffa;
 	struct tpm2_crb_pluton *crb_pluton;
 	acpi_status status;
 	u32 sm;
@@ -775,6 +817,27 @@ static int crb_acpi_add(struct acpi_device *device)
 		priv->smc_func_id = crb_smc->smc_func_id;
 	}
 
+	if (sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		if (buf->header.length < (sizeof(*buf) + sizeof(*crb_ffa))) {
+			dev_err(dev,
+				FW_BUG "TPM2 ACPI table has wrong size %u for start method type %d\n",
+				buf->header.length,
+				ACPI_TPM2_CRB_WITH_ARM_FFA);
+			rc = -EINVAL;
+			goto out;
+		}
+		crb_ffa = ACPI_ADD_PTR(struct tpm2_crb_ffa, buf, sizeof(*buf));
+		priv->ffa_flags = crb_ffa->flags;
+		priv->ffa_attributes = crb_ffa->attributes;
+		rc = ffa_crb_init();
+		if (rc) {
+			if (rc == -ENOENT) {  // FF-A driver is not available yet
+				rc = -EPROBE_DEFER;
+			}
+			goto out;
+		}
+	}
+
 	if (sm == ACPI_TPM2_COMMAND_BUFFER_WITH_PLUTON) {
 		if (buf->header.length < (sizeof(*buf) + sizeof(*crb_pluton))) {
 			dev_err(dev,
-- 
2.34.1

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Sumit Garg]

+ Jens

Hi Stuart,

On Tue, 11 Feb 2025 at 04:52, Stuart Yoder <stuart.yoder@arm.com> wrote:
>
> These patches add support for the CRB FF-A start method defined
> in the TCG ACPI specification v1.4 and the FF-A ABI defined
> in the Arm TPM Service CRB over FF-A (DEN0138) specification.
> (https://developer.arm.com/documentation/den0138/latest/)

Nice to have a specification standardizing interface to TPM
managed/implemented by the firmware. Care to add corresponding kernel
documentation under Documentation/security/tpm/.

BTW, we already have drivers/char/tpm/tpm_ftpm_tee.c, so do you see
possibilities for an abstraction layer on top of communication channel
based on either FF-A or TEE or platform bus?

>
> FF-A is a messaging framework for Arm-based systems and in the
> context of the TPM driver is used to signal 'start' to a CRB-based
> TPM service which is hosted in an FF-A secure partition running in
> TrustZone.

Is there any open source implementation for such a secure partition
managing the TPM? Also, is that really a discrete TPM or firmware TPM
managed by the firmware?

If it supports firmware TPM, I would be interested to see how you plan
to handle cases related to secure storage.

-Sumit

>
> The first patch adds an FF-A driver to handle the FF-A messaging when
> communicating with a CRB-based TPM secure partition built on FF-A.
> The driver is probed when the TPM secure partition is discovered by
> the Linux FF-A infrastructure.
>
> The second patch consolidates the check for idle support in the CRB
> driver to one place.
>
> The third patch defines the new ACPI start method enumeration for
> CRB over FF-A.
>
> The fourth patch adds support for the FF-A ACPI start method to
> the TPM crb driver.
>
> Stuart Yoder (4):
>   tpm_crb: implement driver compliant to CRB over FF-A
>   tpm_crb: refactor check for idle support into TPM into inline function
>   ACPICA: add start method for Arm FF-A
>   tpm_crb: add support for the Arm FF-A start method
>
>  drivers/char/tpm/Kconfig   |   9 ++
>  drivers/char/tpm/Makefile  |   1 +
>  drivers/char/tpm/ffa_crb.c | 310 +++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/ffa_crb.h |  30 ++++
>  drivers/char/tpm/tpm_crb.c |  81 +++++++++-
>  include/acpi/actbl3.h      |   1 +
>  6 files changed, 426 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/char/tpm/ffa_crb.c
>  create mode 100644 drivers/char/tpm/ffa_crb.h
>
> --
> 2.34.1
>
>

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Stuart Yoder]

Hi Sumit,

On 2/11/25 12:45 AM, Sumit Garg wrote:
> + Jens
> 
> Hi Stuart,
> 
> On Tue, 11 Feb 2025 at 04:52, Stuart Yoder <stuart.yoder@arm.com> wrote:
>>
>> These patches add support for the CRB FF-A start method defined
>> in the TCG ACPI specification v1.4 and the FF-A ABI defined
>> in the Arm TPM Service CRB over FF-A (DEN0138) specification.
>> (https://developer.arm.com/documentation/den0138/latest/)
> 
> Nice to have a specification standardizing interface to TPM
> managed/implemented by the firmware. Care to add corresponding kernel
> documentation under Documentation/security/tpm/.

Yes, I can add some documentation there.

> BTW, we already have drivers/char/tpm/tpm_ftpm_tee.c, so do you see
> possibilities for an abstraction layer on top of communication channel
> based on either FF-A or TEE or platform bus?

I think the CRB and OP-TEE based messaging approaches for interacting
with a TZ-based TPM are fundamentally different and I don't see how
to harmonize them through some abstraction.

The OP-TEE TPM protocol copies the TPM command into a temp shared memory
buffer and sends a message to the TPM referencing that buffer.

The CRB uses a permanently shared memory carve-out that in addition
to the command/response data has other fields for locality control,
command control, status, TPM idle, etc. The only 'message' needed is
something to signal 'start'.  Any OS that is FF-A aware and has a
CRB driver can simply add a new start method, which is what this
patch series does.

>>
>> FF-A is a messaging framework for Arm-based systems and in the
>> context of the TPM driver is used to signal 'start' to a CRB-based
>> TPM service which is hosted in an FF-A secure partition running in
>> TrustZone.
> 
> Is there any open source implementation for such a secure partition
> managing the TPM?

Nothing yet, but something I am working towards.

> Also, is that really a discrete TPM or firmware TPM
> managed by the firmware?

It could be either. It doesn't matter from the point of view of
the OS CRB driver. For testing this patch series I used an
internal proof-of-concept fTPM with a CRB interface.

> If it supports firmware TPM, I would be interested to see how you plan
> to handle cases related to secure storage.

Yes, this is a challenge and there are various ways it could be
implemented. For example, RPMB or if you have an internal root of
trust with secure storage like an RSE that could play a role.

Thanks,
Stuart

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Sumit Garg]

On Tue, 11 Feb 2025 at 21:39, Stuart Yoder <stuart.yoder@arm.com> wrote:
>
> Hi Sumit,
>
> On 2/11/25 12:45 AM, Sumit Garg wrote:
> > + Jens
> >
> > Hi Stuart,
> >
> > On Tue, 11 Feb 2025 at 04:52, Stuart Yoder <stuart.yoder@arm.com> wrote:
> >>
> >> These patches add support for the CRB FF-A start method defined
> >> in the TCG ACPI specification v1.4 and the FF-A ABI defined
> >> in the Arm TPM Service CRB over FF-A (DEN0138) specification.
> >> (https://developer.arm.com/documentation/den0138/latest/)
> >
> > Nice to have a specification standardizing interface to TPM
> > managed/implemented by the firmware. Care to add corresponding kernel
> > documentation under Documentation/security/tpm/.
>
> Yes, I can add some documentation there.
>
> > BTW, we already have drivers/char/tpm/tpm_ftpm_tee.c, so do you see
> > possibilities for an abstraction layer on top of communication channel
> > based on either FF-A or TEE or platform bus?
>
> I think the CRB and OP-TEE based messaging approaches for interacting
> with a TZ-based TPM are fundamentally different and I don't see how
> to harmonize them through some abstraction.
>
> The OP-TEE TPM protocol copies the TPM command into a temp shared memory
> buffer and sends a message to the TPM referencing that buffer.
>
> The CRB uses a permanently shared memory carve-out that in addition
> to the command/response data has other fields for locality control,
> command control, status, TPM idle, etc. The only 'message' needed is
> something to signal 'start'.  Any OS that is FF-A aware and has a
> CRB driver can simply add a new start method, which is what this
> patch series does.

Okay, I see how the CRB driver is closely tied to the ACPI based
systems. I was expecting the FF-A based TPM interface to be
independent of ACPI or DT such that it's not constrained by the
hardware description a platform chooses to use. I suppose there will
be a different TPM FF-A driver or spec when someone wants to deploy it
on DT based systems, right?

>
> >>
> >> FF-A is a messaging framework for Arm-based systems and in the
> >> context of the TPM driver is used to signal 'start' to a CRB-based
> >> TPM service which is hosted in an FF-A secure partition running in
> >> TrustZone.
> >
> > Is there any open source implementation for such a secure partition
> > managing the TPM?
>
> Nothing yet, but something I am working towards.
>
> > Also, is that really a discrete TPM or firmware TPM
> > managed by the firmware?
>
> It could be either. It doesn't matter from the point of view of
> the OS CRB driver. For testing this patch series I used an
> internal proof-of-concept fTPM with a CRB interface.

Okay I see, having a real firmware managed TPM implementation will
really unlock the adoption of this specification.

>
> > If it supports firmware TPM, I would be interested to see how you plan
> > to handle cases related to secure storage.
>
> Yes, this is a challenge and there are various ways it could be
> implemented. For example, RPMB or if you have an internal root of
> trust with secure storage like an RSE that could play a role.
>

The RPMB kernel routing is what we have for the OP-TEE based fTPM but
I agree there are numerous ways to implement it given the platform's
capability.

-Sumit

> Thanks,
> Stuart
>
>

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Stuart Yoder]

On 2/12/25 1:39 AM, Sumit Garg wrote:
> On Tue, 11 Feb 2025 at 21:39, Stuart Yoder <stuart.yoder@arm.com> wrote:
>>
>> Hi Sumit,
>>
>> On 2/11/25 12:45 AM, Sumit Garg wrote:
>>> + Jens
>>>
>>> Hi Stuart,
>>>
>>> On Tue, 11 Feb 2025 at 04:52, Stuart Yoder <stuart.yoder@arm.com> wrote:
>>>>
>>>> These patches add support for the CRB FF-A start method defined
>>>> in the TCG ACPI specification v1.4 and the FF-A ABI defined
>>>> in the Arm TPM Service CRB over FF-A (DEN0138) specification.
>>>> (https://developer.arm.com/documentation/den0138/latest/)
>>>
>>> Nice to have a specification standardizing interface to TPM
>>> managed/implemented by the firmware. Care to add corresponding kernel
>>> documentation under Documentation/security/tpm/.
>>
>> Yes, I can add some documentation there.
>>
>>> BTW, we already have drivers/char/tpm/tpm_ftpm_tee.c, so do you see
>>> possibilities for an abstraction layer on top of communication channel
>>> based on either FF-A or TEE or platform bus?
>>
>> I think the CRB and OP-TEE based messaging approaches for interacting
>> with a TZ-based TPM are fundamentally different and I don't see how
>> to harmonize them through some abstraction.
>>
>> The OP-TEE TPM protocol copies the TPM command into a temp shared memory
>> buffer and sends a message to the TPM referencing that buffer.
>>
>> The CRB uses a permanently shared memory carve-out that in addition
>> to the command/response data has other fields for locality control,
>> command control, status, TPM idle, etc. The only 'message' needed is
>> something to signal 'start'.  Any OS that is FF-A aware and has a
>> CRB driver can simply add a new start method, which is what this
>> patch series does.
> 
> Okay, I see how the CRB driver is closely tied to the ACPI based
> systems.

The CRB driver is currently probed based on ACPI, but it fundamentally
doesn't have to be.  If there was a DT binding for CRB-based
TPMs the different start methods would be defined there and the
CRB driver could support that.

> I was expecting the FF-A based TPM interface to be
> independent of ACPI or DT such that it's not constrained by the
> hardware description a platform chooses to use. I suppose there will
> be a different TPM FF-A driver or spec when someone wants to deploy it
> on DT based systems, right?

The CRB is just a shared memory buffer, with some architected semantics
defined by TCG. The basic CRB usage model is that a client puts
something in the CRB, such as the bytes of a TPM command, and then
notifies the TPM that a change was made to the CRB. The CRB over
FF-A spec just defines the message to perform that notification
when FF-A is used.

So, whether the fTPM was advertised via ACPI or DT, it doesn't matter.
The FF-A based interface is only about the the notification messages
needed for the OS driver to tell the TPM that something has changed
in the CRB.

Thanks,
Stuart

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Sumit Garg]

+ Rob

On Thu, 13 Feb 2025 at 03:25, Stuart Yoder <stuart.yoder@arm.com> wrote:
>
>
>
> On 2/12/25 1:39 AM, Sumit Garg wrote:
> > On Tue, 11 Feb 2025 at 21:39, Stuart Yoder <stuart.yoder@arm.com> wrote:
> >>
> >> Hi Sumit,
> >>
> >> On 2/11/25 12:45 AM, Sumit Garg wrote:
> >>> + Jens
> >>>
> >>> Hi Stuart,
> >>>
> >>> On Tue, 11 Feb 2025 at 04:52, Stuart Yoder <stuart.yoder@arm.com> wrote:
> >>>>
> >>>> These patches add support for the CRB FF-A start method defined
> >>>> in the TCG ACPI specification v1.4 and the FF-A ABI defined
> >>>> in the Arm TPM Service CRB over FF-A (DEN0138) specification.
> >>>> (https://developer.arm.com/documentation/den0138/latest/)
> >>>
> >>> Nice to have a specification standardizing interface to TPM
> >>> managed/implemented by the firmware. Care to add corresponding kernel
> >>> documentation under Documentation/security/tpm/.
> >>
> >> Yes, I can add some documentation there.
> >>
> >>> BTW, we already have drivers/char/tpm/tpm_ftpm_tee.c, so do you see
> >>> possibilities for an abstraction layer on top of communication channel
> >>> based on either FF-A or TEE or platform bus?
> >>
> >> I think the CRB and OP-TEE based messaging approaches for interacting
> >> with a TZ-based TPM are fundamentally different and I don't see how
> >> to harmonize them through some abstraction.
> >>
> >> The OP-TEE TPM protocol copies the TPM command into a temp shared memory
> >> buffer and sends a message to the TPM referencing that buffer.
> >>
> >> The CRB uses a permanently shared memory carve-out that in addition
> >> to the command/response data has other fields for locality control,
> >> command control, status, TPM idle, etc. The only 'message' needed is
> >> something to signal 'start'.  Any OS that is FF-A aware and has a
> >> CRB driver can simply add a new start method, which is what this
> >> patch series does.
> >
> > Okay, I see how the CRB driver is closely tied to the ACPI based
> > systems.
>
> The CRB driver is currently probed based on ACPI, but it fundamentally
> doesn't have to be.  If there was a DT binding for CRB-based
> TPMs the different start methods would be defined there and the
> CRB driver could support that.
>

Can't we rather enable the CRB driver itself probed based on FF-A bus
and rather dynamically discover the shared memory buffer via FF-A
instead? AFAIU, FF-A provides you with a discovery framework for
firmware bits. But if we still want to overload ACPI or DT with the
discoverable firmware bits then it seems like an overkill here.

> > I was expecting the FF-A based TPM interface to be
> > independent of ACPI or DT such that it's not constrained by the
> > hardware description a platform chooses to use. I suppose there will
> > be a different TPM FF-A driver or spec when someone wants to deploy it
> > on DT based systems, right?
>
> The CRB is just a shared memory buffer, with some architected semantics
> defined by TCG. The basic CRB usage model is that a client puts
> something in the CRB, such as the bytes of a TPM command, and then
> notifies the TPM that a change was made to the CRB. The CRB over
> FF-A spec just defines the message to perform that notification
> when FF-A is used.
>
> So, whether the fTPM was advertised via ACPI or DT, it doesn't matter.
> The FF-A based interface is only about the the notification messages
> needed for the OS driver to tell the TPM that something has changed
> in the CRB.

-Sumit

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Stuart Yoder]

On 2/12/25 11:31 PM, Sumit Garg wrote:
> + Rob
> 
> On Thu, 13 Feb 2025 at 03:25, Stuart Yoder <stuart.yoder@arm.com> wrote:
>>
>>
>>
>> On 2/12/25 1:39 AM, Sumit Garg wrote:
>>> On Tue, 11 Feb 2025 at 21:39, Stuart Yoder <stuart.yoder@arm.com> wrote:
>>>>
>>>> Hi Sumit,
>>>>
>>>> On 2/11/25 12:45 AM, Sumit Garg wrote:
>>>>> + Jens
>>>>>
>>>>> Hi Stuart,
>>>>>
>>>>> On Tue, 11 Feb 2025 at 04:52, Stuart Yoder <stuart.yoder@arm.com> wrote:
>>>>>>
>>>>>> These patches add support for the CRB FF-A start method defined
>>>>>> in the TCG ACPI specification v1.4 and the FF-A ABI defined
>>>>>> in the Arm TPM Service CRB over FF-A (DEN0138) specification.
>>>>>> (https://developer.arm.com/documentation/den0138/latest/)
>>>>>
>>>>> Nice to have a specification standardizing interface to TPM
>>>>> managed/implemented by the firmware. Care to add corresponding kernel
>>>>> documentation under Documentation/security/tpm/.
>>>>
>>>> Yes, I can add some documentation there.
>>>>
>>>>> BTW, we already have drivers/char/tpm/tpm_ftpm_tee.c, so do you see
>>>>> possibilities for an abstraction layer on top of communication channel
>>>>> based on either FF-A or TEE or platform bus?
>>>>
>>>> I think the CRB and OP-TEE based messaging approaches for interacting
>>>> with a TZ-based TPM are fundamentally different and I don't see how
>>>> to harmonize them through some abstraction.
>>>>
>>>> The OP-TEE TPM protocol copies the TPM command into a temp shared memory
>>>> buffer and sends a message to the TPM referencing that buffer.
>>>>
>>>> The CRB uses a permanently shared memory carve-out that in addition
>>>> to the command/response data has other fields for locality control,
>>>> command control, status, TPM idle, etc. The only 'message' needed is
>>>> something to signal 'start'.  Any OS that is FF-A aware and has a
>>>> CRB driver can simply add a new start method, which is what this
>>>> patch series does.
>>>
>>> Okay, I see how the CRB driver is closely tied to the ACPI based
>>> systems.
>>
>> The CRB driver is currently probed based on ACPI, but it fundamentally
>> doesn't have to be.  If there was a DT binding for CRB-based
>> TPMs the different start methods would be defined there and the
>> CRB driver could support that.
>>
> 
> Can't we rather enable the CRB driver itself probed based on FF-A bus
> and rather dynamically discover the shared memory buffer via FF-A
> instead? AFAIU, FF-A provides you with a discovery framework for
> firmware bits.

Yes, you could do this. But, then the TPM CRB drivers in all the
ACPI-based OSes (Linux, Windows) and hypervisors need to be
taught this new method of discovery. Adding new start methods is
reasonably straightforward, but changing the basic discovery
mechanism is a much bigger change.

> But if we still want to overload ACPI or DT with the
> discoverable firmware bits then it seems like an overkill here.

I think it would make sense to do ACPI based discovery or FF-A
based discovery, but doing both I think would be overkill.  For
ease of OS integration ACPI is the way to go.  And, potentially
device tree in the future.

Thanks,
Stuart

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Sumit Garg]

On Thu, Feb 13, 2025 at 09:19:58AM -0600, Stuart Yoder wrote:
> 
> 
> On 2/12/25 11:31 PM, Sumit Garg wrote:
> > + Rob
> > 
> > On Thu, 13 Feb 2025 at 03:25, Stuart Yoder <stuart.yoder@arm.com> wrote:
> > > 
> > > 
> > > 
> > > On 2/12/25 1:39 AM, Sumit Garg wrote:
> > > > On Tue, 11 Feb 2025 at 21:39, Stuart Yoder <stuart.yoder@arm.com> wrote:
> > > > > 
> > > > > Hi Sumit,
> > > > > 
> > > > > On 2/11/25 12:45 AM, Sumit Garg wrote:
> > > > > > + Jens
> > > > > > 
> > > > > > Hi Stuart,
> > > > > > 
> > > > > > On Tue, 11 Feb 2025 at 04:52, Stuart Yoder <stuart.yoder@arm.com> wrote:
> > > > > > > 
> > > > > > > These patches add support for the CRB FF-A start method defined
> > > > > > > in the TCG ACPI specification v1.4 and the FF-A ABI defined
> > > > > > > in the Arm TPM Service CRB over FF-A (DEN0138) specification.
> > > > > > > (https://developer.arm.com/documentation/den0138/latest/)
> > > > > > 
> > > > > > Nice to have a specification standardizing interface to TPM
> > > > > > managed/implemented by the firmware. Care to add corresponding kernel
> > > > > > documentation under Documentation/security/tpm/.
> > > > > 
> > > > > Yes, I can add some documentation there.
> > > > > 
> > > > > > BTW, we already have drivers/char/tpm/tpm_ftpm_tee.c, so do you see
> > > > > > possibilities for an abstraction layer on top of communication channel
> > > > > > based on either FF-A or TEE or platform bus?
> > > > > 
> > > > > I think the CRB and OP-TEE based messaging approaches for interacting
> > > > > with a TZ-based TPM are fundamentally different and I don't see how
> > > > > to harmonize them through some abstraction.
> > > > > 
> > > > > The OP-TEE TPM protocol copies the TPM command into a temp shared memory
> > > > > buffer and sends a message to the TPM referencing that buffer.
> > > > > 
> > > > > The CRB uses a permanently shared memory carve-out that in addition
> > > > > to the command/response data has other fields for locality control,
> > > > > command control, status, TPM idle, etc. The only 'message' needed is
> > > > > something to signal 'start'.  Any OS that is FF-A aware and has a
> > > > > CRB driver can simply add a new start method, which is what this
> > > > > patch series does.
> > > > 
> > > > Okay, I see how the CRB driver is closely tied to the ACPI based
> > > > systems.
> > > 
> > > The CRB driver is currently probed based on ACPI, but it fundamentally
> > > doesn't have to be.  If there was a DT binding for CRB-based
> > > TPMs the different start methods would be defined there and the
> > > CRB driver could support that.
> > > 
> > 
> > Can't we rather enable the CRB driver itself probed based on FF-A bus
> > and rather dynamically discover the shared memory buffer via FF-A
> > instead? AFAIU, FF-A provides you with a discovery framework for
> > firmware bits.
> 
> Yes, you could do this. But, then the TPM CRB drivers in all the
> ACPI-based OSes (Linux, Windows) and hypervisors need to be
> taught this new method of discovery. Adding new start methods is
> reasonably straightforward, but changing the basic discovery
> mechanism is a much bigger change.

We will be teaching every other OS or hypervisor about FF-A
communication regardless. So it's rather about if we want to do it
properly leveraging auto discovery mechanisms supported by FF-A or not.

> 
> > But if we still want to overload ACPI or DT with the
> > discoverable firmware bits then it seems like an overkill here.
> 
> I think it would make sense to do ACPI based discovery or FF-A
> based discovery, but doing both I think would be overkill.  For
> ease of OS integration ACPI is the way to go.  And, potentially
> device tree in the future.

Encoding firmware bits in ACPI/DT can be seen as an easy upstream path
in the shorter run. But when the ACPI/DT becomes overloaded with
information that has to be passed from firmware to the OS rather than
purely describing hardware to the OS, it's ABI maintainability becomes
complex. We are already dealing with DT ABI compatibility challenges
especially the forward compatibility, so let's not make it even worse
with firmware information that can be discovered automatically.

The other benefit of auto discovery is that platform enablement becomes
really smooth. Once the firmware starts supporting a particular feature
like TPM over FF-A then the OS can discover and support it.

-Sumit

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Stuart Yoder]

On 2/16/25 11:17 PM, Sumit Garg wrote:
> On Thu, Feb 13, 2025 at 09:19:58AM -0600, Stuart Yoder wrote:
>>
>>
>> On 2/12/25 11:31 PM, Sumit Garg wrote:
>>> + Rob
>>>
>>> On Thu, 13 Feb 2025 at 03:25, Stuart Yoder <stuart.yoder@arm.com> wrote:
>>>>
>>>>
>>>>
>>>> On 2/12/25 1:39 AM, Sumit Garg wrote:
>>>>> On Tue, 11 Feb 2025 at 21:39, Stuart Yoder <stuart.yoder@arm.com> wrote:
>>>>>>
>>>>>> Hi Sumit,
>>>>>>
>>>>>> On 2/11/25 12:45 AM, Sumit Garg wrote:
>>>>>>> + Jens
>>>>>>>
>>>>>>> Hi Stuart,
>>>>>>>
>>>>>>> On Tue, 11 Feb 2025 at 04:52, Stuart Yoder <stuart.yoder@arm.com> wrote:
>>>>>>>>
>>>>>>>> These patches add support for the CRB FF-A start method defined
>>>>>>>> in the TCG ACPI specification v1.4 and the FF-A ABI defined
>>>>>>>> in the Arm TPM Service CRB over FF-A (DEN0138) specification.
>>>>>>>> (https://developer.arm.com/documentation/den0138/latest/)
>>>>>>>
>>>>>>> Nice to have a specification standardizing interface to TPM
>>>>>>> managed/implemented by the firmware. Care to add corresponding kernel
>>>>>>> documentation under Documentation/security/tpm/.
>>>>>>
>>>>>> Yes, I can add some documentation there.
>>>>>>
>>>>>>> BTW, we already have drivers/char/tpm/tpm_ftpm_tee.c, so do you see
>>>>>>> possibilities for an abstraction layer on top of communication channel
>>>>>>> based on either FF-A or TEE or platform bus?
>>>>>>
>>>>>> I think the CRB and OP-TEE based messaging approaches for interacting
>>>>>> with a TZ-based TPM are fundamentally different and I don't see how
>>>>>> to harmonize them through some abstraction.
>>>>>>
>>>>>> The OP-TEE TPM protocol copies the TPM command into a temp shared memory
>>>>>> buffer and sends a message to the TPM referencing that buffer.
>>>>>>
>>>>>> The CRB uses a permanently shared memory carve-out that in addition
>>>>>> to the command/response data has other fields for locality control,
>>>>>> command control, status, TPM idle, etc. The only 'message' needed is
>>>>>> something to signal 'start'.  Any OS that is FF-A aware and has a
>>>>>> CRB driver can simply add a new start method, which is what this
>>>>>> patch series does.
>>>>>
>>>>> Okay, I see how the CRB driver is closely tied to the ACPI based
>>>>> systems.
>>>>
>>>> The CRB driver is currently probed based on ACPI, but it fundamentally
>>>> doesn't have to be.  If there was a DT binding for CRB-based
>>>> TPMs the different start methods would be defined there and the
>>>> CRB driver could support that.
>>>>
>>>
>>> Can't we rather enable the CRB driver itself probed based on FF-A bus
>>> and rather dynamically discover the shared memory buffer via FF-A
>>> instead? AFAIU, FF-A provides you with a discovery framework for
>>> firmware bits.
>>
>> Yes, you could do this. But, then the TPM CRB drivers in all the
>> ACPI-based OSes (Linux, Windows) and hypervisors need to be
>> taught this new method of discovery. Adding new start methods is
>> reasonably straightforward, but changing the basic discovery
>> mechanism is a much bigger change.
> 
> We will be teaching every other OS or hypervisor about FF-A
> communication regardless. So it's rather about if we want to do it
> properly leveraging auto discovery mechanisms supported by FF-A or not.
> 
>>
>>> But if we still want to overload ACPI or DT with the
>>> discoverable firmware bits then it seems like an overkill here.
>>
>> I think it would make sense to do ACPI based discovery or FF-A
>> based discovery, but doing both I think would be overkill.  For
>> ease of OS integration ACPI is the way to go.  And, potentially
>> device tree in the future.
> 
> Encoding firmware bits in ACPI/DT can be seen as an easy upstream path
> in the shorter run. But when the ACPI/DT becomes overloaded with
> information that has to be passed from firmware to the OS rather than
> purely describing hardware to the OS, it's ABI maintainability becomes
> complex. We are already dealing with DT ABI compatibility challenges
> especially the forward compatibility, so let's not make it even worse
> with firmware information that can be discovered automatically.

The TCG defined ACPI table has the following:
    -Physical address of the TPM
    -Start method
    -Start method specific parameters
    -event log address

This has been in place 8+ years and this is what OSes expect.
The start method advertises the mechanism a driver uses to
signal the TPM that something has changed in the CRB, and
this allows different types of TPM implementations:
    -memory mapped
    -signal via ACPI
    -signal via ARM SMC (legacy)
    -signal via Pluton mailbox
    -signal via FF-A

I don't see this as overloading the ACPI table, it's just what
the OS needs to know.

The TPM does not know (and should not know) the address of
the event log. An FF-A based TPM has no way to know this.

I don't see how changing TPM discovery to be via FF-A directly
would improve maintainability.

> The other benefit of auto discovery is that platform enablement becomes
> really smooth. Once the firmware starts supporting a particular feature
> like TPM over FF-A then the OS can discover and support it.

If we added new CRB/FF-A ABIs to get the CRB physical address,
start method specific parameters, event log, it would mean that
all OSes and hypervisors need to re-architect their CRB drivers
or create new FF-A specific CRB drivers.  That will not smooth
enablement for TPMs.  And I don't see advantages for
maintainability.

Thanks,
Stuart

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Sumit Garg]

On Mon, Feb 17, 2025 at 10:56:58AM -0600, Stuart Yoder wrote:
> 
> 
> On 2/16/25 11:17 PM, Sumit Garg wrote:
> > On Thu, Feb 13, 2025 at 09:19:58AM -0600, Stuart Yoder wrote:
> > > 
> > > 
> > > On 2/12/25 11:31 PM, Sumit Garg wrote:
> > > > + Rob
> > > > 
> > > > On Thu, 13 Feb 2025 at 03:25, Stuart Yoder <stuart.yoder@arm.com> wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > On 2/12/25 1:39 AM, Sumit Garg wrote:
> > > > > > On Tue, 11 Feb 2025 at 21:39, Stuart Yoder <stuart.yoder@arm.com> wrote:
> > > > > > > 
> > > > > > > Hi Sumit,
> > > > > > > 
> > > > > > > On 2/11/25 12:45 AM, Sumit Garg wrote:
> > > > > > > > + Jens
> > > > > > > > 
> > > > > > > > Hi Stuart,
> > > > > > > > 
> > > > > > > > On Tue, 11 Feb 2025 at 04:52, Stuart Yoder <stuart.yoder@arm.com> wrote:
> > > > > > > > > 
> > > > > > > > > These patches add support for the CRB FF-A start method defined
> > > > > > > > > in the TCG ACPI specification v1.4 and the FF-A ABI defined
> > > > > > > > > in the Arm TPM Service CRB over FF-A (DEN0138) specification.
> > > > > > > > > (https://developer.arm.com/documentation/den0138/latest/)
> > > > > > > > 
> > > > > > > > Nice to have a specification standardizing interface to TPM
> > > > > > > > managed/implemented by the firmware. Care to add corresponding kernel
> > > > > > > > documentation under Documentation/security/tpm/.
> > > > > > > 
> > > > > > > Yes, I can add some documentation there.
> > > > > > > 
> > > > > > > > BTW, we already have drivers/char/tpm/tpm_ftpm_tee.c, so do you see
> > > > > > > > possibilities for an abstraction layer on top of communication channel
> > > > > > > > based on either FF-A or TEE or platform bus?
> > > > > > > 
> > > > > > > I think the CRB and OP-TEE based messaging approaches for interacting
> > > > > > > with a TZ-based TPM are fundamentally different and I don't see how
> > > > > > > to harmonize them through some abstraction.
> > > > > > > 
> > > > > > > The OP-TEE TPM protocol copies the TPM command into a temp shared memory
> > > > > > > buffer and sends a message to the TPM referencing that buffer.
> > > > > > > 
> > > > > > > The CRB uses a permanently shared memory carve-out that in addition
> > > > > > > to the command/response data has other fields for locality control,
> > > > > > > command control, status, TPM idle, etc. The only 'message' needed is
> > > > > > > something to signal 'start'.  Any OS that is FF-A aware and has a
> > > > > > > CRB driver can simply add a new start method, which is what this
> > > > > > > patch series does.
> > > > > > 
> > > > > > Okay, I see how the CRB driver is closely tied to the ACPI based
> > > > > > systems.
> > > > > 
> > > > > The CRB driver is currently probed based on ACPI, but it fundamentally
> > > > > doesn't have to be.  If there was a DT binding for CRB-based
> > > > > TPMs the different start methods would be defined there and the
> > > > > CRB driver could support that.
> > > > > 
> > > > 
> > > > Can't we rather enable the CRB driver itself probed based on FF-A bus
> > > > and rather dynamically discover the shared memory buffer via FF-A
> > > > instead? AFAIU, FF-A provides you with a discovery framework for
> > > > firmware bits.
> > > 
> > > Yes, you could do this. But, then the TPM CRB drivers in all the
> > > ACPI-based OSes (Linux, Windows) and hypervisors need to be
> > > taught this new method of discovery. Adding new start methods is
> > > reasonably straightforward, but changing the basic discovery
> > > mechanism is a much bigger change.
> > 
> > We will be teaching every other OS or hypervisor about FF-A
> > communication regardless. So it's rather about if we want to do it
> > properly leveraging auto discovery mechanisms supported by FF-A or not.
> > 
> > > 
> > > > But if we still want to overload ACPI or DT with the
> > > > discoverable firmware bits then it seems like an overkill here.
> > > 
> > > I think it would make sense to do ACPI based discovery or FF-A
> > > based discovery, but doing both I think would be overkill.  For
> > > ease of OS integration ACPI is the way to go.  And, potentially
> > > device tree in the future.
> > 
> > Encoding firmware bits in ACPI/DT can be seen as an easy upstream path
> > in the shorter run. But when the ACPI/DT becomes overloaded with
> > information that has to be passed from firmware to the OS rather than
> > purely describing hardware to the OS, it's ABI maintainability becomes
> > complex. We are already dealing with DT ABI compatibility challenges
> > especially the forward compatibility, so let's not make it even worse
> > with firmware information that can be discovered automatically.
> 
> The TCG defined ACPI table has the following:
>    -Physical address of the TPM
>    -Start method
>    -Start method specific parameters
>    -event log address
> 
> This has been in place 8+ years and this is what OSes expect.
> The start method advertises the mechanism a driver uses to
> signal the TPM that something has changed in the CRB, and
> this allows different types of TPM implementations:
>    -memory mapped
>    -signal via ACPI
>    -signal via ARM SMC (legacy)
>    -signal via Pluton mailbox
>    -signal via FF-A
> 
> I don't see this as overloading the ACPI table, it's just what
> the OS needs to know.
> 
> The TPM does not know (and should not know) the address of
> the event log. An FF-A based TPM has no way to know this.

The TPM event log is rather something that the firmware should propogate
to the OS rather than the TPM itself. The standard way for firmware to
pass that on is via UEFI [1] but still for non-UEFI compatible platforms
it gets propogated via ACPI/DT from the firmware. As UEFI gets adopted
further in embedded space, the reliance on ACPI/DT will reduce.

[1] Documentation/security/tpm/tpm_event_log.rst

> 
> I don't see how changing TPM discovery to be via FF-A directly
> would improve maintainability.

You are considering ACPI at this point but when people want to use this
TPM over FF-A on a platform using DT then it will require corresponding
DT bindings. After that each platform has to enable TPM over FF-A in
their corresponding ACPI/DT. All that won't be needed with auto
discovery over FF-A.

> 
> > The other benefit of auto discovery is that platform enablement becomes
> > really smooth. Once the firmware starts supporting a particular feature
> > like TPM over FF-A then the OS can discover and support it.
> 
> If we added new CRB/FF-A ABIs to get the CRB physical address,
> start method specific parameters, event log, it would mean that
> all OSes and hypervisors need to re-architect their CRB drivers
> or create new FF-A specific CRB drivers.  That will not smooth
> enablement for TPMs.  And I don't see advantages for
> maintainability.

We don't need to pass event log over FF-A as UEFI interface is already
there. And we are already adding support for start method over FF-A, so
all I am asking is what significant effort do you see with discovering CRB
pysical address over FF-A.

-Sumit

> 
> Thanks,
> Stuart
> 
> 

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Sudeep Holla]

Hi Sumit,

On Fri, Feb 21, 2025 at 07:16:35PM +0530, Sumit Garg wrote:
> On Mon, Feb 17, 2025 at 10:56:58AM -0600, Stuart Yoder wrote:
> >
> > I don't see how changing TPM discovery to be via FF-A directly
> > would improve maintainability.
>
> You are considering ACPI at this point but when people want to use this
> TPM over FF-A on a platform using DT then it will require corresponding
> DT bindings. After that each platform has to enable TPM over FF-A in
> their corresponding ACPI/DT. All that won't be needed with auto
> discovery over FF-A.

I hear you and completely agree. However, someone thought it was a good idea
to align with other start methods and duplicate information in the TCG ACPI
specification. This is definitely a bad idea, as it may contradict the
firmware. All we needed was a simple flag to indicate whether FF-A is the
start method.

It sounds like a classic case of misalignment between specification authors
and practical implementation needs. Instead of a simple flag to indicate FF-A
as the start method, duplicating information in the TCG ACPI specification
seems unnecessary and potentially problematic—especially if it risks
conflicting with firmware behavior.

Anyway, I can't comment on how we ended up here, but this seems to be the reality.

--
Regards,
Sudeep

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Stuart Yoder]

On 2/21/25 8:02 AM, Sudeep Holla wrote:
> Hi Sumit,
> 
> On Fri, Feb 21, 2025 at 07:16:35PM +0530, Sumit Garg wrote:
>> On Mon, Feb 17, 2025 at 10:56:58AM -0600, Stuart Yoder wrote:
>>>
>>> I don't see how changing TPM discovery to be via FF-A directly
>>> would improve maintainability.
>>
>> You are considering ACPI at this point but when people want to use this
>> TPM over FF-A on a platform using DT then it will require corresponding
>> DT bindings. After that each platform has to enable TPM over FF-A in
>> their corresponding ACPI/DT. All that won't be needed with auto
>> discovery over FF-A.

Yes, we would need a new DT binding.

> I hear you and completely agree. However, someone thought it was a good idea
> to align with other start methods and duplicate information in the TCG ACPI
> specification. This is definitely a bad idea, as it may contradict the
> firmware. All we needed was a simple flag to indicate whether FF-A is the
> start method.

Do you mean a flag exposed via ACPI?  If you do FF-A based discovery you
don't even need that.  Everything could be determined via an FF-A
interface.

> It sounds like a classic case of misalignment between specification authors
> and practical implementation needs. Instead of a simple flag to indicate FF-A
> as the start method, duplicating information in the TCG ACPI specification
> seems unnecessary and potentially problematic—especially if it risks
> conflicting with firmware behavior.

There is a lot of history, but I think it was simply that ACPI
advertisement of an FF-A based TPM seemed like the approach
with the least friction. And Linux is not the only target OS.

> Anyway, I can't comment on how we ended up here, but this seems to be the reality.

I don't think we are locked into ACPI (or DT) only discovery.
It's possible that with a modest delta on top of this patch series
that the tpm_crb driver could also probe based on FF-A.

The CRB over FF-A spec (DEN0138) could be extended in a backwards
compatible way to expose additional info like the base address of the
CRB.

Thanks,
Stuart

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Sudeep Holla]

On Fri, Feb 21, 2025 at 12:29:03PM -0600, Stuart Yoder wrote:
>
>
> On 2/21/25 8:02 AM, Sudeep Holla wrote:
> > Hi Sumit,
> >
> > On Fri, Feb 21, 2025 at 07:16:35PM +0530, Sumit Garg wrote:
> > > On Mon, Feb 17, 2025 at 10:56:58AM -0600, Stuart Yoder wrote:
> > > >
> > > > I don't see how changing TPM discovery to be via FF-A directly
> > > > would improve maintainability.
> > >
> > > You are considering ACPI at this point but when people want to use this
> > > TPM over FF-A on a platform using DT then it will require corresponding
> > > DT bindings. After that each platform has to enable TPM over FF-A in
> > > their corresponding ACPI/DT. All that won't be needed with auto
> > > discovery over FF-A.
>
> Yes, we would need a new DT binding.
>

Not sure how that would look like, so I will hold off my comments on this
topic. But we really should strive towards auto-discovery as much as possible.

> > I hear you and completely agree. However, someone thought it was a good idea
> > to align with other start methods and duplicate information in the TCG ACPI
> > specification. This is definitely a bad idea, as it may contradict the
> > firmware. All we needed was a simple flag to indicate whether FF-A is the
> > start method.
>
> Do you mean a flag exposed via ACPI?  If you do FF-A based discovery you
> don't even need that.  Everything could be determined via an FF-A
> interface.
>
> > It sounds like a classic case of misalignment between specification authors
> > and practical implementation needs. Instead of a simple flag to indicate FF-A
> > as the start method, duplicating information in the TCG ACPI specification
> > seems unnecessary and potentially problematic—especially if it risks
> > conflicting with firmware behavior.
>
> There is a lot of history, but I think it was simply that ACPI
> advertisement of an FF-A based TPM seemed like the approach
> with the least friction. And Linux is not the only target OS.
>

I guess so. I understand sometimes we need to consider multiple target OS.

> > Anyway, I can't comment on how we ended up here, but this seems to be the reality.
>
> I don't think we are locked into ACPI (or DT) only discovery.
> It's possible that with a modest delta on top of this patch series
> that the tpm_crb driver could also probe based on FF-A.
>
> The CRB over FF-A spec (DEN0138) could be extended in a backwards
> compatible way to expose additional info like the base address of the
> CRB.
>

Ideally, we should manage with dynamic buffers. But I do understand the
reasons why we may need static curve outs. I prefer the ffa client driver
take care of that without needing to build FF-A bindings just for that.

I will wait and see how all these shape up (soon ?)

--
Regards,
Sudeep

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Sudeep Holla]

On Mon, Feb 10, 2025 at 05:22:23PM -0600, Stuart Yoder wrote:
> These patches add support for the CRB FF-A start method defined
> in the TCG ACPI specification v1.4 and the FF-A ABI defined
> in the Arm TPM Service CRB over FF-A (DEN0138) specification. 
> (https://developer.arm.com/documentation/den0138/latest/)
> 
> FF-A is a messaging framework for Arm-based systems and in the
> context of the TPM driver is used to signal 'start' to a CRB-based
> TPM service which is hosted in an FF-A secure partition running in
> TrustZone.
> 
> The first patch adds an FF-A driver to handle the FF-A messaging when
> communicating with a CRB-based TPM secure partition built on FF-A.
> The driver is probed when the TPM secure partition is discovered by
> the Linux FF-A infrastructure.
>

All the FF-A related changes look good to me.

Acked-by: Sudeep Holla <sudeep.holla@arm.com>

-- 
Regards,
Sudeep

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Jarkko Sakkinen]

On Mon, Feb 10, 2025 at 05:22:23PM -0600, Stuart Yoder wrote:
> These patches add support for the CRB FF-A start method defined
> in the TCG ACPI specification v1.4 and the FF-A ABI defined
> in the Arm TPM Service CRB over FF-A (DEN0138) specification. 
> (https://developer.arm.com/documentation/den0138/latest/)


It would bring a whole a lot of clarity to open up acronyms.
What are F, F and A? There's quite a few of these in this
industry.

Also, probably you could take the spec out of parentheses and
use double colon.

BR, Jarkko 

Re: [PATCH 0/4] Add support for the TPM FF-A start method

[Stuart Yoder]

On 2/11/25 3:07 PM, Jarkko Sakkinen wrote:
> On Mon, Feb 10, 2025 at 05:22:23PM -0600, Stuart Yoder wrote:
>> These patches add support for the CRB FF-A start method defined
>> in the TCG ACPI specification v1.4 and the FF-A ABI defined
>> in the Arm TPM Service CRB over FF-A (DEN0138) specification.
>> (https://developer.arm.com/documentation/den0138/latest/)
> 
> 
> It would bring a whole a lot of clarity to open up acronyms.
> What are F, F and A? There's quite a few of these in this
> industry.
> 
> Also, probably you could take the spec out of parentheses and
> use double colon.

Will do this in v2.

Stuart