| Message ID | 20170202174921.902084298@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 { > >
On Fri, Feb 03, 2017 at 06:03:37PM +0100, Radim Krcmar wrote: > 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.) Not sure. What if you change the role of vcpus and now want to change vcpu-1 from a realtime vcpu (one where only DPDK runs) to a multi-user vcpu without a reboot? > > + 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. Fixed. > > + 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.) Fixed. > > + 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. Fixed.
2017-02-22 18:18-0300, Marcelo Tosatti: > On Fri, Feb 03, 2017 at 06:03:37PM +0100, Radim Krcmar wrote: >> 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.) > > Not sure. What if you change the role of vcpus and now want > to change vcpu-1 from a realtime vcpu (one where only DPDK runs) > to a multi-user vcpu without a reboot? If we want to be dynamic, I'd rather have it as a hypercall that toggles the permissions for frequency change under CPL > 0. This would require guest kernel changes, though. As a benefit, we could enable the capability by default for all VCPUs and just let the guest kernel control which task can change frequency without exits into userspace. Having QEMU toggle the whole feature would require some QEMU emulated device, unless we really want to do it manually on both sides. Doing it manually doesn't sound useful outside of testing ... is DPDK actually being used "dynamically"? (I thought that the setup is decided when the host boots.) Thanks.
On 23/02/2017 17:48, Radim Krcmar wrote: > Doing it manually doesn't sound useful outside of testing ... > is DPDK actually being used "dynamically"? > (I thought that the setup is decided when the host boots.) Or at the very least when the guest boots. I agree that KVM_ENABLE_CAP is enough. However, changing KVM_CPUID_FEATURES should be handled by userspace (KVM doesn't know about 0x400000xx leaves at all).
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: { + 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; + r = 0; + break; + } + case KVM_GET_VCPU_ALLOW_FREQ_HC: { + 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: + 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 {
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(+)