On Tue, Mar 11, 2025 at 10:42:23AM +0100, Stefano Garzarella wrote:

Across the board below data structures: I'd svsm_vtpm_ prefix them.
The rational is quite practical: it would easier to grep them later
on.

+/**
+ * struct tpm_req - generic request header for single word command
+ *
+ * @cmd:	The command to send
+ */
+struct tpm_req {
+	u32 cmd;
+} __packed;

__packed is useless here.

+
+/**
+ * struct tpm_resp - generic response header
+ *
+ * @size:	The response size (zero if nothing follows)
+ *
+ * Note: most TCG Simulator commands simply return zero here with no indication
+ * of success or failure.
+ */
+struct tpm_resp {
+	u32 size;
+} __packed;

Ditto.

+
+/**
+ * struct tpm_send_cmd_req - Structure for a TPM_SEND_COMMAND request
+ *
+ * @hdr:	The request header whit the command (must be TPM_SEND_COMMAND)
+ * @locality:	The locality
+ * @inbuf_size:	The size of the input buffer following
+ * @inbuf:	A buffer of size inbuf_size
+ *
+ * Note that TCG Simulator expects @inbuf_size to be equal to the size of the
+ * specific TPM command, otherwise an TPM_RC_COMMAND_SIZE error is
+ * returned.
+ */
+struct tpm_send_cmd_req {
+	struct tpm_req hdr;

Useless nesting that makes this obfuscated: you can just as well put
that single field here, i.e.

	u32 cmd;

+	u8 locality;
+	u32 inbuf_size;
+	u8 inbuf[];

Why not just buf?

+} __packed;

Since we don't care about TCG Simulator compatibility I'd expect that
these are ordered in a way that they align nicely. E.g.,

struct svsm_vtpm_request {
	u32 command;
	u16 locality;
	u16 buffer_size;
	u8 buffer[];
};

64k should enough for any possible TPM command.

+
+/**
+ * struct tpm_send_cmd_req - Structure for a TPM_SEND_COMMAND response
+ *
+ * @hdr:	The response header whit the following size
+ * @outbuf:	A buffer of size hdr.size
+ */
+struct tpm_send_cmd_resp {
+	struct tpm_resp hdr;
+	u8 outbuf[];
+} __packed;

Why this does not have size? Here also __packed is useless even with the
pre-existing layout, and something like svsm_tpm_response would be a
factor more reasonable name.

+
+/**
+ * svsm_vtpm_fill_cmd_req() - fill a struct tpm_send_cmd_req to be sent to SVSM

+ * @req: The struct tpm_send_cmd_req 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_fill_cmd_req(struct tpm_send_cmd_req *req, u8 locality,
+		       const u8 *buf, size_t len)

svsm_vtpm_fill_request()

+{
+	if (len > SVSM_VTPM_MAX_BUFFER - sizeof(*req))
+		return -EINVAL;
+
+	req->hdr.cmd = TPM_SEND_COMMAND;
+	req->locality = locality;
+	req->inbuf_size = len;
+
+	memcpy(req->inbuf, buf, len);
+
+	return 0;
+}
+
+/**
+ * svsm_vtpm_parse_cmd_resp() - Parse a struct tpm_send_cmd_resp received from
+ * SVSM
+ * @resp: The struct tpm_send_cmd_resp 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_parse_cmd_resp(const struct tpm_send_cmd_resp *resp, u8 *buf,
+			 size_t len)

svsm_vtpm_parse_response()

+{
+	if (len < resp->hdr.size)
+		return -E2BIG;
+
+	if (resp->hdr.size > SVSM_VTPM_MAX_BUFFER - sizeof(*resp))
+		return -EINVAL;  // Invalid response from the platform TPM
+
+	memcpy(buf, resp->outbuf, resp->hdr.size);
+
+	return resp->hdr.size;
+}
+
+#endif /* _SVSM_VTPM_H_ */
-- 
2.48.1

BR, Jarkko