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,quote-generation-service=localhost:1234 \
> -machine q35,confidential-guest-support=lsec0
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxx>
> ---
> 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 ?
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
> + "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.
> + 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
> + !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/
> + return -1;
> + }
This is where we need to valid 'policy' bits are supported.
> + break;
> case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
> case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
> virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype);
> --
> 2.34.1
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
