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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux