On incompatible firmware versions, SEV-SNP support is pulled and the
setup is not performed. However, the platform and subsequently the KVM
capability may continue to advertize support for it. Disable support for
SEV-SNP if the FW version validation fails.

Fixes: 1dfe571c12cf ("KVM: SEV: Add initial SEV-SNP support")
Signed-off-by: Pratik R. Sampat <pratikrajesh.sampat@amd.com>
---
 arch/x86/kvm/svm/sev.c       | 4 +++-
 drivers/crypto/ccp/sev-dev.c | 6 ++++++
 include/linux/psp-sev.h      | 3 +++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 72674b8825c4..5ced00e54f0e 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3053,7 +3053,9 @@ void __init sev_hardware_setup(void)
 	sev_es_asid_count = min_sev_asid - 1;
 	WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV_ES, sev_es_asid_count));
 	sev_es_supported = true;
-	sev_snp_supported = sev_snp_enabled && cc_platform_has(CC_ATTR_HOST_SEV_SNP);
+	sev_snp_supported = (sev_snp_enabled &&
+			    cc_platform_has(CC_ATTR_HOST_SEV_SNP) &&
+			    snp_fw_valid());
 
 out:
 	if (boot_cpu_has(X86_FEATURE_SEV))
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index af018afd9cd7..b45cd60c19b0 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -122,6 +122,12 @@ static inline bool sev_version_greater_or_equal(u8 maj, u8 min)
 	return false;
 }
 
+bool snp_fw_valid(void)
+{
+	return sev_version_greater_or_equal(SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR);
+}
+EXPORT_SYMBOL_GPL(snp_fw_valid);
+
 static void sev_irq_handler(int irq, void *data, unsigned int status)
 {
 	struct sev_device *sev = data;
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 903ddfea8585..e841a8fbbb15 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -945,6 +945,7 @@ int sev_do_cmd(int cmd, void *data, int *psp_ret);
 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);
 
 #else	/* !CONFIG_CRYPTO_DEV_SP_PSP */
 
@@ -979,6 +980,8 @@ static inline void *snp_alloc_firmware_page(gfp_t mask)
 
 static inline void snp_free_firmware_page(void *addr) { }
 
+static inline bool snp_fw_valid(void) { return false; }
+
 #endif	/* CONFIG_CRYPTO_DEV_SP_PSP */
 
 #endif	/* __PSP_SEV_H__ */
-- 
2.43.0

On 11/15/2024 5:10 AM, Pratik R. Sampat wrote:
> On incompatible firmware versions, SEV-SNP support is pulled and the
> setup is not performed. However, the platform and subsequently the KVM
> capability may continue to advertize support for it. Disable support for
> SEV-SNP if the FW version validation fails.

Additionally, can we ensure that if sev_platform_init() fails, we do not
indicate SNP support?

Regards
Nikunj

Hi Nikunj,

On 1/9/25 11:21 PM, Nikunj A. Dadhania wrote:
> 
> 
> On 11/15/2024 5:10 AM, Pratik R. Sampat wrote:
>> On incompatible firmware versions, SEV-SNP support is pulled and the
>> setup is not performed. However, the platform and subsequently the KVM
>> capability may continue to advertize support for it. Disable support for
>> SEV-SNP if the FW version validation fails.
> 
> Additionally, can we ensure that if sev_platform_init() fails, we do not
> indicate SNP support?

That sounds good to me. Although if the platform initialization fails,
I think we should not be advertising SEV, SEV-ES as well.

If that makes sense, we could do something similar to before by
exporting another function from ccp that returns whether the platform
is initialized. Then, within kvm's sev_hardware_setup(), we can check
this to ensure that none of the capabilities are set if the platform
initialization has failed?

Thanks!
Pratik

On 1/11/2025 3:20 AM, Pratik Rajesh Sampat wrote:
> Hi Nikunj,
> 
> On 1/9/25 11:21 PM, Nikunj A. Dadhania wrote:
>>
>>
>> On 11/15/2024 5:10 AM, Pratik R. Sampat wrote:
>>> On incompatible firmware versions, SEV-SNP support is pulled and the
>>> setup is not performed. However, the platform and subsequently the KVM
>>> capability may continue to advertize support for it. Disable support for
>>> SEV-SNP if the FW version validation fails.
>>
>> Additionally, can we ensure that if sev_platform_init() fails, we do not
>> indicate SNP support?
> 
> That sounds good to me. Although if the platform initialization fails,
> I think we should not be advertising SEV, SEV-ES as well.

Even better!

> 
> If that makes sense, we could do something similar to before by
> exporting another function from ccp that returns whether the platform
> is initialized. Then, within kvm's sev_hardware_setup(), we can check
> this to ensure that none of the capabilities are set if the platform
> initialization has failed?

Yes, that will ensure we do not advertise any of the SEV capabilities
if the ccp driver has failed loading the firmware or initializing the
platform.

Regards
Nikunj