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

[Sean Christopherson]

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@xxxxxxxxxx>
> Cc: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
> Cc: Sean Christopherson <seanjc@xxxxxxxxxx>
> Cc: Carlos Bilbao <carlos.bilbao@xxxxxxx>
> Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
> Cc: Michael Roth <michael.roth@xxxxxxx>
> Signed-off-by: Peter Gonda <pgonda@xxxxxxxxxx>
> ---
>  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
>