>-----Original Message-----
>From: Daniel P. Berrangé <berrange@xxxxxxxxxx>
>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@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 ?
They are defined at
https://github.com/intel/qemu-tdx/blob/d20f93da3197fff3a30c3fb9ebdc4303a06a3343/target/i386/kvm/tdx.c#L49
>
>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
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
