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

[Huang, Kai]

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.

> 
> 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.

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