[PATCH v2] x86: svm: use kvm_fast_pio_in()

[PATCH v2] x86: svm: use kvm_fast_pio_in()

[Joel Schopp]

From: David Kaplan <David.Kaplan@amd.com>

We can make the in instruction go faster the same way the out instruction is
already.

Changes from v1
	* Added kvm_fast_pio_in() implementation that was left out of v1

Signed-off-by: David Kaplan <David.Kaplan@amd.com>
[extracted from larger unlrelated patch, forward ported, tested]
Signed-off-by: Joel Schopp <joel.schopp@amd.com>
---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/svm.c              |    4 +++-
 arch/x86/kvm/x86.c              |   33 +++++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index a236e39..b976824 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -931,6 +931,7 @@ int kvm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr);
 struct x86_emulate_ctxt;
 
 int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port);
+int kvm_fast_pio_in(struct kvm_vcpu *vcpu, int size, unsigned short port);
 void kvm_emulate_cpuid(struct kvm_vcpu *vcpu);
 int kvm_emulate_halt(struct kvm_vcpu *vcpu);
 int kvm_emulate_wbinvd(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d319e0c..f8c906b 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1899,7 +1899,7 @@ static int io_interception(struct vcpu_svm *svm)
 	++svm->vcpu.stat.io_exits;
 	string = (io_info & SVM_IOIO_STR_MASK) != 0;
 	in = (io_info & SVM_IOIO_TYPE_MASK) != 0;
-	if (string || in)
+	if (string)
 		return emulate_instruction(vcpu, 0) == EMULATE_DONE;
 
 	port = io_info >> 16;
@@ -1907,6 +1907,8 @@ static int io_interception(struct vcpu_svm *svm)
 	svm->next_rip = svm->vmcb->control.exit_info_2;
 	skip_emulated_instruction(&svm->vcpu);
 
+	if (in)
+		return kvm_fast_pio_in(vcpu, size, port);
 	return kvm_fast_pio_out(vcpu, size, port);
 }
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index bd7a70b..089247c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5463,6 +5463,39 @@ int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port)
 }
 EXPORT_SYMBOL_GPL(kvm_fast_pio_out);
 
+static int complete_fast_pio(struct kvm_vcpu *vcpu)
+{
+	u32 new_rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
+
+	if (!vcpu->arch.pio.count)
+		return 0;
+	if (vcpu->arch.pio.count * vcpu->arch.pio.size > 8)
+		return 0;
+
+	memcpy(&new_rax, vcpu->arch.pio_data,
+	       vcpu->arch.pio.count * vcpu->arch.pio.size);
+	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);
+
+	vcpu->arch.pio.count = 0;
+	return 1;
+}
+
+int kvm_fast_pio_in(struct kvm_vcpu *vcpu, int size, unsigned short port)
+{
+	unsigned long val;
+	int ret = emulator_pio_in_emulated(&vcpu->arch.emulate_ctxt, size,
+					   port, &val, 1);
+
+	if (ret) {
+		kvm_register_write(vcpu, VCPU_REGS_RAX, val);
+		vcpu->arch.pio.count = 0;
+	} else
+		vcpu->arch.complete_userspace_io = complete_fast_pio;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kvm_fast_pio_in);
+
 static void tsc_bad(void *info)
 {
 	__this_cpu_write(cpu_tsc_khz, 0);

Re: [PATCH v2] x86: svm: use kvm_fast_pio_in()

[Radim Krčmář]

2015-03-02 10:40-0600, Joel Schopp:
> From: David Kaplan <David.Kaplan@amd.com>
> 
> We can make the in instruction go faster the same way the out instruction is
> already.
> 
> Changes from v1
> 	* Added kvm_fast_pio_in() implementation that was left out of v1
> 
> Signed-off-by: David Kaplan <David.Kaplan@amd.com>
> [extracted from larger unlrelated patch, forward ported, tested]
> Signed-off-by: Joel Schopp <joel.schopp@amd.com>
> ---
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index d319e0c..f8c906b 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1899,7 +1899,7 @@ static int io_interception(struct vcpu_svm *svm)
>  	++svm->vcpu.stat.io_exits;
>  	string = (io_info & SVM_IOIO_STR_MASK) != 0;
>  	in = (io_info & SVM_IOIO_TYPE_MASK) != 0;
> -	if (string || in)
> +	if (string)

(I guess that most accesses are covered now, so we don't need to make
 REP case a bit faster ...)

>  		return emulate_instruction(vcpu, 0) == EMULATE_DONE;
>  
>  	port = io_info >> 16;
> @@ -1907,6 +1907,8 @@ static int io_interception(struct vcpu_svm *svm)
>  	svm->next_rip = svm->vmcb->control.exit_info_2;
>  	skip_emulated_instruction(&svm->vcpu);
>  
> +	if (in)
> +		return kvm_fast_pio_in(vcpu, size, port);
>  	return kvm_fast_pio_out(vcpu, size, port);

(kvm_fast_pio() comes to mind.)

>  }
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index bd7a70b..089247c 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5463,6 +5463,39 @@ int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port)
>  }
>  EXPORT_SYMBOL_GPL(kvm_fast_pio_out);
>  
> +static int complete_fast_pio(struct kvm_vcpu *vcpu)
> +{
> +	u32 new_rax = kvm_register_read(vcpu, VCPU_REGS_RAX);

u64.

> +
> +	if (!vcpu->arch.pio.count)
> +		return 0;
> +	if (vcpu->arch.pio.count * vcpu->arch.pio.size > 8)
> +		return 0;

sizeof(new_rax).  (safer and easier to understand)

Both should never happen in KVM code, BUG_ON().

> +
> +	memcpy(&new_rax, vcpu->arch.pio_data,
> +	       vcpu->arch.pio.count * vcpu->arch.pio.size);

Use emulator_pio_in_emulated() here, for code sharing.
(We want to trace the read here too;  it could be better to split
 the path from emulator_pio_in_emulated() first.)

> +	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);
> +
> +	vcpu->arch.pio.count = 0;
> +	return 1;
> +}
> +
> +int kvm_fast_pio_in(struct kvm_vcpu *vcpu, int size, unsigned short port)
> +{
> +	unsigned long val;
> +	int ret = emulator_pio_in_emulated(&vcpu->arch.emulate_ctxt, size,
> +					   port, &val, 1);
> +
> +	if (ret) {
> +		kvm_register_write(vcpu, VCPU_REGS_RAX, val);
> +		vcpu->arch.pio.count = 0;

(emulator_pio_in_emulated() sets count to zero if it returns true.)

> +	} else
> +		vcpu->arch.complete_userspace_io = complete_fast_pio;
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(kvm_fast_pio_in);
> +
>  static void tsc_bad(void *info)
>  {
>  	__this_cpu_write(cpu_tsc_khz, 0);
> 

Re: [PATCH v2] x86: svm: use kvm_fast_pio_in()

[Joel Schopp]

>
>>   		return emulate_instruction(vcpu, 0) == EMULATE_DONE;
>>   
>>   	port = io_info >> 16;
>> @@ -1907,6 +1907,8 @@ static int io_interception(struct vcpu_svm *svm)
>>   	svm->next_rip = svm->vmcb->control.exit_info_2;
>>   	skip_emulated_instruction(&svm->vcpu);
>>   
>> +	if (in)
>> +		return kvm_fast_pio_in(vcpu, size, port);
>>   	return kvm_fast_pio_out(vcpu, size, port);
> (kvm_fast_pio() comes to mind.)
If you combined them you'd have to have an extra argument to say if it 
was in or out. You'd then have to have code to parse that.  I prefer 
this way.

>
>>   }
>>   
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index bd7a70b..089247c 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -5463,6 +5463,39 @@ int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port)
>>   }
>>   EXPORT_SYMBOL_GPL(kvm_fast_pio_out);
>>   
>> +static int complete_fast_pio(struct kvm_vcpu *vcpu)
>> +{
>> +	u32 new_rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
> u64.
Good call.  I'll use unsigned long like kvm_fast_pio_out() uses.

> arch/x86/kvm/x86.c
>
>> +
>> +	if (!vcpu->arch.pio.count)
>> +		return 0;
>> +	if (vcpu->arch.pio.count * vcpu->arch.pio.size > 8)
>> +		return 0;
> sizeof(new_rax).  (safer and easier to understand)
>
> Both should never happen in KVM code, BUG_ON().
Agreed on both counts.

>
>> +
>> +	memcpy(&new_rax, vcpu->arch.pio_data,
>> +	       vcpu->arch.pio.count * vcpu->arch.pio.size);
> Use emulator_pio_in_emulated() here, for code sharing.
> (We want to trace the read here too;  it could be better to split
>   the path from emulator_pio_in_emulated() first.)
I looked at pulling this out, it was a painful.  I'll add the trace hook.

>
>> +	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);
>> +
>> +	vcpu->arch.pio.count = 0;
>> +	return 1;
>> +}
>> +
>> +int kvm_fast_pio_in(struct kvm_vcpu *vcpu, int size, unsigned short port)
>> +{
>> +	unsigned long val;
>> +	int ret = emulator_pio_in_emulated(&vcpu->arch.emulate_ctxt, size,
>> +					   port, &val, 1);
>> +
>> +	if (ret) {
>> +		kvm_register_write(vcpu, VCPU_REGS_RAX, val);
>> +		vcpu->arch.pio.count = 0;
> (emulator_pio_in_emulated() sets count to zero if it returns true.)
will remove = 0 line