Re: [PATCH v7 0/5] Add TDX Guest Attestation support

[Sathyanarayanan Kuppuswamy]

Hi Dave/Boris/Thomas,

On 5/23/22 9:05 PM, Kuppuswamy Sathyanarayanan wrote:
> Hi All,

Gentle ping!

Can you please let me know your comments on this patch set? This series
is so far reviewed by Kai, Wander, and Isaku. I have addressed all the
comments raised by them. So to progress further, your comments would be
appreciated.

> 
> Intel's Trust Domain Extensions (TDX) protect guest VMs from malicious
> hosts and some physical attacks. VM guest with TDX support is called
> as TD Guest.
> 
> In TD Guest, the attestation process is used to verify the 
> trustworthiness of TD guest to the 3rd party servers. Such attestation
> process is required by 3rd party servers before sending sensitive
> information to TD guests. One usage example is to get encryption keys
> from the key server for mounting the encrypted rootfs or secondary drive.
>     
> Following patches add the attestation support to TDX guest which
> includes attestation user interface driver and related hypercall support.
> 
> Any distribution enabling TDX is also expected to need attestation. So
> enable it by default with TDX guest support. The compiled size is
> quite small (~500 bytes).
> 
> Changes since v6:
>  * Fixed race between wait_for_completion_*() and
>    quote_callback_handler() in tdx_get_quote() when user terminates the
>    request.
>  * Fixed commit log and comments.
> 
> Changes since v5:
>  * Added support for parallel GetQuote requests.
>  * Add noalias variants of set_memory_*crypted() functions to
>    changes page attribute without touching direct map.
>  * Made set_memory_*crypted() functions vmalloc address compatible.
>  * Use vmap()/set_memory_*crypted() functions to share/unshare
>    memory without touching the direct map.
>  * Add support to let driver handle the memory cleanup for the
>    early termination of user requests.
>  * Removed unused headers in attest.c
>  * Fixed commit log and comments as per review comments.
> 
> Changes since v4:
>  * Removed platform driver model in attestation driver and used
>    miscdevice and initcall approach.
>  * Since dma_alloc*() APIs require a valid device reference,
>    replaced it with __get_free_pages() and set_memory_decrypted()
>    for quote memory allocation.
>  * Removed tdx_mcall_tdreport() and moved TDG.MR.REPORT TDCALL code
>    to tdx_get_report().
>  * Used kmalloc() for TDREPORT memory allocation instead of
>    get_zeroed_page().
>  * Returned -EINVAL in default case of tdx_attest_ioctl().
>  * Added struct tdx_report_req to explicitly mention the
>    TDX_CMD_GET_REPORT IOCTL argument.
>  * Removed tdx_get_quote_hypercall() and moved hypercall code to
>    attestation driver itself.
>  * Removed GetQuote timeout support (since it is not defined in
>    spec)
>  * Added support to check for spurious callback interrupt in GetQuote
>    request.
>  * Fixed commit log and comments as per review suggestions.
>    
> 
> Changes since v3:
>  * Moved the attestation driver from platform/x86 to arch/x86/coco/tdx/ and
>    renamed intel_tdx_attest.c to attest.c.
>  * Dropped CONFIG_INTEL_TDX_ATTESTATION and added support to compile
>    attestation changes with CONFIG_INTEL_TDX_GUEST option.
>  * Merged patch titled "x86/tdx: Add tdx_mcall_tdreport() API support" and
>    "platform/x86: intel_tdx_attest: Add TDX Guest attestation interface" into
>    a single patch.
>  * Moved GetQuote IOCTL support changes from patch titled "platform/x86:
>    intel_tdx_attest: Add TDX Guest attestation interface driver" to a
>    separate patch.
>  * Removed 8K size restriction when requesting quote, and added support
>    to let userspace decide the quote size.
>  * Added support to allow attestation agent configure quote generation
>    timeout value.
>  * Fixed commit log and comments as per review comments.
> 
> Changes since v2:
>  * As per Han's suggestion, modified the attestation driver to use
>    platform device driver model.
>  * Modified tdx_hcall_get_quote() and tdx_mcall_tdreport() APIs to
>    return TDCALL error code instead of generic error info (like -EIO).
>  * Removed attestation test app patch from this series to simplify
>    the patchset and review process. Test app patches will be submitted
>    once attestation support patches are merged.
>  * Since patches titled "x86/tdx: Add SetupEventNotifyInterrupt TDX
>    hypercall support" and "x86/tdx: Add TDX Guest event notify
>    interrupt vector support" are related, combining them into a
>    single patch.
> 
> Changes since v1:
>  * Moved test driver from "tools/tdx/attest/tdx-attest-test.c" to
>    "tools/arch/x86/tdx/attest/tdx-attest-test.c" as per Hans review
>    suggestion.
>  * Minor commit log and comment fixes in patches titled
>    "x86/tdx: Add tdx_mcall_tdreport() API support" and "x86/tdx:
>    Add tdx_hcall_get_quote() API support"
>  * Extended tdx_hcall_get_quote() API to accept GPA length as argument
>    to accomodate latest TDQUOTE TDVMCALL related specification update.
>  * Added support for tdx_setup_ev_notify_handler() and
>    tdx_remove_ev_notify_handler() in patch titled "x86/tdx: Add TDX
>    Guest event notify interrupt vector support"
> 
> 
> Kuppuswamy Sathyanarayanan (5):
>   x86/tdx: Add TDX Guest attestation interface driver
>   x86/tdx: Add TDX Guest event notify interrupt support
>   x86/mm: Make tdx_enc_status_changed() vmalloc address compatible
>   x86/mm: Add noalias variants of set_memory_*crypted() functions
>   x86/tdx: Add Quote generation support
> 
>  arch/x86/coco/tdx/Makefile         |   2 +-
>  arch/x86/coco/tdx/attest.c         | 432 +++++++++++++++++++++++++++++
>  arch/x86/coco/tdx/tdx.c            |  84 +++++-
>  arch/x86/include/asm/hardirq.h     |   3 +
>  arch/x86/include/asm/idtentry.h    |   4 +
>  arch/x86/include/asm/irq_vectors.h |   7 +-
>  arch/x86/include/asm/set_memory.h  |   2 +
>  arch/x86/include/asm/tdx.h         |   4 +
>  arch/x86/include/uapi/asm/tdx.h    |  87 ++++++
>  arch/x86/kernel/irq.c              |   7 +
>  arch/x86/mm/pat/set_memory.c       |  26 +-
>  11 files changed, 648 insertions(+), 10 deletions(-)
>  create mode 100644 arch/x86/coco/tdx/attest.c
>  create mode 100644 arch/x86/include/uapi/asm/tdx.h
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer