linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] Enlightened vTPM support for SVSM on SEV-SNP
@ 2025-03-24 10:46 Stefano Garzarella
  2025-03-24 10:46 ` [PATCH v4 1/4] x86/sev: add SVSM vTPM probe/send_command functions Stefano Garzarella
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Stefano Garzarella @ 2025-03-24 10:46 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Joerg Roedel, Ingo Molnar, Dave Hansen, Peter Huewe, Tom Lendacky,
	Thomas Gleixner, x86, James Bottomley, linux-kernel,
	Borislav Petkov, Jason Gunthorpe, H. Peter Anvin, linux-coco,
	Claudio Carvalho, Dov Murik, Dionna Glaze, linux-integrity,
	Stefano Garzarella

AMD SEV-SNP defined a new mechanism for adding privileged levels (VMPLs)
in the context of a Confidential VM. These levels can be used to run the
guest OS at a lower privilege level than a Secure VM Service Module (SVSM).
In this way SVSM can be used to emulate those devices (such as TPM) that
cannot be delegated to an untrusted host.

The guest OS can talk to SVSM using a specific calling convention and
instructions (a kind of system call/hyper call) and request services such
as TPM emulation.

The main goal of this series is to add a driver for the vTPM defined by
the AMD SVSM spec [3]. The specification defines a protocol that a
SEV-SNP guest OS (running on VMPL >= 1) can use to discover and talk to
a vTPM emulated by the SVSM in the guest context, but at a more
privileged level (VMPL0).

This series is based on the RFC sent by James last year [1].
In the meantime, the patches have been maintained and tested in the
Coconut Linux fork [2] along with the work to support the vTPM
emulation in Coconut SVSM.

The first patch adds public APIs to use AMD SVSM vTPM. They use
SVSM_VTPM_QUERY call to probe for the vTPM device and SVSM_VTPM_CMD call
to execute vTPM operations as defined in the AMD SVSM spec [3].
The second patch adds an interface with helpers for the SVSM_VTPM_CMD calls
used by the vTPM protocol defined by the AMD SVSM spec and then used by the
third patch to implement the SVSM vTPM driver. The fourth patch simply
registers the platform device.

Since all SEV-SNP dependencies are now upstream, this series can be
applied directly to the Linus' tree.

These patches were tested in an AMD SEV-SNP guest running:
- a recent version of Coconut SVSM [4] containing an ephemeral vTPM
- a PoC [5] containing a stateful vTPM used for sealing/unsealing a LUKS key

Changelog:

v3 -> v4
- Added more documentation around public API [Jarkko]
- Simplified TPM_SEND_COMMAND check [Tom/Jarkko]
- Renamed header in tpm_svsm.h so it will fall under TPM DEVICE DRIVER
  section [Borislav, Jarkko]
- Fixed several comments to improve svsm_vtpm_ helpers and structures [Jarkko]
- Moved "asm" includes after the "linux" includes [Tom]
- Allocated buffer used in the driver separately [Tom/Jarkko/Jason]
- Explained better why we register tpm-svsm device anyway in the commit message

v2 RFC -> v3: https://lore.kernel.org/linux-integrity/20250311094225.35129-1-sgarzare@redhat.com/
- Removed send_recv() ops and followed the ftpm driver implementing .status,
  .req_complete_mask, .req_complete_val, etc. [Jarkko]
  As we agreed, I will send another series with that patch to continue the
  discussion along with the changes in this driver and ftpm driver.
- Renamed fill/parse functions [Tom]
- Renamed helpers header and prefix to make clear it's related to the
  SVSM vTPM protocol and not to the TCG TPM Simulator
- Slimmed down snp_svsm_vtpm_probe() [Borislav]
- Removed link to the spec because those URLs are unstable [Borislav]
- Removed features check and any print related [Tom]
- Squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch
  with the next one [Borislav]

v1 -> v2 RFC: https://lore.kernel.org/linux-integrity/20250228170720.144739-1-sgarzare@redhat.com/
- Added send_recv() tpm_class_ops callback
- Removed the intermediate tpm_platform.ko driver
- Renamed tpm_platform.h to tpm_tcgsim.h and included some API to fill
  TPM_SEND_COMMAND requests and parse responses from a device emulated using
  the TCG Simulatore reference implementation
- Added public API in x86/sev usable to discover and talk with the SVSM vTPM
- Added the tpm-svsm platform driver in driver/char/tpm/
- Fixed some SVSM TPM related issues (resp_size as u32, don't fail on
  features !=0, s/VTPM/vTPM)

v0 RFC -> v1: https://lore.kernel.org/linux-integrity/20241210143423.101774-1-sgarzare@redhat.com/
- Used SVSM_VTPM_QUERY to probe the TPM as Tom Lendacky suggested
- Changed references/links to TCG TPM repo since in the last year MS
  donated the reference TPM implementation to the TCG.
- Addressed Dov Murik's comments:
  https://lore.kernel.org/all/f7d0bd07-ba1b-894e-5e39-15fb1817bc8b@linux.ibm.com/
- Added a new patch with SVSM call macros for the vTPM protocol, following
  what we already have for SVSM_CORE and SVSM_ATTEST
- Rebased on v6.13-rc2

Thanks,
Stefano

[1] https://lore.kernel.org/all/acb06bc7f329dfee21afa1b2ff080fe29b799021.camel@linux.ibm.com/
[2] https://github.com/coconut-svsm/linux/tree/svsm
[3] "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
[4] https://github.com/coconut-svsm/svsm/commit/6522c67e1e414f192a6f014b122ca8a1066e3bf5
[5] https://github.com/stefano-garzarella/snp-svsm-vtpm

Stefano Garzarella (4):
  x86/sev: add SVSM vTPM probe/send_command functions
  svsm: add header with SVSM_VTPM_CMD helpers
  tpm: add SNP SVSM vTPM driver
  x86/sev: register tpm-svsm platform device

 arch/x86/include/asm/sev.h  |   7 ++
 include/linux/tpm_svsm.h    | 149 ++++++++++++++++++++++++++++++++++
 arch/x86/coco/sev/core.c    |  67 ++++++++++++++++
 drivers/char/tpm/tpm_svsm.c | 155 ++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/Kconfig    |  10 +++
 drivers/char/tpm/Makefile   |   1 +
 6 files changed, 389 insertions(+)
 create mode 100644 include/linux/tpm_svsm.h
 create mode 100644 drivers/char/tpm/tpm_svsm.c

-- 
2.49.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4 1/4] x86/sev: add SVSM vTPM probe/send_command functions
  2025-03-24 10:46 [PATCH v4 0/4] Enlightened vTPM support for SVSM on SEV-SNP Stefano Garzarella
@ 2025-03-24 10:46 ` Stefano Garzarella
  2025-03-25 16:56   ` Dionna Amalie Glaze
  2025-03-26 17:12   ` Jarkko Sakkinen
  2025-03-24 10:46 ` [PATCH v4 2/4] svsm: add header with SVSM_VTPM_CMD helpers Stefano Garzarella
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 16+ messages in thread
From: Stefano Garzarella @ 2025-03-24 10:46 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Joerg Roedel, Ingo Molnar, Dave Hansen, Peter Huewe, Tom Lendacky,
	Thomas Gleixner, x86, James Bottomley, linux-kernel,
	Borislav Petkov, Jason Gunthorpe, H. Peter Anvin, linux-coco,
	Claudio Carvalho, Dov Murik, Dionna Glaze, linux-integrity,
	Stefano Garzarella

From: Stefano Garzarella <sgarzare@redhat.com>

Add two new functions to probe and send commands to the SVSM vTPM.
They leverage the two calls defined by the AMD SVSM specification [1]
for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD.

Expose these functions to be used by other modules such as a tpm
driver.

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

Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v4:
- added Tom's R-b
- added functions documentation [Jarkko]
- simplified TPM_SEND_COMMAND check [Tom/Jarkko]
v3:
- removed link to the spec because those URLs are unstable [Borislav]
- squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch
  in this one [Borislav]
- slimmed down snp_svsm_vtpm_probe() [Borislav]
- removed features check and any print related [Tom]
---
 arch/x86/include/asm/sev.h |  7 +++++
 arch/x86/coco/sev/core.c   | 59 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index ba7999f66abe..09471d058ce5 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -384,6 +384,10 @@ struct svsm_call {
 #define SVSM_ATTEST_SERVICES		0
 #define SVSM_ATTEST_SINGLE_SERVICE	1
 
+#define SVSM_VTPM_CALL(x)		((2ULL << 32) | (x))
+#define SVSM_VTPM_QUERY			0
+#define SVSM_VTPM_CMD			1
+
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 
 extern u8 snp_vmpl;
@@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc);
 int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req,
 			   struct snp_guest_request_ioctl *rio);
 
+bool snp_svsm_vtpm_probe(void);
+int snp_svsm_vtpm_send_command(u8 *buffer);
+
 void __init snp_secure_tsc_prepare(void);
 void __init snp_secure_tsc_init(void);
 
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 96c7bc698e6b..034aab7e76d2 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -2628,6 +2628,65 @@ static int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_dat
 	return ret;
 }
 
+/**
+ * snp_svsm_vtpm_probe() - Probe if SVSM provides a vTPM device
+ *
+ * This function checks that there is SVSM and that it supports at least
+ * TPM_SEND_COMMAND which is the only request we use so far.
+ *
+ * Return: true if the platform provides a vTPM SVSM device, false otherwise.
+ */
+bool snp_svsm_vtpm_probe(void)
+{
+	struct svsm_call call = {};
+
+	/* The vTPM device is available only if a SVSM is present */
+	if (!snp_vmpl)
+		return false;
+
+	call.caa = svsm_get_caa();
+	call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY);
+
+	if (svsm_perform_call_protocol(&call))
+		return false;
+
+	/* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */
+	return call.rcx_out & BIT_ULL(8);
+}
+EXPORT_SYMBOL_GPL(snp_svsm_vtpm_probe);
+
+/**
+ * snp_svsm_vtpm_send_command() - execute a vTPM operation on SVSM
+ * @buffer: A buffer used to both send the command and receive the response.
+ *
+ * This function executes a SVSM_VTPM_CMD call as defined by
+ * "Secure VM Service Module for SEV-SNP Guests" Publication # 58019 Revision: 1.00
+ *
+ * All command request/response buffers have a common structure as specified by
+ * the following table:
+ *     Byte      Size       In/Out    Description
+ *     Offset    (Bytes)
+ *     0x000     4          In        Platform command
+ *                          Out       Platform command response size
+ *
+ * Each command can build upon this common request/response structure to create
+ * a structure specific to the command.
+ * See include/linux/tpm_svsm.h for more details.
+ *
+ * Return: 0 on success, -errno on failure
+ */
+int snp_svsm_vtpm_send_command(u8 *buffer)
+{
+	struct svsm_call call = {};
+
+	call.caa = svsm_get_caa();
+	call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD);
+	call.rcx = __pa(buffer);
+
+	return svsm_perform_call_protocol(&call);
+}
+EXPORT_SYMBOL_GPL(snp_svsm_vtpm_send_command);
+
 static struct platform_device sev_guest_device = {
 	.name		= "sev-guest",
 	.id		= -1,
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 2/4] svsm: add header with SVSM_VTPM_CMD helpers
  2025-03-24 10:46 [PATCH v4 0/4] Enlightened vTPM support for SVSM on SEV-SNP Stefano Garzarella
  2025-03-24 10:46 ` [PATCH v4 1/4] x86/sev: add SVSM vTPM probe/send_command functions Stefano Garzarella
@ 2025-03-24 10:46 ` Stefano Garzarella
  2025-03-26 19:27   ` Jarkko Sakkinen
  2025-03-24 10:46 ` [PATCH v4 3/4] tpm: add SNP SVSM vTPM driver Stefano Garzarella
  2025-03-24 10:46 ` [PATCH v4 4/4] x86/sev: register tpm-svsm platform device Stefano Garzarella
  3 siblings, 1 reply; 16+ messages in thread
From: Stefano Garzarella @ 2025-03-24 10:46 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Joerg Roedel, Ingo Molnar, Dave Hansen, Peter Huewe, Tom Lendacky,
	Thomas Gleixner, x86, James Bottomley, linux-kernel,
	Borislav Petkov, Jason Gunthorpe, H. Peter Anvin, linux-coco,
	Claudio Carvalho, Dov Murik, Dionna Glaze, linux-integrity,
	Stefano Garzarella

From: Stefano Garzarella <sgarzare@redhat.com>

Helpers for the SVSM_VTPM_CMD calls used by the vTPM protocol defined by
the AMD SVSM spec [1].

The vTPM protocol follows the Official TPM 2.0 Reference Implementation
(originally by Microsoft, now part of the TCG) simulator protocol.

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

Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v4:
- used svsm_vtpm_ prefix consistently [Jarkko]
- removed __packed where not needed [Jarkko]
- expanded headers to avoid obfuscation [Jarkko]
- used `buf` instead of `inbuf`/`outbuf` [Jarkko]
- added more documentation quoting the specification
- removed TPM_* macros since we only use TPM_SEND_COMMAND in one place
  and don't want dependencies on external headers, but put the value
  directly as specified in the AMD SVSM specification
- header renamed in tpm_svsm.h so it will fall under TPM DEVICE DRIVER
  section [Borislav, Jarkko]
v3:
- renamed header and prefix to make clear it's related to the SVSM vTPM
  protocol
- renamed fill/parse functions [Tom]
- removed link to the spec because those URLs are unstable [Borislav]
---
 include/linux/tpm_svsm.h | 149 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 149 insertions(+)
 create mode 100644 include/linux/tpm_svsm.h

diff --git a/include/linux/tpm_svsm.h b/include/linux/tpm_svsm.h
new file mode 100644
index 000000000000..38e341f9761a
--- /dev/null
+++ b/include/linux/tpm_svsm.h
@@ -0,0 +1,149 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2023 James.Bottomley@HansenPartnership.com
+ * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved.
+ *
+ * Helpers for the SVSM_VTPM_CMD calls used by the vTPM protocol defined by the
+ * AMD SVSM spec [1].
+ *
+ * The vTPM protocol follows the Official TPM 2.0 Reference Implementation
+ * (originally by Microsoft, now part of the TCG) simulator protocol.
+ *
+ * [1] "Secure VM Service Module for SEV-SNP Guests"
+ *     Publication # 58019 Revision: 1.00
+ */
+#ifndef _TPM_SVSM_H_
+#define _TPM_SVSM_H_
+
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#define SVSM_VTPM_MAX_BUFFER		4096 /* max req/resp buffer size */
+
+/**
+ * struct svsm_vtpm_request - Generic request for single word command
+ * @cmd:	The command to send
+ *
+ * Defined by AMD SVSM spec [1] in section "8.2 SVSM_VTPM_CMD Call" -
+ * Table 15: vTPM Common Request/Response Structure
+ *     Byte      Size       In/Out    Description
+ *     Offset    (Bytes)
+ *     0x000     4          In        Platform command
+ *                          Out       Platform command response size
+ */
+struct svsm_vtpm_request {
+	u32 cmd;
+};
+
+/**
+ * struct svsm_vtpm_response - Generic response
+ * @size:	The response size (zero if nothing follows)
+ *
+ * Defined by AMD SVSM spec [1] in section "8.2 SVSM_VTPM_CMD Call" -
+ * Table 15: vTPM Common Request/Response Structure
+ *     Byte      Size       In/Out    Description
+ *     Offset    (Bytes)
+ *     0x000     4          In        Platform command
+ *                          Out       Platform command response size
+ *
+ * Note: most TCG Simulator commands simply return zero here with no indication
+ * of success or failure.
+ */
+struct svsm_vtpm_response {
+	u32 size;
+};
+
+/**
+ * struct svsm_vtpm_cmd_request - Structure for a TPM_SEND_COMMAND request
+ * @cmd:	The command to send (must be TPM_SEND_COMMAND)
+ * @locality:	The locality
+ * @buf_size:	The size of the input buffer following
+ * @buf:	A buffer of size buf_size
+ *
+ * Defined by AMD SVSM spec [1] in section "8.2 SVSM_VTPM_CMD Call" -
+ * Table 16: TPM_SEND_COMMAND Request Structure
+ *     Byte      Size       Meaning
+ *     Offset    (Bytes)
+ *     0x000     4          Platform command (8)
+ *     0x004     1          Locality (must-be-0)
+ *     0x005     4          TPM Command size (in bytes)
+ *     0x009     Variable   TPM Command
+ *
+ * Note: the TCG Simulator expects @buf_size to be equal to the size of the
+ * specific TPM command, otherwise an TPM_RC_COMMAND_SIZE error is returned.
+ */
+struct svsm_vtpm_cmd_request {
+	u32 cmd;
+	u8 locality;
+	u32 buf_size;
+	u8 buf[];
+} __packed;
+
+/**
+ * struct svsm_vtpm_cmd_response - Structure for a TPM_SEND_COMMAND response
+ * @buf_size:	The size of the output buffer following
+ * @buf:	A buffer of size buf_size
+ *
+ * Defined by AMD SVSM spec [1] in section "8.2 SVSM_VTPM_CMD Call" -
+ * Table 17: TPM_SEND_COMMAND Response Structure
+ *     Byte      Size       Meaning
+ *     Offset    (Bytes)
+ *     0x000     4          Response size (in bytes)
+ *     0x004     Variable   Response
+ */
+struct svsm_vtpm_cmd_response {
+	u32 buf_size;
+	u8 buf[];
+};
+
+/**
+ * svsm_vtpm_cmd_request_fill() - Fill a TPM_SEND_COMMAND request to be sent to SVSM
+ * @req: The struct svsm_vtpm_cmd_request to fill
+ * @locality: The locality
+ * @buf: The buffer from where to copy the payload of the command
+ * @len: The size of the buffer
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+static inline int
+svsm_vtpm_cmd_request_fill(struct svsm_vtpm_cmd_request *req, u8 locality,
+			   const u8 *buf, size_t len)
+{
+	if (len > SVSM_VTPM_MAX_BUFFER - sizeof(*req))
+		return -EINVAL;
+
+	req->cmd = 8; /* TPM_SEND_COMMAND */
+	req->locality = locality;
+	req->buf_size = len;
+
+	memcpy(req->buf, buf, len);
+
+	return 0;
+}
+
+/**
+ * svsm_vtpm_cmd_response_parse() - Parse a TPM_SEND_COMMAND response received from SVSM
+ * @resp: The struct svsm_vtpm_cmd_response to parse
+ * @buf: The buffer where to copy the response
+ * @len: The size of the buffer
+ *
+ * Return: buffer size filled with the response on success, negative error
+ * code on failure.
+ */
+static inline int
+svsm_vtpm_cmd_response_parse(const struct svsm_vtpm_cmd_response *resp, u8 *buf,
+			     size_t len)
+{
+	if (len < resp->buf_size)
+		return -E2BIG;
+
+	if (resp->buf_size > SVSM_VTPM_MAX_BUFFER - sizeof(*resp))
+		return -EINVAL;  // Invalid response from the platform TPM
+
+	memcpy(buf, resp->buf, resp->buf_size);
+
+	return resp->buf_size;
+}
+
+#endif /* _TPM_SVSM_H_ */
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 3/4] tpm: add SNP SVSM vTPM driver
  2025-03-24 10:46 [PATCH v4 0/4] Enlightened vTPM support for SVSM on SEV-SNP Stefano Garzarella
  2025-03-24 10:46 ` [PATCH v4 1/4] x86/sev: add SVSM vTPM probe/send_command functions Stefano Garzarella
  2025-03-24 10:46 ` [PATCH v4 2/4] svsm: add header with SVSM_VTPM_CMD helpers Stefano Garzarella
@ 2025-03-24 10:46 ` Stefano Garzarella
  2025-03-26 19:30   ` Jarkko Sakkinen
  2025-03-24 10:46 ` [PATCH v4 4/4] x86/sev: register tpm-svsm platform device Stefano Garzarella
  3 siblings, 1 reply; 16+ messages in thread
From: Stefano Garzarella @ 2025-03-24 10:46 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Joerg Roedel, Ingo Molnar, Dave Hansen, Peter Huewe, Tom Lendacky,
	Thomas Gleixner, x86, James Bottomley, linux-kernel,
	Borislav Petkov, Jason Gunthorpe, H. Peter Anvin, linux-coco,
	Claudio Carvalho, Dov Murik, Dionna Glaze, linux-integrity,
	Stefano Garzarella

From: Stefano Garzarella <sgarzare@redhat.com>

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 <sgarzare@redhat.com>
---
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 | 155 ++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/Kconfig    |  10 +++
 drivers/char/tpm/Makefile   |   1 +
 3 files changed, 166 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..1281ff265927
--- /dev/null
+++ b/drivers/char/tpm/tpm_svsm.c
@@ -0,0 +1,155 @@
+// 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 void tpm_svsm_cancel(struct tpm_chip *chip)
+{
+	/* not supported */
+}
+
+static u8 tpm_svsm_status(struct tpm_chip *chip)
+{
+	return 0;
+}
+
+static bool tpm_svsm_req_canceled(struct tpm_chip *chip, u8 status)
+{
+	return false;
+}
+
+static struct tpm_class_ops tpm_chip_ops = {
+	.flags = TPM_OPS_AUTO_STARTUP,
+	.recv = tpm_svsm_recv,
+	.send = tpm_svsm_send,
+	.cancel = tpm_svsm_cancel,
+	.status = tpm_svsm_status,
+	.req_complete_mask = 0,
+	.req_complete_val = 0,
+	.req_canceled = tpm_svsm_req_canceled,
+};
+
+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 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.49.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 4/4] x86/sev: register tpm-svsm platform device
  2025-03-24 10:46 [PATCH v4 0/4] Enlightened vTPM support for SVSM on SEV-SNP Stefano Garzarella
                   ` (2 preceding siblings ...)
  2025-03-24 10:46 ` [PATCH v4 3/4] tpm: add SNP SVSM vTPM driver Stefano Garzarella
@ 2025-03-24 10:46 ` Stefano Garzarella
  3 siblings, 0 replies; 16+ messages in thread
From: Stefano Garzarella @ 2025-03-24 10:46 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Joerg Roedel, Ingo Molnar, Dave Hansen, Peter Huewe, Tom Lendacky,
	Thomas Gleixner, x86, James Bottomley, linux-kernel,
	Borislav Petkov, Jason Gunthorpe, H. Peter Anvin, linux-coco,
	Claudio Carvalho, Dov Murik, Dionna Glaze, linux-integrity,
	Stefano Garzarella

From: Stefano Garzarella <sgarzare@redhat.com>

SNP platform can provide a vTPM device emulated by SVSM.

The "tpm-svsm" device can be handled by the platform driver added
by the previous commit in drivers/char/tpm/tpm_svsm.c

Register the device unconditionally. The support check (e.g. SVSM, cmd)
is in snp_svsm_vtpm_probe(), keeping all logic in one place.
This function is called during the driver's probe along with other
setup tasks like memory allocation.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v4:
- explained better why we register it anyway in the commit message
---
 arch/x86/coco/sev/core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 034aab7e76d2..0abcac87af89 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -2692,6 +2692,11 @@ static struct platform_device sev_guest_device = {
 	.id		= -1,
 };
 
+static struct platform_device tpm_svsm_device = {
+	.name		= "tpm-svsm",
+	.id		= -1,
+};
+
 static int __init snp_init_platform_device(void)
 {
 	if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
@@ -2700,6 +2705,9 @@ static int __init snp_init_platform_device(void)
 	if (platform_device_register(&sev_guest_device))
 		return -ENODEV;
 
+	if (platform_device_register(&tpm_svsm_device))
+		return -ENODEV;
+
 	pr_info("SNP guest platform device initialized.\n");
 	return 0;
 }
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/4] x86/sev: add SVSM vTPM probe/send_command functions
  2025-03-24 10:46 ` [PATCH v4 1/4] x86/sev: add SVSM vTPM probe/send_command functions Stefano Garzarella
@ 2025-03-25 16:56   ` Dionna Amalie Glaze
  2025-03-25 17:20     ` Stefano Garzarella
  2025-03-26 17:12   ` Jarkko Sakkinen
  1 sibling, 1 reply; 16+ messages in thread
From: Dionna Amalie Glaze @ 2025-03-25 16:56 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Jarkko Sakkinen, Joerg Roedel, Ingo Molnar, Dave Hansen,
	Peter Huewe, Tom Lendacky, Thomas Gleixner, x86, James Bottomley,
	linux-kernel, Borislav Petkov, Jason Gunthorpe, H. Peter Anvin,
	linux-coco, Claudio Carvalho, Dov Murik, linux-integrity

On Mon, Mar 24, 2025 at 3:47 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> From: Stefano Garzarella <sgarzare@redhat.com>
>
> Add two new functions to probe and send commands to the SVSM vTPM.
> They leverage the two calls defined by the AMD SVSM specification [1]
> for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD.
>
> Expose these functions to be used by other modules such as a tpm
> driver.
>
> [1] "Secure VM Service Module for SEV-SNP Guests"
>     Publication # 58019 Revision: 1.00
>
> Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> v4:
> - added Tom's R-b
> - added functions documentation [Jarkko]
> - simplified TPM_SEND_COMMAND check [Tom/Jarkko]
> v3:
> - removed link to the spec because those URLs are unstable [Borislav]
> - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch
>   in this one [Borislav]
> - slimmed down snp_svsm_vtpm_probe() [Borislav]
> - removed features check and any print related [Tom]
> ---
>  arch/x86/include/asm/sev.h |  7 +++++
>  arch/x86/coco/sev/core.c   | 59 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
>
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index ba7999f66abe..09471d058ce5 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -384,6 +384,10 @@ struct svsm_call {
>  #define SVSM_ATTEST_SERVICES           0
>  #define SVSM_ATTEST_SINGLE_SERVICE     1
>
> +#define SVSM_VTPM_CALL(x)              ((2ULL << 32) | (x))
> +#define SVSM_VTPM_QUERY                        0
> +#define SVSM_VTPM_CMD                  1
> +
>  #ifdef CONFIG_AMD_MEM_ENCRYPT
>
>  extern u8 snp_vmpl;
> @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc);
>  int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req,
>                            struct snp_guest_request_ioctl *rio);
>
> +bool snp_svsm_vtpm_probe(void);
> +int snp_svsm_vtpm_send_command(u8 *buffer);
> +

These should be defined static inline with trivial definitions in the
#else case, yes?

>  void __init snp_secure_tsc_prepare(void);
>  void __init snp_secure_tsc_init(void);
>
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index 96c7bc698e6b..034aab7e76d2 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -2628,6 +2628,65 @@ static int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_dat
>         return ret;
>  }
>
> +/**
> + * snp_svsm_vtpm_probe() - Probe if SVSM provides a vTPM device
> + *
> + * This function checks that there is SVSM and that it supports at least
> + * TPM_SEND_COMMAND which is the only request we use so far.
> + *
> + * Return: true if the platform provides a vTPM SVSM device, false otherwise.
> + */
> +bool snp_svsm_vtpm_probe(void)
> +{
> +       struct svsm_call call = {};
> +
> +       /* The vTPM device is available only if a SVSM is present */
> +       if (!snp_vmpl)
> +               return false;
> +
> +       call.caa = svsm_get_caa();
> +       call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY);
> +
> +       if (svsm_perform_call_protocol(&call))
> +               return false;
> +
> +       /* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */
> +       return call.rcx_out & BIT_ULL(8);
> +}
> +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_probe);
> +
> +/**
> + * snp_svsm_vtpm_send_command() - execute a vTPM operation on SVSM
> + * @buffer: A buffer used to both send the command and receive the response.
> + *
> + * This function executes a SVSM_VTPM_CMD call as defined by
> + * "Secure VM Service Module for SEV-SNP Guests" Publication # 58019 Revision: 1.00
> + *
> + * All command request/response buffers have a common structure as specified by
> + * the following table:
> + *     Byte      Size       In/Out    Description
> + *     Offset    (Bytes)
> + *     0x000     4          In        Platform command
> + *                          Out       Platform command response size
> + *
> + * Each command can build upon this common request/response structure to create
> + * a structure specific to the command.
> + * See include/linux/tpm_svsm.h for more details.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +int snp_svsm_vtpm_send_command(u8 *buffer)
> +{
> +       struct svsm_call call = {};
> +
> +       call.caa = svsm_get_caa();
> +       call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD);
> +       call.rcx = __pa(buffer);
> +
> +       return svsm_perform_call_protocol(&call);
> +}
> +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_send_command);
> +
>  static struct platform_device sev_guest_device = {
>         .name           = "sev-guest",
>         .id             = -1,
> --
> 2.49.0
>


-- 
-Dionna Glaze, PhD, CISSP, CCSP (she/her)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/4] x86/sev: add SVSM vTPM probe/send_command functions
  2025-03-25 16:56   ` Dionna Amalie Glaze
@ 2025-03-25 17:20     ` Stefano Garzarella
  2025-03-26 17:14       ` Jarkko Sakkinen
  0 siblings, 1 reply; 16+ messages in thread
From: Stefano Garzarella @ 2025-03-25 17:20 UTC (permalink / raw)
  To: Dionna Amalie Glaze
  Cc: Jarkko Sakkinen, Joerg Roedel, Ingo Molnar, Dave Hansen,
	Peter Huewe, Tom Lendacky, Thomas Gleixner, x86, James Bottomley,
	linux-kernel, Borislav Petkov, Jason Gunthorpe, H. Peter Anvin,
	linux-coco, Claudio Carvalho, Dov Murik, linux-integrity

On Tue, Mar 25, 2025 at 09:56:53AM -0700, Dionna Amalie Glaze wrote:
>On Mon, Mar 24, 2025 at 3:47 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>>
>> From: Stefano Garzarella <sgarzare@redhat.com>
>>
>> Add two new functions to probe and send commands to the SVSM vTPM.
>> They leverage the two calls defined by the AMD SVSM specification [1]
>> for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD.
>>
>> Expose these functions to be used by other modules such as a tpm
>> driver.
>>
>> [1] "Secure VM Service Module for SEV-SNP Guests"
>>     Publication # 58019 Revision: 1.00
>>
>> Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>> Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com>
>> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
>> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>> v4:
>> - added Tom's R-b
>> - added functions documentation [Jarkko]
>> - simplified TPM_SEND_COMMAND check [Tom/Jarkko]
>> v3:
>> - removed link to the spec because those URLs are unstable [Borislav]
>> - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch
>>   in this one [Borislav]
>> - slimmed down snp_svsm_vtpm_probe() [Borislav]
>> - removed features check and any print related [Tom]
>> ---
>>  arch/x86/include/asm/sev.h |  7 +++++
>>  arch/x86/coco/sev/core.c   | 59 ++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 66 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
>> index ba7999f66abe..09471d058ce5 100644
>> --- a/arch/x86/include/asm/sev.h
>> +++ b/arch/x86/include/asm/sev.h
>> @@ -384,6 +384,10 @@ struct svsm_call {
>>  #define SVSM_ATTEST_SERVICES           0
>>  #define SVSM_ATTEST_SINGLE_SERVICE     1
>>
>> +#define SVSM_VTPM_CALL(x)              ((2ULL << 32) | (x))
>> +#define SVSM_VTPM_QUERY                        0
>> +#define SVSM_VTPM_CMD                  1
>> +
>>  #ifdef CONFIG_AMD_MEM_ENCRYPT
>>
>>  extern u8 snp_vmpl;
>> @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc);
>>  int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req,
>>                            struct snp_guest_request_ioctl *rio);
>>
>> +bool snp_svsm_vtpm_probe(void);
>> +int snp_svsm_vtpm_send_command(u8 *buffer);
>> +
>
>These should be defined static inline with trivial definitions in the
>#else case, yes?

For now the only user of those is the tpm_svsm driver which is build 
only if CONFIG_AMD_MEM_ENCRYPT is defined, so there should be no 
problem, but you are right, better to follow the other functions and 
define the stubs

I'll fix in v5, good catch!

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/4] x86/sev: add SVSM vTPM probe/send_command functions
  2025-03-24 10:46 ` [PATCH v4 1/4] x86/sev: add SVSM vTPM probe/send_command functions Stefano Garzarella
  2025-03-25 16:56   ` Dionna Amalie Glaze
@ 2025-03-26 17:12   ` Jarkko Sakkinen
  1 sibling, 0 replies; 16+ messages in thread
From: Jarkko Sakkinen @ 2025-03-26 17:12 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Joerg Roedel, Ingo Molnar, Dave Hansen, Peter Huewe, Tom Lendacky,
	Thomas Gleixner, x86, James Bottomley, linux-kernel,
	Borislav Petkov, Jason Gunthorpe, H. Peter Anvin, linux-coco,
	Claudio Carvalho, Dov Murik, Dionna Glaze, linux-integrity

On Mon, Mar 24, 2025 at 11:46:46AM +0100, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> 
> Add two new functions to probe and send commands to the SVSM vTPM.
> They leverage the two calls defined by the AMD SVSM specification [1]
> for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD.
> 
> Expose these functions to be used by other modules such as a tpm
> driver.
> 
> [1] "Secure VM Service Module for SEV-SNP Guests"
>     Publication # 58019 Revision: 1.00
> 
> Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> v4:
> - added Tom's R-b
> - added functions documentation [Jarkko]
> - simplified TPM_SEND_COMMAND check [Tom/Jarkko]
> v3:
> - removed link to the spec because those URLs are unstable [Borislav]
> - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch
>   in this one [Borislav]
> - slimmed down snp_svsm_vtpm_probe() [Borislav]
> - removed features check and any print related [Tom]
> ---
>  arch/x86/include/asm/sev.h |  7 +++++
>  arch/x86/coco/sev/core.c   | 59 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index ba7999f66abe..09471d058ce5 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -384,6 +384,10 @@ struct svsm_call {
>  #define SVSM_ATTEST_SERVICES		0
>  #define SVSM_ATTEST_SINGLE_SERVICE	1
>  
> +#define SVSM_VTPM_CALL(x)		((2ULL << 32) | (x))
> +#define SVSM_VTPM_QUERY			0
> +#define SVSM_VTPM_CMD			1
> +
>  #ifdef CONFIG_AMD_MEM_ENCRYPT
>  
>  extern u8 snp_vmpl;
> @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc);
>  int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req,
>  			   struct snp_guest_request_ioctl *rio);
>  
> +bool snp_svsm_vtpm_probe(void);
> +int snp_svsm_vtpm_send_command(u8 *buffer);
> +
>  void __init snp_secure_tsc_prepare(void);
>  void __init snp_secure_tsc_init(void);
>  
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index 96c7bc698e6b..034aab7e76d2 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -2628,6 +2628,65 @@ static int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_dat
>  	return ret;
>  }
>  
> +/**
> + * snp_svsm_vtpm_probe() - Probe if SVSM provides a vTPM device
> + *
> + * This function checks that there is SVSM and that it supports at least
> + * TPM_SEND_COMMAND which is the only request we use so far.
> + *
> + * Return: true if the platform provides a vTPM SVSM device, false otherwise.
> + */
> +bool snp_svsm_vtpm_probe(void)
> +{
> +	struct svsm_call call = {};
> +
> +	/* The vTPM device is available only if a SVSM is present */
> +	if (!snp_vmpl)
> +		return false;
> +
> +	call.caa = svsm_get_caa();
> +	call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY);
> +
> +	if (svsm_perform_call_protocol(&call))
> +		return false;
> +
> +	/* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */
> +	return call.rcx_out & BIT_ULL(8);
> +}
> +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_probe);
> +
> +/**
> + * snp_svsm_vtpm_send_command() - execute a vTPM operation on SVSM
> + * @buffer: A buffer used to both send the command and receive the response.
> + *
> + * This function executes a SVSM_VTPM_CMD call as defined by
> + * "Secure VM Service Module for SEV-SNP Guests" Publication # 58019 Revision: 1.00
> + *
> + * All command request/response buffers have a common structure as specified by
> + * the following table:
> + *     Byte      Size       In/Out    Description
> + *     Offset    (Bytes)
> + *     0x000     4          In        Platform command
> + *                          Out       Platform command response size
> + *
> + * Each command can build upon this common request/response structure to create
> + * a structure specific to the command.
> + * See include/linux/tpm_svsm.h for more details.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +int snp_svsm_vtpm_send_command(u8 *buffer)
> +{
> +	struct svsm_call call = {};
> +
> +	call.caa = svsm_get_caa();
> +	call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD);
> +	call.rcx = __pa(buffer);
> +
> +	return svsm_perform_call_protocol(&call);
> +}
> +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_send_command);
> +
>  static struct platform_device sev_guest_device = {
>  	.name		= "sev-guest",
>  	.id		= -1,
> -- 
> 2.49.0
> 
> 

This looks good enough as far as I'm concerned:

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

BR, Jarkko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/4] x86/sev: add SVSM vTPM probe/send_command functions
  2025-03-25 17:20     ` Stefano Garzarella
@ 2025-03-26 17:14       ` Jarkko Sakkinen
  2025-03-27  9:59         ` Stefano Garzarella
  0 siblings, 1 reply; 16+ messages in thread
From: Jarkko Sakkinen @ 2025-03-26 17:14 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Dionna Amalie Glaze, Joerg Roedel, Ingo Molnar, Dave Hansen,
	Peter Huewe, Tom Lendacky, Thomas Gleixner, x86, James Bottomley,
	linux-kernel, Borislav Petkov, Jason Gunthorpe, H. Peter Anvin,
	linux-coco, Claudio Carvalho, Dov Murik, linux-integrity

On Tue, Mar 25, 2025 at 06:20:48PM +0100, Stefano Garzarella wrote:
> On Tue, Mar 25, 2025 at 09:56:53AM -0700, Dionna Amalie Glaze wrote:
> > On Mon, Mar 24, 2025 at 3:47 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
> > > 
> > > From: Stefano Garzarella <sgarzare@redhat.com>
> > > 
> > > Add two new functions to probe and send commands to the SVSM vTPM.
> > > They leverage the two calls defined by the AMD SVSM specification [1]
> > > for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD.
> > > 
> > > Expose these functions to be used by other modules such as a tpm
> > > driver.
> > > 
> > > [1] "Secure VM Service Module for SEV-SNP Guests"
> > >     Publication # 58019 Revision: 1.00
> > > 
> > > Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> > > Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> > > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > > v4:
> > > - added Tom's R-b
> > > - added functions documentation [Jarkko]
> > > - simplified TPM_SEND_COMMAND check [Tom/Jarkko]
> > > v3:
> > > - removed link to the spec because those URLs are unstable [Borislav]
> > > - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch
> > >   in this one [Borislav]
> > > - slimmed down snp_svsm_vtpm_probe() [Borislav]
> > > - removed features check and any print related [Tom]
> > > ---
> > >  arch/x86/include/asm/sev.h |  7 +++++
> > >  arch/x86/coco/sev/core.c   | 59 ++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 66 insertions(+)
> > > 
> > > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> > > index ba7999f66abe..09471d058ce5 100644
> > > --- a/arch/x86/include/asm/sev.h
> > > +++ b/arch/x86/include/asm/sev.h
> > > @@ -384,6 +384,10 @@ struct svsm_call {
> > >  #define SVSM_ATTEST_SERVICES           0
> > >  #define SVSM_ATTEST_SINGLE_SERVICE     1
> > > 
> > > +#define SVSM_VTPM_CALL(x)              ((2ULL << 32) | (x))
> > > +#define SVSM_VTPM_QUERY                        0
> > > +#define SVSM_VTPM_CMD                  1
> > > +
> > >  #ifdef CONFIG_AMD_MEM_ENCRYPT
> > > 
> > >  extern u8 snp_vmpl;
> > > @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc);
> > >  int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req,
> > >                            struct snp_guest_request_ioctl *rio);
> > > 
> > > +bool snp_svsm_vtpm_probe(void);
> > > +int snp_svsm_vtpm_send_command(u8 *buffer);
> > > +
> > 
> > These should be defined static inline with trivial definitions in the
> > #else case, yes?
> 
> For now the only user of those is the tpm_svsm driver which is build only if
> CONFIG_AMD_MEM_ENCRYPT is defined, so there should be no problem, but you
> are right, better to follow the other functions and define the stubs
> 
> I'll fix in v5, good catch!

Only denoting here that you can keep my reviewed-by in the v5 for
this despite the minor change.

> 
> Thanks,
> Stefano
> 

BR, Jarkko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 2/4] svsm: add header with SVSM_VTPM_CMD helpers
  2025-03-24 10:46 ` [PATCH v4 2/4] svsm: add header with SVSM_VTPM_CMD helpers Stefano Garzarella
@ 2025-03-26 19:27   ` Jarkko Sakkinen
  0 siblings, 0 replies; 16+ messages in thread
From: Jarkko Sakkinen @ 2025-03-26 19:27 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Joerg Roedel, Ingo Molnar, Dave Hansen, Peter Huewe, Tom Lendacky,
	Thomas Gleixner, x86, James Bottomley, linux-kernel,
	Borislav Petkov, Jason Gunthorpe, H. Peter Anvin, linux-coco,
	Claudio Carvalho, Dov Murik, Dionna Glaze, linux-integrity

On Mon, Mar 24, 2025 at 11:46:47AM +0100, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> 
> Helpers for the SVSM_VTPM_CMD calls used by the vTPM protocol defined by
> the AMD SVSM spec [1].
> 
> The vTPM protocol follows the Official TPM 2.0 Reference Implementation
> (originally by Microsoft, now part of the TCG) simulator protocol.
> 
> [1] "Secure VM Service Module for SEV-SNP Guests"
>     Publication # 58019 Revision: 1.00
> 
> Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> v4:
> - used svsm_vtpm_ prefix consistently [Jarkko]
> - removed __packed where not needed [Jarkko]
> - expanded headers to avoid obfuscation [Jarkko]
> - used `buf` instead of `inbuf`/`outbuf` [Jarkko]
> - added more documentation quoting the specification
> - removed TPM_* macros since we only use TPM_SEND_COMMAND in one place
>   and don't want dependencies on external headers, but put the value
>   directly as specified in the AMD SVSM specification
> - header renamed in tpm_svsm.h so it will fall under TPM DEVICE DRIVER
>   section [Borislav, Jarkko]
> v3:
> - renamed header and prefix to make clear it's related to the SVSM vTPM
>   protocol
> - renamed fill/parse functions [Tom]
> - removed link to the spec because those URLs are unstable [Borislav]
> ---
>  include/linux/tpm_svsm.h | 149 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 149 insertions(+)
>  create mode 100644 include/linux/tpm_svsm.h
> 
> diff --git a/include/linux/tpm_svsm.h b/include/linux/tpm_svsm.h
> new file mode 100644
> index 000000000000..38e341f9761a
> --- /dev/null
> +++ b/include/linux/tpm_svsm.h
> @@ -0,0 +1,149 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2023 James.Bottomley@HansenPartnership.com
> + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved.
> + *
> + * Helpers for the SVSM_VTPM_CMD calls used by the vTPM protocol defined by the
> + * AMD SVSM spec [1].
> + *
> + * The vTPM protocol follows the Official TPM 2.0 Reference Implementation
> + * (originally by Microsoft, now part of the TCG) simulator protocol.
> + *
> + * [1] "Secure VM Service Module for SEV-SNP Guests"
> + *     Publication # 58019 Revision: 1.00
> + */
> +#ifndef _TPM_SVSM_H_
> +#define _TPM_SVSM_H_
> +
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +
> +#define SVSM_VTPM_MAX_BUFFER		4096 /* max req/resp buffer size */
> +
> +/**
> + * struct svsm_vtpm_request - Generic request for single word command
> + * @cmd:	The command to send
> + *
> + * Defined by AMD SVSM spec [1] in section "8.2 SVSM_VTPM_CMD Call" -
> + * Table 15: vTPM Common Request/Response Structure
> + *     Byte      Size       In/Out    Description
> + *     Offset    (Bytes)
> + *     0x000     4          In        Platform command
> + *                          Out       Platform command response size
> + */
> +struct svsm_vtpm_request {
> +	u32 cmd;
> +};
> +
> +/**
> + * struct svsm_vtpm_response - Generic response
> + * @size:	The response size (zero if nothing follows)
> + *
> + * Defined by AMD SVSM spec [1] in section "8.2 SVSM_VTPM_CMD Call" -
> + * Table 15: vTPM Common Request/Response Structure
> + *     Byte      Size       In/Out    Description
> + *     Offset    (Bytes)
> + *     0x000     4          In        Platform command
> + *                          Out       Platform command response size
> + *
> + * Note: most TCG Simulator commands simply return zero here with no indication
> + * of success or failure.
> + */
> +struct svsm_vtpm_response {
> +	u32 size;
> +};
> +
> +/**
> + * struct svsm_vtpm_cmd_request - Structure for a TPM_SEND_COMMAND request
> + * @cmd:	The command to send (must be TPM_SEND_COMMAND)
> + * @locality:	The locality
> + * @buf_size:	The size of the input buffer following
> + * @buf:	A buffer of size buf_size
> + *
> + * Defined by AMD SVSM spec [1] in section "8.2 SVSM_VTPM_CMD Call" -
> + * Table 16: TPM_SEND_COMMAND Request Structure
> + *     Byte      Size       Meaning
> + *     Offset    (Bytes)
> + *     0x000     4          Platform command (8)
> + *     0x004     1          Locality (must-be-0)
> + *     0x005     4          TPM Command size (in bytes)
> + *     0x009     Variable   TPM Command
> + *
> + * Note: the TCG Simulator expects @buf_size to be equal to the size of the
> + * specific TPM command, otherwise an TPM_RC_COMMAND_SIZE error is returned.
> + */
> +struct svsm_vtpm_cmd_request {
> +	u32 cmd;
> +	u8 locality;
> +	u32 buf_size;
> +	u8 buf[];
> +} __packed;
> +
> +/**
> + * struct svsm_vtpm_cmd_response - Structure for a TPM_SEND_COMMAND response
> + * @buf_size:	The size of the output buffer following
> + * @buf:	A buffer of size buf_size
> + *
> + * Defined by AMD SVSM spec [1] in section "8.2 SVSM_VTPM_CMD Call" -
> + * Table 17: TPM_SEND_COMMAND Response Structure
> + *     Byte      Size       Meaning
> + *     Offset    (Bytes)
> + *     0x000     4          Response size (in bytes)
> + *     0x004     Variable   Response
> + */
> +struct svsm_vtpm_cmd_response {
> +	u32 buf_size;
> +	u8 buf[];
> +};
> +
> +/**
> + * svsm_vtpm_cmd_request_fill() - Fill a TPM_SEND_COMMAND request to be sent to SVSM
> + * @req: The struct svsm_vtpm_cmd_request to fill
> + * @locality: The locality
> + * @buf: The buffer from where to copy the payload of the command
> + * @len: The size of the buffer
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +static inline int
> +svsm_vtpm_cmd_request_fill(struct svsm_vtpm_cmd_request *req, u8 locality,
> +			   const u8 *buf, size_t len)
> +{
> +	if (len > SVSM_VTPM_MAX_BUFFER - sizeof(*req))
> +		return -EINVAL;
> +
> +	req->cmd = 8; /* TPM_SEND_COMMAND */
> +	req->locality = locality;
> +	req->buf_size = len;
> +
> +	memcpy(req->buf, buf, len);
> +
> +	return 0;
> +}
> +
> +/**
> + * svsm_vtpm_cmd_response_parse() - Parse a TPM_SEND_COMMAND response received from SVSM
> + * @resp: The struct svsm_vtpm_cmd_response to parse
> + * @buf: The buffer where to copy the response
> + * @len: The size of the buffer
> + *
> + * Return: buffer size filled with the response on success, negative error
> + * code on failure.
> + */
> +static inline int
> +svsm_vtpm_cmd_response_parse(const struct svsm_vtpm_cmd_response *resp, u8 *buf,
> +			     size_t len)
> +{
> +	if (len < resp->buf_size)
> +		return -E2BIG;
> +
> +	if (resp->buf_size > SVSM_VTPM_MAX_BUFFER - sizeof(*resp))
> +		return -EINVAL;  // Invalid response from the platform TPM
> +
> +	memcpy(buf, resp->buf, resp->buf_size);
> +
> +	return resp->buf_size;
> +}
> +
> +#endif /* _TPM_SVSM_H_ */
> -- 
> 2.49.0
> 

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

BR, Jarkko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 3/4] tpm: add SNP SVSM vTPM driver
  2025-03-24 10:46 ` [PATCH v4 3/4] tpm: add SNP SVSM vTPM driver Stefano Garzarella
@ 2025-03-26 19:30   ` Jarkko Sakkinen
  2025-03-27 10:03     ` Stefano Garzarella
  0 siblings, 1 reply; 16+ messages in thread
From: Jarkko Sakkinen @ 2025-03-26 19:30 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Joerg Roedel, Ingo Molnar, Dave Hansen, Peter Huewe, Tom Lendacky,
	Thomas Gleixner, x86, James Bottomley, linux-kernel,
	Borislav Petkov, Jason Gunthorpe, H. Peter Anvin, linux-coco,
	Claudio Carvalho, Dov Murik, Dionna Glaze, linux-integrity

On Mon, Mar 24, 2025 at 11:46:48AM +0100, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> 
> 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 <sgarzare@redhat.com>
> ---
> 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 | 155 ++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/Kconfig    |  10 +++
>  drivers/char/tpm/Makefile   |   1 +
>  3 files changed, 166 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..1281ff265927
> --- /dev/null
> +++ b/drivers/char/tpm/tpm_svsm.c
> @@ -0,0 +1,155 @@
> +// 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 void tpm_svsm_cancel(struct tpm_chip *chip)
> +{
> +	/* not supported */
> +}
> +
> +static u8 tpm_svsm_status(struct tpm_chip *chip)
> +{
> +	return 0;
> +}
> +
> +static bool tpm_svsm_req_canceled(struct tpm_chip *chip, u8 status)
> +{
> +	return false;
> +}
> +
> +static struct tpm_class_ops tpm_chip_ops = {
> +	.flags = TPM_OPS_AUTO_STARTUP,
> +	.recv = tpm_svsm_recv,
> +	.send = tpm_svsm_send,
> +	.cancel = tpm_svsm_cancel,
> +	.status = tpm_svsm_status,
> +	.req_complete_mask = 0,
> +	.req_complete_val = 0,
> +	.req_canceled = tpm_svsm_req_canceled,

If this was bundled with the patch set, this would short a lot:

https://lore.kernel.org/linux-integrity/20250326161838.123606-1-jarkko@kernel.org/T/#u

So maybe for v5? Including this patch does not take send_recv()
out of consideration, it is just smart thing to do in all cases.

It would be probably easiest to roll out my patch together with
rest of the patch set.

BR, Jarkko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/4] x86/sev: add SVSM vTPM probe/send_command functions
  2025-03-26 17:14       ` Jarkko Sakkinen
@ 2025-03-27  9:59         ` Stefano Garzarella
  0 siblings, 0 replies; 16+ messages in thread
From: Stefano Garzarella @ 2025-03-27  9:59 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Dionna Amalie Glaze, Joerg Roedel, Ingo Molnar, Dave Hansen,
	Peter Huewe, Tom Lendacky, Thomas Gleixner, x86, James Bottomley,
	linux-kernel, Borislav Petkov, Jason Gunthorpe, H. Peter Anvin,
	linux-coco, Claudio Carvalho, Dov Murik, linux-integrity

On Wed, Mar 26, 2025 at 07:14:22PM +0200, Jarkko Sakkinen wrote:
>On Tue, Mar 25, 2025 at 06:20:48PM +0100, Stefano Garzarella wrote:
>> On Tue, Mar 25, 2025 at 09:56:53AM -0700, Dionna Amalie Glaze wrote:
>> > On Mon, Mar 24, 2025 at 3:47 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>> > >
>> > > From: Stefano Garzarella <sgarzare@redhat.com>
>> > >
>> > > Add two new functions to probe and send commands to the SVSM vTPM.
>> > > They leverage the two calls defined by the AMD SVSM specification [1]
>> > > for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD.
>> > >
>> > > Expose these functions to be used by other modules such as a tpm
>> > > driver.
>> > >
>> > > [1] "Secure VM Service Module for SEV-SNP Guests"
>> > >     Publication # 58019 Revision: 1.00
>> > >
>> > > Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>> > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>> > > Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com>
>> > > Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
>> > > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
>> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> > > ---
>> > > v4:
>> > > - added Tom's R-b
>> > > - added functions documentation [Jarkko]
>> > > - simplified TPM_SEND_COMMAND check [Tom/Jarkko]
>> > > v3:
>> > > - removed link to the spec because those URLs are unstable [Borislav]
>> > > - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch
>> > >   in this one [Borislav]
>> > > - slimmed down snp_svsm_vtpm_probe() [Borislav]
>> > > - removed features check and any print related [Tom]
>> > > ---
>> > >  arch/x86/include/asm/sev.h |  7 +++++
>> > >  arch/x86/coco/sev/core.c   | 59 ++++++++++++++++++++++++++++++++++++++
>> > >  2 files changed, 66 insertions(+)
>> > >
>> > > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
>> > > index ba7999f66abe..09471d058ce5 100644
>> > > --- a/arch/x86/include/asm/sev.h
>> > > +++ b/arch/x86/include/asm/sev.h
>> > > @@ -384,6 +384,10 @@ struct svsm_call {
>> > >  #define SVSM_ATTEST_SERVICES           0
>> > >  #define SVSM_ATTEST_SINGLE_SERVICE     1
>> > >
>> > > +#define SVSM_VTPM_CALL(x)              ((2ULL << 32) | (x))
>> > > +#define SVSM_VTPM_QUERY                        0
>> > > +#define SVSM_VTPM_CMD                  1
>> > > +
>> > >  #ifdef CONFIG_AMD_MEM_ENCRYPT
>> > >
>> > >  extern u8 snp_vmpl;
>> > > @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc);
>> > >  int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req,
>> > >                            struct snp_guest_request_ioctl *rio);
>> > >
>> > > +bool snp_svsm_vtpm_probe(void);
>> > > +int snp_svsm_vtpm_send_command(u8 *buffer);
>> > > +
>> >
>> > These should be defined static inline with trivial definitions in the
>> > #else case, yes?
>>
>> For now the only user of those is the tpm_svsm driver which is build only if
>> CONFIG_AMD_MEM_ENCRYPT is defined, so there should be no problem, but you
>> are right, better to follow the other functions and define the stubs
>>
>> I'll fix in v5, good catch!
>
>Only denoting here that you can keep my reviewed-by in the v5 for
>this despite the minor change.

Thanks for pointing that out!
Stefano


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 3/4] tpm: add SNP SVSM vTPM driver
  2025-03-26 19:30   ` Jarkko Sakkinen
@ 2025-03-27 10:03     ` Stefano Garzarella
  2025-03-27 11:53       ` Jarkko Sakkinen
  0 siblings, 1 reply; 16+ messages in thread
From: Stefano Garzarella @ 2025-03-27 10:03 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Joerg Roedel, Ingo Molnar, Dave Hansen, Peter Huewe, Tom Lendacky,
	Thomas Gleixner, x86, James Bottomley, linux-kernel,
	Borislav Petkov, Jason Gunthorpe, H. Peter Anvin, linux-coco,
	Claudio Carvalho, Dov Murik, Dionna Glaze, linux-integrity

On Wed, Mar 26, 2025 at 09:30:53PM +0200, Jarkko Sakkinen wrote:
>On Mon, Mar 24, 2025 at 11:46:48AM +0100, Stefano Garzarella wrote:
>> From: Stefano Garzarella <sgarzare@redhat.com>
>>
>> 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 <sgarzare@redhat.com>
>> ---
>> 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 | 155 ++++++++++++++++++++++++++++++++++++
>>  drivers/char/tpm/Kconfig    |  10 +++
>>  drivers/char/tpm/Makefile   |   1 +
>>  3 files changed, 166 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..1281ff265927
>> --- /dev/null
>> +++ b/drivers/char/tpm/tpm_svsm.c
>> @@ -0,0 +1,155 @@
>> +// 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 void tpm_svsm_cancel(struct tpm_chip *chip)
>> +{
>> +	/* not supported */
>> +}
>> +
>> +static u8 tpm_svsm_status(struct tpm_chip *chip)
>> +{
>> +	return 0;
>> +}
>> +
>> +static bool tpm_svsm_req_canceled(struct tpm_chip *chip, u8 status)
>> +{
>> +	return false;
>> +}
>> +
>> +static struct tpm_class_ops tpm_chip_ops = {
>> +	.flags = TPM_OPS_AUTO_STARTUP,
>> +	.recv = tpm_svsm_recv,
>> +	.send = tpm_svsm_send,
>> +	.cancel = tpm_svsm_cancel,
>> +	.status = tpm_svsm_status,
>> +	.req_complete_mask = 0,
>> +	.req_complete_val = 0,
>> +	.req_canceled = tpm_svsm_req_canceled,
>
>If this was bundled with the patch set, this would short a lot:
>
>https://lore.kernel.org/linux-integrity/20250326161838.123606-1-jarkko@kernel.org/T/#u
>
>So maybe for v5? Including this patch does not take send_recv()
>out of consideration, it is just smart thing to do in all cases.
>
>It would be probably easiest to roll out my patch together with
>rest of the patch set.

Yeah, I agree. I'll include it in this series and adapt this patch on 
top of it.

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 3/4] tpm: add SNP SVSM vTPM driver
  2025-03-27 10:03     ` Stefano Garzarella
@ 2025-03-27 11:53       ` Jarkko Sakkinen
  2025-03-27 11:57         ` Jarkko Sakkinen
  0 siblings, 1 reply; 16+ messages in thread
From: Jarkko Sakkinen @ 2025-03-27 11:53 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Joerg Roedel, Ingo Molnar, Dave Hansen, Peter Huewe, Tom Lendacky,
	Thomas Gleixner, x86, James Bottomley, linux-kernel,
	Borislav Petkov, Jason Gunthorpe, H. Peter Anvin, linux-coco,
	Claudio Carvalho, Dov Murik, Dionna Glaze, linux-integrity

On Thu, Mar 27, 2025 at 11:03:07AM +0100, Stefano Garzarella wrote:
> On Wed, Mar 26, 2025 at 09:30:53PM +0200, Jarkko Sakkinen wrote:
> > On Mon, Mar 24, 2025 at 11:46:48AM +0100, Stefano Garzarella wrote:
> > > From: Stefano Garzarella <sgarzare@redhat.com>
> > > 
> > > 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 <sgarzare@redhat.com>
> > > ---
> > > 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 | 155 ++++++++++++++++++++++++++++++++++++
> > >  drivers/char/tpm/Kconfig    |  10 +++
> > >  drivers/char/tpm/Makefile   |   1 +
> > >  3 files changed, 166 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..1281ff265927
> > > --- /dev/null
> > > +++ b/drivers/char/tpm/tpm_svsm.c
> > > @@ -0,0 +1,155 @@
> > > +// 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 void tpm_svsm_cancel(struct tpm_chip *chip)
> > > +{
> > > +	/* not supported */
> > > +}
> > > +
> > > +static u8 tpm_svsm_status(struct tpm_chip *chip)
> > > +{
> > > +	return 0;
> > > +}
> > > +
> > > +static bool tpm_svsm_req_canceled(struct tpm_chip *chip, u8 status)
> > > +{
> > > +	return false;
> > > +}
> > > +
> > > +static struct tpm_class_ops tpm_chip_ops = {
> > > +	.flags = TPM_OPS_AUTO_STARTUP,
> > > +	.recv = tpm_svsm_recv,
> > > +	.send = tpm_svsm_send,
> > > +	.cancel = tpm_svsm_cancel,
> > > +	.status = tpm_svsm_status,
> > > +	.req_complete_mask = 0,
> > > +	.req_complete_val = 0,
> > > +	.req_canceled = tpm_svsm_req_canceled,
> > 
> > If this was bundled with the patch set, this would short a lot:
> > 
> > https://lore.kernel.org/linux-integrity/20250326161838.123606-1-jarkko@kernel.org/T/#u
> > 
> > So maybe for v5? Including this patch does not take send_recv()
> > out of consideration, it is just smart thing to do in all cases.
> > 
> > It would be probably easiest to roll out my patch together with
> > rest of the patch set.
> 
> Yeah, I agree. I'll include it in this series and adapt this patch on top of
> it.

Yeah, and you could simplify to goal in the other patch set: it's about
avoiding double-copy of a buffer.

It's a totally legit argument that we can measure. So in the end this
will help out landing that too because it takes away the extra cruft
and streamlines the goal.

> 
> Thanks,
> Stefano
> 
> 

BR, Jarkko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 3/4] tpm: add SNP SVSM vTPM driver
  2025-03-27 11:53       ` Jarkko Sakkinen
@ 2025-03-27 11:57         ` Jarkko Sakkinen
  2025-03-27 14:10           ` Stefano Garzarella
  0 siblings, 1 reply; 16+ messages in thread
From: Jarkko Sakkinen @ 2025-03-27 11:57 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Joerg Roedel, Ingo Molnar, Dave Hansen, Peter Huewe, Tom Lendacky,
	Thomas Gleixner, x86, James Bottomley, linux-kernel,
	Borislav Petkov, Jason Gunthorpe, H. Peter Anvin, linux-coco,
	Claudio Carvalho, Dov Murik, Dionna Glaze, linux-integrity

On Thu, Mar 27, 2025 at 01:53:59PM +0200, Jarkko Sakkinen wrote:
> On Thu, Mar 27, 2025 at 11:03:07AM +0100, Stefano Garzarella wrote:
> > On Wed, Mar 26, 2025 at 09:30:53PM +0200, Jarkko Sakkinen wrote:
> > > On Mon, Mar 24, 2025 at 11:46:48AM +0100, Stefano Garzarella wrote:
> > > > From: Stefano Garzarella <sgarzare@redhat.com>
> > > > 
> > > > 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 <sgarzare@redhat.com>
> > > > ---
> > > > 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 | 155 ++++++++++++++++++++++++++++++++++++
> > > >  drivers/char/tpm/Kconfig    |  10 +++
> > > >  drivers/char/tpm/Makefile   |   1 +
> > > >  3 files changed, 166 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..1281ff265927
> > > > --- /dev/null
> > > > +++ b/drivers/char/tpm/tpm_svsm.c
> > > > @@ -0,0 +1,155 @@
> > > > +// 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 void tpm_svsm_cancel(struct tpm_chip *chip)
> > > > +{
> > > > +	/* not supported */
> > > > +}
> > > > +
> > > > +static u8 tpm_svsm_status(struct tpm_chip *chip)
> > > > +{
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static bool tpm_svsm_req_canceled(struct tpm_chip *chip, u8 status)
> > > > +{
> > > > +	return false;
> > > > +}
> > > > +
> > > > +static struct tpm_class_ops tpm_chip_ops = {
> > > > +	.flags = TPM_OPS_AUTO_STARTUP,
> > > > +	.recv = tpm_svsm_recv,
> > > > +	.send = tpm_svsm_send,
> > > > +	.cancel = tpm_svsm_cancel,
> > > > +	.status = tpm_svsm_status,
> > > > +	.req_complete_mask = 0,
> > > > +	.req_complete_val = 0,
> > > > +	.req_canceled = tpm_svsm_req_canceled,
> > > 
> > > If this was bundled with the patch set, this would short a lot:
> > > 
> > > https://lore.kernel.org/linux-integrity/20250326161838.123606-1-jarkko@kernel.org/T/#u
> > > 
> > > So maybe for v5? Including this patch does not take send_recv()
> > > out of consideration, it is just smart thing to do in all cases.
> > > 
> > > It would be probably easiest to roll out my patch together with
> > > rest of the patch set.
> > 
> > Yeah, I agree. I'll include it in this series and adapt this patch on top of
> > it.
> 
> Yeah, and you could simplify to goal in the other patch set: it's about
> avoiding double-copy of a buffer.
> 
> It's a totally legit argument that we can measure. So in the end this
> will help out landing that too because it takes away the extra cruft
> and streamlines the goal.

... IMHO there is this unwritten law for upstreaming kernel features
that goes something like "further the goals are from white papers,
closer they are to mainline" ;-)

BR, Jarkkko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 3/4] tpm: add SNP SVSM vTPM driver
  2025-03-27 11:57         ` Jarkko Sakkinen
@ 2025-03-27 14:10           ` Stefano Garzarella
  0 siblings, 0 replies; 16+ messages in thread
From: Stefano Garzarella @ 2025-03-27 14:10 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Joerg Roedel, Ingo Molnar, Dave Hansen, Peter Huewe, Tom Lendacky,
	Thomas Gleixner, x86, James Bottomley, linux-kernel,
	Borislav Petkov, Jason Gunthorpe, H. Peter Anvin, linux-coco,
	Claudio Carvalho, Dov Murik, Dionna Glaze, linux-integrity

On Thu, Mar 27, 2025 at 01:57:31PM +0200, Jarkko Sakkinen wrote:
>On Thu, Mar 27, 2025 at 01:53:59PM +0200, Jarkko Sakkinen wrote:
>> On Thu, Mar 27, 2025 at 11:03:07AM +0100, Stefano Garzarella wrote:
>> > On Wed, Mar 26, 2025 at 09:30:53PM +0200, Jarkko Sakkinen wrote:
>> > > On Mon, Mar 24, 2025 at 11:46:48AM +0100, Stefano Garzarella wrote:

[...]

>> > > > +
>> > > > +static struct tpm_class_ops tpm_chip_ops = {
>> > > > +	.flags = TPM_OPS_AUTO_STARTUP,
>> > > > +	.recv = tpm_svsm_recv,
>> > > > +	.send = tpm_svsm_send,
>> > > > +	.cancel = tpm_svsm_cancel,
>> > > > +	.status = tpm_svsm_status,
>> > > > +	.req_complete_mask = 0,
>> > > > +	.req_complete_val = 0,
>> > > > +	.req_canceled = tpm_svsm_req_canceled,
>> > >
>> > > If this was bundled with the patch set, this would short a lot:
>> > >
>> > > https://lore.kernel.org/linux-integrity/20250326161838.123606-1-jarkko@kernel.org/T/#u
>> > >
>> > > So maybe for v5? Including this patch does not take send_recv()
>> > > out of consideration, it is just smart thing to do in all cases.
>> > >
>> > > It would be probably easiest to roll out my patch together with
>> > > rest of the patch set.
>> >
>> > Yeah, I agree. I'll include it in this series and adapt this patch on top of
>> > it.
>>
>> Yeah, and you could simplify to goal in the other patch set: it's about
>> avoiding double-copy of a buffer.

Yep, agree!

>>
>> It's a totally legit argument that we can measure. So in the end this
>> will help out landing that too because it takes away the extra cruft
>> and streamlines the goal.
>
>... IMHO there is this unwritten law for upstreaming kernel features
>that goes something like "further the goals are from white papers,
>closer they are to mainline" ;-)

:-D I'll make a note of it!

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-03-27 14:11 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-24 10:46 [PATCH v4 0/4] Enlightened vTPM support for SVSM on SEV-SNP Stefano Garzarella
2025-03-24 10:46 ` [PATCH v4 1/4] x86/sev: add SVSM vTPM probe/send_command functions Stefano Garzarella
2025-03-25 16:56   ` Dionna Amalie Glaze
2025-03-25 17:20     ` Stefano Garzarella
2025-03-26 17:14       ` Jarkko Sakkinen
2025-03-27  9:59         ` Stefano Garzarella
2025-03-26 17:12   ` Jarkko Sakkinen
2025-03-24 10:46 ` [PATCH v4 2/4] svsm: add header with SVSM_VTPM_CMD helpers Stefano Garzarella
2025-03-26 19:27   ` Jarkko Sakkinen
2025-03-24 10:46 ` [PATCH v4 3/4] tpm: add SNP SVSM vTPM driver Stefano Garzarella
2025-03-26 19:30   ` Jarkko Sakkinen
2025-03-27 10:03     ` Stefano Garzarella
2025-03-27 11:53       ` Jarkko Sakkinen
2025-03-27 11:57         ` Jarkko Sakkinen
2025-03-27 14:10           ` Stefano Garzarella
2025-03-24 10:46 ` [PATCH v4 4/4] x86/sev: register tpm-svsm platform device Stefano Garzarella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).