linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] KVM: x86: guest exit microoptimization
@ 2016-06-16  8:21 Paolo Bonzini
  2016-06-16  8:21 ` [RFC PATCH 1/2] KVM: x86: always use "acknowledge interrupt on exit" Paolo Bonzini
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Paolo Bonzini @ 2016-06-16  8:21 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: bsd, rkrcmar

This saves about 20 clock cycles per vmexit by avoiding a
local_irq_save/restore pair.  The price is that nested VMX will break
with KVM hosts < 3.16, because the "acknowledge interrupt on exit"
feature becomes mandatory.  What do you think?

Paolo

Paolo Bonzini (2):
  KVM: x86: always use "acknowledge interrupt on exit"
  KVM: x86: use __kvm_guest_exit

 arch/x86/kvm/svm.c |  6 ++++++
 arch/x86/kvm/vmx.c | 11 ++++-------
 arch/x86/kvm/x86.c | 11 ++---------
 3 files changed, 12 insertions(+), 16 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH 1/2] KVM: x86: always use "acknowledge interrupt on exit"
  2016-06-16  8:21 [RFC PATCH 0/2] KVM: x86: guest exit microoptimization Paolo Bonzini
@ 2016-06-16  8:21 ` Paolo Bonzini
  2016-06-16 21:53   ` Bandan Das
  2016-06-16  8:21 ` [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit Paolo Bonzini
  2016-07-01 16:45 ` [RFC PATCH 0/2] KVM: x86: guest exit microoptimization Paolo Bonzini
  2 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2016-06-16  8:21 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: bsd, rkrcmar

This is necessary to simplify handle_external_intr in the next patch.
It means that nested KVM will require 3.16 on the host (or 3.17 if you
have APICv enabled).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/vmx.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index e185649fb8b7..4e9657730bf6 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3362,12 +3362,12 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 		      vmx_capability.ept, vmx_capability.vpid);
 	}
 
-	min = VM_EXIT_SAVE_DEBUG_CONTROLS;
+	min = VM_EXIT_SAVE_DEBUG_CONTROLS | VM_EXIT_ACK_INTR_ON_EXIT;
 #ifdef CONFIG_X86_64
 	min |= VM_EXIT_HOST_ADDR_SPACE_SIZE;
 #endif
 	opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT |
-		VM_EXIT_ACK_INTR_ON_EXIT | VM_EXIT_CLEAR_BNDCFGS;
+		VM_EXIT_CLEAR_BNDCFGS;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS,
 				&_vmexit_control) < 0)
 		return -EIO;
@@ -3380,8 +3380,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 		return -EIO;
 
 	if (!(_cpu_based_2nd_exec_control &
-		SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY) ||
-		!(_vmexit_control & VM_EXIT_ACK_INTR_ON_EXIT))
+		SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY))
 		_pin_based_exec_control &= ~PIN_BASED_POSTED_INTR;
 
 	min = VM_ENTRY_LOAD_DEBUG_CONTROLS;
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit
  2016-06-16  8:21 [RFC PATCH 0/2] KVM: x86: guest exit microoptimization Paolo Bonzini
  2016-06-16  8:21 ` [RFC PATCH 1/2] KVM: x86: always use "acknowledge interrupt on exit" Paolo Bonzini
@ 2016-06-16  8:21 ` Paolo Bonzini
  2016-06-16 16:43   ` David Matlack
  2016-06-16 22:01   ` Bandan Das
  2016-07-01 16:45 ` [RFC PATCH 0/2] KVM: x86: guest exit microoptimization Paolo Bonzini
  2 siblings, 2 replies; 11+ messages in thread
From: Paolo Bonzini @ 2016-06-16  8:21 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: bsd, rkrcmar

This gains ~20 clock cycles per vmexit.  On Intel there is no need
anymore to enable the interrupts in vmx_handle_external_intr, since we
are using the "acknowledge interrupt on exit" feature.  AMD needs to do
that temporarily, and must be careful to avoid the interrupt shadow.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/svm.c |  6 ++++++
 arch/x86/kvm/vmx.c |  4 +---
 arch/x86/kvm/x86.c | 11 ++---------
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 5ff292778110..5bfdbbf1ce79 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4935,6 +4935,12 @@ out:
 static void svm_handle_external_intr(struct kvm_vcpu *vcpu)
 {
 	local_irq_enable();
+	/*
+	 * We must execute an instruction with interrupts enabled, so
+	 * the "cli" doesn't fall right on the interrupt shadow.
+	 */
+	asm("nop");
+	local_irq_disable();
 }
 
 static void svm_sched_in(struct kvm_vcpu *vcpu, int cpu)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 4e9657730bf6..a46bce9e3683 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8544,7 +8544,6 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
 			"push %[sp]\n\t"
 #endif
 			"pushf\n\t"
-			"orl $0x200, (%%" _ASM_SP ")\n\t"
 			__ASM_SIZE(push) " $%c[cs]\n\t"
 			"call *%[entry]\n\t"
 			:
@@ -8557,8 +8556,7 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
 			[ss]"i"(__KERNEL_DS),
 			[cs]"i"(__KERNEL_CS)
 			);
-	} else
-		local_irq_enable();
+	}
 }
 
 static bool vmx_has_high_real_mode_segbase(void)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7e3041ef050f..cc741b68139c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6706,21 +6706,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 
 	kvm_put_guest_xcr0(vcpu);
 
-	/* Interrupt is enabled by handle_external_intr() */
 	kvm_x86_ops->handle_external_intr(vcpu);
 
 	++vcpu->stat.exits;
 
-	/*
-	 * We must have an instruction between local_irq_enable() and
-	 * kvm_guest_exit(), so the timer interrupt isn't delayed by
-	 * the interrupt shadow.  The stat.exits increment will do nicely.
-	 * But we need to prevent reordering, hence this barrier():
-	 */
-	barrier();
-
-	kvm_guest_exit();
+	__kvm_guest_exit();
 
+	local_irq_enable();
 	preempt_enable();
 
 	vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit
  2016-06-16  8:21 ` [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit Paolo Bonzini
@ 2016-06-16 16:43   ` David Matlack
  2016-06-16 16:47     ` Paolo Bonzini
  2016-06-16 22:01   ` Bandan Das
  1 sibling, 1 reply; 11+ messages in thread
From: David Matlack @ 2016-06-16 16:43 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm list, bsd, Radim Krčmář

On Thu, Jun 16, 2016 at 1:21 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This gains ~20 clock cycles per vmexit.  On Intel there is no need
> anymore to enable the interrupts in vmx_handle_external_intr, since we
> are using the "acknowledge interrupt on exit" feature.  AMD needs to do
> that temporarily, and must be careful to avoid the interrupt shadow.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/svm.c |  6 ++++++
>  arch/x86/kvm/vmx.c |  4 +---
>  arch/x86/kvm/x86.c | 11 ++---------
>  3 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 5ff292778110..5bfdbbf1ce79 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -4935,6 +4935,12 @@ out:
>  static void svm_handle_external_intr(struct kvm_vcpu *vcpu)
>  {
>         local_irq_enable();
> +       /*
> +        * We must execute an instruction with interrupts enabled, so
> +        * the "cli" doesn't fall right on the interrupt shadow.
> +        */
> +       asm("nop");
> +       local_irq_disable();
>  }
>
>  static void svm_sched_in(struct kvm_vcpu *vcpu, int cpu)
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 4e9657730bf6..a46bce9e3683 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8544,7 +8544,6 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
>                         "push %[sp]\n\t"
>  #endif
>                         "pushf\n\t"
> -                       "orl $0x200, (%%" _ASM_SP ")\n\t"
>                         __ASM_SIZE(push) " $%c[cs]\n\t"
>                         "call *%[entry]\n\t"
>                         :
> @@ -8557,8 +8556,7 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
>                         [ss]"i"(__KERNEL_DS),
>                         [cs]"i"(__KERNEL_CS)
>                         );
> -       } else
> -               local_irq_enable();
> +       }

If you make the else case the same as svm_handle_external_intr, can we
avoid requiring ack-intr-on-exit?

>  }
>
>  static bool vmx_has_high_real_mode_segbase(void)
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 7e3041ef050f..cc741b68139c 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6706,21 +6706,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>
>         kvm_put_guest_xcr0(vcpu);
>
> -       /* Interrupt is enabled by handle_external_intr() */
>         kvm_x86_ops->handle_external_intr(vcpu);
>
>         ++vcpu->stat.exits;
>
> -       /*
> -        * We must have an instruction between local_irq_enable() and
> -        * kvm_guest_exit(), so the timer interrupt isn't delayed by
> -        * the interrupt shadow.  The stat.exits increment will do nicely.
> -        * But we need to prevent reordering, hence this barrier():
> -        */
> -       barrier();
> -
> -       kvm_guest_exit();
> +       __kvm_guest_exit();
>
> +       local_irq_enable();
>         preempt_enable();
>
>         vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
> --
> 1.8.3.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit
  2016-06-16 16:43   ` David Matlack
@ 2016-06-16 16:47     ` Paolo Bonzini
  2016-06-16 17:03       ` David Matlack
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2016-06-16 16:47 UTC (permalink / raw)
  To: David Matlack; +Cc: linux-kernel, kvm list, bsd, Radim Krčmář



On 16/06/2016 18:43, David Matlack wrote:
> On Thu, Jun 16, 2016 at 1:21 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> This gains ~20 clock cycles per vmexit.  On Intel there is no need
>> anymore to enable the interrupts in vmx_handle_external_intr, since we
>> are using the "acknowledge interrupt on exit" feature.  AMD needs to do
>> that temporarily, and must be careful to avoid the interrupt shadow.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  arch/x86/kvm/svm.c |  6 ++++++
>>  arch/x86/kvm/vmx.c |  4 +---
>>  arch/x86/kvm/x86.c | 11 ++---------
>>  3 files changed, 9 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index 5ff292778110..5bfdbbf1ce79 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -4935,6 +4935,12 @@ out:
>>  static void svm_handle_external_intr(struct kvm_vcpu *vcpu)
>>  {
>>         local_irq_enable();
>> +       /*
>> +        * We must execute an instruction with interrupts enabled, so
>> +        * the "cli" doesn't fall right on the interrupt shadow.
>> +        */
>> +       asm("nop");
>> +       local_irq_disable();
>>  }
>>
>>  static void svm_sched_in(struct kvm_vcpu *vcpu, int cpu)
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 4e9657730bf6..a46bce9e3683 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -8544,7 +8544,6 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
>>                         "push %[sp]\n\t"
>>  #endif
>>                         "pushf\n\t"
>> -                       "orl $0x200, (%%" _ASM_SP ")\n\t"
>>                         __ASM_SIZE(push) " $%c[cs]\n\t"
>>                         "call *%[entry]\n\t"
>>                         :
>> @@ -8557,8 +8556,7 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
>>                         [ss]"i"(__KERNEL_DS),
>>                         [cs]"i"(__KERNEL_CS)
>>                         );
>> -       } else
>> -               local_irq_enable();
>> +       }
> 
> If you make the else case the same as svm_handle_external_intr, can we
> avoid requiring ack-intr-on-exit?

Yes, but the sti/nop/cli would be useless if ack-intr-on-exit is
available.  It's a bit ugly, so I RFCed the bold thing instead.

Are you thinking of some distros in particular that lack nested
ack-intr-on-exit?  All processors have it as far as I know.

Paolo


>>  }
>>
>>  static bool vmx_has_high_real_mode_segbase(void)
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 7e3041ef050f..cc741b68139c 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -6706,21 +6706,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>>
>>         kvm_put_guest_xcr0(vcpu);
>>
>> -       /* Interrupt is enabled by handle_external_intr() */
>>         kvm_x86_ops->handle_external_intr(vcpu);
>>
>>         ++vcpu->stat.exits;
>>
>> -       /*
>> -        * We must have an instruction between local_irq_enable() and
>> -        * kvm_guest_exit(), so the timer interrupt isn't delayed by
>> -        * the interrupt shadow.  The stat.exits increment will do nicely.
>> -        * But we need to prevent reordering, hence this barrier():
>> -        */
>> -       barrier();
>> -
>> -       kvm_guest_exit();
>> +       __kvm_guest_exit();
>>
>> +       local_irq_enable();
>>         preempt_enable();
>>
>>         vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
>> --
>> 1.8.3.1
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit
  2016-06-16 16:47     ` Paolo Bonzini
@ 2016-06-16 17:03       ` David Matlack
  2016-06-16 17:21         ` Paolo Bonzini
  0 siblings, 1 reply; 11+ messages in thread
From: David Matlack @ 2016-06-16 17:03 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm list, bsd, Radim Krčmář

On Thu, Jun 16, 2016 at 9:47 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 16/06/2016 18:43, David Matlack wrote:
>> On Thu, Jun 16, 2016 at 1:21 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> This gains ~20 clock cycles per vmexit.  On Intel there is no need
>>> anymore to enable the interrupts in vmx_handle_external_intr, since we
>>> are using the "acknowledge interrupt on exit" feature.  AMD needs to do
>>> that temporarily, and must be careful to avoid the interrupt shadow.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>  arch/x86/kvm/svm.c |  6 ++++++
>>>  arch/x86/kvm/vmx.c |  4 +---
>>>  arch/x86/kvm/x86.c | 11 ++---------
>>>  3 files changed, 9 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>>> index 5ff292778110..5bfdbbf1ce79 100644
>>> --- a/arch/x86/kvm/svm.c
>>> +++ b/arch/x86/kvm/svm.c
>>> @@ -4935,6 +4935,12 @@ out:
>>>  static void svm_handle_external_intr(struct kvm_vcpu *vcpu)
>>>  {
>>>         local_irq_enable();
>>> +       /*
>>> +        * We must execute an instruction with interrupts enabled, so
>>> +        * the "cli" doesn't fall right on the interrupt shadow.
>>> +        */
>>> +       asm("nop");
>>> +       local_irq_disable();
>>>  }
>>>
>>>  static void svm_sched_in(struct kvm_vcpu *vcpu, int cpu)
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index 4e9657730bf6..a46bce9e3683 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -8544,7 +8544,6 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
>>>                         "push %[sp]\n\t"
>>>  #endif
>>>                         "pushf\n\t"
>>> -                       "orl $0x200, (%%" _ASM_SP ")\n\t"
>>>                         __ASM_SIZE(push) " $%c[cs]\n\t"
>>>                         "call *%[entry]\n\t"
>>>                         :
>>> @@ -8557,8 +8556,7 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
>>>                         [ss]"i"(__KERNEL_DS),
>>>                         [cs]"i"(__KERNEL_CS)
>>>                         );
>>> -       } else
>>> -               local_irq_enable();
>>> +       }
>>
>> If you make the else case the same as svm_handle_external_intr, can we
>> avoid requiring ack-intr-on-exit?
>
> Yes, but the sti/nop/cli would be useless if ack-intr-on-exit is
> available.  It's a bit ugly, so I RFCed the bold thing instead.

Ahh, and handle_external_intr is called on every VM-exit, not just
VM-exits caused by external interrupts. So we'd be doing the
sti/nop/cli quite often. I was thinking we never hit the else case
when the CPU supports ack-intr-on-exit.

>
> Are you thinking of some distros in particular that lack nested
> ack-intr-on-exit?  All processors have it as far as I know.

Nope, I just thought it was possible to avoid the requirement.

>
> Paolo
>
>
>>>  }
>>>
>>>  static bool vmx_has_high_real_mode_segbase(void)
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> index 7e3041ef050f..cc741b68139c 100644
>>> --- a/arch/x86/kvm/x86.c
>>> +++ b/arch/x86/kvm/x86.c
>>> @@ -6706,21 +6706,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>>>
>>>         kvm_put_guest_xcr0(vcpu);
>>>
>>> -       /* Interrupt is enabled by handle_external_intr() */
>>>         kvm_x86_ops->handle_external_intr(vcpu);
>>>
>>>         ++vcpu->stat.exits;
>>>
>>> -       /*
>>> -        * We must have an instruction between local_irq_enable() and
>>> -        * kvm_guest_exit(), so the timer interrupt isn't delayed by
>>> -        * the interrupt shadow.  The stat.exits increment will do nicely.
>>> -        * But we need to prevent reordering, hence this barrier():
>>> -        */
>>> -       barrier();
>>> -
>>> -       kvm_guest_exit();
>>> +       __kvm_guest_exit();
>>>
>>> +       local_irq_enable();
>>>         preempt_enable();
>>>
>>>         vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
>>> --
>>> 1.8.3.1
>>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit
  2016-06-16 17:03       ` David Matlack
@ 2016-06-16 17:21         ` Paolo Bonzini
  0 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2016-06-16 17:21 UTC (permalink / raw)
  To: David Matlack; +Cc: linux-kernel, kvm list, bsd, Radim Krčmář



On 16/06/2016 19:03, David Matlack wrote:
> > > If you make the else case the same as svm_handle_external_intr, can we
> > > avoid requiring ack-intr-on-exit?
> >
> > Yes, but the sti/nop/cli would be useless if ack-intr-on-exit is
> > available.  It's a bit ugly, so I RFCed the bold thing instead.
>
> Ahh, and handle_external_intr is called on every VM-exit, not just
> VM-exits caused by external interrupts. So we'd be doing the
> sti/nop/cli quite often. I was thinking we never hit the else case
> when the CPU supports ack-intr-on-exit.

Actually it's really just aesthetics, because the sti and cli are pretty
cheap.  It's the pushf/popf that kills performance for kvm_guest_exit.
I also thought of just doing a cli/sti around __kvm_guest_exit and
calling it a day.

Ubuntu 14.04 had kernel 3.13, but the latest hardware enablement kernels
are as recent as 4.4.  And the most recent released RHEL (7.2) has all
the fixes too.

Debian Jessie has 3.16.7-ckt25, and all three patches for APICv support
have been backported to 3.16.7-ckt11.  So they should be there (but I
can only check tomorrow).

Paolo

>> >
>> > Are you thinking of some distros in particular that lack nested
>> > ack-intr-on-exit?  All processors have it as far as I know.
> Nope, I just thought it was possible to avoid the requirement.
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 1/2] KVM: x86: always use "acknowledge interrupt on exit"
  2016-06-16  8:21 ` [RFC PATCH 1/2] KVM: x86: always use "acknowledge interrupt on exit" Paolo Bonzini
@ 2016-06-16 21:53   ` Bandan Das
  0 siblings, 0 replies; 11+ messages in thread
From: Bandan Das @ 2016-06-16 21:53 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, rkrcmar

Paolo Bonzini <pbonzini@redhat.com> writes:

> This is necessary to simplify handle_external_intr in the next patch.
> It means that nested KVM will require 3.16 on the host (or 3.17 if you
> have APICv enabled).
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/vmx.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index e185649fb8b7..4e9657730bf6 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3362,12 +3362,12 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>  		      vmx_capability.ept, vmx_capability.vpid);
>  	}
>  
> -	min = VM_EXIT_SAVE_DEBUG_CONTROLS;
> +	min = VM_EXIT_SAVE_DEBUG_CONTROLS | VM_EXIT_ACK_INTR_ON_EXIT;
>  #ifdef CONFIG_X86_64
>  	min |= VM_EXIT_HOST_ADDR_SPACE_SIZE;
>  #endif
>  	opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT |
> -		VM_EXIT_ACK_INTR_ON_EXIT | VM_EXIT_CLEAR_BNDCFGS;
> +		VM_EXIT_CLEAR_BNDCFGS;
>  	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS,
>  				&_vmexit_control) < 0)
>  		return -EIO;

Even if it breaks, this will complain quite loudly for the user to
upgrade. Maybe, a ack_intr specific message would be more direct (since that
is the one we are breaking) but imo it's fine either way.

Bandan


> @@ -3380,8 +3380,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>  		return -EIO;
>  
>  	if (!(_cpu_based_2nd_exec_control &
> -		SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY) ||
> -		!(_vmexit_control & VM_EXIT_ACK_INTR_ON_EXIT))
> +		SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY))
>  		_pin_based_exec_control &= ~PIN_BASED_POSTED_INTR;
>  
>  	min = VM_ENTRY_LOAD_DEBUG_CONTROLS;

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit
  2016-06-16  8:21 ` [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit Paolo Bonzini
  2016-06-16 16:43   ` David Matlack
@ 2016-06-16 22:01   ` Bandan Das
  2016-06-17  5:20     ` Paolo Bonzini
  1 sibling, 1 reply; 11+ messages in thread
From: Bandan Das @ 2016-06-16 22:01 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, rkrcmar

...
>  static bool vmx_has_high_real_mode_segbase(void)
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 7e3041ef050f..cc741b68139c 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6706,21 +6706,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>  
>  	kvm_put_guest_xcr0(vcpu);
>  
> -	/* Interrupt is enabled by handle_external_intr() */
>  	kvm_x86_ops->handle_external_intr(vcpu);
>  
>  	++vcpu->stat.exits;
>  
> -	/*
> -	 * We must have an instruction between local_irq_enable() and
> -	 * kvm_guest_exit(), so the timer interrupt isn't delayed by
> -	 * the interrupt shadow.  The stat.exits increment will do nicely.
> -	 * But we need to prevent reordering, hence this barrier():
> -	 */
> -	barrier();
> -
> -	kvm_guest_exit();
> +	__kvm_guest_exit();

kvm_guest_exit has no more callers and so can be removed.

Bandan

> +	local_irq_enable();
>  	preempt_enable();
>  
>  	vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit
  2016-06-16 22:01   ` Bandan Das
@ 2016-06-17  5:20     ` Paolo Bonzini
  0 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2016-06-17  5:20 UTC (permalink / raw)
  To: Bandan Das; +Cc: linux-kernel, kvm, rkrcmar

> >  static bool vmx_has_high_real_mode_segbase(void)
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 7e3041ef050f..cc741b68139c 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -6706,21 +6706,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >  
> >  	kvm_put_guest_xcr0(vcpu);
> >  
> > -	/* Interrupt is enabled by handle_external_intr() */
> >  	kvm_x86_ops->handle_external_intr(vcpu);
> >  
> >  	++vcpu->stat.exits;
> >  
> > -	/*
> > -	 * We must have an instruction between local_irq_enable() and
> > -	 * kvm_guest_exit(), so the timer interrupt isn't delayed by
> > -	 * the interrupt shadow.  The stat.exits increment will do nicely.
> > -	 * But we need to prevent reordering, hence this barrier():
> > -	 */
> > -	barrier();
> > -
> > -	kvm_guest_exit();
> > +	__kvm_guest_exit();
> 
> kvm_guest_exit has no more callers and so can be removed.

ARM and PPC call it.

Paolo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 0/2] KVM: x86: guest exit microoptimization
  2016-06-16  8:21 [RFC PATCH 0/2] KVM: x86: guest exit microoptimization Paolo Bonzini
  2016-06-16  8:21 ` [RFC PATCH 1/2] KVM: x86: always use "acknowledge interrupt on exit" Paolo Bonzini
  2016-06-16  8:21 ` [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit Paolo Bonzini
@ 2016-07-01 16:45 ` Paolo Bonzini
  2 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2016-07-01 16:45 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: bsd, rkrcmar



On 16/06/2016 10:21, Paolo Bonzini wrote:
> This saves about 20 clock cycles per vmexit by avoiding a
> local_irq_save/restore pair.  The price is that nested VMX will break
> with KVM hosts < 3.16, because the "acknowledge interrupt on exit"
> feature becomes mandatory.  What do you think?

I'm pushing these patches.

Paolo

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-07-01 16:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-16  8:21 [RFC PATCH 0/2] KVM: x86: guest exit microoptimization Paolo Bonzini
2016-06-16  8:21 ` [RFC PATCH 1/2] KVM: x86: always use "acknowledge interrupt on exit" Paolo Bonzini
2016-06-16 21:53   ` Bandan Das
2016-06-16  8:21 ` [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit Paolo Bonzini
2016-06-16 16:43   ` David Matlack
2016-06-16 16:47     ` Paolo Bonzini
2016-06-16 17:03       ` David Matlack
2016-06-16 17:21         ` Paolo Bonzini
2016-06-16 22:01   ` Bandan Das
2016-06-17  5:20     ` Paolo Bonzini
2016-07-01 16:45 ` [RFC PATCH 0/2] KVM: x86: guest exit microoptimization Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).