[PATCH v8 02/55] i386: Introduce tdx-guest object

Xiaoyao Li posted 55 patches 10 months, 2 weeks ago
[PATCH v8 02/55] i386: Introduce tdx-guest object
Posted by Xiaoyao Li 10 months, 2 weeks ago
Introduce tdx-guest object which inherits X86_CONFIDENTIAL_GUEST,
and will be used to create TDX VMs (TDs) by

  qemu -machine ...,confidential-guest-support=tdx0	\
       -object tdx-guest,id=tdx0

It has one QAPI member 'attributes' defined, which allows user to set
TD's attributes directly.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
---
Changes in v7:
 - update QAPI version to 10.0;
 - update to use SPDX tags for license info;
 - update copyright to 2025;

Chanegs in v6:
 - Make tdx-guest inherits X86_CONFIDENTIAL_GUEST;
 - set cgs->require_guest_memfd;
 - allow attributes settable via QAPI;
 - update QAPI version to since 9.2;

Changes in v4:
 - update the new qapi `since` filed from 8.2 to 9.0

Changes in v1
 - make @attributes not user-settable
---
 configs/devices/i386-softmmu/default.mak |  1 +
 hw/i386/Kconfig                          |  5 +++
 qapi/qom.json                            | 15 +++++++++
 target/i386/kvm/meson.build              |  2 ++
 target/i386/kvm/tdx.c                    | 43 ++++++++++++++++++++++++
 target/i386/kvm/tdx.h                    | 21 ++++++++++++
 6 files changed, 87 insertions(+)
 create mode 100644 target/i386/kvm/tdx.c
 create mode 100644 target/i386/kvm/tdx.h

diff --git a/configs/devices/i386-softmmu/default.mak b/configs/devices/i386-softmmu/default.mak
index 4faf2f0315e2..bc0479a7e0a3 100644
--- a/configs/devices/i386-softmmu/default.mak
+++ b/configs/devices/i386-softmmu/default.mak
@@ -18,6 +18,7 @@
 #CONFIG_QXL=n
 #CONFIG_SEV=n
 #CONFIG_SGA=n
+#CONFIG_TDX=n
 #CONFIG_TEST_DEVICES=n
 #CONFIG_TPM_CRB=n
 #CONFIG_TPM_TIS_ISA=n
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index d34ce07b215d..cce9521ba934 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -10,6 +10,10 @@ config SGX
     bool
     depends on KVM
 
+config TDX
+    bool
+    depends on KVM
+
 config PC
     bool
     imply APPLESMC
@@ -26,6 +30,7 @@ config PC
     imply QXL
     imply SEV
     imply SGX
+    imply TDX
     imply TEST_DEVICES
     imply TPM_CRB
     imply TPM_TIS_ISA
diff --git a/qapi/qom.json b/qapi/qom.json
index 28ce24cd8d08..c0b61df964ef 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1047,6 +1047,19 @@
             '*host-data': 'str',
             '*vcek-disabled': 'bool' } }
 
+##
+# @TdxGuestProperties:
+#
+# Properties for tdx-guest objects.
+#
+# @attributes: The 'attributes' of a TD guest that is passed to
+#     KVM_TDX_INIT_VM
+#
+# Since: 10.1
+##
+{ 'struct': 'TdxGuestProperties',
+  'data': { '*attributes': 'uint64' } }
+
 ##
 # @ThreadContextProperties:
 #
@@ -1132,6 +1145,7 @@
     'sev-snp-guest',
     'thread-context',
     's390-pv-guest',
+    'tdx-guest',
     'throttle-group',
     'tls-creds-anon',
     'tls-creds-psk',
@@ -1204,6 +1218,7 @@
                                       'if': 'CONFIG_SECRET_KEYRING' },
       'sev-guest':                  'SevGuestProperties',
       'sev-snp-guest':              'SevSnpGuestProperties',
+      'tdx-guest':                  'TdxGuestProperties',
       'thread-context':             'ThreadContextProperties',
       'throttle-group':             'ThrottleGroupProperties',
       'tls-creds-anon':             'TlsCredsAnonProperties',
diff --git a/target/i386/kvm/meson.build b/target/i386/kvm/meson.build
index 3996cafaf29f..466bccb9cb17 100644
--- a/target/i386/kvm/meson.build
+++ b/target/i386/kvm/meson.build
@@ -8,6 +8,8 @@ i386_kvm_ss.add(files(
 
 i386_kvm_ss.add(when: 'CONFIG_XEN_EMU', if_true: files('xen-emu.c'))
 
+i386_kvm_ss.add(when: 'CONFIG_TDX', if_true: files('tdx.c'))
+
 i386_system_ss.add(when: 'CONFIG_HYPERV', if_true: files('hyperv.c'), if_false: files('hyperv-stub.c'))
 
 i386_system_ss.add_all(when: 'CONFIG_KVM', if_true: i386_kvm_ss)
diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
new file mode 100644
index 000000000000..ec84ae2947bb
--- /dev/null
+++ b/target/i386/kvm/tdx.c
@@ -0,0 +1,43 @@
+/*
+ * QEMU TDX support
+ *
+ * Copyright (c) 2025 Intel Corporation
+ *
+ * Author:
+ *      Xiaoyao Li <xiaoyao.li@intel.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qom/object_interfaces.h"
+
+#include "tdx.h"
+
+/* tdx guest */
+OBJECT_DEFINE_TYPE_WITH_INTERFACES(TdxGuest,
+                                   tdx_guest,
+                                   TDX_GUEST,
+                                   X86_CONFIDENTIAL_GUEST,
+                                   { TYPE_USER_CREATABLE },
+                                   { NULL })
+
+static void tdx_guest_init(Object *obj)
+{
+    ConfidentialGuestSupport *cgs = CONFIDENTIAL_GUEST_SUPPORT(obj);
+    TdxGuest *tdx = TDX_GUEST(obj);
+
+    cgs->require_guest_memfd = true;
+    tdx->attributes = 0;
+
+    object_property_add_uint64_ptr(obj, "attributes", &tdx->attributes,
+                                   OBJ_PROP_FLAG_READWRITE);
+}
+
+static void tdx_guest_finalize(Object *obj)
+{
+}
+
+static void tdx_guest_class_init(ObjectClass *oc, void *data)
+{
+}
diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h
new file mode 100644
index 000000000000..f3b725336161
--- /dev/null
+++ b/target/i386/kvm/tdx.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef QEMU_I386_TDX_H
+#define QEMU_I386_TDX_H
+
+#include "confidential-guest.h"
+
+#define TYPE_TDX_GUEST "tdx-guest"
+#define TDX_GUEST(obj)  OBJECT_CHECK(TdxGuest, (obj), TYPE_TDX_GUEST)
+
+typedef struct TdxGuestClass {
+    X86ConfidentialGuestClass parent_class;
+} TdxGuestClass;
+
+typedef struct TdxGuest {
+    X86ConfidentialGuest parent_obj;
+
+    uint64_t attributes;    /* TD attributes */
+} TdxGuest;
+
+#endif /* QEMU_I386_TDX_H */
-- 
2.34.1
Re: [PATCH v8 02/55] i386: Introduce tdx-guest object
Posted by Zhao Liu 9 months, 3 weeks ago
>  configs/devices/i386-softmmu/default.mak |  1 +
>  hw/i386/Kconfig                          |  5 +++
>  qapi/qom.json                            | 15 +++++++++
>  target/i386/kvm/meson.build              |  2 ++
>  target/i386/kvm/tdx.c                    | 43 ++++++++++++++++++++++++
>  target/i386/kvm/tdx.h                    | 21 ++++++++++++

SEV.* and confidential-guest.* are all placed in target/i386/.
It's best if all of these can be in the same place.

>  6 files changed, 87 insertions(+)
>  create mode 100644 target/i386/kvm/tdx.c
>  create mode 100644 target/i386/kvm/tdx.h

...

> diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h
> new file mode 100644
> index 000000000000..f3b725336161
> --- /dev/null
> +++ b/target/i386/kvm/tdx.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef QEMU_I386_TDX_H
> +#define QEMU_I386_TDX_H

I386_TDX_H is enough... the QEMU prefix is rarely seen in the whole
project.

Others look good,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Re: [PATCH v8 02/55] i386: Introduce tdx-guest object
Posted by Daniel P. Berrangé 9 months, 3 weeks ago
On Fri, Apr 18, 2025 at 05:17:02PM +0800, Zhao Liu wrote:
> >  configs/devices/i386-softmmu/default.mak |  1 +
> >  hw/i386/Kconfig                          |  5 +++
> >  qapi/qom.json                            | 15 +++++++++
> >  target/i386/kvm/meson.build              |  2 ++
> >  target/i386/kvm/tdx.c                    | 43 ++++++++++++++++++++++++
> >  target/i386/kvm/tdx.h                    | 21 ++++++++++++
> 
> SEV.* and confidential-guest.* are all placed in target/i386/.
> It's best if all of these can be in the same place.
> 
> >  6 files changed, 87 insertions(+)
> >  create mode 100644 target/i386/kvm/tdx.c
> >  create mode 100644 target/i386/kvm/tdx.h
> 
> ...
> 
> > diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h
> > new file mode 100644
> > index 000000000000..f3b725336161
> > --- /dev/null
> > +++ b/target/i386/kvm/tdx.h
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +
> > +#ifndef QEMU_I386_TDX_H
> > +#define QEMU_I386_TDX_H
> 
> I386_TDX_H is enough... the QEMU prefix is rarely seen in the whole
> project.

IMHO having a QEMU_ prefix here is "best practice", so don't remove it.

That lots of other QEMU code doesn't follow best practice is unfortunate.

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 :|
Re: [PATCH v8 02/55] i386: Introduce tdx-guest object
Posted by Zhao Liu 9 months, 3 weeks ago
> > > index 000000000000..f3b725336161
> > > --- /dev/null
> > > +++ b/target/i386/kvm/tdx.h
> > > @@ -0,0 +1,21 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +
> > > +#ifndef QEMU_I386_TDX_H
> > > +#define QEMU_I386_TDX_H
> > 
> > I386_TDX_H is enough... the QEMU prefix is rarely seen in the whole
> > project.
> 
> IMHO having a QEMU_ prefix here is "best practice", so don't remove it.
> 
> That lots of other QEMU code doesn't follow best practice is unfortunate.

Thanks, now I see!
Re: [PATCH v8 02/55] i386: Introduce tdx-guest object
Posted by Xiaoyao Li 9 months, 3 weeks ago
On 4/18/2025 5:17 PM, Zhao Liu wrote:
>>   configs/devices/i386-softmmu/default.mak |  1 +
>>   hw/i386/Kconfig                          |  5 +++
>>   qapi/qom.json                            | 15 +++++++++
>>   target/i386/kvm/meson.build              |  2 ++
>>   target/i386/kvm/tdx.c                    | 43 ++++++++++++++++++++++++
>>   target/i386/kvm/tdx.h                    | 21 ++++++++++++
> 
> SEV.* and confidential-guest.* are all placed in target/i386/.
> It's best if all of these can be in the same place.

I think it's more reasonable to put TDX code under KVM directory since 
TDX depends on KVM.

Regarding SEV, I think the same applies. But it was merged as is and I 
don't see it strongly that it has to move.

>>   6 files changed, 87 insertions(+)
>>   create mode 100644 target/i386/kvm/tdx.c
>>   create mode 100644 target/i386/kvm/tdx.h
> 
> ...
> 
>> diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h
>> new file mode 100644
>> index 000000000000..f3b725336161
>> --- /dev/null
>> +++ b/target/i386/kvm/tdx.h
>> @@ -0,0 +1,21 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +
>> +#ifndef QEMU_I386_TDX_H
>> +#define QEMU_I386_TDX_H
> 
> I386_TDX_H is enough... the QEMU prefix is rarely seen in the whole
> project.

# git grep -r "#ifndef QEMU_" ./ | wc -l
# 329

I'm not sure if 329 means rarely?

I will just keep it unchanged unless someone objects it strongly.

> Others look good,
> 
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Thanks!
Re: [PATCH v8 02/55] i386: Introduce tdx-guest object
Posted by Daniel P. Berrangé 10 months, 1 week ago
On Tue, Apr 01, 2025 at 09:01:12AM -0400, Xiaoyao Li wrote:
> Introduce tdx-guest object which inherits X86_CONFIDENTIAL_GUEST,
> and will be used to create TDX VMs (TDs) by
> 
>   qemu -machine ...,confidential-guest-support=tdx0	\
>        -object tdx-guest,id=tdx0
> 
> It has one QAPI member 'attributes' defined, which allows user to set
> TD's attributes directly.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> Acked-by: Markus Armbruster <armbru@redhat.com>
> ---
> Changes in v7:
>  - update QAPI version to 10.0;
>  - update to use SPDX tags for license info;
>  - update copyright to 2025;
> 
> Chanegs in v6:
>  - Make tdx-guest inherits X86_CONFIDENTIAL_GUEST;
>  - set cgs->require_guest_memfd;
>  - allow attributes settable via QAPI;
>  - update QAPI version to since 9.2;
> 
> Changes in v4:
>  - update the new qapi `since` filed from 8.2 to 9.0
> 
> Changes in v1
>  - make @attributes not user-settable
> ---
>  configs/devices/i386-softmmu/default.mak |  1 +
>  hw/i386/Kconfig                          |  5 +++
>  qapi/qom.json                            | 15 +++++++++
>  target/i386/kvm/meson.build              |  2 ++
>  target/i386/kvm/tdx.c                    | 43 ++++++++++++++++++++++++
>  target/i386/kvm/tdx.h                    | 21 ++++++++++++
>  6 files changed, 87 insertions(+)
>  create mode 100644 target/i386/kvm/tdx.c
>  create mode 100644 target/i386/kvm/tdx.h

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|