Messages in this thread | | | From | "Huang, Kai" <> | Subject | Re: [PATCH v1] virt: tdx-guest: Add Quote generation support using TSM_REPORTS | Date | Mon, 11 Sep 2023 10:00:42 +0000 |
| |
On Fri, 2023-09-08 at 09:19 -0700, Dan Williams wrote: > Huang, Kai wrote: > > > > > > > > Changes since previous version: > > > * Used ConfigFS interface instead of IOCTL interface. > > > * Used polling model for Quote generation and dropped the event notification IRQ support. > > > > Can you elaborate why the notification IRQ is dropped? > > Because it was a pile of hacks and non-idiomatic complexity. It can come > back when / if driver code can treat it like a typical interrupt.
Thanks. Agreed.
> > [..] > > > > > > +/* > > > + * Intel's SGX QE implementation generally uses Quote size less > > > + * than 8K (2K Quote data + ~5K of ceritificate blob). > > > + */ > > > +#define GET_QUOTE_BUF_SIZE SZ_8K > > > > SZ_8K is defined in <linux/sizes.h>. It seems it's not explicitly included. > > It's better to explicitly include it. > > > > Btw, although the size of the certificate blob shouldn't change dramatically, > > the Quote can also include the "QE Authentication Data", which can vary a lot > > depending on different QE implementation (e.g., containing geography > > information, etc). > > > > I wish eventually there's some /sysfs entry to configure the size of Quote > > buffer, but I guess it can be done in the future. > > How would userspace have any idea of how big the quote buffer is to be > able to set it? The output format at least needs to standardized within > a given vendor's implementation, and future variation should be > de-emphasized relative to getting to a common report format across > vendors.
The Quoting enclave implements what to be included into the Quote. The admin who deploys the Quoting enclave knows the Quote size.
> > [..] > > > +/* > > > + * wait_for_quote_completion() - Wait for Quote request completion > > > + * @quote_buf: Address of Quote buffer. > > > + * @timeout: Timeout in seconds to wait for the Quote generation. > > > + * > > > + * As per TDX GHCI v1.0 specification, sec titled "TDG.VP.VMCALL<GetQuote>", > > > + * the status field in the Quote buffer will be set to GET_QUOTE_IN_FLIGHT > > > + * while VMM processes the GetQuote request, and will change it to success > > > + * or error code after processing is complete. So wait till the status > > > + * changes from GET_QUOTE_IN_FLIGHT or the request timedout. > > > + */ > > > +static int wait_for_quote_completion(struct tdx_quote_buf *quote_buf, u32 timeout) > > > +{ > > > + int i = 0; > > > + > > > + /* > > > + * Quote requests usually take a few seconds to complete, so waking up > > > + * once per second to recheck the status is fine for this use case. > > > + */ > > > + while (quote_buf->status == GET_QUOTE_IN_FLIGHT && i++ < timeout) > > > + ssleep(1); > > > + > > > + return (i == timeout) ? -ETIMEDOUT : 0; > > > +} > > > > Why can't we use wait_for_completion_timeout() provided by the kernel? > > > > Btw, can we use _killable() variant? Supporting timeout brings extra > > complication, and I think supporting timeout isn't mandatory for the first > > version. > > It definitely is required even if interrupts were supported. The kernel > needs to give up on stalled operations in a reasonable amount of time.
Thanks for explaining.
| |