Re: [PATCH 0/9] Introduce support for IGVM files

[Stefano Garzarella]

Hi Roy,
thanks for this series!

On Tue, Feb 27, 2024 at 02:50:06PM +0000, Roy Hopkins wrote:
> Hi everyone,
>
> This initial patch series submission adds the capability to configure
> confidential guests using files that conform to the Independent Guest Virtual
> Machine (IGVM) file format. The series is based on the master branch commit
> 1b330da. Alternatively, the series is available here:
> https://github.com/roy-hopkins/qemu/tree/igvm_master_v1
>
> I look forward to welcoming your comments!

I saw that the series has been a posted for a while and maybe you're 
going to send v2, so I'll bring back some little things that I saw, but 
I didn't do yet a deep review:

- We use "Isolated Guest Virtual Machine" or "Independent Guest Virtual
  Machine", are they interchangeable for IGVM?

- `./scripts/checkpatch.pl --codespell` reported some warnings:

5/9 Checking commit 81f60e5cdd01 (i386/pc: Process IGVM file during PC 
initialization if present)
WARNING: 'initalization' may be misspelled - perhaps 'initialization'?
#15:
    initalization of the target.
    ^^^^^^^^^^^^^

9/9 Checking commit 66745c0bb940 (docs/system: Add documentation on support for 
IGVM)

WARNING: 'encaspulate' may be misspelled - perhaps 'encapsulate'?
#27: FILE: docs/system/igvm.rst:4:
+IGVM files are designed to encaspulate all the information required to launch a
                            ^^^^^^^^^^^

Thanks,
Stefano

> Why do we need Independent Guest Virtual Machine (IGVM) files?
> ==============================================================
>
> IGVM files describe, using a set of directives, the memory layout and initial
> configuration of a guest that supports isolation technologies such as AMD
> SEV-SNP and Intel TDX. By encapsulating all of this information in a single
> configuration file and applying the directives in the order they are specified
> when the guest is initialized, it becomes straightforward to pre-calculate the
> cryptographic measurement of the guest initial state, thus aiding in remote
> attestation processes.
>
> IGVM files can also be used to configure non-standard guest memory layouts,
> payloads or startup configurations. A good example of this is to use IGVM to
> deploy and configure an SVSM module in the guest which supports running at
> multiple VMPLs. The SVSM can be configured to start directly into 32-bit or
> 64-bit code. This patch series was developed with this purpose in mind to
> support the COCONUT-SVSM project:
> https://github.com/coconut-svsm/svsm
>
> More information and background on the IGVM file format can be found on the
> project page at:
> https://github.com/microsoft/igvm
>
> What this patch series introduces
> =================================
>
> This series adds a build-time configuration option (--enable-igvm) to add
> support for launching a guest using an IGVM file. It extends the current
> ConfidentialGuestSupport object to allow an IGVM filename to be specified.
>
> The directives in the IGVM file are parsed and the confidential guest is
> configured through new virtual methods added to the ConfidentialGuestSupport
> object. These virtual functions have been implemented for AMD SEV and AMD
> SEV-ES.
>
> Many of the IGVM directives require capabilities that are not supported in SEV
> and SEV-ES, so support for IGVM directives will need to be considered when
> support for SEV-SNP, TDX or other technologies is introduced to QEMU. Any
> directive that is not currently supported results in an error report.
>
> Dependencies
> ============
>
> In order to enable IGVM support, you will need the IGVM library installed.
> Instructions on building and installing it can be found here:
> https://github.com/microsoft/igvm/tree/main/igvm_c
>
> As mentioned above, this series was developed as part of the effort for
> COCONUT-SVSM. COCONUT-SVSM requires support for AMD SEV-SNP which is not
> available in current QEMU. Therefore this series has also been applied on top of
> the AMD SEV-SNP branch (https://github.com/AMDESE/qemu/tree/snp-v3-wip). You can
> find that version of the series here:
> https://github.com/roy-hopkins/qemu/commits/snp-v3-wip-igvm_v2/
>
> Generating IGVM files
> =====================
>
> To try this out you will need to generate an IGVM file that is compatible with
> the SEV platform you are testing on. I've created a tool that can create a
> simple IGVM file that packages an OVMF binary for AMD SEV or AMD SEV-ES. The
> tool is available here:
> https://github.com/roy-hopkins/buildigvm
>
> I have tested this on an AMD EPYC Genoa system configured to support SEV. Both
> SEV and SEV-ES have been tested using IGVM files generated using the buildigvm
> tool. The SEV-SNP alternative patch set has also been tested using COCONUT-SVSM.
>
> Roy Hopkins (9):
>  meson: Add optional dependency on IGVM library
>  backends/confidential-guest-support: Add IGVM file parameter
>  backends/confidential-guest-support: Add functions to support IGVM
>  backends/igvm: Implement parsing and processing of IGVM files
>  i386/pc: Process IGVM file during PC initialization if present
>  i386/pc: Skip initialization of system FW when using IGVM
>  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
>
> backends/confidential-guest-support.c     |  69 +++
> backends/igvm.c                           | 718 ++++++++++++++++++++++
> backends/meson.build                      |   4 +
> docs/system/igvm.rst                      |  58 ++
> docs/system/index.rst                     |   1 +
> hw/i386/pc.c                              |  12 +-
> hw/i386/pc_piix.c                         |   4 +
> hw/i386/pc_q35.c                          |   4 +
> include/exec/confidential-guest-support.h | 107 ++++
> include/exec/igvm.h                       |  35 ++
> meson.build                               |   8 +
> meson_options.txt                         |   2 +
> qapi/qom.json                             |  13 +
> qemu-options.hx                           |   8 +-
> scripts/meson-buildoptions.sh             |   3 +
> target/i386/sev.c                         | 365 ++++++++++-
> target/i386/sev.h                         | 110 ++++
> 17 files changed, 1489 insertions(+), 32 deletions(-)
> create mode 100644 backends/igvm.c
> create mode 100644 docs/system/igvm.rst
> create mode 100644 include/exec/igvm.h
>
> --
> 2.43.0