KVM: VMX: use kvm_fast_pio_in for handling IN I/O
diff mbox

Message ID 20180306205838.2710-1-sean.j.christopherson@intel.com
State New
Headers show

Commit Message

Sean Christopherson March 6, 2018, 8:58 p.m. UTC
Fast emulation of processor I/O for IN was disabled on x86 (both VMX
and SVM) some years ago due to a buggy implementation.  The addition
of kvm_fast_pio_in(), used by SVM, re-introduced (functional!) fast
emulation of IN.  Piggyback SVM's work and use kvm_fast_pio_in() on
VMX instead of performing full emulation of IN.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kvm/vmx.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Paolo Bonzini March 7, 2018, 3:19 p.m. UTC | #1
On 06/03/2018 21:58, Sean Christopherson wrote:
> Fast emulation of processor I/O for IN was disabled on x86 (both VMX
> and SVM) some years ago due to a buggy implementation.  The addition
> of kvm_fast_pio_in(), used by SVM, re-introduced (functional!) fast
> emulation of IN.  Piggyback SVM's work and use kvm_fast_pio_in() on
> VMX instead of performing full emulation of IN.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>

There's a little code duplication that would be nice to avoid, moving
kvm_skip_emulated_instruction to kvm_fast_pio_{in,out}.  Otherwise,

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

> ---
>  arch/x86/kvm/vmx.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 051dab74e4e9..c8a8391e95e5 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6231,15 +6231,15 @@ static int handle_io(struct kvm_vcpu *vcpu)
>  
>  	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
>  	string = (exit_qualification & 16) != 0;
> -	in = (exit_qualification & 8) != 0;
>  
>  	++vcpu->stat.io_exits;
>  
> -	if (string || in)
> +	if (string)
>  		return emulate_instruction(vcpu, 0) == EMULATE_DONE;
>  
>  	port = exit_qualification >> 16;
>  	size = (exit_qualification & 7) + 1;
> +	in = (exit_qualification & 8) != 0;
>  
>  	ret = kvm_skip_emulated_instruction(vcpu);
>  
> @@ -6247,7 +6247,10 @@ static int handle_io(struct kvm_vcpu *vcpu)
>  	 * TODO: we might be squashing a KVM_GUESTDBG_SINGLESTEP-triggered
>  	 * KVM_EXIT_DEBUG here.
>  	 */
> -	return kvm_fast_pio_out(vcpu, size, port) && ret;
> +	if (in)
> +		return kvm_fast_pio_in(vcpu, size, port) && ret;
> +	else
> +		return kvm_fast_pio_out(vcpu, size, port) && ret;
>  }
>  
>  static void
>
Sean Christopherson March 7, 2018, 3:35 p.m. UTC | #2
On Wed, 2018-03-07 at 16:19 +0100, Paolo Bonzini wrote:
> On 06/03/2018 21:58, Sean Christopherson wrote:
> > 
> > Fast emulation of processor I/O for IN was disabled on x86 (both VMX
> > and SVM) some years ago due to a buggy implementation.  The addition
> > of kvm_fast_pio_in(), used by SVM, re-introduced (functional!) fast
> > emulation of IN.  Piggyback SVM's work and use kvm_fast_pio_in() on
> > VMX instead of performing full emulation of IN.
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> There's a little code duplication that would be nice to avoid, moving
> kvm_skip_emulated_instruction to kvm_fast_pio_{in,out}.  Otherwise,

What about adding kvm_fast_pio() to also avoid duplication of branching
on 'in' versus 'out'?  That seems worthy of a v2.

> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Paolo
> 
> > 
> > ---
> >  arch/x86/kvm/vmx.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 051dab74e4e9..c8a8391e95e5 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -6231,15 +6231,15 @@ static int handle_io(struct kvm_vcpu *vcpu)
> >  
> >  	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
> >  	string = (exit_qualification & 16) != 0;
> > -	in = (exit_qualification & 8) != 0;
> >  
> >  	++vcpu->stat.io_exits;
> >  
> > -	if (string || in)
> > +	if (string)
> >  		return emulate_instruction(vcpu, 0) == EMULATE_DONE;
> >  
> >  	port = exit_qualification >> 16;
> >  	size = (exit_qualification & 7) + 1;
> > +	in = (exit_qualification & 8) != 0;
> >  
> >  	ret = kvm_skip_emulated_instruction(vcpu);
> >  
> > @@ -6247,7 +6247,10 @@ static int handle_io(struct kvm_vcpu *vcpu)
> >  	 * TODO: we might be squashing a KVM_GUESTDBG_SINGLESTEP-triggered
> >  	 * KVM_EXIT_DEBUG here.
> >  	 */
> > -	return kvm_fast_pio_out(vcpu, size, port) && ret;
> > +	if (in)
> > +		return kvm_fast_pio_in(vcpu, size, port) && ret;
> > +	else
> > +		return kvm_fast_pio_out(vcpu, size, port) && ret;
> >  }
> >  
> >  static void
> >
Paolo Bonzini March 7, 2018, 4:13 p.m. UTC | #3
On 07/03/2018 16:35, Sean Christopherson wrote:
> On Wed, 2018-03-07 at 16:19 +0100, Paolo Bonzini wrote:
>> On 06/03/2018 21:58, Sean Christopherson wrote:
>>>
>>> Fast emulation of processor I/O for IN was disabled on x86 (both VMX
>>> and SVM) some years ago due to a buggy implementation.  The addition
>>> of kvm_fast_pio_in(), used by SVM, re-introduced (functional!) fast
>>> emulation of IN.  Piggyback SVM's work and use kvm_fast_pio_in() on
>>> VMX instead of performing full emulation of IN.
>>>
>>> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
>> There's a little code duplication that would be nice to avoid, moving
>> kvm_skip_emulated_instruction to kvm_fast_pio_{in,out}.  Otherwise,
> 
> What about adding kvm_fast_pio() to also avoid duplication of branching
> on 'in' versus 'out'?  That seems worthy of a v2.

If you would like to do that for a v2, that makes sense as well, indeed.

Paolo

>>
>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>>
>> Paolo
>>
>>>
>>> ---
>>>  arch/x86/kvm/vmx.c | 9 ++++++---
>>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index 051dab74e4e9..c8a8391e95e5 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -6231,15 +6231,15 @@ static int handle_io(struct kvm_vcpu *vcpu)
>>>  
>>>  	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
>>>  	string = (exit_qualification & 16) != 0;
>>> -	in = (exit_qualification & 8) != 0;
>>>  
>>>  	++vcpu->stat.io_exits;
>>>  
>>> -	if (string || in)
>>> +	if (string)
>>>  		return emulate_instruction(vcpu, 0) == EMULATE_DONE;
>>>  
>>>  	port = exit_qualification >> 16;
>>>  	size = (exit_qualification & 7) + 1;
>>> +	in = (exit_qualification & 8) != 0;
>>>  
>>>  	ret = kvm_skip_emulated_instruction(vcpu);
>>>  
>>> @@ -6247,7 +6247,10 @@ static int handle_io(struct kvm_vcpu *vcpu)
>>>  	 * TODO: we might be squashing a KVM_GUESTDBG_SINGLESTEP-triggered
>>>  	 * KVM_EXIT_DEBUG here.
>>>  	 */
>>> -	return kvm_fast_pio_out(vcpu, size, port) && ret;
>>> +	if (in)
>>> +		return kvm_fast_pio_in(vcpu, size, port) && ret;
>>> +	else
>>> +		return kvm_fast_pio_out(vcpu, size, port) && ret;
>>>  }
>>>  
>>>  static void
>>>

Patch
diff mbox

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 051dab74e4e9..c8a8391e95e5 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6231,15 +6231,15 @@  static int handle_io(struct kvm_vcpu *vcpu)
 
 	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
 	string = (exit_qualification & 16) != 0;
-	in = (exit_qualification & 8) != 0;
 
 	++vcpu->stat.io_exits;
 
-	if (string || in)
+	if (string)
 		return emulate_instruction(vcpu, 0) == EMULATE_DONE;
 
 	port = exit_qualification >> 16;
 	size = (exit_qualification & 7) + 1;
+	in = (exit_qualification & 8) != 0;
 
 	ret = kvm_skip_emulated_instruction(vcpu);
 
@@ -6247,7 +6247,10 @@  static int handle_io(struct kvm_vcpu *vcpu)
 	 * TODO: we might be squashing a KVM_GUESTDBG_SINGLESTEP-triggered
 	 * KVM_EXIT_DEBUG here.
 	 */
-	return kvm_fast_pio_out(vcpu, size, port) && ret;
+	if (in)
+		return kvm_fast_pio_in(vcpu, size, port) && ret;
+	else
+		return kvm_fast_pio_out(vcpu, size, port) && ret;
 }
 
 static void