Re: [PATCH v4 50/66] i386/tdx: handle TDG.VP.VMCALL<GetQuote>

[Xiaoyao Li]

On 2/19/2024 10:41 PM, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > On Mon, Feb 19, 2024 at 01:50:12PM +0100, Markus Armbruster wrote:
> > > Xiaoyao Li <xiaoyao.li@intel.com> writes:
> > >
> > > > From: Isaku Yamahata <isaku.yamahata@intel.com>
> > > >
> > > > Add property "quote-generation-socket" to tdx-guest, which is a property
> > > > of type SocketAddress to specify Quote Generation Service(QGS).
> > > >
> > > > On request of GetQuote, it connects to the QGS socket, read request
> > > > data from shared guest memory, send the request data to the QGS,
> > > > and store the response into shared guest memory, at last notify
> > > > TD guest by interrupt.
> > > >
> > > > command line example:
> > > >    qemu-system-x86_64 \
> > > >      -object '{"qom-type":"tdx-guest","id":"tdx0","quote-generation-socket":{"type": "vsock", 
> > > > "cid":"1","port":"1234"}}' \
> > > >      -machine confidential-guest-support=tdx0
> > > >
> > > > Note, above example uses vsock type socket because the QGS we used
> > > > implements the vsock socket. It can be other types, like UNIX socket,
> > > > which depends on the implementation of QGS.
> > > >
> > > > To avoid no response from QGS server, setup a timer for the transaction.
> > > > If timeout, make it an error and interrupt guest. Define the threshold of
> > > > time to 30s at present, maybe change to other value if not appropriate.
> > > >
> > > > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > > > Codeveloped-by: Chenyi Qiang <chenyi.qiang@intel.com>
> > > > Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
> > > > Codeveloped-by: Xiaoyao Li <xiaoyao.li@intel.com>
> > > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> > > > ---
> > > > Changes in v4:
> > > > - merge next patch "i386/tdx: setup a timer for the qio channel";
> > > >
> > > > Changes in v3:
> > > > - rename property "quote-generation-service" to "quote-generation-socket";
> > > > - change the type of "quote-generation-socket" from str to
> > > >    SocketAddress;
> > > > - squash next patch into this one;
> > > > ---
> > > >   qapi/qom.json                         |   6 +-
> > > >   target/i386/kvm/meson.build           |   2 +-
> > > >   target/i386/kvm/tdx-quote-generator.c | 170 ++++++++++++++++++++
> > > >   target/i386/kvm/tdx-quote-generator.h |  95 +++++++++++
> > > >   target/i386/kvm/tdx.c                 | 216 ++++++++++++++++++++++++++
> > > >   target/i386/kvm/tdx.h                 |   6 +
> > > >   6 files changed, 493 insertions(+), 2 deletions(-)
> > > >   create mode 100644 target/i386/kvm/tdx-quote-generator.c
> > > >   create mode 100644 target/i386/kvm/tdx-quote-generator.h
> > > >
> > > > diff --git a/qapi/qom.json b/qapi/qom.json
> > > > index 15445f9e41fc..c60fb5710961 100644
> > > > --- a/qapi/qom.json
> > > > +++ b/qapi/qom.json
> > > > @@ -914,13 +914,17 @@
> > > >   #     e.g., specific to the workload rather than the run-time or OS.
> > > >   #     base64 encoded SHA384 digest.
> > > >   #
> > > > +# @quote-generation-socket: socket address for Quote Generation
> > > > +#     Service(QGS)
> > >
> > > Space between "Service" and "(QGS)", please.
> > >
> > > The description feels too terse.  What is the "Quote Generation
> > > Service", and why should I care?
> >
> > The "Quote Generation Service" is a daemon running on the host.
> > The reference implementation is at
> >
> >    
> > https://github.com/intel/SGXDataCenterAttestationPrimitives/tree/master/QuoteGeneration/quote_wrapper/qgs
> >
> > If you don't provide this, then quests won't bet able to generate
> > quotes needed for attestation. So although this is technically
> > optional, in practice for a sane deployment, an admin should always
> > provide this
>
> Thanks.  Care to work some of this information into the doc comment?

Sure. Will add a new section of Attestation in the last patch.

> > > > +#
> > > >   # Since: 9.0
> > > >   ##
> > > >   { 'struct': 'TdxGuestProperties',
> > > >     'data': { '*sept-ve-disable': 'bool',
> > > >               '*mrconfigid': 'str',
> > > >               '*mrowner': 'str',
> > > > -            '*mrownerconfig': 'str' } }
> > > > +            '*mrownerconfig': 'str',
> > > > +            '*quote-generation-socket': 'SocketAddress' } }
> > > >   
> > > >   ##
> > > >   # @ThreadContextProperties:
> > >
> > > [...]
> >
> > With regards,
> > Daniel