KVM: nVMX: Improve I/O exit handling

PATCH Superseded Jan Kiszka 5 participants 24 messages Feb 10, 2013 – Feb 19, 2013

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Jan Kiszka

author

2013-02-10 20:42 UTC #0 lore raw

PATCH 1/1 KVM: nVMX: Improve I/O exit handling

From: Jan Kiszka <[email protected]>

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 <[email protected]>

---

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(-)

arch/x86/kvm/vmx.c +52 -2

index fe9a9cf..056bd95 100644
@@ -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);

Signature · 1 line

1.7.3.4

Nadav Har'El

2013-02-11 10:21 UTC #1 lore raw

On Sun, Feb 10, 2013, Jan Kiszka wrote about "[PATCH] KVM: nVMX: Improve I/O exit handling":

arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5927

Quoted 12 lines

arch/x86/kvm/vmx.c +12 -0

+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 :(

arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5952

Quoted 5 lines

arch/x86/kvm/vmx.c +5 -0

+		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.

Signature · 4 lines

Nadav Har'El                        |         Monday, Feb 11 2013, 1 Adar 5773
[email protected]             |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |Attention: There will be a rain dance
http://nadav.harel.org.il           |Friday night, weather permitting.

Jan Kiszka

author

2013-02-11 10:16 UTC #2 lore raw

Quoted 17 lines · Nadav Har'El

On Sun, Feb 10, 2013, Jan Kiszka wrote about "[PATCH] KVM: nVMX: Improve I/O exit handling":
arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5927
Quoted 12 lines
arch/x86/kvm/vmx.c +12 -0
+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...

Quoted 9 lines

arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5952
Quoted 6 lines
arch/x86/kvm/vmx.c +5 -0
+		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

Jan Kiszka

2013-02-11 11:28 UTC #3 lore raw

PATCH v2 1/1 KVM: nVMX: Improve I/O exit handling

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- fix two brown-paper-bag bugs (offset for bitmap_b, wrong direction of mask shift)
- use vmcs12 argument instead of get_vmcs12

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

arch/x86/kvm/vmx.c +52 -2

index fe9a9cf..64e1233 100644
@@ -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(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, 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 & 0x7fff) / 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);

Signature · 1 line

1.7.3.4

Gleb Natapov

2013-02-14 09:33 UTC #4 lore raw

Quoted 54 lines · Jan Kiszka

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- fix two brown-paper-bag bugs (offset for bitmap_b, wrong direction of mask shift)
- use vmcs12 argument instead of get_vmcs12

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

arch/x86/kvm/vmx.c +30 -0

index fe9a9cf..64e1233 100644
@@ -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(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, 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;

arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5955

Quoted 10 lines

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

+
+	while (size > 0) {
+		if (port < 0x8000)
+			bitmap = vmcs12->io_bitmap_a;
+		else
+			bitmap = vmcs12->io_bitmap_b;
+		bitmap += (port & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			kvm_read_guest(vcpu->kvm, bitmap, &b, 1);

Return value is ignored.

Quoted 26 lines

arch/x86/kvm/vmx.c +12 -2

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

Signature · 1 line

1.7.3.4

--
Gleb.

Jan Kiszka

2013-02-14 11:19 UTC #5 lore raw

Quoted 58 lines · Gleb Natapov

Quoted 57 lines · Jan Kiszka

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- fix two brown-paper-bag bugs (offset for bitmap_b, wrong direction of mask shift)
- use vmcs12 argument instead of get_vmcs12

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

arch/x86/kvm/vmx.c +33 -0

index fe9a9cf..64e1233 100644
@@ -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(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, 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)

Quoted 8 lines

arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5955
Quoted 7 lines
arch/x86/kvm/vmx.c +7 -0
+			bitmap = vmcs12->io_bitmap_a;
+		else
+			bitmap = vmcs12->io_bitmap_b;
+		bitmap += (port & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			kvm_read_guest(vcpu->kvm, bitmap, &b, 1);
Return value is ignored.

Not sure how to map a failure on real HW behaviour. I guess it's best to simply initialize b to -1 before each call, enforcing an exit on unaccessible bitmaps.

BTW, nested_vmx_exit_handled_msr needs some improvement in this regard, too.

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Gleb Natapov

2013-02-14 12:11 UTC #6 lore raw

Quoted 68 lines · Jan Kiszka

Quoted 58 lines · Gleb Natapov
Quoted 57 lines · Jan Kiszka
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 <[email protected]>
---
Changes in v2:
- fix two brown-paper-bag bugs (offset for bitmap_b, wrong direction of mask shift)
- use vmcs12 argument instead of get_vmcs12
 arch/x86/kvm/vmx.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 52 insertions(+), 2 deletions(-)
arch/x86/kvm/vmx.c +33 -0
index fe9a9cf..64e1233 100644
@@ -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(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, 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)
Quoted 8 lines
arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5955
Quoted 7 lines
arch/x86/kvm/vmx.c +7 -0
+			bitmap = vmcs12->io_bitmap_a;
+		else
+			bitmap = vmcs12->io_bitmap_b;
+		bitmap += (port & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			kvm_read_guest(vcpu->kvm, bitmap, &b, 1);
Return value is ignored.

Quoted 1 line

Not sure how to map a failure on real HW behaviour. I guess it's best to

Exit to L1 with nested_vmx_failValid() may be?

Quoted 3 lines

simply initialize b to -1 before each call, enforcing an exit on unaccessible bitmaps.

I'd just make it explicit:
if (kvm_read_guest())

    return 1;

Quoted 2 lines

BTW, nested_vmx_exit_handled_msr needs some improvement in this regard, too.

Yes, nested_vmx_exit_handled_msr() use uninitialized 'b' from the stack. We are leaking host kernel data to a guest here. Patch?

--
Gleb.

Jan Kiszka

2013-02-14 12:22 UTC #7 lore raw

Quoted 69 lines · Gleb Natapov

Quoted 68 lines · Jan Kiszka
Quoted 66 lines · Gleb Natapov
Quoted 54 lines · Jan Kiszka
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 <[email protected]>
---
Changes in v2:
- fix two brown-paper-bag bugs (offset for bitmap_b, wrong direction of mask shift)
- use vmcs12 argument instead of get_vmcs12
 arch/x86/kvm/vmx.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 52 insertions(+), 2 deletions(-)
arch/x86/kvm/vmx.c +30 -0
index fe9a9cf..64e1233 100644
@@ -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(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, 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;
arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5955
Quoted 10 lines
arch/x86/kvm/vmx.c +10 -0
+
+	while (size > 0) {
+		if (port < 0x8000)
+			bitmap = vmcs12->io_bitmap_a;
+		else
+			bitmap = vmcs12->io_bitmap_b;
+		bitmap += (port & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			kvm_read_guest(vcpu->kvm, bitmap, &b, 1);
Return value is ignored.

Quoted 2 lines

Quoted 1 line
Not sure how to map a failure on real HW behaviour. I guess it's best to
Exit to L1 with nested_vmx_failValid() may be?

To my understanding, nested_vmx_failValid/Invalid are related to errors directly related to vm instruction execution. This one is triggered by the guest later on.

Quoted 7 lines

Quoted 4 lines
simply initialize b to -1 before each call, enforcing an exit on unaccessible bitmaps.
I'd just make it explicit:
if (kvm_read_guest())
    return 1;

OK.

Quoted 5 lines

Quoted 3 lines
BTW, nested_vmx_exit_handled_msr needs some improvement in this regard, too.
Yes, nested_vmx_exit_handled_msr() use uninitialized 'b' from the stack. We are leaking host kernel data to a guest here. Patch?

Will follow.

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Gleb Natapov

2013-02-14 12:56 UTC #8 lore raw

Quoted 70 lines · Jan Kiszka

Quoted 69 lines · Gleb Natapov
Quoted 68 lines · Jan Kiszka
Quoted 66 lines · Gleb Natapov
Quoted 54 lines · Jan Kiszka
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 <[email protected]>
---
Changes in v2:
- fix two brown-paper-bag bugs (offset for bitmap_b, wrong direction of mask shift)
- use vmcs12 argument instead of get_vmcs12
 arch/x86/kvm/vmx.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 52 insertions(+), 2 deletions(-)
arch/x86/kvm/vmx.c +30 -0
index fe9a9cf..64e1233 100644
@@ -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(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, 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;
arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5955
Quoted 10 lines
arch/x86/kvm/vmx.c +10 -0
+
+	while (size > 0) {
+		if (port < 0x8000)
+			bitmap = vmcs12->io_bitmap_a;
+		else
+			bitmap = vmcs12->io_bitmap_b;
+		bitmap += (port & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			kvm_read_guest(vcpu->kvm, bitmap, &b, 1);
Return value is ignored.

Quoted 7 lines

Quoted 2 lines
Quoted 1 line
Not sure how to map a failure on real HW behaviour. I guess it's best to
Exit to L1 with nested_vmx_failValid() may be?
To my understanding, nested_vmx_failValid/Invalid are related to errors directly related to vm instruction execution. This one is triggered by the guest later on.

You are right. We need some kind of error vmexit here, but nothing appropriate is defined by the spec, so lets just assume that exit to L1 is needed if we cannot read permission bitmaps.

Quoted 11 lines

Quoted 7 lines
Quoted 4 lines
simply initialize b to -1 before each call, enforcing an exit on unaccessible bitmaps.
I'd just make it explicit:
if (kvm_read_guest())
    return 1;
OK.
Quoted 1 line

Quoted 12 lines

Quoted 4 lines
Quoted 2 lines
BTW, nested_vmx_exit_handled_msr needs some improvement in this regard, too.
Yes, nested_vmx_exit_handled_msr() use uninitialized 'b' from the stack. We are leaking host kernel data to a guest here. Patch?
Will follow.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

--
Gleb.

Nadav Har'El

2013-02-14 13:55 UTC #9 lore raw

On Thu, Feb 14, 2013, Gleb Natapov wrote about "Re: [PATCH v2] KVM: nVMX: Improve I/O exit handling":

Quoted 10 lines

Quoted 7 lines
Quoted 2 lines
Quoted 1 line
Not sure how to map a failure on real HW behaviour. I guess it's best to
Exit to L1 with nested_vmx_failValid() may be?
To my understanding, nested_vmx_failValid/Invalid are related to errors directly related to vm instruction execution. This one is triggered by the guest later on.
You are right. We need some kind of error vmexit here, but nothing appropriate is defined by the spec, so lets just assume that exit to L1 is needed if we cannot read permission bitmaps.

On real hardware, note that the MSR-bitmap address specified in the VMCS is a physical address, so there can never be an error - if I understand correctly, there is no such thing as a non-existant physical address - reading from non-existant physical memory returns random garbage (please correct me if I'm wrong here!). So if I'm correct, using a non-existent address for an MSR-bitmap will give you an undefined behavior - not any sort of entry error to special type of exit.

The current code, using a random value on the stack, fits with the "undefined behavior" definition, but you're right the more logical behavior is to, indeed, always exit on the MSR access when the bitmap cannot be read. This will make an unreadable bitmap equivalent to no bitmap at all - which I think makes most sense.

You're also right that this case is identical in both MSR and I/O bitmap cases, and should be fixed in both.

Signature · 4 lines

Nadav Har'El                        |       Thursday, Feb 14 2013, 4 Adar 5773
[email protected]             |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |A fanatic is one who can't change his
http://nadav.harel.org.il           |mind and won't change the subject.

Gleb Natapov

2013-02-14 14:44 UTC #10 lore raw

Quoted 12 lines · Nadav Har'El

On Thu, Feb 14, 2013, Gleb Natapov wrote about "Re: [PATCH v2] KVM: nVMX: Improve I/O exit handling":
Quoted 10 lines
Quoted 7 lines
Quoted 2 lines
Quoted 1 line
Not sure how to map a failure on real HW behaviour. I guess it's best to
Exit to L1 with nested_vmx_failValid() may be?
To my understanding, nested_vmx_failValid/Invalid are related to errors directly related to vm instruction execution. This one is triggered by the guest later on.
You are right. We need some kind of error vmexit here, but nothing appropriate is defined by the spec, so lets just assume that exit to L1 is needed if we cannot read permission bitmaps.

Quoted 8 lines

On real hardware, note that the MSR-bitmap address specified in the VMCS is a physical address, so there can never be an error - if I understand correctly, there is no such thing as a non-existant physical address - reading from non-existant physical memory returns random garbage (please correct me if I'm wrong here!). So if I'm correct, using a non-existent address for an MSR-bitmap will give you an undefined behavior - not any sort of entry error to special type of exit.

That's true for real HW. Reading from physical address outside of physical memory will either access some random device MMIO or nothing. Either way result is unpredictable and may hang the machine. I am fine with killing a guest that tries to do that.

Quoted 5 lines

The current code, using a random value on the stack, fits with the "undefined behavior" definition, but you're right the more logical behavior is to, indeed, always exit on the MSR access when the bitmap cannot be read. This will make an unreadable bitmap equivalent to no bitmap at all - which I think makes most sense.

Current case leaks data from host kernel, not just exhibit random behaviour.

Quoted 10 lines

You're also right that this case is identical in both MSR and I/O bitmap cases, and should be fixed in both.

Signature · 4 lines

Nadav Har'El                        |       Thursday, Feb 14 2013, 4 Adar 5773
[email protected]             |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |A fanatic is one who can't change his
http://nadav.harel.org.il           |mind and won't change the subject.

--
Gleb.

Jan Kiszka

2013-02-14 18:46 UTC #11 lore raw

PATCH v3 1/1 KVM: nVMX: Improve I/O exit handling

In reply to Gleb Natapov

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v3:
- trap unconditionally if bitmap access fails

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

arch/x86/kvm/vmx.c +53 -2

index 6667042..2633199 100644
@@ -5908,6 +5908,58 @@ 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(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, 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 & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			if (kvm_read_guest(vcpu->kvm, bitmap, &b, 1))
+				return 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

@@ -6097,8 +6149,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);

Signature · 1 line

1.7.3.4

Gleb Natapov

Reviewed-by on v3 1/1

2013-02-17 08:55 UTC #12 lore raw

Quoted 6 lines · Jan Kiszka

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 <[email protected]>

Reviewed-by Gleb Natapov <[email protected]>

Quoted 83 lines

---

Changes in v3:
- trap unconditionally if bitmap access fails

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

arch/x86/kvm/vmx.c +53 -2

index 6667042..2633199 100644
@@ -5908,6 +5908,58 @@ 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(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, 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 & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			if (kvm_read_guest(vcpu->kvm, bitmap, &b, 1))
+				return 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

@@ -6097,8 +6149,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);

Signature · 1 line

1.7.3.4

--
Gleb.

Jan Kiszka

author

2013-02-18 06:33 UTC #13 lore raw

In reply to Jan Kiszka

Quoted 36 lines · Jan Kiszka

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v3:
- trap unconditionally if bitmap access fails

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

arch/x86/kvm/vmx.c +14 -0

index 6667042..2633199 100644
@@ -5908,6 +5908,58 @@ 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(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))

arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5934

Quoted 10 lines

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

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

Nonsense, rep ins/outs always works against the same port. We can simply drop this check and be done with the feature. I'll come up with v4.

Jan

Quoted 42 lines

arch/x86/kvm/vmx.c +29 -2

+
+	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 & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			if (kvm_read_guest(vcpu->kvm, bitmap, &b, 1))
+				return 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

@@ -6097,8 +6149,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);

Jan Kiszka

author

2013-02-18 06:45 UTC #14 lore raw

PATCH v4 1/1 KVM: nVMX: Improve I/O exit handling

From: Jan Kiszka <[email protected]>

This prevents trapping L2 I/O exits if L1 has neither unconditional nor bitmap-based exiting enabled. Furthermore, it implements I/O bitmap handling.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v4:
- drop pointless check for rep+string - we are already done with this feature

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

arch/x86/kvm/vmx.c +45 -2

index 6667042..a6c4a32 100644
@@ -5908,6 +5908,50 @@ 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;
+	u16 port;
+	int size;
+	u8 b;
+
+	if (nested_cpu_has(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))
+		return 0;
+
+	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
+
+	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 & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			if (kvm_read_guest(vcpu->kvm, bitmap, &b, 1))
+				return 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

@@ -6097,8 +6141,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);

Signature · 1 line

1.7.3.4

Gleb Natapov

2013-02-18 08:44 UTC #15 lore raw

In reply to Jan Kiszka

Quoted 42 lines · Jan Kiszka

Quoted 41 lines · Jan Kiszka

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v3:
- trap unconditionally if bitmap access fails

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

arch/x86/kvm/vmx.c +19 -0

index 6667042..2633199 100644
@@ -5908,6 +5908,58 @@ 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(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))
+		return 0;
+
+	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
+
+	string = exit_qualification & 16;

Quoted 9 lines

arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5934
Quoted 5 lines
arch/x86/kvm/vmx.c +5 -0
+	rep = exit_qualification & 32;
+
+	/* TODO: interpret instruction and check range against bitmap */
+	if (string && rep)
+		return 1;
Nonsense, rep ins/outs always works against the same port. We can simply drop this check and be done with the feature. I'll come up with v4.

Actually this reminds me that we should check range of ports depending on operand size, not one port. But here is a catch, older cpus do not provide operand size as part of exit information.

Quoted 36 lines

Jan

Quoted 34 lines

arch/x86/kvm/vmx.c +28 -0

+
+	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 & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			if (kvm_read_guest(vcpu->kvm, bitmap, &b, 1))
+				return 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

@@ -6097,8 +6149,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);

Quoted 10 lines

arch/x86/kvm/vmx.c static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu) · line 6155

Quoted 8 lines

arch/x86/kvm/vmx.c +1 -2

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

--
Gleb.

Jan Kiszka

author

2013-02-18 08:53 UTC #16 lore raw

Quoted 52 lines · Gleb Natapov

Quoted 42 lines · Jan Kiszka
Quoted 41 lines · Jan Kiszka
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 <[email protected]>
---
Changes in v3:
- trap unconditionally if bitmap access fails
 arch/x86/kvm/vmx.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 53 insertions(+), 2 deletions(-)
arch/x86/kvm/vmx.c +19 -0
index 6667042..2633199 100644
@@ -5908,6 +5908,58 @@ 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(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))
+		return 0;
+
+	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
+
+	string = exit_qualification & 16;
Quoted 9 lines
arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5934
Quoted 5 lines
arch/x86/kvm/vmx.c +5 -0
+	rep = exit_qualification & 32;
+
+	/* TODO: interpret instruction and check range against bitmap */
+	if (string && rep)
+		return 1;
Nonsense, rep ins/outs always works against the same port. We can simply drop this check and be done with the feature. I'll come up with v4.

Quoted 3 lines

Actually this reminds me that we should check range of ports depending on operand size, not one port. But here is a catch, older cpus do not provide operand size as part of exit information.

You mean what bit 54 in VMX_BASIC is telling us? Too bad. OK, will write v5 which takes this into account.

Jan

Gleb Natapov

2013-02-18 08:57 UTC #17 lore raw

Quoted 53 lines · Jan Kiszka

Quoted 52 lines · Gleb Natapov
Quoted 51 lines · Jan Kiszka
Quoted 36 lines · Jan Kiszka
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 <[email protected]>
---
Changes in v3:
- trap unconditionally if bitmap access fails
 arch/x86/kvm/vmx.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 53 insertions(+), 2 deletions(-)
arch/x86/kvm/vmx.c +14 -0
index 6667042..2633199 100644
@@ -5908,6 +5908,58 @@ 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(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))
arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5934
Quoted 10 lines
arch/x86/kvm/vmx.c +10 -0
+		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;
Nonsense, rep ins/outs always works against the same port. We can simply drop this check and be done with the feature. I'll come up with v4.

Quoted 7 lines

Quoted 3 lines
Actually this reminds me that we should check range of ports depending on operand size, not one port. But here is a catch, older cpus do not provide operand size as part of exit information.
You mean what bit 54 in VMX_BASIC is telling us? Too bad. OK, will write v5 which takes this into account.

Yes, this one. We can just exit unconditionally on older cpus.

--
Gleb.

Jan Kiszka

author

2013-02-18 09:17 UTC #18 lore raw

PATCH v5 1/1 KVM: nVMX: Improve I/O exit handling

From: Jan Kiszka <[email protected]>

This prevents trapping L2 I/O exits if L1 has neither unconditional nor bitmap-based exiting enabled. Furthermore, it implements I/O bitmap handling. We still exit unconditionally in case the CPU does not provide information for ins/outs.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v5:
- still exit unconditionally if CPU refuses to provide exit information on ins/outs

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

arch/x86/kvm/vmx.c +56 -2

index 6667042..ccc7c17 100644
@@ -651,6 +651,7 @@ static struct vmcs_config {
 	int size;
 	int order;
 	u32 revision_id;
+	u32 vmx_basic_high;
 	u32 pin_based_exec_ctrl;
 	u32 cpu_based_exec_ctrl;
 	u32 cpu_based_2nd_exec_ctrl;

@@ -752,6 +753,11 @@ static inline bool vm_need_tpr_shadow(struct kvm *kvm)
 	return (cpu_has_vmx_tpr_shadow()) && (irqchip_in_kernel(kvm));
 }
 
+static inline bool cpu_has_stringio_exit_info(void)
+{
+	return vmcs_config.vmx_basic_high & (VMX_BASIC_INOUT >> 32);
+}
+
 static inline bool cpu_has_secondary_exec_ctrls(void)
 {
 	return vmcs_config.cpu_based_exec_ctrl &

@@ -2635,6 +2641,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 	vmcs_conf->size = vmx_msr_high & 0x1fff;
 	vmcs_conf->order = get_order(vmcs_config.size);
 	vmcs_conf->revision_id = vmx_msr_low;
+	vmcs_conf->vmx_basic_high = vmx_msr_high;
 
 	vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
 	vmcs_conf->cpu_based_exec_ctrl = _cpu_based_exec_control;

@@ -5908,6 +5915,54 @@ 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;
+	u16 port;
+	int size;
+	u8 b;
+
+	if (nested_cpu_has(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))
+		return 0;
+
+	/* TODO: for older CPUs, derive access width from instruction */
+	if (!cpu_has_stringio_exit_info())
+		return 1;
+
+	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
+
+	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 & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			if (kvm_read_guest(vcpu->kvm, bitmap, &b, 1))
+				return 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

@@ -6097,8 +6152,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);

Signature · 1 line

1.7.3.4

Gleb Natapov

2013-02-18 09:37 UTC #19 lore raw

Quoted 62 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v5:
- still exit unconditionally if CPU refuses to provide exit information on ins/outs

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

arch/x86/kvm/vmx.c +16 -0

index 6667042..ccc7c17 100644
@@ -651,6 +651,7 @@ static struct vmcs_config {
 	int size;
 	int order;
 	u32 revision_id;
+	u32 vmx_basic_high;
 	u32 pin_based_exec_ctrl;
 	u32 cpu_based_exec_ctrl;
 	u32 cpu_based_2nd_exec_ctrl;

@@ -752,6 +753,11 @@ static inline bool vm_need_tpr_shadow(struct kvm *kvm)
 	return (cpu_has_vmx_tpr_shadow()) && (irqchip_in_kernel(kvm));
 }
 
+static inline bool cpu_has_stringio_exit_info(void)
+{
+	return vmcs_config.vmx_basic_high & (VMX_BASIC_INOUT >> 32);
+}
+
 static inline bool cpu_has_secondary_exec_ctrls(void)
 {
 	return vmcs_config.cpu_based_exec_ctrl &

@@ -2635,6 +2641,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 	vmcs_conf->size = vmx_msr_high & 0x1fff;
 	vmcs_conf->order = get_order(vmcs_config.size);
 	vmcs_conf->revision_id = vmx_msr_low;
+	vmcs_conf->vmx_basic_high = vmx_msr_high;
 
 	vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
 	vmcs_conf->cpu_based_exec_ctrl = _cpu_based_exec_control;

@@ -5908,6 +5915,54 @@ 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;
+	u16 port;
+	int size;
+	u8 b;
+

arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5936

Quoted 10 lines

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

+	if (nested_cpu_has(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))
+		return 0;
+
+	/* TODO: for older CPUs, derive access width from instruction */
+	if (!cpu_has_stringio_exit_info())
+		return 1;
+

Sigh, actually I am stupid :( The information that is not available on older cpus is address size in "VM-exit instruction information", not operand size on exit qualification, so your v4 is correct. Except handling of port wrap around:

If the “use I/O bitmaps” VM-execution control is 1, the instruction causes a VM exit if it attempts to access an I/O port corresponding to a bit set to 1 in the appropriate I/O bitmap (see Section 24.6.4). If an I/O operation “wraps around” the 16-bit I/O-port space (accesses ports FFFFH and 0000H), the I/O instruction causes a VM exit (the “unconditional I/O exiting” VM-execution control is ignored if the “use I/O bitmaps” VM-execution control is 1).

Quoted 44 lines

arch/x86/kvm/vmx.c +30 -2

+	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
+
+	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 & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			if (kvm_read_guest(vcpu->kvm, bitmap, &b, 1))
+				return 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

@@ -6097,8 +6152,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);

Signature · 1 line

1.7.3.4

--
Gleb.

Jan Kiszka

author

2013-02-18 10:02 UTC #20 lore raw

Quoted 79 lines · Gleb Natapov

Quoted 62 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v5:
- still exit unconditionally if CPU refuses to provide exit information on ins/outs

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

arch/x86/kvm/vmx.c +16 -0

index 6667042..ccc7c17 100644
@@ -651,6 +651,7 @@ static struct vmcs_config {
 	int size;
 	int order;
 	u32 revision_id;
+	u32 vmx_basic_high;
 	u32 pin_based_exec_ctrl;
 	u32 cpu_based_exec_ctrl;
 	u32 cpu_based_2nd_exec_ctrl;

@@ -752,6 +753,11 @@ static inline bool vm_need_tpr_shadow(struct kvm *kvm)
 	return (cpu_has_vmx_tpr_shadow()) && (irqchip_in_kernel(kvm));
 }
 
+static inline bool cpu_has_stringio_exit_info(void)
+{
+	return vmcs_config.vmx_basic_high & (VMX_BASIC_INOUT >> 32);
+}
+
 static inline bool cpu_has_secondary_exec_ctrls(void)
 {
 	return vmcs_config.cpu_based_exec_ctrl &

@@ -2635,6 +2641,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 	vmcs_conf->size = vmx_msr_high & 0x1fff;
 	vmcs_conf->order = get_order(vmcs_config.size);
 	vmcs_conf->revision_id = vmx_msr_low;
+	vmcs_conf->vmx_basic_high = vmx_msr_high;
 
 	vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
 	vmcs_conf->cpu_based_exec_ctrl = _cpu_based_exec_control;

@@ -5908,6 +5915,54 @@ 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;
+	u16 port;
+	int size;
+	u8 b;
+

arch/x86/kvm/vmx.c static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { · line 5936

Quoted 10 lines

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

+	if (nested_cpu_has(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))
+		return 0;
+
+	/* TODO: for older CPUs, derive access width from instruction */
+	if (!cpu_has_stringio_exit_info())
+		return 1;
+

Quoted 7 lines

If the “use I/O bitmaps” VM-execution control is 1, the instruction causes a VM exit if it attempts to access an I/O port corresponding to a bit set to 1 in the appropriate I/O bitmap (see Section 24.6.4). If an I/O operation “wraps around” the 16-bit I/O-port space (accesses ports FFFFH and 0000H), the I/O instruction causes a VM exit (the “unconditional I/O exiting” VM-execution control is ignored if the “use I/O bitmaps” VM-execution control is 1).

Ah, indeed.

OK, let's see if this "simple" patch manages to reach two-digit revision numbers...

Jan

Jan Kiszka

2013-02-18 10:21 UTC #21 lore raw

PATCH v6 1/1 KVM: nVMX: Improve I/O exit handling

This prevents trapping L2 I/O exits if L1 has neither unconditional nor bitmap-based exiting enabled. Furthermore, it implements I/O bitmap handling.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v6:
- drop the bogus check of vmx_basix.54 again
- exit unconditionally on I/O address wrap around

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

arch/x86/kvm/vmx.c +47 -2

index 6667042..b4ce43c 100644
@@ -5908,6 +5908,52 @@ 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;
+	unsigned int port;
+	int size;
+	u8 b;
+
+	if (nested_cpu_has(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))
+		return 0;
+
+	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
+
+	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 if (port < 0x10000)
+			bitmap = vmcs12->io_bitmap_b;
+		else
+			return 1;
+		bitmap += (port & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			if (kvm_read_guest(vcpu->kvm, bitmap, &b, 1))
+				return 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

@@ -6097,8 +6143,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);

Signature · 1 line

1.7.3.4

Gleb Natapov

Reviewed-by on v6 1/1

2013-02-18 10:32 UTC #22 lore raw

Quoted 5 lines · Jan Kiszka

This prevents trapping L2 I/O exits if L1 has neither unconditional nor bitmap-based exiting enabled. Furthermore, it implements I/O bitmap handling.
Signed-off-by Jan Kiszka <[email protected]>

Reviewed-by Gleb Natapov <[email protected]>

Quoted 78 lines

---

Changes in v6:
- drop the bogus check of vmx_basix.54 again
- exit unconditionally on I/O address wrap around

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

arch/x86/kvm/vmx.c +47 -2

index 6667042..b4ce43c 100644
@@ -5908,6 +5908,52 @@ 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;
+	unsigned int port;
+	int size;
+	u8 b;
+
+	if (nested_cpu_has(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
+		return 1;
+
+	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))
+		return 0;
+
+	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
+
+	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 if (port < 0x10000)
+			bitmap = vmcs12->io_bitmap_b;
+		else
+			return 1;
+		bitmap += (port & 0x7fff) / 8;
+
+		if (last_bitmap != bitmap)
+			if (kvm_read_guest(vcpu->kvm, bitmap, &b, 1))
+				return 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

@@ -6097,8 +6143,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);

Signature · 1 line

1.7.3.4

--
Gleb.

Marcelo Tosatti

Applied

2013-02-19 02:14 UTC #23 lore raw

Quoted 7 lines · Gleb Natapov

Quoted 5 lines · Jan Kiszka
This prevents trapping L2 I/O exits if L1 has neither unconditional nor bitmap-based exiting enabled. Furthermore, it implements I/O bitmap handling.
Signed-off-by Jan Kiszka <[email protected]>
Reviewed-by Gleb Natapov <[email protected]>

Applied, thanks.