[v5,3/4] tpm: add SNP SVSM vTPM driver

Commit Message

Stefano Garzarella March 31, 2025, 10:38 a.m. UTC

From: Stefano Garzarella <[e-mail]>

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

This device does not support interrupts and sends responses to commands
synchronously. In order to have .recv() called just after .send() in
tpm_try_transmit(), the .status() callback returns 0, and both
.req_complete_mask and .req_complete_val are set to 0.

[1] "Secure VM Service Module for SEV-SNP Guests"
    Publication # 58019 Revision: 1.00

Signed-off-by: Stefano Garzarella <[e-mail]>
---
v5:
- removed cancel/status/req_* ops after rebase on master that cotains
  commit 980a573621ea ("tpm: Make chip->{status,cancel,req_canceled} opt")
v4:
- moved "asm" includes after the "linux" includes [Tom]
- allocated buffer separately [Tom/Jarkko/Jason]
v3:
- removed send_recv() ops and followed the ftpm driver implementing .status,
  .req_complete_mask, .req_complete_val, etc. [Jarkko]
- removed link to the spec because those URLs are unstable [Borislav]
---
 drivers/char/tpm/tpm_svsm.c | 135 ++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/Kconfig    |  10 +++
 drivers/char/tpm/Makefile   |   1 +
 3 files changed, 146 insertions(+)
 create mode 100644 drivers/char/tpm/tpm_svsm.c

Comments

Jarkko Sakkinen March 31, 2025, 5:34 p.m. UTC | #1

On Mon, Mar 31, 2025 at 12:38:56PM +0200, Stefano Garzarella wrote:
> From: Stefano Garzarella <[e-mail]>
> 
> 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 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.
> 
> This device does not support interrupts and sends responses to commands
> synchronously. In order to have .recv() called just after .send() in
> tpm_try_transmit(), the .status() callback returns 0, and both
> .req_complete_mask and .req_complete_val are set to 0.
> 
> [1] "Secure VM Service Module for SEV-SNP Guests"
>     Publication # 58019 Revision: 1.00
> 
> Signed-off-by: Stefano Garzarella <[e-mail]>
> ---
> v5:
> - removed cancel/status/req_* ops after rebase on master that cotains
>   commit 980a573621ea ("tpm: Make chip->{status,cancel,req_canceled} opt")
> v4:
> - moved "asm" includes after the "linux" includes [Tom]
> - allocated buffer separately [Tom/Jarkko/Jason]
> v3:
> - removed send_recv() ops and followed the ftpm driver implementing .status,
>   .req_complete_mask, .req_complete_val, etc. [Jarkko]
> - removed link to the spec because those URLs are unstable [Borislav]
> ---
>  drivers/char/tpm/tpm_svsm.c | 135 ++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/Kconfig    |  10 +++
>  drivers/char/tpm/Makefile   |   1 +
>  3 files changed, 146 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..04c532421ff2
> --- /dev/null
> +++ b/drivers/char/tpm/tpm_svsm.c
> @@ -0,0 +1,135 @@
> +// 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).
> + *
> + * [1] "Secure VM Service Module for SEV-SNP Guests"
> + *     Publication # 58019 Revision: 1.00
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/tpm_svsm.h>
> +
> +#include <asm/sev.h>
> +
> +#include "tpm.h"
> +
> +struct tpm_svsm_priv {
> +	void *buffer;
> +	u8 locality;
> +};
> +
> +static int tpm_svsm_send(struct tpm_chip *chip, u8 *buf, size_t len)
> +{
> +	struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
> +	int ret;
> +
> +	ret = svsm_vtpm_cmd_request_fill(priv->buffer, priv->locality, buf, len);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * The SVSM call uses the same buffer for the command and for the
> +	 * response, so after this call, the buffer will contain the response
> +	 * that can be used by .recv() op.
> +	 */
> +	return snp_svsm_vtpm_send_command(priv->buffer);
> +}
> +
> +static int tpm_svsm_recv(struct tpm_chip *chip, u8 *buf, size_t len)
> +{
> +	struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
> +
> +	/*
> +	 * The internal buffer contains the response after we send the command
> +	 * to SVSM.
> +	 */
> +	return svsm_vtpm_cmd_response_parse(priv->buffer, buf, len);
> +}
> +
> +static struct tpm_class_ops tpm_chip_ops = {
> +	.flags = TPM_OPS_AUTO_STARTUP,
> +	.recv = tpm_svsm_recv,
> +	.send = tpm_svsm_send,
> +};
> +
> +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;
> +
> +	/*
> +	 * The maximum buffer supported is one page (see SVSM_VTPM_MAX_BUFFER
> +	 * in tpm_svsm.h).
> +	 */
> +	priv->buffer = (void *)devm_get_free_pages(dev, GFP_KERNEL, 0);
> +	if (!priv->buffer)
> +		return -ENOMEM;
> +
> +	/*
> +	 * FIXME: before implementing locality we need to agree what it means
> +	 * for the SNP SVSM vTPM
> +	 */
> +	priv->locality = 0;

I don't think we want FIXME's to mainline. Instead, don't declare the
field at all if you don't use it. Just pass zero to *_request_fill().

Maybe "not have the field" is even a better reminder than a random fixme
comment?

> +
> +	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 fe4f3a609934..dddd702b2454 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -234,5 +234,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 2b004df8c04b..9de1b3ea34a9 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -45,3 +45,4 @@ obj-$(CONFIG_TCG_CRB) += tpm_crb.o
>  obj-$(CONFIG_TCG_ARM_CRB_FFA) += tpm_crb_ffa.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.49.0
> 

BR, Jarkko

Dionna Amalie Glaze March 31, 2025, 8:56 p.m. UTC | #2

On Mon, Mar 31, 2025 at 10:34 AM Jarkko Sakkinen <[e-mail]> wrote:
>
> On Mon, Mar 31, 2025 at 12:38:56PM +0200, Stefano Garzarella wrote:
> > From: Stefano Garzarella <[e-mail]>
> >
> > 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 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.
> >
> > This device does not support interrupts and sends responses to commands
> > synchronously. In order to have .recv() called just after .send() in
> > tpm_try_transmit(), the .status() callback returns 0, and both
> > .req_complete_mask and .req_complete_val are set to 0.
> >
> > [1] "Secure VM Service Module for SEV-SNP Guests"
> >     Publication # 58019 Revision: 1.00
> >
> > Signed-off-by: Stefano Garzarella <[e-mail]>
> > ---
> > v5:
> > - removed cancel/status/req_* ops after rebase on master that cotains
> >   commit 980a573621ea ("tpm: Make chip->{status,cancel,req_canceled} opt")
> > v4:
> > - moved "asm" includes after the "linux" includes [Tom]
> > - allocated buffer separately [Tom/Jarkko/Jason]
> > v3:
> > - removed send_recv() ops and followed the ftpm driver implementing .status,
> >   .req_complete_mask, .req_complete_val, etc. [Jarkko]
> > - removed link to the spec because those URLs are unstable [Borislav]
> > ---
> >  drivers/char/tpm/tpm_svsm.c | 135 ++++++++++++++++++++++++++++++++++++
> >  drivers/char/tpm/Kconfig    |  10 +++
> >  drivers/char/tpm/Makefile   |   1 +
> >  3 files changed, 146 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..04c532421ff2
> > --- /dev/null
> > +++ b/drivers/char/tpm/tpm_svsm.c
> > @@ -0,0 +1,135 @@
> > +// 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).
> > + *
> > + * [1] "Secure VM Service Module for SEV-SNP Guests"
> > + *     Publication # 58019 Revision: 1.00
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/tpm_svsm.h>
> > +
> > +#include <asm/sev.h>
> > +
> > +#include "tpm.h"
> > +
> > +struct tpm_svsm_priv {
> > +     void *buffer;
> > +     u8 locality;
> > +};
> > +
> > +static int tpm_svsm_send(struct tpm_chip *chip, u8 *buf, size_t len)
> > +{
> > +     struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
> > +     int ret;
> > +
> > +     ret = svsm_vtpm_cmd_request_fill(priv->buffer, priv->locality, buf, len);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /*
> > +      * The SVSM call uses the same buffer for the command and for the
> > +      * response, so after this call, the buffer will contain the response
> > +      * that can be used by .recv() op.
> > +      */
> > +     return snp_svsm_vtpm_send_command(priv->buffer);
> > +}
> > +
> > +static int tpm_svsm_recv(struct tpm_chip *chip, u8 *buf, size_t len)
> > +{
> > +     struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
> > +
> > +     /*
> > +      * The internal buffer contains the response after we send the command
> > +      * to SVSM.
> > +      */
> > +     return svsm_vtpm_cmd_response_parse(priv->buffer, buf, len);
> > +}
> > +
> > +static struct tpm_class_ops tpm_chip_ops = {
> > +     .flags = TPM_OPS_AUTO_STARTUP,
> > +     .recv = tpm_svsm_recv,
> > +     .send = tpm_svsm_send,
> > +};
> > +
> > +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;
> > +
> > +     /*
> > +      * The maximum buffer supported is one page (see SVSM_VTPM_MAX_BUFFER
> > +      * in tpm_svsm.h).
> > +      */
> > +     priv->buffer = (void *)devm_get_free_pages(dev, GFP_KERNEL, 0);
> > +     if (!priv->buffer)
> > +             return -ENOMEM;
> > +
> > +     /*
> > +      * FIXME: before implementing locality we need to agree what it means
> > +      * for the SNP SVSM vTPM
> > +      */
> > +     priv->locality = 0;
>
> I don't think we want FIXME's to mainline. Instead, don't declare the
> field at all if you don't use it. Just pass zero to *_request_fill().
>
> Maybe "not have the field" is even a better reminder than a random fixme
> comment?
>
> > +
> > +     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);

I might be unclear on how I should be testing this, but I do see
/dev/tpm0 and /dev/tpmrm0 when I build with CONFIG_TCG_SVSM=y, but I
don't see the event log in securityfs. What am I missing?

> > +
> > +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 fe4f3a609934..dddd702b2454 100644
> > --- a/drivers/char/tpm/Kconfig
> > +++ b/drivers/char/tpm/Kconfig
> > @@ -234,5 +234,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 2b004df8c04b..9de1b3ea34a9 100644
> > --- a/drivers/char/tpm/Makefile
> > +++ b/drivers/char/tpm/Makefile
> > @@ -45,3 +45,4 @@ obj-$(CONFIG_TCG_CRB) += tpm_crb.o
> >  obj-$(CONFIG_TCG_ARM_CRB_FFA) += tpm_crb_ffa.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.49.0
> >
>
> BR, Jarkko

James Bottomley March 31, 2025, 9:26 p.m. UTC | #3

On Mon, 2025-03-31 at 13:56 -0700, Dionna Amalie Glaze wrote:
[...]
> I might be unclear on how I should be testing this, but I do see
> /dev/tpm0 and /dev/tpmrm0 when I build with CONFIG_TCG_SVSM=y, but I
> don't see the event log in securityfs. What am I missing?

The vtpm driver for EDK2/OVMF I suspect ... without that the UEFI won't
lay down and event log for the kernel to pick up.

Regards,

James

Dionna Amalie Glaze March 31, 2025, 10:23 p.m. UTC | #4

On Mon, Mar 31, 2025 at 2:26 PM James Bottomley
<[e-mail]> wrote:
>
> On Mon, 2025-03-31 at 13:56 -0700, Dionna Amalie Glaze wrote:
> [...]
> > I might be unclear on how I should be testing this, but I do see
> > /dev/tpm0 and /dev/tpmrm0 when I build with CONFIG_TCG_SVSM=y, but I
> > don't see the event log in securityfs. What am I missing?
>
> The vtpm driver for EDK2/OVMF I suspect ... without that the UEFI won't
> lay down and event log for the kernel to pick up.

This test is with Oliver's PR https://github.com/tianocore/edk2/pull/6527

>
> Regards,
>
> James
>
>

James Bottomley March 31, 2025, 10:59 p.m. UTC | #5

On Mon, 2025-03-31 at 15:23 -0700, Dionna Amalie Glaze wrote:
> On Mon, Mar 31, 2025 at 2:26 PM James Bottomley
> <[e-mail]> wrote:
> > 
> > On Mon, 2025-03-31 at 13:56 -0700, Dionna Amalie Glaze wrote:
> > [...]
> > > I might be unclear on how I should be testing this, but I do see
> > > /dev/tpm0 and /dev/tpmrm0 when I build with CONFIG_TCG_SVSM=y,
> > > but I don't see the event log in securityfs. What am I missing?
> > 
> > The vtpm driver for EDK2/OVMF I suspect ... without that the UEFI
> > won't lay down and event log for the kernel to pick up.
> 
> This test is with Oliver's PR
> https://github.com/tianocore/edk2/pull/6527

Well, since the event log is searched for in tpm_chip_register(), I
really don't think it can be the kernel driver.  Best guess is there's
something wrong with that patch set (or the vTPM didn't activate in
OVMF for some reason).

Regards,

James

Stefano Garzarella April 1, 2025, 8:58 a.m. UTC | #6

On Tue, 1 Apr 2025 at 00:59, James Bottomley <[e-mail]> wrote:
>
> On Mon, 2025-03-31 at 15:23 -0700, Dionna Amalie Glaze wrote:
> > On Mon, Mar 31, 2025 at 2:26 PM James Bottomley
> > <[e-mail]> wrote:
> > >
> > > On Mon, 2025-03-31 at 13:56 -0700, Dionna Amalie Glaze wrote:
> > > [...]
> > > > I might be unclear on how I should be testing this, but I do see
> > > > /dev/tpm0 and /dev/tpmrm0 when I build with CONFIG_TCG_SVSM=y,
> > > > but I don't see the event log in securityfs. What am I missing?
> > >
> > > The vtpm driver for EDK2/OVMF I suspect ... without that the UEFI
> > > won't lay down and event log for the kernel to pick up.
> >
> > This test is with Oliver's PR
> > https://github.com/tianocore/edk2/pull/6527
>
> Well, since the event log is searched for in tpm_chip_register(), I
> really don't think it can be the kernel driver.  Best guess is there's
> something wrong with that patch set (or the vTPM didn't activate in
> OVMF for some reason).

Yep, I also think it should be something in edk2.

I'm using edk2 from https://github.com/coconut-svsm/edk2/pull/62 which 
should contain the commits from that PR + a fix not yet merged upstream.

I'm building it with:
build -a X64 -b DEBUG -t GCC5 -DTPM2_ENABLE \
  --pcd PcdUninstallMemAttrProtocol=TRUE -p OvmfPkg/OvmfPkgX64.dsc

And in Linux I see the devices and the event log:

# ls /dev/tpm*
/dev/tpm0  /dev/tpmrm0

# ls /sys/kernel/security/tpm0/
binary_bios_measurements

# tpm2_eventlog /sys/kernel/security/tpm0/binary_bios_measurements
---
version: 1
events:
- EventNum: 0
  PCRIndex: 0
  EventType: EV_NO_ACTION
  Digest: "0000000000000000000000000000000000000000"
  EventSize: 37
...

If I remove `-DTPM2_ENABLE` when building edk2, I can still see the 
/dev/tpm* devices (of course), but I can't see the event log anymore.
And also most PCRs are 0 (unlike when I have tpm driver enabled in 
edk2).

Thanks,
Stefano

Stefano Garzarella April 1, 2025, 9:08 a.m. UTC | #7

On Mon, Mar 31, 2025 at 08:34:42PM +0300, Jarkko Sakkinen wrote:
>On Mon, Mar 31, 2025 at 12:38:56PM +0200, Stefano Garzarella wrote:
>> From: Stefano Garzarella <[e-mail]>
>>
>> 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 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.
>>
>> This device does not support interrupts and sends responses to commands
>> synchronously. In order to have .recv() called just after .send() in
>> tpm_try_transmit(), the .status() callback returns 0, and both
>> .req_complete_mask and .req_complete_val are set to 0.
>>
>> [1] "Secure VM Service Module for SEV-SNP Guests"
>>     Publication # 58019 Revision: 1.00
>>
>> Signed-off-by: Stefano Garzarella <[e-mail]>
>> ---
>> v5:
>> - removed cancel/status/req_* ops after rebase on master that cotains
>>   commit 980a573621ea ("tpm: Make chip->{status,cancel,req_canceled} opt")
>> v4:
>> - moved "asm" includes after the "linux" includes [Tom]
>> - allocated buffer separately [Tom/Jarkko/Jason]
>> v3:
>> - removed send_recv() ops and followed the ftpm driver implementing .status,
>>   .req_complete_mask, .req_complete_val, etc. [Jarkko]
>> - removed link to the spec because those URLs are unstable [Borislav]
>> ---
>>  drivers/char/tpm/tpm_svsm.c | 135 ++++++++++++++++++++++++++++++++++++
>>  drivers/char/tpm/Kconfig    |  10 +++
>>  drivers/char/tpm/Makefile   |   1 +
>>  3 files changed, 146 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..04c532421ff2
>> --- /dev/null
>> +++ b/drivers/char/tpm/tpm_svsm.c
>> @@ -0,0 +1,135 @@
>> +// 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).
>> + *
>> + * [1] "Secure VM Service Module for SEV-SNP Guests"
>> + *     Publication # 58019 Revision: 1.00
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/tpm_svsm.h>
>> +
>> +#include <asm/sev.h>
>> +
>> +#include "tpm.h"
>> +
>> +struct tpm_svsm_priv {
>> +	void *buffer;
>> +	u8 locality;
>> +};
>> +
>> +static int tpm_svsm_send(struct tpm_chip *chip, u8 *buf, size_t len)
>> +{
>> +	struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
>> +	int ret;
>> +
>> +	ret = svsm_vtpm_cmd_request_fill(priv->buffer, priv->locality, buf, len);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/*
>> +	 * The SVSM call uses the same buffer for the command and for the
>> +	 * response, so after this call, the buffer will contain the response
>> +	 * that can be used by .recv() op.
>> +	 */
>> +	return snp_svsm_vtpm_send_command(priv->buffer);
>> +}
>> +
>> +static int tpm_svsm_recv(struct tpm_chip *chip, u8 *buf, size_t len)
>> +{
>> +	struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
>> +
>> +	/*
>> +	 * The internal buffer contains the response after we send the command
>> +	 * to SVSM.
>> +	 */
>> +	return svsm_vtpm_cmd_response_parse(priv->buffer, buf, len);
>> +}
>> +
>> +static struct tpm_class_ops tpm_chip_ops = {
>> +	.flags = TPM_OPS_AUTO_STARTUP,
>> +	.recv = tpm_svsm_recv,
>> +	.send = tpm_svsm_send,
>> +};
>> +
>> +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;
>> +
>> +	/*
>> +	 * The maximum buffer supported is one page (see SVSM_VTPM_MAX_BUFFER
>> +	 * in tpm_svsm.h).
>> +	 */
>> +	priv->buffer = (void *)devm_get_free_pages(dev, GFP_KERNEL, 0);
>> +	if (!priv->buffer)
>> +		return -ENOMEM;
>> +
>> +	/*
>> +	 * FIXME: before implementing locality we need to agree what it means
>> +	 * for the SNP SVSM vTPM
>> +	 */
>> +	priv->locality = 0;
>
>I don't think we want FIXME's to mainline. Instead, don't declare the
>field at all if you don't use it. Just pass zero to *_request_fill().
>
>Maybe "not have the field" is even a better reminder than a random fixme
>comment?

Yeah, I had thought the same, but then I left it that way because it was 
there from the first RFC and I saw several FIXME in the codebase, but I 
agree with you, I'll remove the field completely in v6.

That said, `struct tpm_svsm_priv` with this change will only contain the 
pointer to the buffer, does it make sense to have that structure (maybe 
for the future it's easier to add new fields), or do I remove it and use 
dev_set_drvdata() to store the pointer to the buffer directly?

Thanks,
Stefano

Jarkko Sakkinen April 1, 2025, 2:48 p.m. UTC | #8

On Tue, Apr 01, 2025 at 11:08:49AM +0200, Stefano Garzarella wrote:
> On Mon, Mar 31, 2025 at 08:34:42PM +0300, Jarkko Sakkinen wrote:
> > On Mon, Mar 31, 2025 at 12:38:56PM +0200, Stefano Garzarella wrote:
> > > From: Stefano Garzarella <[e-mail]>
> > > 
> > > 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 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.
> > > 
> > > This device does not support interrupts and sends responses to commands
> > > synchronously. In order to have .recv() called just after .send() in
> > > tpm_try_transmit(), the .status() callback returns 0, and both
> > > .req_complete_mask and .req_complete_val are set to 0.
> > > 
> > > [1] "Secure VM Service Module for SEV-SNP Guests"
> > >     Publication # 58019 Revision: 1.00
> > > 
> > > Signed-off-by: Stefano Garzarella <[e-mail]>
> > > ---
> > > v5:
> > > - removed cancel/status/req_* ops after rebase on master that cotains
> > >   commit 980a573621ea ("tpm: Make chip->{status,cancel,req_canceled} opt")
> > > v4:
> > > - moved "asm" includes after the "linux" includes [Tom]
> > > - allocated buffer separately [Tom/Jarkko/Jason]
> > > v3:
> > > - removed send_recv() ops and followed the ftpm driver implementing .status,
> > >   .req_complete_mask, .req_complete_val, etc. [Jarkko]
> > > - removed link to the spec because those URLs are unstable [Borislav]
> > > ---
> > >  drivers/char/tpm/tpm_svsm.c | 135 ++++++++++++++++++++++++++++++++++++
> > >  drivers/char/tpm/Kconfig    |  10 +++
> > >  drivers/char/tpm/Makefile   |   1 +
> > >  3 files changed, 146 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..04c532421ff2
> > > --- /dev/null
> > > +++ b/drivers/char/tpm/tpm_svsm.c
> > > @@ -0,0 +1,135 @@
> > > +// 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).
> > > + *
> > > + * [1] "Secure VM Service Module for SEV-SNP Guests"
> > > + *     Publication # 58019 Revision: 1.00
> > > + */
> > > +
> > > +#include <linux/module.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/tpm_svsm.h>
> > > +
> > > +#include <asm/sev.h>
> > > +
> > > +#include "tpm.h"
> > > +
> > > +struct tpm_svsm_priv {
> > > +	void *buffer;
> > > +	u8 locality;
> > > +};
> > > +
> > > +static int tpm_svsm_send(struct tpm_chip *chip, u8 *buf, size_t len)
> > > +{
> > > +	struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
> > > +	int ret;
> > > +
> > > +	ret = svsm_vtpm_cmd_request_fill(priv->buffer, priv->locality, buf, len);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/*
> > > +	 * The SVSM call uses the same buffer for the command and for the
> > > +	 * response, so after this call, the buffer will contain the response
> > > +	 * that can be used by .recv() op.
> > > +	 */
> > > +	return snp_svsm_vtpm_send_command(priv->buffer);
> > > +}
> > > +
> > > +static int tpm_svsm_recv(struct tpm_chip *chip, u8 *buf, size_t len)
> > > +{
> > > +	struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
> > > +
> > > +	/*
> > > +	 * The internal buffer contains the response after we send the command
> > > +	 * to SVSM.
> > > +	 */
> > > +	return svsm_vtpm_cmd_response_parse(priv->buffer, buf, len);
> > > +}
> > > +
> > > +static struct tpm_class_ops tpm_chip_ops = {
> > > +	.flags = TPM_OPS_AUTO_STARTUP,
> > > +	.recv = tpm_svsm_recv,
> > > +	.send = tpm_svsm_send,
> > > +};
> > > +
> > > +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;
> > > +
> > > +	/*
> > > +	 * The maximum buffer supported is one page (see SVSM_VTPM_MAX_BUFFER
> > > +	 * in tpm_svsm.h).
> > > +	 */
> > > +	priv->buffer = (void *)devm_get_free_pages(dev, GFP_KERNEL, 0);
> > > +	if (!priv->buffer)
> > > +		return -ENOMEM;
> > > +
> > > +	/*
> > > +	 * FIXME: before implementing locality we need to agree what it means
> > > +	 * for the SNP SVSM vTPM
> > > +	 */
> > > +	priv->locality = 0;
> > 
> > I don't think we want FIXME's to mainline. Instead, don't declare the
> > field at all if you don't use it. Just pass zero to *_request_fill().
> > 
> > Maybe "not have the field" is even a better reminder than a random fixme
> > comment?
> 
> Yeah, I had thought the same, but then I left it that way because it was
> there from the first RFC and I saw several FIXME in the codebase, but I
> agree with you, I'll remove the field completely in v6.
> 
> That said, `struct tpm_svsm_priv` with this change will only contain the
> pointer to the buffer, does it make sense to have that structure (maybe for
> the future it's easier to add new fields), or do I remove it and use
> dev_set_drvdata() to store the pointer to the buffer directly?

I'll put it like this: I would not NAK this for having a struct with
a single field. Either way works for me. 

> 
> Thanks,
> Stefano
> 

BR, Jarkko

Patch

diff --git a/drivers/char/tpm/tpm_svsm.c b/drivers/char/tpm/tpm_svsm.c
new file mode 100644
index 000000000000..04c532421ff2
--- /dev/null
+++ b/drivers/char/tpm/tpm_svsm.c
@@ -0,0 +1,135 @@ 
+// 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).
+ *
+ * [1] "Secure VM Service Module for SEV-SNP Guests"
+ *     Publication # 58019 Revision: 1.00
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/tpm_svsm.h>
+
+#include <asm/sev.h>
+
+#include "tpm.h"
+
+struct tpm_svsm_priv {
+	void *buffer;
+	u8 locality;
+};
+
+static int tpm_svsm_send(struct tpm_chip *chip, u8 *buf, size_t len)
+{
+	struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
+	int ret;
+
+	ret = svsm_vtpm_cmd_request_fill(priv->buffer, priv->locality, buf, len);
+	if (ret)
+		return ret;
+
+	/*
+	 * The SVSM call uses the same buffer for the command and for the
+	 * response, so after this call, the buffer will contain the response
+	 * that can be used by .recv() op.
+	 */
+	return snp_svsm_vtpm_send_command(priv->buffer);
+}
+
+static int tpm_svsm_recv(struct tpm_chip *chip, u8 *buf, size_t len)
+{
+	struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
+
+	/*
+	 * The internal buffer contains the response after we send the command
+	 * to SVSM.
+	 */
+	return svsm_vtpm_cmd_response_parse(priv->buffer, buf, len);
+}
+
+static struct tpm_class_ops tpm_chip_ops = {
+	.flags = TPM_OPS_AUTO_STARTUP,
+	.recv = tpm_svsm_recv,
+	.send = tpm_svsm_send,
+};
+
+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;
+
+	/*
+	 * The maximum buffer supported is one page (see SVSM_VTPM_MAX_BUFFER
+	 * in tpm_svsm.h).
+	 */
+	priv->buffer = (void *)devm_get_free_pages(dev, GFP_KERNEL, 0);
+	if (!priv->buffer)
+		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 fe4f3a609934..dddd702b2454 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -234,5 +234,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 2b004df8c04b..9de1b3ea34a9 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -45,3 +45,4 @@  obj-$(CONFIG_TCG_CRB) += tpm_crb.o
 obj-$(CONFIG_TCG_ARM_CRB_FFA) += tpm_crb_ffa.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