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

From: Janis Schoetterl-Glausch
Date: Mon Jul 18 2022 - 05:34:18 EST


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