Re: [PATCH v6 1/5] x86/tdx: Add TDX Guest attestation interface driver

[Kai Huang <kai.huang@intel.com>]

On Tue, 2022-05-17 at 07:54 -0700, Sathyanarayanan Kuppuswamy wrote:
> > > +struct tdx_report_req {
> > > +	union {
> > > +		__u8 reportdata[TDX_REPORTDATA_LEN];
> > > +		__u8 tdreport[TDX_REPORT_LEN];
> > > +	};
> > > +};
> > 
> > As a userspace ABI, one concern is this doesn't provide any space for future
> > extension.  But probably it's OK since I don't see any possible additional
> > input
> > for now.  And although TDREPORT may have additional information in future
> > generation of TDX but the spec says the size is 1024 so perhaps this won't
> > change even in the future.
> > 
> > Anyway will leave to others.
> 
> IMO, if the spec changes in future we can revisit it.

I don't think the problem is how to revisit _this_ ABI.  The problem is, once it
is introduced, you cannot break the ABI for the compatibility of supporting the
userspace software written for old platforms.  So basically you cannot just
increase the TDX_REPORT_LEN to a larger value.  This means if we have a larger
than 1024B TDREPORT in future, the old userspace TD attestation software which
uses this ABI will not work anymore on the new platforms.

If we need to make sure this ABI work for _ANY_ TDX platforms, I think we either
need to make sure TDREPORT will always be 1024B for _ANY_ TDX platforms, or we
need to have a flexible ABI which doesn't assume TDREPORT size.

For instance, we might need another IOCTL (or other interfaces such as /sysfs)
to query the TDREPORT size, and make this IOCTL like below:

	struct tdx_report_req {
		__u8 reportdata[TDX_REPORTDATA_LEN];
		__u8 reserved[...];
		__u8 tdreport[0];
	};

The actual TDREPORT buffer size is allocated by userspace after it queries the
TDREPORT size.

-- 
Thanks,
-Kai