From: Dan Williams <dan.j.williams@intel.com>
To: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>,
Dan Williams <dan.j.williams@intel.com>,
<linux-coco@lists.linux.dev>
Cc: Borislav Petkov <bp@alien8.de>,
Tom Lendacky <thomas.lendacky@amd.com>,
Dionna Glaze <dionnaglaze@google.com>,
Brijesh Singh <brijesh.singh@amd.com>, <peterz@infradead.org>,
<linux-kernel@vger.kernel.org>, <tglx@linutronix.de>
Subject: Re: [PATCH v3 5/5] virt: sevguest: Add TSM_REPORTS support for SNP_{GET, GET_EXT}_REPORT
Date: Fri, 1 Sep 2023 09:38:11 -0700 [thread overview]
Message-ID: <64f2137362305_4c1f3294f4@dwillia2-mobl3.amr.corp.intel.com.notmuch> (raw)
In-Reply-To: <4e4407b0-179d-4be4-bf0b-0d674103d4f6@linux.microsoft.com>
Jeremi Piotrowski wrote:
> On 8/30/2023 9:33 PM, Dan Williams wrote:
> > The sevguest driver was a first mover in the confidential computing
> > space. As a first mover that afforded some leeway to build the driver
> > without concern for common infrastructure.
> >
> > Now that sevguest is no longer a singleton [1] the common operation of
> > building and transmitting attestation report blobs can / should be made
> > common. In this model the so called "TSM-provider" implementations can
> > share a common envelope ABI even if the contents of that envelope remain
> > vendor-specific. When / if the industry agrees on an attestation record
> > format, that definition can also fit in the same ABI. In the meantime
> > the kernel's maintenance burden is reduced and collaboration on the
> > commons is increased.
> >
> > Convert sevguest to use CONFIG_TSM_REPORTS to retrieve the blobs that
> > the SNP_{GET,GET_EXT}_REPORT ioctls produce. An example flow follows for
> > retrieving the SNP_GET_REPORT blob via the TSM interface utility,
> > assuming no nonce and VMPL==2:
> >
> > report=/sys/kernel/config/tsm/report/report0
> > mkdir $report
> > echo 2 > $report/privlevel
> > dd if=/dev/urandom bs=64 count=1 > $report/inblob
> > hexdump -C $report/outblob
> > rmdir $report
> >
> > ...while the SNP_GET_EXT_REPORT flow needs to additionally set the
> > format to "extended":
> >
> > report=/sys/kernel/config/tsm/report/report1
> > mkdir $report
> > echo extended > $report/format
> > dd if=/dev/urandom bs=64 count=1 > $report/inblob
> > hexdump -C $report/outblob
> > rmdir $report
> >
> > The old ioctls can be lazily deprecated, the main motivation of this
> > effort is to stop the proliferation of new ioctls, and to increase
> > cross-vendor collaboration.
> >
> > Note, only compile-tested.
> >
> > Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Cc: Dionna Glaze <dionnaglaze@google.com>
> > Cc: Brijesh Singh <brijesh.singh@amd.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> > drivers/virt/coco/sev-guest/Kconfig | 1
> > drivers/virt/coco/sev-guest/sev-guest.c | 83 +++++++++++++++++++++++++++++++
> > 2 files changed, 84 insertions(+)
> >
> > diff --git a/drivers/virt/coco/sev-guest/Kconfig b/drivers/virt/coco/sev-guest/Kconfig
> > index da2d7ca531f0..1cffc72c41cb 100644
> > --- a/drivers/virt/coco/sev-guest/Kconfig
> > +++ b/drivers/virt/coco/sev-guest/Kconfig
> > @@ -5,6 +5,7 @@ config SEV_GUEST
> > select CRYPTO
> > select CRYPTO_AEAD2
> > select CRYPTO_GCM
> > + select TSM_REPORTS
> > help
> > SEV-SNP firmware provides the guest a mechanism to communicate with
> > the PSP without risk from a malicious hypervisor who wishes to read,
> > diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> > index c3c9e9ea691f..c7bbb8f372a3 100644
> > --- a/drivers/virt/coco/sev-guest/sev-guest.c
> > +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> > @@ -16,10 +16,12 @@
> > #include <linux/miscdevice.h>
> > #include <linux/set_memory.h>
> > #include <linux/fs.h>
> > +#include <linux/tsm.h>
> > #include <crypto/aead.h>
> > #include <linux/scatterlist.h>
> > #include <linux/psp-sev.h>
> > #include <linux/sockptr.h>
> > +#include <linux/cleanup.h>
> > #include <uapi/linux/sev-guest.h>
> > #include <uapi/linux/psp-sev.h>
> >
> > @@ -759,6 +761,79 @@ static u8 *get_vmpck(int id, struct snp_secrets_page_layout *layout, u32 **seqno
> > return key;
> > }
> >
> > +static u8 *sev_report_new(const struct tsm_desc *desc, void *data, size_t *outblob_len)
> > +{
> > + struct snp_guest_dev *snp_dev = data;
> > + const int report_size = SZ_4K;
> > + const int ext_size = SZ_16K;
> > + int ret, size;
> > +
> > + if (desc->inblob_len != 64)
> > + return ERR_PTR(-EINVAL);
> > +
> > + if (desc->outblob_format == TSM_FORMAT_EXTENDED)
> > + size = report_size + ext_size;
> > + else
> > + size = report_size;
> > +
> > + u8 *buf __free(kvfree) = kvzalloc(size, GFP_KERNEL);
> > +
> > + guard(mutex)(&snp_cmd_mutex);
> > + if (desc->outblob_format == TSM_FORMAT_EXTENDED) {
> > + struct snp_ext_report_req ext_req = {
> > + .data = { .vmpl = desc->privlevel },
> > + .certs_address = (__u64)buf + report_size,
> > + .certs_len = ext_size,
> > + };
> > + memcpy(&ext_req.data.user_data, desc->inblob, desc->inblob_len);
> > +
> > + struct snp_guest_request_ioctl input = {
> > + .msg_version = 1,
> > + .req_data = (__u64)&ext_req,
> > + .resp_data = (__u64)buf,
> > + };
> > + struct snp_req_resp io = {
> > + .req_data = KERNEL_SOCKPTR(&ext_req),
> > + .resp_data = KERNEL_SOCKPTR(buf),
> > + };
> > +
> > + ret = get_ext_report(snp_dev, &input, &io);
> > + } else {
> > + struct snp_report_req req = {
> > + .vmpl = desc->privlevel,
> > + };
> > + memcpy(&req.user_data, desc->inblob, desc->inblob_len);
> > +
> > + struct snp_guest_request_ioctl input = {
> > + .msg_version = 1,
> > + .req_data = (__u64)&req,
> > + .resp_data = (__u64)buf,
> > + };
> > + struct snp_req_resp io = {
> > + .req_data = KERNEL_SOCKPTR(&req),
> > + .resp_data = KERNEL_SOCKPTR(buf),
> > + };
> > +
> > + ret = get_report(snp_dev, &input, &io);
> > + }
> > +
> > + if (ret)
> > + return ERR_PTR(ret);
> > +
> > + *outblob_len = size;
> > + return_ptr(buf);
> > +}
> > +
> > +static const struct tsm_ops sev_tsm_ops = {
> > + .name = KBUILD_MODNAME,
> > + .report_new = sev_report_new,
> > +};
> > +
> > +static void unregister_sev_tsm(void *data)
> > +{
> > + unregister_tsm(&sev_tsm_ops);
> > +}
> > +
> > static int __init sev_guest_probe(struct platform_device *pdev)
> > {
> > struct snp_secrets_page_layout *layout;
> > @@ -832,6 +907,14 @@ static int __init sev_guest_probe(struct platform_device *pdev)
> > snp_dev->input.resp_gpa = __pa(snp_dev->response);
> > snp_dev->input.data_gpa = __pa(snp_dev->certs_data);
> >
> > + ret = register_tsm(&sev_tsm_ops, snp_dev, &tsm_report_ext_type);
> > + if (ret)
> > + goto e_free_cert_data;
> > +
> > + ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
> > + if (ret)
> > + goto e_free_cert_data;
> > +
> > ret = misc_register(misc);
> > if (ret)
> > goto e_free_cert_data;
> >
>
> I tried this with the non-extended request
Thanks for testing!
> ...and realized it's a bit awkward from
> a uapi point of view. The returned outblob has a header prepended (table 23 in [1])
> and is arbitrarily sized at 4096. It would be more natural to only return the report
> field and the count bytes that the report actually has. I've attached a rough patch
> below to give you an idea of what I mean.
It makes sense, especially if that is what the legacy ioctl output
buffer is emitting.
> The extended guest request is another topic, since userspace has to be aware of
> where the kernel choses to put the extended data, and fixup all the offsets in the
> table (section 4.1.8.1 in [2]). It would be better to return this data through a
> separate file.
I notice that the TDX report also includes a certificate blob, so if
that is a common concept then yes, it makes sense to have a separate
file for that.
next prev parent reply other threads:[~2023-09-01 16:38 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-30 19:33 [PATCH v3 0/5] configfs-tsm: Attestation Report ABI Dan Williams
2023-08-30 19:33 ` [PATCH v3 1/5] virt: coco: Add a coco/Makefile and coco/Kconfig Dan Williams
2023-08-30 20:48 ` Kuppuswamy Sathyanarayanan
2023-08-30 19:33 ` [PATCH v3 2/5] configfs-tsm: Introduce a shared ABI for attestation reports Dan Williams
2023-08-30 20:45 ` Greg Kroah-Hartman
2023-08-30 20:57 ` Dan Williams
2023-08-30 22:08 ` Kuppuswamy Sathyanarayanan
2023-08-31 1:24 ` Dan Williams
2023-08-31 21:42 ` Dionna Amalie Glaze
2023-08-31 22:13 ` Dan Williams
2023-09-01 18:06 ` Thomas Gleixner
2023-09-01 18:47 ` Dan Williams
2023-09-01 19:06 ` Thomas Gleixner
2023-08-30 19:33 ` [PATCH v3 3/5] virt: sevguest: Prep for kernel internal {get, get_ext}_report() Dan Williams
2023-08-30 19:33 ` [PATCH v3 4/5] mm/slab: Add __free() support for kvfree Dan Williams
2023-08-30 20:46 ` Greg Kroah-Hartman
2023-09-07 8:59 ` Gupta, Pankaj
2023-08-30 19:33 ` [PATCH v3 5/5] virt: sevguest: Add TSM_REPORTS support for SNP_{GET, GET_EXT}_REPORT Dan Williams
2023-09-01 15:25 ` Jeremi Piotrowski
2023-09-01 16:38 ` Dan Williams [this message]
2023-09-04 2:14 ` Huang, Kai
2023-09-04 2:57 ` Kuppuswamy Sathyanarayanan
2023-09-05 23:21 ` Huang, Kai
2023-09-01 16:04 ` [PATCH v3 0/5] configfs-tsm: Attestation Report ABI Jeremi Piotrowski
2023-09-01 16:51 ` Dan Williams
2023-09-07 8:04 ` Samuel Ortiz
2023-09-25 19:26 ` Dan Williams
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=64f2137362305_4c1f3294f4@dwillia2-mobl3.amr.corp.intel.com.notmuch \
--to=dan.j.williams@intel.com \
--cc=bp@alien8.de \
--cc=brijesh.singh@amd.com \
--cc=dionnaglaze@google.com \
--cc=jpiotrowski@linux.microsoft.com \
--cc=linux-coco@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=thomas.lendacky@amd.com \
/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.