On 1/10/2025 6:40 PM, Sean Christopherson wrote: > On Fri, Jan 10, 2025, Ashish Kalra wrote: >> It looks like i have hit a serious blocker issue with this approach of moving >> SEV/SNP initialization to KVM module load time. >> >> While testing with kvm_amd and PSP driver built-in, it looks like kvm_amd >> driver is being loaded/initialized before PSP driver is loaded, and that >> causes sev_platform_init() call from sev_hardware_setup(kvm_amd) to fail: >> >> [ 10.717898] kvm_amd: TSC scaling supported >> [ 10.722470] kvm_amd: Nested Virtualization enabled >> [ 10.727816] kvm_amd: Nested Paging enabled >> [ 10.732388] kvm_amd: LBR virtualization supported >> [ 10.737639] kvm_amd: SEV enabled (ASIDs 100 - 509) >> [ 10.742985] kvm_amd: SEV-ES enabled (ASIDs 1 - 99) >> [ 10.748333] kvm_amd: SEV-SNP enabled (ASIDs 1 - 99) >> [ 10.753768] PSP driver not init <<<---- sev_platform_init() returns failure as PSP driver is still not initialized >> [ 10.757563] kvm_amd: Virtual VMLOAD VMSAVE supported >> [ 10.763124] kvm_amd: Virtual GIF supported >> ... >> ... >> [ 12.514857] ccp 0000:23:00.1: enabling device (0000 -> 0002) >> [ 12.521691] ccp 0000:23:00.1: no command queues available >> [ 12.527991] ccp 0000:23:00.1: sev enabled >> [ 12.532592] ccp 0000:23:00.1: psp enabled >> [ 12.537382] ccp 0000:a2:00.1: enabling device (0000 -> 0002) >> [ 12.544389] ccp 0000:a2:00.1: no command queues available >> [ 12.550627] ccp 0000:a2:00.1: psp enabled >> >> depmod -> modules.builtin show kernel/arch/x86/kvm/kvm_amd.ko higher on the list and before kernel/drivers/crypto/ccp/ccp.ko >> >> modules.builtin: >> kernel/arch/x86/kvm/kvm.ko >> kernel/arch/x86/kvm/kvm-amd.ko >> ... >> ... >> kernel/drivers/crypto/ccp/ccp.ko >> >> I believe that the modules which are compiled first get called first and it >> looks like that the only way to change the order for builtin modules is by >> changing which makefiles get compiled first ? >> >> Is there a way to change the load order of built-in modules and/or change >> dependency of built-in modules ? > > The least awful option I know of would be to have the PSP use a higher priority > initcall type so that it runs before the standard initcalls. When compiled as > a module, all initcall types are #defined to module_init. > > E.g. this should work, /cross fingers > > diff --git a/drivers/crypto/ccp/sp-dev.c b/drivers/crypto/ccp/sp-dev.c > index 7eb3e4668286..02c49fbf6198 100644 > --- a/drivers/crypto/ccp/sp-dev.c > +++ b/drivers/crypto/ccp/sp-dev.c > @@ -295,5 +295,6 @@ static void __exit sp_mod_exit(void) > #endif > } > > -module_init(sp_mod_init); > +/* The PSP needs to be initialized before dependent modules, e.g. before KVM. */ > +subsys_initcall(sp_mod_init); > module_exit(sp_mod_exit); Thanks for the suggestion, but there are actually two major issues here: With the above change, PSP driver initialization fails as following: ... [ 7.274005] pci 0000:20:08.1: bridge window [mem 0xf6200000-0xf64fffff]: not claimed; can't enable device [ 7.277945] pci 0000:20:08.1: Error enabling bridge (-22), continuing [ 7.281947] ccp 0000:23:00.1: BAR 2 [mem 0xf6300000-0xf63fffff]: not claimed; can't enable device [ 7.285945] ccp 0000:23:00.1: pcim_enable_device failed (-22) [ 7.289943] ccp 0000:23:00.1: initialization failed [ 7.293944] ccp 0000:23:00.1: probe with driver ccp failed with error -22 [ 7.301981] pci 0000:a0:08.1: bridge window [mem 0xb6200000-0xb63fffff]: not claimed; can't enable device [ 7.313956] pci 0000:a0:08.1: Error enabling bridge (-22), continuing [ 7.321947] ccp 0000:a2:00.1: BAR 2 [mem 0xb6200000-0xb62fffff]: not claimed; can't enable device [ 7.329945] ccp 0000:a2:00.1: pcim_enable_device failed (-22) [ 7.337943] ccp 0000:a2:00.1: initialization failed [ 7.341946] ccp 0000:a2:00.1: probe with driver ccp failed with error -22 ... It looks as PCI bus resource allocation is still not done, hence PSP driver cannot be enabled as early as subsys_initcall, it can be initialized probably via device_initcall(), but then that will be too late as kvm_amd would have been initialized before that. Additionally, it looks like that there is an issue with SNP host support being enabled with kvm_amd module being built-in: SNP host support is enabled in snp_rmptable_init() in arch/x86/virt/svm/sev.c, which is invoked as a device_initcall(). Here device_initcall() is used as snp_rmptable_init() expects AMD IOMMU SNP support to be enabled prior to it and the AMD IOMMU driver is initialized after PCI bus enumeration. Now, if kvm_amd module is built-in, it gets initialized before SNP host support is enabled in snp_rmptable_init() : [ 10.131811] kvm_amd: TSC scaling supported [ 10.136384] kvm_amd: Nested Virtualization enabled [ 10.141734] kvm_amd: Nested Paging enabled [ 10.146304] kvm_amd: LBR virtualization supported [ 10.151557] kvm_amd: SEV enabled (ASIDs 100 - 509) [ 10.156905] kvm_amd: SEV-ES enabled (ASIDs 1 - 99) [ 10.162256] kvm_amd: SEV-SNP enabled (ASIDs 1 - 99) [ 10.167701] PSP driver not init [ 10.171508] kvm_amd: Virtual VMLOAD VMSAVE supported [ 10.177052] kvm_amd: Virtual GIF supported ... ... [ 10.201648] kvm_amd: in svm_enable_virtualization_cpu WRMSR VM_HSAVE_PA non-zero And then svm_x86_ops->enable_virtualization_cpu() (svm_enable_virtualization_cpu) programs MSR_VM_HSAVE_PA as following: wrmsrl(MSR_VM_HSAVE_PA, sd->save_area_pa); So VM_HSAVE_PA is non-zero before SNP support is enabled on all CPUs. snp_rmptable_init() gets invoked after svm_enable_virtualization_cpu() as following : ... [ 11.256138] kvm_amd: in svm_enable_virtualization_cpu WRMSR VM_HSAVE_PA non-zero ... [ 11.264918] SEV-SNP: in snp_rmptable_init This triggers a #GP exception in snp_rmptable_init() when snp_enable() is invoked to set SNP_EN in SYSCFG MSR: [ 11.294289] unchecked MSR access error: WRMSR to 0xc0010010 (tried to write 0x0000000003fc0000) at rIP: 0xffffffffaf5d5c28 (native_write_msr+0x8/0x30) ... [ 11.294404] Call Trace: [ 11.294482] <IRQ> [ 11.294513] ? show_stack_regs+0x26/0x30 [ 11.294522] ? ex_handler_msr+0x10f/0x180 [ 11.294529] ? search_extable+0x2b/0x40 [ 11.294538] ? fixup_exception+0x2dd/0x340 [ 11.294542] ? exc_general_protection+0x14f/0x440 [ 11.294550] ? asm_exc_general_protection+0x2b/0x30 [ 11.294557] ? __pfx_snp_enable+0x10/0x10 [ 11.294567] ? native_write_msr+0x8/0x30 [ 11.294570] ? __snp_enable+0x5d/0x70 [ 11.294575] snp_enable+0x19/0x20 [ 11.294578] __flush_smp_call_function_queue+0x9c/0x3a0 [ 11.294586] generic_smp_call_function_single_interrupt+0x17/0x20 [ 11.294589] __sysvec_call_function+0x20/0x90 [ 11.294596] sysvec_call_function+0x80/0xb0 [ 11.294601] </IRQ> [ 11.294603] <TASK> [ 11.294605] asm_sysvec_call_function+0x1f/0x30 ... [ 11.294631] arch_cpu_idle+0xd/0x20 [ 11.294633] default_idle_call+0x34/0xd0 [ 11.294636] do_idle+0x1f1/0x230 [ 11.294643] ? complete+0x71/0x80 [ 11.294649] cpu_startup_entry+0x30/0x40 [ 11.294652] start_secondary+0x12d/0x160 [ 11.294655] common_startup_64+0x13e/0x141 [ 11.294662] </TASK> This #GP exception is getting triggered due to the following errata for AMD family 19h Models 10h-1Fh Processors: Processor may generate spurious #GP(0) Exception on WRMSR instruction: Description: The Processor will generate a spurious #GP(0) Exception on a WRMSR instruction if the following conditions are all met: - the target of the WRMSR is a SYSCFG register. - the write changes the value of SYSCFG.SNPEn from 0 to 1. - One of the threads that share the physical core has a non-zero value in the VM_HSAVE_PA MSR. The suggested workaround is when enabling SNP, program VM_HSAVE_PA to 0h on both threads that share a physical core before setting SYSCFG.SNPEn The document being referred to above: https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/revision-guides/57095-PUB_1_01.pdf Therefore, with kvm_amd module being built-in, KVM/SVM initialization happens before Host SNP is enabled and this SVM initialization sets VM_HSAVE_PA to non-zero, which then triggers this #GP when SYSCFG.SNPEn is being set and this will subsequently cause SNP_INIT(_EX) to fail with INVALID_CONFIG error as SYSCFG[SnpEn] is not set on all CPUs. So it looks like the current SNP host enabling code and effectively SNP is broken with respect to the KVM module being built-in. Essentially SNP host enabling code should be invoked before KVM initialization, which is currently not the case when KVM is built-in. Additionally, the PSP driver probably needs to be initialized at device_initcall level if it is built-in, but that is much later than KVM module initialization, therefore, that is blocker for moving SEV/SNP initialization to KVM module load time instead of PSP module probe time. Do note that i have verified and tested that PSP module initialization works when invoked as a device_initcall(). Thanks, Ashish
