Message ID | 14f97f58d6150c6784909261db7f9a05d8d32566.1735931639.git.ashish.kalra@amd.com |
---|---|
State | New |
Headers | show |
Series | Move initializing SEV/SNP functionality to KVM | expand |
+Adding Peter On 1/7/2025 12:34 PM, Kalra, Ashish wrote: > > > On 1/7/2025 10:42 AM, Tom Lendacky wrote: >> On 1/3/25 14:01, Ashish Kalra wrote: >>> From: Ashish Kalra <ashish.kalra@amd.com> >>> >>> Remove platform initialization of SEV/SNP from PSP driver probe time and >> >> Actually, you're not removing it, yet... >> >>> move it to KVM module load time so that KVM can do SEV/SNP platform >>> initialization explicitly if it actually wants to use SEV/SNP >>> functionality. >>> >>> With this patch, KVM will explicitly call into the PSP driver at load time >>> to initialize SEV/SNP by default but this behavior can be altered with KVM >>> module parameters to not do SEV/SNP platform initialization at module load >>> time if required. Additionally SEV/SNP platform shutdown is invoked during >>> KVM module unload time. >>> >>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com> >>> --- >>> arch/x86/kvm/svm/sev.c | 15 ++++++++++++++- >>> 1 file changed, 14 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c >>> index 943bd074a5d3..0dc8294582c6 100644 >>> --- a/arch/x86/kvm/svm/sev.c >>> +++ b/arch/x86/kvm/svm/sev.c >>> @@ -444,7 +444,6 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp, >>> if (ret) >>> goto e_no_asid; >>> >>> - init_args.probe = false; >>> ret = sev_platform_init(&init_args); >>> if (ret) >>> goto e_free; >>> @@ -2953,6 +2952,7 @@ void __init sev_set_cpu_caps(void) >>> void __init sev_hardware_setup(void) >>> { >>> unsigned int eax, ebx, ecx, edx, sev_asid_count, sev_es_asid_count; >>> + struct sev_platform_init_args init_args = {0}; >> >> Will this cause issues if KVM is built-in and INIT_EX is being used >> (init_ex_path ccp parameter)? The probe parameter is used for >> initialization done before the filesystem is available. >> > > Yes, this will cause issues if KVM is builtin and INIT_EX is being used, > but my question is how will INIT_EX be used when we move SEV INIT > to KVM ? > > If we continue to use the probe field here and also continue to support > psp_init_on_probe module parameter for CCP, how will SEV INIT_EX be > invoked ? > > How is SEV INIT_EX invoked in PSP driver currently if psp_init_on_probe > parameter is set to false ? > > The KVM path to invoke sev_platform_init() when a SEV VM is being launched > cannot be used because QEMU checks for SEV to be initialized before > invoking this code path to launch the guest. Peter, I believe that you have a different path to test SEV INIT_EX which won't be affected by this QEMU check. I will add back the probe field and psp_init_on_probe parameter for the CCP module, but i will need your help to test and verify if SEV INIT_EX works with this patch-set. Thanks, Ashish > >> Thanks, >> Tom >> >>> bool sev_snp_supported = false; >>> bool sev_es_supported = false; >>> bool sev_supported = false; >>> @@ -3069,6 +3069,16 @@ void __init sev_hardware_setup(void) >>> sev_supported_vmsa_features = 0; >>> if (sev_es_debug_swap_enabled) >>> sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP; >>> + >>> + if (!sev_enabled) >>> + return; >>> + >>> + /* >>> + * NOTE: Always do SNP INIT regardless of sev_snp_supported >>> + * as SNP INIT has to be done to launch legacy SEV/SEV-ES >>> + * VMs in case SNP is enabled system-wide. >>> + */ >>> + sev_platform_init(&init_args); >>> } >>> >>> void sev_hardware_unsetup(void) >>> @@ -3084,6 +3094,9 @@ void sev_hardware_unsetup(void) >>> >>> misc_cg_set_capacity(MISC_CG_RES_SEV, 0); >>> misc_cg_set_capacity(MISC_CG_RES_SEV_ES, 0); >>> + >>> + /* Do SEV and SNP Shutdown */ >>> + sev_platform_shutdown(); >>> } >>> >>> int sev_cpu_init(struct svm_cpu_data *sd) >
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 943bd074a5d3..0dc8294582c6 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -444,7 +444,6 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp, if (ret) goto e_no_asid; - init_args.probe = false; ret = sev_platform_init(&init_args); if (ret) goto e_free; @@ -2953,6 +2952,7 @@ void __init sev_set_cpu_caps(void) void __init sev_hardware_setup(void) { unsigned int eax, ebx, ecx, edx, sev_asid_count, sev_es_asid_count; + struct sev_platform_init_args init_args = {0}; bool sev_snp_supported = false; bool sev_es_supported = false; bool sev_supported = false; @@ -3069,6 +3069,16 @@ void __init sev_hardware_setup(void) sev_supported_vmsa_features = 0; if (sev_es_debug_swap_enabled) sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP; + + if (!sev_enabled) + return; + + /* + * NOTE: Always do SNP INIT regardless of sev_snp_supported + * as SNP INIT has to be done to launch legacy SEV/SEV-ES + * VMs in case SNP is enabled system-wide. + */ + sev_platform_init(&init_args); } void sev_hardware_unsetup(void) @@ -3084,6 +3094,9 @@ void sev_hardware_unsetup(void) misc_cg_set_capacity(MISC_CG_RES_SEV, 0); misc_cg_set_capacity(MISC_CG_RES_SEV_ES, 0); + + /* Do SEV and SNP Shutdown */ + sev_platform_shutdown(); } int sev_cpu_init(struct svm_cpu_data *sd)