Skip to contentSkip to conversation

x86: svm: use kvm_fast_pio_in()

PATCH Superseded Joel Schopp 2 participants 3 messages Mar 2, 2015

Subsystems (3)
x86 KVM KVM x86

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Joel Schopp

author
#0 lore raw

PATCH v2 1/1 x86: svm: use kvm_fast_pio_in()

From: David Kaplan <[email protected]>

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 <[email protected]>
[extracted from larger unlrelated patch, forward ported, tested]
Signed-off-by Joel Schopp <[email protected]>
---
 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(-)
arch/x86/include/asm/kvm_host.h +1 -0
index a236e39..b976824 100644
@@ -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);
arch/x86/kvm/svm.c +3 -1
index d319e0c..f8c906b 100644
@@ -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);
 }
arch/x86/kvm/x86.c +33 -0
index bd7a70b..089247c 100644
@@ -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);
#1 lore raw

2015-03-02 10:40-0600, Joel Schopp:

Quoted 17 lines

From: David Kaplan <[email protected]>

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 <[email protected]>
[extracted from larger unlrelated patch, forward ported, tested]
Signed-off-by Joel Schopp <[email protected]>

---

arch/x86/kvm/svm.c +0 -0
index d319e0c..f8c906b 100644

arch/x86/kvm/svm.c static int io_interception(struct vcpu_svm *svm) · line 1902

Quoted 5 lines
arch/x86/kvm/svm.c +1 -1
@@ -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 ...)

arch/x86/kvm/svm.c static int io_interception(struct vcpu_svm *svm) · line 1912

Quoted 10 lines
arch/x86/kvm/svm.c +2 -0
 		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.)

arch/x86/kvm/x86.c int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port) · line 5468

Quoted 13 lines
arch/x86/kvm/svm.c +0 -0
 }
arch/x86/kvm/x86.c +3 -0
index bd7a70b..089247c 100644
@@ -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.

arch/x86/kvm/x86.c int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port) · line 5473

Quoted 5 lines
arch/x86/kvm/x86.c +5 -0
+
+	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().

arch/x86/kvm/x86.c int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port) · line 5476

Quoted 3 lines
arch/x86/kvm/x86.c +3 -0
+
+	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.)

Quoted 5 lines
arch/x86/kvm/x86.c +5 -0
+	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);
+
+	vcpu->arch.pio.count = 0;
+	return 1;
+}

arch/x86/kvm/x86.c int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port) · line 5491

Quoted 10 lines
arch/x86/kvm/x86.c +10 -0
+
+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.)

Quoted 11 lines
arch/x86/kvm/x86.c +7 -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);
author
#2 lore raw
Quoted 9 lines
Quoted 9 lines

  		return emulate_instruction(vcpu, 0) == EMULATE_DONE;
  
  	port = io_info >> 16;
arch/x86/kvm/svm.c +0 -0
@@ -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);
Quoted 4 lines

arch/x86/kvm/svm.c static int io_interception(struct vcpu_svm *svm) · line 1912

Quoted 3 lines
arch/x86/kvm/svm.c +2 -0
+	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.

Quoted 11 lines
Quoted 11 lines
(quoted diff) +0 -0
  }
arch/x86/kvm/x86.c +0 -0
index bd7a70b..089247c 100644
@@ -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);
Quoted 4 lines

arch/x86/kvm/x86.c int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port) · line 5468

Quoted 3 lines
arch/x86/kvm/x86.c +3 -0
+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.

Quoted 10 lines

arch/x86/kvm/x86.c

arch/x86/kvm/x86.c int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port) · line 5473

Quoted 5 lines
arch/x86/kvm/x86.c +5 -0
+
+	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.

Quoted 7 lines

arch/x86/kvm/x86.c int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port) · line 5476

Quoted 4 lines
arch/x86/kvm/x86.c +3 -0
+
+	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.

Quoted 17 lines
Quoted 6 lines
arch/x86/kvm/x86.c +5 -0
+	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);
+
+	vcpu->arch.pio.count = 0;
+	return 1;
+}

arch/x86/kvm/x86.c int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port) · line 5491

Quoted 10 lines
arch/x86/kvm/x86.c +10 -0
+
+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