All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Xing, Cedric" <cedric.xing@intel.com>
To: Mikko Ylinen <mikko.ylinen@linux.intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	<x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	<linux-kernel@vger.kernel.org>, <linux-coco@lists.linux.dev>,
	Kuppuswamy Sathyanarayanan
	<sathyanarayanan.kuppuswamy@linux.intel.com>
Subject: Re: [PATCH 0/4] tsm: Unified Measurement Register ABI for TVMs
Date: Wed, 19 Feb 2025 23:07:14 -0600	[thread overview]
Message-ID: <9784e70b-b6c7-44d0-99ef-2c07bf020848@intel.com> (raw)
In-Reply-To: <Z7XkoQ1cJNw8dn3I@himmelriiki>

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.

  reply	other threads:[~2025-02-20  5:07 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-13  2:23 [PATCH 0/4] tsm: Unified Measurement Register ABI for TVMs Cedric Xing
2025-02-13  2:23 ` [PATCH 1/4] tsm: Add TVM Measurement Register support Cedric Xing
2025-02-14  0:55   ` kernel test robot
2025-02-17  0:17   ` Huang, Kai
2025-02-17 10:44     ` Huang, Kai
2025-02-17 20:57     ` Xing, Cedric
2025-02-18  9:14       ` Huang, Kai
2025-02-18 18:13         ` Xing, Cedric
2025-02-18  1:10   ` Sathyanarayanan Kuppuswamy
2025-02-20  1:01     ` Xing, Cedric
2025-02-13  2:23 ` [PATCH 2/4] tsm: Add TSM measurement sample code Cedric Xing
2025-02-13  2:23 ` [PATCH 3/4] x86/tdx: Add tdx_mcall_rtmr_extend() interface Cedric Xing
2025-02-17  0:40   ` Huang, Kai
2025-02-17 20:58     ` Xing, Cedric
2025-02-17 21:39       ` Sathyanarayanan Kuppuswamy
2025-02-13  2:23 ` [PATCH 4/4] x86/tdx: Expose TDX MRs through TSM sysfs interface Cedric Xing
2025-02-13  4:50 ` [PATCH 0/4] tsm: Unified Measurement Register ABI for TVMs Dave Hansen
2025-02-13 16:21   ` Xing, Cedric
2025-02-13 16:58     ` Dave Hansen
2025-02-13 21:50       ` Xing, Cedric
2025-02-13 23:19         ` Dave Hansen
2025-02-14 16:19           ` Xing, Cedric
2025-02-14 16:26             ` Dave Hansen
2025-02-14 21:59               ` Xing, Cedric
2025-02-18 16:25                 ` Dan Middleton
2025-02-18 16:57                   ` Dave Hansen
2025-02-18 23:57                     ` Dionna Amalie Glaze
2025-02-19  0:41                       ` Dave Hansen
2025-02-19  3:21                         ` Dionna Amalie Glaze
2025-02-19 13:29                           ` James Bottomley
2025-02-19 15:24                             ` Dan Middleton
2025-02-19 20:53                               ` James Bottomley
2025-02-19 22:25                                 ` Xing, Cedric
2025-02-19 23:02                                 ` Dan Williams
2025-05-02  1:45                       ` Dan Williams
2025-02-18 14:49         ` Mikko Ylinen
2025-02-19  4:04           ` Xing, Cedric
2025-02-19 11:31             ` Huang, Kai
2025-02-20  4:37               ` Xing, Cedric
2025-02-19 14:03             ` Mikko Ylinen
2025-02-20  5:07               ` Xing, Cedric [this message]
2025-02-18  1:10 ` Sathyanarayanan Kuppuswamy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9784e70b-b6c7-44d0-99ef-2c07bf020848@intel.com \
    --to=cedric.xing@intel.com \
    --cc=bp@alien8.de \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-coco@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikko.ylinen@linux.intel.com \
    --cc=mingo@redhat.com \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.