Re: [PATCH 0/4] keys: Introduce a keys frontend for attestation reports

[Huang, Kai]

On Fri, 2023-08-04 at 17:19 +0100, Daniel P. Berrangé wrote:
> On Tue, Aug 01, 2023 at 11:45:12AM +0000, Huang, Kai wrote:
> > The IOCTL vs /sysfs isn't discussed.
> > 
> > For instance, after rough thinking, why is the IOCTL better than below approach
> > using /sysfs?
> > 
> > echo <REPORTDATA> > /sys/kernel/coco/tdx/attest/reportdata
> > cat /sys/kernel/coco/tdx/attest/tdreport
> > 
> > Each "echo <REPORTDATA>" to '/sys/.../reportdata' triggers the driver to call
> > TDCALL to get the TDREPORT, which is available at '/sys/.../tdreport'.
> 
> What would you suggest as behaviour with multiple processes writing
> into 'reportdata' and trying to read from 'tdreport' in parallel ?
> Splitting input and output across separate files removes any
> transactional relationship between input and output. This approach
> feels like it could easily result in buggy behaviour from concurrent
> application usage, which would not be an issue with ioctl()

At that time we believed attestation is a relatively low frequent operation thus
it's acceptable to not support concurrent report generation.  While kernel is
processing one report the other requests need to wait.  This shouldn't be a
problem because the TDREPORT mentioned above is directly from TDX firmware and
won't block for long time.

And in that context we were splitting getting TDREPORT and Quote, meaning after
getting TDREPORT we could have another mechanism (e.g., using IOCTL()) to get
Quote, which could be made to support concurrent requests. 

Now if we want to use /sysfs to get the Quote, I am still expecting attestation
should be a low frequent operation, thus to me not supporting concurrent
requests is still acceptable.  But since getting Quote involves asking VMM to
get a signed Quote from another userspace process (SGX QE) or even from another
VM (where QE runs) depending on the deployment, the latency of being blocked
from reading /sysfs may be a concern.  But we can support return -EINTR if
needed so not a blocking issue I suppose.

Do you have any use case that supporting concurrent attestation requests is
important?

Btw I am not against using IOCTL() :-)

> 
> Also note, there needs to be scope for more than 1 input and 1 output
> data items. For SNP guests, the VMPL is a input, and if fetching a
> VMPL 0 report from under SVSM [1], an optional service GUID is needed.
> With SVSM, there are three distinct output data blobs - attestation
> report, services manifest and certificate data.

Yeah we need to find someway to unify.