Re: [PATCH] kvm: stats: tell userspace which values are boolean

[Janis Schoetterl-Glausch]

On 7/14/22 14:03, Paolo Bonzini wrote:
> Some of the statistics values exported by KVM are always only 0 or 1.
> It can be useful to export this fact to userspace so that it can track
> them specially (for example by polling the value every now and then to
> compute a % of time spent in a specific state).
> 
> Therefore, add "boolean value" as a new "unit".  While it is not exactly
> a unit, it walks and quacks like one.  In particular, using the type
> would be wrong because boolean values could be instantaneous or peak
> values (e.g. "is the rmap allocated?") or even two-bucket histograms
> (e.g. "number of posted vs. non-posted interrupt injections").
> 
> Suggested-by: Amneesh Singh <natto@xxxxxxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>  Documentation/virt/kvm/api.rst |  6 ++++++
>  arch/x86/kvm/x86.c             |  2 +-
>  include/linux/kvm_host.h       | 11 ++++++++++-
>  include/uapi/linux/kvm.h       |  1 +
>  4 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 11e00a46c610..48bf6e49a7de 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -5657,6 +5657,7 @@ by a string of size ``name_size``.
>  	#define KVM_STATS_UNIT_BYTES		(0x1 << KVM_STATS_UNIT_SHIFT)
>  	#define KVM_STATS_UNIT_SECONDS		(0x2 << KVM_STATS_UNIT_SHIFT)
>  	#define KVM_STATS_UNIT_CYCLES		(0x3 << KVM_STATS_UNIT_SHIFT)
> +	#define KVM_STATS_UNIT_BOOLEAN		(0x4 << KVM_STATS_UNIT_SHIFT)
>  	#define KVM_STATS_UNIT_MAX		KVM_STATS_UNIT_CYCLES
> 
>  	#define KVM_STATS_BASE_SHIFT		8

[...]

> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 5088bd9f1922..811897dadcae 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -2083,6 +2083,7 @@ struct kvm_stats_header {
>  #define KVM_STATS_UNIT_BYTES		(0x1 << KVM_STATS_UNIT_SHIFT)
>  #define KVM_STATS_UNIT_SECONDS		(0x2 << KVM_STATS_UNIT_SHIFT)
>  #define KVM_STATS_UNIT_CYCLES		(0x3 << KVM_STATS_UNIT_SHIFT)
> +#define KVM_STATS_UNIT_BOOLEAN		(0x4 << KVM_STATS_UNIT_SHIFT)
>  #define KVM_STATS_UNIT_MAX		KVM_STATS_UNIT_CYCLES

Shouldn't KVM_STATS_UNIT_MAX be KVM_STATS_UNIT_BOOLEAN then?

The selftest has:
	TEST_ASSERT((pdesc->flags & KVM_STATS_UNIT_MASK)
                                <= KVM_STATS_UNIT_MAX, "Unknown KVM stats unit");
> 
>  #define KVM_STATS_BASE_SHIFT		8