From: Xiaoyao Li <xiaoyao.li@intel.com>
To: Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@linux.intel.com>,
Dan Williams <dan.j.williams@intel.com>,
linux-coco@lists.linux.dev
Cc: Erdem Aktas <erdemaktas@google.com>,
Peter Gonda <pgonda@google.com>,
Tom Lendacky <thomas.lendacky@amd.com>,
peterz@infradead.org, dave.hansen@linux.intel.com,
x86@kernel.org
Subject: Re: [PATCH v7 7/7] virt: tdx-guest: Add Quote generation support using TSM_REPORTS
Date: Tue, 9 Jan 2024 10:13:16 +0800 [thread overview]
Message-ID: <1740f18b-715c-4d48-97f5-c486580cc06b@intel.com> (raw)
In-Reply-To: <6bdf569c-684a-4459-af7c-4430691804eb@linux.intel.com>
On 1/8/2024 10:55 AM, Kuppuswamy Sathyanarayanan wrote:
>
>
> On 12/20/2023 5:50 PM, Xiaoyao Li wrote:
>> On 10/20/2023 9:17 AM, Dan Williams wrote:
>>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ...
>>> +static int tdx_report_new(struct tsm_report *report, void *data)
>>> +{
>>> + u8 *buf, *reportdata = NULL, *tdreport = NULL;
>>> + struct tdx_quote_buf *quote_buf = quote_data;
>>> + struct tsm_desc *desc = &report->desc;
>>> + int ret;
>>> + u64 err;
>>> +
>>> + /* TODO: switch to guard(mutex_intr) */
>>> + if (mutex_lock_interruptible("e_lock))
>>> + return -EINTR;
>>> +
>>> + /*
>>> + * If the previous request is timedout or interrupted, and the
>>> + * Quote buf status is still in GET_QUOTE_IN_FLIGHT (owned by
>>> + * VMM), don't permit any new request.
>>> + */
>>> + if (quote_buf->status == GET_QUOTE_IN_FLIGHT) {
>>> + ret = -EBUSY;
>>> + goto done;
>>> + }
>>> +
>>> + if (desc->inblob_len != TDX_REPORTDATA_LEN) {
>>> + ret = -EINVAL;
>>> + goto done;
>>> + }
>>> +
>>> + reportdata = kmalloc(TDX_REPORTDATA_LEN, GFP_KERNEL);
>>> + if (!reportdata) {
>>> + ret = -ENOMEM;
>>> + goto done;
>>> + }
>>> +
>>> + tdreport = kzalloc(TDX_REPORT_LEN, GFP_KERNEL);
>>> + if (!tdreport) {
>>> + ret = -ENOMEM;
>>> + goto done;
>>> + }
>>> +
>>> + memcpy(reportdata, desc->inblob, desc->inblob_len);
>>> +
>>> + /* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */
>>> + ret = tdx_mcall_get_report0(reportdata, tdreport);
>>> + if (ret) {
>>> + pr_err("GetReport call failed\n");
>>> + goto done;
>>> + }
>>> +
>>> + memset(quote_data, 0, GET_QUOTE_BUF_SIZE);
>>> +
>>> + /* Update Quote buffer header */
>>> + quote_buf->version = GET_QUOTE_CMD_VER;
>>> + quote_buf->in_len = TDX_REPORT_LEN;
>>> +
>>> + memcpy(quote_buf->data, tdreport, TDX_REPORT_LEN);
>>> +
>>> + err = tdx_hcall_get_quote(quote_data, GET_QUOTE_BUF_SIZE);
>>> + if (err) {
>>> + pr_err("GetQuote hypercall failed, status:%llx\n", err);
>>> + ret = -EIO;
>>> + goto done;
>>> + }
>>> +
>>> + ret = wait_for_quote_completion(quote_buf, getquote_timeout);
>>> + if (ret) {
>>> + pr_err("GetQuote request timedout\n");
>>> + goto done;
>>> + }
>>
>> Sorry that I didn't check the previous discussion and don't know if it is by design or not:
>>
>> Why don't check the quote_buf->status? If it indicates errors, we should return some error code instead, right?
>
> For the failed request, outblob_len will be zero and the empty output can be
> treated as failed request. But I agree that it makes sense to return error
> for the failed request. I can submit a patch for it. Something like below:
>
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
> @@ -33,6 +33,8 @@
>
> /* TDX GetQuote status codes */
> #define GET_QUOTE_SUCCESS 0
> +#define GET_QUOTE_ERROR 0x8000000000000000
> +#define GET_QUOTE_SERVICE_UNAVAILABLE 0x8000000000000001
they get defined but not used.
> #define GET_QUOTE_IN_FLIGHT 0xffffffffffffffff
>
> /* struct tdx_quote_buf: Format of Quote request buffer.
> @@ -228,6 +230,12 @@ static int tdx_report_new(struct tsm_report *report, void *data)
> goto done;
> }
>
> + if (quote_buf->status != GET_QUOTE_SUCCESS) {
> + pr_err("GetQuote request failed, ret %llx\n", quote_buf->status);
> + ret = -EIO;
> + goto done;
> + }
> +
Besides above, looks good to me.
>
>>
>>> + buf = kvmemdup(quote_buf->data, quote_buf->out_len, GFP_KERNEL);
>>> + if (!buf) {
>>> + ret = -ENOMEM;
>>> + goto done;
>>> + }
>>> +
>>> + report->outblob = buf;
>>> + report->outblob_len = quote_buf->out_len;
>>> +
>>> + /*
>>> + * TODO: parse the PEM-formatted cert chain out of the quote buffer when
>>> + * provided
>>> + */
>>> +done:
>>> + mutex_unlock("e_lock);
>>> + kfree(reportdata);
>>> + kfree(tdreport);
>>> +
>>> + return ret;
>>> +}
>>> +
>>
>
prev parent reply other threads:[~2024-01-09 2:13 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-20 1:16 [PATCH v7 0/7] configfs-tsm: Attestation Report ABI Dan Williams
2023-10-20 1:16 ` [PATCH v7 1/7] virt: sevguest: Fix passing a stack buffer as a scatterlist target Dan Williams
2023-10-20 1:16 ` [PATCH v7 2/7] virt: coco: Add a coco/Makefile and coco/Kconfig Dan Williams
2023-10-20 1:16 ` [PATCH v7 3/7] configfs-tsm: Introduce a shared ABI for attestation reports Dan Williams
2023-10-25 4:14 ` Qinkun Bao
2023-10-25 20:06 ` Dan Williams
2023-10-27 1:30 ` Chong Cai
2023-10-20 1:16 ` [PATCH v7 4/7] virt: sevguest: Prep for kernel internal get_ext_report() Dan Williams
2023-10-20 1:16 ` [PATCH v7 5/7] mm/slab: Add __free() support for kvfree Dan Williams
2023-10-20 1:17 ` [PATCH v7 6/7] virt: sevguest: Add TSM_REPORTS support for SNP_GET_EXT_REPORT Dan Williams
2023-10-20 3:25 ` Alexey Kardashevskiy
2023-10-20 1:17 ` [PATCH v7 7/7] virt: tdx-guest: Add Quote generation support using TSM_REPORTS Dan Williams
2023-10-24 17:08 ` Dan Williams
2023-12-21 1:50 ` Xiaoyao Li
2024-01-08 2:55 ` Kuppuswamy Sathyanarayanan
2024-01-09 2:13 ` Xiaoyao Li [this message]
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=1740f18b-715c-4d48-97f5-c486580cc06b@intel.com \
--to=xiaoyao.li@intel.com \
--cc=dan.j.williams@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=erdemaktas@google.com \
--cc=linux-coco@lists.linux.dev \
--cc=peterz@infradead.org \
--cc=pgonda@google.com \
--cc=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=thomas.lendacky@amd.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).