mbox series

[v5,00/16] Introduce support for IGVM files

Message ID cover.1723560001.git.roy.hopkins@suse.com
Headers show
Series Introduce support for IGVM files | expand

Message

Roy Hopkins Aug. 13, 2024, 3:01 p.m. UTC
Here is v5 of the set of patches to add support for IGVM files to QEMU. This is
based on commit 0f397dcfec of qemu.

This version addresses the review comments from v4 [1] plus changes required to
rebase on the master commit. As always, thanks to those that have been following
along, reviewing and testing this series. This v5 patch series is also available
on github: [2]

For testing IGVM support in QEMU you need to generate an IGVM file that is
configured for the platform you want to launch. You can use the `buildigvm`
test tool [3] to allow generation of IGVM files for all currently supported
platforms. Patch 11/17 contains information on how to generate an IGVM file
using this tool.

Changes in v5:

* Fix indentation and apply minimum version check for IGVM library in meson.build
* Remove unneeded duplicate macro definitions in confidential-guest-support.h
  and igvm-cvg.h
* Make igvm-cfg object file parameter mandatory instead of optional. Removed
  unused 'igvm_process()' function that checked the file was provided.
* Rename all QEMU IGVM functions and structs using QIGVM/qigvm prefix.
* A few small readability/style fixes.
* Address review comments on error handling, including removal of the v4 patch
  6: "Fix error handling in sev_encrypt_flash()".
* Update `FirmwareMapping` union in firmware.json to include `igvm`.

Patch summary:

1-11: Add support and documentation for processing IGVM files for SEV, SEV-ES,
SEV-SNP and native platforms. 

12-15: Processing of policy and SEV-SNP ID_BLOCK from IGVM file. 

16: Add pre-processing of IGVM file to support synchronization of 'SEV_FEATURES'
from IGVM VMSA to KVM.

[1] Link to v4:
https://lore.kernel.org/qemu-devel/cover.1720004383.git.roy.hopkins@suse.com/

[2] v5 patches also available here:
https://github.com/roy-hopkins/qemu/tree/igvm_master_v5

[3] `buildigvm` tool v0.2.0
https://github.com/roy-hopkins/buildigvm/releases/tag/v0.2.0

Roy Hopkins (16):
  meson: Add optional dependency on IGVM library
  backends/confidential-guest-support: Add functions to support IGVM
  backends/igvm: Add IGVM loader and configuration
  hw/i386: Add igvm-cfg object and processing for IGVM files
  i386/pc_sysfw: Ensure sysfw flash configuration does not conflict with
    IGVM
  sev: Update launch_update_data functions to use Error handling
  target/i386: Allow setting of R_LDTR and R_TR with
    cpu_x86_load_seg_cache()
  i386/sev: Refactor setting of reset vector and initial CPU state
  i386/sev: Implement ConfidentialGuestSupport functions for SEV
  docs/system: Add documentation on support for IGVM
  docs/interop/firmware.json: Add igvm to FirmwareDevice
  backends/confidential-guest-support: Add set_guest_policy() function
  backends/igvm: Process initialization sections in IGVM file
  backends/igvm: Handle policy for SEV guests
  i386/sev: Add implementation of CGS set_guest_policy()
  sev: Provide sev_features flags from IGVM VMSA to KVM_SEV_INIT2

 backends/confidential-guest-support.c      |  43 +
 backends/igvm-cfg.c                        |  52 ++
 backends/igvm.c                            | 964 +++++++++++++++++++++
 backends/igvm.h                            |  23 +
 backends/meson.build                       |   5 +
 docs/interop/firmware.json                 |  30 +-
 docs/system/i386/amd-memory-encryption.rst |   2 +
 docs/system/igvm.rst                       | 173 ++++
 docs/system/index.rst                      |   1 +
 hw/i386/pc.c                               |  12 +
 hw/i386/pc_piix.c                          |  10 +
 hw/i386/pc_q35.c                           |  10 +
 hw/i386/pc_sysfw.c                         |  31 +-
 include/exec/confidential-guest-support.h  |  86 ++
 include/hw/i386/x86.h                      |   3 +
 include/sysemu/igvm-cfg.h                  |  47 +
 meson.build                                |   8 +
 meson_options.txt                          |   2 +
 qapi/qom.json                              |  17 +
 qemu-options.hx                            |  25 +
 scripts/meson-buildoptions.sh              |   3 +
 target/i386/cpu.h                          |   9 +-
 target/i386/sev.c                          | 850 ++++++++++++++++--
 target/i386/sev.h                          | 124 +++
 24 files changed, 2446 insertions(+), 84 deletions(-)
 create mode 100644 backends/igvm-cfg.c
 create mode 100644 backends/igvm.c
 create mode 100644 backends/igvm.h
 create mode 100644 docs/system/igvm.rst
 create mode 100644 include/sysemu/igvm-cfg.h

Comments

Daniel P. Berrangé Aug. 19, 2024, 4:11 p.m. UTC | #1
On Tue, Aug 13, 2024 at 04:01:03PM +0100, Roy Hopkins wrote:
> The IGVM library allows Independent Guest Virtual Machine files to be
> parsed and processed. IGVM files are used to configure guest memory
> layout, initial processor state and other configuration pertaining to
> secure virtual machines.
> 
> This adds the --enable-igvm configure option, enabled by default, which
> attempts to locate and link against the IGVM library via pkgconfig and
> sets CONFIG_IGVM if found.
> 
> The library is added to the system_ss target in backends/meson.build
> where the IGVM parsing will be performed by the ConfidentialGuestSupport
> object.
> 
> Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  backends/meson.build          | 3 +++
>  meson.build                   | 8 ++++++++
>  meson_options.txt             | 2 ++
>  scripts/meson-buildoptions.sh | 3 +++
>  4 files changed, 16 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
Stefano Garzarella Sept. 2, 2024, 1:50 p.m. UTC | #2
On Tue, Aug 13, 2024 at 04:01:03PM GMT, Roy Hopkins wrote:
>The IGVM library allows Independent Guest Virtual Machine files to be
>parsed and processed. IGVM files are used to configure guest memory
>layout, initial processor state and other configuration pertaining to
>secure virtual machines.
>
>This adds the --enable-igvm configure option, enabled by default, which
>attempts to locate and link against the IGVM library via pkgconfig and
>sets CONFIG_IGVM if found.
>
>The library is added to the system_ss target in backends/meson.build
>where the IGVM parsing will be performed by the ConfidentialGuestSupport
>object.
>
>Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
>Acked-by: Michael S. Tsirkin <mst@redhat.com>
>---
> backends/meson.build          | 3 +++
> meson.build                   | 8 ++++++++
> meson_options.txt             | 2 ++
> scripts/meson-buildoptions.sh | 3 +++
> 4 files changed, 16 insertions(+)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

>
>diff --git a/backends/meson.build b/backends/meson.build
>index da714b93d1..b092a19efc 100644
>--- a/backends/meson.build
>+++ b/backends/meson.build
>@@ -32,6 +32,9 @@ if have_vhost_user_crypto
> endif
> system_ss.add(when: gio, if_true: files('dbus-vmstate.c'))
> system_ss.add(when: 'CONFIG_SGX', if_true: files('hostmem-epc.c'))
>+if igvm.found()
>+  system_ss.add(igvm)
>+endif
>
> system_ss.add(when: 'CONFIG_SPDM_SOCKET', if_true: files('spdm-socket.c'))
>
>diff --git a/meson.build b/meson.build
>index c2a050b844..11976674ff 100644
>--- a/meson.build
>+++ b/meson.build
>@@ -1289,6 +1289,12 @@ if host_os == 'linux' and (have_system or have_tools)
>                        method: 'pkg-config',
>                        required: get_option('libudev'))
> endif
>+igvm = not_found
>+if not get_option('igvm').auto() or have_system
>+  igvm = dependency('igvm', version: '>= 0.3.0',
>+                    method: 'pkg-config',
>+                    required: get_option('igvm'))
>+endif
>
> mpathlibs = [libudev]
> mpathpersist = not_found
>@@ -2420,6 +2426,7 @@ config_host_data.set('CONFIG_CFI', get_option('cfi'))
> config_host_data.set('CONFIG_SELINUX', selinux.found())
> config_host_data.set('CONFIG_XEN_BACKEND', xen.found())
> config_host_data.set('CONFIG_LIBDW', libdw.found())
>+config_host_data.set('CONFIG_IGVM', igvm.found())
> if xen.found()
>   # protect from xen.version() having less than three components
>   xen_version = xen.version().split('.') + ['0', '0']
>@@ -4520,6 +4527,7 @@ summary_info += {'seccomp support':   seccomp}
> summary_info += {'GlusterFS support': glusterfs}
> summary_info += {'hv-balloon support': hv_balloon}
> summary_info += {'TPM support':       have_tpm}
>+summary_info += {'IGVM support':      igvm}
> summary_info += {'libssh support':    libssh}
> summary_info += {'lzo support':       lzo}
> summary_info += {'snappy support':    snappy}
>diff --git a/meson_options.txt b/meson_options.txt
>index 0269fa0f16..0b09c152dc 100644
>--- a/meson_options.txt
>+++ b/meson_options.txt
>@@ -111,6 +111,8 @@ option('dbus_display', type: 'feature', value: 'auto',
>        description: '-display dbus support')
> option('tpm', type : 'feature', value : 'auto',
>        description: 'TPM support')
>+option('igvm', type: 'feature', value: 'auto',
>+       description: 'Independent Guest Virtual Machine (IGVM) file support')
>
> # Do not enable it by default even for Mingw32, because it doesn't
> # work on Wine.
>diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
>index c97079a38c..264e46dd4a 100644
>--- a/scripts/meson-buildoptions.sh
>+++ b/scripts/meson-buildoptions.sh
>@@ -128,6 +128,7 @@ meson_options_help() {
>   printf "%s\n" '  hv-balloon      hv-balloon driver (requires Glib 2.68+ GTree API)'
>   printf "%s\n" '  hvf             HVF acceleration support'
>   printf "%s\n" '  iconv           Font glyph conversion support'
>+  printf "%s\n" '  igvm            IGVM file support'
>   printf "%s\n" '  jack            JACK sound support'
>   printf "%s\n" '  keyring         Linux keyring support'
>   printf "%s\n" '  kvm             KVM acceleration support'
>@@ -343,6 +344,8 @@ _meson_option_parse() {
>     --iasl=*) quote_sh "-Diasl=$2" ;;
>     --enable-iconv) printf "%s" -Diconv=enabled ;;
>     --disable-iconv) printf "%s" -Diconv=disabled ;;
>+    --enable-igvm) printf "%s" -Digvm=enabled ;;
>+    --disable-igvm) printf "%s" -Digvm=disabled ;;
>     --includedir=*) quote_sh "-Dincludedir=$2" ;;
>     --enable-install-blobs) printf "%s" -Dinstall_blobs=true ;;
>     --disable-install-blobs) printf "%s" -Dinstall_blobs=false ;;
>-- 
>2.43.0
>
Stefano Garzarella Sept. 2, 2024, 2 p.m. UTC | #3
On Tue, Aug 13, 2024 at 04:01:05PM GMT, Roy Hopkins wrote:
>Adds an IGVM loader to QEMU which processes a given IGVM file and
>applies the directives within the file to the current guest
>configuration.
>
>The IGVM loader can be used to configure both confidential and
>non-confidential guests. For confidential guests, the
>ConfidentialGuestSupport object for the system is used to encrypt
>memory, apply the initial CPU state and perform other confidential guest
>operations.
>
>The loader is configured via a new IgvmCfg QOM object which allows the
>user to provide a path to the IGVM file to process.
>
>Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
>Acked-by: Michael S. Tsirkin <mst@redhat.com>
>---
> backends/igvm-cfg.c       |  52 +++
> backends/igvm.c           | 805 ++++++++++++++++++++++++++++++++++++++
> backends/igvm.h           |  23 ++
> backends/meson.build      |   2 +
> include/sysemu/igvm-cfg.h |  47 +++
> qapi/qom.json             |  17 +
> 6 files changed, 946 insertions(+)
> create mode 100644 backends/igvm-cfg.c
> create mode 100644 backends/igvm.c
> create mode 100644 backends/igvm.h
> create mode 100644 include/sysemu/igvm-cfg.h
>
>diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c
>new file mode 100644
>index 0000000000..63f8856c7b
>--- /dev/null
>+++ b/backends/igvm-cfg.c
>@@ -0,0 +1,52 @@
>+/*
>+ * QEMU IGVM interface
>+ *
>+ * Copyright (C) 2023-2024 SUSE
>+ *
>+ * Authors:
>+ *  Roy Hopkins <roy.hopkins@suse.com>
>+ *
>+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
>+ * See the COPYING file in the top-level directory.
>+ */
>+
>+#include "qemu/osdep.h"
>+
>+#include "sysemu/igvm-cfg.h"
>+#include "igvm.h"
>+#include "qom/object_interfaces.h"
>+
>+static char *get_igvm(Object *obj, Error **errp)
>+{
>+    IgvmCfg *igvm = IGVM_CFG(obj);
>+    return g_strdup(igvm->filename);
>+}
>+
>+static void set_igvm(Object *obj, const char *value, Error **errp)
>+{
>+    IgvmCfg *igvm = IGVM_CFG(obj);
>+    g_free(igvm->filename);
>+    igvm->filename = g_strdup(value);
>+}
>+
>+OBJECT_DEFINE_TYPE_WITH_INTERFACES(IgvmCfg, igvm_cfg, IGVM_CFG, OBJECT,
>+                                   { TYPE_USER_CREATABLE }, { NULL })
>+
>+static void igvm_cfg_class_init(ObjectClass *oc, void *data)
>+{
>+    IgvmCfgClass *igvmc = IGVM_CFG_CLASS(oc);
>+
>+    object_class_property_add_str(oc, "file", get_igvm, set_igvm);
>+    object_class_property_set_description(oc, "file",
>+                                          "Set the IGVM filename to use");
>+
>+    igvmc->process = qigvm_process_file;
>+}
>+
>+static void igvm_cfg_init(Object *obj)
>+{
>+}
>+
>+static void igvm_cfg_finalize(Object *obj)
>+{
>+}
>diff --git a/backends/igvm.c b/backends/igvm.c
>new file mode 100644
>index 0000000000..7a3fedcc76
>--- /dev/null
>+++ b/backends/igvm.c
>@@ -0,0 +1,805 @@
>+/*
>+ * QEMU IGVM configuration backend for guests
>+ *
>+ * Copyright (C) 2023-2024 SUSE
>+ *
>+ * Authors:
>+ *  Roy Hopkins <roy.hopkins@suse.com>
>+ *
>+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
>+ * See the COPYING file in the top-level directory.
>+ */
>+
>+#include "qemu/osdep.h"
>+
>+#include "igvm.h"
>+#include "qapi/error.h"
>+#include "exec/memory.h"
>+#include "exec/address-spaces.h"
>+#include "hw/core/cpu.h"
>+
>+#include <igvm/igvm.h>
>+#include <igvm/igvm_defs.h>
>+
>+typedef struct QIgvmParameterData {
>+    QTAILQ_ENTRY(QIgvmParameterData) next;
>+    uint8_t *data;
>+    uint32_t size;
>+    uint32_t index;
>+} QIgvmParameterData;
>+
>+/*
>+ * QIgvm contains the information required during processing
>+ * of a single IGVM file.
>+ */
>+typedef struct QIgvm {
>+    IgvmHandle file;
>+    ConfidentialGuestSupport *cgs;
>+    ConfidentialGuestSupportClass *cgsc;
>+    uint32_t compatibility_mask;
>+    unsigned current_header_index;
>+    QTAILQ_HEAD(, QIgvmParameterData) parameter_data;
>+
>+    /* These variables keep track of contiguous page regions */
>+    IGVM_VHS_PAGE_DATA region_prev_page_data;
>+    uint64_t region_start;
>+    unsigned region_start_index;
>+    unsigned region_last_index;
>+    unsigned region_page_count;
>+} QIgvm;
>+
>+static int qigvm_directive_page_data(QIgvm *ctx, const uint8_t *header_data,
>+                                     Error **errp);
>+static int qigvm_directive_vp_context(QIgvm *ctx, const uint8_t *header_data,
>+                                      Error **errp);
>+static int qigvm_directive_parameter_area(QIgvm *ctx,
>+                                          const uint8_t *header_data,
>+                                          Error **errp);
>+static int qigvm_directive_parameter_insert(QIgvm *ctx,
>+                                            const uint8_t *header_data,
>+                                            Error **errp);
>+static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data,
>+                                      Error **errp);
>+static int qigvm_directive_vp_count(QIgvm *ctx, const uint8_t *header_data,
>+                                    Error **errp);
>+static int qigvm_directive_environment_info(QIgvm *ctx,
>+                                            const uint8_t *header_data,
>+                                            Error **errp);
>+static int qigvm_directive_required_memory(QIgvm *ctx,
>+                                           const uint8_t *header_data,
>+                                           Error **errp);
>+
>+struct QIGVMHandler {
>+    uint32_t type;
>+    uint32_t section;
>+    int (*handler)(QIgvm *ctx, const uint8_t *header_data, Error **errp);
>+};
>+
>+static struct QIGVMHandler handlers[] = {
>+    { IGVM_VHT_PAGE_DATA, IGVM_HEADER_SECTION_DIRECTIVE,
>+      qigvm_directive_page_data },
>+    { IGVM_VHT_VP_CONTEXT, IGVM_HEADER_SECTION_DIRECTIVE,
>+      qigvm_directive_vp_context },
>+    { IGVM_VHT_PARAMETER_AREA, IGVM_HEADER_SECTION_DIRECTIVE,
>+      qigvm_directive_parameter_area },
>+    { IGVM_VHT_PARAMETER_INSERT, IGVM_HEADER_SECTION_DIRECTIVE,
>+      qigvm_directive_parameter_insert },
>+    { IGVM_VHT_MEMORY_MAP, IGVM_HEADER_SECTION_DIRECTIVE,
>+      qigvm_directive_memory_map },
>+    { IGVM_VHT_VP_COUNT_PARAMETER, IGVM_HEADER_SECTION_DIRECTIVE,
>+      qigvm_directive_vp_count },
>+    { IGVM_VHT_ENVIRONMENT_INFO_PARAMETER, IGVM_HEADER_SECTION_DIRECTIVE,
>+      qigvm_directive_environment_info },
>+    { IGVM_VHT_REQUIRED_MEMORY, IGVM_HEADER_SECTION_DIRECTIVE,
>+      qigvm_directive_required_memory },
>+};
>+
>+static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp)
>+{
>+    size_t handler;
>+    IgvmHandle header_handle;
>+    const uint8_t *header_data;
>+    int result;
>+
>+    for (handler = 0; handler < G_N_ELEMENTS(handlers); handler++) {
>+        if (handlers[handler].type != type) {
>+            continue;
>+        }
>+        header_handle = igvm_get_header(ctx->file, handlers[handler].section,
>+                                        ctx->current_header_index);
>+        if (header_handle < 0) {
>+            error_setg(
>+                errp,
>+                "IGVM file is invalid: Failed to read directive header (code: %d)",
>+                (int)header_handle);
>+            return -1;
>+        }
>+        header_data = igvm_get_buffer(ctx->file, header_handle) +
>+                      sizeof(IGVM_VHS_VARIABLE_HEADER);
>+        result = handlers[handler].handler(ctx, header_data, errp);
>+        igvm_free_buffer(ctx->file, header_handle);
>+        return result;
>+    }
>+    error_setg(errp,
>+               "IGVM: Unknown header type encountered when processing file: "
>+               "(type 0x%X)",
>+               type);
>+    return -1;
>+}
>+
>+static void *qigvm_prepare_memory(QIgvm *ctx, uint64_t addr, uint64_t size,
>+                                  int region_identifier, Error **errp)
>+{
>+    ERRP_GUARD();
>+    MemoryRegion *igvm_pages = NULL;
>+    Int128 gpa_region_size;
>+    MemoryRegionSection mrs =
>+        memory_region_find(get_system_memory(), addr, size);
>+    if (mrs.mr) {
>+        if (!memory_region_is_ram(mrs.mr)) {
>+            memory_region_unref(mrs.mr);
>+            error_setg(
>+                errp,
>+                "Processing of IGVM file failed: Could not prepare memory "
>+                "at address 0x%lX due to existing non-RAM region",
>+                addr);
>+            return NULL;
>+        }
>+
>+        gpa_region_size = int128_make64(size);
>+        if (int128_lt(mrs.size, gpa_region_size)) {
>+            memory_region_unref(mrs.mr);
>+            error_setg(
>+                errp,
>+                "Processing of IGVM file failed: Could not prepare memory "
>+                "at address 0x%lX: region size exceeded",
>+                addr);
>+            return NULL;
>+        }
>+        return qemu_map_ram_ptr(mrs.mr->ram_block, mrs.offset_within_region);
>+    } else {
>+        /*
>+         * The region_identifier is the is the index of the IGVM directive that
>+         * contains the page with the lowest GPA in the region. This will
>+         * generate a unique region name.
>+         */
>+        g_autofree char *region_name =
>+            g_strdup_printf("igvm.%X", region_identifier);
>+        igvm_pages = g_new0(MemoryRegion, 1);
>+        if (ctx->cgs && ctx->cgs->require_guest_memfd) {
>+            if (!memory_region_init_ram_guest_memfd(igvm_pages, NULL,
>+                                                    region_name, size, errp)) {
>+                return NULL;
>+            }
>+        } else {
>+            if (!memory_region_init_ram(igvm_pages, NULL, region_name, size,
>+                                        errp)) {
>+                return NULL;
>+            }
>+        }
>+        memory_region_add_subregion(get_system_memory(), addr, igvm_pages);
>+        return memory_region_get_ram_ptr(igvm_pages);
>+    }
>+}
>+
>+static int qigvm_type_to_cgs_type(IgvmPageDataType memory_type, bool unmeasured,
>+                                  bool zero)
>+{
>+    switch (memory_type) {
>+    case IGVM_PAGE_DATA_TYPE_NORMAL: {
>+        if (unmeasured) {
>+            return CGS_PAGE_TYPE_UNMEASURED;
>+        } else {
>+            return zero ? CGS_PAGE_TYPE_ZERO : CGS_PAGE_TYPE_NORMAL;
>+        }
>+    }
>+    case IGVM_PAGE_DATA_TYPE_SECRETS:
>+        return CGS_PAGE_TYPE_SECRETS;
>+    case IGVM_PAGE_DATA_TYPE_CPUID_DATA:
>+        return CGS_PAGE_TYPE_CPUID;
>+    case IGVM_PAGE_DATA_TYPE_CPUID_XF:
>+        return CGS_PAGE_TYPE_CPUID;
>+    default:
>+        return -1;
>+    }
>+}
>+
>+static bool qigvm_page_attrs_equal(IgvmHandle igvm, unsigned header_index,
>+                                   const IGVM_VHS_PAGE_DATA *page_1,
>+                                   const IGVM_VHS_PAGE_DATA *page_2)
>+{
>+    IgvmHandle data_handle1, data_handle2;
>+
>+    /*
>+     * If one page has data and the other doesn't then this results in different
>+     * page types: NORMAL vs ZERO.
>+     */
>+    data_handle1 = igvm_get_header_data(igvm, IGVM_HEADER_SECTION_DIRECTIVE,
>+                                        header_index - 1);
>+    data_handle2 =
>+        igvm_get_header_data(igvm, IGVM_HEADER_SECTION_DIRECTIVE, header_index);
>+    if ((data_handle1 == IGVMAPI_NO_DATA ||
>+         data_handle2 == IGVMAPI_NO_DATA) &&
>+         data_handle1 != data_handle2) {
>+        return false;
>+    }
>+    return ((*(const uint32_t *)&page_1->flags ==
>+             *(const uint32_t *)&page_2->flags) &&
>+            (page_1->data_type == page_2->data_type) &&
>+            (page_1->compatibility_mask == page_2->compatibility_mask));
>+}
>+
>+static int qigvm_process_mem_region(QIgvm *ctx, unsigned start_index,
>+                                    uint64_t gpa_start, unsigned page_count,
>+                                    const IgvmPageDataFlags *flags,
>+                                    const IgvmPageDataType page_type,
>+                                    Error **errp)
>+{
>+    uint8_t *region;
>+    IgvmHandle data_handle;
>+    const void *data;
>+    uint32_t data_size;
>+    unsigned page_index;
>+    bool zero = true;
>+    const uint64_t page_size = flags->is_2mb_page ? 0x200000 : 0x1000;
>+    int result;
>+    int cgs_page_type;
>+
>+    region = qigvm_prepare_memory(ctx, gpa_start, page_count * page_size,
>+                                  start_index, errp);
>+    if (!region) {
>+        return -1;
>+    }
>+
>+    for (page_index = 0; page_index < page_count; page_index++) {
>+        data_handle = igvm_get_header_data(
>+            ctx->file, IGVM_HEADER_SECTION_DIRECTIVE, page_index + start_index);
>+        if (data_handle == IGVMAPI_NO_DATA) {
>+            /* No data indicates a zero page */
>+            memset(&region[page_index * page_size], 0, page_size);
>+        } else if (data_handle < 0) {
>+            error_setg(
>+                errp,
>+                "IGVM file contains invalid page data for directive with "
>+                "index %d",
>+                page_index + start_index);
>+            return -1;
>+        } else {
>+            zero = false;
>+            data_size = igvm_get_buffer_size(ctx->file, data_handle);
>+            if (data_size < page_size) {
>+                memset(&region[page_index * page_size], 0, page_size);
>+            } else if (data_size > page_size) {
>+                error_setg(errp,
>+                           "IGVM file contains page data with invalid size for "
>+                           "directive with index %d",
>+                           page_index + start_index);
>+                return -1;
>+            }
>+            data = igvm_get_buffer(ctx->file, data_handle);
>+            memcpy(&region[page_index * page_size], data, data_size);
>+            igvm_free_buffer(ctx->file, data_handle);
>+        }
>+    }
>+
>+    /*
>+     * If a confidential guest support object is provided then use it to set the
>+     * guest state.
>+     */
>+    if (ctx->cgs) {
>+        cgs_page_type =
>+            qigvm_type_to_cgs_type(page_type, flags->unmeasured, zero);
>+        if (cgs_page_type < 0) {
>+            error_setg(errp,
>+                       "Invalid page type in IGVM file. Directives: %d to %d, "
>+                       "page type: %d",
>+                       start_index, start_index + page_count, page_type);
>+            return -1;
>+        }
>+
>+        result = ctx->cgsc->set_guest_state(
>+            gpa_start, region, page_size * page_count, cgs_page_type, 0, errp);
>+        if (result < 0) {
>+            return result;
>+        }
>+    }
>+    return 0;
>+}
>+
>+static int qigvm_process_mem_page(QIgvm *ctx,
>+                                  const IGVM_VHS_PAGE_DATA *page_data,
>+                                  Error **errp)
>+{
>+    if (page_data) {
>+        if (ctx->region_page_count == 0) {
>+            ctx->region_start = page_data->gpa;
>+            ctx->region_start_index = ctx->current_header_index;
>+        } else {
>+            if (!qigvm_page_attrs_equal(ctx->file, ctx->current_header_index,
>+                                        page_data,
>+                                        &ctx->region_prev_page_data) ||
>+                ((ctx->region_prev_page_data.gpa +
>+                  (ctx->region_prev_page_data.flags.is_2mb_page ? 0x200000 :
>+                                                                  0x1000)) !=
>+                 page_data->gpa) ||
>+                (ctx->region_last_index != (ctx->current_header_index - 1))) {
>+                /* End of current region */
>+                if (qigvm_process_mem_region(
>+                        ctx, ctx->region_start_index, ctx->region_start,
>+                        ctx->region_page_count,
>+                        &ctx->region_prev_page_data.flags,
>+                        ctx->region_prev_page_data.data_type, errp) < 0) {
>+                    return -1;
>+                }
>+                ctx->region_page_count = 0;
>+                ctx->region_start = page_data->gpa;
>+                ctx->region_start_index = ctx->current_header_index;
>+            }
>+        }
>+        memcpy(&ctx->region_prev_page_data, page_data,
>+               sizeof(ctx->region_prev_page_data));
>+        ctx->region_last_index = ctx->current_header_index;
>+        ctx->region_page_count++;
>+    } else {
>+        if (ctx->region_page_count > 0) {
>+            if (qigvm_process_mem_region(
>+                    ctx, ctx->region_start_index, ctx->region_start,
>+                    ctx->region_page_count, &ctx->region_prev_page_data.flags,
>+                    ctx->region_prev_page_data.data_type, errp) < 0) {
>+                return -1;
>+            }
>+            ctx->region_page_count = 0;
>+        }
>+    }
>+    return 0;
>+}
>+
>+static int qigvm_directive_page_data(QIgvm *ctx, const uint8_t *header_data,
>+                                     Error **errp)
>+{
>+    const IGVM_VHS_PAGE_DATA *page_data =
>+        (const IGVM_VHS_PAGE_DATA *)header_data;
>+    if (page_data->compatibility_mask & ctx->compatibility_mask) {
>+        return qigvm_process_mem_page(ctx, page_data, errp);
>+    }
>+    return 0;
>+}
>+
>+static int qigvm_directive_vp_context(QIgvm *ctx, const uint8_t *header_data,
>+                                      Error **errp)
>+{
>+    const IGVM_VHS_VP_CONTEXT *vp_context =
>+        (const IGVM_VHS_VP_CONTEXT *)header_data;
>+    IgvmHandle data_handle;
>+    uint8_t *data;
>+    int result;
>+
>+    if (!(vp_context->compatibility_mask & ctx->compatibility_mask)) {
>+        return 0;
>+    }
>+
>+    /*
>+     * A confidential guest support object must be provided for setting
>+     * a VP context.
>+     */
>+    if (!ctx->cgs) {
>+        error_setg(
>+            errp,
>+            "A VP context is present in the IGVM file but is not supported "
>+            "by the current system.");
>+        return -1;
>+    }
>+
>+    data_handle = igvm_get_header_data(ctx->file, IGVM_HEADER_SECTION_DIRECTIVE,
>+                                       ctx->current_header_index);
>+    if (data_handle < 0) {
>+        error_setg(errp, "Invalid VP context in IGVM file. Error code: %X",
>+                   data_handle);
>+        return -1;
>+    }
>+
>+    data = (uint8_t *)igvm_get_buffer(ctx->file, data_handle);
>+    result = ctx->cgsc->set_guest_state(
>+        vp_context->gpa, data, igvm_get_buffer_size(ctx->file, data_handle),
>+        CGS_PAGE_TYPE_VMSA, vp_context->vp_index, errp);
>+    igvm_free_buffer(ctx->file, data_handle);
>+    if (result < 0) {
>+        return result;
>+    }
>+    return 0;
>+}
>+
>+static int qigvm_directive_parameter_area(QIgvm *ctx,
>+                                          const uint8_t *header_data,
>+                                          Error **errp)
>+{
>+    const IGVM_VHS_PARAMETER_AREA *param_area =
>+        (const IGVM_VHS_PARAMETER_AREA *)header_data;
>+    QIgvmParameterData *param_entry;
>+
>+    param_entry = g_new0(QIgvmParameterData, 1);
>+    param_entry->size = param_area->number_of_bytes;
>+    param_entry->index = param_area->parameter_area_index;
>+    param_entry->data = g_malloc0(param_entry->size);
>+
>+    QTAILQ_INSERT_TAIL(&ctx->parameter_data, param_entry, next);
>+    return 0;
>+}
>+
>+static int qigvm_directive_parameter_insert(QIgvm *ctx,
>+                                            const uint8_t *header_data,
>+                                            Error **errp)
>+{
>+    const IGVM_VHS_PARAMETER_INSERT *param =
>+        (const IGVM_VHS_PARAMETER_INSERT *)header_data;
>+    QIgvmParameterData *param_entry;
>+    int result;
>+    void *region;
>+
>+    if (!(param->compatibility_mask & ctx->compatibility_mask)) {
>+        return 0;
>+    }
>+
>+    QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next)
>+    {
>+        if (param_entry->index == param->parameter_area_index) {
>+            region = qigvm_prepare_memory(ctx, param->gpa, param_entry->size,
>+                                          ctx->current_header_index, errp);
>+            if (!region) {
>+                return -1;
>+            }
>+            memcpy(region, param_entry->data, param_entry->size);
>+            g_free(param_entry->data);
>+            param_entry->data = NULL;
>+
>+            /*
>+             * If a confidential guest support object is provided then use it to
>+             * set the guest state.
>+             */
>+            if (ctx->cgs) {
>+                result = ctx->cgsc->set_guest_state(param->gpa, region,
>+                                                    param_entry->size,
>+                                                    CGS_PAGE_TYPE_UNMEASURED, 0,
>+                                                    errp);
>+                if (result < 0) {
>+                    return -1;
>+                }
>+            }
>+        }
>+    }
>+    return 0;
>+}
>+
>+static int qigvm_cmp_mm_entry(const void *a, const void *b)
>+{
>+    const IGVM_VHS_MEMORY_MAP_ENTRY *entry_a =
>+        (const IGVM_VHS_MEMORY_MAP_ENTRY *)a;
>+    const IGVM_VHS_MEMORY_MAP_ENTRY *entry_b =
>+        (const IGVM_VHS_MEMORY_MAP_ENTRY *)b;
>+    if (entry_a->starting_gpa_page_number < entry_b->starting_gpa_page_number) {
>+        return -1;
>+    } else if (entry_a->starting_gpa_page_number >
>+               entry_b->starting_gpa_page_number) {
>+        return 1;
>+    } else {
>+        return 0;
>+    }
>+}
>+
>+static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data,
>+                                      Error **errp)
>+{
>+    const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
>+    QIgvmParameterData *param_entry;
>+    int max_entry_count;
>+    int entry = 0;
>+    IGVM_VHS_MEMORY_MAP_ENTRY *mm_entry;
>+    ConfidentialGuestMemoryMapEntry cgmm_entry;
>+    int retval = 0;
>+
>+    if (!ctx->cgs) {
>+        error_setg(errp,
>+                   "IGVM file contains a memory map but this is not supported "
>+                   "by the current system.");
>+        return -1;
>+    }
>+
>+    /* Find the parameter area that should hold the memory map */
>+    QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next)
>+    {
>+        if (param_entry->index == param->parameter_area_index) {
>+            max_entry_count =
>+                param_entry->size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
>+            mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)param_entry->data;
>+
>+            retval = ctx->cgsc->get_mem_map_entry(entry, &cgmm_entry, errp);
>+            while (retval == 0) {
>+                if (entry > max_entry_count) {
>+                    error_setg(
>+                        errp,
>+                        "IGVM: guest memory map size exceeds parameter area defined in IGVM file");
>+                    return -1;
>+                }
>+                mm_entry[entry].starting_gpa_page_number = cgmm_entry.gpa >> 12;
>+                mm_entry[entry].number_of_pages = cgmm_entry.size >> 12;
>+
>+                switch (cgmm_entry.type) {
>+                case CGS_MEM_RAM:
>+                    mm_entry[entry].entry_type =
>+                        IGVM_MEMORY_MAP_ENTRY_TYPE_MEMORY;
>+                    break;
>+                case CGS_MEM_RESERVED:
>+                    mm_entry[entry].entry_type =
>+                        IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED;
>+                    break;
>+                case CGS_MEM_ACPI:
>+                    mm_entry[entry].entry_type =
>+                        IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED;
>+                    break;
>+                case CGS_MEM_NVS:
>+                    mm_entry[entry].entry_type =
>+                        IGVM_MEMORY_MAP_ENTRY_TYPE_PERSISTENT;
>+                    break;
>+                case CGS_MEM_UNUSABLE:
>+                    mm_entry[entry].entry_type =
>+                        IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED;
>+                    break;
>+                }
>+                retval =
>+                    ctx->cgsc->get_mem_map_entry(++entry, &cgmm_entry, errp);
>+            }
>+            if (retval < 0) {
>+                return retval;
>+            }
>+            /* The entries need to be sorted */
>+            qsort(mm_entry, entry, sizeof(IGVM_VHS_MEMORY_MAP_ENTRY),
>+                  qigvm_cmp_mm_entry);
>+
>+            break;
>+        }
>+    }
>+    return 0;
>+}
>+
>+static int qigvm_directive_vp_count(QIgvm *ctx, const uint8_t *header_data,
>+                                    Error **errp)
>+{
>+    const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
>+    QIgvmParameterData *param_entry;
>+    uint32_t *vp_count;
>+    CPUState *cpu;
>+
>+    QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next)
>+    {
>+        if (param_entry->index == param->parameter_area_index) {
>+            vp_count = (uint32_t *)(param_entry->data + param->byte_offset);
>+            *vp_count = 0;
>+            CPU_FOREACH(cpu)
>+            {
>+                (*vp_count)++;
>+            }
>+            break;
>+        }
>+    }
>+    return 0;
>+}
>+
>+static int qigvm_directive_environment_info(QIgvm *ctx,
>+                                            const uint8_t *header_data,
>+                                            Error **errp)
>+{
>+    const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
>+    QIgvmParameterData *param_entry;
>+    IgvmEnvironmentInfo *environmental_state;
>+
>+    QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next)
>+    {
>+        if (param_entry->index == param->parameter_area_index) {
>+            environmental_state =
>+                (IgvmEnvironmentInfo *)(param_entry->data + param->byte_offset);
>+            environmental_state->memory_is_shared = 1;
>+            break;
>+        }
>+    }
>+    return 0;
>+}
>+
>+static int qigvm_directive_required_memory(QIgvm *ctx,
>+                                           const uint8_t *header_data,
>+                                           Error **errp)
>+{
>+    const IGVM_VHS_REQUIRED_MEMORY *mem =
>+        (const IGVM_VHS_REQUIRED_MEMORY *)header_data;
>+    uint8_t *region;
>+    int result;
>+
>+    if (!(mem->compatibility_mask & ctx->compatibility_mask)) {
>+        return 0;
>+    }
>+
>+    region = qigvm_prepare_memory(ctx, mem->gpa, mem->number_of_bytes,
>+                                  ctx->current_header_index, errp);
>+    if (!region) {
>+        return -1;
>+    }
>+    if (ctx->cgs) {
>+        result =
>+            ctx->cgsc->set_guest_state(mem->gpa, region, mem->number_of_bytes,
>+                                       CGS_PAGE_TYPE_REQUIRED_MEMORY, 0, errp);
>+        if (result < 0) {
>+            return result;
>+        }
>+    }
>+    return 0;
>+}
>+
>+static int qigvm_supported_platform_compat_mask(QIgvm *ctx, Error **errp)
>+{
>+    int32_t header_count;
>+    unsigned header_index;
>+    IgvmHandle header_handle;
>+    IGVM_VHS_SUPPORTED_PLATFORM *platform;
>+    uint32_t compatibility_mask_sev = 0;
>+    uint32_t compatibility_mask_sev_es = 0;
>+    uint32_t compatibility_mask_sev_snp = 0;
>+    uint32_t compatibility_mask = 0;
>+
>+    header_count = igvm_header_count(ctx->file, IGVM_HEADER_SECTION_PLATFORM);
>+    if (header_count < 0) {
>+        error_setg(errp,
>+                   "Invalid platform header count in IGVM file. Error code: %X",
>+                   header_count);
>+        return -1;
>+    }
>+
>+    for (header_index = 0; header_index < (unsigned)header_count;
>+         header_index++) {
>+        IgvmVariableHeaderType typ = igvm_get_header_type(
>+            ctx->file, IGVM_HEADER_SECTION_PLATFORM, header_index);
>+        if (typ == IGVM_VHT_SUPPORTED_PLATFORM) {
>+            header_handle = igvm_get_header(
>+                ctx->file, IGVM_HEADER_SECTION_PLATFORM, header_index);
>+            if (header_handle < 0) {
>+                error_setg(errp,
>+                           "Invalid platform header in IGVM file. "
>+                           "Index: %d, Error code: %X",
>+                           header_index, header_handle);
>+                return -1;
>+            }
>+            platform =
>+                (IGVM_VHS_SUPPORTED_PLATFORM *)(igvm_get_buffer(ctx->file,
>+                                                                header_handle) +
>+                                                sizeof(
>+                                                    IGVM_VHS_VARIABLE_HEADER));
>+            if ((platform->platform_type == IGVM_PLATFORM_TYPE_SEV_ES) &&
>+                ctx->cgs) {
>+                if (ctx->cgsc->check_support(
>+                        CGS_PLATFORM_SEV_ES, platform->platform_version,
>+                        platform->highest_vtl, platform->shared_gpa_boundary)) {
>+                    compatibility_mask_sev_es = platform->compatibility_mask;
>+                }
>+            } else if ((platform->platform_type == IGVM_PLATFORM_TYPE_SEV) &&
>+                       ctx->cgs) {
>+                if (ctx->cgsc->check_support(
>+                        CGS_PLATFORM_SEV, platform->platform_version,
>+                        platform->highest_vtl, platform->shared_gpa_boundary)) {
>+                    compatibility_mask_sev = platform->compatibility_mask;
>+                }
>+            } else if ((platform->platform_type ==
>+                        IGVM_PLATFORM_TYPE_SEV_SNP) &&
>+                       ctx->cgs) {
>+                if (ctx->cgsc->check_support(
>+                        CGS_PLATFORM_SEV_SNP, platform->platform_version,
>+                        platform->highest_vtl, platform->shared_gpa_boundary)) {
>+                    compatibility_mask_sev_snp = platform->compatibility_mask;
>+                }
>+            } else if (platform->platform_type == IGVM_PLATFORM_TYPE_NATIVE) {
>+                compatibility_mask = platform->compatibility_mask;
>+            }
>+            igvm_free_buffer(ctx->file, header_handle);
>+        }
>+    }
>+    /* Choose the strongest supported isolation technology */
>+    if (compatibility_mask_sev_snp != 0) {
>+        ctx->compatibility_mask = compatibility_mask_sev_snp;
>+    } else if (compatibility_mask_sev_es != 0) {
>+        ctx->compatibility_mask = compatibility_mask_sev_es;
>+    } else if (compatibility_mask_sev != 0) {
>+        ctx->compatibility_mask = compatibility_mask_sev;
>+    } else if (compatibility_mask != 0) {
>+        ctx->compatibility_mask = compatibility_mask;
>+    } else {
>+        error_setg(
>+            errp,
>+            "IGVM file does not describe a compatible supported platform");
>+        return -1;
>+    }
>+    return 0;
>+}
>+
>+static IgvmHandle qigvm_file_init(char *filename, Error **errp)
>+{
>+    IgvmHandle igvm;
>+    g_autofree uint8_t *buf = NULL;
>+    unsigned long len;
>+    g_autoptr(GError) gerr = NULL;
>+
>+    if (!g_file_get_contents(filename, (gchar **)&buf, &len, &gerr)) {
>+        error_setg(errp, "Unable to load %s: %s", filename, gerr->message);
>+        return -1;
>+    }
>+
>+    igvm = igvm_new_from_binary(buf, len);
>+    if (igvm < 0) {
>+        error_setg(errp, "Unable to parse IGVM file %s: %d", filename, igvm);
>+        return -1;
>+    }

Should we call igvm_free() in the error path of qigvm_process_file()?

>+    return igvm;
>+}
>+
>+int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
>+                       Error **errp)
>+{
>+    int32_t header_count;
>+    QIgvmParameterData *parameter;
>+    int retval = -1;
>+    QIgvm ctx;
>+
>+    memset(&ctx, 0, sizeof(ctx));
>+    ctx.file = qigvm_file_init(cfg->filename, errp);
>+    if (ctx.file < 0) {
>+        return -1;
>+    }
>+
>+    /*
>+     * The ConfidentialGuestSupport object is optional and allows a confidential
>+     * guest platform to perform extra processing, such as page measurement, on
>+     * IGVM directives.
>+     */
>+    ctx.cgs = cgs;
>+    ctx.cgsc = cgs ? CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(cgs) : NULL;
>+
>+    /*
>+     * Check that the IGVM file provides configuration for the current
>+     * platform
>+     */
>+    if (qigvm_supported_platform_compat_mask(&ctx, errp) < 0) {
>+        return -1;
>+    }
>+
>+    header_count = igvm_header_count(ctx.file, IGVM_HEADER_SECTION_DIRECTIVE);
>+    if (header_count <= 0) {
>+        error_setg(
>+            errp, "Invalid directive header count in IGVM file. Error code: %X",
>+            header_count);
>+        return -1;
>+    }
>+
>+    QTAILQ_INIT(&ctx.parameter_data);
>+
>+    for (ctx.current_header_index = 0;
>+         ctx.current_header_index < (unsigned)header_count;
>+         ctx.current_header_index++) {
>+        IgvmVariableHeaderType type = igvm_get_header_type(
>+            ctx.file, IGVM_HEADER_SECTION_DIRECTIVE, ctx.current_header_index);
>+        if (qigvm_handler(&ctx, type, errp) < 0) {
>+            goto cleanup;
>+        }
>+    }
>+
>+    /*
>+     * Contiguous pages of data with compatible flags are grouped together in
>+     * order to reduce the number of memory regions we create. Make sure the
>+     * last group is processed with this call.
>+     */
>+    retval = qigvm_process_mem_page(&ctx, NULL, errp);
>+
>+cleanup:
>+    QTAILQ_FOREACH(parameter, &ctx.parameter_data, next)
>+    {
>+        g_free(parameter->data);
>+        parameter->data = NULL;
>+    }
>+
>+    return retval;
>+}
>diff --git a/backends/igvm.h b/backends/igvm.h
>new file mode 100644
>index 0000000000..436408c539
>--- /dev/null
>+++ b/backends/igvm.h
>@@ -0,0 +1,23 @@
>+/*
>+ * QEMU IGVM configuration backend for Confidential Guests
>+ *
>+ * Copyright (C) 2023-2024 SUSE
>+ *
>+ * Authors:
>+ *  Roy Hopkins <roy.hopkins@suse.com>
>+ *
>+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
>+ * See the COPYING file in the top-level directory.
>+ */
>+
>+#ifndef BACKENDS_IGVM_H
>+#define BACKENDS_IGVM_H
>+
>+#include "exec/confidential-guest-support.h"
>+#include "sysemu/igvm-cfg.h"
>+#include "qapi/error.h"
>+
>+int qigvm_process_file(IgvmCfg *igvm, ConfidentialGuestSupport *cgs,
>+                      Error **errp);
>+
>+#endif
>diff --git a/backends/meson.build b/backends/meson.build
>index b092a19efc..3b07471993 100644
>--- a/backends/meson.build
>+++ b/backends/meson.build
>@@ -34,6 +34,8 @@ system_ss.add(when: gio, if_true: files('dbus-vmstate.c'))
> system_ss.add(when: 'CONFIG_SGX', if_true: files('hostmem-epc.c'))
> if igvm.found()
>   system_ss.add(igvm)
>+  system_ss.add(files('igvm-cfg.c'), igvm)
>+  system_ss.add(files('igvm.c'), igvm)
> endif
>
> system_ss.add(when: 'CONFIG_SPDM_SOCKET', if_true: files('spdm-socket.c'))
>diff --git a/include/sysemu/igvm-cfg.h b/include/sysemu/igvm-cfg.h
>new file mode 100644
>index 0000000000..21fadfe5b7
>--- /dev/null
>+++ b/include/sysemu/igvm-cfg.h
>@@ -0,0 +1,47 @@
>+/*
>+ * QEMU IGVM interface
>+ *
>+ * Copyright (C) 2024 SUSE
>+ *
>+ * Authors:
>+ *  Roy Hopkins <roy.hopkins@suse.com>
>+ *
>+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
>+ * See the COPYING file in the top-level directory.
>+ */
>+
>+#ifndef QEMU_IGVM_CFG_H
>+#define QEMU_IGVM_CFG_H
>+
>+#include "qom/object.h"
>+
>+typedef struct IgvmCfg {
>+    ObjectClass parent_class;
>+
>+    /*
>+     * filename: Filename that specifies a file that contains the configuration
>+     *           of the guest in Independent Guest Virtual Machine (IGVM)
>+     *           format.
>+     */
>+    char *filename;
>+} IgvmCfg;
>+
>+typedef struct IgvmCfgClass {
>+    ObjectClass parent_class;
>+
>+    /*
>+     * If an IGVM filename has been specified then process the IGVM file.
>+     * Performs a no-op if no filename has been specified.
>+     *
>+     * Returns 0 for ok and -1 on error.
>+     */
>+    int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
>+                   Error **errp);
>+
>+} IgvmCfgClass;
>+
>+#define TYPE_IGVM_CFG "igvm-cfg"
>+
>+OBJECT_DECLARE_TYPE(IgvmCfg, IgvmCfgClass, IGVM_CFG)
>+
>+#endif
>diff --git a/qapi/qom.json b/qapi/qom.json
>index 321ccd708a..e148f3d005 100644
>--- a/qapi/qom.json
>+++ b/qapi/qom.json
>@@ -893,6 +893,19 @@
>   'data': { '*filename': 'str' },
>   'if': 'CONFIG_POSIX' }
>
>+##
>+# @IgvmCfgProperties:
>+#
>+# Properties common to objects that handle IGVM files.
>+#
>+# @file: IGVM file to use to configure guest
>+#
>+# Since: 9.1

Since we missed 9.1 release, we should update to 9.2.
Please check also other place.

The rest LGTM.

Thanks,
Stefano

>+##
>+{ 'struct': 'IgvmCfgProperties',
>+  'if': 'CONFIG_IGVM',
>+  'data': { 'file': 'str' } }
>+
> ##
> # @SevCommonProperties:
> #
>@@ -1063,6 +1076,8 @@
>     'filter-redirector',
>     'filter-replay',
>     'filter-rewriter',
>+    { 'name': 'igvm-cfg',
>+      'if': 'CONFIG_IGVM' },
>     'input-barrier',
>     { 'name': 'input-linux',
>       'if': 'CONFIG_LINUX' },
>@@ -1137,6 +1152,8 @@
>       'filter-redirector':          'FilterRedirectorProperties',
>       'filter-replay':              'NetfilterProperties',
>       'filter-rewriter':            'FilterRewriterProperties',
>+      'igvm-cfg':                   { 'type': 'IgvmCfgProperties',
>+                                      'if': 'CONFIG_IGVM' },
>       'input-barrier':              'InputBarrierProperties',
>       'input-linux':                { 'type': 'InputLinuxProperties',
>                                       'if': 'CONFIG_LINUX' },
>-- 
>2.43.0
>
Stefano Garzarella Sept. 2, 2024, 2:11 p.m. UTC | #4
On Tue, Aug 13, 2024 at 04:01:06PM GMT, 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 an X86 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>
>Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>---
> hw/i386/pc.c          | 12 ++++++++++++
> hw/i386/pc_piix.c     | 10 ++++++++++
> hw/i386/pc_q35.c      | 10 ++++++++++
> include/hw/i386/x86.h |  3 +++
> qemu-options.hx       | 25 +++++++++++++++++++++++++
> 5 files changed, 60 insertions(+)
>
>diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>index c74931d577..30bbe05e3e 100644
>--- a/hw/i386/pc.c
>+++ b/hw/i386/pc.c
>@@ -1827,6 +1827,18 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>     object_class_property_add_bool(oc, "fd-bootchk",
>         pc_machine_get_fd_bootchk,
>         pc_machine_set_fd_bootchk);
>+
>+#if defined(CONFIG_IGVM)
>+    object_class_property_add_link(oc, "igvm-cfg",
>+                                   TYPE_IGVM_CFG,
>+                                   offsetof(X86MachineState, igvm),
>+                                   object_property_allow_set_link,
>+                                   OBJ_PROP_LINK_STRONG);
>+    object_class_property_set_description(oc, "igvm-cfg",
>+                                          "Set IGVM configuration");
>+#endif
>+
>+
> }
>
> static const TypeInfo pc_machine_info = {
>diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>index d9e69243b4..78367985b4 100644
>--- a/hw/i386/pc_piix.c
>+++ b/hw/i386/pc_piix.c
>@@ -365,6 +365,16 @@ static void pc_init1(MachineState *machine, const char *pci_type)
>                                x86_nvdimm_acpi_dsmio,
>                                x86ms->fw_cfg, OBJECT(pcms));
>     }
>+
>+#if defined(CONFIG_IGVM)
>+    /* Apply guest state from IGVM if supplied */
>+    if (x86ms->igvm) {
>+        if (IGVM_CFG_GET_CLASS(x86ms->igvm)
>+                ->process(x86ms->igvm, machine->cgs, &error_fatal) < 0) {
>+            g_assert_not_reached();
>+        }
>+    }
>+#endif
> }
>
> typedef enum PCSouthBridgeOption {
>diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>index 9d108b194e..08ef8dc17a 100644
>--- a/hw/i386/pc_q35.c
>+++ b/hw/i386/pc_q35.c
>@@ -329,6 +329,16 @@ static void pc_q35_init(MachineState *machine)
>                                x86_nvdimm_acpi_dsmio,
>                                x86ms->fw_cfg, OBJECT(pcms));
>     }
>+
>+#if defined(CONFIG_IGVM)
>+    /* Apply guest state from IGVM if supplied */
>+    if (x86ms->igvm) {
>+        if (IGVM_CFG_GET_CLASS(x86ms->igvm)
>+                ->process(x86ms->igvm, machine->cgs, &error_fatal) < 0) {
>+            g_assert_not_reached();
>+        }
>+    }
>+#endif
> }
>
> #define DEFINE_Q35_MACHINE(major, minor) \
>diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
>index d43cb3908e..01ac29acf6 100644
>--- a/include/hw/i386/x86.h
>+++ b/include/hw/i386/x86.h
>@@ -25,6 +25,7 @@
> #include "hw/intc/ioapic.h"
> #include "hw/isa/isa.h"
> #include "qom/object.h"
>+#include "sysemu/igvm-cfg.h"
>
> struct X86MachineClass {
>     /*< private >*/
>@@ -97,6 +98,8 @@ struct X86MachineState {
>      * which means no limitation on the guest's bus locks.
>      */
>     uint64_t bus_lock_ratelimit;
>+
>+    IgvmCfg *igvm;
> };
>
> #define X86_MACHINE_SMM              "smm"
>diff --git a/qemu-options.hx b/qemu-options.hx
>index cee0da2014..b6eee49075 100644
>--- a/qemu-options.hx
>+++ b/qemu-options.hx
>@@ -5927,6 +5927,31 @@ SRST
>                  -machine ...,memory-encryption=sev0 \\
>                  .....
>
>+    ``-object igvm-cfg,file=file``
>+        Create an IGVM configuration object that defines the initial state
>+        of the guest using a file in that conforms to the Independent Guest
>+        Virtual Machine (IGVM) file format.
>+
>+        The ``file`` parameter is used to specify the IGVM file to load.
>+        When provided, the IGVM file is used to populate the initial
>+        memory of the virtual machine and, depending on the platform, can
>+        define the initial processor state, memory map and parameters.
>+
>+        The IGVM file is expected to contain the firmware for the virtual
>+        machine, therefore an ``igvm-cfg`` object cannot be provided along
>+        with other ways of specifying firmware, such as the ``-bios``
>+        parameter on x86 machines.
>+
>+        e.g to launch a machine providing the firmware in an IGVM file
>+
>+        .. parsed-literal::
>+
>+             # |qemu_system_x86| \\
>+                 ...... \\
>+                 -object igvm-cfg,id=igvm0,file=bios.igvm \\
>+                 -machine ...,igvm-cfg=igvm0 \\
>+                 .....
>+

Should we mention that this is supported only by `q35` and `pc` machines?

>     ``-object authz-simple,id=id,identity=string``
>         Create an authorization object that will control access to
>         network services.
>-- 
>2.43.0
>
Stefano Garzarella Sept. 2, 2024, 2:19 p.m. UTC | #5
On Tue, Aug 13, 2024 at 04:01:09PM GMT, Roy Hopkins wrote:
>The x86 segment registers are identified by the X86Seg enumeration which
>includes LDTR and TR as well as the normal segment registers. The
>function 'cpu_x86_load_seg_cache()' uses the enum to determine which
>segment to set. However, specifying R_LDTR or R_TR results in an
>out-of-bounds access of the segment array.
>
>Possibly by coincidence, the function does correctly set LDTR or TR in
>this case as the structures for these registers immediately follow the
>array which is accessed out of bounds.
>
>This patch adds correct handling for R_LDTR and R_TR in the function.
>
>Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
>Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>---
> target/i386/cpu.h | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

>
>diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>index c6cc035df3..227bf2600a 100644
>--- a/target/i386/cpu.h
>+++ b/target/i386/cpu.h
>@@ -2256,7 +2256,14 @@ static inline void cpu_x86_load_seg_cache(CPUX86State *env,
>     SegmentCache *sc;
>     unsigned int new_hflags;
>
>-    sc = &env->segs[seg_reg];
>+    if (seg_reg == R_LDTR) {
>+        sc = &env->ldt;
>+    } else if (seg_reg == R_TR) {
>+        sc = &env->tr;
>+    } else {
>+        sc = &env->segs[seg_reg];
>+    }
>+
>     sc->selector = selector;
>     sc->base = base;
>     sc->limit = limit;
>-- 
>2.43.0
>
Stefano Garzarella Sept. 2, 2024, 2:28 p.m. UTC | #6
On Tue, Aug 13, 2024 at 04:01:13PM GMT, Roy Hopkins wrote:
>Create an enum entry within FirmwareDevice for 'igvm' to describe that
>an IGVM file can be used to map firmware into memory as an alternative
>to pre-existing firmware devices.
>
>Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
>Acked-by: Michael S. Tsirkin <mst@redhat.com>
>---
> docs/interop/firmware.json | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
>diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
>index 57f55f6c54..59ae39cb13 100644
>--- a/docs/interop/firmware.json
>+++ b/docs/interop/firmware.json
>@@ -57,10 +57,17 @@
> #
> # @memory: The firmware is to be mapped into memory.
> #
>+# @igvm: The firmware is defined by a file conforming to the IGVM
>+#        specification and mapped into memory according to directives
>+#        defined in the file. This is similar to @memory but may
>+#        include additional processing defined by the IGVM file
>+#        including initial CPU state or population of metadata into
>+#        the guest address space. Since: 9.1

Since: 9.2

>+#
> # Since: 3.0
> ##
> { 'enum' : 'FirmwareDevice',
>-  'data' : [ 'flash', 'kernel', 'memory' ] }
>+  'data' : [ 'flash', 'kernel', 'memory', 'igvm' ] }
>
> ##
> # @FirmwareArchitecture:
>@@ -367,6 +374,24 @@
> { 'struct' : 'FirmwareMappingMemory',
>   'data'   : { 'filename' : 'str' } }
>
>+##
>+# @FirmwareMappingIgvm:
>+#
>+# Describes loading and mapping properties for the firmware executable,
>+# when @FirmwareDevice is @igvm.
>+#
>+# @filename: Identifies the IGVM file containing the firmware executable
>+#            along with other information used to configure the initial
>+#            state of the guest. The IGVM file may be shared by multiple
>+#            virtual machine definitions. This corresponds to creating
>+#            an object on the command line with "-object igvm-cfg,
>+#            file=@filename".
>+#
>+# Since: 9.1

Ditto

With them fixed:

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

>+##
>+{ 'struct' : 'FirmwareMappingIgvm',
>+  'data'   : { 'filename' : 'str' } }
>+
> ##
> # @FirmwareMapping:
> #
>@@ -383,7 +408,8 @@
>   'discriminator' : 'device',
>   'data'          : { 'flash'  : 'FirmwareMappingFlash',
>                       'kernel' : 'FirmwareMappingKernel',
>-                      'memory' : 'FirmwareMappingMemory' } }
>+                      'memory' : 'FirmwareMappingMemory',
>+                      'igvm'   : 'FirmwareMappingIgvm' } }
>
> ##
> # @Firmware:
>-- 
>2.43.0
>
Stefano Garzarella Sept. 2, 2024, 2:32 p.m. UTC | #7
On Tue, Aug 13, 2024 at 04:01:15PM GMT, Roy Hopkins wrote:
>The initialization sections in IGVM files contain configuration that
>should be applied to the guest platform before it is started. This
>includes guest policy and other information that can affect the security
>level and the startup measurement of a guest.
>
>This commit introduces handling of the initialization sections during
>processing of the IGVM file.
>
>Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
>Acked-by: Michael S. Tsirkin <mst@redhat.com>
>---
> backends/igvm.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

>
>diff --git a/backends/igvm.c b/backends/igvm.c
>index 7a3fedcc76..9120922a95 100644
>--- a/backends/igvm.c
>+++ b/backends/igvm.c
>@@ -787,6 +787,27 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
>         }
>     }
>
>+    header_count =
>+        igvm_header_count(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION);
>+    if (header_count < 0) {
>+        error_setg(
>+            errp,
>+            "Invalid initialization header count in IGVM file. Error code: %X",
>+            header_count);
>+        return -1;
>+    }
>+
>+    for (ctx.current_header_index = 0;
>+         ctx.current_header_index < (unsigned)header_count;
>+         ctx.current_header_index++) {
>+        IgvmVariableHeaderType type =
>+            igvm_get_header_type(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION,
>+                                 ctx.current_header_index);
>+        if (qigvm_handler(&ctx, type, errp) < 0) {
>+            goto cleanup;
>+        }
>+    }
>+
>     /*
>      * Contiguous pages of data with compatible flags are grouped together in
>      * order to reduce the number of memory regions we create. Make sure the
>-- 
>2.43.0
>
Stefano Garzarella Sept. 2, 2024, 2:40 p.m. UTC | #8
On Tue, Aug 13, 2024 at 04:01:02PM GMT, Roy Hopkins wrote:
>Here is v5 of the set of patches to add support for IGVM files to QEMU. This is
>based on commit 0f397dcfec of qemu.
>
>This version addresses the review comments from v4 [1] plus changes required to
>rebase on the master commit. As always, thanks to those that have been following
>along, reviewing and testing this series. This v5 patch series is also available
>on github: [2]
>
>For testing IGVM support in QEMU you need to generate an IGVM file that is
>configured for the platform you want to launch. You can use the `buildigvm`
>test tool [3] to allow generation of IGVM files for all currently supported
>platforms. Patch 11/17 contains information on how to generate an IGVM file
>using this tool.

I left some minor comments, the patches I didn't respond to are too much
in detail for my knowledge, but I looked at them and I didn't find
anything obviously wrong, so for those feel free to add:

Acked-by: Stefano Garzarella <sgarzare@redhat.com>

Thanks,
Stefano

>
>Changes in v5:
>
>* Fix indentation and apply minimum version check for IGVM library in meson.build
>* Remove unneeded duplicate macro definitions in confidential-guest-support.h
>  and igvm-cvg.h
>* Make igvm-cfg object file parameter mandatory instead of optional. Removed
>  unused 'igvm_process()' function that checked the file was provided.
>* Rename all QEMU IGVM functions and structs using QIGVM/qigvm prefix.
>* A few small readability/style fixes.
>* Address review comments on error handling, including removal of the v4 patch
>  6: "Fix error handling in sev_encrypt_flash()".
>* Update `FirmwareMapping` union in firmware.json to include `igvm`.
>
>Patch summary:
>
>1-11: Add support and documentation for processing IGVM files for SEV, SEV-ES,
>SEV-SNP and native platforms.
>
>12-15: Processing of policy and SEV-SNP ID_BLOCK from IGVM file.
>
>16: Add pre-processing of IGVM file to support synchronization of 'SEV_FEATURES'
>from IGVM VMSA to KVM.
>
>[1] Link to v4:
>https://lore.kernel.org/qemu-devel/cover.1720004383.git.roy.hopkins@suse.com/
>
>[2] v5 patches also available here:
>https://github.com/roy-hopkins/qemu/tree/igvm_master_v5
>
>[3] `buildigvm` tool v0.2.0
>https://github.com/roy-hopkins/buildigvm/releases/tag/v0.2.0
>
>Roy Hopkins (16):
>  meson: Add optional dependency on IGVM library
>  backends/confidential-guest-support: Add functions to support IGVM
>  backends/igvm: Add IGVM loader and configuration
>  hw/i386: Add igvm-cfg object and processing for IGVM files
>  i386/pc_sysfw: Ensure sysfw flash configuration does not conflict with
>    IGVM
>  sev: Update launch_update_data functions to use Error handling
>  target/i386: Allow setting of R_LDTR and R_TR with
>    cpu_x86_load_seg_cache()
>  i386/sev: Refactor setting of reset vector and initial CPU state
>  i386/sev: Implement ConfidentialGuestSupport functions for SEV
>  docs/system: Add documentation on support for IGVM
>  docs/interop/firmware.json: Add igvm to FirmwareDevice
>  backends/confidential-guest-support: Add set_guest_policy() function
>  backends/igvm: Process initialization sections in IGVM file
>  backends/igvm: Handle policy for SEV guests
>  i386/sev: Add implementation of CGS set_guest_policy()
>  sev: Provide sev_features flags from IGVM VMSA to KVM_SEV_INIT2
>
> backends/confidential-guest-support.c      |  43 +
> backends/igvm-cfg.c                        |  52 ++
> backends/igvm.c                            | 964 +++++++++++++++++++++
> backends/igvm.h                            |  23 +
> backends/meson.build                       |   5 +
> docs/interop/firmware.json                 |  30 +-
> docs/system/i386/amd-memory-encryption.rst |   2 +
> docs/system/igvm.rst                       | 173 ++++
> docs/system/index.rst                      |   1 +
> hw/i386/pc.c                               |  12 +
> hw/i386/pc_piix.c                          |  10 +
> hw/i386/pc_q35.c                           |  10 +
> hw/i386/pc_sysfw.c                         |  31 +-
> include/exec/confidential-guest-support.h  |  86 ++
> include/hw/i386/x86.h                      |   3 +
> include/sysemu/igvm-cfg.h                  |  47 +
> meson.build                                |   8 +
> meson_options.txt                          |   2 +
> qapi/qom.json                              |  17 +
> qemu-options.hx                            |  25 +
> scripts/meson-buildoptions.sh              |   3 +
> target/i386/cpu.h                          |   9 +-
> target/i386/sev.c                          | 850 ++++++++++++++++--
> target/i386/sev.h                          | 124 +++
> 24 files changed, 2446 insertions(+), 84 deletions(-)
> create mode 100644 backends/igvm-cfg.c
> create mode 100644 backends/igvm.c
> create mode 100644 backends/igvm.h
> create mode 100644 docs/system/igvm.rst
> create mode 100644 include/sysemu/igvm-cfg.h
>
>-- 
>2.43.0
>