linux-kernel - Re: [RFC 1/5] selftests: KVM: Add a basic 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <3c35c4fd-3cae-4779-8b54-24a6fccb3017@amd.com>
Date: Thu, 11 Jul 2024 11:21:05 -0500
From: "Sampat, Pratik Rajesh" <pratikrajesh.sampat@....com>
To: Peter Gonda <pgonda@...gle.com>
CC: <kvm@...r.kernel.org>, <shuah@...nel.org>, <thomas.lendacky@....com>,
	<michael.roth@....com>, <seanjc@...gle.com>, <pbonzini@...hat.com>,
	<linux-kselftest@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC 1/5] selftests: KVM: Add a basic SNP smoke test

Hi Peter,

Thank you for your review!

On 7/11/2024 10:16 AM, Peter Gonda wrote:
> On Wed, Jul 10, 2024 at 4:06 PM Pratik R. Sampat
> <pratikrajesh.sampat@....com> wrote:
>>
>> Extend sev_smoke_test to also run a minimal SEV-SNP smoke test that
>> initializes and sets up private memory regions required to run a simple
>> SEV-SNP guest.
>>
>> Similar to it's SEV-ES smoke test counterpart, this also does not support
>> GHCB and ucall yet and uses the GHCB MSR protocol to trigger an exit of
>> the type KVM_EXIT_SYSTEM_EVENT.
>>
>> Also, decouple policy and type and require functions to provide both
>> such that there is no assumption regarding the type using policy.
>>
>> Signed-off-by: Pratik R. Sampat <pratikrajesh.sampat@....com>
> 
> Tested-by: Peter Gonda <pgonda@...gle.com>
> 
>>
>> -       test_sev(guest_sev_code, SEV_POLICY_NO_DBG);
>> -       test_sev(guest_sev_code, 0);
>> +       test_sev(guest_sev_code, KVM_X86_SEV_VM, SEV_POLICY_NO_DBG);
>> +       test_sev(guest_sev_code, KVM_X86_SEV_VM, 0);
>>
>>         if (kvm_cpu_has(X86_FEATURE_SEV_ES)) {
>> -               test_sev(guest_sev_es_code, SEV_POLICY_ES | SEV_POLICY_NO_DBG);
>> -               test_sev(guest_sev_es_code, SEV_POLICY_ES);
>> +               test_sev(guest_sev_es_code, KVM_X86_SEV_ES_VM, SEV_POLICY_ES | SEV_POLICY_NO_DBG);
>> +               test_sev(guest_sev_es_code, KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
>> +
>> +               if (kvm_has_cap(KVM_CAP_XCRS) &&
>> +                   (xgetbv(0) & XFEATURE_MASK_X87_AVX) == XFEATURE_MASK_X87_AVX) {
>> +                       test_sync_vmsa(KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
>> +                       test_sync_vmsa(KVM_X86_SEV_ES_VM, SEV_POLICY_ES | SEV_POLICY_NO_DBG);
>> +               }
>> +       }
>> +
>> +       if (kvm_cpu_has(X86_FEATURE_SNP) && is_kvm_snp_supported()) {
>> +               test_sev(guest_snp_code, KVM_X86_SNP_VM, SNP_POLICY_SMT | SNP_POLICY_RSVD_MBO);
> 
> I'd guess most systems have SMT enabled, but is there a way we can
> check and toggle the SNP_POLICY_SMT policy bit programmatically?
> 

We could do that by making a check to /sys/devices/system/cpu/smt/active
maybe?

> Also should we have a base SNP policy so we don't have to read
> `SNP_POLICY_SMT | SNP_POLICY_RSVD_MBO` every time? Not sure I think
> selftests prefer more verbosity.

Sure, that makes sense. I can also include the following to save us a
few keystrokes and help read easier.
#define SNP_POLICY	SNP_POLICY_SMT | SNP_POLICY_RSVD_MBO

Thanks!
Pratik