KVM: nVMX: Improve I/O exit handling

Commit Message

Jan Kiszka Feb. 10, 2013, 8:42 p.m. UTC

From: Jan Kiszka <[e-mail]>

This prevents trapping L2 I/O exits if L1 has neither unconditional nor
bitmap-based exiting enabled. Furthermore, it implements basic I/O
bitmap handling. Repeated string accesses are still reported to L1
unconditionally for now.

Signed-off-by: Jan Kiszka <[e-mail]>
---

If someone tells me how to figure out the effective I/O access range
with rep ins/outs in all possible CPU modes in three lines, I'll
complete this patch. For now I had no use and was too lazy.

 arch/x86/kvm/vmx.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 52 insertions(+), 2 deletions(-)

Comments

Nadav Har'El Feb. 11, 2013, 10:07 a.m. UTC | #1

On Sun, Feb 10, 2013, Jan Kiszka wrote about "[PATCH] KVM: nVMX: Improve I/O exit handling":
> +static bool nested_vmx_exit_handled_io(struct kvm_vcpu *vcpu,
> +				       struct vmcs12 *vmcs12)
> +{
> +	unsigned long exit_qualification;
> +	gpa_t bitmap, last_bitmap;
> +	bool string, rep;
> +	u16 port;
> +	int size;
> +	u8 b;
> +
> +	if (nested_cpu_has(get_vmcs12(vcpu), CPU_BASED_UNCOND_IO_EXITING))
> +		return 1;

Instead of calling get_vmcs12(vcpu), you can just use "vmcs12" variable
which you already have. I see I left the same redundant call also in
nested_vmx_exit_handled_msr :(

> +		if (port < 0x8000)
> +			bitmap = vmcs12->io_bitmap_a;
> +		else
> +			bitmap = vmcs12->io_bitmap_b;
> +		bitmap += port / 8;

In the port >= 0x8000, I believe need to subtract 0x8000 from the port
number before using it as an offset into io_bitmap_b?

Nadav.

Jan Kiszka Feb. 11, 2013, 10:16 a.m. UTC | #2

On 2013-02-11 11:07, Nadav Har'El wrote:
> On Sun, Feb 10, 2013, Jan Kiszka wrote about "[PATCH] KVM: nVMX: Improve I/O exit handling":
>> +static bool nested_vmx_exit_handled_io(struct kvm_vcpu *vcpu,
>> +				       struct vmcs12 *vmcs12)
>> +{
>> +	unsigned long exit_qualification;
>> +	gpa_t bitmap, last_bitmap;
>> +	bool string, rep;
>> +	u16 port;
>> +	int size;
>> +	u8 b;
>> +
>> +	if (nested_cpu_has(get_vmcs12(vcpu), CPU_BASED_UNCOND_IO_EXITING))
>> +		return 1;
> 
> Instead of calling get_vmcs12(vcpu), you can just use "vmcs12" variable
> which you already have. I see I left the same redundant call also in
> nested_vmx_exit_handled_msr :(

Indeed, copy&pasted from there...

> 
>> +		if (port < 0x8000)
>> +			bitmap = vmcs12->io_bitmap_a;
>> +		else
>> +			bitmap = vmcs12->io_bitmap_b;
>> +		bitmap += port / 8;
> 
> In the port >= 0x8000, I believe need to subtract 0x8000 from the port
> number before using it as an offset into io_bitmap_b?

Oops. There was already a stupid bug in that path, failed to validate it
again. v2 will follow.

Thanks for reviewing,
Jan

Patch

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index fe9a9cf..056bd95 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5913,6 +5913,57 @@  static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
 static const int kvm_vmx_max_exit_handlers =
 	ARRAY_SIZE(kvm_vmx_exit_handlers);
 
+static bool nested_vmx_exit_handled_io(struct kvm_vcpu *vcpu,
+				       struct vmcs12 *vmcs12)
+{
+	unsigned long exit_qualification;
+	gpa_t bitmap, last_bitmap;
+	bool string, rep;
+	u16 port;
+	int size;
+	u8 b;
+
+	if (nested_cpu_has(get_vmcs12(vcpu), CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(get_vmcs12(vcpu), CPU_BASED_USE_IO_BITMAPS))
+		return 0;
+
+	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
+
+	string = exit_qualification & 16;
+	rep = exit_qualification & 32;
+
+	/* TODO: interpret instruction and check range against bitmap */
+	if (string && rep)
+		return 1;
+
+	port = exit_qualification >> 16;
+	size = (exit_qualification & 7) + 1;
+
+	last_bitmap = (gpa_t)-1;
+	b = -1;
+
+	while (size > 0) {
+		if (port < 0x8000)
+			bitmap = vmcs12->io_bitmap_a;
+		else
+			bitmap = vmcs12->io_bitmap_b;
+		bitmap += port / 8;
+
+		if (last_bitmap != bitmap)
+			kvm_read_guest(vcpu->kvm, bitmap, &b, 1);
+		if (b & (1 >> (port & 7)))
+			return 1;
+
+		port++;
+		size--;
+		last_bitmap = bitmap;
+	}
+
+	return 0;
+}
+
 /*
  * Return 1 if we should exit from L2 to L1 to handle an MSR access access,
  * rather than handle it ourselves in L0. I.e., check whether L1 expressed
@@ -6102,8 +6153,7 @@  static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
 	case EXIT_REASON_DR_ACCESS:
 		return nested_cpu_has(vmcs12, CPU_BASED_MOV_DR_EXITING);
 	case EXIT_REASON_IO_INSTRUCTION:
-		/* TODO: support IO bitmaps */
-		return 1;
+		return nested_vmx_exit_handled_io(vcpu, vmcs12);
 	case EXIT_REASON_MSR_READ:
 	case EXIT_REASON_MSR_WRITE:
 		return nested_vmx_exit_handled_msr(vcpu, vmcs12, exit_reason);