On 11/14/22 16:33, Sathyanarayanan Kuppuswamy wrote:
On 11/11/22 10:35 AM, Dave Hansen wrote:
This is *NOT* "a wrapper to get TDREPORT from the TDX Module", this is at best "a wrapper to get TDREPORT sub type 0 from the TDX Module".
In both the commit log and the comments, I can highlight the "subtype 0" information. Will that work for you, or do you prefer that this wrapper take the "subtype" option as argument and we pass 0 for the subtype value from the TDX guest driver?
I actually think it's a *lot* more clear if the User<->Kernel ABI just takes the subtype. But, I also heard Greg's concerns about making the ABI _too_ open-ended.
So, I really don't care. Just make it clear that, as is, this ABI is not the "TDREPORT ABI".
It also occurs to me that "sub type 0" could use an actual name. Could we give it one, please?
Although the subtype option is mentioned in the TDX Module spec, it is not currently used (it expects this value to be zero), and the spec also does not explain why this option is required. According to TDX architects, this option was primarily added to handle any future requirements that may arise that require additional information to be added to the TDREPORT. However, they do not currently have any valid use cases for it. So the current version can only be described as "Type-0." Once a new use case for Subtype 1 is defined, we may be able to come up with a suitable name. Are you okay with calling it "Type-0" for the time being?
That sounds like a cop out to me. I'd really appreciate some effort on your part to look deeply into the problem.
The blob that the kernel is passing back and forth here _has_ content. I guess it's somewhat hard to name because it's got a bunch of inputs (ATTRIBUTES, XFAM, MRTD, MRCONFIGID, MROWNER, MROWNERCONFIG and RTMRs) and a fixed hash algorithm (SHA-384).
Any time that those inputs change or, for instance, the hash algorithm changes, it would need a new subtype. Right?
I guess we can't call "subtype 0" TDREPORT_SHA384 because "subtype 1" might still use SHA-384, but have the set of inputs change.
But, it'll also get maddeningly inconsistent if we have a "TDREPORT" ioctl() that does "subtype 0" and "TDREPORT1" that does "subtype 1".
So, let's at *least* call this thing "TDREPORT0" in the ABI, along with a description of why we're numbering it that way as opposed to taking 'subtype' as a numeric ioctl() argument.
Any better ideas?