Re: [PATCH V2] tpm_crb: Add support for CRB devices based on Pluton

[Jarkko Sakkinen <jarkko@kernel.org>]

On Sat, Dec 31, 2022 at 01:14:32AM -0800, Matthew Garrett wrote:
> Pluton is an integrated security processor present in some recent Ryzen
> parts. If it's enabled, it presents two devices - an MSFT0101 ACPI device
> that's broadly an implementation of a Command Response Buffer TPM2, and
> an MSFT0200 ACPI device whose functionality I haven't examined in detail
> yet. This patch only attempts to add support for the TPM device.
> 
> There's a few things that need to be handled here. The first is that the
> TPM2 ACPI table uses a previously undefined start method identifier. The
> table format appears to include 16 bytes of startup data, which corresponds
> to one 64-bit address for a start message and one 64-bit address for a
> completion response. The second is that the ACPI tables on the Thinkpad Z13
> I'm testing this on don't define any memory windows in _CRS (or, more
> accurately, there are two empty memory windows). This check doesn't seem
> strictly necessary, so I've skipped that.
> 
> Finally, it seems like chip needs to be explicitly asked to transition into
> ready status on every command. Failing to do this means that if two commands
> are sent in succession without an idle/ready transition in between,
> everything will appear to work fine but the response is simply the original
> command. I'm working without any docs here, so I'm not sure if this is
> actually the required behaviour or if I'm missing something somewhere else,
> but doing this results in the chip working reliably.
> 
> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

I'm applying this with these minor adjustments:

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 1d7b27d57974..d43a0d7b97a8 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -181,7 +181,6 @@ static int __crb_go_idle(struct device *dev, struct crb_priv *priv)
        iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->regs_t->ctrl_req);
 
        rc = crb_try_pluton_doorbell(priv, true);
-
        if (rc)
                return rc;
 
@@ -231,7 +230,6 @@ static int __crb_cmd_ready(struct device *dev, struct crb_priv *priv)
        iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->regs_t->ctrl_req);
 
        rc = crb_try_pluton_doorbell(priv, true);
-
        if (rc)
                return rc;
 
@@ -442,9 +440,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
        if (rc)
                return rc;
 
-       rc = crb_try_pluton_doorbell(priv, false);
-
-       return rc;
+       return crb_try_pluton_doorbell(priv, false);
 }

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

BR, Jarkko