linux-kernel - Re: [PATCH] tpm_crb: Add idle support for the Arm FF-A start method

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aKzcaaXGQyLfDPrf@kernel.org>
Date: Tue, 26 Aug 2025 00:58:01 +0300
From: Jarkko Sakkinen <jarkko@...nel.org>
To: Stuart Yoder <stuart.yoder@....com>
Cc: linux-integrity@...r.kernel.org, peterhuewe@....de, jgg@...pe.ca,
	sudeep.holla@....com, Prachotan.Bathi@....com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tpm_crb: Add idle support for the Arm FF-A start method

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@....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