On Wed, Apr 12, 2023 at 8:42 PM Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> wrote:
>
> In TDX guest, the second stage in attestation process is to send the
> TDREPORT to QE/QGS to generate the TD Quote. For platforms that does
Is it common to state TDREPORT when TDREPORT_STRUCT is meant? Here and below.
The GHCI documentation seems to use the two as synonyms but doesn't
explicitly say so.
nit: platforms that do not
> not support communication channels like vsock or TCP/IP, implement
> support to get TD Quote using hypercall. GetQuote hypercall can be used
> by the TD guest to request VMM facilitate the Quote generation via
nit: request the VMM to facilitate
> QE/QGS. 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>".
>
> Add support for TDX_CMD_GET_QUOTE IOCTL to allow attestation agent
nit: an attestation agent to
> submit GetQuote requests from the user space using GetQuote hypercall.
>
> Since GetQuote is an asynchronous request hypercall, VMM will use
> callback interrupt vector configured by SetupEventNotifyInterrupt
nit: a callback interrupt vector configured by the SetupEventNotifyInterrupt
> hypercall to notify the guest about Quote generation completion or
> failure. So register an IRQ handler for it.
>
> 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 the required memory using alloc_pages() and
> mark it shared using set_memory_decrypted() in tdx_guest_init(). This
> buffer will be re-used for GetQuote requests in TDX_CMD_GET_QUOTE
suggestion: will be cleared and re-used
> IOCTL handler.
>
> Although this method will reserve a fixed chunk of memory for
> GetQuote requests during the init time, it is preferable to the
The reservation isn't just during the init time. The reservation is
for the lifetime of the driver.
> alternative choice of allocating/freeing the shared buffer in the
> TDX_CMD_GET_QUOTE IOCTL handler, which will damage the direct map.
>
Why does allocation during the ioctl damage the direct map and
allocation on init doesn't?
I would suggest rephrasing to say that you're avoiding multiple
bookkeeping round-trips with the VMM for direct map updates.
> Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx>
> Reviewed-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> Acked-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> ---
>
> Changes since v1:
> * Removed platform bus device support.
> * Instead of allocating the shared buffers using DMA APIs in IOCTL
> handler, allocated it once in tdx_guest_init() and re-used it in
> GetQuote IOCTL handler.
> * To simplify the design, removed the support for parallel GetQuote
> requests. It can be added when there is a real requirement for it.
> * Fixed commit log and comments to reflect the latest changes.
>
> Documentation/virt/coco/tdx-guest.rst | 11 ++
> arch/x86/coco/tdx/tdx.c | 40 ++++++
> arch/x86/include/asm/tdx.h | 2 +
> drivers/virt/coco/tdx-guest/tdx-guest.c | 168 +++++++++++++++++++++++-
> include/uapi/linux/tdx-guest.h | 43 ++++++
> 5 files changed, 263 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/virt/coco/tdx-guest.rst b/Documentation/virt/coco/tdx-guest.rst
> index 46e316db6bb4..54601dcd5864 100644
> --- a/Documentation/virt/coco/tdx-guest.rst
> +++ b/Documentation/virt/coco/tdx-guest.rst
> @@ -42,6 +42,17 @@ ABI. However, in the future, if the TDX Module supports more than one subtype,
> a new IOCTL CMD will be created to handle it. To keep the IOCTL naming
> consistent, a subtype index is added as part of the IOCTL CMD.
>
> +2.2 TDX_CMD_GET_QUOTE
> +----------------------
> +
> +:Input parameters: struct tdx_quote_req
> +:Output: Return 0 on success, -EIO on TDCALL failure or standard error number
> + on common failures. Upon successful execution, QUOTE data is copied
> + to tdx_quote_req.buf.
> +
> +The TDX_CMD_GET_QUOTE IOCTL can be used by attestation software to generate
> +QUOTE for the given TDREPORT using TDG.VP.VMCALL<GetQuote> hypercall.
> +
> Reference
> ---------
>
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 26f6e2eaf5c8..09b5925eec67 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -33,6 +33,7 @@
> #define TDVMCALL_MAP_GPA 0x10001
> #define TDVMCALL_REPORT_FATAL_ERROR 0x10003
> #define TDVMCALL_SETUP_NOTIFY_INTR 0x10004
> +#define TDVMCALL_GET_QUOTE 0x10002
>
> /* MMIO direction */
> #define EPT_READ 0
> @@ -198,6 +199,45 @@ static void __noreturn tdx_panic(const char *msg)
> __tdx_hypercall(&args, 0);
> }
>
> +/**
> + * tdx_hcall_get_quote() - Wrapper to request TD Quote using GetQuote
> + * hypercall.
> + * @tdquote: Address of the direct mapped shared kernel buffer which
> + * contains TDREPORT data. The same buffer will be used by
> + * VMM to store the generated TD Quote output.
> + * @size: size of the tdquote buffer.
> + *
> + * 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.
> + */
> +int tdx_hcall_get_quote(u8 *tdquote, size_t size)
> +{
> + struct tdx_hypercall_args args = {0};
> +
> + /*
> + * TDX guest driver is the only user of this function and it uses
> + * the kernel mapped memory. So use virt_to_phys() to get the
> + * physical address of the TDQuote buffer without any additional
> + * checks for memory type.
> + */
> + args.r10 = TDX_HYPERCALL_STANDARD;
> + args.r11 = TDVMCALL_GET_QUOTE;
> + args.r12 = cc_mkdec(virt_to_phys(tdquote));
> + args.r13 = size;
> +
> + /*
> + * Pass the physical address of TDREPORT to the VMM and
> + * trigger the Quote generation. It is not a blocking
> + * call, hence completion of this request will be notified to
> + * the TD guest via a callback interrupt.
> + */
> + return __tdx_hypercall(&args, 0);
> +}
> +EXPORT_SYMBOL_GPL(tdx_hcall_get_quote);
> +
> static void tdx_parse_tdinfo(u64 *cc_mask)
> {
> struct tdx_module_output out;
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 8807fe1b1f3f..a72bd7b96564 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -75,6 +75,8 @@ int tdx_register_event_irq_cb(tdx_event_irq_cb_t handler, void *data);
>
> int tdx_unregister_event_irq_cb(tdx_event_irq_cb_t handler, void *data);
>
> +int tdx_hcall_get_quote(u8 *tdquote, size_t size);
> +
> #else
>
> static inline void tdx_early_init(void) { };
> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
> index 5e44a0fa69bd..a275d6b55f33 100644
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
> @@ -12,12 +12,105 @@
> #include <linux/mod_devicetable.h>
> #include <linux/string.h>
> #include <linux/uaccess.h>
> +#include <linux/set_memory.h>
>
> #include <uapi/linux/tdx-guest.h>
>
> #include <asm/cpu_device_id.h>
> #include <asm/tdx.h>
>
> +#define GET_QUOTE_MAX_SIZE (4 * PAGE_SIZE)
> +
> +/**
> + * struct quote_entry - Quote request struct
> + * @valid: Flag to check validity of the GetQuote request.
> + * @buf: Kernel buffer to share data with VMM (size is page aligned).
> + * @buf_len: Size of the buf in bytes.
> + * @compl: Completion object to track completion of GetQuote request.
> + */
> +struct quote_entry {
> + bool valid;
> + void *buf;
> + size_t buf_len;
> + struct completion compl;
> +};
> +
> +/* Quote data entry */
> +static struct quote_entry *qentry;
> +
> +/* Lock to streamline quote requests */
> +static DEFINE_MUTEX(quote_lock);
> +
> +static int quote_cb_handler(void *dev_id)
> +{
> + struct quote_entry *entry = dev_id;
> + struct tdx_quote_hdr *quote_hdr = entry->buf;
> +
> + if (entry->valid && quote_hdr->status != GET_QUOTE_IN_FLIGHT)
> + complete(&entry->compl);
> +
> + return 0;
> +}
> +
> +static void free_shared_pages(void *buf, size_t len)
> +{
> + unsigned int count = PAGE_ALIGN(len) >> PAGE_SHIFT;
> +
> + if (!buf)
> + return;
> +
> + set_memory_encrypted((unsigned long)buf, count);
> +
> + __free_pages(virt_to_page(buf), get_order(len));
> +}
> +
> +static void *alloc_shared_pages(size_t len)
> +{
> + unsigned int count = PAGE_ALIGN(len) >> PAGE_SHIFT;
> + struct page *page;
> + int ret;
> +
> + page = alloc_pages(GFP_KERNEL, get_order(len));
> + if (!page)
> + return NULL;
> +
> + ret = set_memory_decrypted((unsigned long)page_address(page), count);
> + if (ret) {
> + __free_pages(page, get_order(len));
> + return NULL;
> + }
> +
> + return page_address(page);
> +}
> +
> +static struct quote_entry *alloc_quote_entry(size_t len)
> +{
> + struct quote_entry *entry = NULL;
> + size_t new_len = PAGE_ALIGN(len);
> +
> + entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> + if (!entry)
> + return NULL;
> +
> + entry->buf = alloc_shared_pages(new_len);
> + if (!entry->buf) {
> + kfree(entry);
> + return NULL;
> + }
> +
> + entry->buf_len = new_len;
> + init_completion(&entry->compl);
> + entry->valid = false;
> +
> + return entry;
> +}
> +
> +static void free_quote_entry(struct quote_entry *entry)
> +{
> + free_shared_pages(entry->buf, entry->buf_len);
> + kfree(entry);
> +}
> +
> static long tdx_get_report0(struct tdx_report_req __user *req)
> {
> u8 *reportdata, *tdreport;
> @@ -53,12 +146,59 @@ static long tdx_get_report0(struct tdx_report_req __user *req)
> return ret;
> }
>
> +static long tdx_get_quote(struct tdx_quote_req __user *ureq)
> +{
> + struct tdx_quote_req req;
> + long ret;
> +
> + if (copy_from_user(&req, ureq, sizeof(req)))
> + return -EFAULT;
> +
> + mutex_lock("e_lock);
> +
> + if (!req.len || req.len > qentry->buf_len) {
> + ret = -EINVAL;
> + goto quote_failed;
> + }
> +
Since the qentry is reused across calls, I think you need to clear it
out before repopulating it with maybe less data:
memset(qentry->buf, 0, qentry->buf_len)
I'm not particularly clear on why there is a length though, since the
buffer should always be a TDREPORT_STRUCT. I guess version updates
could expand the size.
> + if (copy_from_user(qentry->buf, (void __user *)req.buf, req.len)) {
> + ret = -EFAULT;
> + goto quote_failed;
> + }
> +
> + qentry->valid = true;
Is the valid field used anywhere? I only see it written and never read.
> +
> + reinit_completion(&qentry->compl);
> +
> + /* Submit GetQuote Request using GetQuote hypercall */
> + ret = tdx_hcall_get_quote(qentry->buf, qentry->buf_len);
> + if (ret) {
> + pr_err("GetQuote hypercall failed, status:%lx\n", ret);
> + ret = -EIO;
> + goto quote_failed;
> + }
> +
> + /* Wait till GetQuote completion */
> + wait_for_completion(&qentry->compl);
> +
> + if (copy_to_user((void __user *)req.buf, qentry->buf, req.len))
> + ret = -EFAULT;
> +
> +quote_failed:
> + qentry->valid = false;
> + mutex_unlock("e_lock);
> +
> + return ret;
> +}
> +
> static long tdx_guest_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg)
> {
> switch (cmd) {
> case TDX_CMD_GET_REPORT0:
> return tdx_get_report0((struct tdx_report_req __user *)arg);
> + case TDX_CMD_GET_QUOTE:
> + return tdx_get_quote((struct tdx_quote_req *)arg);
> default:
> return -ENOTTY;
> }
> @@ -84,15 +224,41 @@ MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);
>
> 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;
> +
> + qentry = alloc_quote_entry(GET_QUOTE_MAX_SIZE);
> + if (!qentry) {
> + pr_err("Quote entry allocation failed\n");
> + ret = -ENOMEM;
> + goto free_misc;
> + }
> +
> + ret = tdx_register_event_irq_cb(quote_cb_handler, qentry);
> + if (ret)
> + goto free_quote;
> +
> + return 0;
> +
> +free_quote:
> + free_quote_entry(qentry);
> +free_misc:
> + misc_deregister(&tdx_misc_dev);
> +
> + return ret;
> }
> module_init(tdx_guest_init);
>
> static void __exit tdx_guest_exit(void)
> {
> + tdx_unregister_event_irq_cb(quote_cb_handler, qentry);
> + free_quote_entry(qentry);
> misc_deregister(&tdx_misc_dev);
> }
> module_exit(tdx_guest_exit);
> diff --git a/include/uapi/linux/tdx-guest.h b/include/uapi/linux/tdx-guest.h
> index a6a2098c08ff..500cdfa025ad 100644
> --- a/include/uapi/linux/tdx-guest.h
> +++ b/include/uapi/linux/tdx-guest.h
> @@ -17,6 +17,12 @@
> /* Length of TDREPORT used in TDG.MR.REPORT TDCALL */
> #define TDX_REPORT_LEN 1024
>
> +/* TD Quote status codes */
> +#define GET_QUOTE_SUCCESS 0
> +#define GET_QUOTE_IN_FLIGHT 0xffffffffffffffff
> +#define GET_QUOTE_ERROR 0x8000000000000000
> +#define GET_QUOTE_SERVICE_UNAVAILABLE 0x8000000000000001
> +
> /**
> * struct tdx_report_req - Request struct for TDX_CMD_GET_REPORT0 IOCTL.
> *
> @@ -30,6 +36,35 @@ struct tdx_report_req {
> __u8 tdreport[TDX_REPORT_LEN];
> };
>
> +/* struct tdx_quote_hdr: Format of Quote request buffer header.
> + * @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 data header can be found in TDX
> + * Guest-Host Communication Interface (GHCI) for Intel TDX 1.0,
> + * section titled "TDG.VP.VMCALL<GetQuote>"
> + */
> +struct tdx_quote_hdr {
> + __u64 version;
> + __u64 status;
> + __u32 in_len;
> + __u32 out_len;
> + __u64 data[];
> +};
> +
> +/* struct tdx_quote_req: Request struct for TDX_CMD_GET_QUOTE IOCTL.
> + * @buf: Address of user buffer that includes TDREPORT. Upon successful
> + * completion of IOCTL, output is copied back to the same buffer.
> + * @len: Length of the Quote buffer.
> + */
> +struct tdx_quote_req {
> + __u64 buf;
> + __u64 len;
> +};
> +
> /*
> * TDX_CMD_GET_REPORT0 - Get TDREPORT0 (a.k.a. TDREPORT subtype 0) using
> * TDCALL[TDG.MR.REPORT]
> @@ -39,4 +74,12 @@ struct tdx_report_req {
> */
> #define TDX_CMD_GET_REPORT0 _IOWR('T', 1, struct tdx_report_req)
>
> +/*
> + * TDX_CMD_GET_QUOTE - Get TD Guest Quote from QE/QGS using GetQuote
> + * TDVMCALL.
> + *
> + * Returns 0 on success or standard errno on other failures.
> + */
> +#define TDX_CMD_GET_QUOTE _IOR('T', 2, struct tdx_quote_req)
> +
> #endif /* _UAPI_LINUX_TDX_GUEST_H_ */
> --
> 2.34.1
>
--
-Dionna Glaze, PhD (she/her)
