Subject: [PATCH v2] virt: tdx-guest: Add Quote generation support using TSM_REPORTS

In TDX guest, the attestation process is used to verify the TDX guest
trustworthiness to other entities before provisioning secrets to the
guest. The First step in the attestation process is TDREPORT
generation, which involves getting the guest measurement data in the
format of TDREPORT, which is further used to validate the authenticity
of the TDX guest. TDREPORT by design is integrity-protected and can
only be verified on the local machine.

To support remote verification of the TDREPORT (in a SGX-based
attestation), the TDREPORT needs to be sent to the SGX Quoting Enclave
(QE) to convert it to a remotely verifiable Quote. SGX QE by design can
only run outside of the TDX guest (i.e. in a host process or in a
normal VM) and guest can use communication channels like vsock or
TCP/IP to send the TDREPORT to the QE. But for security concerns, the
TDX guest may not support these communication channels. To handle such
cases, TDX defines a GetQuote hypercall which can be used by the guest
to request the host VMM to communicate with the SGX QE. More details
about GetQuote hypercall can be found in TDX Guest-Host Communication
Interface (GHCI) for Intel TDX 1.0, section titled
"TDG.VP.VMCALL<GetQuote>".

Trusted Security Module (TSM) [1] exposes a common ABI for Confidential
Computing Guest platforms to get the measurement data via ConfigFS.
Extend the TSM framework and add support to allow an attestation agent
to get the TDX Quote data (included usage example below).

report=/sys/kernel/config/tsm/report/report0
mkdir $report
dd if=/dev/urandom bs=64 count=1 > $report/inblob
hexdump -C $report/outblob
rmdir $report

GetQuote TDVMCALL requires TD guest pass a 4K aligned shared buffer
with TDREPORT data as input, which is further used by the VMM to copy
the TD Quote result after successful Quote generation. To create the
shared buffer, allocate a large enough memory and mark it shared using
set_memory_decrypted() in tdx_guest_init(). This buffer will be re-used
for GetQuote requests in the TDX TSM handler.

Although this method reserves a fixed chunk of memory for GetQuote
requests, such one time allocation can help avoid memory fragmentation
related allocation failures later in the uptime of the guest.

Since the Quote generation process is not time-critical or frequently
used, the current version uses a polling model for Quote requests and
it also does not support parallel GetQuote requests.

Link: https://lore.kernel.org/lkml/169342399185.3934343.3035845348326944519.stgit@dwillia2-xfh.jf.intel.com/ [1]
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
---

Hi All,

The original version of this patch series [1] added support for TDX
Guest Quote generation via an IOCTL interface. Since we have multiple
vendors implementing such interface, to avoid ABI proliferation, Dan
proposed using a common ABI for it and submitted the Trusted Secure
module (TSM) report ABI support [2]. This patchset extends the
TSM REPORTS to implement the TDX Quote generation support. Since there
is a change in interface type, I have dropped the previous Acks.

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/169342399185.3934343.3035845348326944519.stgit@dwillia2-xfh.jf.intel.com/

Changes since v1:
* Used mutext_lock_interruptible() for quote_lock to allow user
interruption.
* Used msleep_interruptible() instead of ssleep() to allow user
interruption.
* Added check for the set_memory_encrypted() return value.
* Fixed typos in comments and commit log.

Changes since previous version:
* Used ConfigFS interface instead of IOCTL interface.
* Used polling model for Quote generation and dropped the event notification IRQ support.

arch/x86/coco/tdx/tdx.c | 21 +++
arch/x86/include/asm/shared/tdx.h | 1 +
arch/x86/include/asm/tdx.h | 2 +
drivers/virt/coco/tdx-guest/Kconfig | 1 +
drivers/virt/coco/tdx-guest/tdx-guest.c | 225 +++++++++++++++++++++++-
5 files changed, 249 insertions(+), 1 deletion(-)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 1d6b863c42b0..752867b1d11b 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -104,6 +104,27 @@ int tdx_mcall_get_report0(u8 *reportdata, u8 *tdreport)
}
EXPORT_SYMBOL_GPL(tdx_mcall_get_report0);

+/**
+ * tdx_hcall_get_quote() - Wrapper to request TD Quote using GetQuote
+ * hypercall.
+ * @buf: Address of the directly mapped shared kernel buffer which
+ * contains TDREPORT. The same buffer will be used by VMM to
+ * store the generated TD Quote output.
+ * @size: size of the tdquote buffer (4KB-aligned).
+ *
+ * Refer to section titled "TDG.VP.VMCALL<GetQuote>" in the TDX GHCI
+ * v1.0 specification for more information on GetQuote hypercall.
+ * It is used in the TDX guest driver module to get the TD Quote.
+ *
+ * Return 0 on success or error code on failure.
+ */
+u64 tdx_hcall_get_quote(u8 *buf, size_t size)
+{
+ /* Since buf is a shared memory, set the shared (decrypted) bits */
+ return _tdx_hypercall(TDVMCALL_GET_QUOTE, cc_mkdec(virt_to_phys(buf)), size, 0, 0);
+}
+EXPORT_SYMBOL_GPL(tdx_hcall_get_quote);
+
static void __noreturn tdx_panic(const char *msg)
{
struct tdx_hypercall_args args = {
diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
index 7513b3bb69b7..9eab19950f39 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -22,6 +22,7 @@

/* TDX hypercall Leaf IDs */
#define TDVMCALL_MAP_GPA 0x10001
+#define TDVMCALL_GET_QUOTE 0x10002
#define TDVMCALL_REPORT_FATAL_ERROR 0x10003

#ifndef __ASSEMBLY__
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 603e6d1e9d4a..ebd1cda4875f 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -52,6 +52,8 @@ bool tdx_early_handle_ve(struct pt_regs *regs);

int tdx_mcall_get_report0(u8 *reportdata, u8 *tdreport);

+u64 tdx_hcall_get_quote(u8 *buf, size_t size);
+
#else

static inline void tdx_early_init(void) { };
diff --git a/drivers/virt/coco/tdx-guest/Kconfig b/drivers/virt/coco/tdx-guest/Kconfig
index 14246fc2fb02..22dd59e19431 100644
--- a/drivers/virt/coco/tdx-guest/Kconfig
+++ b/drivers/virt/coco/tdx-guest/Kconfig
@@ -1,6 +1,7 @@
config TDX_GUEST_DRIVER
tristate "TDX Guest driver"
depends on INTEL_TDX_GUEST
+ select TSM_REPORTS
help
The driver provides userspace interface to communicate with
the TDX module to request the TDX guest details like attestation
diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
index 5e44a0fa69bd..9d9b92ca9b16 100644
--- a/drivers/virt/coco/tdx-guest/tdx-guest.c
+++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
@@ -12,12 +12,60 @@
#include <linux/mod_devicetable.h>
#include <linux/string.h>
#include <linux/uaccess.h>
+#include <linux/set_memory.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/tsm.h>
+#include <linux/sizes.h>

#include <uapi/linux/tdx-guest.h>

#include <asm/cpu_device_id.h>
#include <asm/tdx.h>

+/*
+ * Intel's SGX QE implementation generally uses Quote size less
+ * than 8K (2K Quote data + ~5K of certificate blob).
+ */
+#define GET_QUOTE_BUF_SIZE SZ_8K
+
+#define GET_QUOTE_CMD_VER 1
+
+/* TDX GetQuote status codes */
+#define GET_QUOTE_SUCCESS 0
+#define GET_QUOTE_IN_FLIGHT 0xffffffffffffffff
+
+/* struct tdx_quote_buf: Format of Quote request buffer.
+ * @version: Quote format version, filled by TD.
+ * @status: Status code of Quote request, filled by VMM.
+ * @in_len: Length of TDREPORT, filled by TD.
+ * @out_len: Length of Quote data, filled by VMM.
+ * @data: Quote data on output or TDREPORT on input.
+ *
+ * More details of Quote request buffer can be found in TDX
+ * Guest-Host Communication Interface (GHCI) for Intel TDX 1.0,
+ * section titled "TDG.VP.VMCALL<GetQuote>"
+ */
+struct tdx_quote_buf {
+ u64 version;
+ u64 status;
+ u32 in_len;
+ u32 out_len;
+ u8 data[];
+};
+
+/* Quote data buffer */
+static void *quote_data;
+
+/* Lock to streamline quote requests */
+static DEFINE_MUTEX(quote_lock);
+
+/*
+ * GetQuote request timeout in seconds. Expect that 30 seconds
+ * is enough time for QE to respond to any Quote requests.
+ */
+static u32 getquote_timeout = 30;
+
static long tdx_get_report0(struct tdx_report_req __user *req)
{
u8 *reportdata, *tdreport;
@@ -53,6 +101,150 @@ static long tdx_get_report0(struct tdx_report_req __user *req)
return ret;
}

+static void free_quote_buf(void *buf)
+{
+ size_t len = PAGE_ALIGN(GET_QUOTE_BUF_SIZE);
+ unsigned int count = len >> PAGE_SHIFT;
+
+ if (set_memory_encrypted((unsigned long)buf, count)) {
+ pr_err("Failed to restore encryption mask for Quote buffer, leak it\n");
+ return;
+ }
+
+ free_pages_exact(buf, len);
+}
+
+static void *alloc_quote_buf(void)
+{
+ size_t len = PAGE_ALIGN(GET_QUOTE_BUF_SIZE);
+ unsigned int count = len >> PAGE_SHIFT;
+ void *addr;
+
+ addr = alloc_pages_exact(len, GFP_KERNEL | __GFP_ZERO);
+ if (!addr)
+ return NULL;
+
+ if (set_memory_decrypted((unsigned long)addr, count)) {
+ free_pages_exact(addr, len);
+ return NULL;
+ }
+
+ return addr;
+}
+
+/*
+ * wait_for_quote_completion() - Wait for Quote request completion
+ * @quote_buf: Address of Quote buffer.
+ * @timeout: Timeout in seconds to wait for the Quote generation.
+ *
+ * As per TDX GHCI v1.0 specification, sec titled "TDG.VP.VMCALL<GetQuote>",
+ * the status field in the Quote buffer will be set to GET_QUOTE_IN_FLIGHT
+ * while VMM processes the GetQuote request, and will change it to success
+ * or error code after processing is complete. So wait till the status
+ * changes from GET_QUOTE_IN_FLIGHT or the request being timed out.
+ */
+static int wait_for_quote_completion(struct tdx_quote_buf *quote_buf, u32 timeout)
+{
+ int i = 0;
+
+ /*
+ * Quote requests usually take a few seconds to complete, so waking up
+ * once per second to recheck the status is fine for this use case.
+ */
+ while (quote_buf->status == GET_QUOTE_IN_FLIGHT && i++ < timeout) {
+ if (msleep_interruptible(MSEC_PER_SEC))
+ return -EINTR;
+ }
+
+ return (i == timeout) ? -ETIMEDOUT : 0;
+}
+
+static u8 *tdx_report_new(const struct tsm_desc *desc, void *data, size_t *outblob_len)
+{
+ struct tdx_quote_buf *quote_buf = quote_data;
+ int ret;
+ u8 *buf;
+ u64 err;
+
+ if (mutex_lock_interruptible(&quote_lock))
+ return ERR_PTR(-EINTR);
+
+ /*
+ * If the previous request is timedout or interrupted, and the
+ * Quote buf status is still in GET_QUOTE_IN_FLIGHT (owned by
+ * VMM), don't permit any new request.
+ */
+ if (quote_buf->status == GET_QUOTE_IN_FLIGHT) {
+ ret = -EBUSY;
+ goto done;
+ }
+
+ if (desc->inblob_len != TDX_REPORTDATA_LEN) {
+ ret = -EINVAL;
+ goto done;
+ }
+
+ /* TDX attestation only supports default format request */
+ if (desc->outblob_format != TSM_FORMAT_DEFAULT) {
+ ret = -EINVAL;
+ goto done;
+ }
+
+ u8 *reportdata __free(kfree) = kmalloc(TDX_REPORTDATA_LEN, GFP_KERNEL);
+ if (!reportdata) {
+ ret = -ENOMEM;
+ goto done;
+ }
+
+ u8 *tdreport __free(kfree) = kzalloc(TDX_REPORT_LEN, GFP_KERNEL);
+ if (!tdreport) {
+ ret = -ENOMEM;
+ goto done;
+ }
+
+ memcpy(reportdata, desc->inblob, desc->inblob_len);
+
+ /* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */
+ ret = tdx_mcall_get_report0(reportdata, tdreport);
+ if (ret) {
+ pr_err("GetReport call failed\n");
+ goto done;
+ }
+
+ memset(quote_data, 0, GET_QUOTE_BUF_SIZE);
+
+ /* Update Quote buffer header */
+ quote_buf->version = GET_QUOTE_CMD_VER;
+ quote_buf->in_len = TDX_REPORT_LEN;
+
+ memcpy(quote_buf->data, tdreport, TDX_REPORT_LEN);
+
+ err = tdx_hcall_get_quote(quote_data, GET_QUOTE_BUF_SIZE);
+ if (err) {
+ pr_err("GetQuote hypercall failed, status:%llx\n", err);
+ ret = -EIO;
+ goto done;
+ }
+
+ ret = wait_for_quote_completion(quote_buf, getquote_timeout);
+ if (ret) {
+ pr_err("GetQuote request timedout\n");
+ goto done;
+ }
+
+ buf = kvmemdup(quote_buf->data, quote_buf->out_len, GFP_KERNEL);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto done;
+ }
+
+ *outblob_len = quote_buf->out_len;
+
+done:
+ mutex_unlock(&quote_lock);
+ return ret ? ERR_PTR(ret) : buf;
+}
+
static long tdx_guest_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
@@ -82,17 +274,48 @@ static const struct x86_cpu_id tdx_guest_ids[] = {
};
MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);

+static const struct tsm_ops tdx_tsm_ops = {
+ .name = KBUILD_MODNAME,
+ .report_new = tdx_report_new,
+};
+
static int __init tdx_guest_init(void)
{
+ int ret;
+
if (!x86_match_cpu(tdx_guest_ids))
return -ENODEV;

- return misc_register(&tdx_misc_dev);
+ ret = misc_register(&tdx_misc_dev);
+ if (ret)
+ return ret;
+
+ quote_data = alloc_quote_buf();
+ if (!quote_data) {
+ pr_err("Failed to allocate Quote buffer\n");
+ ret = -ENOMEM;
+ goto free_misc;
+ }
+
+ ret = register_tsm(&tdx_tsm_ops, NULL, NULL);
+ if (ret)
+ goto free_quote;
+
+ return 0;
+
+free_quote:
+ free_quote_buf(quote_data);
+free_misc:
+ misc_deregister(&tdx_misc_dev);
+
+ return ret;
}
module_init(tdx_guest_init);

static void __exit tdx_guest_exit(void)
{
+ unregister_tsm(&tdx_tsm_ops);
+ free_quote_buf(quote_data);
misc_deregister(&tdx_misc_dev);
}
module_exit(tdx_guest_exit);
--
2.25.1


2023-09-20 17:05:00

by Kirill A. Shutemov

[permalink] [raw]
Subject: Re: [PATCH v2] virt: tdx-guest: Add Quote generation support using TSM_REPORTS

On Thu, Sep 14, 2023 at 03:13:49AM +0000, Kuppuswamy Sathyanarayanan wrote:
> In TDX guest, the attestation process is used to verify the TDX guest
> trustworthiness to other entities before provisioning secrets to the
> guest. The First step in the attestation process is TDREPORT

s/First/first/ ?

> generation, which involves getting the guest measurement data in the
> format of TDREPORT, which is further used to validate the authenticity
> of the TDX guest. TDREPORT by design is integrity-protected and can
> only be verified on the local machine.
>
> To support remote verification of the TDREPORT (in a SGX-based
> attestation), the TDREPORT needs to be sent to the SGX Quoting Enclave

Parentheses can be dropped.

> (QE) to convert it to a remotely verifiable Quote. SGX QE by design can
> only run outside of the TDX guest (i.e. in a host process or in a
> normal VM) and guest can use communication channels like vsock or
> TCP/IP to send the TDREPORT to the QE. But for security concerns, the
> TDX guest may not support these communication channels. To handle such
> cases, TDX defines a GetQuote hypercall which can be used by the guest
> to request the host VMM to communicate with the SGX QE. More details
> about GetQuote hypercall can be found in TDX Guest-Host Communication
> Interface (GHCI) for Intel TDX 1.0, section titled
> "TDG.VP.VMCALL<GetQuote>".
>
> Trusted Security Module (TSM) [1] exposes a common ABI for Confidential
> Computing Guest platforms to get the measurement data via ConfigFS.
> Extend the TSM framework and add support to allow an attestation agent
> to get the TDX Quote data (included usage example below).
>
> report=/sys/kernel/config/tsm/report/report0
> mkdir $report
> dd if=/dev/urandom bs=64 count=1 > $report/inblob
> hexdump -C $report/outblob
> rmdir $report
>
> GetQuote TDVMCALL requires TD guest pass a 4K aligned shared buffer
> with TDREPORT data as input, which is further used by the VMM to copy
> the TD Quote result after successful Quote generation. To create the
> shared buffer, allocate a large enough memory and mark it shared using
> set_memory_decrypted() in tdx_guest_init(). This buffer will be re-used
> for GetQuote requests in the TDX TSM handler.
>
> Although this method reserves a fixed chunk of memory for GetQuote
> requests, such one time allocation can help avoid memory fragmentation
> related allocation failures later in the uptime of the guest.
>
> Since the Quote generation process is not time-critical or frequently
> used, the current version uses a polling model for Quote requests and
> it also does not support parallel GetQuote requests.
>
> Link: https://lore.kernel.org/lkml/169342399185.3934343.3035845348326944519.stgit@dwillia2-xfh.jf.intel.com/ [1]
> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>

The patch looks good to me. See one question below.

> ---
>
> Hi All,
>
> The original version of this patch series [1] added support for TDX
> Guest Quote generation via an IOCTL interface. Since we have multiple
> vendors implementing such interface, to avoid ABI proliferation, Dan
> proposed using a common ABI for it and submitted the Trusted Secure
> module (TSM) report ABI support [2]. This patchset extends the
> TSM REPORTS to implement the TDX Quote generation support. Since there
> is a change in interface type, I have dropped the previous Acks.
>
> [1] https://lore.kernel.org/lkml/[email protected]/
> [2] https://lore.kernel.org/lkml/169342399185.3934343.3035845348326944519.stgit@dwillia2-xfh.jf.intel.com/
>
> Changes since v1:
> * Used mutext_lock_interruptible() for quote_lock to allow user
> interruption.
> * Used msleep_interruptible() instead of ssleep() to allow user
> interruption.
> * Added check for the set_memory_encrypted() return value.
> * Fixed typos in comments and commit log.
>
> Changes since previous version:
> * Used ConfigFS interface instead of IOCTL interface.
> * Used polling model for Quote generation and dropped the event notification IRQ support.
>
> arch/x86/coco/tdx/tdx.c | 21 +++
> arch/x86/include/asm/shared/tdx.h | 1 +
> arch/x86/include/asm/tdx.h | 2 +
> drivers/virt/coco/tdx-guest/Kconfig | 1 +
> drivers/virt/coco/tdx-guest/tdx-guest.c | 225 +++++++++++++++++++++++-
> 5 files changed, 249 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 1d6b863c42b0..752867b1d11b 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -104,6 +104,27 @@ int tdx_mcall_get_report0(u8 *reportdata, u8 *tdreport)
> }
> EXPORT_SYMBOL_GPL(tdx_mcall_get_report0);
>
> +/**
> + * tdx_hcall_get_quote() - Wrapper to request TD Quote using GetQuote
> + * hypercall.
> + * @buf: Address of the directly mapped shared kernel buffer which
> + * contains TDREPORT. The same buffer will be used by VMM to
> + * store the generated TD Quote output.
> + * @size: size of the tdquote buffer (4KB-aligned).
> + *
> + * Refer to section titled "TDG.VP.VMCALL<GetQuote>" in the TDX GHCI
> + * v1.0 specification for more information on GetQuote hypercall.
> + * It is used in the TDX guest driver module to get the TD Quote.
> + *
> + * Return 0 on success or error code on failure.
> + */
> +u64 tdx_hcall_get_quote(u8 *buf, size_t size)
> +{
> + /* Since buf is a shared memory, set the shared (decrypted) bits */
> + return _tdx_hypercall(TDVMCALL_GET_QUOTE, cc_mkdec(virt_to_phys(buf)), size, 0, 0);
> +}
> +EXPORT_SYMBOL_GPL(tdx_hcall_get_quote);
> +
> static void __noreturn tdx_panic(const char *msg)
> {
> struct tdx_hypercall_args args = {
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index 7513b3bb69b7..9eab19950f39 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -22,6 +22,7 @@
>
> /* TDX hypercall Leaf IDs */
> #define TDVMCALL_MAP_GPA 0x10001
> +#define TDVMCALL_GET_QUOTE 0x10002
> #define TDVMCALL_REPORT_FATAL_ERROR 0x10003
>
> #ifndef __ASSEMBLY__
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 603e6d1e9d4a..ebd1cda4875f 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -52,6 +52,8 @@ bool tdx_early_handle_ve(struct pt_regs *regs);
>
> int tdx_mcall_get_report0(u8 *reportdata, u8 *tdreport);
>
> +u64 tdx_hcall_get_quote(u8 *buf, size_t size);
> +
> #else
>
> static inline void tdx_early_init(void) { };
> diff --git a/drivers/virt/coco/tdx-guest/Kconfig b/drivers/virt/coco/tdx-guest/Kconfig
> index 14246fc2fb02..22dd59e19431 100644
> --- a/drivers/virt/coco/tdx-guest/Kconfig
> +++ b/drivers/virt/coco/tdx-guest/Kconfig
> @@ -1,6 +1,7 @@
> config TDX_GUEST_DRIVER
> tristate "TDX Guest driver"
> depends on INTEL_TDX_GUEST
> + select TSM_REPORTS
> help
> The driver provides userspace interface to communicate with
> the TDX module to request the TDX guest details like attestation
> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
> index 5e44a0fa69bd..9d9b92ca9b16 100644
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
> @@ -12,12 +12,60 @@
> #include <linux/mod_devicetable.h>
> #include <linux/string.h>
> #include <linux/uaccess.h>
> +#include <linux/set_memory.h>
> +#include <linux/io.h>
> +#include <linux/delay.h>
> +#include <linux/tsm.h>
> +#include <linux/sizes.h>
>
> #include <uapi/linux/tdx-guest.h>
>
> #include <asm/cpu_device_id.h>
> #include <asm/tdx.h>
>
> +/*
> + * Intel's SGX QE implementation generally uses Quote size less
> + * than 8K (2K Quote data + ~5K of certificate blob).
> + */
> +#define GET_QUOTE_BUF_SIZE SZ_8K
> +
> +#define GET_QUOTE_CMD_VER 1
> +
> +/* TDX GetQuote status codes */
> +#define GET_QUOTE_SUCCESS 0
> +#define GET_QUOTE_IN_FLIGHT 0xffffffffffffffff
> +
> +/* struct tdx_quote_buf: Format of Quote request buffer.
> + * @version: Quote format version, filled by TD.
> + * @status: Status code of Quote request, filled by VMM.
> + * @in_len: Length of TDREPORT, filled by TD.
> + * @out_len: Length of Quote data, filled by VMM.
> + * @data: Quote data on output or TDREPORT on input.
> + *
> + * More details of Quote request buffer can be found in TDX
> + * Guest-Host Communication Interface (GHCI) for Intel TDX 1.0,
> + * section titled "TDG.VP.VMCALL<GetQuote>"
> + */
> +struct tdx_quote_buf {
> + u64 version;
> + u64 status;
> + u32 in_len;
> + u32 out_len;
> + u8 data[];
> +};
> +
> +/* Quote data buffer */
> +static void *quote_data;
> +
> +/* Lock to streamline quote requests */
> +static DEFINE_MUTEX(quote_lock);
> +
> +/*
> + * GetQuote request timeout in seconds. Expect that 30 seconds
> + * is enough time for QE to respond to any Quote requests.
> + */
> +static u32 getquote_timeout = 30;
> +
> static long tdx_get_report0(struct tdx_report_req __user *req)
> {
> u8 *reportdata, *tdreport;
> @@ -53,6 +101,150 @@ static long tdx_get_report0(struct tdx_report_req __user *req)
> return ret;
> }
>
> +static void free_quote_buf(void *buf)
> +{
> + size_t len = PAGE_ALIGN(GET_QUOTE_BUF_SIZE);
> + unsigned int count = len >> PAGE_SHIFT;
> +
> + if (set_memory_encrypted((unsigned long)buf, count)) {
> + pr_err("Failed to restore encryption mask for Quote buffer, leak it\n");
> + return;
> + }
> +
> + free_pages_exact(buf, len);
> +}
> +
> +static void *alloc_quote_buf(void)
> +{
> + size_t len = PAGE_ALIGN(GET_QUOTE_BUF_SIZE);
> + unsigned int count = len >> PAGE_SHIFT;
> + void *addr;
> +
> + addr = alloc_pages_exact(len, GFP_KERNEL | __GFP_ZERO);
> + if (!addr)
> + return NULL;
> +
> + if (set_memory_decrypted((unsigned long)addr, count)) {
> + free_pages_exact(addr, len);
> + return NULL;
> + }
> +
> + return addr;
> +}
> +
> +/*
> + * wait_for_quote_completion() - Wait for Quote request completion
> + * @quote_buf: Address of Quote buffer.
> + * @timeout: Timeout in seconds to wait for the Quote generation.
> + *
> + * As per TDX GHCI v1.0 specification, sec titled "TDG.VP.VMCALL<GetQuote>",
> + * the status field in the Quote buffer will be set to GET_QUOTE_IN_FLIGHT
> + * while VMM processes the GetQuote request, and will change it to success
> + * or error code after processing is complete. So wait till the status
> + * changes from GET_QUOTE_IN_FLIGHT or the request being timed out.
> + */
> +static int wait_for_quote_completion(struct tdx_quote_buf *quote_buf, u32 timeout)
> +{
> + int i = 0;
> +
> + /*
> + * Quote requests usually take a few seconds to complete, so waking up
> + * once per second to recheck the status is fine for this use case.
> + */
> + while (quote_buf->status == GET_QUOTE_IN_FLIGHT && i++ < timeout) {
> + if (msleep_interruptible(MSEC_PER_SEC))
> + return -EINTR;
> + }
> +
> + return (i == timeout) ? -ETIMEDOUT : 0;
> +}
> +
> +static u8 *tdx_report_new(const struct tsm_desc *desc, void *data, size_t *outblob_len)
> +{
> + struct tdx_quote_buf *quote_buf = quote_data;
> + int ret;
> + u8 *buf;
> + u64 err;
> +
> + if (mutex_lock_interruptible(&quote_lock))
> + return ERR_PTR(-EINTR);
> +
> + /*
> + * If the previous request is timedout or interrupted, and the
> + * Quote buf status is still in GET_QUOTE_IN_FLIGHT (owned by
> + * VMM), don't permit any new request.
> + */
> + if (quote_buf->status == GET_QUOTE_IN_FLIGHT) {
> + ret = -EBUSY;
> + goto done;
> + }
> +
> + if (desc->inblob_len != TDX_REPORTDATA_LEN) {
> + ret = -EINVAL;
> + goto done;
> + }
> +
> + /* TDX attestation only supports default format request */
> + if (desc->outblob_format != TSM_FORMAT_DEFAULT) {
> + ret = -EINVAL;
> + goto done;
> + }
> +
> + u8 *reportdata __free(kfree) = kmalloc(TDX_REPORTDATA_LEN, GFP_KERNEL);

__free() is new to me. Good to know.

But are we okay now with declaring variables in the middle of the
function? Any reason we can't do at the top?

> + if (!reportdata) {
> + ret = -ENOMEM;
> + goto done;
> + }
> +
> + u8 *tdreport __free(kfree) = kzalloc(TDX_REPORT_LEN, GFP_KERNEL);
> + if (!tdreport) {
> + ret = -ENOMEM;
> + goto done;
> + }
> +
> + memcpy(reportdata, desc->inblob, desc->inblob_len);
> +
> + /* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */
> + ret = tdx_mcall_get_report0(reportdata, tdreport);
> + if (ret) {
> + pr_err("GetReport call failed\n");
> + goto done;
> + }
> +
> + memset(quote_data, 0, GET_QUOTE_BUF_SIZE);
> +
> + /* Update Quote buffer header */
> + quote_buf->version = GET_QUOTE_CMD_VER;
> + quote_buf->in_len = TDX_REPORT_LEN;
> +
> + memcpy(quote_buf->data, tdreport, TDX_REPORT_LEN);
> +
> + err = tdx_hcall_get_quote(quote_data, GET_QUOTE_BUF_SIZE);
> + if (err) {
> + pr_err("GetQuote hypercall failed, status:%llx\n", err);
> + ret = -EIO;
> + goto done;
> + }
> +
> + ret = wait_for_quote_completion(quote_buf, getquote_timeout);
> + if (ret) {
> + pr_err("GetQuote request timedout\n");
> + goto done;
> + }
> +
> + buf = kvmemdup(quote_buf->data, quote_buf->out_len, GFP_KERNEL);
> + if (!buf) {
> + ret = -ENOMEM;
> + goto done;
> + }
> +
> + *outblob_len = quote_buf->out_len;
> +
> +done:
> + mutex_unlock(&quote_lock);
> + return ret ? ERR_PTR(ret) : buf;
> +}
> +
> static long tdx_guest_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg)
> {
> @@ -82,17 +274,48 @@ static const struct x86_cpu_id tdx_guest_ids[] = {
> };
> MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);
>
> +static const struct tsm_ops tdx_tsm_ops = {
> + .name = KBUILD_MODNAME,
> + .report_new = tdx_report_new,
> +};
> +
> static int __init tdx_guest_init(void)
> {
> + int ret;
> +
> if (!x86_match_cpu(tdx_guest_ids))
> return -ENODEV;
>
> - return misc_register(&tdx_misc_dev);
> + ret = misc_register(&tdx_misc_dev);
> + if (ret)
> + return ret;
> +
> + quote_data = alloc_quote_buf();
> + if (!quote_data) {
> + pr_err("Failed to allocate Quote buffer\n");
> + ret = -ENOMEM;
> + goto free_misc;
> + }
> +
> + ret = register_tsm(&tdx_tsm_ops, NULL, NULL);
> + if (ret)
> + goto free_quote;
> +
> + return 0;
> +
> +free_quote:
> + free_quote_buf(quote_data);
> +free_misc:
> + misc_deregister(&tdx_misc_dev);
> +
> + return ret;
> }
> module_init(tdx_guest_init);
>
> static void __exit tdx_guest_exit(void)
> {
> + unregister_tsm(&tdx_tsm_ops);
> + free_quote_buf(quote_data);
> misc_deregister(&tdx_misc_dev);
> }
> module_exit(tdx_guest_exit);
> --
> 2.25.1
>

--
Kiryl Shutsemau / Kirill A. Shutemov

2023-09-20 17:35:36

by Daniel P. Berrangé

[permalink] [raw]
Subject: Re: [PATCH v2] virt: tdx-guest: Add Quote generation support using TSM_REPORTS

On Wed, Sep 20, 2023 at 04:16:33PM +0300, Kirill A . Shutemov wrote:
> On Thu, Sep 14, 2023 at 03:13:49AM +0000, Kuppuswamy Sathyanarayanan wrote:
> > In TDX guest, the attestation process is used to verify the TDX guest
> > trustworthiness to other entities before provisioning secrets to the
> > guest. The First step in the attestation process is TDREPORT
>
> s/First/first/ ?
>
> > generation, which involves getting the guest measurement data in the
> > format of TDREPORT, which is further used to validate the authenticity
> > of the TDX guest. TDREPORT by design is integrity-protected and can
> > only be verified on the local machine.
> >
> > To support remote verification of the TDREPORT (in a SGX-based
> > attestation), the TDREPORT needs to be sent to the SGX Quoting Enclave
>
> Parentheses can be dropped.
>
> > (QE) to convert it to a remotely verifiable Quote. SGX QE by design can
> > only run outside of the TDX guest (i.e. in a host process or in a
> > normal VM) and guest can use communication channels like vsock or
> > TCP/IP to send the TDREPORT to the QE. But for security concerns, the
> > TDX guest may not support these communication channels. To handle such
> > cases, TDX defines a GetQuote hypercall which can be used by the guest
> > to request the host VMM to communicate with the SGX QE. More details
> > about GetQuote hypercall can be found in TDX Guest-Host Communication
> > Interface (GHCI) for Intel TDX 1.0, section titled
> > "TDG.VP.VMCALL<GetQuote>".
> >
> > Trusted Security Module (TSM) [1] exposes a common ABI for Confidential
> > Computing Guest platforms to get the measurement data via ConfigFS.
> > Extend the TSM framework and add support to allow an attestation agent
> > to get the TDX Quote data (included usage example below).
> >
> > report=/sys/kernel/config/tsm/report/report0
> > mkdir $report
> > dd if=/dev/urandom bs=64 count=1 > $report/inblob
> > hexdump -C $report/outblob
> > rmdir $report
> >
> > GetQuote TDVMCALL requires TD guest pass a 4K aligned shared buffer
> > with TDREPORT data as input, which is further used by the VMM to copy
> > the TD Quote result after successful Quote generation. To create the
> > shared buffer, allocate a large enough memory and mark it shared using
> > set_memory_decrypted() in tdx_guest_init(). This buffer will be re-used
> > for GetQuote requests in the TDX TSM handler.
> >
> > Although this method reserves a fixed chunk of memory for GetQuote
> > requests, such one time allocation can help avoid memory fragmentation
> > related allocation failures later in the uptime of the guest.
> >
> > Since the Quote generation process is not time-critical or frequently
> > used, the current version uses a polling model for Quote requests and
> > it also does not support parallel GetQuote requests.
> >
> > Link: https://lore.kernel.org/lkml/169342399185.3934343.3035845348326944519.stgit@dwillia2-xfh.jf.intel.com/ [1]
> > Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
>
> The patch looks good to me. See one question below.

> > +static u8 *tdx_report_new(const struct tsm_desc *desc, void *data, size_t *outblob_len)
> > +{
> > + struct tdx_quote_buf *quote_buf = quote_data;
> > + int ret;
> > + u8 *buf;
> > + u64 err;
> > +
> > + if (mutex_lock_interruptible(&quote_lock))
> > + return ERR_PTR(-EINTR);
> > +
> > + /*
> > + * If the previous request is timedout or interrupted, and the
> > + * Quote buf status is still in GET_QUOTE_IN_FLIGHT (owned by
> > + * VMM), don't permit any new request.
> > + */
> > + if (quote_buf->status == GET_QUOTE_IN_FLIGHT) {
> > + ret = -EBUSY;
> > + goto done;
> > + }
> > +
> > + if (desc->inblob_len != TDX_REPORTDATA_LEN) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> > +
> > + /* TDX attestation only supports default format request */
> > + if (desc->outblob_format != TSM_FORMAT_DEFAULT) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> > +
> > + u8 *reportdata __free(kfree) = kmalloc(TDX_REPORTDATA_LEN, GFP_KERNEL);
>
> __free() is new to me. Good to know.
>
> But are we okay now with declaring variables in the middle of the
> function? Any reason we can't do at the top?

I expect that to be unsafe and result in uninitialized data from the
stack being passed to kfree, if a "goto done" in the lines prior to
this declaration is triggered.

The 'reportdata' variable will be in scope at the "done:" label,
thus triggering the __free callback, but the 'kmalloc' initializer
will not have executed.

Variables should always be declared prior to any 'goto' statement
within the same block of scope, if relying on __attribute__((cleanup))
callbacks.

>
> > + if (!reportdata) {
> > + ret = -ENOMEM;
> > + goto done;
> > + }
> > +
> > + u8 *tdreport __free(kfree) = kzalloc(TDX_REPORT_LEN, GFP_KERNEL);
> > + if (!tdreport) {
> > + ret = -ENOMEM;
> > + goto done;
> > + }
> > +
> > + memcpy(reportdata, desc->inblob, desc->inblob_len);
> > +
> > + /* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */
> > + ret = tdx_mcall_get_report0(reportdata, tdreport);
> > + if (ret) {
> > + pr_err("GetReport call failed\n");
> > + goto done;
> > + }
> > +
> > + memset(quote_data, 0, GET_QUOTE_BUF_SIZE);
> > +
> > + /* Update Quote buffer header */
> > + quote_buf->version = GET_QUOTE_CMD_VER;
> > + quote_buf->in_len = TDX_REPORT_LEN;
> > +
> > + memcpy(quote_buf->data, tdreport, TDX_REPORT_LEN);
> > +
> > + err = tdx_hcall_get_quote(quote_data, GET_QUOTE_BUF_SIZE);
> > + if (err) {
> > + pr_err("GetQuote hypercall failed, status:%llx\n", err);
> > + ret = -EIO;
> > + goto done;
> > + }
> > +
> > + ret = wait_for_quote_completion(quote_buf, getquote_timeout);
> > + if (ret) {
> > + pr_err("GetQuote request timedout\n");
> > + goto done;
> > + }
> > +
> > + buf = kvmemdup(quote_buf->data, quote_buf->out_len, GFP_KERNEL);
> > + if (!buf) {
> > + ret = -ENOMEM;
> > + goto done;
> > + }
> > +
> > + *outblob_len = quote_buf->out_len;
> > +
> > +done:
> > + mutex_unlock(&quote_lock);
> > + return ret ? ERR_PTR(ret) : buf;
> > +}

With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Subject: Re: [PATCH v2] virt: tdx-guest: Add Quote generation support using TSM_REPORTS



On 9/20/2023 6:16 AM, Kirill A . Shutemov wrote:
>> +static u8 *tdx_report_new(const struct tsm_desc *desc, void *data, size_t *outblob_len)
>> +{
>> + struct tdx_quote_buf *quote_buf = quote_data;
>> + int ret;
>> + u8 *buf;
>> + u64 err;
>> +
>> + if (mutex_lock_interruptible(&quote_lock))
>> + return ERR_PTR(-EINTR);
>> +
>> + /*
>> + * If the previous request is timedout or interrupted, and the
>> + * Quote buf status is still in GET_QUOTE_IN_FLIGHT (owned by
>> + * VMM), don't permit any new request.
>> + */
>> + if (quote_buf->status == GET_QUOTE_IN_FLIGHT) {
>> + ret = -EBUSY;
>> + goto done;
>> + }
>> +
>> + if (desc->inblob_len != TDX_REPORTDATA_LEN) {
>> + ret = -EINVAL;
>> + goto done;
>> + }
>> +
>> + /* TDX attestation only supports default format request */
>> + if (desc->outblob_format != TSM_FORMAT_DEFAULT) {
>> + ret = -EINVAL;
>> + goto done;
>> + }
>> +
>> + u8 *reportdata __free(kfree) = kmalloc(TDX_REPORTDATA_LEN, GFP_KERNEL);
> __free() is new to me. Good to know.
>
> But are we okay now with declaring variables in the middle of the
> function? Any reason we can't do at the top?

Declaring variables at the top is no longer a hard requirement. The main reason
for declaring it here is to use __free cleanup function. If we use top
declaration, then we have free it manually.

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

2023-09-20 20:39:14

by Kirill A. Shutemov

[permalink] [raw]
Subject: Re: [PATCH v2] virt: tdx-guest: Add Quote generation support using TSM_REPORTS

On Wed, Sep 20, 2023 at 08:27:39AM -0700, Kuppuswamy Sathyanarayanan wrote:
>
>
> On 9/20/2023 6:16 AM, Kirill A . Shutemov wrote:
> >> +static u8 *tdx_report_new(const struct tsm_desc *desc, void *data, size_t *outblob_len)
> >> +{
> >> + struct tdx_quote_buf *quote_buf = quote_data;
> >> + int ret;
> >> + u8 *buf;
> >> + u64 err;
> >> +
> >> + if (mutex_lock_interruptible(&quote_lock))
> >> + return ERR_PTR(-EINTR);
> >> +
> >> + /*
> >> + * If the previous request is timedout or interrupted, and the
> >> + * Quote buf status is still in GET_QUOTE_IN_FLIGHT (owned by
> >> + * VMM), don't permit any new request.
> >> + */
> >> + if (quote_buf->status == GET_QUOTE_IN_FLIGHT) {
> >> + ret = -EBUSY;
> >> + goto done;
> >> + }
> >> +
> >> + if (desc->inblob_len != TDX_REPORTDATA_LEN) {
> >> + ret = -EINVAL;
> >> + goto done;
> >> + }
> >> +
> >> + /* TDX attestation only supports default format request */
> >> + if (desc->outblob_format != TSM_FORMAT_DEFAULT) {
> >> + ret = -EINVAL;
> >> + goto done;
> >> + }
> >> +
> >> + u8 *reportdata __free(kfree) = kmalloc(TDX_REPORTDATA_LEN, GFP_KERNEL);
> > __free() is new to me. Good to know.
> >
> > But are we okay now with declaring variables in the middle of the
> > function? Any reason we can't do at the top?
>
> Declaring variables at the top is no longer a hard requirement. The main reason
> for declaring it here is to use __free cleanup function. If we use top
> declaration, then we have free it manually.

What's wrong with allocating it it there too?

--
Kiryl Shutsemau / Kirill A. Shutemov

2023-09-20 21:52:01

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v2] virt: tdx-guest: Add Quote generation support using TSM_REPORTS

On 9/20/23 11:08, Kuppuswamy Sathyanarayanan wrote:
> My thinking is to allocate it when we really need it. We only need this memory if the
> GetQuote hypercall is successful. We can also allocate it at the top and there is
> nothing wrong with it, but it will not be used in failure cases. Since top declarations
> are not a requirement, why allocate it early?

Do folks *REALLY* want this patch set to be a trailblazer where we can
all nitpick the nuances of how we want to deal with this snazzy new
__free() mechanism?

Or, do you want it to be old and boring and do it the way we've done it
forever?

Your choice.

Subject: Re: [PATCH v2] virt: tdx-guest: Add Quote generation support using TSM_REPORTS



On 9/20/2023 10:52 AM, Kirill A . Shutemov wrote:
> On Wed, Sep 20, 2023 at 08:27:39AM -0700, Kuppuswamy Sathyanarayanan wrote:
>>
>>
>> On 9/20/2023 6:16 AM, Kirill A . Shutemov wrote:
>>>> +static u8 *tdx_report_new(const struct tsm_desc *desc, void *data, size_t *outblob_len)
>>>> +{
>>>> + struct tdx_quote_buf *quote_buf = quote_data;
>>>> + int ret;
>>>> + u8 *buf;
>>>> + u64 err;
>>>> +
>>>> + if (mutex_lock_interruptible(&quote_lock))
>>>> + return ERR_PTR(-EINTR);
>>>> +
>>>> + /*
>>>> + * If the previous request is timedout or interrupted, and the
>>>> + * Quote buf status is still in GET_QUOTE_IN_FLIGHT (owned by
>>>> + * VMM), don't permit any new request.
>>>> + */
>>>> + if (quote_buf->status == GET_QUOTE_IN_FLIGHT) {
>>>> + ret = -EBUSY;
>>>> + goto done;
>>>> + }
>>>> +
>>>> + if (desc->inblob_len != TDX_REPORTDATA_LEN) {
>>>> + ret = -EINVAL;
>>>> + goto done;
>>>> + }
>>>> +
>>>> + /* TDX attestation only supports default format request */
>>>> + if (desc->outblob_format != TSM_FORMAT_DEFAULT) {
>>>> + ret = -EINVAL;
>>>> + goto done;
>>>> + }
>>>> +
>>>> + u8 *reportdata __free(kfree) = kmalloc(TDX_REPORTDATA_LEN, GFP_KERNEL);
>>> __free() is new to me. Good to know.
>>>
>>> But are we okay now with declaring variables in the middle of the
>>> function? Any reason we can't do at the top?
>>
>> Declaring variables at the top is no longer a hard requirement. The main reason
>> for declaring it here is to use __free cleanup function. If we use top
>> declaration, then we have free it manually.
>
> What's wrong with allocating it it there too?

My thinking is to allocate it when we really need it. We only need this memory if the
GetQuote hypercall is successful. We can also allocate it at the top and there is
nothing wrong with it, but it will not be used in failure cases. Since top declarations
are not a requirement, why allocate it early?


>

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

2023-09-21 08:28:03

by Erdem Aktas

[permalink] [raw]
Subject: Re: [PATCH v2] virt: tdx-guest: Add Quote generation support using TSM_REPORTS

On Wed, Sep 13, 2023 at 8:14 PM Kuppuswamy Sathyanarayanan
<[email protected]> wrote:
>
> In TDX guest, the attestation process is used to verify the TDX guest
> trustworthiness to other entities before provisioning secrets to the
> guest. The First step in the attestation process is TDREPORT
> generation, which involves getting the guest measurement data in the
> format of TDREPORT, which is further used to validate the authenticity
> of the TDX guest. TDREPORT by design is integrity-protected and can
> only be verified on the local machine.
>
> To support remote verification of the TDREPORT (in a SGX-based
> attestation), the TDREPORT needs to be sent to the SGX Quoting Enclave
> (QE) to convert it to a remotely verifiable Quote. SGX QE by design can
> only run outside of the TDX guest (i.e. in a host process or in a
> normal VM) and guest can use communication channels like vsock or
> TCP/IP to send the TDREPORT to the QE. But for security concerns, the
> TDX guest may not support these communication channels. To handle such
> cases, TDX defines a GetQuote hypercall which can be used by the guest
> to request the host VMM to communicate with the SGX QE. More details
> about GetQuote hypercall can be found in TDX Guest-Host Communication
> Interface (GHCI) for Intel TDX 1.0, section titled
> "TDG.VP.VMCALL<GetQuote>".
>
> Trusted Security Module (TSM) [1] exposes a common ABI for Confidential
> Computing Guest platforms to get the measurement data via ConfigFS.
> Extend the TSM framework and add support to allow an attestation agent
> to get the TDX Quote data (included usage example below).
>
> report=/sys/kernel/config/tsm/report/report0
> mkdir $report
> dd if=/dev/urandom bs=64 count=1 > $report/inblob
> hexdump -C $report/outblob
> rmdir $report
>
> GetQuote TDVMCALL requires TD guest pass a 4K aligned shared buffer
> with TDREPORT data as input, which is further used by the VMM to copy
> the TD Quote result after successful Quote generation. To create the
> shared buffer, allocate a large enough memory and mark it shared using
> set_memory_decrypted() in tdx_guest_init(). This buffer will be re-used
> for GetQuote requests in the TDX TSM handler.
>
> Although this method reserves a fixed chunk of memory for GetQuote
> requests, such one time allocation can help avoid memory fragmentation
> related allocation failures later in the uptime of the guest.
>
> Since the Quote generation process is not time-critical or frequently
> used, the current version uses a polling model for Quote requests and
> it also does not support parallel GetQuote requests.
>
> Link: https://lore.kernel.org/lkml/169342399185.3934343.3035845348326944519.stgit@dwillia2-xfh.jf.intel.com/ [1]
> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
> ---
>
> Hi All,
>
> The original version of this patch series [1] added support for TDX
> Guest Quote generation via an IOCTL interface. Since we have multiple
> vendors implementing such interface, to avoid ABI proliferation, Dan
> proposed using a common ABI for it and submitted the Trusted Secure
> module (TSM) report ABI support [2]. This patchset extends the
> TSM REPORTS to implement the TDX Quote generation support. Since there
> is a change in interface type, I have dropped the previous Acks.
>
> [1] https://lore.kernel.org/lkml/[email protected]/
> [2] https://lore.kernel.org/lkml/169342399185.3934343.3035845348326944519.stgit@dwillia2-xfh.jf.intel.com/
>
> Changes since v1:
> * Used mutext_lock_interruptible() for quote_lock to allow user
> interruption.
> * Used msleep_interruptible() instead of ssleep() to allow user
> interruption.
> * Added check for the set_memory_encrypted() return value.
> * Fixed typos in comments and commit log.
>
Thanks for the changes, the patch looks good to me.
Reviewed-by : Erdem Aktas <[email protected]>

2023-09-22 05:29:02

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2] virt: tdx-guest: Add Quote generation support using TSM_REPORTS

Hi Kuppuswamy,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on linus/master v6.6-rc2 next-20230921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Kuppuswamy-Sathyanarayanan/virt-tdx-guest-Add-Quote-generation-support-using-TSM_REPORTS/20230914-111637
base: tip/x86/core
patch link: https://lore.kernel.org/r/20230914031349.23516-1-sathyanarayanan.kuppuswamy%40linux.intel.com
patch subject: [PATCH v2] virt: tdx-guest: Add Quote generation support using TSM_REPORTS
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230922/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230922/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/virt/coco/tdx-guest/tdx-guest.c:18:10: fatal error: linux/tsm.h: No such file or directory
18 | #include <linux/tsm.h>
| ^~~~~~~~~~~~~
compilation terminated.


vim +18 drivers/virt/coco/tdx-guest/tdx-guest.c

> 18 #include <linux/tsm.h>
19 #include <linux/sizes.h>
20

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki