Re: [PATCH v4 00/17] Introduce support for IGVM files

[Michael S. Tsirkin]

On Tue, Aug 13, 2024 at 10:53:58AM +0100, Roy Hopkins wrote:
> On Sat, 2024-07-20 at 14:26 -0400, Michael S. Tsirkin wrote:
> > On Wed, Jul 03, 2024 at 12:05:38PM +0100, Roy Hopkins wrote:
> > > Here is v4 of the set of patches to add support for IGVM files to QEMU. 
> > > This
> > > is
> > > based on commit 1a2d52c7fc of qemu.
> > > 
> > > This version addresses all of the review comments from v3 along with a
> > > couple of
> > > small bug fixes. This is a much smaller increment than in the previous
> > > version
> > > of the series [1]. Thanks once again to the reviewers that have been 
> > > looking
> > > at
> > > this series. This v4 patch series is also available on github: [2]
> > > 
> > > The previous version had a build issue when building without debug 
> > > enabled.
> > > Patch 8/17 has been added to fix this and I've updated my own process to
> > > test
> > > both debug and release builds of QEMU.
> > > 
> > > 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.
> > 
> > PC things:
> > 
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > 
> 
> Hi Michael,
> 
> Thanks for this. Can I add your ack to all commits, or just the PC specific
> ones?
> 
> Regards,
> Roy


I reviewed the pc things and skimmed the rest. So reviewed-by
for pc things and Ack for the rest.

> > > Changes in v4:
> > > 
> > > * Remove unused '#ifdef CONFIG_IGVM' sections
> > > * Add "'if': 'CONFIG_IGVM'" for IgvmCfgProperties in qom.json
> > > * Use error_fatal instead of error_abort in suggested locations
> > > * Prevent addition of bios code when an IGVM file is provided and
> > > pci_enabled is false
> > > * Add patch 6/17 to fix error handling from sev_encrypt_flash()
> > > * Revert unrequired changes to return values in sev/*_launch_update()
> > > functions
> > > * Add documentation to igvm.rst to describe how to use 'buildigvm'
> > > * Various convention and code style changes as suggested in reviews
> > > * Fix handling of sev_features for kernels that do not support 
> > > KVM_SEV_INIT2
> > > * Move igvm-cfg from MachineState to X86MachineState
> > > 
> > > Patch summary:
> > > 
> > > 1-12: Add support and documentation for processing IGVM files for SEV, 
> > > SEV-
> > > ES,
> > > SEV-SNP and native platforms. 
> > > 
> > > 13-16: Processing of policy and SEV-SNP ID_BLOCK from IGVM file. 
> > > 
> > > 17: Add pre-processing of IGVM file to support synchronization of
> > > 'SEV_FEATURES'
> > > from IGVM VMSA to KVM.
> > > 
> > > [1] Link to v3:
> > > https://lore.kernel.org/qemu-devel/cover.1718979106.git.roy.hopkins@suse.com/
> > > 
> > > [2] v4 patches also available here:
> > > https://github.com/roy-hopkins/qemu/tree/igvm_master_v4
> > > 
> > > [3] `buildigvm` tool v0.2.0
> > > https://github.com/roy-hopkins/buildigvm/releases/tag/v0.2.0
> > > 
> > > Roy Hopkins (17):
> > >   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: Fix error handling in sev_encrypt_flash()
> > >   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
> > > 
> > >  docs/interop/firmware.json                 |   9 +-
> > >  docs/system/i386/amd-memory-encryption.rst |   2 +
> > >  docs/system/igvm.rst                       | 173 ++++
> > >  docs/system/index.rst                      |   1 +
> > >  meson.build                                |   8 +
> > >  qapi/qom.json                              |  17 +
> > >  backends/igvm.h                            |  23 +
> > >  include/exec/confidential-guest-support.h  |  96 +++
> > >  include/hw/i386/x86.h                      |   3 +
> > >  include/sysemu/igvm-cfg.h                  |  54 ++
> > >  target/i386/cpu.h                          |   9 +-
> > >  target/i386/sev.h                          | 124 +++
> > >  backends/confidential-guest-support.c      |  43 +
> > >  backends/igvm-cfg.c                        |  66 ++
> > >  backends/igvm.c                            | 958 +++++++++++++++++++++
> > >  hw/i386/pc.c                               |  12 +
> > >  hw/i386/pc_piix.c                          |  10 +
> > >  hw/i386/pc_q35.c                           |  10 +
> > >  hw/i386/pc_sysfw.c                         |  31 +-
> > >  target/i386/sev.c                          | 844 ++++++++++++++++--
> > >  backends/meson.build                       |   5 +
> > >  meson_options.txt                          |   2 +
> > >  qemu-options.hx                            |  25 +
> > >  scripts/meson-buildoptions.sh              |   3 +
> > >  24 files changed, 2447 insertions(+), 81 deletions(-)
> > >  create mode 100644 docs/system/igvm.rst
> > >  create mode 100644 backends/igvm.h
> > >  create mode 100644 include/sysemu/igvm-cfg.h
> > >  create mode 100644 backends/igvm-cfg.c
> > >  create mode 100644 backends/igvm.c
> > > 
> > > -- 
> > > 2.43.0
> > 
>