Re: [PATCH v2 41/58] i386/tdx: handle TDG.VP.VMCALL<GetQuote>

[Markus Armbruster]

I sent this reply to your question on the same day, but it got eaten by
malfunctioning servers, and I noticed only now after another failure
made me dig through my logs.  Sorry for the inconvenience!

Chenyi Qiang <chenyi.qiang@intel.com> writes:

> On 8/22/2023 4:24 PM, Daniel P. Berrangé wrote:
>> On Tue, Aug 22, 2023 at 08:52:30AM +0200, Markus Armbruster wrote:
>>> Xiaoyao Li <xiaoyao.li@intel.com> writes:
>>>
>>>> From: Isaku Yamahata <isaku.yamahata@intel.com>
>>>>
>>>> For GetQuote, delegate a request to Quote Generation Service.  Add property
>>>> of address of quote generation server and On request, connect to the
>>>> server, read request buffer from shared guest memory, send the request
>>>> buffer to the server and store the response into shared guest memory and
>>>> notify TD guest by interrupt.
>>>>
>>>> "quote-generation-service" is a property to specify Quote Generation
>>>> Service(QGS) in qemu socket address format.  The examples of the supported
>>>> format are "vsock:2:1234", "unix:/run/qgs", "localhost:1234".
>>>>
>>>> command line example:
>>>>   qemu-system-x86_64 \
>>>>     -object 'tdx-guest,id=tdx0,quote-generation-service=localhost:1234' \
>>>>     -machine confidential-guest-support=tdx0
>>>>
>>>> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
>>>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>>>> ---
>>>>  qapi/qom.json         |   5 +-
>>>>  target/i386/kvm/tdx.c | 380 ++++++++++++++++++++++++++++++++++++++++++
>>>>  target/i386/kvm/tdx.h |   7 +
>>>>  3 files changed, 391 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/qapi/qom.json b/qapi/qom.json
>>>> index 87c1d440f331..37139949d761 100644
>>>> --- a/qapi/qom.json
>>>> +++ b/qapi/qom.json
>>>> @@ -879,13 +879,16 @@
>>>>  #
>>>>  # @mrownerconfig: MROWNERCONFIG SHA384 hex string of 48 * 2 length 
>>>> (default: 0)
>>>>  #
>>>> +# @quote-generation-service: socket address for Quote Generation 
>>>> Service(QGS)
>>>> +#
>>>>  # Since: 8.2
>>>>  ##
>>>>  { 'struct': 'TdxGuestProperties',
>>>>    'data': { '*sept-ve-disable': 'bool',
>>>>              '*mrconfigid': 'str',
>>>>              '*mrowner': 'str',
>>>> -            '*mrownerconfig': 'str' } }
>>>> +            '*mrownerconfig': 'str',
>>>> +            '*quote-generation-service': 'str' } }
>>>
>>> Why not type SocketAddress?
>> 
>> Yes, the code uses SocketAddress internally when it eventually
>> calls qio_channel_socket_connect_async(), so we should directly
>> use SocketAddress in the QAPI from the start.
>
> Any benefit to directly use SocketAddress?

Design principle: use JSON to encode structured data as text in
QAPI/QMP.

Do: "mumble": [1, 2, 3]

Don't: "mumble": "1,2,3"

Do: "server": { "type": "inet", "host": "localhost", "port": "12345" }

Don't: "server": "host=localhost,port=12345"

We violate the principle in a couple of places.  Some are arguably
mistakes, some are pragmatic exceptions.

The principle implies "the only parser QAPI needs is the JSON parser".

The other benefit is consistency with existing interfaces.  They use
SocketAddress (a few old ones use SocketAddressLegacy).

> "quote-generation-service" here is optional, it seems not trivial to add
> and parse the SocketAddress type in QEMU command. After I change 'str'
> to 'SocketAddress' and specify the command like "-object
> tdx-guest,type=vsock,cid=2,port=1234...", it will report "invalid
> parameter cid".

Try "quote-generation-service.port=1234".