[RFC PATCH 0/4] Confidential Guest Support: Introduce kvm_init() and kvm_reset() virtual functions

[RFC PATCH 0/4] Confidential Guest Support: Introduce kvm_init() and kvm_reset() virtual functions

[Xiaoyao Li]

This series is inspired and suggested by Daniel:
https://lore.kernel.org/qemu-devel/ZbfoQsEuv6_zwl3b@redhat.com/

Currently, different confidential VMs in different architectures have
their own specific *_kvm_init() (and some have *_kvm_reset()) exposed
for KVM stuff when it's a confidential VM. e.g., sev_kmv_init() for x86
SEV, pef_kvm_init() and pef_kvm_reset() for PPC PEF, and s390_pv_init()
for s390 PV VMs.

Introduce a generic .kvm_init() and .kvm_reset() functions in
ConfidentialGuestSupportClass, so that different cgs technologies in
different architectures can implement their own, while common interface
of cgs can be used.

This RFC implements two helper functions confidential_guest_kvm_init()
and confidential_guest_kvm_reset() in Patch 1. In the following patches,
they are called in arch specific implementation. X86 will benefit more
for the generic implementation when TDX support is added.

There is one step forward possible, that calling
confidential_guest_kvm_init() before kvm_arch_init() in kvm_int() in
accel/kvm/kvm-all.c. This way, each arch doesn't need to call in their
arch specific code.
	
X86 fits it, however I'm not sure if ppc and s390 fit it as well.
Because currently, ppc calls it in machine->init()
and s390 calls in MachineClass->init(). I'm not sure if there is any
order dependency. 

Xiaoyao Li (4):
  confidential guest support: Add kvm_init() and kvm_reset() in class
  i386/sev: Switch to use confidential_guest_kvm_init()
  ppc/pef: switch to use confidential_guest_kvm_init/reset()
  s390: Switch to use confidential_guest_kvm_init()

 hw/ppc/pef.c                              |   9 +-
 hw/ppc/spapr.c                            |   6 +-
 hw/s390x/s390-virtio-ccw.c                |   3 +-
 include/exec/confidential-guest-support.h |  42 +++++++-
 include/hw/ppc/pef.h                      |  17 ---
 target/i386/kvm/kvm.c                     |   2 +-
 target/i386/kvm/meson.build               |   2 -
 target/i386/kvm/sev-stub.c                |   5 -
 target/i386/sev.c                         | 120 +++++++++++-----------
 target/i386/sev.h                         |   2 -
 target/s390x/kvm/pv.c                     |   8 ++
 target/s390x/kvm/pv.h                     |  14 ---
 12 files changed, 123 insertions(+), 107 deletions(-)
 delete mode 100644 include/hw/ppc/pef.h

-- 
2.34.1

[RFC PATCH 1/4] confidential guest support: Add kvm_init() and kvm_reset() in class

[Xiaoyao Li]

Different confidential VMs in different architectures all have the same
needs to do their specific initialization (and maybe resetting) stuffs
with KVM. Currently each of them exposes individual *_kvm_init()
functions and let machine code or kvm code to call it.

To make it more object oriented, add two virtual functions, kvm_init()
and kvm_reset() in ConfidentialGuestSupportClass, and expose two helpers
functions for invodking them.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 include/exec/confidential-guest-support.h | 42 ++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/include/exec/confidential-guest-support.h b/include/exec/confidential-guest-support.h
index ba2dd4b5dfc4..ff0bfb26ad7a 100644
--- a/include/exec/confidential-guest-support.h
+++ b/include/exec/confidential-guest-support.h
@@ -23,7 +23,10 @@
 #include "qom/object.h"
 
 #define TYPE_CONFIDENTIAL_GUEST_SUPPORT "confidential-guest-support"
-OBJECT_DECLARE_SIMPLE_TYPE(ConfidentialGuestSupport, CONFIDENTIAL_GUEST_SUPPORT)
+OBJECT_DECLARE_TYPE(ConfidentialGuestSupport,
+                    ConfidentialGuestSupportClass,
+                    CONFIDENTIAL_GUEST_SUPPORT)
+
 
 struct ConfidentialGuestSupport {
     Object parent;
@@ -55,8 +58,45 @@ struct ConfidentialGuestSupport {
 
 typedef struct ConfidentialGuestSupportClass {
     ObjectClass parent;
+
+    int (*kvm_init)(ConfidentialGuestSupport *cgs, Error **errp);
+    int (*kvm_reset)(ConfidentialGuestSupport *cgs, Error **errp);
 } ConfidentialGuestSupportClass;
 
+static inline int confidential_guest_kvm_init(ConfidentialGuestSupport *cgs,
+                                              Error **errp)
+{
+    ConfidentialGuestSupportClass *klass;
+
+    if (!cgs) {
+        return 0;
+    }
+
+    klass = CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(cgs);
+    if (klass->kvm_init) {
+        return klass->kvm_init(cgs, errp);
+    }
+
+    return 0;
+}
+
+static inline int confidential_guest_kvm_reset(ConfidentialGuestSupport *cgs,
+                                               Error **errp)
+{
+    ConfidentialGuestSupportClass *klass;
+
+    if (!cgs) {
+        return 0;
+    }
+
+    klass = CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(cgs);
+    if (klass->kvm_reset) {
+        return klass->kvm_reset(cgs, errp);
+    }
+
+    return 0;
+}
+
 #endif /* !CONFIG_USER_ONLY */
 
 #endif /* QEMU_CONFIDENTIAL_GUEST_SUPPORT_H */
-- 
2.34.1

[RFC PATCH 2/4] i386/sev: Switch to use confidential_guest_kvm_init()

[Xiaoyao Li]

Use confidential_guest_kvm_init() instead of calling SEV specific
sev_kvm_init(). As a bouns, it fits to future TDX when TDX implements
its own confidential_guest_support and .kvm_init().

Move the "TypeInfo sev_guest_info" definition and related functions to
the end of the file, to avoid declaring the sev_kvm_init() ahead.

Clean up the sve-stub.c since it's not needed anymore.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 target/i386/kvm/kvm.c       |   2 +-
 target/i386/kvm/meson.build |   2 -
 target/i386/kvm/sev-stub.c  |   5 --
 target/i386/sev.c           | 120 +++++++++++++++++++-----------------
 target/i386/sev.h           |   2 -
 5 files changed, 63 insertions(+), 68 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 76a66246eb72..bb63bba61fa1 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2534,7 +2534,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
      * mechanisms are supported in future (e.g. TDX), they'll need
      * their own initialization either here or elsewhere.
      */
-    ret = sev_kvm_init(ms->cgs, &local_err);
+    ret = confidential_guest_kvm_init(ms->cgs, &local_err);
     if (ret < 0) {
         error_report_err(local_err);
         return ret;
diff --git a/target/i386/kvm/meson.build b/target/i386/kvm/meson.build
index 84d9143e6029..e7850981e62d 100644
--- a/target/i386/kvm/meson.build
+++ b/target/i386/kvm/meson.build
@@ -7,8 +7,6 @@ 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_SEV', if_false: files('sev-stub.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/sev-stub.c b/target/i386/kvm/sev-stub.c
index 1be5341e8a6a..4a1560cf8ad7 100644
--- a/target/i386/kvm/sev-stub.c
+++ b/target/i386/kvm/sev-stub.c
@@ -14,8 +14,3 @@
 #include "qemu/osdep.h"
 #include "sev.h"
 
-int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
-{
-    /* If we get here, cgs must be some non-SEV thing */
-    return 0;
-}
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 173de91afe7d..19e79d3631d0 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -353,63 +353,6 @@ static void sev_guest_set_kernel_hashes(Object *obj, bool value, Error **errp)
     sev->kernel_hashes = value;
 }
 
-static void
-sev_guest_class_init(ObjectClass *oc, void *data)
-{
-    object_class_property_add_str(oc, "sev-device",
-                                  sev_guest_get_sev_device,
-                                  sev_guest_set_sev_device);
-    object_class_property_set_description(oc, "sev-device",
-            "SEV device to use");
-    object_class_property_add_str(oc, "dh-cert-file",
-                                  sev_guest_get_dh_cert_file,
-                                  sev_guest_set_dh_cert_file);
-    object_class_property_set_description(oc, "dh-cert-file",
-            "guest owners DH certificate (encoded with base64)");
-    object_class_property_add_str(oc, "session-file",
-                                  sev_guest_get_session_file,
-                                  sev_guest_set_session_file);
-    object_class_property_set_description(oc, "session-file",
-            "guest owners session parameters (encoded with base64)");
-    object_class_property_add_bool(oc, "kernel-hashes",
-                                   sev_guest_get_kernel_hashes,
-                                   sev_guest_set_kernel_hashes);
-    object_class_property_set_description(oc, "kernel-hashes",
-            "add kernel hashes to guest firmware for measured Linux boot");
-}
-
-static void
-sev_guest_instance_init(Object *obj)
-{
-    SevGuestState *sev = SEV_GUEST(obj);
-
-    sev->sev_device = g_strdup(DEFAULT_SEV_DEVICE);
-    sev->policy = DEFAULT_GUEST_POLICY;
-    object_property_add_uint32_ptr(obj, "policy", &sev->policy,
-                                   OBJ_PROP_FLAG_READWRITE);
-    object_property_add_uint32_ptr(obj, "handle", &sev->handle,
-                                   OBJ_PROP_FLAG_READWRITE);
-    object_property_add_uint32_ptr(obj, "cbitpos", &sev->cbitpos,
-                                   OBJ_PROP_FLAG_READWRITE);
-    object_property_add_uint32_ptr(obj, "reduced-phys-bits",
-                                   &sev->reduced_phys_bits,
-                                   OBJ_PROP_FLAG_READWRITE);
-}
-
-/* sev guest info */
-static const TypeInfo sev_guest_info = {
-    .parent = TYPE_CONFIDENTIAL_GUEST_SUPPORT,
-    .name = TYPE_SEV_GUEST,
-    .instance_size = sizeof(SevGuestState),
-    .instance_finalize = sev_guest_finalize,
-    .class_init = sev_guest_class_init,
-    .instance_init = sev_guest_instance_init,
-    .interfaces = (InterfaceInfo[]) {
-        { TYPE_USER_CREATABLE },
-        { }
-    }
-};
-
 bool
 sev_enabled(void)
 {
@@ -906,7 +849,7 @@ sev_vm_state_change(void *opaque, bool running, RunState state)
     }
 }
 
-int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
+static int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
 {
     SevGuestState *sev
         = (SevGuestState *)object_dynamic_cast(OBJECT(cgs), TYPE_SEV_GUEST);
@@ -1383,6 +1326,67 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
     return ret;
 }
 
+static void
+sev_guest_class_init(ObjectClass *oc, void *data)
+{
+    ConfidentialGuestSupportClass *klass = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
+
+    klass->kvm_init = sev_kvm_init;
+
+    object_class_property_add_str(oc, "sev-device",
+                                  sev_guest_get_sev_device,
+                                  sev_guest_set_sev_device);
+    object_class_property_set_description(oc, "sev-device",
+            "SEV device to use");
+    object_class_property_add_str(oc, "dh-cert-file",
+                                  sev_guest_get_dh_cert_file,
+                                  sev_guest_set_dh_cert_file);
+    object_class_property_set_description(oc, "dh-cert-file",
+            "guest owners DH certificate (encoded with base64)");
+    object_class_property_add_str(oc, "session-file",
+                                  sev_guest_get_session_file,
+                                  sev_guest_set_session_file);
+    object_class_property_set_description(oc, "session-file",
+            "guest owners session parameters (encoded with base64)");
+    object_class_property_add_bool(oc, "kernel-hashes",
+                                   sev_guest_get_kernel_hashes,
+                                   sev_guest_set_kernel_hashes);
+    object_class_property_set_description(oc, "kernel-hashes",
+            "add kernel hashes to guest firmware for measured Linux boot");
+}
+
+static void
+sev_guest_instance_init(Object *obj)
+{
+    SevGuestState *sev = SEV_GUEST(obj);
+
+    sev->sev_device = g_strdup(DEFAULT_SEV_DEVICE);
+    sev->policy = DEFAULT_GUEST_POLICY;
+    object_property_add_uint32_ptr(obj, "policy", &sev->policy,
+                                   OBJ_PROP_FLAG_READWRITE);
+    object_property_add_uint32_ptr(obj, "handle", &sev->handle,
+                                   OBJ_PROP_FLAG_READWRITE);
+    object_property_add_uint32_ptr(obj, "cbitpos", &sev->cbitpos,
+                                   OBJ_PROP_FLAG_READWRITE);
+    object_property_add_uint32_ptr(obj, "reduced-phys-bits",
+                                   &sev->reduced_phys_bits,
+                                   OBJ_PROP_FLAG_READWRITE);
+}
+
+/* sev guest info */
+static const TypeInfo sev_guest_info = {
+    .parent = TYPE_CONFIDENTIAL_GUEST_SUPPORT,
+    .name = TYPE_SEV_GUEST,
+    .instance_size = sizeof(SevGuestState),
+    .instance_finalize = sev_guest_finalize,
+    .class_init = sev_guest_class_init,
+    .instance_init = sev_guest_instance_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_USER_CREATABLE },
+        { }
+    }
+};
+
 static void
 sev_register_types(void)
 {
diff --git a/target/i386/sev.h b/target/i386/sev.h
index e7499c95b1e8..9e10d09539a7 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -57,6 +57,4 @@ int sev_inject_launch_secret(const char *hdr, const char *secret,
 int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size);
 void sev_es_set_reset_vector(CPUState *cpu);
 
-int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
-
 #endif
-- 
2.34.1

[RFC PATCH 3/4] ppc/pef: switch to use confidential_guest_kvm_init/reset()

[Xiaoyao Li]

Use the unified interface to call confidential guest related kvm_init()
and kvm_reset(), to avoid exposing pef specific functions.

remove perf.h since it is now blank..

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 hw/ppc/pef.c         |  9 ++++++---
 hw/ppc/spapr.c       |  6 +++---
 include/hw/ppc/pef.h | 17 -----------------
 3 files changed, 9 insertions(+), 23 deletions(-)
 delete mode 100644 include/hw/ppc/pef.h

diff --git a/hw/ppc/pef.c b/hw/ppc/pef.c
index d28ed3ba7333..47553348b1e7 100644
--- a/hw/ppc/pef.c
+++ b/hw/ppc/pef.c
@@ -15,7 +15,6 @@
 #include "sysemu/kvm.h"
 #include "migration/blocker.h"
 #include "exec/confidential-guest-support.h"
-#include "hw/ppc/pef.h"
 
 #define TYPE_PEF_GUEST "pef-guest"
 OBJECT_DECLARE_SIMPLE_TYPE(PefGuest, PEF_GUEST)
@@ -93,7 +92,7 @@ static int kvmppc_svm_off(Error **errp)
 #endif
 }
 
-int pef_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
+static int pef_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
 {
     if (!object_dynamic_cast(OBJECT(cgs), TYPE_PEF_GUEST)) {
         return 0;
@@ -107,7 +106,7 @@ int pef_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
     return kvmppc_svm_init(cgs, errp);
 }
 
-int pef_kvm_reset(ConfidentialGuestSupport *cgs, Error **errp)
+static int pef_kvm_reset(ConfidentialGuestSupport *cgs, Error **errp)
 {
     if (!object_dynamic_cast(OBJECT(cgs), TYPE_PEF_GUEST)) {
         return 0;
@@ -131,6 +130,10 @@ OBJECT_DEFINE_TYPE_WITH_INTERFACES(PefGuest,
 
 static void pef_guest_class_init(ObjectClass *oc, void *data)
 {
+    ConfidentialGuestSupportClass *klass = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
+
+    klass->kvm_init = pef_kvm_init;
+    klass->kvm_reset = pef_kvm_reset;
 }
 
 static void pef_guest_init(Object *obj)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0d72d286d80f..d1459157e0cf 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -74,6 +74,7 @@
 #include "hw/virtio/vhost-scsi-common.h"
 
 #include "exec/ram_addr.h"
+#include "exec/confidential-guest-support.h"
 #include "hw/usb.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
@@ -86,7 +87,6 @@
 #include "hw/ppc/spapr_tpm_proxy.h"
 #include "hw/ppc/spapr_nvdimm.h"
 #include "hw/ppc/spapr_numa.h"
-#include "hw/ppc/pef.h"
 
 #include "monitor/monitor.h"
 
@@ -1687,7 +1687,7 @@ static void spapr_machine_reset(MachineState *machine, ShutdownCause reason)
         qemu_guest_getrandom_nofail(spapr->fdt_rng_seed, 32);
     }
 
-    pef_kvm_reset(machine->cgs, &error_fatal);
+    confidential_guest_kvm_reset(machine->cgs, &error_fatal);
     spapr_caps_apply(spapr);
 
     first_ppc_cpu = POWERPC_CPU(first_cpu);
@@ -2811,7 +2811,7 @@ static void spapr_machine_init(MachineState *machine)
     /*
      * if Secure VM (PEF) support is configured, then initialize it
      */
-    pef_kvm_init(machine->cgs, &error_fatal);
+    confidential_guest_kvm_init(machine->cgs, &error_fatal);
 
     msi_nonbroken = true;
 
diff --git a/include/hw/ppc/pef.h b/include/hw/ppc/pef.h
deleted file mode 100644
index 707dbe524c42..000000000000
--- a/include/hw/ppc/pef.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * PEF (Protected Execution Facility) for POWER support
- *
- * Copyright Red Hat.
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#ifndef HW_PPC_PEF_H
-#define HW_PPC_PEF_H
-
-int pef_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
-int pef_kvm_reset(ConfidentialGuestSupport *cgs, Error **errp);
-
-#endif /* HW_PPC_PEF_H */
-- 
2.34.1

[RFC PATCH 4/4] s390: Switch to use confidential_guest_kvm_init()

[Xiaoyao Li]

Use unified confidential_guest_kvm_init(), to avoid exposing specific
functions.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 hw/s390x/s390-virtio-ccw.c |  3 ++-
 target/s390x/kvm/pv.c      |  8 ++++++++
 target/s390x/kvm/pv.h      | 14 --------------
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 62804cc2281d..27096ccd9409 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -14,6 +14,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "exec/ram_addr.h"
+#include "exec/confidential-guest-support.h"
 #include "hw/s390x/s390-virtio-hcall.h"
 #include "hw/s390x/sclp.h"
 #include "hw/s390x/s390_flic.h"
@@ -260,7 +261,7 @@ static void ccw_init(MachineState *machine)
     s390_init_cpus(machine);
 
     /* Need CPU model to be determined before we can set up PV */
-    s390_pv_init(machine->cgs, &error_fatal);
+    confidential_guest_kvm_init(machine->cgs, &error_fatal);
 
     s390_flic_init();
 
diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c
index 7ca7faec73e9..c04d53753bfa 100644
--- a/target/s390x/kvm/pv.c
+++ b/target/s390x/kvm/pv.c
@@ -340,6 +340,11 @@ int s390_pv_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
         return 0;
     }
 
+    if (!kvm_enabled()) {
+        error_setg(errp, "Protected Virtualization requires KVM");
+        return -1;
+    }
+
     if (!s390_has_feat(S390_FEAT_UNPACK)) {
         error_setg(errp,
                    "CPU model does not support Protected Virtualization");
@@ -364,6 +369,9 @@ OBJECT_DEFINE_TYPE_WITH_INTERFACES(S390PVGuest,
 
 static void s390_pv_guest_class_init(ObjectClass *oc, void *data)
 {
+    ConfidentialGuestSupportClass *klass = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
+
+    klass->kvm_init = s390_pv_kvm_init;
 }
 
 static void s390_pv_guest_init(Object *obj)
diff --git a/target/s390x/kvm/pv.h b/target/s390x/kvm/pv.h
index 5877d28ff10a..4b408174391a 100644
--- a/target/s390x/kvm/pv.h
+++ b/target/s390x/kvm/pv.h
@@ -80,18 +80,4 @@ static inline int kvm_s390_dump_mem_state(uint64_t addr, size_t len,
 static inline int kvm_s390_dump_completion_data(void *buff) { return 0; }
 #endif /* CONFIG_KVM */
 
-int s390_pv_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
-static inline int s390_pv_init(ConfidentialGuestSupport *cgs, Error **errp)
-{
-    if (!cgs) {
-        return 0;
-    }
-    if (kvm_enabled()) {
-        return s390_pv_kvm_init(cgs, errp);
-    }
-
-    error_setg(errp, "Protected Virtualization requires KVM");
-    return -1;
-}
-
 #endif /* HW_S390_PV_H */
-- 
2.34.1

Re: [RFC PATCH 1/4] confidential guest support: Add kvm_init() and kvm_reset() in class

[Daniel P. Berrangé]

On Tue, Feb 06, 2024 at 03:28:49AM -0500, Xiaoyao Li wrote:
> Different confidential VMs in different architectures all have the same
> needs to do their specific initialization (and maybe resetting) stuffs
> with KVM. Currently each of them exposes individual *_kvm_init()
> functions and let machine code or kvm code to call it.
> 
> To make it more object oriented, add two virtual functions, kvm_init()
> and kvm_reset() in ConfidentialGuestSupportClass, and expose two helpers
> functions for invodking them.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  include/exec/confidential-guest-support.h | 42 ++++++++++++++++++++++-
>  1 file changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/include/exec/confidential-guest-support.h b/include/exec/confidential-guest-support.h
> index ba2dd4b5dfc4..ff0bfb26ad7a 100644
> --- a/include/exec/confidential-guest-support.h
> +++ b/include/exec/confidential-guest-support.h
> @@ -23,7 +23,10 @@
>  #include "qom/object.h"
>  
>  #define TYPE_CONFIDENTIAL_GUEST_SUPPORT "confidential-guest-support"
> -OBJECT_DECLARE_SIMPLE_TYPE(ConfidentialGuestSupport, CONFIDENTIAL_GUEST_SUPPORT)
> +OBJECT_DECLARE_TYPE(ConfidentialGuestSupport,
> +                    ConfidentialGuestSupportClass,
> +                    CONFIDENTIAL_GUEST_SUPPORT)
> +
>  
>  struct ConfidentialGuestSupport {
>      Object parent;
> @@ -55,8 +58,45 @@ struct ConfidentialGuestSupport {
>  
>  typedef struct ConfidentialGuestSupportClass {
>      ObjectClass parent;
> +
> +    int (*kvm_init)(ConfidentialGuestSupport *cgs, Error **errp);
> +    int (*kvm_reset)(ConfidentialGuestSupport *cgs, Error **errp);
>  } ConfidentialGuestSupportClass;
>  
> +static inline int confidential_guest_kvm_init(ConfidentialGuestSupport *cgs,
> +                                              Error **errp)
> +{
> +    ConfidentialGuestSupportClass *klass;
> +
> +    if (!cgs) {
> +        return 0;
> +    }

Typically I would expect any class/object methods to mandate a non-NULL
class/instance pointer.

IOW, the caller would generally be expected to check NULL and not
call this method in that case.

> +
> +    klass = CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(cgs);
> +    if (klass->kvm_init) {
> +        return klass->kvm_init(cgs, errp);
> +    }
> +
> +    return 0;
> +}
> +
> +static inline int confidential_guest_kvm_reset(ConfidentialGuestSupport *cgs,
> +                                               Error **errp)
> +{
> +    ConfidentialGuestSupportClass *klass;
> +
> +    if (!cgs) {
> +        return 0;
> +    }
> +
> +    klass = CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(cgs);
> +    if (klass->kvm_reset) {
> +        return klass->kvm_reset(cgs, errp);
> +    }
> +
> +    return 0;
> +}
> +
>  #endif /* !CONFIG_USER_ONLY */
>  
>  #endif /* QEMU_CONFIDENTIAL_GUEST_SUPPORT_H */
> -- 
> 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 :|

Re: [RFC PATCH 2/4] i386/sev: Switch to use confidential_guest_kvm_init()

[Daniel P. Berrangé]

On Tue, Feb 06, 2024 at 03:28:50AM -0500, Xiaoyao Li wrote:
> Use confidential_guest_kvm_init() instead of calling SEV specific
> sev_kvm_init(). As a bouns, it fits to future TDX when TDX implements
> its own confidential_guest_support and .kvm_init().
> 
> Move the "TypeInfo sev_guest_info" definition and related functions to
> the end of the file, to avoid declaring the sev_kvm_init() ahead.
> 
> Clean up the sve-stub.c since it's not needed anymore.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  target/i386/kvm/kvm.c       |   2 +-
>  target/i386/kvm/meson.build |   2 -
>  target/i386/kvm/sev-stub.c  |   5 --
>  target/i386/sev.c           | 120 +++++++++++++++++++-----------------
>  target/i386/sev.h           |   2 -
>  5 files changed, 63 insertions(+), 68 deletions(-)
> 
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 76a66246eb72..bb63bba61fa1 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -2534,7 +2534,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>       * mechanisms are supported in future (e.g. TDX), they'll need
>       * their own initialization either here or elsewhere.
>       */
> -    ret = sev_kvm_init(ms->cgs, &local_err);
> +    ret = confidential_guest_kvm_init(ms->cgs, &local_err);

If you agree with my comment in patch 1 about the API expecting non-NULL,
then this would need to be conditionalized (same for the 2 following
patches too)

   if (ms->cgs) {
      ret = confidential_guest_kvm_init(....)
      if (ret < 0) {
         ....
      }
   }

>      if (ret < 0) {
>          error_report_err(local_err);
>          return ret;
> diff --git a/target/i386/kvm/meson.build b/target/i386/kvm/meson.build
> index 84d9143e6029..e7850981e62d 100644
> --- a/target/i386/kvm/meson.build
> +++ b/target/i386/kvm/meson.build
> @@ -7,8 +7,6 @@ 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_SEV', if_false: files('sev-stub.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/sev-stub.c b/target/i386/kvm/sev-stub.c
> index 1be5341e8a6a..4a1560cf8ad7 100644
> --- a/target/i386/kvm/sev-stub.c
> +++ b/target/i386/kvm/sev-stub.c
> @@ -14,8 +14,3 @@
>  #include "qemu/osdep.h"
>  #include "sev.h"
>  
> -int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> -{
> -    /* If we get here, cgs must be some non-SEV thing */
> -    return 0;
> -}

You can actually delete this entire file, since you removed the
only method in it, and stopped building it in the meson.build
patch above.

> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 173de91afe7d..19e79d3631d0 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -353,63 +353,6 @@ static void sev_guest_set_kernel_hashes(Object *obj, bool value, Error **errp)
>      sev->kernel_hashes = value;
>  }
>  
> -static void
> -sev_guest_class_init(ObjectClass *oc, void *data)
> -{
> -    object_class_property_add_str(oc, "sev-device",
> -                                  sev_guest_get_sev_device,
> -                                  sev_guest_set_sev_device);
> -    object_class_property_set_description(oc, "sev-device",
> -            "SEV device to use");
> -    object_class_property_add_str(oc, "dh-cert-file",
> -                                  sev_guest_get_dh_cert_file,
> -                                  sev_guest_set_dh_cert_file);
> -    object_class_property_set_description(oc, "dh-cert-file",
> -            "guest owners DH certificate (encoded with base64)");
> -    object_class_property_add_str(oc, "session-file",
> -                                  sev_guest_get_session_file,
> -                                  sev_guest_set_session_file);
> -    object_class_property_set_description(oc, "session-file",
> -            "guest owners session parameters (encoded with base64)");
> -    object_class_property_add_bool(oc, "kernel-hashes",
> -                                   sev_guest_get_kernel_hashes,
> -                                   sev_guest_set_kernel_hashes);
> -    object_class_property_set_description(oc, "kernel-hashes",
> -            "add kernel hashes to guest firmware for measured Linux boot");
> -}
> -
> -static void
> -sev_guest_instance_init(Object *obj)
> -{
> -    SevGuestState *sev = SEV_GUEST(obj);
> -
> -    sev->sev_device = g_strdup(DEFAULT_SEV_DEVICE);
> -    sev->policy = DEFAULT_GUEST_POLICY;
> -    object_property_add_uint32_ptr(obj, "policy", &sev->policy,
> -                                   OBJ_PROP_FLAG_READWRITE);
> -    object_property_add_uint32_ptr(obj, "handle", &sev->handle,
> -                                   OBJ_PROP_FLAG_READWRITE);
> -    object_property_add_uint32_ptr(obj, "cbitpos", &sev->cbitpos,
> -                                   OBJ_PROP_FLAG_READWRITE);
> -    object_property_add_uint32_ptr(obj, "reduced-phys-bits",
> -                                   &sev->reduced_phys_bits,
> -                                   OBJ_PROP_FLAG_READWRITE);
> -}
> -
> -/* sev guest info */
> -static const TypeInfo sev_guest_info = {
> -    .parent = TYPE_CONFIDENTIAL_GUEST_SUPPORT,
> -    .name = TYPE_SEV_GUEST,
> -    .instance_size = sizeof(SevGuestState),
> -    .instance_finalize = sev_guest_finalize,
> -    .class_init = sev_guest_class_init,
> -    .instance_init = sev_guest_instance_init,
> -    .interfaces = (InterfaceInfo[]) {
> -        { TYPE_USER_CREATABLE },
> -        { }
> -    }
> -};
> -
>  bool
>  sev_enabled(void)
>  {
> @@ -906,7 +849,7 @@ sev_vm_state_change(void *opaque, bool running, RunState state)
>      }
>  }
>  
> -int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> +static int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>  {
>      SevGuestState *sev
>          = (SevGuestState *)object_dynamic_cast(OBJECT(cgs), TYPE_SEV_GUEST);
> @@ -1383,6 +1326,67 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
>      return ret;
>  }
>  
> +static void
> +sev_guest_class_init(ObjectClass *oc, void *data)
> +{
> +    ConfidentialGuestSupportClass *klass = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
> +
> +    klass->kvm_init = sev_kvm_init;
> +
> +    object_class_property_add_str(oc, "sev-device",
> +                                  sev_guest_get_sev_device,
> +                                  sev_guest_set_sev_device);
> +    object_class_property_set_description(oc, "sev-device",
> +            "SEV device to use");
> +    object_class_property_add_str(oc, "dh-cert-file",
> +                                  sev_guest_get_dh_cert_file,
> +                                  sev_guest_set_dh_cert_file);
> +    object_class_property_set_description(oc, "dh-cert-file",
> +            "guest owners DH certificate (encoded with base64)");
> +    object_class_property_add_str(oc, "session-file",
> +                                  sev_guest_get_session_file,
> +                                  sev_guest_set_session_file);
> +    object_class_property_set_description(oc, "session-file",
> +            "guest owners session parameters (encoded with base64)");
> +    object_class_property_add_bool(oc, "kernel-hashes",
> +                                   sev_guest_get_kernel_hashes,
> +                                   sev_guest_set_kernel_hashes);
> +    object_class_property_set_description(oc, "kernel-hashes",
> +            "add kernel hashes to guest firmware for measured Linux boot");
> +}
> +
> +static void
> +sev_guest_instance_init(Object *obj)
> +{
> +    SevGuestState *sev = SEV_GUEST(obj);
> +
> +    sev->sev_device = g_strdup(DEFAULT_SEV_DEVICE);
> +    sev->policy = DEFAULT_GUEST_POLICY;
> +    object_property_add_uint32_ptr(obj, "policy", &sev->policy,
> +                                   OBJ_PROP_FLAG_READWRITE);
> +    object_property_add_uint32_ptr(obj, "handle", &sev->handle,
> +                                   OBJ_PROP_FLAG_READWRITE);
> +    object_property_add_uint32_ptr(obj, "cbitpos", &sev->cbitpos,
> +                                   OBJ_PROP_FLAG_READWRITE);
> +    object_property_add_uint32_ptr(obj, "reduced-phys-bits",
> +                                   &sev->reduced_phys_bits,
> +                                   OBJ_PROP_FLAG_READWRITE);
> +}
> +
> +/* sev guest info */
> +static const TypeInfo sev_guest_info = {
> +    .parent = TYPE_CONFIDENTIAL_GUEST_SUPPORT,
> +    .name = TYPE_SEV_GUEST,
> +    .instance_size = sizeof(SevGuestState),
> +    .instance_finalize = sev_guest_finalize,
> +    .class_init = sev_guest_class_init,
> +    .instance_init = sev_guest_instance_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_USER_CREATABLE },
> +        { }
> +    }
> +};
> +
>  static void
>  sev_register_types(void)
>  {
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index e7499c95b1e8..9e10d09539a7 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -57,6 +57,4 @@ int sev_inject_launch_secret(const char *hdr, const char *secret,
>  int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size);
>  void sev_es_set_reset_vector(CPUState *cpu);
>  
> -int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
> -
>  #endif
> -- 
> 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 :|

Re: [RFC PATCH 0/4] Confidential Guest Support: Introduce kvm_init() and kvm_reset() virtual functions

[Daniel P. Berrangé]

On Tue, Feb 06, 2024 at 03:28:48AM -0500, Xiaoyao Li wrote:
> This series is inspired and suggested by Daniel:
> https://lore.kernel.org/qemu-devel/ZbfoQsEuv6_zwl3b@redhat.com/
> 
> Currently, different confidential VMs in different architectures have
> their own specific *_kvm_init() (and some have *_kvm_reset()) exposed
> for KVM stuff when it's a confidential VM. e.g., sev_kmv_init() for x86
> SEV, pef_kvm_init() and pef_kvm_reset() for PPC PEF, and s390_pv_init()
> for s390 PV VMs.
> 
> Introduce a generic .kvm_init() and .kvm_reset() functions in
> ConfidentialGuestSupportClass, so that different cgs technologies in
> different architectures can implement their own, while common interface
> of cgs can be used.
> 
> This RFC implements two helper functions confidential_guest_kvm_init()
> and confidential_guest_kvm_reset() in Patch 1. In the following patches,
> they are called in arch specific implementation. X86 will benefit more
> for the generic implementation when TDX support is added.
> 
> There is one step forward possible, that calling
> confidential_guest_kvm_init() before kvm_arch_init() in kvm_int() in
> accel/kvm/kvm-all.c. This way, each arch doesn't need to call in their
> arch specific code.
> 
> X86 fits it, however I'm not sure if ppc and s390 fit it as well.
> Because currently, ppc calls it in machine->init()
> and s390 calls in MachineClass->init(). I'm not sure if there is any
> order dependency.

IIUC that s390 call is still a machine->init method, rather than
class init.

I think this series is nice, but its up to the KVM maintainers
to decide...


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: [RFC PATCH 2/4] i386/sev: Switch to use confidential_guest_kvm_init()

[Xiaoyao Li]

On 2/6/2024 10:16 PM, Daniel P. Berrangé wrote:
> On Tue, Feb 06, 2024 at 03:28:50AM -0500, Xiaoyao Li wrote:
>> Use confidential_guest_kvm_init() instead of calling SEV specific
>> sev_kvm_init(). As a bouns, it fits to future TDX when TDX implements
>> its own confidential_guest_support and .kvm_init().
>>
>> Move the "TypeInfo sev_guest_info" definition and related functions to
>> the end of the file, to avoid declaring the sev_kvm_init() ahead.
>>
>> Clean up the sve-stub.c since it's not needed anymore.
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>>   target/i386/kvm/kvm.c       |   2 +-
>>   target/i386/kvm/meson.build |   2 -
>>   target/i386/kvm/sev-stub.c  |   5 --
>>   target/i386/sev.c           | 120 +++++++++++++++++++-----------------
>>   target/i386/sev.h           |   2 -
>>   5 files changed, 63 insertions(+), 68 deletions(-)
>>
>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
>> index 76a66246eb72..bb63bba61fa1 100644
>> --- a/target/i386/kvm/kvm.c
>> +++ b/target/i386/kvm/kvm.c
>> @@ -2534,7 +2534,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>>        * mechanisms are supported in future (e.g. TDX), they'll need
>>        * their own initialization either here or elsewhere.
>>        */
>> -    ret = sev_kvm_init(ms->cgs, &local_err);
>> +    ret = confidential_guest_kvm_init(ms->cgs, &local_err);
> 
> If you agree with my comment in patch 1 about the API expecting non-NULL,
> then this would need to be conditionalized (same for the 2 following
> patches too)

sure. Will change.

>     if (ms->cgs) {
>        ret = confidential_guest_kvm_init(....)
>        if (ret < 0) {
>           ....
>        }
>     }
> 
>>       if (ret < 0) {
>>           error_report_err(local_err);
>>           return ret;
>> diff --git a/target/i386/kvm/meson.build b/target/i386/kvm/meson.build
>> index 84d9143e6029..e7850981e62d 100644
>> --- a/target/i386/kvm/meson.build
>> +++ b/target/i386/kvm/meson.build
>> @@ -7,8 +7,6 @@ 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_SEV', if_false: files('sev-stub.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/sev-stub.c b/target/i386/kvm/sev-stub.c
>> index 1be5341e8a6a..4a1560cf8ad7 100644
>> --- a/target/i386/kvm/sev-stub.c
>> +++ b/target/i386/kvm/sev-stub.c
>> @@ -14,8 +14,3 @@
>>   #include "qemu/osdep.h"
>>   #include "sev.h"
>>   
>> -int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>> -{
>> -    /* If we get here, cgs must be some non-SEV thing */
>> -    return 0;
>> -}
> 
> You can actually delete this entire file, since you removed the
> only method in it, and stopped building it in the meson.build
> patch above.

I intented to do it. Apprarently I missed it somehow and didn't catch it 
before sending out.

will fix in next version.

>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>> index 173de91afe7d..19e79d3631d0 100644
>> --- a/target/i386/sev.c
>> +++ b/target/i386/sev.c
>> @@ -353,63 +353,6 @@ static void sev_guest_set_kernel_hashes(Object *obj, bool value, Error **errp)
>>       sev->kernel_hashes = value;
>>   }
>>   
>> -static void
>> -sev_guest_class_init(ObjectClass *oc, void *data)
>> -{
>> -    object_class_property_add_str(oc, "sev-device",
>> -                                  sev_guest_get_sev_device,
>> -                                  sev_guest_set_sev_device);
>> -    object_class_property_set_description(oc, "sev-device",
>> -            "SEV device to use");
>> -    object_class_property_add_str(oc, "dh-cert-file",
>> -                                  sev_guest_get_dh_cert_file,
>> -                                  sev_guest_set_dh_cert_file);
>> -    object_class_property_set_description(oc, "dh-cert-file",
>> -            "guest owners DH certificate (encoded with base64)");
>> -    object_class_property_add_str(oc, "session-file",
>> -                                  sev_guest_get_session_file,
>> -                                  sev_guest_set_session_file);
>> -    object_class_property_set_description(oc, "session-file",
>> -            "guest owners session parameters (encoded with base64)");
>> -    object_class_property_add_bool(oc, "kernel-hashes",
>> -                                   sev_guest_get_kernel_hashes,
>> -                                   sev_guest_set_kernel_hashes);
>> -    object_class_property_set_description(oc, "kernel-hashes",
>> -            "add kernel hashes to guest firmware for measured Linux boot");
>> -}
>> -
>> -static void
>> -sev_guest_instance_init(Object *obj)
>> -{
>> -    SevGuestState *sev = SEV_GUEST(obj);
>> -
>> -    sev->sev_device = g_strdup(DEFAULT_SEV_DEVICE);
>> -    sev->policy = DEFAULT_GUEST_POLICY;
>> -    object_property_add_uint32_ptr(obj, "policy", &sev->policy,
>> -                                   OBJ_PROP_FLAG_READWRITE);
>> -    object_property_add_uint32_ptr(obj, "handle", &sev->handle,
>> -                                   OBJ_PROP_FLAG_READWRITE);
>> -    object_property_add_uint32_ptr(obj, "cbitpos", &sev->cbitpos,
>> -                                   OBJ_PROP_FLAG_READWRITE);
>> -    object_property_add_uint32_ptr(obj, "reduced-phys-bits",
>> -                                   &sev->reduced_phys_bits,
>> -                                   OBJ_PROP_FLAG_READWRITE);
>> -}
>> -
>> -/* sev guest info */
>> -static const TypeInfo sev_guest_info = {
>> -    .parent = TYPE_CONFIDENTIAL_GUEST_SUPPORT,
>> -    .name = TYPE_SEV_GUEST,
>> -    .instance_size = sizeof(SevGuestState),
>> -    .instance_finalize = sev_guest_finalize,
>> -    .class_init = sev_guest_class_init,
>> -    .instance_init = sev_guest_instance_init,
>> -    .interfaces = (InterfaceInfo[]) {
>> -        { TYPE_USER_CREATABLE },
>> -        { }
>> -    }
>> -};
>> -
>>   bool
>>   sev_enabled(void)
>>   {
>> @@ -906,7 +849,7 @@ sev_vm_state_change(void *opaque, bool running, RunState state)
>>       }
>>   }
>>   
>> -int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>> +static int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>>   {
>>       SevGuestState *sev
>>           = (SevGuestState *)object_dynamic_cast(OBJECT(cgs), TYPE_SEV_GUEST);
>> @@ -1383,6 +1326,67 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
>>       return ret;
>>   }
>>   
>> +static void
>> +sev_guest_class_init(ObjectClass *oc, void *data)
>> +{
>> +    ConfidentialGuestSupportClass *klass = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>> +
>> +    klass->kvm_init = sev_kvm_init;
>> +
>> +    object_class_property_add_str(oc, "sev-device",
>> +                                  sev_guest_get_sev_device,
>> +                                  sev_guest_set_sev_device);
>> +    object_class_property_set_description(oc, "sev-device",
>> +            "SEV device to use");
>> +    object_class_property_add_str(oc, "dh-cert-file",
>> +                                  sev_guest_get_dh_cert_file,
>> +                                  sev_guest_set_dh_cert_file);
>> +    object_class_property_set_description(oc, "dh-cert-file",
>> +            "guest owners DH certificate (encoded with base64)");
>> +    object_class_property_add_str(oc, "session-file",
>> +                                  sev_guest_get_session_file,
>> +                                  sev_guest_set_session_file);
>> +    object_class_property_set_description(oc, "session-file",
>> +            "guest owners session parameters (encoded with base64)");
>> +    object_class_property_add_bool(oc, "kernel-hashes",
>> +                                   sev_guest_get_kernel_hashes,
>> +                                   sev_guest_set_kernel_hashes);
>> +    object_class_property_set_description(oc, "kernel-hashes",
>> +            "add kernel hashes to guest firmware for measured Linux boot");
>> +}
>> +
>> +static void
>> +sev_guest_instance_init(Object *obj)
>> +{
>> +    SevGuestState *sev = SEV_GUEST(obj);
>> +
>> +    sev->sev_device = g_strdup(DEFAULT_SEV_DEVICE);
>> +    sev->policy = DEFAULT_GUEST_POLICY;
>> +    object_property_add_uint32_ptr(obj, "policy", &sev->policy,
>> +                                   OBJ_PROP_FLAG_READWRITE);
>> +    object_property_add_uint32_ptr(obj, "handle", &sev->handle,
>> +                                   OBJ_PROP_FLAG_READWRITE);
>> +    object_property_add_uint32_ptr(obj, "cbitpos", &sev->cbitpos,
>> +                                   OBJ_PROP_FLAG_READWRITE);
>> +    object_property_add_uint32_ptr(obj, "reduced-phys-bits",
>> +                                   &sev->reduced_phys_bits,
>> +                                   OBJ_PROP_FLAG_READWRITE);
>> +}
>> +
>> +/* sev guest info */
>> +static const TypeInfo sev_guest_info = {
>> +    .parent = TYPE_CONFIDENTIAL_GUEST_SUPPORT,
>> +    .name = TYPE_SEV_GUEST,
>> +    .instance_size = sizeof(SevGuestState),
>> +    .instance_finalize = sev_guest_finalize,
>> +    .class_init = sev_guest_class_init,
>> +    .instance_init = sev_guest_instance_init,
>> +    .interfaces = (InterfaceInfo[]) {
>> +        { TYPE_USER_CREATABLE },
>> +        { }
>> +    }
>> +};
>> +
>>   static void
>>   sev_register_types(void)
>>   {
>> diff --git a/target/i386/sev.h b/target/i386/sev.h
>> index e7499c95b1e8..9e10d09539a7 100644
>> --- a/target/i386/sev.h
>> +++ b/target/i386/sev.h
>> @@ -57,6 +57,4 @@ int sev_inject_launch_secret(const char *hdr, const char *secret,
>>   int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size);
>>   void sev_es_set_reset_vector(CPUState *cpu);
>>   
>> -int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
>> -
>>   #endif
>> -- 
>> 2.34.1
>>
> 
> With regards,
> Daniel

Re: [RFC PATCH 0/4] Confidential Guest Support: Introduce kvm_init() and kvm_reset() virtual functions

[Xiaoyao Li]

On 2/6/2024 10:19 PM, Daniel P. Berrangé wrote:
> On Tue, Feb 06, 2024 at 03:28:48AM -0500, Xiaoyao Li wrote:
>> This series is inspired and suggested by Daniel:
>> https://lore.kernel.org/qemu-devel/ZbfoQsEuv6_zwl3b@redhat.com/
>>
>> Currently, different confidential VMs in different architectures have
>> their own specific *_kvm_init() (and some have *_kvm_reset()) exposed
>> for KVM stuff when it's a confidential VM. e.g., sev_kmv_init() for x86
>> SEV, pef_kvm_init() and pef_kvm_reset() for PPC PEF, and s390_pv_init()
>> for s390 PV VMs.
>>
>> Introduce a generic .kvm_init() and .kvm_reset() functions in
>> ConfidentialGuestSupportClass, so that different cgs technologies in
>> different architectures can implement their own, while common interface
>> of cgs can be used.
>>
>> This RFC implements two helper functions confidential_guest_kvm_init()
>> and confidential_guest_kvm_reset() in Patch 1. In the following patches,
>> they are called in arch specific implementation. X86 will benefit more
>> for the generic implementation when TDX support is added.
>>
>> There is one step forward possible, that calling
>> confidential_guest_kvm_init() before kvm_arch_init() in kvm_int() in
>> accel/kvm/kvm-all.c. This way, each arch doesn't need to call in their
>> arch specific code.
>>
>> X86 fits it, however I'm not sure if ppc and s390 fit it as well.
>> Because currently, ppc calls it in machine->init()
>> and s390 calls in MachineClass->init(). I'm not sure if there is any
>> order dependency.
> 
> IIUC that s390 call is still a machine->init method, rather than
> class init.

I double check the code again. Only struct MachineClass has .init() 
function defined. And I find both ppc and s390 calls the 
confidential_guest_kvm_init() (or their specific cgs kvm_init()) inside 
their machine_class->init().

> I think this series is nice, but its up to the KVM maintainers
> to decide...
> 
> 
> With regards,
> Daniel