[PATCH 00/11] KVM updates for Linux 2.6.24-rc review

[PATCH 00/11] KVM updates for Linux 2.6.24-rc review

[Avi Kivity]

The following patches fix fallout from the main 2.6.24 KVM merge.  An
exception is the movnti emulation patch, which adds support for Linux
2.6.16 guests.

The patches can be found in kvm.git in the branch kvm-updates-2.6.24.
There is also a branch kvm-updates-2.6.25 which will form the basis of
the next merge window submission.

Please review the patches and let me know if something is wrong or if
a patch is missing.


Aurelien Jarno (1):
      KVM: x86 emulator: fix access registers for instructions with ModR/M byte and Mod = 3

Avi Kivity (2):
      KVM: VMX: Handle NMIs before enabling interrupts and preemption
      KVM: VMX: Force vm86 mode if setting flags during real mode

Eddie Dong (1):
      KVM: VMX: Reset mmu context when entering real mode

Izik Eidus (1):
      KVM: MMU: Set shadow pte atomically in mmu_pte_write_zap_pte()

Kevin Pedretti (2):
      KVM: Fix local apic timer divide by zero
      KVM: Improve local apic timer wraparound handling

Laurent Vivier (2):
      KVM: x86 emulator: fix repne/repnz decoding
      KVM: Move kvm_guest_exit() after local_irq_enable()

Nitin A Kamble (1):
      KVM: x86 emulator: fix merge screwup due to emulator split

Sheng Yang (1):
      KVM: x86 emulator: implement 'movnti mem, reg'

 drivers/kvm/kvm_main.c    |   11 ++++++-
 drivers/kvm/lapic.c       |   38 ++++++++++++++++------
 drivers/kvm/mmu.c         |    3 +-
 drivers/kvm/vmx.c         |   16 +++++++--
 drivers/kvm/x86_emulate.c |   77 ++++++++++++++++++++++++++++----------------
 5 files changed, 100 insertions(+), 45 deletions(-)

[PATCH 01/11] KVM: x86 emulator: fix merge screwup due to emulator split

[Avi Kivity]

From: Nitin A Kamble <nitin.a.kamble@intel.com>

This code has gone to wrong place in the file. Moving it back to
right location.

Signed-off-by: Nitin A Kamble <nitin.a.kamble@intel.com>
Signed-off-by: Avi Kivity <avi@qumranet.com>
---
 drivers/kvm/x86_emulate.c |   51 +++++++++++++++++++++++----------------------
 1 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/drivers/kvm/x86_emulate.c b/drivers/kvm/x86_emulate.c
index 9737c3b..b1026d2 100644
--- a/drivers/kvm/x86_emulate.c
+++ b/drivers/kvm/x86_emulate.c
@@ -1083,31 +1083,6 @@ push:
 	case 0xd2 ... 0xd3:	/* Grp2 */
 		src.val = _regs[VCPU_REGS_RCX];
 		goto grp2;
-	case 0xe8: /* call (near) */ {
-		long int rel;
-		switch (op_bytes) {
-		case 2:
-			rel = insn_fetch(s16, 2, _eip);
-			break;
-		case 4:
-			rel = insn_fetch(s32, 4, _eip);
-			break;
-		case 8:
-			rel = insn_fetch(s64, 8, _eip);
-			break;
-		default:
-			DPRINTF("Call: Invalid op_bytes\n");
-			goto cannot_emulate;
-		}
-		src.val = (unsigned long) _eip;
-		JMP_REL(rel);
-		goto push;
-	}
-	case 0xe9: /* jmp rel */
-	case 0xeb: /* jmp rel short */
-		JMP_REL(src.val);
-		no_wb = 1; /* Disable writeback. */
-		break;
 	case 0xf6 ... 0xf7:	/* Grp3 */
 		switch (modrm_reg) {
 		case 0 ... 1:	/* test */
@@ -1350,6 +1325,32 @@ special_insn:
 	case 0xae ... 0xaf:	/* scas */
 		DPRINTF("Urk! I don't handle SCAS.\n");
 		goto cannot_emulate;
+	case 0xe8: /* call (near) */ {
+		long int rel;
+		switch (op_bytes) {
+		case 2:
+			rel = insn_fetch(s16, 2, _eip);
+			break;
+		case 4:
+			rel = insn_fetch(s32, 4, _eip);
+			break;
+		case 8:
+			rel = insn_fetch(s64, 8, _eip);
+			break;
+		default:
+			DPRINTF("Call: Invalid op_bytes\n");
+			goto cannot_emulate;
+		}
+		src.val = (unsigned long) _eip;
+		JMP_REL(rel);
+		goto push;
+	}
+	case 0xe9: /* jmp rel */
+	case 0xeb: /* jmp rel short */
+		JMP_REL(src.val);
+		no_wb = 1; /* Disable writeback. */
+		break;
+
 
 	}
 	goto writeback;
-- 
1.5.3

[PATCH 02/11] KVM: x86 emulator: fix repne/repnz decoding

[Avi Kivity]

From: Laurent Vivier <Laurent.Vivier@bull.net>

The repnz/repne instructions must set rep_prefix to 1 like rep/repe/repz.

This patch correct the disk probe problem met with OpenBSD.

This issue appears with commit e70669abd4e60dfea3ac1639848e20e2b8dd1255
because before it, the decoding was done internally to kvm and after it
is done by x86_emulate.c (which doesn't do it correctly).

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
Signed-off-by: Avi Kivity <avi@qumranet.com>
---
 drivers/kvm/x86_emulate.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/kvm/x86_emulate.c b/drivers/kvm/x86_emulate.c
index b1026d2..80b1758 100644
--- a/drivers/kvm/x86_emulate.c
+++ b/drivers/kvm/x86_emulate.c
@@ -596,11 +596,10 @@ x86_emulate_memop(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
 		case 0xf0:	/* LOCK */
 			lock_prefix = 1;
 			break;
+		case 0xf2:	/* REPNE/REPNZ */
 		case 0xf3:	/* REP/REPE/REPZ */
 			rep_prefix = 1;
 			break;
-		case 0xf2:	/* REPNE/REPNZ */
-			break;
 		default:
 			goto done_prefixes;
 		}
-- 
1.5.3

[PATCH 03/11] KVM: MMU: Set shadow pte atomically in mmu_pte_write_zap_pte()

[Avi Kivity]

From: Izik Eidus <izike@qumranet.com>

Setting shadow page table entry should be set atomicly using set_shadow_pte().

Signed-off-by: Izik Eidus <izike@qumranet.com>
Signed-off-by: Avi Kivity <avi@qumranet.com>
---
 drivers/kvm/mmu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/kvm/mmu.c b/drivers/kvm/mmu.c
index 6d84d30..7171618 100644
--- a/drivers/kvm/mmu.c
+++ b/drivers/kvm/mmu.c
@@ -1088,7 +1088,7 @@ static void mmu_pte_write_zap_pte(struct kvm_vcpu *vcpu,
 			mmu_page_remove_parent_pte(child, spte);
 		}
 	}
-	*spte = 0;
+	set_shadow_pte(spte, 0);
 	kvm_flush_remote_tlbs(vcpu->kvm);
 }
 
-- 
1.5.3

[PATCH 04/11] KVM: VMX: Handle NMIs before enabling interrupts and preemption

[Avi Kivity]

This makes sure we handle NMI on the current cpu, and that we don't service
maskable interrupts before non-maskable ones.

Signed-off-by: Avi Kivity <avi@qumranet.com>
---
 drivers/kvm/vmx.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 4f115a8..bcc1e39 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -1760,10 +1760,8 @@ static int handle_exception(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		set_bit(irq / BITS_PER_LONG, &vcpu->irq_summary);
 	}
 
-	if ((intr_info & INTR_INFO_INTR_TYPE_MASK) == 0x200) { /* nmi */
-		asm ("int $2");
-		return 1;
-	}
+	if ((intr_info & INTR_INFO_INTR_TYPE_MASK) == 0x200) /* nmi */
+		return 1;  /* already handled by vmx_vcpu_run() */
 
 	if (is_no_device(intr_info)) {
 		vmx_fpu_activate(vcpu);
@@ -2196,6 +2194,7 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 static void vmx_vcpu_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
+	u32 intr_info;
 
 	/*
 	 * Loading guest fpu may have cleared host cr0.ts
@@ -2322,6 +2321,12 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 
 	asm ("mov %0, %%ds; mov %0, %%es" : : "r"(__USER_DS));
 	vmx->launched = 1;
+
+	intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
+
+	/* We need to handle NMIs before interrupts are enabled */
+	if ((intr_info & INTR_INFO_INTR_TYPE_MASK) == 0x200) /* nmi */
+		asm("int $2");
 }
 
 static void vmx_inject_page_fault(struct kvm_vcpu *vcpu,
-- 
1.5.3

[PATCH 05/11] KVM: VMX: Reset mmu context when entering real mode

[Avi Kivity]

From: Eddie Dong <eddie.dong@intel.com>

Resetting an SMP guest will force AP enter real mode (RESET) with
paging enabled in protected mode. While current enter_rmode() can
only handle mode switch from nonpaging mode to real mode which leads
to SMP reboot failure.

Fix by reloading the mmu context on entering real mode.

Signed-off-by: Yaozu (Eddie) Dong <eddie.dong@intel.com>
Signed-off-by: Qing He <qing.he@intel.com>
Signed-off-by: Avi Kivity <avi@qumranet.com>
---
 drivers/kvm/mmu.c |    1 +
 drivers/kvm/vmx.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/kvm/mmu.c b/drivers/kvm/mmu.c
index 7171618..feb5ac9 100644
--- a/drivers/kvm/mmu.c
+++ b/drivers/kvm/mmu.c
@@ -1049,6 +1049,7 @@ int kvm_mmu_reset_context(struct kvm_vcpu *vcpu)
 	destroy_kvm_mmu(vcpu);
 	return init_kvm_mmu(vcpu);
 }
+EXPORT_SYMBOL_GPL(kvm_mmu_reset_context);
 
 int kvm_mmu_load(struct kvm_vcpu *vcpu)
 {
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index bcc1e39..f130c01 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -1128,6 +1128,7 @@ static void enter_rmode(struct kvm_vcpu *vcpu)
 	fix_rmode_seg(VCPU_SREG_GS, &vcpu->rmode.gs);
 	fix_rmode_seg(VCPU_SREG_FS, &vcpu->rmode.fs);
 
+	kvm_mmu_reset_context(vcpu);
 	init_rmode_tss(vcpu->kvm);
 }
 
-- 
1.5.3

[PATCH 06/11] KVM: x86 emulator: implement 'movnti mem, reg'

[Avi Kivity]

From: Sheng Yang <sheng.yang@intel.com>

Implement emulation of instruction:
    movnti m32/m64, r32/r64
    opcode: 0x0f 0xc3

Signed-off-by: Sheng Yang <sheng.yang@intel.com>
Signed-off-by: Avi Kivity <avi@qumranet.com>
---
 drivers/kvm/x86_emulate.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/kvm/x86_emulate.c b/drivers/kvm/x86_emulate.c
index 80b1758..0a8696d 100644
--- a/drivers/kvm/x86_emulate.c
+++ b/drivers/kvm/x86_emulate.c
@@ -212,7 +212,8 @@ static u16 twobyte_table[256] = {
 	0, 0, ByteOp | DstReg | SrcMem | ModRM | Mov,
 	    DstReg | SrcMem16 | ModRM | Mov,
 	/* 0xC0 - 0xCF */
-	0, 0, 0, 0, 0, 0, 0, ImplicitOps | ModRM, 0, 0, 0, 0, 0, 0, 0, 0,
+	0, 0, 0, DstMem | SrcReg | ModRM | Mov, 0, 0, 0, ImplicitOps | ModRM,
+	0, 0, 0, 0, 0, 0, 0, 0,
 	/* 0xD0 - 0xDF */
 	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 	/* 0xE0 - 0xEF */
@@ -1501,6 +1502,10 @@ twobyte_insn:
 		dst.bytes = op_bytes;
 		dst.val = (d & ByteOp) ? (s8) src.val : (s16) src.val;
 		break;
+	case 0xc3:		/* movnti */
+		dst.bytes = op_bytes;
+		dst.val = (op_bytes == 4) ? (u32) src.val : (u64) src.val;
+		break;
 	}
 	goto writeback;
 
-- 
1.5.3

[PATCH 07/11] KVM: VMX: Force vm86 mode if setting flags during real mode

[Avi Kivity]

When resetting from userspace, we need to handle the flags being cleared
even after we are in real mode.

Signed-off-by: Avi Kivity <avi@qumranet.com>
---
 drivers/kvm/vmx.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index f130c01..bb56ae3 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -523,6 +523,8 @@ static unsigned long vmx_get_rflags(struct kvm_vcpu *vcpu)
 
 static void vmx_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags)
 {
+	if (vcpu->rmode.active)
+		rflags |= IOPL_MASK | X86_EFLAGS_VM;
 	vmcs_writel(GUEST_RFLAGS, rflags);
 }
 
-- 
1.5.3

[PATCH 08/11] KVM: x86 emulator: fix access registers for instructions with ModR/M byte and Mod = 3

[Avi Kivity]

From: Aurelien Jarno <aurelien@aurel32.net>

The patch belows changes the access type to register from memory for
instructions that are declared as SrcMem or DstMem, but have a
ModR/M byte with Mod = 3.

It fixes (at least) the lmsw and smsw instructions on an AMD64 CPU,
which are needed for FreeBSD.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Avi Kivity <avi@qumranet.com>
---
 drivers/kvm/x86_emulate.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/drivers/kvm/x86_emulate.c b/drivers/kvm/x86_emulate.c
index 0a8696d..a6ace30 100644
--- a/drivers/kvm/x86_emulate.c
+++ b/drivers/kvm/x86_emulate.c
@@ -825,6 +825,14 @@ done_prefixes:
 		if (twobyte && b == 0x01 && modrm_reg == 7)
 			break;
 	      srcmem_common:
+		/*
+		 * For instructions with a ModR/M byte, switch to register
+		 * access if Mod = 3.
+		 */
+		if ((d & ModRM) && modrm_mod == 3) {
+			src.type = OP_REG;
+			break;
+		}
 		src.type = OP_MEM;
 		src.ptr = (unsigned long *)cr2;
 		src.val = 0;
@@ -893,6 +901,14 @@ done_prefixes:
 		dst.ptr = (unsigned long *)cr2;
 		dst.bytes = (d & ByteOp) ? 1 : op_bytes;
 		dst.val = 0;
+		/*
+		 * For instructions with a ModR/M byte, switch to register
+		 * access if Mod = 3.
+		 */
+		if ((d & ModRM) && modrm_mod == 3) {
+			dst.type = OP_REG;
+			break;
+		}
 		if (d & BitOp) {
 			unsigned long mask = ~(dst.bytes * 8 - 1);
 
-- 
1.5.3

[PATCH 09/11] KVM: Move kvm_guest_exit() after local_irq_enable()

[Avi Kivity]

From: Laurent Vivier <Laurent.Vivier@bull.net>

We need to make sure that the timer interrupt happens before we clear
PF_VCPU, so the accounting code actually sees guest mode.

http://lkml.org/lkml/2007/10/15/114

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
Signed-off-by: Avi Kivity <avi@qumranet.com>
---
 drivers/kvm/kvm_main.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index af2d288..8c458f2 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -2054,12 +2054,21 @@ again:
 
 	kvm_x86_ops->run(vcpu, kvm_run);
 
-	kvm_guest_exit();
 	vcpu->guest_mode = 0;
 	local_irq_enable();
 
 	++vcpu->stat.exits;
 
+	/*
+	 * We must have an instruction between local_irq_enable() and
+	 * kvm_guest_exit(), so the timer interrupt isn't delayed by
+	 * the interrupt shadow.  The stat.exits increment will do nicely.
+	 * But we need to prevent reordering, hence this barrier():
+	 */
+	barrier();
+
+	kvm_guest_exit();
+
 	preempt_enable();
 
 	/*
-- 
1.5.3

[PATCH 10/11] KVM: Fix local apic timer divide by zero

[Avi Kivity]

From: Kevin Pedretti <kevin.pedretti@gmail.com>

kvm_lapic_reset() was initializing apic->timer.divide_count to 0,
which could potentially lead to a divide by zero error in
apic_get_tmcct().  Any guest that reads the APIC's CCR (current count)
register before setting DCR (divide configuration) would trigger a divide
by zero exception in the host kernel, leading to a host-OS crash.

This patch results in apic->timer.divide_count being initialized to
2 at reset, eliminating the bug (DCR=0 at reset, meaning divide by 2).

Signed-off-by: Kevin Pedretti <kevin.pedretti@gmail.com>
Signed-off-by: Avi Kivity <avi@qumranet.com>
---
 drivers/kvm/lapic.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index a190587..443730e 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -853,7 +853,7 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu)
 		apic_set_reg(apic, APIC_ISR + 0x10 * i, 0);
 		apic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
 	}
-	apic->timer.divide_count = 0;
+	update_divide_count(apic);
 	atomic_set(&apic->timer.pending, 0);
 	if (vcpu->vcpu_id == 0)
 		vcpu->apic_base |= MSR_IA32_APICBASE_BSP;
-- 
1.5.3

[PATCH 11/11] KVM: Improve local apic timer wraparound handling

[Avi Kivity]

From: Kevin Pedretti <kevin.pedretti@gmail.com>

Better handle wrap-around cases when reading the APIC CCR
(current count register).  Also, if ICR is 0, CCR should also
be 0... previously reading CCR before setting ICR would result
in a large kinda-random number.

Signed-off-by: Kevin Pedretti <kevin.pedretti@gmail.com>
Signed-off-by: Avi Kivity <avi@qumranet.com>
---
 drivers/kvm/lapic.c |   36 ++++++++++++++++++++++++++----------
 1 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 443730e..238fcad 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -494,12 +494,19 @@ static void apic_send_ipi(struct kvm_lapic *apic)
 
 static u32 apic_get_tmcct(struct kvm_lapic *apic)
 {
-	u32 counter_passed;
-	ktime_t passed, now = apic->timer.dev.base->get_time();
-	u32 tmcct = apic_get_reg(apic, APIC_TMICT);
+	u64 counter_passed;
+	ktime_t passed, now;
+	u32 tmcct;
 
 	ASSERT(apic != NULL);
 
+	now = apic->timer.dev.base->get_time();
+	tmcct = apic_get_reg(apic, APIC_TMICT);
+
+	/* if initial count is 0, current count should also be 0 */
+	if (tmcct == 0)
+		return 0;
+
 	if (unlikely(ktime_to_ns(now) <=
 		ktime_to_ns(apic->timer.last_update))) {
 		/* Wrap around */
@@ -514,15 +521,24 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
 
 	counter_passed = div64_64(ktime_to_ns(passed),
 				  (APIC_BUS_CYCLE_NS * apic->timer.divide_count));
-	tmcct -= counter_passed;
 
-	if (tmcct <= 0) {
-		if (unlikely(!apic_lvtt_period(apic)))
+	if (counter_passed > tmcct) {
+		if (unlikely(!apic_lvtt_period(apic))) {
+			/* one-shot timers stick at 0 until reset */
 			tmcct = 0;
-		else
-			do {
-				tmcct += apic_get_reg(apic, APIC_TMICT);
-			} while (tmcct <= 0);
+		} else {
+			/*
+			 * periodic timers reset to APIC_TMICT when they
+			 * hit 0. The while loop simulates this happening N
+			 * times. (counter_passed %= tmcct) would also work,
+			 * but might be slower or not work on 32-bit??
+			 */
+			while (counter_passed > tmcct)
+				counter_passed -= tmcct;
+			tmcct -= counter_passed;
+		}
+	} else {
+		tmcct -= counter_passed;
 	}
 
 	return tmcct;
-- 
1.5.3

Re: [PATCH 00/11] KVM updates for Linux 2.6.24-rc review

[Avi Kivity]

Avi Kivity wrote:
> The following patches fix fallout from the main 2.6.24 KVM merge.  An
> exception is the movnti emulation patch, which adds support for Linux
> 2.6.16 guests.
>
> The patches can be found in kvm.git in the branch kvm-updates-2.6.24.
> There is also a branch kvm-updates-2.6.25 which will form the basis of
> the next merge window submission.
>
> Please review the patches and let me know if something is wrong or if
> a patch is missing.
>
>   


Laurent, I believe the following patch (in kvm-updates-2.6.25) needs to 
go into 2.6.24 as well.  Can you comment on this?

> commit 6de232e39be372f85bea96eb741962acc7fcb1f7
> Author: Laurent Vivier <Laurent.Vivier@bull.net>
> Date:   Mon Oct 1 11:01:06 2007 +0200
>
>     KVM: x86 emulator: Correct management of REP prefix
>
>     This patch corrects some errors appearing when we have an 
> emulation failure
>     on an operation using REP prefix.
>
>     When x86_emulate_insn() fails, saving EIP and ECX is not enough as 
> emulation
>     should have modified other registers like RSI or RDI. Moreover, 
> the emulation
>     can fail on the writeback, and in this case we are not able to restore
>     registers.
>
>     At beginning of x86_emulate_insn(), we restore registers from vcpu 
> as they were
>     not modified by x86d_decode_insn() and we save EIP to be able to 
> restore it
>     in case of failure.
>

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 00/11] KVM updates for Linux 2.6.24-rc review

[Laurent Vivier]

Avi Kivity a écrit :
> Avi Kivity wrote:
>> The following patches fix fallout from the main 2.6.24 KVM merge.  An
>> exception is the movnti emulation patch, which adds support for Linux
>> 2.6.16 guests.
>>
>> The patches can be found in kvm.git in the branch kvm-updates-2.6.24.
>> There is also a branch kvm-updates-2.6.25 which will form the basis of
>> the next merge window submission.
>>
>> Please review the patches and let me know if something is wrong or if
>> a patch is missing.
>>
>>   
> 
> 
> Laurent, I believe the following patch (in kvm-updates-2.6.25) needs to 
> go into 2.6.24 as well.  Can you comment on this?

What kind of comment do you want ?
What are the requirements to go in 2.6.24 instead of 2.6.25 ?

Is a bug correction enough ? :-P

This patch correct a bad behavior of x86_emulate_insn() in case of error with a 
REP prefix.

This patch is needed because, without it, when REP prefix is used with an 
instruction failing for some reasons (like IO or page fault) we don't restore 
all modified registers (like RSI and RDI), but only ECX and EIP, so when we 
re-enter in x86_emulate_insn() we modify again an already modified value. 
Moreover, this patch manages correctly the case where the instruction fails in 
writeback().


>> commit 6de232e39be372f85bea96eb741962acc7fcb1f7
>> Author: Laurent Vivier <Laurent.Vivier@bull.net>
>> Date:   Mon Oct 1 11:01:06 2007 +0200
>>
>>     KVM: x86 emulator: Correct management of REP prefix
>>
>>     This patch corrects some errors appearing when we have an 
>> emulation failure
>>     on an operation using REP prefix.
>>
>>     When x86_emulate_insn() fails, saving EIP and ECX is not enough as 
>> emulation
>>     should have modified other registers like RSI or RDI. Moreover, 
>> the emulation
>>     can fail on the writeback, and in this case we are not able to 
>> restore
>>     registers.
>>
>>     At beginning of x86_emulate_insn(), we restore registers from vcpu 
>> as they were
>>     not modified by x86d_decode_insn() and we save EIP to be able to 
>> restore it
>>     in case of failure.
>>
> 


-- 
---------------- Laurent.Vivier@bull.net  -----------------
"Given enough eyeballs, all bugs are shallow" E. S. Raymond

Re: [PATCH 00/11] KVM updates for Linux 2.6.24-rc review

[Avi Kivity]

Laurent Vivier wrote:
> Avi Kivity a écrit :
>> Avi Kivity wrote:
>>> The following patches fix fallout from the main 2.6.24 KVM merge.  An
>>> exception is the movnti emulation patch, which adds support for Linux
>>> 2.6.16 guests.
>>>
>>> The patches can be found in kvm.git in the branch kvm-updates-2.6.24.
>>> There is also a branch kvm-updates-2.6.25 which will form the basis of
>>> the next merge window submission.
>>>
>>> Please review the patches and let me know if something is wrong or if
>>> a patch is missing.
>>>
>>>   
>>
>>
>> Laurent, I believe the following patch (in kvm-updates-2.6.25) needs 
>> to go into 2.6.24 as well.  Can you comment on this?
>
> What kind of comment do you want ?

Well, whether it needs to go into .24.

> What are the requirements to go in 2.6.24 instead of 2.6.25 ?
>
> Is a bug correction enough ? :-P
>

It has to fix something real-life, in a real guest.

> This patch correct a bad behavior of x86_emulate_insn() in case of 
> error with a REP prefix.
>
> This patch is needed because, without it, when REP prefix is used with 
> an instruction failing for some reasons (like IO or page fault) we 
> don't restore all modified registers (like RSI and RDI), but only ECX 
> and EIP, so when we re-enter in x86_emulate_insn() we modify again an 
> already modified value. Moreover, this patch manages correctly the 
> case where the instruction fails in writeback().
>

Okay, I guess it is needed.  Can you backport it to the branch?  It's 
very different from kvm-updates-2.6.24...

-- 
error compiling committee.c: too many arguments to function

Re: [kvm-devel] [PATCH 00/11] KVM updates for Linux 2.6.24-rc review

[Laurent Vivier]

Avi Kivity a écrit :
> Laurent Vivier wrote:
>> Avi Kivity a écrit :
>>> Avi Kivity wrote:
>>>> The following patches fix fallout from the main 2.6.24 KVM merge.  An
>>>> exception is the movnti emulation patch, which adds support for Linux
>>>> 2.6.16 guests.
>>>>
>>>> The patches can be found in kvm.git in the branch kvm-updates-2.6.24.
>>>> There is also a branch kvm-updates-2.6.25 which will form the basis of
>>>> the next merge window submission.
>>>>
>>>> Please review the patches and let me know if something is wrong or if
>>>> a patch is missing.
>>>>
>>>>   
>>>
>>> Laurent, I believe the following patch (in kvm-updates-2.6.25) needs 
>>> to go into 2.6.24 as well.  Can you comment on this?
>> What kind of comment do you want ?
> 
> Well, whether it needs to go into .24.
> 
>> What are the requirements to go in 2.6.24 instead of 2.6.25 ?
>>
>> Is a bug correction enough ? :-P
>>
> 
> It has to fix something real-life, in a real guest.
> 
>> This patch correct a bad behavior of x86_emulate_insn() in case of 
>> error with a REP prefix.
>>
>> This patch is needed because, without it, when REP prefix is used with 
>> an instruction failing for some reasons (like IO or page fault) we 
>> don't restore all modified registers (like RSI and RDI), but only ECX 
>> and EIP, so when we re-enter in x86_emulate_insn() we modify again an 
>> already modified value. Moreover, this patch manages correctly the 
>> case where the instruction fails in writeback().
>>
> 
> Okay, I guess it is needed.  Can you backport it to the branch?  It's 
> very different from kvm-updates-2.6.24...
> 

Well, in fact it is not needed in 2.6.24, because this patch correct a bad 
behavior introduced by commit 57f4e446ebca4aad5c11364baf8477c8cfcb16a4 (which is 
not in kvm-update-2.6.24):

KVM: Call x86_decode_insn() only when needed

Move emulate_ctxt to kvm_vcpu to keep emulate context when we exit from kvm
module. Call x86_decode_insn() only when needed. Modify x86_emulate_insn() to
not modify the context if it must be re-entered.

So, in fact, the answer is (after correctly understanding the question): no, 
this patch is not needed in kvm-update-2.6.24.

Regards,
Laurent
-- 
---------------- Laurent.Vivier@bull.net  -----------------
"Given enough eyeballs, all bugs are shallow" E. S. Raymond

Re: [kvm-devel] [PATCH 00/11] KVM updates for Linux 2.6.24-rc review

[Avi Kivity]

Laurent Vivier wrote:
>
> Well, in fact it is not needed in 2.6.24, because this patch correct a 
> bad behavior introduced by commit 
> 57f4e446ebca4aad5c11364baf8477c8cfcb16a4 (which is not in 
> kvm-update-2.6.24):
>
> KVM: Call x86_decode_insn() only when needed
>
> Move emulate_ctxt to kvm_vcpu to keep emulate context when we exit 
> from kvm
> module. Call x86_decode_insn() only when needed. Modify 
> x86_emulate_insn() to
> not modify the context if it must be re-entered.
>
> So, in fact, the answer is (after correctly understanding the 
> question): no, this patch is not needed in kvm-update-2.6.24.



Thanks.  I've folded it into 57f4e446ebca4aad5c11364baf8477c8cfcb16a4 
for 2.6.25.

-- 
error compiling committee.c: too many arguments to function