Re: [PATCH 0/4] tsm: Unified Measurement Register ABI for TVMs

["Xing, Cedric" <cedric.xing@intel.com>]

On 2/19/2025 5:31 AM, Huang, Kai wrote:
> On Tue, 2025-02-18 at 22:04 -0600, Xing, Cedric wrote:
>> On 2/18/2025 8:49 AM, Mikko Ylinen wrote:
>>> On Thu, Feb 13, 2025 at 03:50:19PM -0600, Xing, Cedric wrote:
>>>> On 2/13/2025 10:58 AM, Dave Hansen wrote:
>>>>> On 2/13/25 08:21, Xing, Cedric wrote:
>>>>>> On 2/12/2025 10:50 PM, Dave Hansen wrote:
>>>>>>> On 2/12/25 18:23, Cedric Xing wrote:
>>>>>>>> NOTE: This patch series introduces the Measurement Register (MR) ABI,
>>>>>>>> and
>>>>>>>> is a continuation of the RFC series on the same topic [1].
>>>>>>>
>>>>>>> Could you please explain how the benefits of this series are helpful to
>>>>>>> end users?
>>>>>>
>>>>>> This series exposes MRs as sysfs attributes, allowing end users to
>>>>>> access them effortlessly without needing to write any code. This
>>>>>> simplifies the process of debugging and diagnosing measurement-related
>>>>>> issues. Additionally, it makes the CC architecture more intuitive for
>>>>>> newcomers.
>>>>>
>>>>> Wait a sec, so there's already ABI for manipulating these? This just
>>>>> adds a parallel sysfs interface to the existing ABI?
>>>>>
>>>> No, this is new. There's no existing ABI for accessing measurement registers
>>>> from within a TVM (TEE VM). Currently, on TDX for example, reading TDX
>>>> measurement registers (MRs) must be done by getting a TD quote. And there's
>>>> no way to extend any RTMRs. Therefore, it would be much easier end users to
>>>
>>> TD reports *are* available through the tdx_guest ioctl so there's overlap
>>> with the suggested reportdata/report0 entries at least. Since configfs-tsm
>>> provides the generic transport for TVM reports, the best option to make report0
>>> available is through configfs-tsm reports.
>>>
>> Given the purpose of TSM, I once thought this TDX_CMD_GET_REPORT0 ioctl
>> of /dev/tdx_guest had been deprecated but I was wrong.
>>
>> However, unlocked_ioctl is the only fops remaining on /dev/tdx_guest and
>> TDX_CMD_GET_REPORT0 is the only command supported. It might soon be time
>> to deprecate this interface.
> 
> I agree.
> 
>>
>>> The use case on MRCONFIGID mentioned later in this thread does not depend
>>> on this series. It's easy for the user-space to interprete the full report
>>> to find MRCONFIGID or any other register value (the same is true for HOSTDATA
>>> on SNP).
>>>
>> Yes, parsing the full report will always be an option. But reading
>> static MRs like MRCONDFIGID or HOSTDATA from sysfs attributes will be
>> way more convenient.
> 
> But, theoretically, you cannot really trust what your read from the kernel for
> such *single field*, because to truly get verified you will need to get the full
> report anyway.
> 
Not exactly. Whatever the kernel extracts from a report deemed 
trustworthy by the kernel itself is implicitly trusted by any 
application having the kernel in its TCB. And in fact, every application 
has the kernel in its TCB. Therefore, MRCONFIGID or HOSTDATA read from 
sysfs can be trusted/used directly by any applications.

>>
>> Additionally, this sysfs interface is more friendly to newcomers, as
>> everyone can tell what MRs are available from the directory tree
>> structure, rather than studying processor manuals.
>>
>>> The question here is whether there's any real benefit for the kernel to
>>> expose the provider specific report details through sysfs or could we focus on
>>> the RTMR extend capability only.
>>>
>> Again, parsing the full report is always an alternative for reading any
>> MRs from the underlying arch. But it's much more convenient to read them
>> from files, which I believe is a REAL benefit.
>>
>> Or can we flip the question around and ask: is there any real benefit
>> NOT to allow reading MRs as files and force users and applications to go
>> through an arch specific IOCTL interface?
>>
> 
> As above, I am not convinced that *reading* MRs alone is that useful.  What you
> need is a unified way to *extend* those MRs.
> 
See my response above.

> And yeah I agree extending arch-specific IOCTL to support extending any runtime
> MR isn't a good idea.
>