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@xxxxxxxxx>
> Acked-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
> ---
> 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
--
|: 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 :|
