linux-kernel - Re: [PATCH v6 9/9] KVM: selftests: Add a basic SEV-SNP smoke test

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z6wIDsbjt2ZaiX0I@google.com>
Date: Tue, 11 Feb 2025 18:31:42 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: "Pratik R. Sampat" <prsampat@....com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org, kvm@...r.kernel.org, 
	linux-crypto@...r.kernel.org, linux-kselftest@...r.kernel.org, 
	pbonzini@...hat.com, thomas.lendacky@....com, tglx@...utronix.de, 
	mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com, shuah@...nel.org, 
	pgonda@...gle.com, ashish.kalra@....com, nikunj@....com, pankaj.gupta@....com, 
	michael.roth@....com, sraithal@....com
Subject: Re: [PATCH v6 9/9] KVM: selftests: Add a basic SEV-SNP smoke test

On Mon, Feb 03, 2025, Pratik R. Sampat wrote:
> @@ -217,5 +244,20 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> +	if (kvm_cpu_has(X86_FEATURE_SEV_SNP)) {
> +		uint64_t snp_policy = snp_default_policy();
> +
> +		test_snp(snp_policy);
> +		/* Test minimum firmware level */
> +		test_snp(snp_policy | SNP_FW_VER_MAJOR(SNP_MIN_API_MAJOR) |
> +			SNP_FW_VER_MINOR(SNP_MIN_API_MINOR));

Ah, this is where the firmware policy stuff is used.  Refresh me, can userspace
request _any_ major/minor as the min, and expect failure if the version isn't
supported?  If so, the test should iterate over the major/minor combinations that
are guaranteed to fail.  And if userspace can query the supported minor/major,
the test should iterate over all the happy versions too. 

Unless there's nothing interesting to test, I would move the major/minor stuff to
a separate patch.

> +
> +		test_snp_shutdown(snp_policy);
> +
> +		if (kvm_has_cap(KVM_CAP_XCRS) &&
> +		    (xgetbv(0) & kvm_cpu_supported_xcr0() & xf_mask) == xf_mask)
> +			test_sync_vmsa_snp(snp_policy);

This is all copy+paste from SEV-ES tests, minus SEV_POLICY_NO_DBG.  There's gotta
be a way to dedup this code.

Something like this?

static void needs_a_better_name(uint32_t type, uint64_t policy)
{
	const u64 xf_mask = XFEATURE_MASK_X87_AVX;

	test_sev(guest_sev_code, policy | SEV_POLICY_NO_DBG);
	test_sev(guest_sev_code, policy);

	if (type == KVM_X86_SEV_VM)
		return;

	test_sev_shutdown(policy);

	if (kvm_has_cap(KVM_CAP_XCRS) &&
	    (xgetbv(0) & kvm_cpu_supported_xcr0() & xf_mask) == xf_mask) {
		test_sync_vmsa(policy);
		test_sync_vmsa(policy | SEV_POLICY_NO_DBG);
	}
}

int main(int argc, char *argv[])
{
	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SEV));

	needs_a_better_name(KVM_X86_SEV_VM, 0);

	if (kvm_cpu_has(X86_FEATURE_SEV_ES))
		needs_a_better_name(KVM_X86_SEV_ES_VM, 0);

	if (kvm_cpu_has(X86_FEATURE_SEV_SNP))
		needs_a_better_name(KVM_X86_SEV_SNP_VM, 0);

	return 0;
}