Message-ID: <9784e70b-b6c7-44d0-99ef-2c07bf020848@intel.com>
Date: Wed, 19 Feb 2025 23:07:14 -0600
From: "Xing, Cedric" <cedric.xing@...el.com>
To: Mikko Ylinen <mikko.ylinen@...ux.intel.com>
CC: Dave Hansen <dave.hansen@...el.com>, Dan Williams
	<dan.j.williams@...el.com>, "Kirill A. Shutemov"
	<kirill.shutemov@...ux.intel.com>, Dave Hansen <dave.hansen@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
	Borislav Petkov <bp@...en8.de>, <x86@...nel.org>, "H. Peter Anvin"
	<hpa@...or.com>, <linux-kernel@...r.kernel.org>,
	<linux-coco@...ts.linux.dev>, Kuppuswamy Sathyanarayanan
	<sathyanarayanan.kuppuswamy@...ux.intel.com>
Subject: Re: [PATCH 0/4] tsm: Unified Measurement Register ABI for TVMs

On 2/19/2025 8:03 AM, Mikko Ylinen wrote:
> On Tue, Feb 18, 2025 at 10:04:19PM -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.
> 
> Once an alternative is available but it's still in use because of this
> use case (i.e., read registers from a TD report). AFAUI, SEV has its
> reports available through configfs-tsm reports so it'd be a good fit here too.
> 
I think every CC arch should have a presence in configfs-tsm for 
generating remotely verifiable reports. This patch series offers 
read/extend functionality to MRs. The overlap here is that any reports 
should include the values of all MRs. However, obtaining a report may 
not be the only way to read MRs. For example, TPM supports commands for 
reading PCRs without attesting to their values. The read functionality 
is definitely a convenience to applications, and helps performance, and 
can also help educating developers. The configfs-tsm report interface 
will work for sure but I don't how it could be as a good fit as this 
sysfs interface.

> Obviously, if the registers get exposed through this series, the use case
> can be covered but full TD report is still good to keep available.
> 
With this series, the full TD report can be requested by writing to 
`reportdata` followed by reading from `report0`.

>>
>>> 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.
>>
>> 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?
> 
> FWIW, I'm not thinking about IOCTLs here but configfs-tsm reports: a
> single read gives you all registers as specified by the report without
> having to add anything to the kernel ABI.
> 
Guess through configfs-tsm you'd have to select the service provider 
first. In contrast, it'd take only a single read from `report0` to get 
the full report through this series. Moreover, it is table driven, so 
these `report0`/`reportdata` attributes add ZERO code and take only 2 
table entries (<100 bytes) to implement on TDX. I'm not sure if adding a 
local report provider to configfs-tsm can be any simpler.

Powered by blists - more mailing lists