x86: svm: use kvm_fast_pio_in()

PATCH In review Joel Schopp 4 participants 9 messages Mar 2, 2015 – Apr 7, 2015

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Joel Schopp

author

2015-03-02 20:58 UTC #0 lore raw

PATCH v3 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 v2[Joel]:
* changed rax from u32 to unsigned long
* changed a couple return 0 to BUG_ON()
* changed 8 to sizeof(new_rax)
* added trace hook
* removed redundant clearing of count
Changes from v1[Joel]
* 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, addressed reviews, 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              |   30 ++++++++++++++++++++++++++++++
 3 files changed, 34 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 +30 -0

index bd7a70b..d05efaf 100644
@@ -5463,6 +5463,36 @@ 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)
+{
+	unsigned long new_rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
+
+	BUG_ON(!vcpu->arch.pio.count);
+	BUG_ON(vcpu->arch.pio.count * vcpu->arch.pio.size > sizeof(new_rax));
+
+	memcpy(&new_rax, vcpu, sizeof(new_rax));
+	trace_kvm_pio(KVM_PIO_IN, vcpu->arch.pio.port, vcpu->arch.pio.size,
+		      vcpu->arch.pio.count, vcpu->arch.pio_data);
+	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);
+	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);

Radim Krčmář

2015-03-03 16:42 UTC #1 lore raw

2015-03-02 15:02-0600, Joel Schopp:

Quoted 4 lines

From: David Kaplan <[email protected]>
We can make the in instruction go faster the same way the out instruction is already.

(How much faster do benchmarks run?)

Quoted 15 lines

Changes from v2[Joel]:
* changed rax from u32 to unsigned long
* changed a couple return 0 to BUG_ON()
* changed 8 to sizeof(new_rax)
* added trace hook
* removed redundant clearing of count
Changes from v1[Joel]
* 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, addressed reviews, tested]
Signed-off-by Joel Schopp <[email protected]>
---

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

Quoted 4 lines

arch/x86/kvm/x86.c +1 -0

@@ -5463,6 +5463,36 @@ 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)

(complete_fast_pio_in()?)

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

Quoted 2 lines

arch/x86/kvm/x86.c +2 -0

+{
+	unsigned long new_rax = kvm_register_read(vcpu, VCPU_REGS_RAX);

Shouldn't we handle writes in EAX differently than in AX and AL, because of implicit zero extension.

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

Quoted 3 lines

arch/x86/kvm/x86.c +3 -0

+
+	BUG_ON(!vcpu->arch.pio.count);
+	BUG_ON(vcpu->arch.pio.count * vcpu->arch.pio.size > sizeof(new_rax));

(Looking at it again, a check for 'vcpu->arch.pio.count == 1' would be
sufficient.)

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

Quoted 6 lines

arch/x86/kvm/x86.c +6 -0

+
+	memcpy(&new_rax, vcpu, sizeof(new_rax));
+	trace_kvm_pio(KVM_PIO_IN, vcpu->arch.pio.port, vcpu->arch.pio.size,
+		      vcpu->arch.pio.count, vcpu->arch.pio_data);
+	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);
+	vcpu->arch.pio.count = 0;

I think it is better to call emulator_pio_in_emulated directly, like

   	emulator_pio_in_out(&vcpu->arch.emulate_ctxt, vcpu->arch.pio.size,
   			vcpu->arch.pio.port, &new_rax, 1);
   	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);

because we know that vcpu->arch.pio.count != 0.

Refactoring could avoid the weird vcpu->ctxt->vcpu conversion. (A better name is always welcome.)

---

arch/x86/kvm/x86.c +11 ~~-10~~

index 96a8333f3db0..d0e5b086f2e1 100644
@@ -4663,22 +4663,23 @@ static int emulator_pio_in_out(struct kvm_vcpu *vcpu, int size,
 	return 0;
 }
 
+static void emulator_complete_pio_in(struct kvm_vcpu *vcpu, int size,
+		unsigned short port, void *val, unsigned int count)
+{
+	memcpy(val, vcpu->arch.pio_data, size * count);
+	trace_kvm_pio(KVM_PIO_IN, port, size, count, vcpu->arch.pio_data);
+	vcpu->arch.pio.count = 0;
+}
+
 static int emulator_pio_in_emulated(struct x86_emulate_ctxt *ctxt,
 				    int size, unsigned short port, void *val,
 				    unsigned int count)
 {
 	struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
-	int ret;
 
-	if (vcpu->arch.pio.count)
-		goto data_avail;
-
-	ret = emulator_pio_in_out(vcpu, size, port, val, count, true);
-	if (ret) {
-data_avail:
-		memcpy(val, vcpu->arch.pio_data, size * count);
-		trace_kvm_pio(KVM_PIO_IN, port, size, count, vcpu->arch.pio_data);
-		vcpu->arch.pio.count = 0;
+	if (vcpu->arch.pio.count ||
+	    emulator_pio_in_out(vcpu, size, port, val, count, true)) {
+		emulator_complete_pio_in(vcpu, size, port, val, count);
 		return 1;
 	}

Joel Schopp

author

2015-03-03 19:48 UTC #2 lore raw

Thank you for your detailed review on several of my patches.

Quoted 3 lines

Quoted 2 lines
+static int complete_fast_pio(struct kvm_vcpu *vcpu)
(complete_fast_pio_in()?)

If I do a v4 I'll adopt that name.

Quoted 4 lines

Review
Quoted 2 lines
(quoted diff) +2 -0
+{
+	unsigned long new_rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
Shouldn't we handle writes in EAX differently than in AX and AL, because of implicit zero extension.

I don't think the implicit zero extension hurts us here, but maybe there is something I'm missing that I need understand. Could you explain this further?

Quoted 6 lines

Review
Quoted 4 lines
(quoted diff) +3 -0
+
+	BUG_ON(!vcpu->arch.pio.count);
+	BUG_ON(vcpu->arch.pio.count * vcpu->arch.pio.size > sizeof(new_rax));
(Looking at it again, a check for 'vcpu->arch.pio.count == 1' would be
sufficient.)

I prefer the checks that are there now after your last review, especially since surrounded by BUG_ON they only run on debug kernels.

Quoted 9 lines

Review

Quoted 7 lines

(quoted diff) +6 -0

+
+	memcpy(&new_rax, vcpu, sizeof(new_rax));
+	trace_kvm_pio(KVM_PIO_IN, vcpu->arch.pio.port, vcpu->arch.pio.size,
+		      vcpu->arch.pio.count, vcpu->arch.pio_data);
+	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);
+	vcpu->arch.pio.count = 0;

I think it is better to call emulator_pio_in_emulated directly, like

Quoted 5 lines

   	emulator_pio_in_out(&vcpu->arch.emulate_ctxt, vcpu->arch.pio.size,
   			vcpu->arch.pio.port, &new_rax, 1);
   	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);

because we know that vcpu->arch.pio.count != 0.

I think two extra lines of code in my patch vs your suggestion are worth it to a) reduce execution path length b) increase readability c) avoid breaking the abstraction by not checking the return code d) avoid any future bugs introduced by changes the function that would return a value other than 1.

Quoted 3 lines

Refactoring could avoid the weird vcpu->ctxt->vcpu conversion. (A better name is always welcome.)

The pointer chasing is making me dizzy. I'm not sure why emulator_pio_in_emulated takes a x86_emulate_ctxt when all it does it immediately translate that to a vcpu and never use the x86_emulate_ctxt, why not pass the vcpu in the first place?

Radim Krčmář

2015-03-03 20:42 UTC #3 lore raw

2015-03-03 13:48-0600, Joel Schopp:

Quoted 6 lines

Quoted 3 lines
Review
Quoted 1 line
(quoted diff) +1 -0
+	unsigned long new_rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
Shouldn't we handle writes in EAX differently than in AX and AL, because of implicit zero extension.
I don't think the implicit zero extension hurts us here, but maybe there is something I'm missing that I need understand. Could you explain this further?

According to APM vol.2, 2.5.3 Operands and Results, when using EAX, we should zero upper 32 bits of RAX:

Zero Extension of Results. In 64-bit mode, when performing 32-bit operations with a GPR destination, the processor zero-extends the 32-bit result into the full 64-bit destination. Both 8-bit and 16-bit operations on GPRs preserve all unwritten upper bits of the destination GPR. This is consistent with legacy 16-bit and 32-bit semantics for partial-width results.

Is IN not covered?

Quoted 6 lines

Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2 -0
+	BUG_ON(!vcpu->arch.pio.count);
+	BUG_ON(vcpu->arch.pio.count * vcpu->arch.pio.size > sizeof(new_rax));
(Looking at it again, a check for 'vcpu->arch.pio.count == 1' would be
sufficient.)
I prefer the checks that are there now after your last review, especially since surrounded by BUG_ON they only run on debug kernels.

BUG_ON is checked on essentially all kernels that run KVM. (All distribution-based configs should have it.)

If we wanted to validate the size, then this is strictly better:
BUG_ON(vcpu->arch.pio.count != 1 || vcpu->arch.pio.size > sizeof(new_rax))

Quoted 11 lines

Review

Quoted 5 lines

(quoted diff) +5 -0

+	memcpy(&new_rax, vcpu, sizeof(new_rax));
+	trace_kvm_pio(KVM_PIO_IN, vcpu->arch.pio.port, vcpu->arch.pio.size,
+		      vcpu->arch.pio.count, vcpu->arch.pio_data);
+	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);
+	vcpu->arch.pio.count = 0;

I think it is better to call emulator_pio_in_emulated directly, like

   	emulator_pio_in_out(&vcpu->arch.emulate_ctxt, vcpu->arch.pio.size,
   			vcpu->arch.pio.port, &new_rax, 1);
   	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);

Quoted 6 lines

Quoted 1 line
because we know that vcpu->arch.pio.count != 0.
I think two extra lines of code in my patch vs your suggestion are worth it to a) reduce execution path length b) increase readability c) avoid breaking the abstraction by not checking the return code d) avoid any future bugs introduced by changes the function that would return a value other than 1.

True, it is horrible, the attached patch should have addressed (c) and (d), and it could be inlined to match (a).

Pasting the same code creates bug opportunities when we forget to modify all places. This class of problems can be harder to deal with, that (c) and (d), because we can't simply print all callers.

Quoted 6 lines

Quoted 2 lines
Refactoring could avoid the weird vcpu->ctxt->vcpu conversion. (A better name is always welcome.)
The pointer chasing is making me dizzy. I'm not sure why emulator_pio_in_emulated takes a x86_emulate_ctxt when all it does it immediately translate that to a vcpu and never use the x86_emulate_ctxt, why not pass the vcpu in the first place?

It is a part of x86_emulate_ops, where ctxt is more important ...

Paolo Bonzini

maintainer

2015-04-07 12:55 UTC #4 lore raw

Quoted 11 lines · Radim Krčmář

2015-03-03 13:48-0600, Joel Schopp:
Quoted 6 lines
Quoted 3 lines
Review
Quoted 1 line
(quoted diff) +1 -0
+	unsigned long new_rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
Shouldn't we handle writes in EAX differently than in AX and AL, because of implicit zero extension.
I don't think the implicit zero extension hurts us here, but maybe there is something I'm missing that I need understand. Could you explain this further?
According to APM vol.2, 2.5.3 Operands and Results, when using EAX, we should zero upper 32 bits of RAX:

Quoted 8 lines

Zero Extension of Results. In 64-bit mode, when performing 32-bit operations with a GPR destination, the processor zero-extends the 32-bit result into the full 64-bit destination. Both 8-bit and 16-bit operations on GPRs preserve all unwritten upper bits of the destination GPR. This is consistent with legacy 16-bit and 32-bit semantics for partial-width results.
Is IN not covered?

It is. You need to zero the upper 32 bits.

Quoted 9 lines

Quoted 6 lines
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2 -0
+	BUG_ON(!vcpu->arch.pio.count);
+	BUG_ON(vcpu->arch.pio.count * vcpu->arch.pio.size > sizeof(new_rax));
(Looking at it again, a check for 'vcpu->arch.pio.count == 1' would be
sufficient.)
I prefer the checks that are there now after your last review, especially since surrounded by BUG_ON they only run on debug kernels.
BUG_ON is checked on essentially all kernels that run KVM. (All distribution-based configs should have it.)

Correct.

Quoted 2 lines

If we wanted to validate the size, then this is strictly better:
BUG_ON(vcpu->arch.pio.count != 1 || vcpu->arch.pio.size > sizeof(new_rax))

That would be a very weird assertion considering that vcpu->arch.pio.size will architecturally be at most 4.

The first arm of the || is sufficient.

Quoted 11 lines

Review

Quoted 5 lines

(quoted diff) +5 -0

+	memcpy(&new_rax, vcpu, sizeof(new_rax));
+	trace_kvm_pio(KVM_PIO_IN, vcpu->arch.pio.port, vcpu->arch.pio.size,
+		      vcpu->arch.pio.count, vcpu->arch.pio_data);
+	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);
+	vcpu->arch.pio.count = 0;

I think it is better to call emulator_pio_in_emulated directly, like

   	emulator_pio_in_out(&vcpu->arch.emulate_ctxt, vcpu->arch.pio.size,
   			vcpu->arch.pio.port, &new_rax, 1);
   	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);

Quoted 5 lines

Quoted 1 line
Quoted 1 line
because we know that vcpu->arch.pio.count != 0.
Pasting the same code creates bug opportunities when we forget to modify all places. This class of problems can be harder to deal with, that (c) and (d), because we can't simply print all callers.

I agree with this and prefer calling emulator_pio_in_emulated in complete_fast_pio_in, indeed.

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Refactoring could avoid the weird vcpu->ctxt->vcpu conversion. (A better name is always welcome.)

No need for that.

Quoted 4 lines

Quoted 4 lines
The pointer chasing is making me dizzy. I'm not sure why emulator_pio_in_emulated takes a x86_emulate_ctxt when all it does it immediately translate that to a vcpu and never use the x86_emulate_ctxt, why not pass the vcpu in the first place?

Because the emulator is written to be usable outside the Linux kernel as well.

Also, the fast path (used if kernel_pio returns 0) doesn't read VCPU_REGS_RAX, thus using an uninitialized variable here:

Quoted 6 lines

Review

Quoted 6 lines

(quoted diff) +6 -0

+	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);

Thanks,

Paolo

Radim Krčmář

2015-03-03 16:44 UTC #5 lore raw

In reply to Joel Schopp

2015-03-02 15:02-0600, Joel Schopp:

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

Quoted 6 lines

arch/x86/kvm/x86.c +6 -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);
+

Btw. does this return 1 in some scenario?

Quoted 8 lines

arch/x86/kvm/x86.c +8 -0

+	if (ret)
+		kvm_register_write(vcpu, VCPU_REGS_RAX, val);
+	else
+		vcpu->arch.complete_userspace_io = complete_fast_pio;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kvm_fast_pio_in);

Joel Schopp

author

2015-03-03 20:03 UTC #6 lore raw

Quoted 8 lines · Radim Krčmář

2015-03-02 15:02-0600, Joel Schopp:
arch/x86/kvm/x86.c int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port) · line 5486
Quoted 6 lines
arch/x86/kvm/x86.c +6 -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);
+
Btw. does this return 1 in some scenario?

If a function returns a value it is always a good idea to check it and act appropriately. That said...
emulator_pio_in_emulated will return 1 if emulator_pio_in_out returns 1 or if vcpu->arch.pio.count != 0
emulator_pio_in_out returns 1 if kernel_pio returns 0 kernel_pio returns 0 if kvm_io_bus_read returns 0

Radim Krčmář

2015-03-03 20:44 UTC #7 lore raw

2015-03-03 14:03-0600, Joel Schopp:

Quoted 8 lines · Radim Krčmář

Quoted 4 lines
2015-03-02 15:02-0600, Joel Schopp:
arch/x86/kvm/x86.c int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port) · line 5485
Quoted 2 lines
arch/x86/kvm/x86.c +2 -0
+	int ret = emulator_pio_in_emulated(&vcpu->arch.emulate_ctxt, size,
+					   port, &val, 1);
Btw. does this return 1 in some scenario?
If a function returns a value it is always a good idea to check it and act appropriately. That said...
emulator_pio_in_emulated will return 1 if emulator_pio_in_out returns 1

Ah, I have completely forgotten, thanks!

Marcelo Tosatti

2015-03-13 00:47 UTC #8 lore raw

In reply to Joel Schopp

Quoted 63 lines · Joel Schopp

From: David Kaplan <[email protected]>

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

Signed-off-by David Kaplan <[email protected]>

[extracted from larger unlrelated patch, forward ported, addressed reviews, 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              |   30 ++++++++++++++++++++++++++++++
 3 files changed, 34 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 +0 -0

index bd7a70b..d05efaf 100644
@@ -5463,6 +5463,36 @@ int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port)
 }

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

Quoted 10 lines

arch/x86/kvm/x86.c +8 -0

 EXPORT_SYMBOL_GPL(kvm_fast_pio_out);
 
+static int complete_fast_pio(struct kvm_vcpu *vcpu)
+{
+	unsigned long new_rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
+
+	BUG_ON(!vcpu->arch.pio.count);
+	BUG_ON(vcpu->arch.pio.count * vcpu->arch.pio.size > sizeof(new_rax));
+
+	memcpy(&new_rax, vcpu, sizeof(new_rax));

Weird.

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

Quoted 10 lines

arch/x86/kvm/x86.c +10 -0

+	trace_kvm_pio(KVM_PIO_IN, vcpu->arch.pio.port, vcpu->arch.pio.size,
+		      vcpu->arch.pio.count, vcpu->arch.pio_data);
+	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;

Please zero initialize val.
Please check sanity of size.