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

[Markus Armbruster]

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?

>> > +#
>> >  # 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