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

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: <90e2ebcd-f411-8d44-bf44-5df80fb59b51@linux.ibm.com>
Date:   Mon, 18 Jul 2022 11:34:08 +0200
From:   Janis Schoetterl-Glausch <scgl@...ux.ibm.com>
To:     Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Cc:     Amneesh Singh <natto@...rdnatto.in>
Subject: Re: [PATCH] kvm: stats: tell userspace which values are boolean

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@...rdnatto.in>
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> ---
>  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