On Wed, 10 Mar 2021 00:30:23 +0000, Jing Zhang jingzhangos@google.com wrote:
Three ioctl commands are added to support binary form statistics data retrieval. KVM_STATS_GET_INFO, KVM_STATS_GET_NAMES, KVM_STATS_GET_DATA. KVM_CAP_STATS_BINARY_FORM indicates the capability.
Signed-off-by: Jing Zhang jingzhangos@google.com
virt/kvm/kvm_main.c | 115 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 383df23514b9..87dd62516c8b 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3464,6 +3464,51 @@ static long kvm_vcpu_ioctl(struct file *filp, r = kvm_arch_vcpu_ioctl_set_fpu(vcpu, fpu); break; }
- case KVM_STATS_GET_INFO: {
struct kvm_stats_info stats_info;r = -EFAULT;stats_info.num_stats = VCPU_STAT_COUNT;if (copy_to_user(argp, &stats_info, sizeof(stats_info)))goto out;r = 0;break;- }
- case KVM_STATS_GET_NAMES: {
struct kvm_stats_names stats_names;r = -EFAULT;if (copy_from_user(&stats_names, argp, sizeof(stats_names)))goto out;r = -EINVAL;if (stats_names.size < VCPU_STAT_COUNT * KVM_STATS_NAME_LEN)goto out;r = -EFAULT;if (copy_to_user(argp + sizeof(stats_names),kvm_vcpu_stat_strings,VCPU_STAT_COUNT * KVM_STATS_NAME_LEN))goto out;r = 0;break;- }
- case KVM_STATS_GET_DATA: {
struct kvm_stats_data stats_data;r = -EFAULT;if (copy_from_user(&stats_data, argp, sizeof(stats_data)))goto out;r = -EINVAL;if (stats_data.size < sizeof(vcpu->stat))goto out;r = -EFAULT;argp += sizeof(stats_data);if (copy_to_user(argp, &vcpu->stat, sizeof(vcpu->stat)))goto out;r = 0;break;- } default: r = kvm_arch_vcpu_ioctl(filp, ioctl, arg); }
@@ -3695,6 +3740,7 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg) case KVM_CAP_CHECK_EXTENSION_VM: case KVM_CAP_ENABLE_CAP_VM: case KVM_CAP_HALT_POLL:
- case KVM_CAP_STATS_BINARY_FORM: return 1;
#ifdef CONFIG_KVM_MMIO case KVM_CAP_COALESCED_MMIO: @@ -3825,6 +3871,40 @@ static int kvm_vm_ioctl_enable_cap_generic(struct kvm *kvm, } } +static long kvm_vm_ioctl_stats_get_data(struct kvm *kvm, unsigned long arg) +{
- void __user *argp = (void __user *)arg;
- struct kvm_vcpu *vcpu;
- struct kvm_stats_data stats_data;
- u64 *data = NULL, *pdata;
- int i, j, ret = 0;
- size_t dsize = (VM_STAT_COUNT + VCPU_STAT_COUNT) * sizeof(*data);
- if (copy_from_user(&stats_data, argp, sizeof(stats_data)))
return -EFAULT;- if (stats_data.size < dsize)
return -EINVAL;- data = kzalloc(dsize, GFP_KERNEL_ACCOUNT);
- if (!data)
return -ENOMEM;- for (i = 0; i < VM_STAT_COUNT; i++)
*(data + i) = *((ulong *)&kvm->stat + i);
This kind of dance could be avoided if your stats were just an array, or a union of the current data structure and an array.
- kvm_for_each_vcpu(j, vcpu, kvm) {
pdata = data + VM_STAT_COUNT;for (i = 0; i < VCPU_STAT_COUNT; i++, pdata++)*pdata += *((u64 *)&vcpu->stat + i);
Do you really need the in-kernel copy? Why not directly organise the data structures in a way that would allow a bulk copy using copy_to_user()?
Another thing is the atomicity of what you are reporting. Don't you care about the consistency of the counters?
Thanks,
M.