NAK: [SRU][K][PATCH 0/1] new TDX attestation driver from Intel

[Tim Gardner]

On 3/6/23 2:20 AM, Andrea Righi wrote:
> BugLink: https://bugs.launchpad.net/bugs/2009437
> 
> [Impact]
> 
> TDX guest attestation has been merged as SAUCE patches in the kinetic
> kernel with the following commits:
> 
> https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/kinetic/commit/?h=master-next&id=285d6d8136ebadcee7fd6452b9e4223996a2a0af
> https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/kinetic/commit/?h=master-next&id=0b78a71c7d7630ab7c3c8a03cbe4f78f1361fb45
> 
> However, Intel released a new TDX attestation driver that will be
> submitted upstream. We should align with the new version that will
> likely end upstream.
> 
> See also LP: #1971027
> 
> [Test case]
> 
> Testing this feature requires a special hardware in the host, special
> firmware and special configuration of a guest.
> 
> Right now it can only be tested by Intel.
> 
> [Fix]
> 
> Apply the new driver provided by Intel in LP: #1971027.
> 
> [Regression potential]
> 
> The new driver can potentially break user-space applications that are
> relying on the TDX attestation feature. This is because of this struct
> (used in the user-space/kernel communication, via ioctl):
> 
> + * Used in TDX_CMD_GET_REPORT IOCTL request.
> + */
> +struct tdx_report_req {
> + __u8 subtype;
> + __u64 reportdata;
> + __u32 rpd_len;
> + __u64 tdreport;
> + __u32 tdr_len;
> +};
> 
> The new patch changed the struct as following:
> 
> +struct tdx_report_req {
> + __u8 reportdata[TDX_REPORTDATA_LEN];
> + __u8 tdreport[TDX_REPORT_LEN];
> +};
> 
> In general we should never apply changes that are breaking user-space
> like this (especially for non-devel kernels), but realistically we can
> probably say that nobody is using this feature yet, so nobody has any
> user-space program that is relying on the old struct (and if they do,
> they're probably in touch with Intel, so they're aware of this change).
> 
> In conclusion, this change should be considered pretty safe, despite the
> potential user-space brekage.
> 
> ----------------------------------------------------------------
> Andrea Righi (3):
>        Revert "UBUNTU: SAUCE: selftests: tdx: Test GetReport TDX attestation feature"
>        Revert "UBUNTU: SAUCE: x86/tdx: Add TDX Guest attestation interface driver"
>        UBUNTU: [Config] enable TDX attestation driver as module by default
> 
> Kuppuswamy Sathyanarayanan (3):
>        UBUNTU: SAUCE: x86/tdx: Add a wrapper to get TDREPORT0 from the TDX Module
>        UBUNTU: SAUCE: virt: Add TDX guest driver
>        UBUNTU: SAUCE: selftests/tdx: Test TDX attestation GetReport support
> 
>   Documentation/virt/coco/tdx-guest.rst         |  52 ++++++++
>   Documentation/virt/index.rst                  |   1 +
>   Documentation/x86/tdx.rst                     |  43 +++++++
>   arch/x86/coco/tdx/tdx.c                       | 151 ++++++------------------
>   arch/x86/include/asm/tdx.h                    |   2 +
>   arch/x86/include/uapi/asm/tdx.h               |  51 --------
>   debian.master/config/annotations              |   3 +
>   debian.master/config/config.common.ubuntu     |   1 +
>   drivers/virt/Kconfig                          |   2 +
>   drivers/virt/Makefile                         |   1 +
>   drivers/virt/coco/tdx-guest/Kconfig           |  10 ++
>   drivers/virt/coco/tdx-guest/Makefile          |   2 +
>   drivers/virt/coco/tdx-guest/tdx-guest.c       | 102 ++++++++++++++++
>   include/uapi/linux/tdx-guest.h                |  42 +++++++
>   tools/arch/x86/include/uapi/asm/tdx.h         |  51 --------
>   tools/testing/selftests/tdx/Makefile          |   8 +-
>   tools/testing/selftests/tdx/config            |   2 +-
>   tools/testing/selftests/tdx/tdx_attest_test.c | 156 ------------------------
>   tools/testing/selftests/tdx/tdx_guest_test.c  | 163 ++++++++++++++++++++++++++
>   19 files changed, 464 insertions(+), 379 deletions(-)
>   create mode 100644 Documentation/virt/coco/tdx-guest.rst
>   delete mode 100644 arch/x86/include/uapi/asm/tdx.h
>   create mode 100644 drivers/virt/coco/tdx-guest/Kconfig
>   create mode 100644 drivers/virt/coco/tdx-guest/Makefile
>   create mode 100644 drivers/virt/coco/tdx-guest/tdx-guest.c
>   create mode 100644 include/uapi/linux/tdx-guest.h
>   delete mode 100644 tools/arch/x86/include/uapi/asm/tdx.h
>   delete mode 100644 tools/testing/selftests/tdx/tdx_attest_test.c
>   create mode 100644 tools/testing/selftests/tdx/tdx_guest_test.c
> 
> 

Patches 3-5 are upstream. Shouldn't they be cherry picks (or backports) ?
-- 
-----------
Tim Gardner
Canonical, Inc