Re: [PATCH 5/6] Add is_sev_enabled() helpers

Sean Christopherson
2024-04-23T23:12:58Z

On Tue, Apr 09, 2024, Peter Gonda wrote:
> Add helper functions for guest code to check the status of SEV and
> SEV-ES.

Why? The names are super ambiguous, e.g. they could just as easily mean "is SEV
enabled in KVM" or "is SEV enabled in CPUID". And if an assert fires because
is_sev_es_enabled() returns false, the user will get a _worse_ error message because
all they'll know is _something_ in is_sev_es_enabled() failed, not which MSR bit
came back 'bad.

> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Carlos Bilbao <carlos.bilbao@amd.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Michael Roth <michael.roth@amd.com>
> Signed-off-by: Peter Gonda <pgonda@google.com>
> ---
> tools/testing/selftests/kvm/include/x86_64/sev.h | 3 +++
> tools/testing/selftests/kvm/lib/x86_64/sev.c | 11 +++++++++++
> tools/testing/selftests/kvm/x86_64/sev_smoke_test.c | 5 ++---
> 3 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h b/tools/testing/selftests/kvm/include/x86_64/sev.h
> index bfd481707f67..691dc005e2a1 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/sev.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
> @@ -106,4 +106,7 @@ static inline void sev_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa,
> vm_sev_ioctl(vm, KVM_SEV_LAUNCH_UPDATE_DATA, &update_data);
> }
>
> +bool is_sev_enabled(void);
> +bool is_sev_es_enabled(void);
> +
> #endif /* SELFTEST_KVM_SEV_H */
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c
> index 27ae1d3b1355..5b3f0a8a931a 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/sev.c
> @@ -189,3 +189,14 @@ struct kvm_vm *vm_sev_create_with_one_vcpu(uint32_t policy, void *guest_code,
>
> return vm;
> }
> +
> +bool is_sev_enabled(void)
> +{
> + return rdmsr(MSR_AMD64_SEV) & MSR_AMD64_SEV_ENABLED;
> +}
> +
> +bool is_sev_es_enabled(void)
> +{
> + return is_sev_enabled() &&
> + rdmsr(MSR_AMD64_SEV) & MSR_AMD64_SEV_ES_ENABLED;
> +}
> diff --git a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
> index 026779f3ed06..1d84e78e7ae2 100644
> --- a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
> @@ -16,8 +16,7 @@
> static void guest_sev_es_code(void)
> {
> /* TODO: Check CPUID after GHCB-based hypercall support is added. */
> - GUEST_ASSERT(rdmsr(MSR_AMD64_SEV) & MSR_AMD64_SEV_ENABLED);
> - GUEST_ASSERT(rdmsr(MSR_AMD64_SEV) & MSR_AMD64_SEV_ES_ENABLED);
> + GUEST_ASSERT(is_sev_es_enabled());
>
> /*
> * TODO: Add GHCB and ucall support for SEV-ES guests. For now, simply
> @@ -30,7 +29,7 @@ static void guest_sev_es_code(void)
> static void guest_sev_code(void)
> {
> GUEST_ASSERT(this_cpu_has(X86_FEATURE_SEV));
> - GUEST_ASSERT(rdmsr(MSR_AMD64_SEV) & MSR_AMD64_SEV_ENABLED);
> + GUEST_ASSERT(is_sev_enabled());
>
> GUEST_DONE();
> }
> --
> 2.44.0.478.gd926399ef9-goog
>