linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tpm_crb: Add idle support for the Arm FF-A start method
@ 2025-08-25 20:59 Stuart Yoder
  2025-08-25 21:58 ` Jarkko Sakkinen
  0 siblings, 1 reply; 4+ messages in thread
From: Stuart Yoder @ 2025-08-25 20:59 UTC (permalink / raw)
  To: linux-integrity, jarkko, peterhuewe, jgg, sudeep.holla
  Cc: Prachotan.Bathi, linux-kernel

According to the CRB over FF-A specification [1], a TPM that implements
the ABI must comply with the TCG PTP specification. This requires support
for the Idle and Ready states.

This patch implements CRB control area requests for goIdle and
cmdReady on FF-A based TPMs.

The FF-A message used to notify the TPM of CRB updates includes a
locality parameter, which provides a hint to the TPM about which
locality modified the CRB.  This patch adds a locality parameter
to __crb_go_idle() and __crb_cmd_ready() to support this.

[1] https://developer.arm.com/documentation/den0138/latest/

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

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 876edf2705abb..a18bae0a53717 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -133,8 +133,7 @@ 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_CRB_WITH_ARM_FFA);
+	       start_method == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC);
 }
 
 static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
@@ -191,7 +190,7 @@ static int crb_try_pluton_doorbell(struct crb_priv *priv, bool wait_for_complete
  *
  * Return: 0 always
  */
-static int __crb_go_idle(struct device *dev, struct crb_priv *priv)
+static int __crb_go_idle(struct device *dev, struct crb_priv *priv, int loc)
 {
 	int rc;
 
@@ -200,6 +199,12 @@ static int __crb_go_idle(struct device *dev, struct crb_priv *priv)
 
 	iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->regs_t->ctrl_req);
 
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND, loc);
+		if (rc)
+			return rc;
+	}
+
 	rc = crb_try_pluton_doorbell(priv, true);
 	if (rc)
 		return rc;
@@ -220,7 +225,7 @@ static int crb_go_idle(struct tpm_chip *chip)
 	struct device *dev = &chip->dev;
 	struct crb_priv *priv = dev_get_drvdata(dev);
 
-	return __crb_go_idle(dev, priv);
+	return __crb_go_idle(dev, priv, chip->locality);
 }
 
 /**
@@ -238,7 +243,7 @@ static int crb_go_idle(struct tpm_chip *chip)
  *
  * Return: 0 on success -ETIME on timeout;
  */
-static int __crb_cmd_ready(struct device *dev, struct crb_priv *priv)
+static int __crb_cmd_ready(struct device *dev, struct crb_priv *priv, int loc)
 {
 	int rc;
 
@@ -247,6 +252,12 @@ static int __crb_cmd_ready(struct device *dev, struct crb_priv *priv)
 
 	iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->regs_t->ctrl_req);
 
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND, loc);
+		if (rc)
+			return rc;
+	}
+
 	rc = crb_try_pluton_doorbell(priv, true);
 	if (rc)
 		return rc;
@@ -267,7 +278,7 @@ static int crb_cmd_ready(struct tpm_chip *chip)
 	struct device *dev = &chip->dev;
 	struct crb_priv *priv = dev_get_drvdata(dev);
 
-	return __crb_cmd_ready(dev, priv);
+	return __crb_cmd_ready(dev, priv, chip->locality);
 }
 
 static int __crb_request_locality(struct device *dev,
@@ -444,7 +455,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
 
 	/* Seems to be necessary for every command */
 	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_PLUTON)
-		__crb_cmd_ready(&chip->dev, priv);
+		__crb_cmd_ready(&chip->dev, priv, chip->locality);
 
 	memcpy_toio(priv->cmd, buf, len);
 
@@ -672,7 +683,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 	 * PTT HW bug w/a: wake up the device to access
 	 * possibly not retained registers.
 	 */
-	ret = __crb_cmd_ready(dev, priv);
+	ret = __crb_cmd_ready(dev, priv, 0);
 	if (ret)
 		goto out_relinquish_locality;
 
@@ -744,7 +755,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 	if (!ret)
 		priv->cmd_size = cmd_size;
 
-	__crb_go_idle(dev, priv);
+	__crb_go_idle(dev, priv, 0);
 
 out_relinquish_locality:
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] tpm_crb: Add idle support for the Arm FF-A start method
  2025-08-25 20:59 [PATCH] tpm_crb: Add idle support for the Arm FF-A start method Stuart Yoder
@ 2025-08-25 21:58 ` Jarkko Sakkinen
  2025-08-25 22:19   ` Stuart Yoder
  0 siblings, 1 reply; 4+ messages in thread
From: Jarkko Sakkinen @ 2025-08-25 21:58 UTC (permalink / raw)
  To: Stuart Yoder
  Cc: linux-integrity, peterhuewe, jgg, sudeep.holla, Prachotan.Bathi,
	linux-kernel

On Mon, Aug 25, 2025 at 03:59:43PM -0500, Stuart Yoder wrote:
> According to the CRB over FF-A specification [1], a TPM that implements
> the ABI must comply with the TCG PTP specification. This requires support
> for the Idle and Ready states.
> 
> This patch implements CRB control area requests for goIdle and
> cmdReady on FF-A based TPMs.
> 
> The FF-A message used to notify the TPM of CRB updates includes a
> locality parameter, which provides a hint to the TPM about which
> locality modified the CRB.  This patch adds a locality parameter
> to __crb_go_idle() and __crb_cmd_ready() to support this.
> 
> [1] https://developer.arm.com/documentation/den0138/latest/
> 
> Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>

Perhaps a dummy question but is this "QEMU testable"? I know how
to bind swtpm to QEMU and make it appear as CRB device on x86-64.

I don't see much testing happening with these ARM CRB patches,
and if that works in the first palce  I could probably add
a new board target to my BR2_EXTERNAL [1].

I can of course do "negative testing' i.e. that these don't
break x86 ;-)

> ---
>  drivers/char/tpm/tpm_crb.c | 29 ++++++++++++++++++++---------
>  1 file changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 876edf2705abb..a18bae0a53717 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -133,8 +133,7 @@ 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_CRB_WITH_ARM_FFA);
> +	       start_method == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC);
>  }
>  
>  static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
> @@ -191,7 +190,7 @@ static int crb_try_pluton_doorbell(struct crb_priv *priv, bool wait_for_complete
>   *
>   * Return: 0 always
>   */
> -static int __crb_go_idle(struct device *dev, struct crb_priv *priv)
> +static int __crb_go_idle(struct device *dev, struct crb_priv *priv, int loc)
>  {
>  	int rc;
>  
> @@ -200,6 +199,12 @@ static int __crb_go_idle(struct device *dev, struct crb_priv *priv)
>  
>  	iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->regs_t->ctrl_req);
>  
> +	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
> +		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND, loc);
> +		if (rc)
> +			return rc;
> +	}
> +
>  	rc = crb_try_pluton_doorbell(priv, true);
>  	if (rc)
>  		return rc;
> @@ -220,7 +225,7 @@ static int crb_go_idle(struct tpm_chip *chip)
>  	struct device *dev = &chip->dev;
>  	struct crb_priv *priv = dev_get_drvdata(dev);
>  
> -	return __crb_go_idle(dev, priv);
> +	return __crb_go_idle(dev, priv, chip->locality);
>  }
>  
>  /**
> @@ -238,7 +243,7 @@ static int crb_go_idle(struct tpm_chip *chip)
>   *
>   * Return: 0 on success -ETIME on timeout;
>   */
> -static int __crb_cmd_ready(struct device *dev, struct crb_priv *priv)
> +static int __crb_cmd_ready(struct device *dev, struct crb_priv *priv, int loc)
>  {
>  	int rc;
>  
> @@ -247,6 +252,12 @@ static int __crb_cmd_ready(struct device *dev, struct crb_priv *priv)
>  
>  	iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->regs_t->ctrl_req);
>  
> +	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
> +		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND, loc);
> +		if (rc)
> +			return rc;
> +	}
> +
>  	rc = crb_try_pluton_doorbell(priv, true);
>  	if (rc)
>  		return rc;
> @@ -267,7 +278,7 @@ static int crb_cmd_ready(struct tpm_chip *chip)
>  	struct device *dev = &chip->dev;
>  	struct crb_priv *priv = dev_get_drvdata(dev);
>  
> -	return __crb_cmd_ready(dev, priv);
> +	return __crb_cmd_ready(dev, priv, chip->locality);
>  }
>  
>  static int __crb_request_locality(struct device *dev,
> @@ -444,7 +455,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
>  
>  	/* Seems to be necessary for every command */
>  	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_PLUTON)
> -		__crb_cmd_ready(&chip->dev, priv);
> +		__crb_cmd_ready(&chip->dev, priv, chip->locality);
>  
>  	memcpy_toio(priv->cmd, buf, len);
>  
> @@ -672,7 +683,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
>  	 * PTT HW bug w/a: wake up the device to access
>  	 * possibly not retained registers.
>  	 */
> -	ret = __crb_cmd_ready(dev, priv);
> +	ret = __crb_cmd_ready(dev, priv, 0);
>  	if (ret)
>  		goto out_relinquish_locality;
>  
> @@ -744,7 +755,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
>  	if (!ret)
>  		priv->cmd_size = cmd_size;
>  
> -	__crb_go_idle(dev, priv);
> +	__crb_go_idle(dev, priv, 0);
>  
>  out_relinquish_locality:
>  
> -- 
> 2.34.1
> 
> 

[1] https://codeberg.org/jarkko/linux-tpmdd-test

BR, Jarkko

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tpm_crb: Add idle support for the Arm FF-A start method
  2025-08-25 21:58 ` Jarkko Sakkinen
@ 2025-08-25 22:19   ` Stuart Yoder
  2025-08-25 22:49     ` Jarkko Sakkinen
  0 siblings, 1 reply; 4+ messages in thread
From: Stuart Yoder @ 2025-08-25 22:19 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-integrity, peterhuewe, jgg, sudeep.holla, Prachotan.Bathi,
	linux-kernel



On 8/25/25 4:58 PM, Jarkko Sakkinen wrote:
> On Mon, Aug 25, 2025 at 03:59:43PM -0500, Stuart Yoder wrote:
>> According to the CRB over FF-A specification [1], a TPM that implements
>> the ABI must comply with the TCG PTP specification. This requires support
>> for the Idle and Ready states.
>>
>> This patch implements CRB control area requests for goIdle and
>> cmdReady on FF-A based TPMs.
>>
>> The FF-A message used to notify the TPM of CRB updates includes a
>> locality parameter, which provides a hint to the TPM about which
>> locality modified the CRB.  This patch adds a locality parameter
>> to __crb_go_idle() and __crb_cmd_ready() to support this.
>>
>> [1] https://developer.arm.com/documentation/den0138/latest/
>>
>> Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>
> 
> Perhaps a dummy question but is this "QEMU testable"? I know how
> to bind swtpm to QEMU and make it appear as CRB device on x86-64.
> 
> I don't see much testing happening with these ARM CRB patches,
> and if that works in the first palce  I could probably add
> a new board target to my BR2_EXTERNAL [1].
> 
> I can of course do "negative testing' i.e. that these don't
> break x86 ;-)

Unfortunately this is not currently testable on QEMU.  We are using
the Arm FVP [1], which is also a machine emulator, with the firmware
stack and an fTPM running in TrustZone.  The firmware, fTPM, etc are
not all publicly available yet, but everything is based on open
source projects and the intent is that all the components needed do
test this on FVP will be available at some point.

There is nothing fundamental that would prevent this from running
on QEMU, but just a fair amount of integration and possibly firmware
work.

[1] 
https://developer.arm.com/Tools%20and%20Software/Fixed%20Virtual%20Platforms/Arm%20Architecture%20FVPs

Thanks,
Stuart

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tpm_crb: Add idle support for the Arm FF-A start method
  2025-08-25 22:19   ` Stuart Yoder
@ 2025-08-25 22:49     ` Jarkko Sakkinen
  0 siblings, 0 replies; 4+ messages in thread
From: Jarkko Sakkinen @ 2025-08-25 22:49 UTC (permalink / raw)
  To: Stuart Yoder
  Cc: linux-integrity, peterhuewe, jgg, sudeep.holla, Prachotan.Bathi,
	linux-kernel

On Mon, Aug 25, 2025 at 05:19:34PM -0500, Stuart Yoder wrote:
> 
> 
> On 8/25/25 4:58 PM, Jarkko Sakkinen wrote:
> > On Mon, Aug 25, 2025 at 03:59:43PM -0500, Stuart Yoder wrote:
> > > According to the CRB over FF-A specification [1], a TPM that implements
> > > the ABI must comply with the TCG PTP specification. This requires support
> > > for the Idle and Ready states.
> > > 
> > > This patch implements CRB control area requests for goIdle and
> > > cmdReady on FF-A based TPMs.
> > > 
> > > The FF-A message used to notify the TPM of CRB updates includes a
> > > locality parameter, which provides a hint to the TPM about which
> > > locality modified the CRB.  This patch adds a locality parameter
> > > to __crb_go_idle() and __crb_cmd_ready() to support this.
> > > 
> > > [1] https://developer.arm.com/documentation/den0138/latest/
> > > 
> > > Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>
> > 
> > Perhaps a dummy question but is this "QEMU testable"? I know how
> > to bind swtpm to QEMU and make it appear as CRB device on x86-64.
> > 
> > I don't see much testing happening with these ARM CRB patches,
> > and if that works in the first palce  I could probably add
> > a new board target to my BR2_EXTERNAL [1].
> > 
> > I can of course do "negative testing' i.e. that these don't
> > break x86 ;-)
> 
> Unfortunately this is not currently testable on QEMU.  We are using
> the Arm FVP [1], which is also a machine emulator, with the firmware
> stack and an fTPM running in TrustZone.  The firmware, fTPM, etc are
> not all publicly available yet, but everything is based on open
> source projects and the intent is that all the components needed do
> test this on FVP will be available at some point.
> 
> There is nothing fundamental that would prevent this from running
> on QEMU, but just a fair amount of integration and possibly firmware
> work.

OK, it's cool and the patch looks totally fine and I can
"hallucinate it" so:

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

> 
> [1] https://developer.arm.com/Tools%20and%20Software/Fixed%20Virtual%20Platforms/Arm%20Architecture%20FVPs
> 
> Thanks,
> Stuart

BR, Jarkko

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-08-25 22:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-25 20:59 [PATCH] tpm_crb: Add idle support for the Arm FF-A start method Stuart Yoder
2025-08-25 21:58 ` Jarkko Sakkinen
2025-08-25 22:19   ` Stuart Yoder
2025-08-25 22:49     ` Jarkko Sakkinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).