On 2017/8/7 16:55, David Hildenbrand wrote:
> On 07.08.2017 10:44, Longpeng(Mike) wrote:
>> If the vcpu(me) exit due to request a usermode spinlock, then
>> the spinlock-holder may be preempted in usermode or kernmode.
>>
>> But if the vcpu(me) is in kernmode, then the holder must be
>> preempted in kernmode, so we should choose a vcpu in kernmode
>> as the most eligible candidate.
>>
>> For some architecture(e.g. arm/s390), spin/preempt_in_kernel()
>> are the same, but they are different for X86.
>>
>> Signed-off-by: Longpeng(Mike) <longpeng2@xxxxxxxxxx>
>> ---
>> arch/mips/kvm/mips.c | 10 ++++++++++
>> arch/powerpc/kvm/powerpc.c | 10 ++++++++++
>> arch/s390/kvm/kvm-s390.c | 10 ++++++++++
>> arch/x86/kvm/x86.c | 10 ++++++++++
>> include/linux/kvm_host.h | 2 ++
>> virt/kvm/arm/arm.c | 10 ++++++++++
>> virt/kvm/kvm_main.c | 4 ++++
>> 7 files changed, 56 insertions(+)
>>
>> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
>> index d4b2ad1..e04e6b3 100644
>> --- a/arch/mips/kvm/mips.c
>> +++ b/arch/mips/kvm/mips.c
>> @@ -98,6 +98,16 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
>> return !!(vcpu->arch.pending_exceptions);
>> }
>>
>> +bool kvm_arch_vcpu_spin_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> + return false;
>> +}
>> +
>> +bool kvm_arch_vcpu_preempt_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> + return false;
>> +}
>> +
>> int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>> {
>> return 1;
>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> index 1a75c0b..c573ddd 100644
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -58,6 +58,16 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
>> return !!(v->arch.pending_exceptions) || kvm_request_pending(v);
>> }
>>
>> +bool kvm_arch_vcpu_spin_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> + return false;
>> +}
>> +
>> +bool kvm_arch_vcpu_preempt_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> + return false;
>> +}
>> +
>> int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>> {
>> return 1;
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index af09d34..f78cdc2 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2447,6 +2447,16 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
>> return kvm_s390_vcpu_has_irq(vcpu, 0);
>> }
>>
>> +bool kvm_arch_vcpu_spin_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> + return false;
>> +}
>> +
>> +bool kvm_arch_vcpu_preempt_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> + return false;
>> +}
>> +
>> void kvm_s390_vcpu_block(struct kvm_vcpu *vcpu)
>> {
>> atomic_or(PROG_BLOCK_SIE, &vcpu->arch.sie_block->prog20);
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 6c97c82..04c6a1f 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -8435,6 +8435,16 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
>> return kvm_vcpu_running(vcpu) || kvm_vcpu_has_events(vcpu);
>> }
>>
>> +bool kvm_arch_vcpu_spin_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> + return false;
>> +}
>> +
>> +bool kvm_arch_vcpu_preempt_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> + return false;
>> +}
>> +
>> int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>> {
>> return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index 890b706..9613620 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -798,6 +798,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>> void kvm_arch_hardware_unsetup(void);
>> void kvm_arch_check_processor_compat(void *rtn);
>> int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu);
>> +bool kvm_arch_vcpu_spin_in_kernel(struct kvm_vcpu *vcpu);
>> +bool kvm_arch_vcpu_preempt_in_kernel(struct kvm_vcpu *vcpu);
>> int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);
>>
>> #ifndef __KVM_HAVE_ARCH_VM_ALLOC
>> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
>> index a39a1e1..e45f780 100644
>> --- a/virt/kvm/arm/arm.c
>> +++ b/virt/kvm/arm/arm.c
>> @@ -416,6 +416,16 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
>> && !v->arch.power_off && !v->arch.pause);
>> }
>>
>> +bool kvm_arch_vcpu_spin_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> + return false;
>> +}
>> +
>> +bool kvm_arch_vcpu_preempt_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> + return false;
>> +}
>
> Is the differentiation really necessary?
>
> Can't you cache for x86 in all scenarios and simply introduce
> kvm_arch_vcpu_in_kernel() ?
>
For X86 this is necessary, I have no idea how to avoid this, hopes
someone could give me some suggestion. :)
> Otherwise, we have complexity that might just be avoided (e.g.
> kvm_arch_vcpu_spin_in_kernel must only be called on the loaded VCPU)
>
>> +
>> /* Just ensure a guest exit from a particular CPU */
>> static void exit_vm_noop(void *info)
>> {
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index f3f7427..0d0527b 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -2324,12 +2324,14 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>> {
>> struct kvm *kvm = me->kvm;
>> struct kvm_vcpu *vcpu;
>> + bool in_kern;
>> int last_boosted_vcpu = me->kvm->last_boosted_vcpu;
>> int yielded = 0;
>> int try = 3;
>> int pass;
>> int i;
>>
>> + in_kern = kvm_arch_vcpu_spin_in_kernel(me);
>> kvm_vcpu_set_in_spin_loop(me, true);
>> /*
>> * We boost the priority of a VCPU that is runnable but not
>> @@ -2351,6 +2353,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>> continue;
>> if (swait_active(&vcpu->wq) && !kvm_arch_vcpu_runnable(vcpu))
>> continue;
>> + if (in_kern && !kvm_arch_vcpu_preempt_in_kernel(vcpu))
>> + continue;
>> if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
>> continue;
>>
>>
>
>
--
Regards,
Longpeng(Mike)
