Message-ID: <65cad52616ffa_29b129412@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Date: Mon, 12 Feb 2024 18:34:14 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Tom Lendacky <thomas.lendacky@....com>, Dan Williams
	<dan.j.williams@...el.com>, <linux-kernel@...r.kernel.org>, <x86@...nel.org>
CC: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
	Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...or.com>, Andy Lutomirski <luto@...nel.org>, "Peter
 Zijlstra" <peterz@...radead.org>, Michael Roth <michael.roth@....com>, Ashish
 Kalra <ashish.kalra@....com>
Subject: Re: [PATCH 10/11] x86/sev: Extend the config-fs attestation support
 for an SVSM

Tom Lendacky wrote:
> On 2/2/24 01:10, Dan Williams wrote:
> > Tom Lendacky wrote:
> >> When an SVSM is present, the guest can also request attestation reports
> >> from the SVSM. These SVSM attestation reports can be used to attest the
> >> SVSM and any services running within the SVSM.
> >>
> >> Extend the config-fs attestation support to allow for an SVSM attestation
> >> report. This involves creating four (4) new config-fs attributes:
> >>
> >>    - 'svsm' (input)
> >>      This attribute is used to determine whether the attestation request
> >>      should be sent to the SVSM or to the SEV firmware.
> >>
> >>    - 'service_guid' (input)
> >>      Used for requesting the attestation of a single service within the
> >>      SVSM. A null GUID implies that the SVSM_ATTEST_SERVICES call should
> >>      be used to request the attestation report. A non-null GUID implies
> >>      that the SVSM_ATTEST_SINGLE_SERVICE call should be used.
> >>
> >>    - 'service_version' (input)
> >>      Used with the SVSM_ATTEST_SINGLE_SERVICE call, the service version
> >>      represents a specific service manifest version be used for the
> >>      attestation report.
> >>
> >>    - 'manifestblob' (output)
> >>      Used to return the service manifest associated with the attestation
> >>      report.
> >>
> >> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>
> >> ---
> >>   Documentation/ABI/testing/configfs-tsm  |  55 ++++++++++
> >>   arch/x86/include/asm/sev.h              |  31 +++++-
> >>   arch/x86/kernel/sev.c                   |  50 +++++++++
> >>   drivers/virt/coco/sev-guest/sev-guest.c | 137 ++++++++++++++++++++++++
> >>   drivers/virt/coco/tsm.c                 |  95 +++++++++++++++-
> >>   include/linux/tsm.h                     |  11 ++
> >>   6 files changed, 376 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/ABI/testing/configfs-tsm b/Documentation/ABI/testing/configfs-tsm
> >> index dd24202b5ba5..c5423987d323 100644
> >> --- a/Documentation/ABI/testing/configfs-tsm
> >> +++ b/Documentation/ABI/testing/configfs-tsm
> >> @@ -31,6 +31,21 @@ Description:
> >>   		Standardization v2.03 Section 4.1.8.1 MSG_REPORT_REQ.
> >>   		https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/56421.pdf
> >>   
> >> +What:		/sys/kernel/config/tsm/report/$name/manifestblob
> >> +Date:		January, 2024
> >> +KernelVersion:	v6.9
> >> +Contact:	linux-coco@...ts.linux.dev
> >> +Description:
> >> +		(RO) Optional supplemental data that a TSM may emit, visibility
> >> +		of this attribute depends on TSM, and may be empty if no
> >> +		manifest data is available.
> >> +
> >> +		When @provider is "sev_guest" and the "svsm" attribute is set
> >> +		this file contains the service manifest used for the SVSM
> >> +		attestation report from Secure VM Service Module for SEV-SNP
> >> +		Guests v1.00 Section 7.
> >> +		https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
> > 
> > I wish configfs had better dynamic visibility so that this could be
> > hidden when not active... oh well.
> 
> So do I. I played with the idea of having two different structs and 
> registering one or the other based on whether an SVSM was present. Thoughts?

That's the status quo for the differences between TDX vs SEV
(tsm_report_default_type vs tsm_report_extra_type). I think a
"tsm_report_service_type " can be a new superset of
tsm_report_extra_type. That that just starts to get messy if
implementations start to pick and choose on a finer granularity what
they support. For example, what if TDX supports these new service
attributes, but not privlevel.

It seems straightforward to add an is_visible() callback to
'struct configfs_item_operations'. Then a common superset of all the
attributes could be specified, but with an is_visible() implementation
that consults with data registered with tsm_register() rather than the
@type argument directly.

[..]
> >> +What:		/sys/kernel/config/tsm/report/$name/svsm
> >> +Date:		January, 2024
> >> +KernelVersion:	v6.9
> >> +Contact:	linux-coco@...ts.linux.dev
> >> +Description:
> >> +		(WO) Attribute is visible if a TSM implementation provider
> >> +		supports the concept of attestation reports for TVMs running
> >> +		under an SVSM, like SEV-SNP. Specifying any non-zero value
> > 
> > Just use kstrtobool just to have a bit more form to it, and who knows
> > maybe keeping some non-zero numbers reserved turns out useful someday.
> > 
> > ...or see below, maybe this shouldn't be an "svsm" flag.
> > 
> >> +		implies that the attestation report should come from the SVSM.
> >> +		Secure VM Service Module for SEV-SNP Guests v1.00 Section 7.
> >> +		https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
> > 
> > So this affects the output format of outblob? I think @outblob should
> > probably document the fact that it depends on @provider, @privlevel, and
> > now @svsm. Probably all of the format links should live under @outblob
> > not @provider.
> 
> It doesn't change the output format, it is still a standard SNP 
> attestation report. What changes is that a SHA-512 hash of the nonce and 
> the manifest are taken and passed as report data as opposed to just the 
> nonce value.

If it is the same format, and the input is user controlled then I am
confused what the new ABI is selecting? Could it not be inferred by
privlevel?

[..]
> >> +
> >> +What:		/sys/kernel/config/tsm/report/$name/service_version
> >> +Date:		January, 2024
> >> +KernelVersion:	v6.9
> >> +Contact:	linux-coco@...ts.linux.dev
> >> +Description:
> >> +		(WO) Attribute is visible if a TSM implementation provider
> >> +		supports the concept of attestation reports for TVMs running
> >> +		under an SVSM, like SEV-SNP. Indicates the service manifest
> >> +		version requested for the attestation report.
> >> +		Secure VM Service Module for SEV-SNP Guests v1.00 Section 7.
> >> +		https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
> > 
> > Perhaps document that version 1 is assumed and is always compatible?
> 
> Can do.
> 
> > 
> > What prompts new versions and how does that discovered by guest software?
> 
> New versions will depend on the service that is running. Changes or 
> updates to that service would document the new versions manifest output.
> There isn't currently a discoverable way other than calling with the 
> requested version and seeing if an error is returned.
> 
> A possible extension to the SVSM attestation protocol could create a 
> version query call.

Can the version be made to not matter, or be inferred by the presence of
a new enumerated service capability? For example, make the same compat
guarantees that ACPI methods do between versions where extensions are
optional and software can always use v1 without breaking? Otherwise, I
am not grokking what software should do with this version. 

Separately, is this a version for the service protocol or a version of
the manifest format? The description makes it sound like the latter, but
the "service_version" name makes it sound like the former.

Powered by blists - more mailing lists