backends/confidential-guest-support.c | 43 + backends/igvm-cfg.c | 52 ++ backends/igvm.c | 967 ++++++++++++++++++++ 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/hw/i386/x86.h | 3 + include/system/confidential-guest-support.h | 88 ++ include/system/igvm-cfg.h | 47 + meson.build | 8 + meson_options.txt | 2 + qapi/qom.json | 17 + qemu-options.hx | 28 + scripts/meson-buildoptions.sh | 3 + target/i386/cpu.h | 9 +- target/i386/sev.c | 850 +++++++++++++++-- target/i386/sev.h | 124 +++ 24 files changed, 2454 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/system/igvm-cfg.h
Here is v7 of the set of patches to add support for IGVM files to QEMU. This is based on commit 40efe733e10cc00e4fb4f9f5790a28e744e63c62 of qemu. Firstly, apologies for the amount of time between the last version and this one. I moved roles to a different company and, although I always planned to see this patch series to completion, it took a while before I found time to setup a development environment and be in a position to send a new version. I will continue this series using a personal email address for now, hence the change to the author and signed-off-by emails. The only changes in this version are to rebase on the current master branch and update commit metadata, including Signed-Off-By and Author emails for my replacement email address, and to include the final Reviewed-By that were added in the last review. There were no requested changes on the previous version [1] so I believe this series is ready to merge. As always, thanks to those that have been following along, reviewing and testing this series. This v7 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 v7: * Update version numbers for IGVM support to 10.0 * Add Reviewed-by to relevant commits. * Update Author email and sign-offs to my new email address 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 v6: https://lore.kernel.org/qemu-devel/cover.1727341768.git.roy.hopkins@suse.com/ [2] v7 patches also available here: https://github.com/roy-hopkins/qemu/tree/igvm_master_v7 [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 | 967 ++++++++++++++++++++ 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/hw/i386/x86.h | 3 + include/system/confidential-guest-support.h | 88 ++ include/system/igvm-cfg.h | 47 + meson.build | 8 + meson_options.txt | 2 + qapi/qom.json | 17 + qemu-options.hx | 28 + scripts/meson-buildoptions.sh | 3 + target/i386/cpu.h | 9 +- target/i386/sev.c | 850 +++++++++++++++-- target/i386/sev.h | 124 +++ 24 files changed, 2454 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/system/igvm-cfg.h -- 2.43.0
Hi Roy, On Thu, Feb 27, 2025 at 01:38:08PM +0000, Roy Hopkins wrote: >Here is v7 of the set of patches to add support for IGVM files to QEMU. This is >based on commit 40efe733e10cc00e4fb4f9f5790a28e744e63c62 of qemu. Thanks again for this work! I noticed that the last patch for this series is missing, also patchew didn't receive it: https://patchew.org/QEMU/cover.1740663410.git.roy.hopkins@randomman.co.uk/ If you're using git-publish you can do: $ git publish --skip 16 -S \ -R cover.1740663410.git.roy.hopkins@randomman.co.uk Thanks, Stefano > >Firstly, apologies for the amount of time between the last version and this one. >I moved roles to a different company and, although I always planned to see this >patch series to completion, it took a while before I found time to setup a >development environment and be in a position to send a new version. I will >continue this series using a personal email address for now, hence the change >to the author and signed-off-by emails. > >The only changes in this version are to rebase on the current master branch and >update commit metadata, including Signed-Off-By and Author emails for my >replacement email address, and to include the final Reviewed-By that were added >in the last review. There were no requested changes on the previous version [1] >so I believe this series is ready to merge. > >As always, thanks to those that have been following along, reviewing and testing >this series. This v7 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 v7: > >* Update version numbers for IGVM support to 10.0 >* Add Reviewed-by to relevant commits. >* Update Author email and sign-offs to my new email address > >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 v6: >https://lore.kernel.org/qemu-devel/cover.1727341768.git.roy.hopkins@suse.com/ > >[2] v7 patches also available here: >https://github.com/roy-hopkins/qemu/tree/igvm_master_v7 > >[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 | 967 ++++++++++++++++++++ > 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/hw/i386/x86.h | 3 + > include/system/confidential-guest-support.h | 88 ++ > include/system/igvm-cfg.h | 47 + > meson.build | 8 + > meson_options.txt | 2 + > qapi/qom.json | 17 + > qemu-options.hx | 28 + > scripts/meson-buildoptions.sh | 3 + > target/i386/cpu.h | 9 +- > target/i386/sev.c | 850 +++++++++++++++-- > target/i386/sev.h | 124 +++ > 24 files changed, 2454 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/system/igvm-cfg.h > >-- >2.43.0 >
On Thu, 2025-02-27 at 16:32 +0100, Stefano Garzarella wrote: > Hi Roy, > > On Thu, Feb 27, 2025 at 01:38:08PM +0000, Roy Hopkins wrote: > > Here is v7 of the set of patches to add support for IGVM files to > > QEMU. This is > > based on commit 40efe733e10cc00e4fb4f9f5790a28e744e63c62 of qemu. > > Thanks again for this work! > > I noticed that the last patch for this series is missing, also > patchew > didn't receive it: > > https://patchew.org/QEMU/cover.1740663410.git.roy.hopkins@randomman.co.uk/ > > If you're using git-publish you can do: > > $ git publish --skip 16 -S \ > -R cover.1740663410.git.roy.hopkins@randomman.co.uk > > Thanks, > Stefano > Thanks Stefano. I had all sorts of problems getting git send-mail to send using my mail service provider. I've sorted it now and sent the missing patch. Regards, Roy > > > > Firstly, apologies for the amount of time between the last version > > and this one. > > I moved roles to a different company and, although I always planned > > to see this > > patch series to completion, it took a while before I found time to > > setup a > > development environment and be in a position to send a new version. > > I will > > continue this series using a personal email address for now, hence > > the change > > to the author and signed-off-by emails. > > > > The only changes in this version are to rebase on the current > > master branch and > > update commit metadata, including Signed-Off-By and Author emails > > for my > > replacement email address, and to include the final Reviewed-By > > that were added > > in the last review. There were no requested changes on the previous > > version [1] > > so I believe this series is ready to merge. > > > > As always, thanks to those that have been following along, > > reviewing and testing > > this series. This v7 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 v7: > > > > * Update version numbers for IGVM support to 10.0 > > * Add Reviewed-by to relevant commits. > > * Update Author email and sign-offs to my new email address > > > > 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 v6: > > https://lore.kernel.org/qemu-devel/cover.1727341768.git.roy.hopkins@suse.com/ > > > > [2] v7 patches also available here: > > https://github.com/roy-hopkins/qemu/tree/igvm_master_v7 > > > > [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 | 967 > > ++++++++++++++++++++ > > 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/hw/i386/x86.h | 3 + > > include/system/confidential-guest-support.h | 88 ++ > > include/system/igvm-cfg.h | 47 + > > meson.build | 8 + > > meson_options.txt | 2 + > > qapi/qom.json | 17 + > > qemu-options.hx | 28 + > > scripts/meson-buildoptions.sh | 3 + > > target/i386/cpu.h | 9 +- > > target/i386/sev.c | 850 +++++++++++++++-- > > target/i386/sev.h | 124 +++ > > 24 files changed, 2454 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/system/igvm-cfg.h > > > > -- > > 2.43.0 > > >
On Thu, 27 Feb 2025 at 17:12, Roy Hopkins <roy.hopkins@randomman.co.uk> wrote: > > On Thu, 2025-02-27 at 16:32 +0100, Stefano Garzarella wrote: > > Hi Roy, > > > > On Thu, Feb 27, 2025 at 01:38:08PM +0000, Roy Hopkins wrote: > > > Here is v7 of the set of patches to add support for IGVM files to > > > QEMU. This is > > > based on commit 40efe733e10cc00e4fb4f9f5790a28e744e63c62 of qemu. > > > > Thanks again for this work! > > > > I noticed that the last patch for this series is missing, also > > patchew > > didn't receive it: > > > > https://patchew.org/QEMU/cover.1740663410.git.roy.hopkins@randomman.co.uk/ > > > > If you're using git-publish you can do: > > > > $ git publish --skip 16 -S \ > > -R cover.1740663410.git.roy.hopkins@randomman.co.uk > > > > Thanks, > > Stefano > > > > Thanks Stefano. I had all sorts of problems getting git send-mail to > send using my mail service provider. I've sorted it now and sent the > missing patch. Yeah, now the other attempts of patch 16 have also arrived xD On Monday I'll go through the series, test it and come back! Thanks, Stefano > > Regards, > Roy > > > > > > > Firstly, apologies for the amount of time between the last version > > > and this one. > > > I moved roles to a different company and, although I always planned > > > to see this > > > patch series to completion, it took a while before I found time to > > > setup a > > > development environment and be in a position to send a new version. > > > I will > > > continue this series using a personal email address for now, hence > > > the change > > > to the author and signed-off-by emails. > > > > > > The only changes in this version are to rebase on the current > > > master branch and > > > update commit metadata, including Signed-Off-By and Author emails > > > for my > > > replacement email address, and to include the final Reviewed-By > > > that were added > > > in the last review. There were no requested changes on the previous > > > version [1] > > > so I believe this series is ready to merge. > > > > > > As always, thanks to those that have been following along, > > > reviewing and testing > > > this series. This v7 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 v7: > > > > > > * Update version numbers for IGVM support to 10.0 > > > * Add Reviewed-by to relevant commits. > > > * Update Author email and sign-offs to my new email address > > > > > > 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 v6: > > > https://lore.kernel.org/qemu-devel/cover.1727341768.git.roy.hopkins@suse.com/ > > > > > > [2] v7 patches also available here: > > > https://github.com/roy-hopkins/qemu/tree/igvm_master_v7 > > > > > > [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 | 967 > > > ++++++++++++++++++++ > > > 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/hw/i386/x86.h | 3 + > > > include/system/confidential-guest-support.h | 88 ++ > > > include/system/igvm-cfg.h | 47 + > > > meson.build | 8 + > > > meson_options.txt | 2 + > > > qapi/qom.json | 17 + > > > qemu-options.hx | 28 + > > > scripts/meson-buildoptions.sh | 3 + > > > target/i386/cpu.h | 9 +- > > > target/i386/sev.c | 850 +++++++++++++++-- > > > target/i386/sev.h | 124 +++ > > > 24 files changed, 2454 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/system/igvm-cfg.h > > > > > > -- > > > 2.43.0 > > > > > >
On Thu, Feb 27, 2025 at 01:38:08PM +0000, Roy Hopkins wrote: > Here is v7 of the set of patches to add support for IGVM files to QEMU. This is > based on commit 40efe733e10cc00e4fb4f9f5790a28e744e63c62 of qemu. > > Firstly, apologies for the amount of time between the last version and this one. > I moved roles to a different company and, although I always planned to see this > patch series to completion, it took a while before I found time to setup a > development environment and be in a position to send a new version. I will > continue this series using a personal email address for now, hence the change > to the author and signed-off-by emails. > > The only changes in this version are to rebase on the current master branch and > update commit metadata, including Signed-Off-By and Author emails for my > replacement email address, and to include the final Reviewed-By that were added > in the last review. There were no requested changes on the previous version [1] > so I believe this series is ready to merge. > > As always, thanks to those that have been following along, reviewing and testing > this series. This v7 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. Looks good to me overall, although I don't know SEV good enough to review these changes in detail. Acked-by: Gerd Hoffmann <kraxel@redhat.com> take care, Gerd
Hi Roy, On Thu, 27 Feb 2025 at 14:47, Roy Hopkins <roy.hopkins@randomman.co.uk> wrote: > > Here is v7 of the set of patches to add support for IGVM files to QEMU. This is > based on commit 40efe733e10cc00e4fb4f9f5790a28e744e63c62 of qemu. > > Firstly, apologies for the amount of time between the last version and this one. > I moved roles to a different company and, although I always planned to see this > patch series to completion, it took a while before I found time to setup a > development environment and be in a position to send a new version. I will > continue this series using a personal email address for now, hence the change > to the author and signed-off-by emails. > > The only changes in this version are to rebase on the current master branch and > update commit metadata, including Signed-Off-By and Author emails for my > replacement email address, and to include the final Reviewed-By that were added > in the last review. There were no requested changes on the previous version [1] > so I believe this series is ready to merge. > > As always, thanks to those that have been following along, reviewing and testing > this series. This v7 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 was testing this series with the IGVM file generated by COCONUT SVSM, but QEMU was failing in this way: qemu-system-x86_64: KVM does not support guest_memfd qemu-system-x86_64: failed to initialize kvm: Operation not permitted After spending some time debugging, I found that IGVM is parsed in kvm_arch_init(). One of the handler called during the parsing is qigvm_prepare_memory(), which adds a new memory region calling memory_region_init_ram_guest_memfd(), but it fails: kvm_arch_init() -> qigvm_prepare_memory() -> memory_region_init_ram_guest_memfd() -> kvm_create_guest_memfd() ... if (!kvm_guest_memfd_supported) { error_setg(errp, "KVM does not support guest_memfd"); return -1; } So, I applied the following change and SVSM booted! diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index f89568bfa3..840f36675e 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -2722,17 +2722,17 @@ static int kvm_init(MachineState *ms) kvm_state = s; - ret = kvm_arch_init(ms, s); - if (ret < 0) { - goto err; - } - kvm_supported_memory_attributes = kvm_vm_check_extension(s, KVM_CAP_MEMORY_ATTRIBUTES); kvm_guest_memfd_supported = kvm_check_extension(s, KVM_CAP_GUEST_MEMFD) && kvm_check_extension(s, KVM_CAP_USER_MEMORY2) && (kvm_supported_memory_attributes & KVM_MEMORY_ATTRIBUTE_PRIVATE); + ret = kvm_arch_init(ms, s); + if (ret < 0) { + goto err; + } + if (s->kernel_irqchip_split == ON_OFF_AUTO_AUTO) { s->kernel_irqchip_split = mc->default_kernel_irqchip_split ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; } Checking, I discovered that it was done on purpose by Paolo, so not sure if my fix is valid: commit 586d708c1e3e5e29a0b3c05c347290aed9478854 Author: Paolo Bonzini <pbonzini@redhat.com> Date: Fri Oct 11 10:39:58 2024 +0200 accel/kvm: check for KVM_CAP_MEMORY_ATTRIBUTES on vm The exact set of available memory attributes can vary by VM. In the future it might vary depending on enabled capabilities, too. Query the extension on the VM level instead of on the KVM level, and only after architecture-specific initialization. Inspired by an analogous patch by Tom Dohrmann. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> @Paolo any suggestion? Thanks, Stefano
On Wed, 2025-03-05 at 16:47 +0100, Stefano Garzarella wrote: > Hi Roy, > > I was testing this series with the IGVM file generated by COCONUT SVSM, > but QEMU was failing in this way: > > qemu-system-x86_64: KVM does not support guest_memfd > qemu-system-x86_64: failed to initialize kvm: Operation not permitted > > After spending some time debugging, I found that IGVM is parsed in > kvm_arch_init(). One of the handler called during the parsing is > qigvm_prepare_memory(), which adds a new memory region calling > memory_region_init_ram_guest_memfd(), but it fails: > > kvm_arch_init() > -> qigvm_prepare_memory() > -> memory_region_init_ram_guest_memfd() > -> kvm_create_guest_memfd() > ... > if (!kvm_guest_memfd_supported) { > error_setg(errp, "KVM does not support guest_memfd"); > return -1; > } > > So, I applied the following change and SVSM booted! > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index f89568bfa3..840f36675e 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -2722,17 +2722,17 @@ static int kvm_init(MachineState *ms) > > kvm_state = s; > > - ret = kvm_arch_init(ms, s); > - if (ret < 0) { > - goto err; > - } > - > kvm_supported_memory_attributes = kvm_vm_check_extension(s, KVM_CAP_MEMORY_ATTRIBUTES); > kvm_guest_memfd_supported = > kvm_check_extension(s, KVM_CAP_GUEST_MEMFD) && > kvm_check_extension(s, KVM_CAP_USER_MEMORY2) && > (kvm_supported_memory_attributes & KVM_MEMORY_ATTRIBUTE_PRIVATE); > > + ret = kvm_arch_init(ms, s); > + if (ret < 0) { > + goto err; > + } > + > if (s->kernel_irqchip_split == ON_OFF_AUTO_AUTO) { > s->kernel_irqchip_split = mc->default_kernel_irqchip_split ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; > } > > Checking, I discovered that it was done on purpose by Paolo, so not sure > if my fix is valid: > > commit 586d708c1e3e5e29a0b3c05c347290aed9478854 > Author: Paolo Bonzini <pbonzini@redhat.com> > Date: Fri Oct 11 10:39:58 2024 +0200 > > accel/kvm: check for KVM_CAP_MEMORY_ATTRIBUTES on vm > > The exact set of available memory attributes can vary by VM. In the > future it might vary depending on enabled capabilities, too. Query the > extension on the VM level instead of on the KVM level, and only after > architecture-specific initialization. > > Inspired by an analogous patch by Tom Dohrmann. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > @Paolo any suggestion? > > Thanks, > Stefano > Hi Stefano, Thanks for testing this. The problem seems to be down to the fact that I had to introduce an initial parsing of the IGVM file during initialization to extract sev_features. I was parsing all directives in the file but it appears this has some unwanted side effects. Please could you try the patch below to see if it fixes the issue? If it does I'll incorporate it into the patch series and resubmit. From 3590460ec3945b02a679ad79735681a642596d60 Mon Sep 17 00:00:00 2001 From: Roy Hopkins <roy.hopkins@randomman.co.uk> Date: Thu, 6 Mar 2025 11:25:07 +0000 Subject: [PATCH 1/1] backends/igvm: Add function to process only VP context When initializing kvm for SEV, the sev_features need to be passed to the initialization function. When using IGVM files, sev_features is provided in the VP context definintions in the file. Currently this is handled in sev.c by processing the entire file to extract the VP context, however this has unwanted side-effects. Therefore this commit adds a new function that allows only the VP context definitions to be parsed in the IGVM file. Signed-off-by: Roy Hopkins <roy.hopkins@randomman.co.uk> --- backends/igvm-cfg.c | 1 + backends/igvm.c | 51 +++++++++++++++++++++++++++++++++++++++ backends/igvm.h | 3 +++ include/system/igvm-cfg.h | 10 ++++++++ target/i386/sev.c | 10 ++++---- 5 files changed, 70 insertions(+), 5 deletions(-) diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c index 38f17dae44..25c4469768 100644 --- a/backends/igvm-cfg.c +++ b/backends/igvm-cfg.c @@ -41,6 +41,7 @@ static void igvm_cfg_class_init(ObjectClass *oc, void *data) "Set the IGVM filename to use"); igvmc->process = qigvm_process_file; + igvmc->process_vp_context = qigvm_process_vp_context; } static void igvm_cfg_init(Object *obj) diff --git a/backends/igvm.c b/backends/igvm.c index 7673e4a882..aae83f8a77 100644 --- a/backends/igvm.c +++ b/backends/igvm.c @@ -965,3 +965,54 @@ cleanup: return retval; } + +int qigvm_process_vp_context(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, + Error **errp) +{ + int32_t header_count; + int retval = -1; + QIgvm ctx; + + memset(&ctx, 0, sizeof(ctx)); + ctx.file = qigvm_file_init(cfg->filename, errp); + if (ctx.file < 0) { + return -1; + } + + 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) { + goto cleanup; + } + + 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); + goto cleanup; + } + + 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 (type == IGVM_VHT_VP_CONTEXT) { + if (qigvm_handler(&ctx, type, errp) < 0) { + goto cleanup; + } + } + } + retval = 0; + +cleanup: + igvm_free(ctx.file); + + return retval; +} diff --git a/backends/igvm.h b/backends/igvm.h index 269eb3a10e..a43b029d56 100644 --- a/backends/igvm.h +++ b/backends/igvm.h @@ -20,4 +20,7 @@ int qigvm_process_file(IgvmCfg *igvm, ConfidentialGuestSupport *cgs, Error **errp); +int qigvm_process_vp_context(IgvmCfg *igvm, ConfidentialGuestSupport *cgs, + Error **errp); + #endif diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h index 21fadfe5b7..0c1a7ef309 100644 --- a/include/system/igvm-cfg.h +++ b/include/system/igvm-cfg.h @@ -38,6 +38,16 @@ typedef struct IgvmCfgClass { int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, Error **errp); + /* + * If an IGVM filename has been specified then only process + * the VMSA sections in the IGVM file. + * Performs a no-op if no filename has been specified. + * + * Returns 0 for ok and -1 on error. + */ + int (*process_vp_context)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, + Error **errp); + } IgvmCfgClass; #define TYPE_IGVM_CFG "igvm-cfg" diff --git a/target/i386/sev.c b/target/i386/sev.c index ef25e64b14..d22e9870ea 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -1893,14 +1893,14 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp) * each vcpu. * * The IGVM file is normally processed after initialization. Therefore - * we need to pre-process it here to extract sev_features in order to - * provide it to KVM_SEV_INIT2. Each cgs_* function that is called by - * the IGVM processor detects this pre-process by observing the state - * as SEV_STATE_UNINIT. + * we need to pre-process it here, just looking for the vp_context to + * extract sev_features in order to provide it to KVM_SEV_INIT2. Each + * cgs_* function that is called by the IGVM processor detects this + * pre-process by observing the state as SEV_STATE_UNINIT. */ if (x86machine->igvm) { if (IGVM_CFG_GET_CLASS(x86machine->igvm) - ->process(x86machine->igvm, machine->cgs, errp) == -1) { + ->process_vp_context(x86machine->igvm, machine->cgs, errp) == -1) { return -1; } /* -- 2.43.0
Hi Roy, On Thu, Mar 06, 2025 at 11:48:29AM +0000, Roy Hopkins wrote: >On Wed, 2025-03-05 at 16:47 +0100, Stefano Garzarella wrote: [...] > >Thanks for testing this. The problem seems to be down to the fact that I >had to introduce an initial parsing of the IGVM file during initialization >to extract sev_features. I was parsing all directives in the file but it >appears this has some unwanted side effects. > >Please could you try the patch below to see if it fixes the issue? If it >does I'll incorporate it into the patch series and resubmit. Great, it worked, so feel free to add: Tested-by: Stefano Garzarella <sgarzare@redhat.com> About the patch, I have some comments below: > >From 3590460ec3945b02a679ad79735681a642596d60 Mon Sep 17 00:00:00 2001 >From: Roy Hopkins <roy.hopkins@randomman.co.uk> >Date: Thu, 6 Mar 2025 11:25:07 +0000 >Subject: [PATCH 1/1] backends/igvm: Add function to process only VP context > >When initializing kvm for SEV, the sev_features need to be >passed to the initialization function. When using IGVM files, >sev_features is provided in the VP context definintions in >the file. Currently this is handled in sev.c by processing >the entire file to extract the VP context, however this has >unwanted side-effects. Therefore this commit adds a new >function that allows only the VP context definitions to >be parsed in the IGVM file. > >Signed-off-by: Roy Hopkins <roy.hopkins@randomman.co.uk> >--- > backends/igvm-cfg.c | 1 + > backends/igvm.c | 51 +++++++++++++++++++++++++++++++++++++++ > backends/igvm.h | 3 +++ > include/system/igvm-cfg.h | 10 ++++++++ > target/i386/sev.c | 10 ++++---- > 5 files changed, 70 insertions(+), 5 deletions(-) > >diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c >index 38f17dae44..25c4469768 100644 >--- a/backends/igvm-cfg.c >+++ b/backends/igvm-cfg.c >@@ -41,6 +41,7 @@ static void igvm_cfg_class_init(ObjectClass *oc, void *data) > "Set the IGVM filename to use"); > > igvmc->process = qigvm_process_file; >+ igvmc->process_vp_context = qigvm_process_vp_context; > } > > static void igvm_cfg_init(Object *obj) >diff --git a/backends/igvm.c b/backends/igvm.c >index 7673e4a882..aae83f8a77 100644 >--- a/backends/igvm.c >+++ b/backends/igvm.c >@@ -965,3 +965,54 @@ cleanup: > > return retval; > } >+ >+int qigvm_process_vp_context(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, >+ Error **errp) >+{ This new function share a lot of code with qigvm_process_file(), can we avoid that? e.g. adding a parameter to the process callback, or just factoring out common code? >+ int32_t header_count; >+ int retval = -1; >+ QIgvm ctx; >+ >+ memset(&ctx, 0, sizeof(ctx)); >+ ctx.file = qigvm_file_init(cfg->filename, errp); >+ if (ctx.file < 0) { >+ return -1; >+ } >+ >+ 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) { >+ goto cleanup; >+ } >+ >+ 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); >+ goto cleanup; >+ } >+ >+ 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 (type == IGVM_VHT_VP_CONTEXT) { >+ if (qigvm_handler(&ctx, type, errp) < 0) { Understanding the error I had was a bit tricky, since it didn't mention IGVM at all. Can we add an error_prepend() here or ... >+ goto cleanup; >+ } >+ } >+ } >+ retval = 0; >+ >+cleanup: >+ igvm_free(ctx.file); >+ >+ return retval; >+} >diff --git a/backends/igvm.h b/backends/igvm.h >index 269eb3a10e..a43b029d56 100644 >--- a/backends/igvm.h >+++ b/backends/igvm.h >@@ -20,4 +20,7 @@ > int qigvm_process_file(IgvmCfg *igvm, ConfidentialGuestSupport *cgs, > Error **errp); > >+int qigvm_process_vp_context(IgvmCfg *igvm, ConfidentialGuestSupport *cgs, >+ Error **errp); >+ > #endif >diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h >index 21fadfe5b7..0c1a7ef309 100644 >--- a/include/system/igvm-cfg.h >+++ b/include/system/igvm-cfg.h >@@ -38,6 +38,16 @@ typedef struct IgvmCfgClass { > int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, > Error **errp); > >+ /* >+ * If an IGVM filename has been specified then only process >+ * the VMSA sections in the IGVM file. >+ * Performs a no-op if no filename has been specified. >+ * >+ * Returns 0 for ok and -1 on error. >+ */ >+ int (*process_vp_context)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, >+ Error **errp); >+ > } IgvmCfgClass; > > #define TYPE_IGVM_CFG "igvm-cfg" >diff --git a/target/i386/sev.c b/target/i386/sev.c >index ef25e64b14..d22e9870ea 100644 >--- a/target/i386/sev.c >+++ b/target/i386/sev.c >@@ -1893,14 +1893,14 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp) > * each vcpu. > * > * The IGVM file is normally processed after initialization. Therefore >- * we need to pre-process it here to extract sev_features in order to >- * provide it to KVM_SEV_INIT2. Each cgs_* function that is called by >- * the IGVM processor detects this pre-process by observing the state >- * as SEV_STATE_UNINIT. >+ * we need to pre-process it here, just looking for the vp_context to >+ * extract sev_features in order to provide it to KVM_SEV_INIT2. Each >+ * cgs_* function that is called by the IGVM processor detects this >+ * pre-process by observing the state as SEV_STATE_UNINIT. > */ > if (x86machine->igvm) { > if (IGVM_CFG_GET_CLASS(x86machine->igvm) >- ->process(x86machine->igvm, machine->cgs, errp) == -1) { >+ ->process_vp_context(x86machine->igvm, machine->cgs, errp) == -1) { ... here? BTW we can fix it later, but since you have to repost, I pointed it out. Thanks, Stefano > return -1; > } > /* >-- >2.43.0 > >
© 2016 - 2025 Red Hat, Inc.