Re: [PATCH v5 00/16] Introduce support for IGVM files

[Roy Hopkins <roy.hopkins@suse.com>]

On Mon, 2024-09-02 at 16:40 +0200, Stefano Garzarella wrote:
> 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
> 

Many thanks for reviewing/acking this Stefano.

Regards,
Roy

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