Add support for the TPM FF-A start method

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.

Acked-by Sudeep Holla <[email protected]>

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

Quoted 12 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.

Quoted 2 lines

Acked-by Sudeep Holla <[email protected]>

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

Cutting hairs now but as I cannot test this and this is 1/5:
can this patch be run without 2/5-4/5?

The policy is that every patch should leave kernel tree to a state where it compiles and runs.

Quoted 12 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.

Quoted 2 lines

Acked-by Sudeep Holla <[email protected]>

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

Cutting hairs now but as I cannot test this and this is 1/5:
can this patch be run without 2/5-4/5?

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

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

Quoted 89 lines · Stuart Yoder

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

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) {

Quoted 6 lines

drivers/char/tpm/tpm_crb.c +0 -0

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

Signature · 1 line

2.34.1

Reviewed-by Jarkko Sakkinen <[email protected]>

Dear Stuart,

Thank you for the patch. Should you respin, you could spell *clean up*
in the summary/title with a space.

The diff looks good.

Kind regards,

Paul

Quoted 114 lines · Jarkko Sakkinen

Quoted 113 lines · Stuart Yoder

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

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-

Quoted 1 line

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-

Quoted 1 line

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-

Quoted 1 line

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 8 lines

Quoted 6 lines

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

Signature · 1 line

2.34.1

Reviewed-by Jarkko Sakkinen <[email protected]>

Thanks for the review.  Do you want me to respin and send out a v6 with your Reviewed-by tags on patches 2/5 and 5/5?

Thanks,
Stuart

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