Re: [PATCH rfcv4 06/13] qemu: Add command line and validation for TDX type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 24, 2024 at 02:21:21PM +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 '{"qom-type":"tdx-guest","id":"lsec0","debug":true,"sept-ve-disable":false,"mrconfigid":"xxx","mrowner":"xxx","mrownerconfig":"xxx"}' \
>   -machine pc-q35-6.0,confidential-guest-support=lsec0
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxx>
> ---
>  src/conf/domain_conf.h   |  5 +++++
>  src/qemu/qemu_command.c  | 31 +++++++++++++++++++++++++++++++
>  src/qemu/qemu_validate.c | 11 +++++++++++
>  3 files changed, 47 insertions(+)
> 
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 7882b7a75d..bb4973fce8 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2880,6 +2880,11 @@ struct _virDomainTDXDef {
>      char *mrownerconfig;
>  };
>  
> +#define VIR_DOMAIN_TDX_POLICY_DEBUG              0x1
> +#define VIR_DOMAIN_TDX_POLICY_SEPT_VE_DISABLE    0x10000000
> +#define VIR_DOMAIN_TDX_POLICY_ALLOWED_MASK       (VIR_DOMAIN_TDX_POLICY_DEBUG | \
> +                                                  VIR_DOMAIN_TDX_POLICY_SEPT_VE_DISABLE)
> +
>  struct _virDomainSecDef {
>      virDomainLaunchSecurity sectype;
>      union {
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index dde2d5fa01..d212d80038 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9745,6 +9745,36 @@ qemuBuildPVCommandLine(virDomainObj *vm, virCommand *cmd)
>  }
>  
>  
> +static int
> +qemuBuildTDXCommandLine(virDomainObj *vm, virCommand *cmd,
> +                        virDomainTDXDef *tdx)
> +{
> +    g_autoptr(virJSONValue) props = NULL;
> +    qemuDomainObjPrivate *priv = vm->privateData;
> +    bool sept_ve_disable = tdx->policy & VIR_DOMAIN_TDX_POLICY_SEPT_VE_DISABLE;
> +
> +    VIR_DEBUG("policy=0x%llx", tdx->policy);
> +
> +    if (qemuMonitorCreateObjectProps(&props, "tdx-guest", "lsec0",
> +                                     "B:debug", !!(tdx->policy & VIR_DOMAIN_TDX_POLICY_DEBUG),
> +                                     "S:mrconfigid", tdx->mrconfigid,
> +                                     "S:mrowner", tdx->mrowner,
> +                                     "S:mrownerconfig", tdx->mrownerconfig,
> +                                     NULL) < 0)
> +        return -1;

I like that you've just exposed the "policy" as an int field in libvirt,
but find it unpleasant that we have to unpack it to pass bits to QEMU,
whereupon QEMU re-packs it into the original int field we already had.

I think this is a mistake in the QEMU QAPI design - QEMU shoud just accept
the policy in 'int' format. I've CC'd you on a mail to qemu-devel where I
raise this point.

> +
> +    /* By default, QEMU set sept-ve-disable which is required by linux kernel */
> +    if (!sept_ve_disable &&
> +        virJSONValueObjectAdd(&props, "b:sept-ve-disable", false, NULL) < 0)
> +        return -1;
> +
> +    if (qemuBuildObjectCommandlineFromJSON(cmd, props, priv->qemuCaps) < 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +
>  static int
>  qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd,
>                          virDomainSecDef *sec)
> @@ -9760,6 +9790,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 ee25fd70d9..db8493be68 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -1323,6 +1323,17 @@ qemuValidateDomainDef(const virDomainDef *def,
>              }
>              break;
>          case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
> +            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST)) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("Intel TDX launch security is not supported with this QEMU binary"));
> +                return -1;
> +            }
> +            if (def->sec->data.tdx.policy & ~VIR_DOMAIN_TDX_POLICY_ALLOWED_MASK) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("Only bit0(debug) and bit28(sept-ve-disable) are supported intel TDX launch security policy"));
> +                return -1;
> +            }
> +            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 :|



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux