[RFC,v2,12/18] guest memory protection: Perform KVM init via interface

Commit Message

David Gibson May 21, 2020, 3:42 a.m. UTC

Currently the "memory-encryption" machine option is notionally generic,
but in fact is only used for AMD SEV setups.  Make another step towards it
being actually generic, but having using the GuestMemoryProtection QOM
interface to dispatch the initial setup, rather than directly calling
sev_guest_init() from kvm_init().

Signed-off-by: David Gibson <[e-mail]>
---
 accel/kvm/kvm-all.c                    | 18 ++++++++++---
 include/exec/guest-memory-protection.h |  1 +
 target/i386/sev.c                      | 37 ++++----------------------
 3 files changed, 21 insertions(+), 35 deletions(-)

Comments

Richard Henderson June 2, 2020, 3:39 a.m. UTC | #1

On 5/20/20 8:42 PM, David Gibson wrote:
> +        if (object_dynamic_cast(obj, TYPE_GUEST_MEMORY_PROTECTION)) {
> +            GuestMemoryProtection *gmpo = GUEST_MEMORY_PROTECTION(obj);

This duplicates the interface check.  You should use

  gmpo = (GuestMemoryProtection *)
    object_dynamic_cast(obj, TYPE_GUEST_MEMORY_PROTECTION);
  if (gmpo) {

AFICT.


r~

Richard Henderson June 2, 2020, 3:45 a.m. UTC | #2

On 6/1/20 8:39 PM, Richard Henderson wrote:
> On 5/20/20 8:42 PM, David Gibson wrote:
>> +        if (object_dynamic_cast(obj, TYPE_GUEST_MEMORY_PROTECTION)) {
>> +            GuestMemoryProtection *gmpo = GUEST_MEMORY_PROTECTION(obj);
> 
> This duplicates the interface check.  You should use
> 
>   gmpo = (GuestMemoryProtection *)
>     object_dynamic_cast(obj, TYPE_GUEST_MEMORY_PROTECTION);
>   if (gmpo) {
> 
> AFICT.

Or ignore this nit, since you clean it up in patch 14.

Reviewed-by: Richard Henderson <[e-mail]>

r~

Patch

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 40997de38c..5cf1a397e3 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -39,7 +39,6 @@ 
 #include "qemu/main-loop.h"
 #include "trace.h"
 #include "hw/irq.h"
-#include "sysemu/sev.h"
 #include "sysemu/balloon.h"
 #include "qapi/visitor.h"
 #include "qapi/qapi-types-common.h"
@@ -2104,8 +2103,21 @@  static int kvm_init(MachineState *ms)
      * encryption context.
      */
     if (ms->memory_encryption) {
-        kvm_state->guest_memory_protection = sev_guest_init(ms->memory_encryption);
-        if (!kvm_state->guest_memory_protection) {
+        Object *obj = object_resolve_path_component(object_get_objects_root(),
+                                                    ms->memory_encryption);
+
+        if (object_dynamic_cast(obj, TYPE_GUEST_MEMORY_PROTECTION)) {
+            GuestMemoryProtection *gmpo = GUEST_MEMORY_PROTECTION(obj);
+            GuestMemoryProtectionClass *gmpc =
+                GUEST_MEMORY_PROTECTION_GET_CLASS(gmpo);
+
+            ret = gmpc->kvm_init(gmpo);
+            if (ret < 0) {
+                goto err;
+            }
+
+            kvm_state->guest_memory_protection = gmpo;
+        } else {
             ret = -1;
             goto err;
         }
diff --git a/include/exec/guest-memory-protection.h b/include/exec/guest-memory-protection.h
index eb712a5804..3707b96515 100644
--- a/include/exec/guest-memory-protection.h
+++ b/include/exec/guest-memory-protection.h
@@ -31,6 +31,7 @@  typedef struct GuestMemoryProtection GuestMemoryProtection;
 typedef struct GuestMemoryProtectionClass {
     InterfaceClass parent;
 
+    int (*kvm_init)(GuestMemoryProtection *);
     int (*encrypt_data)(GuestMemoryProtection *, uint8_t *, uint64_t);
 } GuestMemoryProtectionClass;
 
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 986c2fee51..60e9d8c735 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -300,26 +300,6 @@  sev_guest_instance_init(Object *obj)
                                    OBJ_PROP_FLAG_READWRITE);
 }
 
-static SevGuestState *
-lookup_sev_guest_info(const char *id)
-{
-    Object *obj;
-    SevGuestState *info;
-
-    obj = object_resolve_path_component(object_get_objects_root(), id);
-    if (!obj) {
-        return NULL;
-    }
-
-    info = (SevGuestState *)
-            object_dynamic_cast(obj, TYPE_SEV_GUEST);
-    if (!info) {
-        return NULL;
-    }
-
-    return info;
-}
-
 bool
 sev_enabled(void)
 {
@@ -637,23 +617,15 @@  sev_vm_state_change(void *opaque, int running, RunState state)
     }
 }
 
-GuestMemoryProtection *
-sev_guest_init(const char *id)
+static int sev_kvm_init(GuestMemoryProtection *gmpo)
 {
-    SevGuestState *sev;
+    SevGuestState *sev = SEV_GUEST(gmpo);
     char *devname;
     int ret, fw_error;
     uint32_t ebx;
     uint32_t host_cbitpos;
     struct sev_user_data_status status = {};
 
-    sev = lookup_sev_guest_info(id);
-    if (!sev) {
-        error_report("%s: '%s' is not a valid '%s' object",
-                     __func__, id, TYPE_SEV_GUEST);
-        goto err;
-    }
-
     sev_guest = sev;
     sev->state = SEV_STATE_UNINIT;
 
@@ -715,10 +687,10 @@  sev_guest_init(const char *id)
     qemu_add_machine_init_done_notifier(&sev_machine_done_notify);
     qemu_add_vm_change_state_handler(sev_vm_state_change, sev);
 
-    return GUEST_MEMORY_PROTECTION(sev);
+    return 0;
 err:
     sev_guest = NULL;
-    return NULL;
+    return -1;
 }
 
 static int
@@ -757,6 +729,7 @@  sev_guest_class_init(ObjectClass *oc, void *data)
     object_class_property_set_description(oc, "session-file",
         "guest owners session parameters (encoded with base64)");
 
+    gmpc->kvm_init = sev_kvm_init;
     gmpc->encrypt_data = sev_encrypt_data;
 }