| Message ID | 20250203223205.36121-3-prsampat@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Basic SEV-SNP Selftests | expand |
On Mon, Feb 03, 2025, Pratik R. Sampat wrote: > If the platform initialization sev_platform_init() fails, SEV cannot be > set up and a secure VM cannot be spawned. Therefore, in this case, > ensure that KVM does not set up, nor advertise support for SEV, SEV-ES, > and SEV-SNP. > > Suggested-by: Nikunj A Dadhania <nikunj@amd.com> > Tested-by: Srikanth Aithal <sraithal@amd.com> > Signed-off-by: Pratik R. Sampat <prsampat@amd.com> > --- > v5..v6: > > * Rename is_sev_platform_init to sev_fw_initialized (Nikunj) > * Collected tags from Srikanth. > --- > arch/x86/kvm/svm/sev.c | 2 +- > drivers/crypto/ccp/sev-dev.c | 10 ++++++++++ > include/linux/psp-sev.h | 3 +++ > 3 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index b709c2f0945c..42d1309f8a54 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -2957,7 +2957,7 @@ void __init sev_hardware_setup(void) > bool sev_es_supported = false; > bool sev_supported = false; > > - if (!sev_enabled || !npt_enabled || !nrips) > + if (!sev_fw_initialized() || !sev_enabled || !npt_enabled || !nrips) > goto out; Me thinks this wasn't tested with KVM_AMD built-in[1]. I'm pretty sure Ashish's fix[2] solves all of this? [1] https://lore.kernel.org/all/d6d08c6b-9602-4f3d-92c2-8db6d50a1b92@amd.com [2] https://lore.kernel.org/all/f78ddb64087df27e7bcb1ae0ab53f55aa0804fab.1739226950.git.ashish.kalra@amd.com
On 2/11/25 7:58 PM, Sean Christopherson wrote: > On Mon, Feb 03, 2025, Pratik R. Sampat wrote: >> If the platform initialization sev_platform_init() fails, SEV cannot be >> set up and a secure VM cannot be spawned. Therefore, in this case, >> ensure that KVM does not set up, nor advertise support for SEV, SEV-ES, >> and SEV-SNP. >> >> Suggested-by: Nikunj A Dadhania <nikunj@amd.com> >> Tested-by: Srikanth Aithal <sraithal@amd.com> >> Signed-off-by: Pratik R. Sampat <prsampat@amd.com> >> --- >> v5..v6: >> >> * Rename is_sev_platform_init to sev_fw_initialized (Nikunj) >> * Collected tags from Srikanth. >> --- >> arch/x86/kvm/svm/sev.c | 2 +- >> drivers/crypto/ccp/sev-dev.c | 10 ++++++++++ >> include/linux/psp-sev.h | 3 +++ >> 3 files changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c >> index b709c2f0945c..42d1309f8a54 100644 >> --- a/arch/x86/kvm/svm/sev.c >> +++ b/arch/x86/kvm/svm/sev.c >> @@ -2957,7 +2957,7 @@ void __init sev_hardware_setup(void) >> bool sev_es_supported = false; >> bool sev_supported = false; >> >> - if (!sev_enabled || !npt_enabled || !nrips) >> + if (!sev_fw_initialized() || !sev_enabled || !npt_enabled || !nrips) >> goto out; > > Me thinks this wasn't tested with KVM_AMD built-in[1]. I'm pretty sure Ashish's > fix[2] solves all of this? > > [1] https://lore.kernel.org/all/d6d08c6b-9602-4f3d-92c2-8db6d50a1b92@amd.com > [2] https://lore.kernel.org/all/f78ddb64087df27e7bcb1ae0ab53f55aa0804fab.1739226950.git.ashish.kalra@amd.com Thanks for pointing this out. I hadn't considered this earlier. The combination of both of these should solve all of the issues caused by a failing SEV INIT. I will remove this patch for the next iteration then. Thanks!
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index b709c2f0945c..42d1309f8a54 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -2957,7 +2957,7 @@ void __init sev_hardware_setup(void) bool sev_es_supported = false; bool sev_supported = false; - if (!sev_enabled || !npt_enabled || !nrips) + if (!sev_fw_initialized() || !sev_enabled || !npt_enabled || !nrips) goto out; /* diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c index b45cd60c19b0..bd166b997ac9 100644 --- a/drivers/crypto/ccp/sev-dev.c +++ b/drivers/crypto/ccp/sev-dev.c @@ -81,6 +81,8 @@ MODULE_FIRMWARE("amd/amd_sev_fam19h_model1xh.sbin"); /* 4th gen EPYC */ static bool psp_dead; static int psp_timeout; +static bool platform_init; + /* Trusted Memory Region (TMR): * The TMR is a 1MB area that must be 1MB aligned. Use the page allocator * to allocate the memory, which will return aligned memory for the specified @@ -1358,6 +1360,12 @@ int sev_platform_init(struct sev_platform_init_args *args) } EXPORT_SYMBOL_GPL(sev_platform_init); +bool sev_fw_initialized(void) +{ + return platform_init; +} +EXPORT_SYMBOL_GPL(sev_fw_initialized); + static int __sev_platform_shutdown_locked(int *error) { struct psp_device *psp = psp_master; @@ -2427,6 +2435,8 @@ void sev_pci_init(void) if (rc) dev_err(sev->dev, "SEV: failed to INIT error %#x, rc %d\n", args.error, rc); + else + platform_init = true; dev_info(sev->dev, "SEV%s API:%d.%d build:%d\n", sev->snp_initialized ? "-SNP" : "", sev->api_major, sev->api_minor, sev->build); diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h index e841a8fbbb15..55ef473953e3 100644 --- a/include/linux/psp-sev.h +++ b/include/linux/psp-sev.h @@ -946,6 +946,7 @@ void *psp_copy_user_blob(u64 uaddr, u32 len); void *snp_alloc_firmware_page(gfp_t mask); void snp_free_firmware_page(void *addr); bool snp_fw_valid(void); +bool sev_fw_initialized(void); #else /* !CONFIG_CRYPTO_DEV_SP_PSP */ @@ -982,6 +983,8 @@ static inline void snp_free_firmware_page(void *addr) { } static inline bool snp_fw_valid(void) { return false; } +static inline bool sev_fw_initialized(void) { return false; } + #endif /* CONFIG_CRYPTO_DEV_SP_PSP */ #endif /* __PSP_SEV_H__ */