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

[Andrea Righi]

On Mon, Mar 06, 2023 at 07:41:20AM -0700, Tim Gardner wrote:
> 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) ?

Definitely. Will send a v2 with the proper cherry pick / backport line.

Thanks,
-Andrea