diff mbox series

[v4,13/66] target/i386: Introduce kvm_confidential_guest_init()

Message ID 20240125032328.2522472-14-xiaoyao.li@intel.com (mailing list archive)
State New, archived
Headers show
Series QEMU Guest memfd + QEMU TDX support | expand

Commit Message

Xiaoyao Li Jan. 25, 2024, 3:22 a.m. UTC 
Introduce a separate function kvm_confidential_guest_init(), which
dispatches specific confidential guest initialization function by
ms->cgs type.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/i386/kvm/kvm.c | 11 ++++++++++-
 target/i386/sev.c     |  1 -
 target/i386/sev.h     |  2 ++
 3 files changed, 12 insertions(+), 2 deletions(-)

Comments

Daniel P. Berrangé Jan. 29, 2024, 6:02 p.m. UTC | #1 
On Wed, Jan 24, 2024 at 10:22:35PM -0500, Xiaoyao Li wrote:
> Introduce a separate function kvm_confidential_guest_init(), which
> dispatches specific confidential guest initialization function by
> ms->cgs type.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/i386/kvm/kvm.c | 11 ++++++++++-
>  target/i386/sev.c     |  1 -
>  target/i386/sev.h     |  2 ++
>  3 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index c961846777cc..f9a774925cf6 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -2541,6 +2541,15 @@ int kvm_arch_get_default_type(MachineState *ms)
>      return 0;
>  }
>  
> +static int kvm_confidential_guest_init(MachineState *ms, Error **errp)
> +{
> +    if (object_dynamic_cast(OBJECT(ms->cgs), TYPE_SEV_GUEST)) {
> +        return sev_kvm_init(ms->cgs, errp);
> +    }
> +
> +    return 0;
> +}

if/else ladders checking object subclass type and then invoking a
subclass specific method are quite an object oriented code
anti-pattern. I think this suggests that ConfidentialGuestSupportClass
should gain a member

   void (*kvm_init)(ConfidentialGuestSpport *cgs, Error **errp);

and then an impl

   void confidential_guest_kvm_init(ConfidentialGuestSupport *cgs, Error *errp) {
       ConfidentialGuestSupportClass *klass = CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(cgs);
       klass->kvm_init(cgs, errp)
   }

that way, kvm.c arch code doesn't get directly bound to the APIs
for the different CVM technologies, the ConfidentialGuestSupport
object will act as a proper isolation layer. 

This is likely to apply in other parts of KVM code that need to
call into SEV/TDX specific functions too.

> +
>  int kvm_arch_init(MachineState *ms, KVMState *s)
>  {
>      uint64_t identity_base = 0xfffbc000;
> @@ -2561,7 +2570,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 = kvm_confidential_guest_init(ms, &local_err);
>      if (ret < 0) {
>          error_report_err(local_err);
>          return ret;
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 173de91afe7d..27d58702d6dc 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -39,7 +39,6 @@
>  #include "hw/i386/pc.h"
>  #include "exec/address-spaces.h"
>  
> -#define TYPE_SEV_GUEST "sev-guest"
>  OBJECT_DECLARE_SIMPLE_TYPE(SevGuestState, SEV_GUEST)
>  
>  
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index e7499c95b1e8..1fe25d096dc4 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -20,6 +20,8 @@
>  
>  #include "exec/confidential-guest-support.h"
>  
> +#define TYPE_SEV_GUEST "sev-guest"
> +
>  #define SEV_POLICY_NODBG        0x1
>  #define SEV_POLICY_NOKS         0x2
>  #define SEV_POLICY_ES           0x4
> -- 
> 2.34.1
> 

With regards,
Daniel
diff mbox series

Patch

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index c961846777cc..f9a774925cf6 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2541,6 +2541,15 @@  int kvm_arch_get_default_type(MachineState *ms)
     return 0;
 }
 
+static int kvm_confidential_guest_init(MachineState *ms, Error **errp)
+{
+    if (object_dynamic_cast(OBJECT(ms->cgs), TYPE_SEV_GUEST)) {
+        return sev_kvm_init(ms->cgs, errp);
+    }
+
+    return 0;
+}
+
 int kvm_arch_init(MachineState *ms, KVMState *s)
 {
     uint64_t identity_base = 0xfffbc000;
@@ -2561,7 +2570,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 = kvm_confidential_guest_init(ms, &local_err);
     if (ret < 0) {
         error_report_err(local_err);
         return ret;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 173de91afe7d..27d58702d6dc 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -39,7 +39,6 @@ 
 #include "hw/i386/pc.h"
 #include "exec/address-spaces.h"
 
-#define TYPE_SEV_GUEST "sev-guest"
 OBJECT_DECLARE_SIMPLE_TYPE(SevGuestState, SEV_GUEST)
 
 
diff --git a/target/i386/sev.h b/target/i386/sev.h
index e7499c95b1e8..1fe25d096dc4 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -20,6 +20,8 @@ 
 
 #include "exec/confidential-guest-support.h"
 
+#define TYPE_SEV_GUEST "sev-guest"
+
 #define SEV_POLICY_NODBG        0x1
 #define SEV_POLICY_NOKS         0x2
 #define SEV_POLICY_ES           0x4