Add support for the TPM FF-A start method

PATCH Superseded Stuart Yoder 4 participants 14 messages Feb 17, 2025 – Mar 5, 2025

Subsystems (5)

Documentation ACPI ACPI component architecture (acpica) Keys-trusted +1 more subsystems

lore

Share & tools

lore

Stuart Yoder

author

2025-02-17 22:49 UTC #0 lore raw

v4 cover Add support for the TPM FF-A start method

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

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/

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.

The fifth patch adds documentation explaining how the CRB driver and FF-A relate.

Version 4
-fix warning from kernel test robot in patch 1
-fix warnings from checkpatch.pl --strict
-clean up unecessary parenthesis usage
-update variable declaration to be reverse tree order -document exported functions in tpm_crb_ffa driver -remove unnecessary author and maintainer info in tpm_crb_ffa driver -fix declaration of variables to be in reverse tree order

Version 3
-changed prefixes used throughout patch series to tpm_crb_ffa*

Version 2
-updates to cover letter to define FF-A
-added new patch with documentation
-created pull request in ACPIA and added link to the patch
updating actbl3.h
-added tpm_ prefix to the FF-A CRB driver

Stuart Yoder (5):
tpm_crb: implement driver compliant to CRB over FF-A
tpm_crb: clean-up and refactor check for idle support
ACPICA: add start method for Arm FF-A
tpm_crb: add support for the Arm FF-A start method
Documentation: tpm: add documentation for the CRB FF-A interface

 Documentation/security/tpm/tpm_ffa_crb.rst |  65 ++++
 drivers/char/tpm/Kconfig                   |   9 +
 drivers/char/tpm/Makefile                  |   1 +
 drivers/char/tpm/tpm_crb.c                 | 105 +++++--
 drivers/char/tpm/tpm_crb_ffa.c             | 348 +++++++++++++++++++++
 drivers/char/tpm/tpm_crb_ffa.h             |  25 ++
 include/acpi/actbl3.h                      |   1 +
 7 files changed, 535 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/security/tpm/tpm_ffa_crb.rst
 create mode 100644 drivers/char/tpm/tpm_crb_ffa.c
 create mode 100644 drivers/char/tpm/tpm_crb_ffa.h

Signature · 1 line

2.34.1

Stuart Yoder

author

2025-02-17 22:49 UTC #1 lore raw

PATCH v4 1/5 tpm_crb: implement driver compliant to CRB over FF-A

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 <[email protected]>

---
 drivers/char/tpm/Kconfig       |   9 +
 drivers/char/tpm/Makefile      |   1 +
 drivers/char/tpm/tpm_crb_ffa.c | 348 +++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm_crb_ffa.h |  25 +++
 4 files changed, 383 insertions(+)
 create mode 100644 drivers/char/tpm/tpm_crb_ffa.c
 create mode 100644 drivers/char/tpm/tpm_crb_ffa.h

drivers/char/tpm/Kconfig +9 -0

index 0fc9a510e059..4c85b8c00b12 100644
@@ -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_CRB_FFA
+	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 tpm_crb_ffa.
+
 config TCG_VTPM_PROXY
 	tristate "VTPM Proxy Interface"
 	depends on TCG_TPM

drivers/char/tpm/Makefile +1 -0

index 9bb142c75243..2b004df8c04b 100644
@@ -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_CRB_FFA) += tpm_crb_ffa.o
 obj-$(CONFIG_TCG_VTPM_PROXY) += tpm_vtpm_proxy.o
 obj-$(CONFIG_TCG_FTPM_TEE) += tpm_ftpm_tee.o

drivers/char/tpm/tpm_crb_ffa.c new file +348 -0

348 lines not on this page · raw message

drivers/char/tpm/tpm_crb_ffa.h new file +25 -0

new file mode 100644
index 000000000000..23a9b93287a1
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2024 Arm Ltd.
+ *
+ * This device driver implements the TPM CRB start method
+ * as defined in the TPM Service Command Response Buffer
+ * Interface Over FF-A (DEN0138).
+ */
+#ifndef _TPM_CRB_FFA_H
+#define _TPM_CRB_FFA_H
+
+#if IS_ENABLED(CONFIG_TCG_ARM_CRB_FFA)
+int tpm_crb_ffa_init(void);
+int tpm_crb_ffa_get_interface_version(u16 *major, u16 *minor);
+int tpm_crb_ffa_start(int request_type, int locality);
+#else
+static inline int tpm_crb_ffa_init(void) { return 0; }
+static inline int tpm_crb_ffa_get_interface_version(u16 *major, u16 *minor) { return 0; }
+static inline int tpm_crb_ffa_start(int request_type, int locality) { return 0; }
+#endif
+
+#define CRB_FFA_START_TYPE_COMMAND 0
+#define CRB_FFA_START_TYPE_LOCALITY_REQUEST 1
+
+#endif

Signature · 1 line

2.34.1

Jarkko Sakkinen

maintainer

2025-02-18 15:08 UTC #2 lore raw

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

Quoted 64 lines · 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 <[email protected]>
---
 drivers/char/tpm/Kconfig       |   9 +
 drivers/char/tpm/Makefile      |   1 +
 drivers/char/tpm/tpm_crb_ffa.c | 348
+++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm_crb_ffa.h |  25 +++
 4 files changed, 383 insertions(+)
 create mode 100644 drivers/char/tpm/tpm_crb_ffa.c
 create mode 100644 drivers/char/tpm/tpm_crb_ffa.h
drivers/char/tpm/Kconfig +0 -0
index 0fc9a510e059..4c85b8c00b12 100644
@@ -210,6 +210,15 @@ config TCG_CRB
 	  from within Linux.  To compile this driver as a module,
choose
drivers/char/tpm/Kconfig +6 -0
 	  M here; the module will be called tpm_crb.
 
+config TCG_ARM_CRB_FFA
+	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.
drivers/char/tpm/Kconfig +1 -0
+	  To compile this driver as a module, choose M here; the
module
drivers/char/tpm/Kconfig +2 -0
+	  will be called tpm_crb_ffa.
+
 config TCG_VTPM_PROXY
 	tristate "VTPM Proxy Interface"
 	depends on TCG_TPM
drivers/char/tpm/Makefile +1 -0
index 9bb142c75243..2b004df8c04b 100644
@@ -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_CRB_FFA) += tpm_crb_ffa.o
 obj-$(CONFIG_TCG_VTPM_PROXY) += tpm_vtpm_proxy.o
 obj-$(CONFIG_TCG_FTPM_TEE) += tpm_ftpm_tee.o
drivers/char/tpm/tpm_crb_ffa.c new file +0 -0
new file mode 100644
index 000000000000..bd3c6c41264b

drivers/char/tpm/tpm_crb_ffa.c line 3

Quoted 3 lines

drivers/char/tpm/tpm_crb_ffa.c new file +3 -0

@@ -0,0 +1,348 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 Arm Ltd.

Agreed that, this type of copyright makes sense. It would be harder to decipher the data from the commit log.

Also one corner case where I plan to use copyrighta in-file personally:
I'm mainlining an OOT driver with a bunch of contributors over time, and in that I'll list all the historical contributors with appropriate copyright platters :-)

Generally speaking, I think can be added as long as it has an actual function that brings measurable value.

Quoted 114 lines

drivers/char/tpm/tpm_crb_ffa.c new file +29 -0
+ *
+ * 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) "CRB_FFA: " fmt
+
+#include <linux/arm_ffa.h>
+#include "tpm_crb_ffa.h"
+
+/* TPM service function status codes */
+#define CRB_FFA_OK			0x05000001
+#define CRB_FFA_OK_RESULTS_RETURNED	0x05000002
+#define CRB_FFA_NOFUNC			0x8e000001
+#define CRB_FFA_NOTSUP			0x8e000002
+#define CRB_FFA_INVARG			0x8e000005
+#define CRB_FFA_INV_CRB_CTRL_DATA	0x8e000006
+#define CRB_FFA_ALREADY			0x8e000009
+#define CRB_FFA_DENIED			0x8e00000a
+#define CRB_FFA_NOMEM			0x8e00000b
+
+#define CRB_FFA_VERSION_MAJOR	1
+#define CRB_FFA_VERSION_MINOR	0
+
+/* version encoding */
+#define CRB_FFA_MAJOR_VERSION_MASK  GENMASK(30, 16)
+#define CRB_FFA_MINOR_VERSION_MASK  GENMASK(15, 0)
+#define CRB_FFA_MAJOR_VERSION(x)   
((u16)(FIELD_GET(CRB_FFA_MAJOR_VERSION_MASK, (x)))) +#define CRB_FFA_MINOR_VERSION(x) ((u16)(FIELD_GET(CRB_FFA_MINOR_VERSION_MASK, (x))))
drivers/char/tpm/tpm_crb_ffa.c new file +7 -0
+
+/*
+ * 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
+ */
drivers/char/tpm/tpm_crb_ffa.c new file +18 -0
+
+/*
+ * Returns the version of the interface that is available
+ * Call register usage:
+ * w3:    Not used (MBZ)
+ * w4:    TPM service function ID, CRB_FFA_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.
+ */
drivers/char/tpm/tpm_crb_ffa.c new file +11 -0
+#define CRB_FFA_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, CRB_FFA_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:
+ *     CRB_FFA_OK: the TPM service has been notified successfully
+ *     CRB_FFA_INVARG: one or more arguments are not valid
+ *     CRB_FFA_INV_CRB_CTRL_DATA: CRB control data or locality
control
+ * data at the given TPM locality is not valid
+ * CRB_FFA_DENIED: the TPM has previously disabled locality
requests and
+ * command processing at the given locality
+ */
drivers/char/tpm/tpm_crb_ffa.c new file +7 -0
+#define CRB_FFA_START 0x0f000201
+
+struct tpm_crb_ffa {
+	struct ffa_device *ffa_dev;
+	u16 major_version;
+	u16 minor_version;
+	struct mutex msg_data_lock; /* lock to protect sending of

Quoted 1 line

FF-A messages */

Nit:

	/* Lock to protect sending of FFA-messages: */
	struct mutex msg_data_lock;

It is a bit more readable and double comma makes pretty obvious which field it is pointing at.

Quoted 252 lines

drivers/char/tpm/tpm_crb_ffa.c new file +46 -0

+	struct ffa_send_direct_data direct_msg_data;
+};
+
+static struct tpm_crb_ffa *tpm_crb_ffa;
+
+static int tpm_crb_ffa_to_linux_errno(int errno)
+{
+	int rc;
+
+	switch (errno) {
+	case CRB_FFA_OK:
+		rc = 0;
+		break;
+	case CRB_FFA_OK_RESULTS_RETURNED:
+		rc = 0;
+		break;
+	case CRB_FFA_NOFUNC:
+		rc = -ENOENT;
+		break;
+	case CRB_FFA_NOTSUP:
+		rc = -EPERM;
+		break;
+	case CRB_FFA_INVARG:
+		rc = -EINVAL;
+		break;
+	case CRB_FFA_INV_CRB_CTRL_DATA:
+		rc = -ENOEXEC;
+		break;
+	case CRB_FFA_ALREADY:
+		rc = -EEXIST;
+		break;
+	case CRB_FFA_DENIED:
+		rc = -EACCES;
+		break;
+	case CRB_FFA_NOMEM:
+		rc = -ENOMEM;
+		break;
+	default:
+		rc = -EINVAL;
+	}
+
+	return rc;
+}
+
+/**
+ * tpm_crb_ffa_init - called by the CRB driver to do any needed

initialization
+ *
+ * This function is called by the tpm_crb driver during the tpm_crb
+ * driver's initialization. If the tpm_crb_ffa has not been probed
+ * yet, returns -ENOENT in order to force a retry. If th ffa_crb
+ * driver had been probed but failed with an error, returns -ENODEV
+ * in order to prevent further retries.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */

drivers/char/tpm/tpm_crb_ffa.c new file +37 -0

+int tpm_crb_ffa_init(void)
+{
+	if (!tpm_crb_ffa)
+		return -ENOENT;
+
+	if (IS_ERR_VALUE(tpm_crb_ffa))
+		return -ENODEV;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
+
+static int __tpm_crb_ffa_send_recieve(unsigned long func_id,
+				      unsigned long a0,
+				      unsigned long a1,
+				      unsigned long a2)
+{
+	const struct ffa_msg_ops *msg_ops;
+	int ret;
+
+	if (!tpm_crb_ffa)
+		return -ENOENT;
+
+	msg_ops = tpm_crb_ffa->ffa_dev->ops->msg_ops;
+
+	memset(&tpm_crb_ffa->direct_msg_data, 0x00,
+	       sizeof(struct ffa_send_direct_data));
+
+	tpm_crb_ffa->direct_msg_data.data1 = func_id;
+	tpm_crb_ffa->direct_msg_data.data2 = a0;
+	tpm_crb_ffa->direct_msg_data.data3 = a1;
+	tpm_crb_ffa->direct_msg_data.data4 = a2;
+
+	ret = msg_ops->sync_send_receive(tpm_crb_ffa->ffa_dev,
+			&tpm_crb_ffa->direct_msg_data);
+	if (!ret)
+		ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa-

Quoted 1 line

direct_msg_data.data1);

drivers/char/tpm/tpm_crb_ffa.c new file +6 -0

+
+	return ret;
+}
+
+/**
+ * tpm_crb_ffa_get_interface_version() - gets the ABI version of the

TPM service
+ * @major: Pointer to caller-allocated buffer to hold the major version
+ * number the ABI
+ * @minor: Pointer to caller-allocated buffer to hold the minor version
+ * number the ABI
+ *
+ * Returns the major and minor version of the ABI of the FF-A based TPM.
+ * Allows the caller to evaluate its compatibility with the version of
+ * the ABI.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */

drivers/char/tpm/tpm_crb_ffa.c new file +16 -0

+int tpm_crb_ffa_get_interface_version(u16 *major, u16 *minor)
+{
+	int rc;
+
+	if (!tpm_crb_ffa)
+		return -ENOENT;
+
+	if (IS_ERR_VALUE(tpm_crb_ffa))
+		return -ENODEV;
+
+	if (!major || !minor)
+		return -EINVAL;
+
+	guard(mutex)(&tpm_crb_ffa->msg_data_lock);
+
+	rc =

__tpm_crb_ffa_send_recieve(CRB_FFA_GET_INTERFACE_VERSION, 0x00, 0x00, 0x00);

drivers/char/tpm/tpm_crb_ffa.c new file +2 -0

+	if (!rc) {
+		*major = CRB_FFA_MAJOR_VERSION(tpm_crb_ffa-

Quoted 1 line

direct_msg_data.data2);

drivers/char/tpm/tpm_crb_ffa.c new file +1 -0

+		*minor = CRB_FFA_MINOR_VERSION(tpm_crb_ffa-

Quoted 1 line

direct_msg_data.data2);

drivers/char/tpm/tpm_crb_ffa.c new file +8 -0

+	}
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(tpm_crb_ffa_get_interface_version);
+
+/**
+ * tpm_crb_ffa_start() - signals the TPM that a field has changed in

the CRB
+ * @request_type: Identifies whether the change to the CRB is in the command
+ * fields or locality fields.
+ * @locality: Specifies the locality number.
+ *
+ * Used by the CRB driver
+ * that might be useful to those using or modifying it. Begins with
+ * empty comment line, and may include additional embedded empty
+ * comment lines.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */

drivers/char/tpm/tpm_crb_ffa.c new file +11 -0

+int tpm_crb_ffa_start(int request_type, int locality)
+{
+	if (!tpm_crb_ffa)
+		return -ENOENT;
+
+	if (IS_ERR_VALUE(tpm_crb_ffa))
+		return -ENODEV;
+
+	guard(mutex)(&tpm_crb_ffa->msg_data_lock);
+
+	return __tpm_crb_ffa_send_recieve(CRB_FFA_START,

request_type, locality, 0x00);

drivers/char/tpm/tpm_crb_ffa.c new file +13 -0

+}
+EXPORT_SYMBOL_GPL(tpm_crb_ffa_start);
+
+static int tpm_crb_ffa_probe(struct ffa_device *ffa_dev)
+{
+	struct tpm_crb_ffa *p;
+	int rc;
+
+	/* only one instance of a TPM partition is supported */
+	if (tpm_crb_ffa && !IS_ERR_VALUE(tpm_crb_ffa))
+		return -EEXIST;
+
+	tpm_crb_ffa = ERR_PTR(-ENODEV); // set tpm_crb_ffa so we can

detect probe failure

drivers/char/tpm/tpm_crb_ffa.c new file +3 -0

+
+	if (!ffa_partition_supports_direct_recv(ffa_dev)) {
+		pr_err("TPM partition doesn't support direct message

receive.\n");

drivers/char/tpm/tpm_crb_ffa.c new file +14 -0

+		return -EINVAL;
+	}
+
+	p = kzalloc(sizeof(*tpm_crb_ffa), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+	tpm_crb_ffa = p;
+
+	mutex_init(&tpm_crb_ffa->msg_data_lock);
+	tpm_crb_ffa->ffa_dev = ffa_dev;
+	ffa_dev_set_drvdata(ffa_dev, tpm_crb_ffa);
+
+	/* if TPM is aarch32 use 32-bit SMCs */
+	if (!ffa_partition_check_property(ffa_dev,

FFA_PARTITION_AARCH64_EXEC))

drivers/char/tpm/tpm_crb_ffa.c new file +4 -0

+		ffa_dev->ops->msg_ops->mode_32bit_set(ffa_dev);
+
+	/* verify compatibility of TPM service version number */
+	rc = tpm_crb_ffa_get_interface_version(&tpm_crb_ffa-

Quoted 1 line

major_version,

drivers/char/tpm/tpm_crb_ffa.c new file +1 -0

+					       &tpm_crb_ffa-

Quoted 1 line

minor_version);

drivers/char/tpm/tpm_crb_ffa.c new file +2 -0

+	if (rc) {
+		pr_err("failed to get crb interface version. rc:%d",

rc);

drivers/char/tpm/tpm_crb_ffa.c new file +31 -0

+		goto out;
+	}
+
+	pr_info("ABI version %u.%u", tpm_crb_ffa->major_version,
+		tpm_crb_ffa->minor_version);
+
+	if (tpm_crb_ffa->major_version != CRB_FFA_VERSION_MAJOR ||
+	   (tpm_crb_ffa->minor_version > 0 &&
+	    tpm_crb_ffa->minor_version < CRB_FFA_VERSION_MINOR)) {
+		pr_err("Incompatible ABI version");
+		goto out;
+	}
+
+	return 0;
+
+out:
+	kfree(tpm_crb_ffa);
+	tpm_crb_ffa = ERR_PTR(-ENODEV);
+	return -EINVAL;
+}
+
+static void tpm_crb_ffa_remove(struct ffa_device *ffa_dev)
+{
+	kfree(tpm_crb_ffa);
+	tpm_crb_ffa = NULL;
+}
+
+static const struct ffa_device_id tpm_crb_ffa_device_id[] = {
+	/* 17b862a4-1806-4faf-86b3-089a58353861 */
+	{ UUID_INIT(0x17b862a4, 0x1806, 0x4faf,
+		    0x86, 0xb3, 0x08, 0x9a, 0x58, 0x35, 0x38, 0x61)

drivers/char/tpm/tpm_crb_ffa.c new file +5 -0

+	{}
+};
+
+static struct ffa_driver tpm_crb_ffa_driver = {
+	.name = "ffa-crb",

drivers/char/tpm/tpm_crb_ffa.c line 347

Quoted 10 lines

drivers/char/tpm/tpm_crb_ffa.c new file +10 -0

+	.probe = tpm_crb_ffa_probe,
+	.remove = tpm_crb_ffa_remove,
+	.id_table = tpm_crb_ffa_device_id,
+};
+
+module_ffa_driver(tpm_crb_ffa_driver);
+
+MODULE_AUTHOR("Arm");
+MODULE_DESCRIPTION("FFA CRB driver");
+MODULE_VERSION("1.0");

Take this with a grain of salt, i.e. I'm not going to NAK for this but I would not personally add MODULE_VERSION() for new drivers as it does not have any function.

Quoted 29 lines

drivers/char/tpm/tpm_crb_ffa.c new file +1 -0

+MODULE_LICENSE("GPL");

drivers/char/tpm/tpm_crb_ffa.h new file +18 -0

new file mode 100644
index 000000000000..23a9b93287a1
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2024 Arm Ltd.
+ *
+ * This device driver implements the TPM CRB start method
+ * as defined in the TPM Service Command Response Buffer
+ * Interface Over FF-A (DEN0138).
+ */
+#ifndef _TPM_CRB_FFA_H
+#define _TPM_CRB_FFA_H
+
+#if IS_ENABLED(CONFIG_TCG_ARM_CRB_FFA)
+int tpm_crb_ffa_init(void);
+int tpm_crb_ffa_get_interface_version(u16 *major, u16 *minor);
+int tpm_crb_ffa_start(int request_type, int locality);
+#else
+static inline int tpm_crb_ffa_init(void) { return 0; }
+static inline int tpm_crb_ffa_get_interface_version(u16 *major, u16

*minor) { return 0; }
+static inline int tpm_crb_ffa_start(int request_type, int locality) { return 0; }

drivers/char/tpm/tpm_crb_ffa.h line 25

Quoted 6 lines

drivers/char/tpm/tpm_crb_ffa.h new file +6 -0

+#endif
+
+#define CRB_FFA_START_TYPE_COMMAND 0
+#define CRB_FFA_START_TYPE_LOCALITY_REQUEST 1
+
+#endif

BR, Jarkko

Stuart Yoder

author

2025-02-17 22:49 UTC #3 lore raw

PATCH v4 2/5 tpm_crb: clean-up and refactor check for idle support

In reply to Stuart Yoder

-Clean up unnecessary parenthesis usage
-Refactor the two checks for whether the TPM supports idle into a single
inline function.

Signed-off-by Stuart Yoder <[email protected]>

---
 drivers/char/tpm/tpm_crb.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

drivers/char/tpm/tpm_crb.c +21 ~~-15~~

index ea085b14ab7c..31db879f1324 100644
@@ -115,6 +115,16 @@ struct tpm2_crb_pluton {
 	u64 reply_addr;
 };
 
+/*
+ * Returns true if the start method supports idle.
+ */
+static inline bool tpm_crb_has_idle(u32 start_method)
+{
+	return start_method == ACPI_TPM2_START_METHOD ||
+	       start_method == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD ||
+	       start_method == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC;
+}
+
 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 (!tpm_crb_has_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 (!tpm_crb_has_idle(priv->sm))
 		return 0;
 
 	iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->regs_t->ctrl_req);

@@ -423,13 +429,13 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
 	 * report only ACPI start but in practice seems to require both
 	 * CRB start, hence invoking CRB start method if hid == MSFT0101.
 	 */
-	if ((priv->sm == ACPI_TPM2_COMMAND_BUFFER) ||
-	    (priv->sm == ACPI_TPM2_MEMORY_MAPPED) ||
-	    (!strcmp(priv->hid, "MSFT0101")))
+	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER ||
+	    priv->sm == ACPI_TPM2_MEMORY_MAPPED ||
+	    !strcmp(priv->hid, "MSFT0101"))
 		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
 
-	if ((priv->sm == ACPI_TPM2_START_METHOD) ||
-	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD))
+	if (priv->sm == ACPI_TPM2_START_METHOD ||
+	    priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
 		rc = crb_do_acpi_start(chip);
 
 	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {

@@ -449,8 +455,8 @@ static void crb_cancel(struct tpm_chip *chip)
 
 	iowrite32(CRB_CANCEL_INVOKE, &priv->regs_t->ctrl_cancel);
 
-	if (((priv->sm == ACPI_TPM2_START_METHOD) ||
-	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)) &&
+	if ((priv->sm == ACPI_TPM2_START_METHOD ||
+	     priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD) &&
 	     crb_do_acpi_start(chip))
 		dev_err(&chip->dev, "ACPI Start failed\n");
 }

@@ -609,8 +615,8 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 	 * the control area, as one nice sane region except for some older
 	 * stuff that puts the control area outside the ACPI IO region.
 	 */
-	if ((priv->sm == ACPI_TPM2_COMMAND_BUFFER) ||
-	    (priv->sm == ACPI_TPM2_MEMORY_MAPPED)) {
+	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER ||
+	    priv->sm == ACPI_TPM2_MEMORY_MAPPED) {
 		if (iores &&
 		    buf->control_address == iores->start +
 		    sizeof(*priv->regs_h))

Signature · 1 line

2.34.1

Jarkko Sakkinen

2025-02-18 15:52 UTC #4 lore raw

Re: [PATCH v4 2/5] tpm_crb: clean-up and refactor check for idle support

Quoted 4 lines · Stuart Yoder

(quoted diff) +0 -2

-Clean up unnecessary parenthesis usage
-Refactor the two checks for whether the TPM supports idle into a

single
inline function.

IMHO, this would be better:

"Refactor TPM idle check to tpm_crb_has_idle(), and reduce paraentheses usage in start method checks."

Quoted 104 lines

Signed-off-by Stuart Yoder <[email protected]>
---
drivers/char/tpm/tpm_crb.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)
drivers/char/tpm/tpm_crb.c +0 -0
index ea085b14ab7c..31db879f1324 100644
@@ -115,6 +115,16 @@ struct tpm2_crb_pluton {
  	u64 reply_addr;
  };
drivers/char/tpm/tpm_crb.c +7 -0
+/*
+ * Returns true if the start method supports idle.
+ */
+static inline bool tpm_crb_has_idle(u32 start_method)
+{
+	return start_method == ACPI_TPM2_START_METHOD ||
+	       start_method ==
ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD ||
drivers/char/tpm/tpm_crb.c +1 -0
+	       start_method ==
ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC;
drivers/char/tpm/tpm_crb.c +2 -0
+}
+
static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
unsigned long timeout) {
drivers/char/tpm/tpm_crb.c +0 -0
@@ -173,9 +183,7 @@ static int __crb_go_idle(struct device *dev,
 struct crb_priv *priv)
  {
  	int rc;
drivers/char/tpm/tpm_crb.c +0 -2
-	if ((priv->sm == ACPI_TPM2_START_METHOD) ||
-	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
||
drivers/char/tpm/tpm_crb.c +1 -1
-	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC))
+	if (!tpm_crb_has_idle(priv->sm))
return 0;

iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->regs_t->ctrl_req);
drivers/char/tpm/tpm_crb.c +0 -0
@@ -222,9 +230,7 @@ static int __crb_cmd_ready(struct device *dev,
 struct crb_priv *priv)
  {
  	int rc;
drivers/char/tpm/tpm_crb.c +0 -2
-	if ((priv->sm == ACPI_TPM2_START_METHOD) ||
-	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
||
drivers/char/tpm/tpm_crb.c +1 -1
-	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC))
+	if (!tpm_crb_has_idle(priv->sm))
return 0;

iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->regs_t->ctrl_req);
drivers/char/tpm/tpm_crb.c +0 -0
@@ -423,13 +429,13 @@ static int crb_send(struct tpm_chip *chip, u8
 *buf, size_t len)
  	 * report only ACPI start but in practice seems to require
both
* CRB start, hence invoking CRB start method if hid == MSFT0101.
*/
drivers/char/tpm/tpm_crb.c +3 -3
-	if ((priv->sm == ACPI_TPM2_COMMAND_BUFFER) ||
-	    (priv->sm == ACPI_TPM2_MEMORY_MAPPED) ||
-	    (!strcmp(priv->hid, "MSFT0101")))
+	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER ||
+	    priv->sm == ACPI_TPM2_MEMORY_MAPPED ||
+	    !strcmp(priv->hid, "MSFT0101"))
iowrite32(CRB_START_INVOKE, &priv->regs_t-
Quoted 1 line
ctrl_start);
drivers/char/tpm/tpm_crb.c +0 -2
-	if ((priv->sm == ACPI_TPM2_START_METHOD) ||
-	    (priv->sm ==
ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD))
drivers/char/tpm/tpm_crb.c +2 -0
+	if (priv->sm == ACPI_TPM2_START_METHOD ||
+	    priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
rc = crb_do_acpi_start(chip);

if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
drivers/char/tpm/tpm_crb.c +0 -0
@@ -449,8 +455,8 @@ static void crb_cancel(struct tpm_chip *chip)
iowrite32(CRB_CANCEL_INVOKE, &priv->regs_t->ctrl_cancel);
drivers/char/tpm/tpm_crb.c +0 -2
-	if (((priv->sm == ACPI_TPM2_START_METHOD) ||
-	    (priv->sm ==
ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)) &&
drivers/char/tpm/tpm_crb.c +2 -0
+	if ((priv->sm == ACPI_TPM2_START_METHOD ||
+	     priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
&&
crb_do_acpi_start(chip)) dev_err(&chip->dev, "ACPI Start failed\n"); }
drivers/char/tpm/tpm_crb.c +0 -0
@@ -609,8 +615,8 @@ static int crb_map_io(struct acpi_device *device,
 struct crb_priv *priv,
  	 * the control area, as one nice sane region except for some
older
* stuff that puts the control area outside the ACPI IO region.
*/
drivers/char/tpm/tpm_crb.c +2 -2
-	if ((priv->sm == ACPI_TPM2_COMMAND_BUFFER) ||
-	    (priv->sm == ACPI_TPM2_MEMORY_MAPPED)) {
+	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER ||
+	    priv->sm == ACPI_TPM2_MEMORY_MAPPED) {

Quoted 3 lines

if (iores &&
buf->control_address == iores->start + sizeof(*priv->regs_h))

BR, Jarkko

Stuart Yoder

author

2025-02-17 22:49 UTC #5 lore raw

PATCH v4 3/5 ACPICA: add start method for Arm FF-A

In reply to Stuart Yoder

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

Link https://github.com/acpica/acpica/pull/1000

Signed-off-by Stuart Yoder <[email protected]>

---
 include/acpi/actbl3.h | 1 +
 1 file changed, 1 insertion(+)

include/acpi/actbl3.h +1 -0

index 5cd755143b7d..a97b1dbab975 100644
@@ -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 */

Signature · 1 line

2.34.1

Jarkko Sakkinen

Reviewed-by on v4 3/5

2025-02-18 15:53 UTC #6 lore raw

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

Quoted 15 lines · Stuart Yoder

Add TPM start method for Arm FF-A defined in the TCG ACPI specification v1.4.
Link https://github.com/acpica/acpica/pull/1000
Signed-off-by Stuart Yoder <[email protected]>
---
include/acpi/actbl3.h | 1 +
 1 file changed, 1 insertion(+)
include/acpi/actbl3.h +0 -0
index 5cd755143b7d..a97b1dbab975 100644
@@ -466,6 +466,7 @@ struct acpi_tpm2_phy {
  #define ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC       11	/* V1.2 Rev

Quoted 7 lines

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 */

Reviewed-by Jarkko Sakkinen <[email protected]>

BR, Jarkko

Stuart Yoder

author

2025-02-17 22:49 UTC #7 lore raw

PATCH v4 4/5 tpm_crb: add support for the Arm FF-A start method

In reply to 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 <[email protected]>

---
 drivers/char/tpm/tpm_crb.c | 71 +++++++++++++++++++++++++++++++++++---
 1 file changed, 66 insertions(+), 5 deletions(-)

drivers/char/tpm/tpm_crb.c +66 -5

index 31db879f1324..2a57650ba9b4 100644
@@ -19,6 +19,7 @@
 #ifdef CONFIG_ARM64
 #include <linux/arm-smccc.h>
 #endif
+#include "tpm_crb_ffa.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;

@@ -122,7 +133,8 @@ static inline bool tpm_crb_has_idle(u32 start_method)
 {
 	return 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;
 }
 
 static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,

@@ -261,13 +273,20 @@ static int crb_cmd_ready(struct tpm_chip *chip)
 static int __crb_request_locality(struct device *dev,
 				  struct crb_priv *priv, int loc)
 {
-	u32 value = CRB_LOC_STATE_LOC_ASSIGNED |
-		    CRB_LOC_STATE_TPM_REG_VALID_STS;
+	u32 value = CRB_LOC_STATE_LOC_ASSIGNED | CRB_LOC_STATE_TPM_REG_VALID_STS;
+	int rc;
 
 	if (!priv->regs_h)
 		return 0;
 
 	iowrite32(CRB_LOC_CTRL_REQUEST_ACCESS, &priv->regs_h->loc_ctrl);
+
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		rc = tpm_crb_ffa_start(CRB_FFA_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,14 +306,21 @@ 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)
 {
-	u32 mask = CRB_LOC_STATE_LOC_ASSIGNED |
-		   CRB_LOC_STATE_TPM_REG_VALID_STS;
+	u32 mask = CRB_LOC_STATE_LOC_ASSIGNED | CRB_LOC_STATE_TPM_REG_VALID_STS;
 	u32 value = CRB_LOC_STATE_TPM_REG_VALID_STS;
+	int rc;
 
 	if (!priv->regs_h)
 		return 0;
 
 	iowrite32(CRB_LOC_CTRL_RELINQUISH, &priv->regs_h->loc_ctrl);
+
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		rc = tpm_crb_ffa_start(CRB_FFA_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 +469,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 = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND, chip->locality);
+	}
+
 	if (rc)
 		return rc;

@@ -452,6 +483,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
 static void crb_cancel(struct tpm_chip *chip)
 {
 	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+	int rc;
 
 	iowrite32(CRB_CANCEL_INVOKE, &priv->regs_t->ctrl_cancel);

@@ -459,6 +491,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 = tpm_crb_ffa_start(CRB_FFA_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 +654,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 +776,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 +815,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 = tpm_crb_ffa_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,

Signature · 1 line

2.34.1

Jarkko Sakkinen

Reviewed-by on v4 4/5

2025-02-18 15:54 UTC #8 lore raw

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

Quoted 210 lines · 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 <[email protected]>
---
drivers/char/tpm/tpm_crb.c | 71 +++++++++++++++++++++++++++++++++++- --
 1 file changed, 66 insertions(+), 5 deletions(-)
drivers/char/tpm/tpm_crb.c +1 -0
index 31db879f1324..2a57650ba9b4 100644
@@ -19,6 +19,7 @@
  #ifdef CONFIG_ARM64
  #include <linux/arm-smccc.h>
  #endif
+#include "tpm_crb_ffa.h"
  #include "tpm.h"
#define ACPI_SIG_TPM2 "TPM2"
drivers/char/tpm/tpm_crb.c +2 -0
@@ -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 {
drivers/char/tpm/tpm_crb.c +0 -0
@@ -110,6 +113,14 @@ struct tpm2_crb_smc {
  	u32 smc_func_id;
  };
drivers/char/tpm/tpm_crb.c +8 -0
+/* 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;
drivers/char/tpm/tpm_crb.c +0 -0
@@ -122,7 +133,8 @@ static inline bool tpm_crb_has_idle(u32
 start_method)
  {
return start_method == ACPI_TPM2_START_METHOD || start_method == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD ||
drivers/char/tpm/tpm_crb.c +0 -1
-	       start_method ==
ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC;
drivers/char/tpm/tpm_crb.c +1 -0
+	       start_method == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC
||
drivers/char/tpm/tpm_crb.c +1 -0
+	       start_method == ACPI_TPM2_CRB_WITH_ARM_FFA;
}

static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
drivers/char/tpm/tpm_crb.c +2 -2
@@ -261,13 +273,20 @@ static int crb_cmd_ready(struct tpm_chip *chip)
  static int __crb_request_locality(struct device *dev,
  				  struct crb_priv *priv, int loc)
  {
-	u32 value = CRB_LOC_STATE_LOC_ASSIGNED |
-		    CRB_LOC_STATE_TPM_REG_VALID_STS;
+	u32 value = CRB_LOC_STATE_LOC_ASSIGNED |
 CRB_LOC_STATE_TPM_REG_VALID_STS;
+	int rc;
if (!priv->regs_h)
return 0;

iowrite32(CRB_LOC_CTRL_REQUEST_ACCESS, &priv->regs_h-
Quoted 1 line
loc_ctrl);
drivers/char/tpm/tpm_crb.c +3 -0
+
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		rc =
tpm_crb_ffa_start(CRB_FFA_START_TYPE_LOCALITY_REQUEST, loc);
drivers/char/tpm/tpm_crb.c +4 -0
+		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");
drivers/char/tpm/tpm_crb.c +2 -2
@@ -287,14 +306,21 @@ 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)
  {
-	u32 mask = CRB_LOC_STATE_LOC_ASSIGNED |
-		   CRB_LOC_STATE_TPM_REG_VALID_STS;
+	u32 mask = CRB_LOC_STATE_LOC_ASSIGNED |
 CRB_LOC_STATE_TPM_REG_VALID_STS;
  	u32 value = CRB_LOC_STATE_TPM_REG_VALID_STS;
+	int rc;
if (!priv->regs_h)
return 0;

iowrite32(CRB_LOC_CTRL_RELINQUISH, &priv->regs_h->loc_ctrl);
drivers/char/tpm/tpm_crb.c +3 -0
+
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		rc =
tpm_crb_ffa_start(CRB_FFA_START_TYPE_LOCALITY_REQUEST, loc);
drivers/char/tpm/tpm_crb.c +4 -0
+		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");
drivers/char/tpm/tpm_crb.c +0 -0
@@ -443,6 +469,11 @@ static int crb_send(struct tpm_chip *chip, u8
 *buf, size_t len)
rc = tpm_crb_smc_start(&chip->dev, priv-
Quoted 1 line
smc_func_id);
}
drivers/char/tpm/tpm_crb.c +2 -0
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		iowrite32(CRB_START_INVOKE, &priv->regs_t-
Quoted 1 line
ctrl_start);
drivers/char/tpm/tpm_crb.c +1 -0
+		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND,
chip->locality);
drivers/char/tpm/tpm_crb.c +2 -0
+	}
+
if (rc)
return rc;
drivers/char/tpm/tpm_crb.c +1 -0
@@ -452,6 +483,7 @@ static int crb_send(struct tpm_chip *chip, u8
 *buf, size_t len)
  static void crb_cancel(struct tpm_chip *chip)
  {
  	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+	int rc;
iowrite32(CRB_CANCEL_INVOKE, &priv->regs_t->ctrl_cancel);
drivers/char/tpm/tpm_crb.c +0 -0
@@ -459,6 +491,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");
drivers/char/tpm/tpm_crb.c +3 -0
+
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND,
chip->locality);
drivers/char/tpm/tpm_crb.c +3 -0
+		if (rc)
+			dev_err(&chip->dev, "FF-A Start failed\n");
+	}
}

static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
drivers/char/tpm/tpm_crb.c +0 -0
@@ -616,6 +654,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 ||
drivers/char/tpm/tpm_crb.c +1 -0
+	    priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA ||
priv->sm == ACPI_TPM2_MEMORY_MAPPED) { if (iores &&
buf->control_address == iores->start +
drivers/char/tpm/tpm_crb.c +1 -0
@@ -737,6 +776,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;
drivers/char/tpm/tpm_crb.c +0 -0
@@ -775,6 +815,27 @@ static int crb_acpi_add(struct acpi_device
 *device)
  		priv->smc_func_id = crb_smc->smc_func_id;
  	}
drivers/char/tpm/tpm_crb.c +2 -0
+	if (sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		if (buf->header.length < (sizeof(*buf) +
sizeof(*crb_ffa))) {
drivers/char/tpm/tpm_crb.c +2 -0
+			dev_err(dev,
+				FW_BUG "TPM2 ACPI table has wrong
size %u for start method type %d\n",
drivers/char/tpm/tpm_crb.c +6 -0
+				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));
drivers/char/tpm/tpm_crb.c +5 -0
+		priv->ffa_flags = crb_ffa->flags;
+		priv->ffa_attributes = crb_ffa->attributes;
+		rc = tpm_crb_ffa_init();
+		if (rc) {
+			if (rc == -ENOENT) {  // FF-A driver is not
available yet
drivers/char/tpm/tpm_crb.c +6 -0
+				rc = -EPROBE_DEFER;
+			}
+			goto out;
+		}
+	}
+

Quoted 4 lines

if (sm == ACPI_TPM2_COMMAND_BUFFER_WITH_PLUTON) { if (buf->header.length < (sizeof(*buf) + sizeof(*crb_pluton))) {
dev_err(dev,

Reviewed-by Jarkko Sakkinen <[email protected]>

BR, Jarkko

Stuart Yoder

author

2025-02-17 22:50 UTC #9 lore raw

PATCH v4 5/5 Documentation: tpm: add documentation for the CRB FF-A interface

In reply to Stuart Yoder

Add documentation providing details of how the CRB driver interacts with FF-A.

Signed-off-by Stuart Yoder <[email protected]>

---
 Documentation/security/tpm/tpm_ffa_crb.rst | 65 ++++++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/security/tpm/tpm_ffa_crb.rst

Documentation/security/tpm/tpm_ffa_crb.rst new file +65 -0

new file mode 100644
index 000000000000..0184193da3c7
@@ -0,0 +1,65 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+========================
+TPM CRB over FF-A Driver
+========================
+
+The TPM Command Response Buffer (CRB) interface is a standard TPM interface
+defined in the TCG PC Client Platform TPM Profile (PTP) Specification [1]_.
+The CRB provides a structured set of control registers a client uses when
+interacting with a TPM as well as a data buffer for storing TPM commands and
+responses. A CRB interface can be implemented in:
+
+- hardware registers in a discrete TPM chip
+
+- in memory for a TPM running in isolated environment where shared memory
+  allows a client to interact with the TPM
+
+The Firmware Framework for Arm A-profile (FF-A) [2]_ is a specification
+that defines interfaces and protocols for the following purposes:
+
+- Compartmentalize firmware into software partitions that run in the Arm
+  Secure world environment (also know as TrustZone)
+
+- Provide a standard interface for software components in the Non-secure
+  state, for example OS and Hypervisors, to communicate with this firmware.
+
+A TPM can be implemented as an FF-A secure service.  This could be a firmware
+TPM or could potentially be a TPM service that acts as a proxy to a discrete
+TPM chip. An FF-A based TPM abstracts hardware details (e.g. bus controller
+and chip selects) away from the OS and can protect locality 4 from access
+by an OS.  The TCG-defined CRB interface is used by clients to interact
+with the TPM service.
+
+The Arm TPM Service Command Response Buffer Interface Over FF-A [3]_
+specification defines FF-A messages that can be used by a client to signal
+when updates have been made to the CRB.
+
+How the Linux CRB driver interacts with FF-A is summarized below:
+
+- The tpm_crb_ffa driver registers with the FF-A subsystem in the kernel
+  with an architected TPM service UUID defined in the CRB over FF-A spec.
+
+- If a TPM service is discovered by FF-A, the probe() function in the
+  tpm_crb_ffa driver runs, and the driver initializes.
+
+- The probing and initialization of the Linux CRB driver is triggered
+  by the discovery of a TPM advertised via ACPI.  The CRB driver can
+  detect the type of TPM through the ACPI 'start' method.  The start
+  method for Arm FF-A was defined in TCG ACPI v1.4 [4]_.
+
+- When the CRB driver performs its normal functions such as signaling 'start'
+  and locality request/relinquish it invokes the tpm_crb_ffa_start() funnction
+  in the tpm_crb_ffa driver which handles the FF-A messaging to the TPM.
+
+References
+==========
+
+.. [1] **TCG PC Client Platform TPM Profile (PTP) Specification**
+   https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/
+.. [2] **Arm Firmware Framework for Arm A-profile (FF-A)**
+   https://developer.arm.com/documentation/den0077/latest/
+.. [3] **Arm TPM Service Command Response Buffer Interface Over FF-A**
+   https://developer.arm.com/documentation/den0138/latest/
+.. [4] **TCG ACPI Specification**
+   https://trustedcomputinggroup.org/resource/tcg-acpi-specification/

Signature · 1 line

2.34.1

Jarkko Sakkinen

maintainer

2025-03-02 19:34 UTC #10 lore raw

In reply to Stuart Yoder

Quoted 53 lines · Stuart Yoder

Firmware Framework for Arm A-profile (FF-A) is a messaging framework for Arm-based systems, and in the context of the TPM CRB driver is used to signal 'start' to a CRB-based TPM service which is hosted in an FF-A secure partition running in TrustZone.
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/
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.
The fifth patch adds documentation explaining how the CRB driver and FF-A relate.
Version 4
(quoted diff) +0 -7
-fix warning from kernel test robot in patch 1
-fix warnings from checkpatch.pl --strict
-clean up unecessary parenthesis usage
-update variable declaration to be reverse tree order
-document exported functions in tpm_crb_ffa driver
-remove unnecessary author and maintainer info in tpm_crb_ffa driver
-fix declaration of variables to be in reverse tree order
Version 3
-changed prefixes used throughout patch series to tpm_crb_ffa*
Version 2
(quoted diff) +0 -4
-updates to cover letter to define FF-A
-added new patch with documentation
-created pull request in ACPIA and added link to the patch
 updating actbl3.h
-added tpm_ prefix to the FF-A CRB driver
Stuart Yoder (5):
tpm_crb: implement driver compliant to CRB over FF-A
tpm_crb: clean-up and refactor check for idle support
ACPICA: add start method for Arm FF-A
tpm_crb: add support for the Arm FF-A start method
Documentation: tpm: add documentation for the CRB FF-A interface

Quoted 15 lines

 Documentation/security/tpm/tpm_ffa_crb.rst |  65 ++++
 drivers/char/tpm/Kconfig                   |   9 +
 drivers/char/tpm/Makefile                  |   1 +
 drivers/char/tpm/tpm_crb.c                 | 105 +++++--
 drivers/char/tpm/tpm_crb_ffa.c             | 348 +++++++++++++++++++++
 drivers/char/tpm/tpm_crb_ffa.h             |  25 ++
 include/acpi/actbl3.h                      |   1 +
 7 files changed, 535 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/security/tpm/tpm_ffa_crb.rst
 create mode 100644 drivers/char/tpm/tpm_crb_ffa.c
 create mode 100644 drivers/char/tpm/tpm_crb_ffa.h

Signature · 1 line

2.34.1

checkpatch.pl --strict reported me some issues.

The first one is:

WARNING: please write a help paragraph that fully describes the config symbol #41: FILE: drivers/char/tpm/Kconfig:213:
+config TCG_ARM_CRB_FFA

+       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 tpm_crb_ffa.

To be totally honest with I've never fully grabbed what checkpatch means by that message, i.e. what is the threshold for "fully described" :-)

So if someone can give advice on this, awesome, and let's do +1 round of the patch, but with my limited knowledge I have no legit reason to block this.

Nit: checkpatch could improve a bit that error message because "fully describe" is not unambiguous terminology.

The second issue:

CHECK: Alignment should match open parenthesis
#378: FILE: drivers/char/tpm/tpm_crb_ffa.c:309:
+ if (tpm_crb_ffa->major_version != CRB_FFA_VERSION_MAJOR ||
+ (tpm_crb_ffa->minor_version > 0 &&

if (tpm_crb_ffa->major_version != CRB_FFA_VERSION_MAJOR ||

    (tpm_crb_ffa->minor_version > 0 &&

I think it should be like this.

The final issue reported:

WARNING: line length of 102 exceeds 100 columns
#764: FILE: drivers/char/tpm/tpm_crb.c:821:
+ FW_BUG "TPM2 ACPI table has wrong size %u for start method type %d\n",

You could put that into two separate lines but maybe it would even make sense to create helper for the branch given the deep levels of nesting.

I.e. maybe it would make sense to encapsulate this into a helper:

		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 = tpm_crb_ffa_init();
		if (rc) {
			if (rc == -ENOENT) {  // FF-A driver is not available yet
				rc = -EPROBE_DEFER;
			}
			goto out;
		}

I looked at tpm_crb_ffa_init() too, and it's somewhat trivial, not very useful and has only single call site in kernel:

/**
* tpm_crb_ffa_init - called by the CRB driver to do any needed initialization *
* This function is called by the tpm_crb driver during the tpm_crb
* driver's initialization. If the tpm_crb_ffa has not been probed
* yet, returns -ENOENT in order to force a retry. If th ffa_crb
* driver had been probed but failed with an error, returns -ENODEV
* in order to prevent further retries.
*
* Return: 0 on success, negative error code on failure.
*/
int tpm_crb_ffa_init(void)

{
	if (!tpm_crb_ffa)
		return -ENOENT;

	if (IS_ERR_VALUE(tpm_crb_ffa))
		return -ENODEV;

	return 0;
}

EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);

Some questions arise here:

1. Wouldn't it be better idea to implement the code block residing in tpm_crb here?
2. Why it needs to be exported? Why it is not in the same compilation unit with tpm_crb? Cross-compilation unit calls extrapolates complexity and should have strong reasoning behind...

So yeah, this is area where we definitely need some rework. Sorry about this boomerang. Sometimes checkpatch opens up new thoughts and ideas :-) It's better to take step back always and do right thing right. It's much more time consuming for all of us to fixup things and backport fixes etc.

BR, Jarkko

Jason Gunthorpe

reviewer

2025-03-03 16:55 UTC #11 lore raw

Quoted 3 lines · Jarkko Sakkinen

WARNING: line length of 102 exceeds 100 columns
#764: FILE: drivers/char/tpm/tpm_crb.c:821:
+ FW_BUG "TPM2 ACPI table has wrong size %u for start method type %d\n",

Just ignore that, it is an error in checkpatch. Strings are required to be long. I suspect FW_BUG confused it.

Jason

Jarkko Sakkinen

maintainer

2025-03-04 16:02 UTC #12 lore raw

Quoted 9 lines · Jason Gunthorpe

Quoted 4 lines · Jarkko Sakkinen
WARNING: line length of 102 exceeds 100 columns
#764: FILE: drivers/char/tpm/tpm_crb.c:821:
+ FW_BUG "TPM2 ACPI table has wrong size %u for start method type %d\n",
Just ignore that, it is an error in checkpatch. Strings are required to
be long. I suspect FW_BUG confused it.

Yep, as its own issue I think you are right.

I also noticed couple of additional style issues not picked by checkpatch:

	if (rc == -ENOENT) {  // FF-A driver is not available yet
		rc = -EPROBE_DEFER;
	}

I.e. extra curly braces and "//" comment.

Should be:

	/* If driver is not available yet, request probe retry: */
	if (rc == -ENOENT)
		rc = -EPROBE_DEFER;

Quoted 3 lines

Jason

BR, Jarkko

Stuart Yoder

author

2025-03-05 16:09 UTC #13 lore raw

Quoted 20 lines · Jarkko Sakkinen

Quoted 9 lines · Jason Gunthorpe
Quoted 4 lines · Jarkko Sakkinen
WARNING: line length of 102 exceeds 100 columns
#764: FILE: drivers/char/tpm/tpm_crb.c:821:
+ FW_BUG "TPM2 ACPI table has wrong size %u for start method type %d\n",
Just ignore that, it is an error in checkpatch. Strings are required to
be long. I suspect FW_BUG confused it.
Yep, as its own issue I think you are right.
I also noticed couple of additional style issues not picked by checkpatch:
	if (rc == -ENOENT) {  // FF-A driver is not available yet
		rc = -EPROBE_DEFER;
	}

Quoted 8 lines

I.e. extra curly braces and "//" comment.
Should be:
	/* If driver is not available yet, request probe retry: */
	if (rc == -ENOENT)
		rc = -EPROBE_DEFER;

I will respin and fix this.

Thanks,
Stuart