Re: [patch 2/3] KVM: x86: introduce ioctl to allow frequency hypercalls

Radim Krcmar
2017-02-03T17:03:41Z

2017-02-02 15:47-0200, Marcelo Tosatti:
> For most VMs, modifying the host frequency is an undesired
> operation. Introduce ioctl to enable the guest to
> modify host CPU frequency.
>
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> ---
> arch/x86/include/asm/kvm_host.h | 2 ++
> arch/x86/include/uapi/asm/kvm.h | 5 +++++
> arch/x86/kvm/x86.c | 20 ++++++++++++++++++++
> include/uapi/linux/kvm.h | 3 +++
> virt/kvm/kvm_main.c | 2 ++
> 5 files changed, 32 insertions(+)
>
> Index: kvm-pvfreq/arch/x86/kvm/x86.c
> ===================================================================
> --- kvm-pvfreq.orig/arch/x86/kvm/x86.c 2017-01-31 10:32:33.023378783 -0200
> +++ kvm-pvfreq/arch/x86/kvm/x86.c 2017-01-31 10:34:25.443618639 -0200
> @@ -3665,6 +3665,26 @@
> r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
> break;
> }
> + case KVM_SET_VCPU_ALLOW_FREQ_HC: {

Just enable the frequency hypercalls with KVM_ENABLE_CAP ioctl and get
rid of this ioctl.
(I don't think that we want to allow disabling this capability.)

> + struct kvm_vcpu_allow_freq freq;
> +
> + r = -EFAULT;
> + if (copy_from_user(&freq, argp, sizeof(freq)))
> + goto out;
> + vcpu->arch.allow_freq_hypercall = freq.enable;

Enabling the capability should also set a bit in KVM_CPUID_FEATURES,
a well-behaved guest won't use it otherwise.

> + r = 0;
> + break;
> + }
> + case KVM_GET_VCPU_ALLOW_FREQ_HC: {

And this ioctl has no use, so it should be omitted.

(Userspace doesn't learn anything it doesn't already know.
The feature is unsafe, so the default must be 0.)

> + struct kvm_vcpu_allow_freq freq;
> +
> + memset(&freq, 0, sizeof(struct kvm_vcpu_allow_freq));
> + r = -EFAULT;
> + if (copy_to_user(&freq, argp, sizeof(freq)))
> + break;
> + r = 0;
> + break;
> + }
> default:
> r = -EINVAL;
> }
> Index: kvm-pvfreq/include/uapi/linux/kvm.h
> ===================================================================
> --- kvm-pvfreq.orig/include/uapi/linux/kvm.h 2017-01-31 10:32:33.023378783 -0200
> +++ kvm-pvfreq/include/uapi/linux/kvm.h 2017-01-31 10:32:38.000389402 -0200
> @@ -871,6 +871,7 @@
> #define KVM_CAP_S390_USER_INSTR0 130
> #define KVM_CAP_MSI_DEVID 131
> #define KVM_CAP_PPC_HTM 132
> +#define KVM_CAP_ALLOW_FREQ_HC 133
>
> #ifdef KVM_CAP_IRQ_ROUTING
>
> @@ -1281,6 +1282,8 @@
> #define KVM_S390_GET_IRQ_STATE _IOW(KVMIO, 0xb6, struct kvm_s390_irq_state)
> /* Available with KVM_CAP_X86_SMM */
> #define KVM_SMI _IO(KVMIO, 0xb7)
> +#define KVM_SET_VCPU_ALLOW_FREQ_HC _IO(KVMIO, 0xb8)
> +#define KVM_GET_VCPU_ALLOW_FREQ_HC _IO(KVMIO, 0xb9)
>
> #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
> #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
> Index: kvm-pvfreq/arch/x86/include/uapi/asm/kvm.h
> ===================================================================
> --- kvm-pvfreq.orig/arch/x86/include/uapi/asm/kvm.h 2017-01-31 10:32:33.023378783 -0200
> +++ kvm-pvfreq/arch/x86/include/uapi/asm/kvm.h 2017-01-31 10:32:38.000389402 -0200
> @@ -357,4 +357,9 @@
> #define KVM_X86_QUIRK_LINT0_REENABLED (1 << 0)
> #define KVM_X86_QUIRK_CD_NW_CLEARED (1 << 1)
>
> +struct kvm_vcpu_allow_freq {
> + __u16 enable;
> + __u16 pad[7];
> +};
> +
> #endif /* _ASM_X86_KVM_H */
> Index: kvm-pvfreq/virt/kvm/kvm_main.c
> ===================================================================
> --- kvm-pvfreq.orig/virt/kvm/kvm_main.c 2017-01-31 10:32:33.023378783 -0200
> +++ kvm-pvfreq/virt/kvm/kvm_main.c 2017-01-31 10:32:38.001389404 -0200
> @@ -2938,6 +2938,8 @@
> #endif
> case KVM_CAP_MAX_VCPU_ID:
> return KVM_MAX_VCPU_ID;
> + case KVM_CAP_ALLOW_FREQ_HC:

This can share an existing return 1 and would be elsewhere with
KVM_ENABLE_CAP.

> + return 1;
> default:
> break;
> }
> Index: kvm-pvfreq/arch/x86/include/asm/kvm_host.h
> ===================================================================
> --- kvm-pvfreq.orig/arch/x86/include/asm/kvm_host.h 2017-01-31 10:32:33.023378783 -0200
> +++ kvm-pvfreq/arch/x86/include/asm/kvm_host.h 2017-01-31 10:32:38.001389404 -0200
> @@ -678,6 +678,8 @@
>
> /* GPA available (AMD only) */
> bool gpa_available;
> +
> + bool allow_freq_hypercall;
> };
>
> struct kvm_lpage_info {
>
>