Re: [PATCH v2 1/4] KVM: add spinlock optimization framework

[Longpeng (Mike)]

On 2017/8/8 17:00, Paolo Bonzini wrote:

> On 08/08/2017 10:42, David Hildenbrand wrote:
>>
>>> +bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
>>> +{
>>> +   return false;
>>> +}
>>
>> why don't we need an EXPORT_SYMBOL here?
> 
> Is it used outside the KVM module?  I think no architecture actually needs
> to export it.
> 


Hi Paolo & David,

In my original approach, I call kvm_arch_vcpu_in_kernel() in handle_pause(),
without EXPORT_SYMBOL the compiler reports:
 ERROR: "kvm_arch_vcpu_in_kernel" [arch/x86/kvm/kvm-intel.ko] undefined!
 ERROR: "kvm_arch_vcpu_in_kernel" [arch/x86/kvm/kvm-amd.ko] undefined!

But Paolo's approach is significantly better, it's a work of art, thanks a lot.

-- 
Regards,
Longpeng(Mike)

>>> -void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>>> +void kvm_vcpu_on_spin(struct kvm_vcpu *me, bool me_in_kern)
>>>  {
>>> struct kvm *kvm = me->kvm;
>>> struct kvm_vcpu *vcpu;
>>> @@ -2348,6 +2348,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>>> continue;
>>> if (swait_active(>wq) && 
>>> !kvm_arch_vcpu_runnable(vcpu))
>>> continue;
>>> +   if (me_in_kern && !kvm_arch_vcpu_in_kernel(vcpu))
>>> +   continue;
>>
>>
>> hm, does this patch compile? (me_in_kern)
> 
> Why not? :)  This is what I have:
> 
>>From d62a40d49f44ff7e789a15416316ef1cba93fa85 Mon Sep 17 00:00:00 2001
> From: "Longpeng(Mike)" 
> Date: Tue, 8 Aug 2017 12:05:32 +0800
> Subject: [PATCH 1/4] KVM: add spinlock optimization framework
> 
> If a vcpu exits due to request a user mode spinlock, then
> the spinlock-holder may be preempted in user mode or kernel mode.
> (Note that not all architectures trap spin loops in user mode,
> only AMD x86 and ARM/ARM64 currently do).
> 
> But if a vcpu exits in kernel mode, then the holder must be
> preempted in kernel mode, so we should choose a vcpu in kernel mode
> as a more likely candidate for the lock holder.
> 
> This introduces kvm_arch_vcpu_in_kernel() to decide whether the
> vcpu is in kernel-mode when it's preempted.  kvm_vcpu_on_spin's
> new argument says the same of the spinning VCPU.
> 
> Signed-off-by: Longpeng(Mike) 
> Signed-off-by: Paolo Bonzini 
> ---
>  arch/arm/kvm/handle_exit.c   | 2 +-
>  arch/arm64/kvm/handle_exit.c | 2 +-
>  arch/mips/kvm/mips.c | 5 +
>  arch/powerpc/kvm/powerpc.c   | 5 +
>  arch/s390/kvm/diag.c | 2 +-
>  arch/s390/kvm/kvm-s390.c | 5 +
>  arch/x86/kvm/hyperv.c| 2 +-
>  arch/x86/kvm/svm.c   | 2 +-
>  arch/x86/kvm/vmx.c   | 2 +-
>  arch/x86/kvm/x86.c   | 5 +
>  include/linux/kvm_host.h | 3 ++-
>  virt/kvm/arm/arm.c   | 5 +
>  virt/kvm/kvm_main.c  | 4 +++-
>  13 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
> index 54442e375354..196122bb6968 100644
> --- a/arch/arm/kvm/handle_exit.c
> +++ b/arch/arm/kvm/handle_exit.c
> @@ -67,7 +67,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct 
> kvm_run *run)
>   if (kvm_vcpu_get_hsr(vcpu) & HSR_WFI_IS_WFE) {
>   trace_kvm_wfx(*vcpu_pc(vcpu), true);
>   vcpu->stat.wfe_exit_stat++;
> - kvm_vcpu_on_spin(vcpu);
> + kvm_vcpu_on_spin(vcpu, false);
>   } else {
>   trace_kvm_wfx(*vcpu_pc(vcpu), false);
>   vcpu->stat.wfi_exit_stat++;
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 17d8a1677a0b..da57622cacca 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -84,7 +84,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct 
> kvm_run *run)
>   if (kvm_vcpu_get_hsr(vcpu) & ESR_ELx_WFx_ISS_WFE) {
>   trace_kvm_wfx_arm64(*vcpu_pc(vcpu), true);
>   vcpu->stat.wfe_exit_stat++;
> - kvm_vcpu_on_spin(vcpu);
> + kvm_vcpu_on_spin(vcpu, false);
>   } else {
>   trace_kvm_wfx_arm64(*vcpu_pc(vcpu), false);
>   vcpu->stat.wfi_exit_stat++;
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index d4b2ad18eef2..70208bed5a15 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -98,6 +98,11 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
>   return !!(vcpu->arch.pending_exceptions);
>  }
>  
> +bool kvm_arch_vcpu_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 1a75c0b5f4ca..6184c45015f3 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -58,6 +58,11 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
>   return !!(v->arch.pending_exceptions) || kvm_request_pending(v);
>  }
>  
> +bool 

Re: [PATCH v2 1/4] KVM: add spinlock optimization framework

[Longpeng (Mike)]

On 2017/8/8 17:00, Paolo Bonzini wrote:

> On 08/08/2017 10:42, David Hildenbrand wrote:
>>
>>> +bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
>>> +{
>>> +   return false;
>>> +}
>>
>> why don't we need an EXPORT_SYMBOL here?
> 
> Is it used outside the KVM module?  I think no architecture actually needs
> to export it.
> 


Hi Paolo & David,

In my original approach, I call kvm_arch_vcpu_in_kernel() in handle_pause(),
without EXPORT_SYMBOL the compiler reports:
 ERROR: "kvm_arch_vcpu_in_kernel" [arch/x86/kvm/kvm-intel.ko] undefined!
 ERROR: "kvm_arch_vcpu_in_kernel" [arch/x86/kvm/kvm-amd.ko] undefined!

But Paolo's approach is significantly better, it's a work of art, thanks a lot.

-- 
Regards,
Longpeng(Mike)

>>> -void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>>> +void kvm_vcpu_on_spin(struct kvm_vcpu *me, bool me_in_kern)
>>>  {
>>> struct kvm *kvm = me->kvm;
>>> struct kvm_vcpu *vcpu;
>>> @@ -2348,6 +2348,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>>> continue;
>>> if (swait_active(>wq) && 
>>> !kvm_arch_vcpu_runnable(vcpu))
>>> continue;
>>> +   if (me_in_kern && !kvm_arch_vcpu_in_kernel(vcpu))
>>> +   continue;
>>
>>
>> hm, does this patch compile? (me_in_kern)
> 
> Why not? :)  This is what I have:
> 
>>From d62a40d49f44ff7e789a15416316ef1cba93fa85 Mon Sep 17 00:00:00 2001
> From: "Longpeng(Mike)" 
> Date: Tue, 8 Aug 2017 12:05:32 +0800
> Subject: [PATCH 1/4] KVM: add spinlock optimization framework
> 
> If a vcpu exits due to request a user mode spinlock, then
> the spinlock-holder may be preempted in user mode or kernel mode.
> (Note that not all architectures trap spin loops in user mode,
> only AMD x86 and ARM/ARM64 currently do).
> 
> But if a vcpu exits in kernel mode, then the holder must be
> preempted in kernel mode, so we should choose a vcpu in kernel mode
> as a more likely candidate for the lock holder.
> 
> This introduces kvm_arch_vcpu_in_kernel() to decide whether the
> vcpu is in kernel-mode when it's preempted.  kvm_vcpu_on_spin's
> new argument says the same of the spinning VCPU.
> 
> Signed-off-by: Longpeng(Mike) 
> Signed-off-by: Paolo Bonzini 
> ---
>  arch/arm/kvm/handle_exit.c   | 2 +-
>  arch/arm64/kvm/handle_exit.c | 2 +-
>  arch/mips/kvm/mips.c | 5 +
>  arch/powerpc/kvm/powerpc.c   | 5 +
>  arch/s390/kvm/diag.c | 2 +-
>  arch/s390/kvm/kvm-s390.c | 5 +
>  arch/x86/kvm/hyperv.c| 2 +-
>  arch/x86/kvm/svm.c   | 2 +-
>  arch/x86/kvm/vmx.c   | 2 +-
>  arch/x86/kvm/x86.c   | 5 +
>  include/linux/kvm_host.h | 3 ++-
>  virt/kvm/arm/arm.c   | 5 +
>  virt/kvm/kvm_main.c  | 4 +++-
>  13 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
> index 54442e375354..196122bb6968 100644
> --- a/arch/arm/kvm/handle_exit.c
> +++ b/arch/arm/kvm/handle_exit.c
> @@ -67,7 +67,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct 
> kvm_run *run)
>   if (kvm_vcpu_get_hsr(vcpu) & HSR_WFI_IS_WFE) {
>   trace_kvm_wfx(*vcpu_pc(vcpu), true);
>   vcpu->stat.wfe_exit_stat++;
> - kvm_vcpu_on_spin(vcpu);
> + kvm_vcpu_on_spin(vcpu, false);
>   } else {
>   trace_kvm_wfx(*vcpu_pc(vcpu), false);
>   vcpu->stat.wfi_exit_stat++;
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 17d8a1677a0b..da57622cacca 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -84,7 +84,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct 
> kvm_run *run)
>   if (kvm_vcpu_get_hsr(vcpu) & ESR_ELx_WFx_ISS_WFE) {
>   trace_kvm_wfx_arm64(*vcpu_pc(vcpu), true);
>   vcpu->stat.wfe_exit_stat++;
> - kvm_vcpu_on_spin(vcpu);
> + kvm_vcpu_on_spin(vcpu, false);
>   } else {
>   trace_kvm_wfx_arm64(*vcpu_pc(vcpu), false);
>   vcpu->stat.wfi_exit_stat++;
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index d4b2ad18eef2..70208bed5a15 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -98,6 +98,11 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
>   return !!(vcpu->arch.pending_exceptions);
>  }
>  
> +bool kvm_arch_vcpu_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 1a75c0b5f4ca..6184c45015f3 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -58,6 +58,11 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
>   return !!(v->arch.pending_exceptions) || kvm_request_pending(v);
>  }
>  
> +bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> 

Re: [PATCH v2 1/4] KVM: add spinlock optimization framework

[Paolo Bonzini]

On 08/08/2017 10:42, David Hildenbrand wrote:
> 
>> +bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> +return false;
>> +}
> 
> why don't we need an EXPORT_SYMBOL here?

Is it used outside the KVM module?  I think no architecture actually needs
to export it.

>> -void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>> +void kvm_vcpu_on_spin(struct kvm_vcpu *me, bool me_in_kern)
>>  {
>>  struct kvm *kvm = me->kvm;
>>  struct kvm_vcpu *vcpu;
>> @@ -2348,6 +2348,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>>  continue;
>>  if (swait_active(>wq) && 
>> !kvm_arch_vcpu_runnable(vcpu))
>>  continue;
>> +if (me_in_kern && !kvm_arch_vcpu_in_kernel(vcpu))
>> +continue;
> 
> 
> hm, does this patch compile? (me_in_kern)

Why not? :)  This is what I have:

>From d62a40d49f44ff7e789a15416316ef1cba93fa85 Mon Sep 17 00:00:00 2001
From: "Longpeng(Mike)" 
Date: Tue, 8 Aug 2017 12:05:32 +0800
Subject: [PATCH 1/4] KVM: add spinlock optimization framework

If a vcpu exits due to request a user mode spinlock, then
the spinlock-holder may be preempted in user mode or kernel mode.
(Note that not all architectures trap spin loops in user mode,
only AMD x86 and ARM/ARM64 currently do).

But if a vcpu exits in kernel mode, then the holder must be
preempted in kernel mode, so we should choose a vcpu in kernel mode
as a more likely candidate for the lock holder.

This introduces kvm_arch_vcpu_in_kernel() to decide whether the
vcpu is in kernel-mode when it's preempted.  kvm_vcpu_on_spin's
new argument says the same of the spinning VCPU.

Signed-off-by: Longpeng(Mike) 
Signed-off-by: Paolo Bonzini 
---
 arch/arm/kvm/handle_exit.c   | 2 +-
 arch/arm64/kvm/handle_exit.c | 2 +-
 arch/mips/kvm/mips.c | 5 +
 arch/powerpc/kvm/powerpc.c   | 5 +
 arch/s390/kvm/diag.c | 2 +-
 arch/s390/kvm/kvm-s390.c | 5 +
 arch/x86/kvm/hyperv.c| 2 +-
 arch/x86/kvm/svm.c   | 2 +-
 arch/x86/kvm/vmx.c   | 2 +-
 arch/x86/kvm/x86.c   | 5 +
 include/linux/kvm_host.h | 3 ++-
 virt/kvm/arm/arm.c   | 5 +
 virt/kvm/kvm_main.c  | 4 +++-
 13 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
index 54442e375354..196122bb6968 100644
--- a/arch/arm/kvm/handle_exit.c
+++ b/arch/arm/kvm/handle_exit.c
@@ -67,7 +67,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct 
kvm_run *run)
if (kvm_vcpu_get_hsr(vcpu) & HSR_WFI_IS_WFE) {
trace_kvm_wfx(*vcpu_pc(vcpu), true);
vcpu->stat.wfe_exit_stat++;
-   kvm_vcpu_on_spin(vcpu);
+   kvm_vcpu_on_spin(vcpu, false);
} else {
trace_kvm_wfx(*vcpu_pc(vcpu), false);
vcpu->stat.wfi_exit_stat++;
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 17d8a1677a0b..da57622cacca 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -84,7 +84,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct 
kvm_run *run)
if (kvm_vcpu_get_hsr(vcpu) & ESR_ELx_WFx_ISS_WFE) {
trace_kvm_wfx_arm64(*vcpu_pc(vcpu), true);
vcpu->stat.wfe_exit_stat++;
-   kvm_vcpu_on_spin(vcpu);
+   kvm_vcpu_on_spin(vcpu, false);
} else {
trace_kvm_wfx_arm64(*vcpu_pc(vcpu), false);
vcpu->stat.wfi_exit_stat++;
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index d4b2ad18eef2..70208bed5a15 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -98,6 +98,11 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
return !!(vcpu->arch.pending_exceptions);
 }
 
+bool kvm_arch_vcpu_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 1a75c0b5f4ca..6184c45015f3 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -58,6 +58,11 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
return !!(v->arch.pending_exceptions) || kvm_request_pending(v);
 }
 
+bool kvm_arch_vcpu_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/diag.c b/arch/s390/kvm/diag.c
index ce865bd4f81d..6182edebea3d 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -150,7 +150,7 @@ static int __diag_time_slice_end(struct kvm_vcpu *vcpu)
 {
VCPU_EVENT(vcpu, 5, "%s", "diag time slice end");
vcpu->stat.diagnose_44++;
-   kvm_vcpu_on_spin(vcpu);
+   kvm_vcpu_on_spin(vcpu, false);
return 0;
 }
 
diff --git a/arch/s390/kvm/kvm-s390.c 

Re: [PATCH v2 1/4] KVM: add spinlock optimization framework

[Paolo Bonzini]

On 08/08/2017 10:42, David Hildenbrand wrote:
> 
>> +bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> +return false;
>> +}
> 
> why don't we need an EXPORT_SYMBOL here?

Is it used outside the KVM module?  I think no architecture actually needs
to export it.

>> -void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>> +void kvm_vcpu_on_spin(struct kvm_vcpu *me, bool me_in_kern)
>>  {
>>  struct kvm *kvm = me->kvm;
>>  struct kvm_vcpu *vcpu;
>> @@ -2348,6 +2348,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>>  continue;
>>  if (swait_active(>wq) && 
>> !kvm_arch_vcpu_runnable(vcpu))
>>  continue;
>> +if (me_in_kern && !kvm_arch_vcpu_in_kernel(vcpu))
>> +continue;
> 
> 
> hm, does this patch compile? (me_in_kern)

Why not? :)  This is what I have:

>From d62a40d49f44ff7e789a15416316ef1cba93fa85 Mon Sep 17 00:00:00 2001
From: "Longpeng(Mike)" 
Date: Tue, 8 Aug 2017 12:05:32 +0800
Subject: [PATCH 1/4] KVM: add spinlock optimization framework

If a vcpu exits due to request a user mode spinlock, then
the spinlock-holder may be preempted in user mode or kernel mode.
(Note that not all architectures trap spin loops in user mode,
only AMD x86 and ARM/ARM64 currently do).

But if a vcpu exits in kernel mode, then the holder must be
preempted in kernel mode, so we should choose a vcpu in kernel mode
as a more likely candidate for the lock holder.

This introduces kvm_arch_vcpu_in_kernel() to decide whether the
vcpu is in kernel-mode when it's preempted.  kvm_vcpu_on_spin's
new argument says the same of the spinning VCPU.

Signed-off-by: Longpeng(Mike) 
Signed-off-by: Paolo Bonzini 
---
 arch/arm/kvm/handle_exit.c   | 2 +-
 arch/arm64/kvm/handle_exit.c | 2 +-
 arch/mips/kvm/mips.c | 5 +
 arch/powerpc/kvm/powerpc.c   | 5 +
 arch/s390/kvm/diag.c | 2 +-
 arch/s390/kvm/kvm-s390.c | 5 +
 arch/x86/kvm/hyperv.c| 2 +-
 arch/x86/kvm/svm.c   | 2 +-
 arch/x86/kvm/vmx.c   | 2 +-
 arch/x86/kvm/x86.c   | 5 +
 include/linux/kvm_host.h | 3 ++-
 virt/kvm/arm/arm.c   | 5 +
 virt/kvm/kvm_main.c  | 4 +++-
 13 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
index 54442e375354..196122bb6968 100644
--- a/arch/arm/kvm/handle_exit.c
+++ b/arch/arm/kvm/handle_exit.c
@@ -67,7 +67,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct 
kvm_run *run)
if (kvm_vcpu_get_hsr(vcpu) & HSR_WFI_IS_WFE) {
trace_kvm_wfx(*vcpu_pc(vcpu), true);
vcpu->stat.wfe_exit_stat++;
-   kvm_vcpu_on_spin(vcpu);
+   kvm_vcpu_on_spin(vcpu, false);
} else {
trace_kvm_wfx(*vcpu_pc(vcpu), false);
vcpu->stat.wfi_exit_stat++;
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 17d8a1677a0b..da57622cacca 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -84,7 +84,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct 
kvm_run *run)
if (kvm_vcpu_get_hsr(vcpu) & ESR_ELx_WFx_ISS_WFE) {
trace_kvm_wfx_arm64(*vcpu_pc(vcpu), true);
vcpu->stat.wfe_exit_stat++;
-   kvm_vcpu_on_spin(vcpu);
+   kvm_vcpu_on_spin(vcpu, false);
} else {
trace_kvm_wfx_arm64(*vcpu_pc(vcpu), false);
vcpu->stat.wfi_exit_stat++;
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index d4b2ad18eef2..70208bed5a15 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -98,6 +98,11 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
return !!(vcpu->arch.pending_exceptions);
 }
 
+bool kvm_arch_vcpu_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 1a75c0b5f4ca..6184c45015f3 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -58,6 +58,11 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
return !!(v->arch.pending_exceptions) || kvm_request_pending(v);
 }
 
+bool kvm_arch_vcpu_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/diag.c b/arch/s390/kvm/diag.c
index ce865bd4f81d..6182edebea3d 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -150,7 +150,7 @@ static int __diag_time_slice_end(struct kvm_vcpu *vcpu)
 {
VCPU_EVENT(vcpu, 5, "%s", "diag time slice end");
vcpu->stat.diagnose_44++;
-   kvm_vcpu_on_spin(vcpu);
+   kvm_vcpu_on_spin(vcpu, false);
return 0;
 }
 
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index af09d3437631..0b0c689f1d9a 100644
--- 

Re: [PATCH v2 1/4] KVM: add spinlock optimization framework

[David Hildenbrand]

On 08.08.2017 10:42, David Hildenbrand wrote:
> 
>> +bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> +return false;
>> +}
> 
> why don't we need an EXPORT_SYMBOL here?
> 
>> +
>>  /* 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 15252d7..e7720d2 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -2317,7 +2317,7 @@ static bool 
>> kvm_vcpu_eligible_for_directed_yield(struct kvm_vcpu *vcpu)
>>  #endif
>>  }
>>  
>> -void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>> +void kvm_vcpu_on_spin(struct kvm_vcpu *me, bool me_in_kern)
>>  {
>>  struct kvm *kvm = me->kvm;
>>  struct kvm_vcpu *vcpu;
>> @@ -2348,6 +2348,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>>  continue;
>>  if (swait_active(>wq) && 
>> !kvm_arch_vcpu_runnable(vcpu))
>>  continue;
>> +if (me_in_kern && !kvm_arch_vcpu_in_kernel(vcpu))
>> +continue;
> 
> 
> hm, does this patch compile? (me_in_kern)

pardon me, missed the parameter, so ignore this comment. comment
regarding splitting up below still holds :)

> 
> I would even move this to an other patch.
> 
> Maybe even split into
> 
> a) introducing kvm_arch_vcpu_in_kernel() for all archs
> b) modifying kvm_vcpu_on_spin(), passing the result from
> kvm_arch_vcpu_in_kernel()
> c) filling kvm_arch_vcpu_in_kernel() with life for different archs
> (multiple patches)
> d) pimping kvm_vcpu_on_spin()
> 
>>  if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
>>  continue;
>>  
>>
> 
> 


-- 

Thanks,

David

Re: [PATCH v2 1/4] KVM: add spinlock optimization framework

[David Hildenbrand]

On 08.08.2017 10:42, David Hildenbrand wrote:
> 
>> +bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
>> +{
>> +return false;
>> +}
> 
> why don't we need an EXPORT_SYMBOL here?
> 
>> +
>>  /* 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 15252d7..e7720d2 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -2317,7 +2317,7 @@ static bool 
>> kvm_vcpu_eligible_for_directed_yield(struct kvm_vcpu *vcpu)
>>  #endif
>>  }
>>  
>> -void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>> +void kvm_vcpu_on_spin(struct kvm_vcpu *me, bool me_in_kern)
>>  {
>>  struct kvm *kvm = me->kvm;
>>  struct kvm_vcpu *vcpu;
>> @@ -2348,6 +2348,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>>  continue;
>>  if (swait_active(>wq) && 
>> !kvm_arch_vcpu_runnable(vcpu))
>>  continue;
>> +if (me_in_kern && !kvm_arch_vcpu_in_kernel(vcpu))
>> +continue;
> 
> 
> hm, does this patch compile? (me_in_kern)

pardon me, missed the parameter, so ignore this comment. comment
regarding splitting up below still holds :)

> 
> I would even move this to an other patch.
> 
> Maybe even split into
> 
> a) introducing kvm_arch_vcpu_in_kernel() for all archs
> b) modifying kvm_vcpu_on_spin(), passing the result from
> kvm_arch_vcpu_in_kernel()
> c) filling kvm_arch_vcpu_in_kernel() with life for different archs
> (multiple patches)
> d) pimping kvm_vcpu_on_spin()
> 
>>  if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
>>  continue;
>>  
>>
> 
> 


-- 

Thanks,

David

Re: [PATCH v2 1/4] KVM: add spinlock optimization framework

[David Hildenbrand]

> +bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}

why don't we need an EXPORT_SYMBOL here?

> +
>  /* 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 15252d7..e7720d2 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2317,7 +2317,7 @@ static bool kvm_vcpu_eligible_for_directed_yield(struct 
> kvm_vcpu *vcpu)
>  #endif
>  }
>  
> -void kvm_vcpu_on_spin(struct kvm_vcpu *me)
> +void kvm_vcpu_on_spin(struct kvm_vcpu *me, bool me_in_kern)
>  {
>   struct kvm *kvm = me->kvm;
>   struct kvm_vcpu *vcpu;
> @@ -2348,6 +2348,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>   continue;
>   if (swait_active(>wq) && 
> !kvm_arch_vcpu_runnable(vcpu))
>   continue;
> + if (me_in_kern && !kvm_arch_vcpu_in_kernel(vcpu))
> + continue;


hm, does this patch compile? (me_in_kern)

I would even move this to an other patch.

Maybe even split into

a) introducing kvm_arch_vcpu_in_kernel() for all archs
b) modifying kvm_vcpu_on_spin(), passing the result from
kvm_arch_vcpu_in_kernel()
c) filling kvm_arch_vcpu_in_kernel() with life for different archs
(multiple patches)
d) pimping kvm_vcpu_on_spin()

>   if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
>   continue;
>  
> 


-- 

Thanks,

David

Re: [PATCH v2 1/4] KVM: add spinlock optimization framework

[David Hildenbrand]

> +bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}

why don't we need an EXPORT_SYMBOL here?

> +
>  /* 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 15252d7..e7720d2 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2317,7 +2317,7 @@ static bool kvm_vcpu_eligible_for_directed_yield(struct 
> kvm_vcpu *vcpu)
>  #endif
>  }
>  
> -void kvm_vcpu_on_spin(struct kvm_vcpu *me)
> +void kvm_vcpu_on_spin(struct kvm_vcpu *me, bool me_in_kern)
>  {
>   struct kvm *kvm = me->kvm;
>   struct kvm_vcpu *vcpu;
> @@ -2348,6 +2348,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>   continue;
>   if (swait_active(>wq) && 
> !kvm_arch_vcpu_runnable(vcpu))
>   continue;
> + if (me_in_kern && !kvm_arch_vcpu_in_kernel(vcpu))
> + continue;


hm, does this patch compile? (me_in_kern)

I would even move this to an other patch.

Maybe even split into

a) introducing kvm_arch_vcpu_in_kernel() for all archs
b) modifying kvm_vcpu_on_spin(), passing the result from
kvm_arch_vcpu_in_kernel()
c) filling kvm_arch_vcpu_in_kernel() with life for different archs
(multiple patches)
d) pimping kvm_vcpu_on_spin()

>   if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
>   continue;
>  
> 


-- 

Thanks,

David

Re: [PATCH v2 1/4] KVM: add spinlock optimization framework

[Cornelia Huck]

On Tue, 8 Aug 2017 09:34:14 +0200
Paolo Bonzini  wrote:

> On 08/08/2017 06:05, Longpeng(Mike) wrote:
> > return 1;
> > diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
> > index ce865bd..4ea8c38 100644
> > --- a/arch/s390/kvm/diag.c
> > +++ b/arch/s390/kvm/diag.c
> > @@ -150,7 +150,7 @@ static int __diag_time_slice_end(struct kvm_vcpu *vcpu)
> >  {
> > VCPU_EVENT(vcpu, 5, "%s", "diag time slice end");
> > vcpu->stat.diagnose_44++;
> > -   kvm_vcpu_on_spin(vcpu);
> > +   kvm_vcpu_on_spin(vcpu, kvm_arch_vcpu_in_kernel(vcpu));
> > return 0;
> >  }
> >
> 
> IIUC, diag is a privileged instruction so this an also be "true".
> 
> Paolo

Yes, indeed.

Re: [PATCH v2 1/4] KVM: add spinlock optimization framework

[Cornelia Huck]

On Tue, 8 Aug 2017 09:34:14 +0200
Paolo Bonzini  wrote:

> On 08/08/2017 06:05, Longpeng(Mike) wrote:
> > return 1;
> > diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
> > index ce865bd..4ea8c38 100644
> > --- a/arch/s390/kvm/diag.c
> > +++ b/arch/s390/kvm/diag.c
> > @@ -150,7 +150,7 @@ static int __diag_time_slice_end(struct kvm_vcpu *vcpu)
> >  {
> > VCPU_EVENT(vcpu, 5, "%s", "diag time slice end");
> > vcpu->stat.diagnose_44++;
> > -   kvm_vcpu_on_spin(vcpu);
> > +   kvm_vcpu_on_spin(vcpu, kvm_arch_vcpu_in_kernel(vcpu));
> > return 0;
> >  }
> >
> 
> IIUC, diag is a privileged instruction so this an also be "true".
> 
> Paolo

Yes, indeed.

Re: [PATCH v2 1/4] KVM: add spinlock optimization framework

[Paolo Bonzini]

On 08/08/2017 06:05, Longpeng(Mike) wrote:
>   return 1;
> diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
> index ce865bd..4ea8c38 100644
> --- a/arch/s390/kvm/diag.c
> +++ b/arch/s390/kvm/diag.c
> @@ -150,7 +150,7 @@ static int __diag_time_slice_end(struct kvm_vcpu *vcpu)
>  {
>   VCPU_EVENT(vcpu, 5, "%s", "diag time slice end");
>   vcpu->stat.diagnose_44++;
> - kvm_vcpu_on_spin(vcpu);
> + kvm_vcpu_on_spin(vcpu, kvm_arch_vcpu_in_kernel(vcpu));
>   return 0;
>  }
>  

IIUC, diag is a privileged instruction so this an also be "true".

Paolo

Re: [PATCH v2 1/4] KVM: add spinlock optimization framework

[Paolo Bonzini]

On 08/08/2017 06:05, Longpeng(Mike) wrote:
>   return 1;
> diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
> index ce865bd..4ea8c38 100644
> --- a/arch/s390/kvm/diag.c
> +++ b/arch/s390/kvm/diag.c
> @@ -150,7 +150,7 @@ static int __diag_time_slice_end(struct kvm_vcpu *vcpu)
>  {
>   VCPU_EVENT(vcpu, 5, "%s", "diag time slice end");
>   vcpu->stat.diagnose_44++;
> - kvm_vcpu_on_spin(vcpu);
> + kvm_vcpu_on_spin(vcpu, kvm_arch_vcpu_in_kernel(vcpu));
>   return 0;
>  }
>  

IIUC, diag is a privileged instruction so this an also be "true".

Paolo