[v4] Add AMD Secure Nested Paging (SEV-SNP) support

[PATCH v4 00/31] Add AMD Secure Nested Paging (SEV-SNP) support

Posted by Pankaj Gupta 5 months, 4 weeks ago

These patches implement SEV-SNP base support along with CPUID enforcement
support for QEMU, and are also available at:

https://github.com/pagupta/qemu/tree/snp_v4

Latest version of kvm changes are posted here [2] and also queued in kvm/next.

Patch Layout
------------
01-03: 'error_setg' independent fix, kvm/next header sync & patch from
       Xiaoyao's TDX v5 patchset.
04-29: Introduction of sev-snp-guest object and various configuration
       requirements for SNP. Support for creating a cryptographic "launch" context
       and populating various OVMF metadata pages, BIOS regions, and vCPU/VMSA
       pages with the initial encrypted/measured/validated launch data prior to
       launching the SNP guest.
30-31: Handling for KVM_HC_MAP_GPA_RANGE hypercall for userspace VMEXIT.

Testing
-------
This series has been tested against the kvm/next tree and the
AMDSEV tree [1].

[1]  https://github.com/AMDESE/linux/commits/snp-host-latest/

Below version of OVMF is used to test the changes.

  https://github.com/mdroth/edk2/commits/apic-mmio-fix1d/

A basic command-line invocation for SNP would be:

 qemu-system-x86_64 -smp 32,maxcpus=255 -cpu EPYC-Milan-v2
  -machine q35,confidential-guest-support=sev0,memory-backend=ram1
  -object memory-backend-memfd,id=ram1,size=4G,share=true,reserve=false
  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,id-auth=
  -bios OVMF_CODE-upstream-20240410-apic-mmio-fix1d-AmdSevX64.fd

With kernel-hashing and certificate data supplied:

 qemu-system-x86_64 -smp 32,maxcpus=255 -cpu EPYC-Milan-v2
  -machine q35,confidential-guest-support=sev0,memory-backend=ram1
  -object memory-backend-memfd,id=ram1,size=4G,share=true,reserve=false
  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,id-auth=,kernel-hashes=on
  -bios OVMF_CODE-upstream-20240410-apic-mmio-fix1d-AmdSevX64.fd
  -kernel /boot/vmlinuz-$ver
  -initrd /boot/initrd.img-$ver
  -append "root=UUID=d72a6d1c-06cf-4b79-af43-f1bac4f620f9 ro console=ttyS0,115200n8"

With standard X64 OVMF package with separate image for persistent NVRAM:

 qemu-system-x86_64 -smp 32,maxcpus=255 -cpu EPYC-Milan-v2
  -machine q35,confidential-guest-support=sev0,memory-backend=ram1
  -object memory-backend-memfd,id=ram1,size=4G,share=true,reserve=false
  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,id-auth=
  -bios OVMF_CODE-upstream-20240410-apic-mmio-fix1d.fd 
  -drive if=pflash,format=raw,unit=0,file=OVMF_VARS-upstream-20240410-apic-mmio-fix1d.fd,readonly=off:
 
 Any comments/feedback would be very much appreciated.

[2] https://lore.kernel.org/all/20240501085210.2213060-1-michael.roth@amd.com/
--------------

Changes since rfc3:

- added class methods (SEV & SNP) for functions changes suggested in RFC v3:
  launch_start(), launch_update_data(), launch_finish(), kvm_init(), kvm_type() (Paolo) 
- improved qom.json, query-sev QAPI text suggestions (Daniel & Markus).
- moved 'pc_system_parse_sev_metadata' to 'target/i386/sev.c' (Isaku).
- moved SNP specific methods (set guest_mem_fd, no smm check, no disable block discard)
  to sev_snp_kvm_init().
- squashed qapi changes for SecCommonProperties into 'sev-guest-common' patch (Daniel, Markus)
- made legacy bios support to SNP only.
- switch to using KVM_HC_MAP_GPA_RANGE to handle page-state change
  requests rather than directly processing GHCB page-state change buffer
- drop attestation certificate support, will revisit once the KVM_EXIT_*
  event mechanism is finalized
- sync headers with kvm/next, which now contains base KVM SNP support
- some more fixes including missing 'return', length checks,
  monitor logs improvements. (Daniel, Markus)


Changes since rfc2:

- reworked on top of guest_memfd support
- added handling for various KVM_EXIT_VMGEXIT events
- various changes/considerations for PCI passthrough support
- general bugfixes/hardening/cleanups
- qapi cmdline doc fixes/rework (Dov, Markus)
- switch to qbase64_decode, more error-checking for cmdline opts (Dov)
- unset id_block_en for 0 input (Dov)
- use error_setg in snp init (Dov)
- report more info in trace_kvm_sev_init (Dov)
- rework bounds-checking for kvm_cpuid_info, rework existing checks for 
  readability, add additional checks (Dov)
- fixups for validated_ranges handling (Dov)
- rename 'policy' field to 'snp-policy' in query-sev when sev-type is SNP

Changes since rfc1:

 - rebased onto latest master
 - drop SNP config file in favor of a new 'sev-snp-guest' object where all
   SNP-related params are passed as strings/integers via command-line
 - report specific error if BIOS reports invalid address/len for
   reserved/pre-validated regions (Connor)
 - use Range helpers for handling validated region overlaps (Dave)
 - simplify error handling in sev_snp_launch_start, and report the correct
   return code when handling LAUNCH_START failures (Dov)
 - add SEV-SNP bit to CPUID 0x8000001f when SNP enabled
 - updated query-sev to handle differences between SEV and SEV-SNP
 - updated to work against v5 of SEV-SNP host kernel / hypervisor patches

Brijesh Singh (6):
  i386/sev: Introduce 'sev-snp-guest' object
  i386/sev: Add the SNP launch start context
  i386/sev: Add handling to encrypt/finalize guest launch data
  hw/i386/sev: Add function to get SEV metadata from OVMF header
  i386/sev: Add support for populating OVMF metadata pages
  hw/i386/sev: Add support to encrypt BIOS when SEV-SNP is enabled

Dov Murik (3):
  i386/sev: Extract build_kernel_loader_hashes
  i386/sev: Reorder struct declarations
  i386/sev: Allow measured direct kernel boot on SNP

Michael Roth (12):
  i386/sev: Introduce "sev-common" type to encapsulate common SEV state
  i386/sev: Add a sev_snp_enabled() helper
  i386/cpu: Set SEV-SNP CPUID bit when SNP enabled
  i386/sev: Don't return launch measurements for SEV-SNP guests
  i386/sev: Update query-sev QAPI format to handle SEV-SNP
  i386/sev: Set CPU state to protected once SNP guest payload is
    finalized
  i386/sev: Add support for SNP CPUID validation
  hw/i386/sev: Use guest_memfd for legacy ROMs
  hw/i386: Add support for loading BIOS using guest_memfd
  hw/i386/sev: Allow use of pflash in conjunction with -bios
  i386/kvm: Add KVM_EXIT_HYPERCALL handling for KVM_HC_MAP_GPA_RANGE
  i386/sev: Enable KVM_HC_MAP_GPA_RANGE hcall for SNP guests

Pankaj Gupta (9):
  i386/sev: Replace error_report with error_setg
  linux-headers: Update to current kvm/next
  i386/sev: Move sev_launch_update to separate class method
  i386/sev: Move sev_launch_finish to separate class method
  i386/sev: Add sev_kvm_init() override for SEV class
  i386/sev: Add snp_kvm_init() override for SNP class
  i386/sev: Add a class method to determine KVM VM type for SNP guests
  i386/sev: Invoke launch_updata_data() for SEV class
  i386/sev: Invoke launch_updata_data() for SNP class

Xiaoyao Li (1):
  memory: Introduce memory_region_init_ram_guest_memfd()

 docs/system/i386/amd-memory-encryption.rst |   70 +-
 hw/i386/pc.c                               |   14 +-
 hw/i386/pc_sysfw.c                         |   76 +-
 hw/i386/x86-common.c                       |   24 +-
 include/exec/memory.h                      |    6 +
 include/hw/i386/pc.h                       |   28 +
 include/hw/i386/x86.h                      |    2 +-
 linux-headers/asm-loongarch/bitsperlong.h  |   23 +
 linux-headers/asm-loongarch/kvm.h          |    4 +
 linux-headers/asm-loongarch/mman.h         |    9 +
 linux-headers/asm-riscv/kvm.h              |    1 +
 linux-headers/asm-riscv/mman.h             |   36 +-
 linux-headers/asm-s390/mman.h              |   36 +-
 linux-headers/asm-x86/kvm.h                |   52 +-
 linux-headers/linux/vhost.h                |   15 +-
 qapi/misc-target.json                      |   72 +-
 qapi/qom.json                              |   97 +-
 system/memory.c                            |   24 +
 target/i386/cpu.c                          |    1 +
 target/i386/kvm/kvm.c                      |   55 +
 target/i386/kvm/kvm_i386.h                 |    1 +
 target/i386/kvm/trace-events               |    1 +
 target/i386/sev-sysemu-stub.c              |    2 +-
 target/i386/sev.c                          | 1588 +++++++++++++++-----
 target/i386/sev.h                          |   13 +-
 target/i386/trace-events                   |    3 +
 26 files changed, 1833 insertions(+), 420 deletions(-)

-- 
2.34.1

Re: [PATCH v4 00/31] Add AMD Secure Nested Paging (SEV-SNP) support

Posted by Paolo Bonzini 5 months, 3 weeks ago

On Thu, May 30, 2024 at 1:16 PM Pankaj Gupta <pankaj.gupta@amd.com> wrote:
>
> These patches implement SEV-SNP base support along with CPUID enforcement
> support for QEMU, and are also available at:
>
> https://github.com/pagupta/qemu/tree/snp_v4
>
> Latest version of kvm changes are posted here [2] and also queued in kvm/next.
>
> Patch Layout
> ------------
> 01-03: 'error_setg' independent fix, kvm/next header sync & patch from
>        Xiaoyao's TDX v5 patchset.
> 04-29: Introduction of sev-snp-guest object and various configuration
>        requirements for SNP. Support for creating a cryptographic "launch" context
>        and populating various OVMF metadata pages, BIOS regions, and vCPU/VMSA
>        pages with the initial encrypted/measured/validated launch data prior to
>        launching the SNP guest.
> 30-31: Handling for KVM_HC_MAP_GPA_RANGE hypercall for userspace VMEXIT.

These patches are more or less okay, with only a few nits, and I can
queue them already:

i386/sev: Replace error_report with error_setg
linux-headers: Update to current kvm/next
i386/sev: Introduce "sev-common" type to encapsulate common SEV state
i386/sev: Move sev_launch_update to separate class method
i386/sev: Move sev_launch_finish to separate class method
i386/sev: Introduce 'sev-snp-guest' object
i386/sev: Add a sev_snp_enabled() helper
i386/sev: Add sev_kvm_init() override for SEV class
i386/sev: Add snp_kvm_init() override for SNP class
i386/cpu: Set SEV-SNP CPUID bit when SNP enabled
i386/sev: Don't return launch measurements for SEV-SNP guests
i386/sev: Add a class method to determine KVM VM type for SNP guests
i386/sev: Update query-sev QAPI format to handle SEV-SNP
i386/sev: Add the SNP launch start context
i386/sev: Add handling to encrypt/finalize guest launch data
i386/sev: Set CPU state to protected once SNP guest payload is finalized
hw/i386/sev: Add function to get SEV metadata from OVMF header
i386/sev: Add support for populating OVMF metadata pages
i386/sev: Add support for SNP CPUID validation
i386/sev: Invoke launch_updata_data() for SEV class
i386/sev: Invoke launch_updata_data() for SNP class
i386/kvm: Add KVM_EXIT_HYPERCALL handling for KVM_HC_MAP_GPA_RANGE
i386/sev: Enable KVM_HC_MAP_GPA_RANGE hcall for SNP guests
i386/sev: Extract build_kernel_loader_hashes
i386/sev: Reorder struct declarations
i386/sev: Allow measured direct kernel boot on SNP
hw/i386/sev: Add support to encrypt BIOS when SEV-SNP is enabled
memory: Introduce memory_region_init_ram_guest_memfd()

These patches need a small prerequisite that I'll post soon:

hw/i386/sev: Use guest_memfd for legacy ROMs
hw/i386: Add support for loading BIOS using guest_memfd

This one definitely requires more work:

hw/i386/sev: Allow use of pflash in conjunction with -bios


Paolo

Re: [PATCH v4 00/31] Add AMD Secure Nested Paging (SEV-SNP) support

Posted by Paolo Bonzini 5 months, 3 weeks ago

On Fri, May 31, 2024 at 1:20 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On Thu, May 30, 2024 at 1:16 PM Pankaj Gupta <pankaj.gupta@amd.com> wrote:
> >
> > These patches implement SEV-SNP base support along with CPUID enforcement
> > support for QEMU, and are also available at:
> >
> > https://github.com/pagupta/qemu/tree/snp_v4
> >
> > Latest version of kvm changes are posted here [2] and also queued in kvm/next.
> >
> > Patch Layout
> > ------------
> > 01-03: 'error_setg' independent fix, kvm/next header sync & patch from
> >        Xiaoyao's TDX v5 patchset.
> > 04-29: Introduction of sev-snp-guest object and various configuration
> >        requirements for SNP. Support for creating a cryptographic "launch" context
> >        and populating various OVMF metadata pages, BIOS regions, and vCPU/VMSA
> >        pages with the initial encrypted/measured/validated launch data prior to
> >        launching the SNP guest.
> > 30-31: Handling for KVM_HC_MAP_GPA_RANGE hypercall for userspace VMEXIT.
>
> These patches are more or less okay, with only a few nits, and I can
> queue them already:

Hey,

please check if branch qemu-coco-queue of
https://gitlab.com/bonzini/qemu works for you!

I tested it successfully on CentOS 9 Stream with kernel from kvm/next
and firmware from edk2-ovmf-20240524-1.fc41.noarch.

Paolo

> i386/sev: Replace error_report with error_setg
> linux-headers: Update to current kvm/next
> i386/sev: Introduce "sev-common" type to encapsulate common SEV state
> i386/sev: Move sev_launch_update to separate class method
> i386/sev: Move sev_launch_finish to separate class method
> i386/sev: Introduce 'sev-snp-guest' object
> i386/sev: Add a sev_snp_enabled() helper
> i386/sev: Add sev_kvm_init() override for SEV class
> i386/sev: Add snp_kvm_init() override for SNP class
> i386/cpu: Set SEV-SNP CPUID bit when SNP enabled
> i386/sev: Don't return launch measurements for SEV-SNP guests
> i386/sev: Add a class method to determine KVM VM type for SNP guests
> i386/sev: Update query-sev QAPI format to handle SEV-SNP
> i386/sev: Add the SNP launch start context
> i386/sev: Add handling to encrypt/finalize guest launch data
> i386/sev: Set CPU state to protected once SNP guest payload is finalized
> hw/i386/sev: Add function to get SEV metadata from OVMF header
> i386/sev: Add support for populating OVMF metadata pages
> i386/sev: Add support for SNP CPUID validation
> i386/sev: Invoke launch_updata_data() for SEV class
> i386/sev: Invoke launch_updata_data() for SNP class
> i386/kvm: Add KVM_EXIT_HYPERCALL handling for KVM_HC_MAP_GPA_RANGE
> i386/sev: Enable KVM_HC_MAP_GPA_RANGE hcall for SNP guests
> i386/sev: Extract build_kernel_loader_hashes
> i386/sev: Reorder struct declarations
> i386/sev: Allow measured direct kernel boot on SNP
> hw/i386/sev: Add support to encrypt BIOS when SEV-SNP is enabled
> memory: Introduce memory_region_init_ram_guest_memfd()
>
> These patches need a small prerequisite that I'll post soon:
>
> hw/i386/sev: Use guest_memfd for legacy ROMs
> hw/i386: Add support for loading BIOS using guest_memfd
>
> This one definitely requires more work:
>
> hw/i386/sev: Allow use of pflash in conjunction with -bios
>
>
> Paolo

Re: [PATCH v4 00/31] Add AMD Secure Nested Paging (SEV-SNP) support

Posted by Gupta, Pankaj 5 months, 3 weeks ago

>>> These patches implement SEV-SNP base support along with CPUID enforcement
>>> support for QEMU, and are also available at:
>>>
>>> https://github.com/pagupta/qemu/tree/snp_v4
>>>
>>> Latest version of kvm changes are posted here [2] and also queued in kvm/next.
>>>
>>> Patch Layout
>>> ------------
>>> 01-03: 'error_setg' independent fix, kvm/next header sync & patch from
>>>         Xiaoyao's TDX v5 patchset.
>>> 04-29: Introduction of sev-snp-guest object and various configuration
>>>         requirements for SNP. Support for creating a cryptographic "launch" context
>>>         and populating various OVMF metadata pages, BIOS regions, and vCPU/VMSA
>>>         pages with the initial encrypted/measured/validated launch data prior to
>>>         launching the SNP guest.
>>> 30-31: Handling for KVM_HC_MAP_GPA_RANGE hypercall for userspace VMEXIT.
>>
>> These patches are more or less okay, with only a few nits, and I can
>> queue them already:
> 
> Hey,
> 
> please check if branch qemu-coco-queue of
> https://gitlab.com/bonzini/qemu works for you!

Getting compilation error here: Hope I am looking at correct branch.

softmmu.fa.p/target_i386_kvm_kvm.c.o.d -o 
libqemu-x86_64-softmmu.fa.p/target_i386_kvm_kvm.c.o -c 
../target/i386/kvm/kvm.c
../target/i386/kvm/kvm.c:171:6: error: ‘KVM_X86_SEV_SNP_VM’ undeclared 
here (not in a function); did you mean ‘KVM_X86_SEV_ES_VM’?
   171 |     [KVM_X86_SEV_SNP_VM] = "SEV-SNP",
       |      ^~~~~~~~~~~~~~~~~~
       |      KVM_X86_SEV_ES_VM

Thanks,
Pankaj

> 
> I tested it successfully on CentOS 9 Stream with kernel from kvm/next
> and firmware from edk2-ovmf-20240524-1.fc41.noarch.
> 
> Paolo
> 
>> i386/sev: Replace error_report with error_setg
>> linux-headers: Update to current kvm/next
>> i386/sev: Introduce "sev-common" type to encapsulate common SEV state
>> i386/sev: Move sev_launch_update to separate class method
>> i386/sev: Move sev_launch_finish to separate class method
>> i386/sev: Introduce 'sev-snp-guest' object
>> i386/sev: Add a sev_snp_enabled() helper
>> i386/sev: Add sev_kvm_init() override for SEV class
>> i386/sev: Add snp_kvm_init() override for SNP class
>> i386/cpu: Set SEV-SNP CPUID bit when SNP enabled
>> i386/sev: Don't return launch measurements for SEV-SNP guests
>> i386/sev: Add a class method to determine KVM VM type for SNP guests
>> i386/sev: Update query-sev QAPI format to handle SEV-SNP
>> i386/sev: Add the SNP launch start context
>> i386/sev: Add handling to encrypt/finalize guest launch data
>> i386/sev: Set CPU state to protected once SNP guest payload is finalized
>> hw/i386/sev: Add function to get SEV metadata from OVMF header
>> i386/sev: Add support for populating OVMF metadata pages
>> i386/sev: Add support for SNP CPUID validation
>> i386/sev: Invoke launch_updata_data() for SEV class
>> i386/sev: Invoke launch_updata_data() for SNP class
>> i386/kvm: Add KVM_EXIT_HYPERCALL handling for KVM_HC_MAP_GPA_RANGE
>> i386/sev: Enable KVM_HC_MAP_GPA_RANGE hcall for SNP guests
>> i386/sev: Extract build_kernel_loader_hashes
>> i386/sev: Reorder struct declarations
>> i386/sev: Allow measured direct kernel boot on SNP
>> hw/i386/sev: Add support to encrypt BIOS when SEV-SNP is enabled
>> memory: Introduce memory_region_init_ram_guest_memfd()
>>
>> These patches need a small prerequisite that I'll post soon:
>>
>> hw/i386/sev: Use guest_memfd for legacy ROMs
>> hw/i386: Add support for loading BIOS using guest_memfd
>>
>> This one definitely requires more work:
>>
>> hw/i386/sev: Allow use of pflash in conjunction with -bios
>>
>>
>> Paolo
>

Re: [PATCH v4 00/31] Add AMD Secure Nested Paging (SEV-SNP) support

Posted by Paolo Bonzini 5 months, 3 weeks ago

On Fri, May 31, 2024 at 7:41 PM Gupta, Pankaj <pankaj.gupta@amd.com> wrote:
> > please check if branch qemu-coco-queue of
> > https://gitlab.com/bonzini/qemu works for you!
>
> Getting compilation error here: Hope I am looking at correct branch.

Oops, sorry:

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 96dc41d355c..ede3ef1225f 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -168,7 +168,7 @@ static const char *vm_type_name[] = {
     [KVM_X86_DEFAULT_VM] = "default",
     [KVM_X86_SEV_VM] = "SEV",
     [KVM_X86_SEV_ES_VM] = "SEV-ES",
-    [KVM_X86_SEV_SNP_VM] = "SEV-SNP",
+    [KVM_X86_SNP_VM] = "SEV-SNP",
 };

 bool kvm_is_vm_type_supported(int type)

Tested the above builds, and pushed!

Paolo

> softmmu.fa.p/target_i386_kvm_kvm.c.o.d -o
> libqemu-x86_64-softmmu.fa.p/target_i386_kvm_kvm.c.o -c
> ../target/i386/kvm/kvm.c
> ../target/i386/kvm/kvm.c:171:6: error: ‘KVM_X86_SEV_SNP_VM’ undeclared
> here (not in a function); did you mean ‘KVM_X86_SEV_ES_VM’?
>    171 |     [KVM_X86_SEV_SNP_VM] = "SEV-SNP",
>        |      ^~~~~~~~~~~~~~~~~~
>        |      KVM_X86_SEV_ES_VM
>
> Thanks,
> Pankaj
>
> >
> > I tested it successfully on CentOS 9 Stream with kernel from kvm/next
> > and firmware from edk2-ovmf-20240524-1.fc41.noarch.
> >
> > Paolo
> >
> >> i386/sev: Replace error_report with error_setg
> >> linux-headers: Update to current kvm/next
> >> i386/sev: Introduce "sev-common" type to encapsulate common SEV state
> >> i386/sev: Move sev_launch_update to separate class method
> >> i386/sev: Move sev_launch_finish to separate class method
> >> i386/sev: Introduce 'sev-snp-guest' object
> >> i386/sev: Add a sev_snp_enabled() helper
> >> i386/sev: Add sev_kvm_init() override for SEV class
> >> i386/sev: Add snp_kvm_init() override for SNP class
> >> i386/cpu: Set SEV-SNP CPUID bit when SNP enabled
> >> i386/sev: Don't return launch measurements for SEV-SNP guests
> >> i386/sev: Add a class method to determine KVM VM type for SNP guests
> >> i386/sev: Update query-sev QAPI format to handle SEV-SNP
> >> i386/sev: Add the SNP launch start context
> >> i386/sev: Add handling to encrypt/finalize guest launch data
> >> i386/sev: Set CPU state to protected once SNP guest payload is finalized
> >> hw/i386/sev: Add function to get SEV metadata from OVMF header
> >> i386/sev: Add support for populating OVMF metadata pages
> >> i386/sev: Add support for SNP CPUID validation
> >> i386/sev: Invoke launch_updata_data() for SEV class
> >> i386/sev: Invoke launch_updata_data() for SNP class
> >> i386/kvm: Add KVM_EXIT_HYPERCALL handling for KVM_HC_MAP_GPA_RANGE
> >> i386/sev: Enable KVM_HC_MAP_GPA_RANGE hcall for SNP guests
> >> i386/sev: Extract build_kernel_loader_hashes
> >> i386/sev: Reorder struct declarations
> >> i386/sev: Allow measured direct kernel boot on SNP
> >> hw/i386/sev: Add support to encrypt BIOS when SEV-SNP is enabled
> >> memory: Introduce memory_region_init_ram_guest_memfd()
> >>
> >> These patches need a small prerequisite that I'll post soon:
> >>
> >> hw/i386/sev: Use guest_memfd for legacy ROMs
> >> hw/i386: Add support for loading BIOS using guest_memfd
> >>
> >> This one definitely requires more work:
> >>
> >> hw/i386/sev: Allow use of pflash in conjunction with -bios
> >>
> >>
> >> Paolo
> >
>

Re: [PATCH v4 00/31] Add AMD Secure Nested Paging (SEV-SNP) support

Posted by Gupta, Pankaj 5 months, 3 weeks ago

Hi Paolo,

>>> please check if branch qemu-coco-queue of
>>> https://gitlab.com/bonzini/qemu works for you!
>>
>> Getting compilation error here: Hope I am looking at correct branch.
> 
> Oops, sorry:
> 
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 96dc41d355c..ede3ef1225f 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -168,7 +168,7 @@ static const char *vm_type_name[] = {
>       [KVM_X86_DEFAULT_VM] = "default",
>       [KVM_X86_SEV_VM] = "SEV",
>       [KVM_X86_SEV_ES_VM] = "SEV-ES",
> -    [KVM_X86_SEV_SNP_VM] = "SEV-SNP",
> +    [KVM_X86_SNP_VM] = "SEV-SNP",
>   };
> 
>   bool kvm_is_vm_type_supported(int type)
> 
> Tested the above builds, and pushed!

Thank you for your work! I tested (quick tests) the updated branch and 
OVMF [1], it works well for single bios option[2] & direct kernel boot 
[3]. For some reason separate 'pflash' & 'bios' option, facing issue 
(maybe some other bug in my code, will try to figure it out and get back 
on this). Also, will check your comment on mailing list on patch [4],
maybe they are related.

For now I think we are good with the 'qemu-coco-queue' & single bios 
binary configuration using 'AmdSevX64'.

[1]  https://github.com/mdroth/edk2/commits/apic-mmio-fix1d/

[2]  -bios 
/home/amd/AMDSEV/OVMF_CODE-upstream-20240228-apicfix-1c-AmdSevX64.fd

[3] Direct kernel loading with '-bios 
/home/amd/AMDSEV/ovmf/OVMF_CODE-apic-mmio-fix1d-AmdSevX64.fd'

[4] "hw/i386/sev: Allow use of pflash in conjunction with -bios"

Thanks,
Pankaj
> 
> Paolo
> 
>> softmmu.fa.p/target_i386_kvm_kvm.c.o.d -o
>> libqemu-x86_64-softmmu.fa.p/target_i386_kvm_kvm.c.o -c
>> ../target/i386/kvm/kvm.c
>> ../target/i386/kvm/kvm.c:171:6: error: ‘KVM_X86_SEV_SNP_VM’ undeclared
>> here (not in a function); did you mean ‘KVM_X86_SEV_ES_VM’?
>>     171 |     [KVM_X86_SEV_SNP_VM] = "SEV-SNP",
>>         |      ^~~~~~~~~~~~~~~~~~
>>         |      KVM_X86_SEV_ES_VM
>>
>> Thanks,
>> Pankaj
>>
>>>
>>> I tested it successfully on CentOS 9 Stream with kernel from kvm/next
>>> and firmware from edk2-ovmf-20240524-1.fc41.noarch.
>>>
>>> Paolo
>>>
>>>> i386/sev: Replace error_report with error_setg
>>>> linux-headers: Update to current kvm/next
>>>> i386/sev: Introduce "sev-common" type to encapsulate common SEV state
>>>> i386/sev: Move sev_launch_update to separate class method
>>>> i386/sev: Move sev_launch_finish to separate class method
>>>> i386/sev: Introduce 'sev-snp-guest' object
>>>> i386/sev: Add a sev_snp_enabled() helper
>>>> i386/sev: Add sev_kvm_init() override for SEV class
>>>> i386/sev: Add snp_kvm_init() override for SNP class
>>>> i386/cpu: Set SEV-SNP CPUID bit when SNP enabled
>>>> i386/sev: Don't return launch measurements for SEV-SNP guests
>>>> i386/sev: Add a class method to determine KVM VM type for SNP guests
>>>> i386/sev: Update query-sev QAPI format to handle SEV-SNP
>>>> i386/sev: Add the SNP launch start context
>>>> i386/sev: Add handling to encrypt/finalize guest launch data
>>>> i386/sev: Set CPU state to protected once SNP guest payload is finalized
>>>> hw/i386/sev: Add function to get SEV metadata from OVMF header
>>>> i386/sev: Add support for populating OVMF metadata pages
>>>> i386/sev: Add support for SNP CPUID validation
>>>> i386/sev: Invoke launch_updata_data() for SEV class
>>>> i386/sev: Invoke launch_updata_data() for SNP class
>>>> i386/kvm: Add KVM_EXIT_HYPERCALL handling for KVM_HC_MAP_GPA_RANGE
>>>> i386/sev: Enable KVM_HC_MAP_GPA_RANGE hcall for SNP guests
>>>> i386/sev: Extract build_kernel_loader_hashes
>>>> i386/sev: Reorder struct declarations
>>>> i386/sev: Allow measured direct kernel boot on SNP
>>>> hw/i386/sev: Add support to encrypt BIOS when SEV-SNP is enabled
>>>> memory: Introduce memory_region_init_ram_guest_memfd()
>>>>
>>>> These patches need a small prerequisite that I'll post soon:
>>>>
>>>> hw/i386/sev: Use guest_memfd for legacy ROMs
>>>> hw/i386: Add support for loading BIOS using guest_memfd
>>>>
>>>> This one definitely requires more work:
>>>>
>>>> hw/i386/sev: Allow use of pflash in conjunction with -bios
>>>>
>>>>
>>>> Paolo
>>>
>>
>

Re: [PATCH v4 00/31] Add AMD Secure Nested Paging (SEV-SNP) support

Posted by Michael Roth 5 months, 3 weeks ago

On Sat, Jun 01, 2024 at 06:57:21AM +0200, Gupta, Pankaj wrote:
> Hi Paolo,
> 
> > > > please check if branch qemu-coco-queue of
> > > > https://gitlab.com/bonzini/qemu works for you!
> > > 
> > > Getting compilation error here: Hope I am looking at correct branch.
> > 
> > Oops, sorry:
> > 
> > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> > index 96dc41d355c..ede3ef1225f 100644
> > --- a/target/i386/kvm/kvm.c
> > +++ b/target/i386/kvm/kvm.c
> > @@ -168,7 +168,7 @@ static const char *vm_type_name[] = {
> >       [KVM_X86_DEFAULT_VM] = "default",
> >       [KVM_X86_SEV_VM] = "SEV",
> >       [KVM_X86_SEV_ES_VM] = "SEV-ES",
> > -    [KVM_X86_SEV_SNP_VM] = "SEV-SNP",
> > +    [KVM_X86_SNP_VM] = "SEV-SNP",
> >   };
> > 
> >   bool kvm_is_vm_type_supported(int type)
> > 
> > Tested the above builds, and pushed!
> 
> Thank you for your work! I tested (quick tests) the updated branch and OVMF
> [1], it works well for single bios option[2] & direct kernel boot [3]. For
> some reason separate 'pflash' & 'bios' option, facing issue (maybe some
> other bug in my code, will try to figure it out and get back on this). Also,

Paolo mentioned he dropped the this hunk from:

  hw/i386: Add support for loading BIOS using guest_memfd

  diff --git a/hw/i386/x86.c b/hw/i386/x86.c
  index de606369b0..d076b30ccb 100644
  --- a/hw/i386/x86.c
  +++ b/hw/i386/x86.c
  @@ -1147,10 +1147,18 @@ void x86_bios_rom_init(MachineState *ms, const char *default_firmware,
       }
       if (bios_size <= 0 ||
           (bios_size % 65536) != 0) {
  -        goto bios_error;
  +        if (!machine_require_guest_memfd(ms)) {
  +            g_warning("%s: Unaligned BIOS size %d", __func__, bios_size);
  +            goto bios_error;
  +        }

without that, OVMF with split CODE/VARS won't work because the CODE
portion is not 64KB aligned.

If I add that back the split builds work for qemu-coco-queue as well.

We need to understand why the 64KB alignment exists in the first place, why
it's not necessary for SNP, and then resubmit the above change with proper
explanation.

> will check your comment on mailing list on patch [4],
> maybe they are related.

However, if based on Daniel's comments we decide not to support split
CODE/VARS for SNP, then the above change won't be needed. But if we do,
then it goes make sense that the above change is grouped with (or
submitted as a fix-up for):

  hw/i386/sev: Allow use of pflash in conjunction with -bios

and untangled from "hw/i386: Add support for loading BIOS using
guest_memfd", since that's the patch where we actually start dealing
with non-64K-aligned OVMF CODE images.

-Mike

> 
> For now I think we are good with the 'qemu-coco-queue' & single bios binary
> configuration using 'AmdSevX64'.
> 
> [1]  https://github.com/mdroth/edk2/commits/apic-mmio-fix1d/
> 
> [2]  -bios
> /home/amd/AMDSEV/OVMF_CODE-upstream-20240228-apicfix-1c-AmdSevX64.fd
> 
> [3] Direct kernel loading with '-bios
> /home/amd/AMDSEV/ovmf/OVMF_CODE-apic-mmio-fix1d-AmdSevX64.fd'
> 
> [4] "hw/i386/sev: Allow use of pflash in conjunction with -bios"
> 
> Thanks,
> Pankaj
> > 
> > Paolo
> > 
> > > softmmu.fa.p/target_i386_kvm_kvm.c.o.d -o
> > > libqemu-x86_64-softmmu.fa.p/target_i386_kvm_kvm.c.o -c
> > > ../target/i386/kvm/kvm.c
> > > ../target/i386/kvm/kvm.c:171:6: error: ‘KVM_X86_SEV_SNP_VM’ undeclared
> > > here (not in a function); did you mean ‘KVM_X86_SEV_ES_VM’?
> > >     171 |     [KVM_X86_SEV_SNP_VM] = "SEV-SNP",
> > >         |      ^~~~~~~~~~~~~~~~~~
> > >         |      KVM_X86_SEV_ES_VM
> > > 
> > > Thanks,
> > > Pankaj
> > > 
> > > > 
> > > > I tested it successfully on CentOS 9 Stream with kernel from kvm/next
> > > > and firmware from edk2-ovmf-20240524-1.fc41.noarch.
> > > > 
> > > > Paolo
> > > > 
> > > > > i386/sev: Replace error_report with error_setg
> > > > > linux-headers: Update to current kvm/next
> > > > > i386/sev: Introduce "sev-common" type to encapsulate common SEV state
> > > > > i386/sev: Move sev_launch_update to separate class method
> > > > > i386/sev: Move sev_launch_finish to separate class method
> > > > > i386/sev: Introduce 'sev-snp-guest' object
> > > > > i386/sev: Add a sev_snp_enabled() helper
> > > > > i386/sev: Add sev_kvm_init() override for SEV class
> > > > > i386/sev: Add snp_kvm_init() override for SNP class
> > > > > i386/cpu: Set SEV-SNP CPUID bit when SNP enabled
> > > > > i386/sev: Don't return launch measurements for SEV-SNP guests
> > > > > i386/sev: Add a class method to determine KVM VM type for SNP guests
> > > > > i386/sev: Update query-sev QAPI format to handle SEV-SNP
> > > > > i386/sev: Add the SNP launch start context
> > > > > i386/sev: Add handling to encrypt/finalize guest launch data
> > > > > i386/sev: Set CPU state to protected once SNP guest payload is finalized
> > > > > hw/i386/sev: Add function to get SEV metadata from OVMF header
> > > > > i386/sev: Add support for populating OVMF metadata pages
> > > > > i386/sev: Add support for SNP CPUID validation
> > > > > i386/sev: Invoke launch_updata_data() for SEV class
> > > > > i386/sev: Invoke launch_updata_data() for SNP class
> > > > > i386/kvm: Add KVM_EXIT_HYPERCALL handling for KVM_HC_MAP_GPA_RANGE
> > > > > i386/sev: Enable KVM_HC_MAP_GPA_RANGE hcall for SNP guests
> > > > > i386/sev: Extract build_kernel_loader_hashes
> > > > > i386/sev: Reorder struct declarations
> > > > > i386/sev: Allow measured direct kernel boot on SNP
> > > > > hw/i386/sev: Add support to encrypt BIOS when SEV-SNP is enabled
> > > > > memory: Introduce memory_region_init_ram_guest_memfd()
> > > > > 
> > > > > These patches need a small prerequisite that I'll post soon:
> > > > > 
> > > > > hw/i386/sev: Use guest_memfd for legacy ROMs
> > > > > hw/i386: Add support for loading BIOS using guest_memfd
> > > > > 
> > > > > This one definitely requires more work:
> > > > > 
> > > > > hw/i386/sev: Allow use of pflash in conjunction with -bios
> > > > > 
> > > > > 
> > > > > Paolo
> > > > 
> > > 
> > 
>

Re: [PATCH v4 00/31] Add AMD Secure Nested Paging (SEV-SNP) support

Posted by Paolo Bonzini 5 months, 3 weeks ago

On Mon, Jun 3, 2024 at 4:16 PM Michael Roth <michael.roth@amd.com> wrote:
> Paolo mentioned he dropped the this hunk from:
>
>   hw/i386: Add support for loading BIOS using guest_memfd
>
>   diff --git a/hw/i386/x86.c b/hw/i386/x86.c
>   index de606369b0..d076b30ccb 100644
>   --- a/hw/i386/x86.c
>   +++ b/hw/i386/x86.c
>   @@ -1147,10 +1147,18 @@ void x86_bios_rom_init(MachineState *ms, const char *default_firmware,
>        }
>        if (bios_size <= 0 ||
>            (bios_size % 65536) != 0) {
>   -        goto bios_error;
>   +        if (!machine_require_guest_memfd(ms)) {
>   +            g_warning("%s: Unaligned BIOS size %d", __func__, bios_size);
>   +            goto bios_error;
>   +        }
>
> without that, OVMF with split CODE/VARS won't work because the CODE
> portion is not 64KB aligned.
>
> If I add that back the split builds work for qemu-coco-queue as well.
>
> We need to understand why the 64KB alignment exists in the first place, why
> it's not necessary for SNP, and then resubmit the above change with proper
> explanation.

I think it was only needed to make sure that people weren't using
"unpadded" BIOS (not OVMF) binaries. I think we can delete it
altogether, and it can be submitted separately from this series.

> However, if based on Daniel's comments we decide not to support split
> CODE/VARS for SNP, then the above change won't be needed. But if we do,
> then it goes make sense that the above change is grouped with (or
> submitted as a fix-up for):

Yes, I think we want to go for a variable store that is not "right
below BIOS". The advantage of something that isn't pflash-based is
that it can be used by any code-only firmware binary.

Paolo