Re: [RFC PATCH v2 5/6] tpm: add SNP SVSM vTPM driver

Stefano Garzarella
2025-03-03T16:46:32Z

On Sat, Mar 01, 2025 at 03:51:46AM +0200, Jarkko Sakkinen wrote:
>On Fri, Feb 28, 2025 at 06:07:19PM +0100, Stefano Garzarella wrote:
>> Add driver for the vTPM defined by the AMD SVSM spec [1].
>>
>> The specification defines a protocol that a SEV-SNP guest OS can use to
>> discover and talk to a vTPM emulated by the Secure VM Service Module (SVSM)
>> in the guest context, but at a more privileged level (VMPL0).
>>
>> The new tpm-svsm platform driver uses two functions exposed by x86/sev
>> to verify that the device is actually emulated by the platform and to
>> send commands and receive responses.
>>
>> The vTPM is emulated through the TCG reference implementation, so this
>> driver leverages tpm_tcgsim.h to fill commands and parse responses.
>
>Why? Please don't.

You mean it's better not to have the external header and have all the
functions here to prepare commands and parse responses?

As I mentioned, I did this because there may be other future drivers
that could use it to talk to emulated devices in the same way, that is,
through the TCG TPM reference implementation,

>
>>
>> The device cannot be hot-plugged/unplugged as it is emulated by the
>> platform, so we can use module_platform_driver_probe(). The probe
>> function will only check whether in the current runtime configuration,
>> SVSM is present and provides a vTPM.
>>
>> [1] "Secure VM Service Module for SEV-SNP Guests"
>> Publication # 58019 Revision: 1.00
>> https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
>>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>> drivers/char/tpm/tpm_svsm.c | 120 ++++++++++++++++++++++++++++++++++++
>> drivers/char/tpm/Kconfig | 10 +++
>> drivers/char/tpm/Makefile | 1 +
>> 3 files changed, 131 insertions(+)
>> create mode 100644 drivers/char/tpm/tpm_svsm.c
>>
>> diff --git a/drivers/char/tpm/tpm_svsm.c b/drivers/char/tpm/tpm_svsm.c
>> new file mode 100644
>> index 000000000000..1c34133990c5
>> --- /dev/null
>> +++ b/drivers/char/tpm/tpm_svsm.c
>> @@ -0,0 +1,120 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved.
>> + *
>> + * Driver for the vTPM defined by the AMD SVSM spec [1].
>> + *
>> + * The specification defines a protocol that a SEV-SNP guest OS can use to
>> + * discover and talk to a vTPM emulated by the Secure VM Service Module (SVSM)
>> + * in the guest context, but at a more privileged level (usually VMPL0).
>> + *
>> + * The vTPM is emulated through the TCG reference implementation, so this
>> + * driver leverages tpm_tcgsim.h to fill commands and parse responses.
>> + *
>> + * [1] "Secure VM Service Module for SEV-SNP Guests"
>> + * Publication # 58019 Revision: 1.00
>> + * https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
>> + */
>> +
>> +#include <asm/sev.h>
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/tpm_tcgsim.h>
>> +
>> +#include "tpm.h"
>> +
>> +struct tpm_svsm_priv {
>> + u8 buffer[TPM_TCGSIM_MAX_BUFFER];
>> + u8 locality;
>> +};
>> +
>> +static int tpm_svsm_send_recv(struct tpm_chip *chip, u8 *buf, size_t buf_len,
>> + size_t to_send)
>> +{
>> + struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
>> + int ret;
>> +
>> + ret = tpm_tcgsim_fill_send_cmd((struct tpm_send_cmd_req *)priv->buffer,
>> + priv->locality, buf, to_send);
>> + if (ret)
>> + return ret;
>> +
>> + ret = snp_svsm_vtpm_send_command(priv->buffer);
>> + if (ret)
>> + return ret;
>> +
>> + return tpm_tcgsim_parse_send_cmd((struct tpm_send_cmd_resp *)priv->buffer,
>> + buf, buf_len);
>> +}
>> +
>> +static struct tpm_class_ops tpm_chip_ops = {
>> + .flags = TPM_OPS_AUTO_STARTUP,
>> + .send_recv = tpm_svsm_send_recv,
>> +};
>> +
>> +static int __init tpm_svsm_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct tpm_svsm_priv *priv;
>> + struct tpm_chip *chip;
>> + int err;
>> +
>> + if (!snp_svsm_vtpm_probe())
>> + return -ENODEV;
>> +
>> + priv = devm_kmalloc(dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + /*
>> + * FIXME: before implementing locality we need to agree what it means
>> + * for the SNP SVSM vTPM
>> + */
>> + priv->locality = 0;
>> +
>> + chip = tpmm_chip_alloc(dev, &tpm_chip_ops);
>> + if (IS_ERR(chip))
>> + return PTR_ERR(chip);
>> +
>> + dev_set_drvdata(&chip->dev, priv);
>> +
>> + err = tpm2_probe(chip);
>> + if (err)
>> + return err;
>> +
>> + err = tpm_chip_register(chip);
>> + if (err)
>> + return err;
>> +
>> + dev_info(dev, "SNP SVSM vTPM %s device\n",
>> + (chip->flags & TPM_CHIP_FLAG_TPM2) ? "2.0" : "1.2");
>> +
>> + return 0;
>> +}
>> +
>> +static void __exit tpm_svsm_remove(struct platform_device *pdev)
>> +{
>> + struct tpm_chip *chip = platform_get_drvdata(pdev);
>> +
>> + tpm_chip_unregister(chip);
>> +}
>> +
>> +/*
>> + * tpm_svsm_remove() lives in .exit.text. For drivers registered via
>> + * module_platform_driver_probe() this is ok because they cannot get unbound
>> + * at runtime. So mark the driver struct with __refdata to prevent modpost
>> + * triggering a section mismatch warning.
>> + */
>> +static struct platform_driver tpm_svsm_driver __refdata = {
>> + .remove = __exit_p(tpm_svsm_remove),
>> + .driver = {
>> + .name = "tpm-svsm",
>> + },
>> +};
>> +
>> +module_platform_driver_probe(tpm_svsm_driver, tpm_svsm_probe);
>> +
>> +MODULE_DESCRIPTION("SNP SVSM vTPM Driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("platform:tpm-svsm");
>> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
>> index 0fc9a510e059..fc3f1d10d31d 100644
>> --- a/drivers/char/tpm/Kconfig
>> +++ b/drivers/char/tpm/Kconfig
>> @@ -225,5 +225,15 @@ config TCG_FTPM_TEE
>> help
>> This driver proxies for firmware TPM running in TEE.
>>
>> +config TCG_SVSM
>> + tristate "SNP SVSM vTPM interface"
>> + depends on AMD_MEM_ENCRYPT
>> + help
>> + This is a driver for the AMD SVSM vTPM protocol that a SEV-SNP guest
>> + OS can use to discover and talk to a vTPM emulated by the Secure VM
>> + Service Module (SVSM) in the guest context, but at a more privileged
>> + level (usually VMPL0). To compile this driver as a module, choose M
>> + here; the module will be called tpm_svsm.
>> +
>> source "drivers/char/tpm/st33zp24/Kconfig"
>> endif # TCG_TPM
>> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
>> index 9bb142c75243..52d9d80a0f56 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_SVSM) += tpm_svsm.o
>> --
>> 2.48.1
>>
>
>OK, so this is like ARM's driver architecture using SMC, and I think
>tpm_ftpm_tee is probably one great reflection for this overall. Is this
>correct analysis, or not?

I just took a closer look at what ARM SMC is, and yes, it looks like a
correct analysis.

Dionna took a look at reusing ftpm and already replied with her
analysis, better to continue in that thread the discussion on eventually
reusing ftpm.

Thanks,
Stefano