RE: [PATCH rfcv3 05/11] qemu: Add command line and validation for TDX type
-----Original Message-----
From: Daniel P. Berrangé <berrange(a)redhat.com>
Subject: Re: [PATCH rfcv3 05/11] qemu: Add command line and validation
for TDX type
On Mon, Nov 27, 2023 at 04:55:15PM +0800, Zhenzhong Duan wrote:
> QEMU will provides 'tdx-guest' object which is used to launch encrypted
> VMs on Intel platform using TDX feature.
>
> Command line looks like:
> $QEMU ... \
> -object tdx-guest,id=lsec0,debug=on,sept-ve-
disable=on,mrconfigid=xxx...xxx,mrowner=xxx...xxx,mrownerconfig=xxx...xxx,q
uote-generation-service=localhost:1234 \
> -machine q35,confidential-guest-support=lsec0
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com>
> ---
> src/qemu/qemu_command.c | 27 +++++++++++++++++++++++++++
> src/qemu/qemu_validate.c | 7 +++++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 89905378e4..45223746f5 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9645,6 +9645,32 @@ qemuBuildPVCommandLine(virDomainObj
*vm, virCommand *cmd)
> }
>
>
> +static int
> +qemuBuildTDXCommandLine(virDomainObj *vm, virCommand *cmd,
> + virDomainTDXDef *tdx)
> +{
> + g_autoptr(virJSONValue) props = NULL;
> + qemuDomainObjPrivate *priv = vm->privateData;
> +
> + VIR_DEBUG("policy=0x%x", tdx->policy);
> +
> + if (qemuMonitorCreateObjectProps(&props, "tdx-guest",
"lsec0",
> + "B:debug", !!(tdx->policy &
0x1),
> + "b:sept-ve-disable",
!!(tdx->policy & 0x10000000),
Here you're expanding the policy integer to named cli args.
What is defining the semantics for bits in the policy integer ?
They are defined at
https://github.com/intel/qemu-tdx/blob/d20f93da3197fff3a30c3fb9ebdc4303a0...
What happens if other bits are set besides 0x1 and 0x10000000 - if we
are not honouring those bits, we need to report an error when
validating the xml
Currently other bits are ignored, will fix it.
> + "S:mrconfigid", tdx->mrconfigid,
> + "S:mrowner", tdx->mrowner,
> + "S:mrownerconfig",
tdx->mrownerconfig,
> + "S:quote-generation-service",
tdx->QGS,
This is passing through QEMU JSON directly from the XML which is certainly
not allowed. As mentioned in previous patch, we need to model
'SocketAddress'
QAPI explicitly in the XML schema.
Will do.
> + NULL) < 0)
> + return -1;
> +
> + if (qemuBuildObjectCommandlineFromJSON(cmd, props, priv-
>qemuCaps) < 0)
> + return -1;
> +
> + return 0;
x> +}
> +
> +
> static int
> qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd,
> virDomainSecDef *sec)
> @@ -9660,6 +9686,7 @@ qemuBuildSecCommandLine(virDomainObj *vm,
virCommand *cmd,
> return qemuBuildPVCommandLine(vm, cmd);
> break;
> case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
> + return qemuBuildTDXCommandLine(vm, cmd, &sec->data.tdx);
> case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
> case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
> virReportEnumRangeError(virDomainLaunchSecurity, sec->sectype);
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index af630796cd..5a9173e8ff 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -1323,6 +1323,13 @@ qemuValidateDomainDef(const virDomainDef
*def,
> }
> break;
> case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
> + if (!virQEMUCapsGet(qemuCaps,
QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) ||
This isn't required as it is implied by CAPS_TDX_GUEST
Will remove.
> + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("INTEL TDX launch security is not supported
with this
QEMU binary"));
s/INTEL/Intel/
Will fix.
> + return -1;
> + }
This is where we need to valid 'policy' bits are supported.
Got it, will do.
Thanks
Zhenzhong