Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1586496 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [patch 2/3] KVM: x86: introduce ioctl to allow frequency  hypercalls Marcelo Tosatti <mtosatti@redhat.com> - 2017-02-22 22:20 +0100
    Re: [patch 2/3] KVM: x86: introduce ioctl to allow frequency  hypercalls Radim Krcmar <rkrcmar@redhat.com> - 2017-02-23 18:00 +0100
      Re: [patch 2/3] KVM: x86: introduce ioctl to allow frequency  hypercalls Paolo Bonzini <pbonzini@redhat.com> - 2017-02-23 18:40 +0100

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

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.

[toc] | [next] | [standalone]

#1587018

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.

[toc] | [prev] | [next] | [standalone]

#1587043

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).

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web