Re: [PATCH v7 2/4] KVM: stats: Add fd-based API to read binary stats data
CLES (0x3 << KVM_STATS_UNIT_SHIFT)
+#define KVM_STATS_UNIT_MAX KVM_STATS_UNIT_CYCLES
+
+#define KVM_STATS_BASE_SHIFT 8
+#define KVM_STATS_BASE_MASK(0xF << KVM_STATS_BASE_SHIFT)
+#define KVM_STATS_BASE_POW10 (0x0 << KVM_STATS_BASE_SHIFT)
+#define KVM_STATS_BASE_POW2(0x1 << KVM_STATS_BASE_SHIFT)
+#define KVM_STATS_BASE_MAX KVM_STATS_BASE_POW2
+
+struct kvm_stats_desc {
+ __u32 flags;
+ __s16 exponent;
+ __u16 size;
+ __u32 unused1;
+ __u32 unused2;
+ char name[0];
+};
+
+struct kvm_vm_stats_data {
+ unsigned long value[0];
+};
+
+struct kvm_vcpu_stats_data {
+ __u64 value[0];
+};
+
+#define KVM_GET_STATS_FD _IOR(KVMIO, 0xcc, struct kvm_stats_header)
+
#endif /* __LINUX_KVM_H */
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f6ad5b080994..d84bb17bdea8 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3409,6 +3409,115 @@ static int kvm_vcpu_ioctl_set_sigmask(struct kvm_vcpu
*vcpu, sigset_t *sigset)
return 0;
}
+static ssize_t kvm_stats_read(char *id, struct _kvm_stats_header *header,
+ struct _kvm_stats_desc *desc, void *stats, size_t size_stats,
+ char __user *user_buffer, size_t size, loff_t *offset)
+{
+ ssize_t copylen, len, remain = size;
+ size_t size_header, size_desc;
+ loff_t pos = *offset;
+ char __user *dest = user_buffer;
+ void *src;
+
+ size_header = sizeof(*header);
+ size_desc = header->count * sizeof(*desc);
+
+ len = KVM_STATS_ID_MAXLEN + size_header + size_desc + size_stats - pos;
+ len = min(len, remain);
+ if (len <= 0)
+ return 0;
+ remain = len;
+
+ /* Copy kvm stats header id string */
+ copylen = KVM_STATS_ID_MAXLEN - pos;
+ copylen = min(copylen, remain);
+ if (copylen > 0) {
+ src = id + pos;
+ if (copy_to_user(dest, src, copylen))
+ return -EFAULT;
+ remain -= copylen;
+ pos += copylen;
+ dest += copylen;
+ }
+ /* Copy kvm stats header */
+ copylen = KVM_STATS_ID_MAXLEN + size_header - pos;
+ copylen = min(copylen, remain);
+ if (copylen > 0) {
+ src = header + pos - KVM_STATS_ID_MAXLEN;
+ if (copy_to_user(dest, src, copylen))
+ return -EFAULT;
+ remain -= copylen;
+ pos += copylen;
+ dest += copylen;
+ }
+ /* Copy kvm stats descriptors */
+ copylen = header->desc_offset + size_desc - pos;
+ copylen = min(copylen, remain);
+ if (copylen > 0) {
+ src = desc + pos - header->desc_offset;
+ if (copy_to_user(dest, src, copylen))
+ return -EFAULT;
+ remain -= copylen;
+ pos += copylen;
+ dest += copylen;
+ }
+ /* Copy kvm stats values */
+ copylen = header->data_offset + size_stats - pos;
+ copylen = min(copylen, remain);
+ if (copylen > 0) {
+ src = stats + pos - header->data_offset;
+ if (copy_to_user(dest, src, copylen))
+ return -EFAULT;
+ remain -= copylen;
+ pos += copylen;
+ dest += copylen;
+ }
+
+ *offset = pos;
+ return len;
+}
+
+static ssize_t kvm_vcpu_stats_read(struct file *file, char __user *user_buffer,
+ size_t size, loff_t *offset)
+{
+ char id[KVM_STATS_ID_MAXLEN];
+ struct kvm_vcpu *vcpu = file->private_data;
+
+ snprintf(id, sizeof(id), "kvm-%d/vcpu-%d",
+ task_pid_nr(current), vcpu->vcpu_id);
+ return kvm_stats_read(id, &kvm_vcpu_stats_header,
+ &kvm_vcpu_stats_desc[0], &vcpu->stat,
+ sizeof(vcpu->stat), user_buffer, size, offset);
+}
+
+static const struct file_operations kvm_vcpu_stats_fops = {
+ .read = kvm_vcpu_stats_read,
+ .llseek = noop_llseek,
+};
+
+static int kvm_vcpu_ioctl_get_stats_fd(struct kvm_vcpu *vcpu)
+{
+ int fd;
+ struct file *file;
+ char name[15 + ITOA_MAX_LEN + 1];
+
+ snprintf(name, sizeof(name), "kvm-vcpu-stats:%d", vcpu->vcpu_id);
+
+ fd = get_unused_fd_flags(O_CLOEXEC);
+ if (fd < 0)
+ return fd;
+
+ file = anon_inode_getfile(name, &kvm_vcpu_stats_fops, vcpu, O_RDONLY);
+ if (IS_ERR(file)) {
+ put_unused_fd(fd);
+ return PTR_ERR(file);
+ }
+ file->f_mode |= FMODE_PREAD;
+ fd_install(fd, file);
+
+ return fd;
+}
+
static long kvm_vcpu_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg)
{
@@ -3606,6 +3715,10 @@ static long kvm_vcpu_ioctl(struct file *filp,
r = kvm_arch_vcpu_ioctl_set_fpu(vcpu, fpu);
break;
}
+ case KVM_GET_STATS_FD: {
+ r = kvm_vcpu_ioctl_get_stats_fd(vcpu);
+ break;
+ }
default:
r = kvm_arch_vcpu_ioctl(filp, ioctl, arg);
}
@@ -3864,6 +3977,8 @@ static long kvm_vm_ioctl_check_extension_generic(struct
kvm *kvm, long arg)
#else
return 0;
#endif
+ case KVM_CAP_STATS_BINARY_FD:
Nit: KVM_CAP_BINARY_STATS_FD
+ return 1;
default:
break;
}
@@ -3967,6 +4082,43 @@ static int kvm_vm_ioctl_enable_cap_generic(struct kvm
*kvm,
}
}
+static ssize_t kvm_vm_stats_read(struct file *file, char __user *user_buffer,
+ size_t size, loff_t *offset)
+{
+ char id[KVM_STATS_ID_MAXLEN];
+ struct kvm *kvm = file->private_data;
+
+ snprintf(id, sizeof(id), "kvm-%d", task_pid_nr(current));
+ return kvm_stats_read(id, &kvm_vm_stats_header, &kvm_vm_stats_desc[0],
+ &kvm->stat, sizeof(kvm->stat), user_buffer, size, offset);
+}
+
+static const struct file_operations kvm_vm_stats_fops = {
+ .read = kvm_vm_stats_read,
+ .llseek = noop_llseek,
+};
+
+static int kvm_vm_ioctl_get_stats_fd(struct kvm *kvm)
+{
+ int fd;
+ struct file *file;
+
+ fd = get_unused_fd_flags(O_CLOEXEC);
+ if (fd < 0)
+ return fd;
+
+ file = anon_inode_getfile("kvm-vm-stats",
+ &kvm_vm_stats_fops, kvm, O_RDONLY);
+ if (IS_ERR(file)) {
+ put_unused_fd(fd);
+ return PTR_ERR(file);
+ }
+ file->f_mode |= FMODE_PREAD;
+ fd_install(fd, file);
+
+ return fd;
+}
+
static long kvm_vm_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg)
{
@@ -4149,6 +4301,9 @@ static long kvm_vm_ioctl(struct file *filp,
case KVM_RESET_DIRTY_RINGS:
r = kvm_vm_ioctl_reset_dirty_pages(kvm);
break;
+ case KVM_GET_STATS_FD:
+ r = kvm_vm_ioctl_get_stats_fd(kvm);
+ break;
default:
r = kvm_arch_vm_ioctl(filp, ioctl, arg);
}
Reviewed-by: Krish Sadhukhan
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v7 4/4] KVM: selftests: Add selftest for KVM statistics data binary interface
;Stats fd not freed");
+}
+
+#define DEFAULT_NUM_VM 4
+#define DEFAULT_NUM_VCPU 4
+
+/*
+ * Usage: kvm_bin_form_stats [#vm] [#vcpu]
+ * The first parameter #vm set the number of VMs being created.
+ * The second parameter #vcpu set the number of VCPUs being created.
+ * By default, DEFAULT_NUM_VM VM and DEFAULT_NUM_VCPU VCPU for the VM would be
+ * created for testing.
+ */
+
+int main(int argc, char *argv[])
+{
+ int max_vm = DEFAULT_NUM_VM, max_vcpu = DEFAULT_NUM_VCPU, ret, i, j;
+ struct kvm_vm **vms;
+
+ /* Get the number of VMs and VCPUs that would be created for testing. */
+ if (argc > 1) {
+ max_vm = strtol(argv[1], NULL, 0);
+ if (max_vm <= 0)
+ max_vm = DEFAULT_NUM_VM;
+ }
+ if (argc > 2) {
+ max_vcpu = strtol(argv[2], NULL, 0);
+ if (max_vcpu <= 0)
+ max_vcpu = DEFAULT_NUM_VCPU;
+ }
+
+ /* Check the extension for binary stats */
+ ret = kvm_check_cap(KVM_CAP_STATS_BINARY_FD);
+ TEST_ASSERT(ret >= 0,
+ "Binary form statistics interface is not supported");
+
+ /* Create VMs and VCPUs */
+ vms = malloc(sizeof(vms[0]) * max_vm);
+ TEST_ASSERT(vms, "Allocate memory for storing VM pointers");
+ for (i = 0; i < max_vm; ++i) {
+ vms[i] = vm_create(VM_MODE_DEFAULT,
+ DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+ for (j = 0; j < max_vcpu; ++j)
+ vm_vcpu_add(vms[i], j);
+ }
+
+ /* Check stats read for every VM and VCPU */
+ for (i = 0; i < max_vm; ++i) {
+ vm_stats_test(vms[i]);
+ for (j = 0; j < max_vcpu; ++j)
+ vcpu_stats_test(vms[i], j);
+ }
+
+ for (i = 0; i < max_vm; ++i)
+ kvm_vm_free(vms[i]);
+ free(vms);
+ return 0;
+}
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
b/tools/testing/selftests/kvm/lib/kvm_util.c
index fc83f6c5902d..10385b76fe11 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -2090,3 +2090,15 @@ unsigned int vm_calc_num_guest_pages(enum vm_guest_mode
mode, size_t size)
n = DIV_ROUND_UP(size, vm_guest_mode_params[mode].page_size);
return vm_adjust_num_guest_pages(mode, n);
}
+
+int vm_get_stats_fd(struct kvm_vm *vm)
+{
+ return ioctl(vm->fd, KVM_GET_STATS_FD, NULL);
+}
+
+int vcpu_get_stats_fd(struct kvm_vm *vm, uint32_t vcpuid)
+{
+ struct vcpu *vcpu = vcpu_find(vm, vcpuid);
+
+ return ioctl(vcpu->fd, KVM_GET_STATS_FD, NULL);
+}
We don't want to add a test case for testing the fd interface on a
deleted VM and a deleted VCPU ?
Anyway, for the current content,
Reviewed-by: Krish Sadhukhan
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v7 3/4] KVM: stats: Add documentation for statistics data binary interface
decreased. This type is usually used as a measurement of some resources,
+like the number of dirty pages, the number of large pages, etc.
+The corresponding ``count`` field for this type is always 1.
+
+Bits 4-7 of ``flags`` encode the unit:
+ * ``KVM_STATS_UNIT_NONE``
+There is no unit for the value of statistics data. This usually means that
+the value is a simple counter of an event.
+ * ``KVM_STATS_UNIT_BYTES``
+It indicates that the statistics data is used to measure memory size, in
the
+unit of Byte, KiByte, MiByte, GiByte, etc. The unit of the data is
+determined by the ``exponent`` field in the descriptor. The
+``KVM_STATS_BASE_POW2`` flag is valid in this case. The unit of the data is
+determined by ``pow(2, exponent)``. For example, if value is 10,
+``exponent`` is 20, which means the unit of statistics data is MiByte, we
+can get the statistics data in the unit of Byte by
+``value * pow(2, exponent) = 10 * pow(2, 20) = 10 MiByte`` which is
+10 * 1024 * 1024 Bytes.
+ * ``KVM_STATS_UNIT_SECONDS``
+It indicates that the statistics data is used to measure time/latency, in
+the unit of nanosecond, microsecond, millisecond and second. The unit of
the
+data is determined by the ``exponent`` field in the descriptor. The
+``KVM_STATS_BASE_POW10`` flag is valid in this case. The unit of the data
+is determined by ``pow(10, exponent)``. For example, if value is 200,
+``exponent`` is -6, which means the unit of statistics data is microsecond,
+we can get the statistics data in the unit of second by
+``value * pow(10, exponent) = 200 * pow(10, -6) = 2 seconds``.
+ * ``KVM_STATS_UNIT_CYCLES``
+It indicates that the statistics data is used to measure CPU clock cycles.
+The ``KVM_STATS_BASE_POW10`` flag is valid in this case. For example, if
+value is 200, ``exponent`` is 4, we can get the number of CPU clock cycles
+by ``value * pow(10, exponent) = 200 * pow(10, 4) = 200``.
+
+Bits 7-11 of ``flags`` encode the base:
+ * ``KVM_STATS_BASE_POW10``
+The scale is based on power of 10. It is used for measurement of time and
+CPU clock cycles.
+ * ``KVM_STATS_BASE_POW2``
+The scale is based on power of 2. It is used for measurement of memory
size.
+
+The ``exponent`` field is the scale of corresponding statistics data. For
+example, if the unit is ``KVM_STATS_UNIT_BYTES``, the base is
+``KVM_STATS_BASE_POW2``, the ``exponent`` is 10, then we know that the real
+unit of the statistics data is KBytes a.k.a pow(2, 10) = 1024 bytes.
+
+The ``size`` field is the number of values of this statistics data. It is in
the
+unit of ``unsigned long`` for VM or ``__u64`` for VCPU.
+
+The ``unused1`` and ``unused2`` fields are reserved for future
+support for other types of statistics data, like log/linear histogram.
+
+The ``name`` field points to the name string of the statistics data. The name
+string starts at the end of ``struct kvm_stats_desc``.
+The maximum length (including trailing '\0') is indicated by ``name_size``
+in ``struct kvm_stats_header``.
+
+The Stats Data block contains an array of data values of type ``struct
+kvm_vm_stats_data`` or ``struct kvm_vcpu_stats_data``. It would be read by
+user space periodically to pull statistics data.
+The order of data value in Stats Data block is the same as the order of
+descriptors in Descriptors block.
+ * Statistics data for VM::
+
+ struct kvm_vm_stats_data {
+ unsigned long value[0];
+ };
+
+ * Statistics data for VCPU::
+
+ struct kvm_vcpu_stats_data {
+ __u64 value[0];
+ };
+
5. The kvm_run structure
@@ -6891,3 +7063,11 @@ This capability is always enabled.
This capability indicates that the KVM virtual PTP service is
supported in the host. A VMM can check whether the service is
available to the guest on migration.
+
+8.33 KVM_CAP_STATS_BINARY_FD
+
+
+:Architectures: all
+
+This capability indicates the feature that user space can create get a file
+descriptor for every VM and VCPU to read statistics data in binary format.
Reviewed-by: Krish Sadhukhan
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v7 1/4] KVM: stats: Separate generic stats from architecture specific ones
uot;directed_yield_successful", directed_yield_successful),
@@ -253,7 +253,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
VM_STAT("mmu_recycled", mmu_recycled),
VM_STAT("mmu_cache_miss", mmu_cache_miss),
VM_STAT("mmu_unsync", mmu_unsync),
- VM_STAT("remote_tlb_flush", remote_tlb_flush),
+ VM_STAT_GENERIC("remote_tlb_flush", remote_tlb_flush),
VM_STAT("largepages", lpages, .mode = 0444),
VM_STAT("nx_largepages_splitted", nx_lpage_splits, .mode = 0444),
VM_STAT("max_mmu_page_hash_collisions", max_mmu_page_hash_collisions),
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 2f34487e21f2..1870fa928762 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1243,10 +1243,15 @@ struct kvm_stats_debugfs_item {
#define KVM_DBGFS_GET_MODE(dbgfs_item)
\
((dbgfs_item)->mode ? (dbgfs_item)->mode : 0644)
-#define VM_STAT(n, x, ...) \
+#define VM_STAT(n, x, ...)\
{ n, offsetof(struct kvm, stat.x), KVM_STAT_VM, ## __VA_ARGS__ }
-#define VCPU_STAT(n, x, ...)
\
+#define VCPU_STAT(n, x, ...) \
{ n, offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU, ## __VA_ARGS__ }
+#define VM_STAT_GENERIC(n, x, ...)\
+ { n, offsetof(struct kvm, stat.generic.x), KVM_STAT_VM, ## __VA_ARGS__ }
+#define VCPU_STAT_GENERIC(n, x, ...) \
+ { n, offsetof(struct kvm_vcpu, stat.generic.x),\
+ KVM_STAT_VCPU, ## __VA_ARGS__ }
extern struct kvm_stats_debugfs_item debugfs_entries[];
extern struct dentry *kvm_debugfs_dir;
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index a7580f69dda0..7c39489f9953 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -76,5 +76,17 @@ struct kvm_mmu_memory_cache {
};
#endif
+struct kvm_vm_stat_generic {
+ ulong remote_tlb_flush;
+};
+
+struct kvm_vcpu_stat_generic {
+ u64 halt_successful_poll;
+ u64 halt_attempted_poll;
+ u64 halt_poll_invalid;
+ u64 halt_wakeup;
+ u64 halt_poll_success_ns;
+ u64 halt_poll_fail_ns;
+};
#endif /* __KVM_TYPES_H__ */
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 6b4feb92dc79..f6ad5b080994 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -330,7 +330,7 @@ void kvm_flush_remote_tlbs(struct kvm *kvm)
*/
if (!kvm_arch_flush_remote_tlb(kvm)
|| kvm_make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH))
- ++kvm->stat.remote_tlb_flush;
+ ++kvm->stat.generic.remote_tlb_flush;
cmpxchg(&kvm->tlbs_dirty, dirty_count, 0);
}
EXPORT_SYMBOL_GPL(kvm_flush_remote_tlbs);
@@ -2940,9 +2940,9 @@ static inline void
update_halt_poll_stats(struct kvm_vcpu *vcpu, u64 poll_ns, bool waited)
{
if (waited)
- vcpu->stat.halt_poll_fail_ns += poll_ns;
+ vcpu->stat.generic.halt_poll_fail_ns += poll_ns;
else
- vcpu->stat.halt_poll_success_ns += poll_ns;
+ vcpu->stat.generic.halt_poll_success_ns += poll_ns;
}
/*
@@ -2960,16 +2960,16 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
if (vcpu->halt_poll_ns && !kvm_arch_no_poll(vcpu)) {
ktime_t stop = ktime_add_ns(ktime_get(), vcpu->halt_poll_ns);
- ++vcpu->stat.halt_attempted_poll;
+ ++vcpu->stat.generic.halt_attempted_poll;
do {
/*
* This sets KVM_REQ_UNHALT if an interrupt
* arrives.
*/
if (kvm_vcpu_check_block(vcpu) < 0) {
- ++vcpu->stat.halt_successful_poll;
+ ++vcpu->stat.generic.halt_successful_poll;
if (!vcpu_valid_wakeup(vcpu))
- ++vcpu->stat.halt_poll_invalid;
+ ++vcpu->stat.generic.halt_poll_invalid;
goto out;
}
poll_end = cur = ktime_get();
@@ -3027,7 +3027,7 @@ bool kvm_vcpu_wake_up(struct kvm_vcpu *vcpu)
waitp = kvm_arch_vcpu_get_wait(vcpu);
if (rcuwait_wake_up(waitp)) {
WRITE_ONCE(vcpu->ready, true);
- ++vcpu->stat.halt_wakeup;
+ ++vcpu->stat.generic.halt_wakeup;
return true;
}
Reviewed-by: Krish Sadhukhan
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v6 4/4] KVM: selftests: Add selftest for KVM statistics data binary interface
On 5/24/21 8:18 AM, Jing Zhang wrote:
Add selftest to check KVM stats descriptors validity.
Reviewed-by: David Matlack
Reviewed-by: Ricardo Koller
Signed-off-by: Jing Zhang
---
tools/testing/selftests/kvm/.gitignore| 1 +
tools/testing/selftests/kvm/Makefile | 3 +
.../testing/selftests/kvm/include/kvm_util.h | 3 +
.../selftests/kvm/kvm_bin_form_stats.c| 216 ++
tools/testing/selftests/kvm/lib/kvm_util.c| 12 +
5 files changed, 235 insertions(+)
create mode 100644 tools/testing/selftests/kvm/kvm_bin_form_stats.c
We should probably follow the naming convention for the majority of the
files in the kvm directory and name it kvm_stats_read_test.c or
kvm_stats_test.c or something like that.
diff --git a/tools/testing/selftests/kvm/.gitignore
b/tools/testing/selftests/kvm/.gitignore
index bd83158e0e0b..35796667c944 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -43,3 +43,4 @@
/memslot_modification_stress_test
/set_memory_region_test
/steal_time
+/kvm_bin_form_stats
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index e439d027939d..2984c86c848a 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -76,6 +76,7 @@ TEST_GEN_PROGS_x86_64 += kvm_page_table_test
TEST_GEN_PROGS_x86_64 += memslot_modification_stress_test
TEST_GEN_PROGS_x86_64 += set_memory_region_test
TEST_GEN_PROGS_x86_64 += steal_time
+TEST_GEN_PROGS_x86_64 += kvm_bin_form_stats
TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list-sve
@@ -87,6 +88,7 @@ TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
TEST_GEN_PROGS_aarch64 += kvm_page_table_test
TEST_GEN_PROGS_aarch64 += set_memory_region_test
TEST_GEN_PROGS_aarch64 += steal_time
+TEST_GEN_PROGS_aarch64 += kvm_bin_form_stats
TEST_GEN_PROGS_s390x = s390x/memop
TEST_GEN_PROGS_s390x += s390x/resets
@@ -96,6 +98,7 @@ TEST_GEN_PROGS_s390x += dirty_log_test
TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
TEST_GEN_PROGS_s390x += kvm_page_table_test
TEST_GEN_PROGS_s390x += set_memory_region_test
+TEST_GEN_PROGS_s390x += kvm_bin_form_stats
TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
LIBKVM += $(LIBKVM_$(UNAME_M))
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
b/tools/testing/selftests/kvm/include/kvm_util.h
index a8f022794ce3..ee01a67022d9 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -387,4 +387,7 @@ uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id,
struct ucall *uc);
#define GUEST_ASSERT_4(_condition, arg1, arg2, arg3, arg4) \
__GUEST_ASSERT((_condition), 4, (arg1), (arg2), (arg3), (arg4))
+int vm_get_statsfd(struct kvm_vm *vm);
+int vcpu_get_statsfd(struct kvm_vm *vm, uint32_t vcpuid);
+
#endif /* SELFTEST_KVM_UTIL_H */
diff --git a/tools/testing/selftests/kvm/kvm_bin_form_stats.c
b/tools/testing/selftests/kvm/kvm_bin_form_stats.c
new file mode 100644
index ..09e12c5838af
--- /dev/null
+++ b/tools/testing/selftests/kvm/kvm_bin_form_stats.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * kvm_bin_form_stats
+ *
+ * Copyright (C) 2021, Google LLC.
+ *
+ * Test the fd-based interface for KVM statistics.
+ */
+
+#define _GNU_SOURCE /* for program_invocation_short_name */
+#include
+#include
+#include
+#include
+#include
+
+#include "test_util.h"
+
+#include "kvm_util.h"
+#include "asm/kvm.h"
+#include "linux/kvm.h"
+
+int stats_test(int stats_fd, int size_stat)
The return value is not used by the caller. Perhaps make it a void ?
+{
+ ssize_t ret;
+ int i;
+ size_t size_desc, size_data = 0;
+ struct kvm_stats_header header;
+ struct kvm_stats_desc *stats_desc, *pdesc;
+ void *stats_data;
+
+ /* Read kvm stats header */
+ ret = read(stats_fd, &header, sizeof(header));
+ TEST_ASSERT(ret == sizeof(header), "Read stats header");
+ size_desc = sizeof(*stats_desc) + header.name_size;
+
+ /* Check id string in header, that should start with "kvm" */
+ TEST_ASSERT(!strncmp(header.id, "kvm", 3) &&
+ strlen(header.id) < KVM_STATS_ID_MAXLEN,
+ "Invalid KVM stats type");
+
+ /* Sanity check for other fields in header */
+ if (header.count == 0)
Does this need a message as to why count is zero ?
+ return 0;
+ /* Check overlap */
+ TEST_ASSERT(header.desc_offset > 0 && header.data_offset > 0
+ && header.desc_offset >= sizeof(header)
+ && header.data_offset >= sizeof(header),
+ "Invalid offset fields in header");
+ TEST_ASSERT(header.desc_offset > header.data_offset
+ || (header.desc_offset + size_desc * header.count <=
+
Re: [PATCH v6 2/4] KVM: stats: Add fd-based API to read binary stats data
On 5/24/21 8:18 AM, Jing Zhang wrote:
Provides a file descriptor per VM to read VM stats info/data.
Provides a file descriptor per vCPU to read vCPU stats info/data.
Reviewed-by: David Matlack
Reviewed-by: Ricardo Koller
Signed-off-by: Jing Zhang
---
arch/arm64/kvm/guest.c| 26 ++
arch/mips/kvm/mips.c | 52
arch/powerpc/kvm/book3s.c | 52
arch/powerpc/kvm/booke.c | 45 +++
arch/s390/kvm/kvm-s390.c | 117 +++
arch/x86/kvm/x86.c| 53
include/linux/kvm_host.h | 132 ++
include/uapi/linux/kvm.h | 50
virt/kvm/kvm_main.c | 165 ++
9 files changed, 692 insertions(+)
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 0e41331b0911..1cc1d83630ac 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -28,6 +28,32 @@
#include "trace.h"
+struct _kvm_stats_desc kvm_vm_stats_desc[] = DEFINE_VM_STATS_DESC();
+
+struct _kvm_stats_header kvm_vm_stats_header = {
+ .name_size = KVM_STATS_NAME_LEN,
+ .count = ARRAY_SIZE(kvm_vm_stats_desc),
+ .desc_offset = sizeof(struct kvm_stats_header),
+ .data_offset = sizeof(struct kvm_stats_header) +
+ sizeof(kvm_vm_stats_desc),
+};
+
+struct _kvm_stats_desc kvm_vcpu_stats_desc[] = DEFINE_VCPU_STATS_DESC(
+ STATS_DESC_COUNTER("hvc_exit_stat"),
+ STATS_DESC_COUNTER("wfe_exit_stat"),
+ STATS_DESC_COUNTER("wfi_exit_stat"),
+ STATS_DESC_COUNTER("mmio_exit_user"),
+ STATS_DESC_COUNTER("mmio_exit_kernel"),
+ STATS_DESC_COUNTER("exits"));
+
+struct _kvm_stats_header kvm_vcpu_stats_header = {
+ .name_size = KVM_STATS_NAME_LEN,
+ .count = ARRAY_SIZE(kvm_vcpu_stats_desc),
+ .desc_offset = sizeof(struct kvm_stats_header),
+ .data_offset = sizeof(struct kvm_stats_header) +
+ sizeof(kvm_vcpu_stats_desc),
+};
+
struct kvm_stats_debugfs_item debugfs_entries[] = {
VCPU_STAT_COM("halt_successful_poll", halt_successful_poll),
VCPU_STAT_COM("halt_attempted_poll", halt_attempted_poll), diff --git a/arch/mips/kvm/mips.c
b/arch/mips/kvm/mips.c index f4fc60c05e9c..f17a65743ccd 100644 ---
a/arch/mips/kvm/mips.c +++ b/arch/mips/kvm/mips.c @@ -38,6 +38,58 @@
#define VECTORSPACING 0x100 /* for EI/VI mode */ #endif +struct
_kvm_stats_desc kvm_vm_stats_desc[] = DEFINE_VM_STATS_DESC(); +
+struct _kvm_stats_header kvm_vm_stats_header = { + .name_size =
KVM_STATS_NAME_LEN, + .count = ARRAY_SIZE(kvm_vm_stats_desc), +
.desc_offset = sizeof(struct kvm_stats_header), + .data_offset =
sizeof(struct kvm_stats_header) + + sizeof(kvm_vm_stats_desc), +}; +
+struct _kvm_stats_desc kvm_vcpu_stats_desc[] =
DEFINE_VCPU_STATS_DESC( + STATS_DESC_COUNTER("wait_exits"),
+ STATS_DESC_COUNTER("cache_exits"),
+ STATS_DESC_COUNTER("signal_exits"),
+ STATS_DESC_COUNTER("int_exits"),
+ STATS_DESC_COUNTER("cop_unusable_exits"),
+ STATS_DESC_COUNTER("tlbmod_exits"),
+ STATS_DESC_COUNTER("tlbmiss_ld_exits"),
+ STATS_DESC_COUNTER("tlbmiss_st_exits"),
+ STATS_DESC_COUNTER("addrerr_st_exits"),
+ STATS_DESC_COUNTER("addrerr_ld_exits"),
+ STATS_DESC_COUNTER("syscall_exits"),
+ STATS_DESC_COUNTER("resvd_inst_exits"),
+ STATS_DESC_COUNTER("break_inst_exits"),
+ STATS_DESC_COUNTER("trap_inst_exits"),
+ STATS_DESC_COUNTER("msa_fpe_exits"),
+ STATS_DESC_COUNTER("fpe_exits"),
+ STATS_DESC_COUNTER("msa_disabled_exits"),
+ STATS_DESC_COUNTER("flush_dcache_exits"),
+#ifdef CONFIG_KVM_MIPS_VZ
+ STATS_DESC_COUNTER("vz_gpsi_exits"),
+ STATS_DESC_COUNTER("vz_gsfc_exits"),
+ STATS_DESC_COUNTER("vz_hc_exits"),
+ STATS_DESC_COUNTER("vz_grr_exits"),
+ STATS_DESC_COUNTER("vz_gva_exits"),
+ STATS_DESC_COUNTER("vz_ghfc_exits"),
+ STATS_DESC_COUNTER("vz_gpa_exits"),
+ STATS_DESC_COUNTER("vz_resvd_exits"),
+#ifdef CONFIG_CPU_LOONGSON64
+ STATS_DESC_COUNTER("vz_cpucfg_exits"),
+#endif
+#endif
+ );
+
+struct _kvm_stats_header kvm_vcpu_stats_header = {
+ .name_size = KVM_STATS_NAME_LEN,
+ .count = ARRAY_SIZE(kvm_vcpu_stats_desc),
+ .desc_offset = sizeof(struct kvm_stats_header),
+ .data_offset = sizeof(struct kvm_stats_header) +
+ sizeof(kvm_vcpu_stats_desc),
+};
+
struct kvm_stats_debugfs_item debugfs_entries[] = {
VCPU_STAT("wait", wait_exits),
VCPU_STAT("cache", cache_exits),
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index bd3a10e1fdaf..5e8ee0d39ef9 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -38,6 +38,58 @@
/* #define EXIT_DEBUG */
+struct _kvm_stats_desc kvm_vm_stats_desc[] = DEFINE_VM_STATS_DESC(
+ STATS_DESC_ICOUNTER("num_2M_pages"),
+ STATS_DESC_ICOUNTER("num_1G_pages"));
+
+struct _kvm_stats_header kvm_vm_sta
Re: [PATCH v6 1/4] KVM: stats: Separate common stats from architecture specific ones
On 5/24/21 8:18 AM, Jing Zhang wrote:
Put all common statistics in a separate structure to ease
statistics handling for the incoming new statistics API.
No functional change intended.
Reviewed-by: David Matlack
Reviewed-by: Ricardo Koller
Signed-off-by: Jing Zhang
---
arch/arm64/include/asm/kvm_host.h | 9 ++---
arch/arm64/kvm/guest.c | 12 ++--
arch/mips/include/asm/kvm_host.h| 9 ++---
arch/mips/kvm/mips.c| 12 ++--
arch/powerpc/include/asm/kvm_host.h | 9 ++---
arch/powerpc/kvm/book3s.c | 12 ++--
arch/powerpc/kvm/book3s_hv.c| 12 ++--
arch/powerpc/kvm/book3s_pr.c| 2 +-
arch/powerpc/kvm/book3s_pr_papr.c | 2 +-
arch/powerpc/kvm/booke.c| 14 +++---
arch/s390/include/asm/kvm_host.h| 9 ++---
arch/s390/kvm/kvm-s390.c| 12 ++--
arch/x86/include/asm/kvm_host.h | 9 ++---
arch/x86/kvm/x86.c | 14 +++---
include/linux/kvm_host.h| 9 +++--
include/linux/kvm_types.h | 12
virt/kvm/kvm_main.c | 14 +++---
17 files changed, 82 insertions(+), 90 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h
b/arch/arm64/include/asm/kvm_host.h
index 7cd7d5c8c4bc..f3ad7a20b0af 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -556,16 +556,11 @@ static inline bool __vcpu_write_sys_reg_to_cpu(u64 val,
int reg)
}
struct kvm_vm_stat {
- ulong remote_tlb_flush;
+ struct kvm_vm_stat_common common;
};
struct kvm_vcpu_stat {
- u64 halt_successful_poll;
- u64 halt_attempted_poll;
- u64 halt_poll_success_ns;
- u64 halt_poll_fail_ns;
- u64 halt_poll_invalid;
- u64 halt_wakeup;
+ struct kvm_vcpu_stat_common common;
u64 hvc_exit_stat;
u64 wfe_exit_stat;
u64 wfi_exit_stat;
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 5cb4a1cd5603..0e41331b0911 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -29,18 +29,18 @@
#include "trace.h"
struct kvm_stats_debugfs_item debugfs_entries[] = {
- VCPU_STAT("halt_successful_poll", halt_successful_poll),
- VCPU_STAT("halt_attempted_poll", halt_attempted_poll),
- VCPU_STAT("halt_poll_invalid", halt_poll_invalid),
- VCPU_STAT("halt_wakeup", halt_wakeup),
+ VCPU_STAT_COM("halt_successful_poll", halt_successful_poll),
+ VCPU_STAT_COM("halt_attempted_poll", halt_attempted_poll),
+ VCPU_STAT_COM("halt_poll_invalid", halt_poll_invalid),
+ VCPU_STAT_COM("halt_wakeup", halt_wakeup),
VCPU_STAT("hvc_exit_stat", hvc_exit_stat),
VCPU_STAT("wfe_exit_stat", wfe_exit_stat),
VCPU_STAT("wfi_exit_stat", wfi_exit_stat),
VCPU_STAT("mmio_exit_user", mmio_exit_user),
VCPU_STAT("mmio_exit_kernel", mmio_exit_kernel),
VCPU_STAT("exits", exits),
- VCPU_STAT("halt_poll_success_ns", halt_poll_success_ns),
- VCPU_STAT("halt_poll_fail_ns", halt_poll_fail_ns),
+ VCPU_STAT_COM("halt_poll_success_ns", halt_poll_success_ns),
+ VCPU_STAT_COM("halt_poll_fail_ns", halt_poll_fail_ns),
{ NULL }
};
diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index fca4547d580f..6f610fbcd8d1 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -109,10 +109,11 @@ static inline bool kvm_is_error_hva(unsigned long addr)
}
struct kvm_vm_stat {
- ulong remote_tlb_flush;
+ struct kvm_vm_stat_common common;
};
struct kvm_vcpu_stat {
+ struct kvm_vcpu_stat_common common;
u64 wait_exits;
u64 cache_exits;
u64 signal_exits;
@@ -142,12 +143,6 @@ struct kvm_vcpu_stat {
#ifdef CONFIG_CPU_LOONGSON64
u64 vz_cpucfg_exits;
#endif
- u64 halt_successful_poll;
- u64 halt_attempted_poll;
- u64 halt_poll_success_ns;
- u64 halt_poll_fail_ns;
- u64 halt_poll_invalid;
- u64 halt_wakeup;
};
struct kvm_arch_memory_slot {
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 4d4af97dcc88..f4fc60c05e9c 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -68,12 +68,12 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
#ifdef CONFIG_CPU_LOONGSON64
VCPU_STAT("vz_cpucfg", vz_cpucfg_exits),
#endif
- VCPU_STAT("halt_successful_poll", halt_successful_poll),
- VCPU_STAT("halt_attempted_poll", halt_attempted_poll),
- VCPU_STAT("halt_poll_invalid", halt_poll_invalid),
- VCPU_STAT("halt_wakeup", halt_wakeup),
- VCPU_STAT("halt_poll_success_ns", halt_poll_success_ns),
- VCPU_STAT("halt_poll_fail_ns", halt_poll_fail_ns),
+ VCPU_STAT_COM("halt_successful_poll", halt_successful_poll),
+ VCPU_STAT_COM("halt_attempted_poll", halt_attempted_poll)
Re: [PATCH v5 1/4] KVM: stats: Separate common stats from architecture specific ones
On 5/18/21 10:25 AM, Jing Zhang wrote:
Hi David,
On Tue, May 18, 2021 at 11:27 AM David Matlack wrote:
On Mon, May 17, 2021 at 5:10 PM Jing Zhang wrote:
Actually the definition of kvm_{vcpu,vm}_stat are arch specific. There is
no real structure for arch agnostic stats. Most of the stats in common
structures are arch agnostic, but not all of them.
There are some benefits to put all common stats in a separate structure.
e.g. if we want to add a stat in kvm_main.c, we only need to add this stat
in the common structure, don't have to update all kvm_{vcpu,vm}_stat
definition for all architectures.
I meant rename the existing arch-specific struct kvm_{vcpu,vm}_stat to
kvm_{vcpu,vm}_stat_arch and rename struct kvm_{vcpu,vm}_stat_common to
kvm_{vcpu,vm}_stat.
So in include/linux/kvm_types.h you'd have:
struct kvm_vm_stat {
ulong remote_tlb_flush;
struct kvm_vm_stat_arch arch;
};
struct kvm_vcpu_stat {
u64 halt_successful_poll;
u64 halt_attempted_poll;
u64 halt_poll_invalid;
u64 halt_wakeup;
u64 halt_poll_success_ns;
u64 halt_poll_fail_ns;
struct kvm_vcpu_stat_arch arch;
};
And in arch/x86/include/asm/kvm_host.h you'd have:
struct kvm_vm_stat_arch {
ulong mmu_shadow_zapped;
...
};
struct kvm_vcpu_stat_arch {
u64 pf_fixed;
u64 pf_guest;
u64 tlb_flush;
...
};
You still have the same benefits of having an arch-neutral place to
store stats but the struct layout more closely resembles struct
kvm_vcpu and struct kvm.
You are right. This is a more reasonable way to layout the structures.
I remember that I didn't choose this way is only because that it needs
touching every arch specific stats in all architectures (stat.name ->
stat.arch.name) instead of only touching arch neutral stats.
Let's see if there is any vote from others about this.
+1
Thanks,
Jing
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
