x86, kvm: common confidential computing subset

[PATCH 03/26] confidential guest support: Add kvm_init() and kvm_reset() in class

Posted by Paolo Bonzini 1 year, 10 months ago

From: Xiaoyao Li <xiaoyao.li@intel.com>

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 facilitate the introduction of confidential guest technology from
different x86 vendors, 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>
Message-Id: <20240229060038.606591-1-xiaoyao.li@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/confidential-guest-support.h | 34 ++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/include/exec/confidential-guest-support.h b/include/exec/confidential-guest-support.h
index ba2dd4b5dfc..e5b188cffbf 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,37 @@ 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;
+
+    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;
+
+    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.44.0

Re: [PATCH 03/26] confidential guest support: Add kvm_init() and kvm_reset() in class

Posted by Philippe Mathieu-Daudé 1 year, 10 months ago

Hi Xiaoyao,

On 22/3/24 19:10, Paolo Bonzini wrote:
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> 
> 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 facilitate the introduction of confidential guest technology from
> different x86 vendors, 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>
> Message-Id: <20240229060038.606591-1-xiaoyao.li@intel.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   include/exec/confidential-guest-support.h | 34 ++++++++++++++++++++++-
>   1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/include/exec/confidential-guest-support.h b/include/exec/confidential-guest-support.h
> index ba2dd4b5dfc..e5b188cffbf 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,37 @@ struct ConfidentialGuestSupport {
>   
>   typedef struct ConfidentialGuestSupportClass {
>       ObjectClass parent;
> +
> +    int (*kvm_init)(ConfidentialGuestSupport *cgs, Error **errp);
> +    int (*kvm_reset)(ConfidentialGuestSupport *cgs, Error **errp);

Can we get a docstring indicating what these functions return?
Looking at the next patch, the KVM specific return value doesn't
seem used, so can we return a boolean instead?

>   } ConfidentialGuestSupportClass;

I suppose it will be easy enough to refactor for future other
HW accelerators.

Regards,

Phil.

Re: [PATCH 03/26] confidential guest support: Add kvm_init() and kvm_reset() in class

Posted by Paolo Bonzini 1 year, 10 months ago

On Mon, Mar 25, 2024 at 9:33 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Hi Xiaoyao,
>
> On 22/3/24 19:10, Paolo Bonzini wrote:
> > From: Xiaoyao Li <xiaoyao.li@intel.com>
> >
> > 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 facilitate the introduction of confidential guest technology from
> > different x86 vendors, 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>
> > Message-Id: <20240229060038.606591-1-xiaoyao.li@intel.com>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >   include/exec/confidential-guest-support.h | 34 ++++++++++++++++++++++-
> >   1 file changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/exec/confidential-guest-support.h b/include/exec/confidential-guest-support.h
> > index ba2dd4b5dfc..e5b188cffbf 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,37 @@ struct ConfidentialGuestSupport {
> >
> >   typedef struct ConfidentialGuestSupportClass {
> >       ObjectClass parent;
> > +
> > +    int (*kvm_init)(ConfidentialGuestSupport *cgs, Error **errp);
> > +    int (*kvm_reset)(ConfidentialGuestSupport *cgs, Error **errp);
>
> Can we get a docstring indicating what these functions return?
> Looking at the next patch, the KVM specific return value doesn't
> seem used, so can we return a boolean instead?

It is propagated all the way up to accel_init_machine().  _There_ it
isn't used, but I think it's not a good idea to return a "wrong" value
from kvm_arch_init() because we know that ultimately it isn't used. It
should be refactored top-down, though I admit that to be honest it
won't happen.

Paolo