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?
