Re: [PATCH 1/3] tpm: add generic platform device

Stefano Garzarella
2024-12-12T09:51:28Z

On Tue, Dec 10, 2024 at 03:34:21PM +0100, Stefano Garzarella wrote:
>From: James Bottomley <James.Bottomley@HansenPartnership.com>
>
>This is primarily designed to support an enlightened driver for the
>AMD svsm based vTPM, but it could be used by any platform which
>communicates with a TPM device. The platform must fill in struct
>tpm_platform_ops as the platform_data and set the device name to "tpm"
>to have the binding by name work correctly. The sole sendrcv
>function is designed to do a single buffer request/response conforming
>to the MSSIM protocol. For the svsm vTPM case, this protocol is
>transmitted directly to the SVSM, but it could be massaged for other
>function type platform interfaces.
>
>Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
>[SG] changed references/links to TCG TPM repo
>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>---
> include/linux/tpm_platform.h | 90 ++++++++++++++++++++
> drivers/char/tpm/tpm_platform.c | 141 ++++++++++++++++++++++++++++++++
> drivers/char/tpm/Kconfig | 7 ++
> drivers/char/tpm/Makefile | 1 +
> 4 files changed, 239 insertions(+)
> create mode 100644 include/linux/tpm_platform.h
> create mode 100644 drivers/char/tpm/tpm_platform.c
>
>diff --git a/include/linux/tpm_platform.h b/include/linux/tpm_platform.h
>new file mode 100644
>index 000000000000..95c17a75d59d
>--- /dev/null
>+++ b/include/linux/tpm_platform.h
>@@ -0,0 +1,90 @@
>+/* SPDX-License-Identifier: GPL-2.0-only */
>+/*
>+ * Copyright (C) 2023 James.Bottomley@HansenPartnership.com
>+ *
>+ * Interface specification for platforms wishing to activate the
>+ * platform tpm device. The device must be a platform device created
>+ * with the name "tpm" and it must populate platform_data with struct
>+ * tpm_platform_ops
>+ */
>+
>+/*
>+ * The current MSSIM TPM commands we support. The complete list is
>+ * in the TcpTpmProtocol header:
>+ *
>+ * https://github.com/TrustedComputingGroup/TPM/blob/main/TPMCmd/Simulator/include/TpmTcpProtocol.h
>+ */
>+
>+#define TPM_SEND_COMMAND 8
>+#define TPM_SIGNAL_CANCEL_ON 9
>+#define TPM_SIGNAL_CANCEL_OFF 10
>+/*
>+ * Any platform specific commands should be placed here and should start
>+ * at 0x8000 to avoid clashes with the MSSIM protocol. They should follow
>+ * the same self describing buffer format below
>+ */
>+
>+#define TPM_PLATFORM_MAX_BUFFER 4096 /* max req/resp buffer size */
>+
>+/**
>+ * struct tpm_platform_ops - the share platform operations
>+ *
>+ * @sendrcv: Send a TPM command using the MSSIM protocol.
>+ *
>+ * The MSSIM protocol is designed for a network, so the buffers are
>+ * self describing. The minimum buffer size is sizeof(u32). Every
>+ * MSSIM command defines its own transport buffer and the command is
>+ * sent in the first u32 array. The only modification we make is that
>+ * the MSSIM uses network order and we use the endianness of the
>+ * architecture. The response to every command (in the same buffer)
>+ * is a u32 size preceded array. Most of the MSSIM commands simply
>+ * return zero here because they have no defined response.
>+ *
>+ * The only command with a defined request/response size is TPM_SEND_COMMAND
>+ * The definition is in the structures below
>+ */
>+struct tpm_platform_ops {
>+ int (*sendrcv)(u8 *buffer);
>+};
>+
>+/**
>+ * struct tpm_send_cmd_req - Structure for a TPM_SEND_COMMAND
>+ *
>+ * @cmd: The command (must be TPM_SEND_COMMAND)
>+ * @locality: The locality
>+ * @inbuf_size: The size of the input buffer following
>+ * @inbuf: A buffer of size inbuf_size
>+ *
>+ * Note that MSSIM expects @inbuf_size to be equal to the size of the
>+ * specific TPM command, otherwise an TPM_RC_COMMAND_SIZE error is
>+ * returned.
>+ */
>+struct tpm_send_cmd_req {
>+ u32 cmd;
>+ u8 locality;
>+ u32 inbuf_size;
>+ u8 inbuf[];
>+} __packed;
>+
>+/**
>+ * struct tpm_req - generic request header for single word command
>+ *
>+ * @cmd: The command to send
>+ */
>+struct tpm_req {
>+ u32 cmd;
>+} __packed;
>+
>+/**
>+ * struct tpm_resp - generic response header
>+ *
>+ * @size: The response size (zero if nothing follows)
>+ *
>+ * Note: most MSSIM commands simply return zero here with no indication
>+ * of success or failure.
>+ */
>+
>+struct tpm_resp {
>+ s32 size;
>+} __packed;
>+
>diff --git a/drivers/char/tpm/tpm_platform.c b/drivers/char/tpm/tpm_platform.c
>new file mode 100644
>index 000000000000..b53d74344d61
>--- /dev/null
>+++ b/drivers/char/tpm/tpm_platform.c
>@@ -0,0 +1,141 @@
>+// SPDX-License-Identifier: GPL-2.0-only
>+/*
>+ * Platform based TPM emulator
>+ *
>+ * Copyright (C) 2023 James.Bottomley@HansenPartnership.com
>+ *
>+ * Designed to handle a simple function request/response single buffer
>+ * TPM or vTPM rooted in the platform. This device driver uses the
>+ * MSSIM protocol from the official TCG reference implementation
>+ *
>+ * https://github.com/TrustedComputingGroup/TPM
>+ *
>+ * to communicate between the driver and the platform. This is rich
>+ * enough to allow platform operations like cancellation The platform
>+ * should not act on platform commands like power on/off and reset
>+ * which can disrupt the TPM guarantees.
>+ *
>+ * This driver is designed to be single threaded (one call in to the
>+ * platform TPM at any one time). The threading guarantees are
>+ * provided by the chip mutex.
>+ */
>+
>+#include <linux/module.h>
>+#include <linux/kernel.h>
>+#include <linux/platform_device.h>
>+#include <linux/tpm_platform.h>
>+
>+#include "tpm.h"
>+
>+static struct tpm_platform_ops *pops;
>+
>+static u8 *buffer;
>+/*
>+ * FIXME: before implementing locality we need to agree what it means
>+ * to the platform
>+ */
>+static u8 locality;
>+
>+static int tpm_platform_send(struct tpm_chip *chip, u8 *buf, size_t len)
>+{
>+ int ret;
>+ struct tpm_send_cmd_req *req = (struct tpm_send_cmd_req *)buffer;
>+
>+ if (len > TPM_PLATFORM_MAX_BUFFER - sizeof(*req))
>+ return -EINVAL;
>+ req->cmd = TPM_SEND_COMMAND;
>+ req->locality = locality;
>+ req->inbuf_size = len;
>+ memcpy(req->inbuf, buf, len);
>+
>+ ret = pops->sendrcv(buffer);
>+ if (ret)
>+ return ret;
>+
>+ return 0;
>+}
>+
>+static int tpm_platform_recv(struct tpm_chip *chip, u8 *buf, size_t len)
>+{
>+ struct tpm_resp *resp = (struct tpm_resp *)buffer;
>+
>+ if (resp->size < 0)
>+ return resp->size;

While reviewing Oliver's work for the driver in edk2[1], I noticed that
there wasn't this check and asked to add it, but talking to him and
looking in the code/spec, we realized that it's strange that
tpm_resp.size field is signed.

From SVSM spec it looks like it can't be negative:

Table 17: TPM_SEND_COMMAND Response Structure

Byte Size Meaning
Offset (Bytes)
0x000 4 Response size (in bytes)
0x004 Variable Variable Response

And also Coconut SVSM remap it to the `responseSize` of the TCG TPM
implementation which is unsigned:

LIB_EXPORT void _plat__RunCommand(
uint32_t requestSize, // IN: command buffer size
unsigned char* request, // IN: command buffer
uint32_t* responseSize, // IN/OUT: response buffer size
unsigned char** response // IN/OUT: response buffer
)

@James, @Claudio, @Tom, should we use u32 for tpm_resp.size?

Thanks,
Stefano

[1] https://github.com/tianocore/edk2/pull/6527#discussion_r1880204144

>+
>+ if (len < resp->size)
>+ return -E2BIG;
>+
>+ if (resp->size > TPM_PLATFORM_MAX_BUFFER - sizeof(*resp))
>+ return -EINVAL; // Invalid response from the platform TPM
>+
>+ memcpy(buf, buffer + sizeof(*resp), resp->size);
>+
>+ return resp->size;
>+}
>+
>+static struct tpm_class_ops tpm_chip_ops = {
>+ .flags = TPM_OPS_AUTO_STARTUP,
>+ .send = tpm_platform_send,
>+ .recv = tpm_platform_recv,
>+};
>+
>+static struct platform_driver tpm_platform_driver = {
>+ .driver = {
>+ .name = "tpm",
>+ },
>+};
>+
>+static int __init tpm_platform_probe(struct platform_device *pdev)
>+{
>+ struct device *dev = &pdev->dev;
>+ struct tpm_chip *chip;
>+ int err;
>+
>+ if (!dev->platform_data)
>+ return -ENODEV;
>+
>+ /*
>+ * in theory platform matching should mean this is always
>+ * true, but just in case anyone tries force binding
>+ */
>+ if (strcmp(pdev->name, tpm_platform_driver.driver.name) != 0)
>+ return -ENODEV;
>+
>+ if (!buffer)
>+ buffer = kmalloc(TPM_PLATFORM_MAX_BUFFER, GFP_KERNEL);
>+
>+ if (!buffer)
>+ return -ENOMEM;
>+
>+ pops = dev->platform_data;
>+
>+ chip = tpmm_chip_alloc(dev, &tpm_chip_ops);
>+ if (IS_ERR(chip))
>+ return PTR_ERR(chip);
>+
>+ /*
>+ * Setting TPM_CHIP_FLAG_IRQ guarantees that ->recv will be
>+ * called straight after ->send and means we don't need to
>+ * implement any other chip ops.
>+ */
>+ chip->flags |= TPM_CHIP_FLAG_IRQ;
>+ err = tpm2_probe(chip);
>+ if (err)
>+ return err;
>+
>+ err = tpm_chip_register(chip);
>+ if (err)
>+ return err;
>+
>+ dev_info(dev, "TPM %s platform device\n",
>+ (chip->flags & TPM_CHIP_FLAG_TPM2) ? "2.0" : "1.2");
>+
>+ return 0;
>+}
>+
>+module_platform_driver_probe(tpm_platform_driver, tpm_platform_probe);
>+
>+MODULE_AUTHOR("James Bottomley <James.Bottomley@HansenPartnership.com>");
>+MODULE_LICENSE("GPL");
>+MODULE_DESCRIPTION("Platform TPM Driver");
>+MODULE_ALIAS("platform:tpm");
>diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
>index 0fc9a510e059..b162f59305ef 100644
>--- a/drivers/char/tpm/Kconfig
>+++ b/drivers/char/tpm/Kconfig
>@@ -225,5 +225,12 @@ config TCG_FTPM_TEE
> help
> This driver proxies for firmware TPM running in TEE.
>
>+config TCG_PLATFORM
>+ tristate "Platform TPM Device"
>+ help
>+ This driver requires a platform implementation to provide the
>+ TPM function. It will not bind if the implementation is not
>+ present.
>+
> source "drivers/char/tpm/st33zp24/Kconfig"
> endif # TCG_TPM
>diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
>index 9bb142c75243..4b2c04e23bd3 100644
>--- a/drivers/char/tpm/Makefile
>+++ b/drivers/char/tpm/Makefile
>@@ -44,3 +44,4 @@ obj-$(CONFIG_TCG_XEN) += xen-tpmfront.o
> obj-$(CONFIG_TCG_CRB) += tpm_crb.o
> obj-$(CONFIG_TCG_VTPM_PROXY) += tpm_vtpm_proxy.o
> obj-$(CONFIG_TCG_FTPM_TEE) += tpm_ftpm_tee.o
>+obj-$(CONFIG_TCG_PLATFORM) += tpm_platform.o
>--
>2.47.1
>