Re: [PATCH v3 04/15] hw/core/machine: Add igvm-cfg object and processing for IGVM files

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Jun 21, 2024 at 03:29:07PM +0100, Roy Hopkins wrote:
> An IGVM file contains configuration of guest state that should be
> applied during configuration of the guest, before the guest is started.
> 
> This patch allows the user to add an igvm-cfg object to the machine
> configuration that allows an IGVM file to be configured that will be
> applied to the guest before it is started.
> 
> If an IGVM configuration is provided then the IGVM file is processed at
> the end of the board initialization, before the state transition to
> PHASE_MACHINE_INITIALIZED.
> 
> Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
> ---
>  include/hw/boards.h |  2 ++
>  hw/core/machine.c   | 20 ++++++++++++++++++++
>  qemu-options.hx     | 25 +++++++++++++++++++++++++
>  3 files changed, 47 insertions(+)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 73ad319d7d..4c1484ba0b 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -4,6 +4,7 @@
>  #define HW_BOARDS_H
>  
>  #include "exec/memory.h"
> +#include "sysemu/igvm-cfg.h"
>  #include "sysemu/hostmem.h"
>  #include "sysemu/blockdev.h"
>  #include "qapi/qapi-types-machine.h"
> @@ -382,6 +383,7 @@ struct MachineState {
>      bool suppress_vmdesc;
>      bool enable_graphics;
>      ConfidentialGuestSupport *cgs;
> +    IgvmCfgState *igvm;
>      HostMemoryBackend *memdev;
>      /*
>       * convenience alias to ram_memdev_id backend memory region
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 655d75c21f..f9f879172c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1094,6 +1094,16 @@ static void machine_class_init(ObjectClass *oc, void *data)
>      object_class_property_set_description(oc, "confidential-guest-support",
>                                            "Set confidential guest scheme to support");
>  
> +#if defined(CONFIG_IGVM)
> +    object_class_property_add_link(oc, "igvm-cfg",
> +                                   TYPE_IGVM_CFG,
> +                                   offsetof(MachineState, igvm),
> +                                   object_property_allow_set_link,
> +                                   OBJ_PROP_LINK_STRONG);
> +    object_class_property_set_description(oc, "igvm-cfg",
> +                                          "Set IGVM configuration");
> +#endif
> +
>      /* For compatibility */
>      object_class_property_add_str(oc, "memory-encryption",
>          machine_get_memory_encryption, machine_set_memory_encryption);
> @@ -1582,6 +1592,16 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
>  
>      accel_init_interfaces(ACCEL_GET_CLASS(machine->accelerator));
>      machine_class->init(machine);
> +
> +#if defined(CONFIG_IGVM)
> +    /* Apply guest state from IGVM if supplied */
> +    if (machine->igvm) {
> +        if (IGVM_CFG_GET_CLASS(machine->igvm)
> +                ->process(machine->igvm, machine->cgs, &error_abort) == -1) {

Perhaps use error_fatal rather than error_abort, since failures here are
more likely to be user errors (incompatible igvm config), rather than QEMU
programmer bugs.

> +            return;
> +        }
> +    }
> +#endif
>      phase_advance(PHASE_MACHINE_INITIALIZED);
>  }
>  

This adds igvm-cfg for all machines, regardless of architecture target.

Are igvm files fully cross-platform portable, or should we just put
this into the TYPE_X86_MACHINE base class to limit it ?

It at least reports errors if I try to load an IGVM file with
qemu-system-aarch64 + virt type

$ ./build/qemu-system-aarch64 -object igvm-cfg,file=../buildigvm/ovmf-sev.igvm,id=igvm -machine virt,igvm-cfg=igvm
qemu-system-aarch64: IGVM file does not describe a compatible supported platform

so that's good.

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 :|