Re: [RFC PATCH] KVM: Only register preempt notifiers and load arch cpu state as needed

Paolo Bonzini
2017-11-23T16:17:00Z

On 23/11/2017 17:05, Christoffer Dall wrote:
> For example,
> arm64 is about to do significant work in vcpu load/put when running a
> vcpu, but not when doing things like KVM_SET_ONE_REG or
> KVM_SET_MP_STATE.

Out of curiosity, in what circumstances are these ioctls a hot path?
Especially KVM_SET_MP_STATE.

> Hi all,
>
> Drew suggested this as an alternative approach to recording the ioctl
> number on the vcpu struct [1] as it may benefit other architectures in
> general.
>
> I had a look at some of the specific ioctls across architectures, but
> must admit that I can't easily tell which architecture specific logic
> relies on having registered preempt notifiers and having called the
> architecture specific load function.
>
> It would be great if you would let me know if you think this is
> generally useful or if you prefer the less invasive approach, and in
> case this is useful, if you could have a look at all the vcpu ioctls for
> your architecture and let me know if I am being too loose or too
> careful in calling __vcpu_load() in this patch.

I can suggest a third approach:

if (ioctl == KVM_GET_ONE_REG || ioctl == KVM_SET_ONE_REG)
return kvm_arch_vcpu_ioctl(filp, ioctl, arg);

in kvm_vcpu_ioctl before "r = vcpu_load(vcpu);", or even better:

if (ioctl == KVM_GET_ONE_REG)
// call kvm_arch_vcpu_get_one_reg_ioctl(vcpu, &reg);
// and do copy_to_user
return kvm_vcpu_get_one_reg_ioctl(vcpu, arg);
if (ioctl == KVM_SET_ONE_REG)
// do copy_from_user then call
// kvm_arch_vcpu_set_one_reg_ioctl(vcpu, &reg);
return kvm_vcpu_set_one_reg_ioctl(vcpu, arg);

so that the kvm_arch_vcpu_get/set_one_reg_ioctl functions are called
without the lock.

Then all architectures except ARM can be switched to do
vcpu_load/vcpu_put in kvm_arch_vcpu_get/set_one_reg_ioctl

Paolo