| Message ID | 20240322181116.1228416-4-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86, kvm: common confidential computing subset | expand |
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.
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
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 */