>-----Original Message-----
>From: Daniel P. Berrangé <berra...@redhat.com>
>Subject: Re: [PATCH rfcv4 08/13] Add Intel TDX Quote Generation Service(QGS)
>support
>
>On Fri, May 24, 2024 at 02:21:23PM +0800, Zhenzhong Duan wrote:
>> Add element "quoteGenerationService" to tdx launch security type.
>> Currently it contains only one sub-element "SocketAddress".
>>
>> "SocketAddress" is modelized according to QEMU QAPI, supporting
>> inet, unix, vsock and fd type and variant attributes depending
>> on type.
>>
>> XML example:
>>
>> <launchSecurity type='tdx'>
>> <policy>0x0</policy>
>> <mrConfigId>xxx</mrConfigId>
>> <mrOwner>xxx</mrOwner>
>> <mrOwnerConfig>xxx</mrOwnerConfig>
>> <quoteGenerationService>
>> <SocketAddress type='vsock' cid='xxx' port='xxx'/>
>> </quoteGenerationService>
>> </launchSecurity>
>>
>> QEMU command line example:
>> qemu-system-x86_64 \
>> -object '{"qom-type":"tdx-guest","id":"lsec0","sept-ve-
>disable":false,"mrconfigid":"xxx","mrowner":"xxx","mrownerconfig":"xxx","quot
>e-generation-socket":{"type":"vsock","cid":"xxx","port":"xxx"}}' \
>> -machine pc-q35-6.0,confidential-guest-support=lsec0
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>> ---
>> src/conf/domain_conf.c | 272 +++++++++++++++++++++++++++++-
>> src/conf/domain_conf.h | 61 +++++++
>> src/conf/schemas/domaincommon.rng | 106 ++++++++++++
>> src/qemu/qemu_command.c | 106 ++++++++++++
>> 4 files changed, 544 insertions(+), 1 deletion(-)
>
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index bb4973fce8..15cdb3e0e6 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -2852,6 +2852,55 @@ struct _virDomainKeyWrapDef {
>> virTristateSwitch dea;
>> };
>>
>> +typedef enum {
>> + VIR_DOMAIN_SOCKET_ADDRESS_NONE,
>> + VIR_DOMAIN_SOCKET_ADDRESS_INET,
>> + VIR_DOMAIN_SOCKET_ADDRESS_UNIX,
>> + VIR_DOMAIN_SOCKET_ADDRESS_VSOCK,
>> + VIR_DOMAIN_SOCKET_ADDRESS_FD,
>> +
>> + VIR_DOMAIN_SOCKET_ADDRESS_LAST
>> +} virDomainSocketAddress;
>> +
>> +typedef struct _InetSocketAddress InetSocketAddress;
>> +typedef struct _UnixSocketAddress UnixSocketAddress;
>> +typedef struct _VsockSocketAddress VsockSocketAddress;
>> +typedef struct _FdSocketAddress FdSocketAddress;
>> +
>> +struct _InetSocketAddress {
>> + char *host;
>> + char *port;
>> + bool has_numeric;
>> + virTristateBool numeric;
>> + bool has_to;
>> + unsigned int to;
>> + bool has_ipv4;
>> + virTristateBool ipv4;
>> + bool has_ipv6;
>> + virTristateBool ipv6;
>> + bool has_keep_alive;
>> + virTristateBool keep_alive;
>> + bool has_mptcp;
>> + virTristateBool mptcp;
>> +};
>> +
>> +struct _UnixSocketAddress {
>> + char *path;
>> + bool has_abstract;
>> + virTristateBool abstract;
>> + bool has_tight;
>> + virTristateBool tight;
>> +};
>> +
>> +struct _VsockSocketAddress {
>> + char *cid;
>> + char *port;
>> +};
>> +
>> +struct _FdSocketAddress {
>> + char *str;
>> +};
>> +
>> typedef enum {
>> VIR_DOMAIN_LAUNCH_SECURITY_NONE,
>> VIR_DOMAIN_LAUNCH_SECURITY_SEV,
>> @@ -2873,11 +2922,22 @@ struct _virDomainSEVDef {
>> virTristateBool kernel_hashes;
>> };
>>
>> +typedef struct SocketAddress {
>> + virDomainSocketAddress type;
>> + union {
>> + InetSocketAddress inet;
>> + UnixSocketAddress Unix;
>> + VsockSocketAddress vsock;
>> + FdSocketAddress fd;
>
>The 'fd' socket type does not make sense to expose
>in libvirt XML. FD passing is something handled
>privately between libvirt & QEMU, not a end user
>choice.
Yes, I can remove ' FdSocketAddress fd'.
>
>Going further I don't think InetSocketAddress
>makes sense to expose, as QGS has no ability
>to listen on IP sockets. It can only do UNIX
>sockets or VSock AFAICT.
Why can't qemu connect to QGS on a remote host?
Even if connect to QGS on localhost, 127.0.0.1 can be used.
> Even vsock looks like
>a remnant of the old way of doing attestation
>before it was integrated into Linux via sysfs
>with the kernel making a call into QEMU.
Not get, qemu does support vsock, see
https://lore.kernel.org/qemu-devel/20240229063726.610065-50-xiaoyao...@intel.com/
>
>IOW, AFAICT, for QGS all we actually need is
>the ability to set a UNIX socket path in libvirt.
>eg
>
> <quoteGenerationService path="/var/run/tdx-qgs/qgs.socket"/>
Hmm, then we go back to opaque string, do you change your mind?
See
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/XCE4F4FCRFIP7IVZCFPB6RDWLEWXXT2G/
>
>and probably libvirt should allow 'path' to be optional
>so an app can just do
>
> <quoteGenerationService/>
>
>and libvirt would fill in the default path which QGS listens
>on out of the box....
Yes if we have such default QGS path, do we?
Thanks
Zhenzhong
