[PATCH] KVM: Fix exiting from HLT emulation with MP_STATE_HALTED

[PATCH] KVM: Fix exiting from HLT emulation with MP_STATE_HALTED

[Yang, Sheng]

[-- Attachment #1: Type: text/plain, Size: 2010 bytes --]

From 8019bdd0c7e2933a4d6f7e046d1cc2ded221784e Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng.yang@intel.com>
Date: Wed, 30 Jul 2008 21:45:58 +0800
Subject: [PATCH] KVM: Fix exiting from HLT emulation with 
MP_STATE_HALTED

VCPU can be forced exit from HLT emulation without setting mp_state to
MP_STATE_RUNNABLE, it's due to QEmu can kick vcpus which are doing HLT
emulation to do something like "stop" or "info cpus". Here are two 
issues of this behaviour:

First, if vcpu exit to QEmu with MP_STATE_HALTED, it would keep in 
this state later for vcpu_run(), which is eerie...

Second, a practical problem: bios load AP boot up code to 0x10000, and 
AP is running HLT there. But later grub load it's stage2 code to the 
same address. Then if the halting vcpu was forced exit to QEmu in 
grub, and come back for vcpu_run later, it can't execute HLT 
instruction anymore, just because the bios code is not there,
and it would follow a piece of code of grub, which would cause 
completely chaos...

The second issue directly lead to guest crash or SMP linux can't boot 
up AP later if we "stop" or "info cpus" in grub.

The patch resumes the HLT emulation after interrupt by QEmu to fix it.

Signed-off-by: Sheng Yang <sheng.yang@intel.com>
---
 arch/x86/kvm/x86.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 94a2165..3b31959 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2881,6 +2881,11 @@ again:
 	clear_bit(KVM_REQ_PENDING_TIMER, &vcpu->requests);
 	kvm_inject_pending_timer_irqs(vcpu);

+	if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED) {
+		r = kvm_emulate_halt(vcpu);
+		goto next_around;
+	}
+
 	preempt_disable();

 	kvm_x86_ops->prepare_guest_switch(vcpu);
@@ -2962,6 +2967,7 @@ again:

 	r = kvm_x86_ops->handle_exit(kvm_run, vcpu);

+next_around:
 	if (r > 0) {
 		if (dm_request_for_irq_injection(vcpu, kvm_run)) {
 			r = -EINTR;
--
1.5.4.5


[-- Attachment #2: 0001-KVM-Fix-exiting-from-HLT-emulation-with-MP_STATE_HA.patch --]
[-- Type: text/x-diff, Size: 2003 bytes --]

From 8019bdd0c7e2933a4d6f7e046d1cc2ded221784e Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng.yang@intel.com>
Date: Wed, 30 Jul 2008 21:45:58 +0800
Subject: [PATCH] KVM: Fix exiting from HLT emulation with MP_STATE_HALTED

VCPU can be forced exit from HLT emulation without setting mp_state to
MP_STATE_RUNNABLE, it's due to QEmu can kick vcpus which are doing HLT
emulation to do something like "stop" or "info cpus". Here are two issues of
this behaviour:

First, if vcpu exit to QEmu with MP_STATE_HALTED, it would keep in this state
later for vcpu_run(), which is eerie...

Second, a practical problem: bios load AP boot up code to 0x10000, and AP is
running HLT there. But later grub load it's stage2 code to the same address. Then
if the halting vcpu was forced exit to QEmu in grub, and come back for vcpu_run later,
it can't execute HLT instruction anymore, just because the bios code is not there,
and it would follow a piece of code of grub, which would cause completely chaos...

The second issue directly lead to guest crash or SMP linux can't boot up AP
later if we "stop" or "info cpus" in grub.

The patch resumes the HLT emulation after interrupt by QEmu to fix it.

Signed-off-by: Sheng Yang <sheng.yang@intel.com>
---
 arch/x86/kvm/x86.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 94a2165..3b31959 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2881,6 +2881,11 @@ again:
 	clear_bit(KVM_REQ_PENDING_TIMER, &vcpu->requests);
 	kvm_inject_pending_timer_irqs(vcpu);
 
+	if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED) {
+		r = kvm_emulate_halt(vcpu);
+		goto next_around;
+	}
+
 	preempt_disable();
 
 	kvm_x86_ops->prepare_guest_switch(vcpu);
@@ -2962,6 +2967,7 @@ again:
 
 	r = kvm_x86_ops->handle_exit(kvm_run, vcpu);
 
+next_around:
 	if (r > 0) {
 		if (dm_request_for_irq_injection(vcpu, kvm_run)) {
 			r = -EINTR;
-- 
1.5.4.5

Re: [PATCH] KVM: Fix exiting from HLT emulation with MP_STATE_HALTED

[Yang, Sheng]

On Wednesday 30 July 2008 21:55:19 Yang, Sheng wrote:
> From 8019bdd0c7e2933a4d6f7e046d1cc2ded221784e Mon Sep 17 00:00:00
> 2001 From: Sheng Yang <sheng.yang@intel.com>
> Date: Wed, 30 Jul 2008 21:45:58 +0800
> Subject: [PATCH] KVM: Fix exiting from HLT emulation with
> MP_STATE_HALTED

Recall this patch, would update it soon. 

-- 
regards
Yang, Sheng

[PATCH] KVM: Fix QEmu interrupted HLT emulation

[Yang, Sheng]

[-- Attachment #1: Type: text/plain, Size: 2262 bytes --]

From: Sheng Yang <sheng.yang@intel.com>
Date: Thu, 31 Jul 2008 13:43:58 +0800
Subject: [PATCH] KVM: Fix QEmu interrupted HLT emulation

QEmu can interrupt VCPU from HLT emulation without setting mp_state to
MP_STATE_RUNNABLE, when it kick vcpus which are doing HLT emulation to 
do something like "stop" or "info cpus". Here are two issues of this 
behaviour:

First, if vcpu exit to QEmu with MP_STATE_HALTED, it would keep in 
this state later for vcpu_run(), which is eerie...

Second, a practical problem: bios load AP boot up code to 0x10000 
(now), and AP is running HLT there. But later grub load it's stage2 
code to the same address. Then if the halting vcpu was forced exit to 
QEmu in grub, and come back for vcpu_run later, it can't execute HLT 
instruction anymore, just because the bios code is not there,
and it would follow a piece of code of grub, which would cause 
completely chaos...

The second issue directly lead to guest crash or SMP linux can't boot 
up AP later if we "stop" or "info cpus" in grub. Though I also sent a 
patch for BIOS, it's necessary to get correct behavior here.

The patch resumes the HLT emulation after interrupt by QEmu to fix it.

Signed-off-by: Sheng Yang <sheng.yang@intel.com>
---
 arch/x86/kvm/x86.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 94a2165..8219074 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2881,6 +2881,19 @@ again:
 	clear_bit(KVM_REQ_PENDING_TIMER, &vcpu->requests);
 	kvm_inject_pending_timer_irqs(vcpu);

+	/*
+	 * If HLT emulating was interrupted by QEmu, we'd better resume it.
+	 * And if QEmu don't interrupt it again, set correct state rather 
than
+	 * keeping running with STATE_HALTED
+	 */
+	if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED) {
+		r = kvm_emulate_halt(vcpu);
+		if (!signal_pending(current) &&
+		    !kvm_arch_vcpu_runnable(vcpu))
+			vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
+		goto next_round;
+	}
+
 	preempt_disable();

 	kvm_x86_ops->prepare_guest_switch(vcpu);
@@ -2962,6 +2975,7 @@ again:

 	r = kvm_x86_ops->handle_exit(kvm_run, vcpu);

+next_round:
 	if (r > 0) {
 		if (dm_request_for_irq_injection(vcpu, kvm_run)) {
 			r = -EINTR;
--
1.5.4.5


[-- Attachment #2: 0001-KVM-Fix-QEmu-interrupted-HLT-emulation.patch --]
[-- Type: text/x-diff, Size: 2391 bytes --]

From ea0a1f70d44590929c9d618ee4bef8af1553b442 Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng.yang@intel.com>
Date: Thu, 31 Jul 2008 13:43:58 +0800
Subject: [PATCH] KVM: Fix QEmu interrupted HLT emulation

QEmu can interrupt VCPU from HLT emulation without setting mp_state to
MP_STATE_RUNNABLE, when it kick vcpus which are doing HLT emulation to do
something like "stop" or "info cpus". Here are two issues of this behaviour:

First, if vcpu exit to QEmu with MP_STATE_HALTED, it would keep in this state
later for vcpu_run(), which is eerie...

Second, a practical problem: bios load AP boot up code to 0x10000 (now), and AP is
running HLT there. But later grub load it's stage2 code to the same address. Then
if the halting vcpu was forced exit to QEmu in grub, and come back for vcpu_run later,
it can't execute HLT instruction anymore, just because the bios code is not there,
and it would follow a piece of code of grub, which would cause completely chaos...

The second issue directly lead to guest crash or SMP linux can't boot up AP
later if we "stop" or "info cpus" in grub. Though I also sent a patch for BIOS,
it's necessary to get correct behavior here.

The patch resumes the HLT emulation after interrupt by QEmu to fix it.

Signed-off-by: Sheng Yang <sheng.yang@intel.com>
---
 arch/x86/kvm/x86.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 94a2165..8219074 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2881,6 +2881,19 @@ again:
 	clear_bit(KVM_REQ_PENDING_TIMER, &vcpu->requests);
 	kvm_inject_pending_timer_irqs(vcpu);
 
+	/*
+	 * If HLT emulating was interrupted by QEmu, we'd better resume it.
+	 * And if QEmu don't interrupt it again, set correct state rather than
+	 * keeping running with STATE_HALTED
+	 */
+	if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED) {
+		r = kvm_emulate_halt(vcpu);
+		if (!signal_pending(current) &&
+		    !kvm_arch_vcpu_runnable(vcpu))
+			vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
+		goto next_round;
+	}
+
 	preempt_disable();
 
 	kvm_x86_ops->prepare_guest_switch(vcpu);
@@ -2962,6 +2975,7 @@ again:
 
 	r = kvm_x86_ops->handle_exit(kvm_run, vcpu);
 
+next_round:
 	if (r > 0) {
 		if (dm_request_for_irq_injection(vcpu, kvm_run)) {
 			r = -EINTR;
-- 
1.5.4.5

Re: [PATCH] KVM: Fix QEmu interrupted HLT emulation

[Avi Kivity]

Yang, Sheng wrote:
> From: Sheng Yang <sheng.yang@intel.com>
> Date: Thu, 31 Jul 2008 13:43:58 +0800
> Subject: [PATCH] KVM: Fix QEmu interrupted HLT emulation
>
> QEmu can interrupt VCPU from HLT emulation without setting mp_state to
> MP_STATE_RUNNABLE, when it kick vcpus which are doing HLT emulation to 
> do something like "stop" or "info cpus". Here are two issues of this 
> behaviour:
>
> First, if vcpu exit to QEmu with MP_STATE_HALTED, it would keep in 
> this state later for vcpu_run(), which is eerie...
>
> Second, a practical problem: bios load AP boot up code to 0x10000 
> (now), and AP is running HLT there. But later grub load it's stage2 
> code to the same address. Then if the halting vcpu was forced exit to 
> QEmu in grub, and come back for vcpu_run later, it can't execute HLT 
> instruction anymore, just because the bios code is not there,
> and it would follow a piece of code of grub, which would cause 
> completely chaos...
>
> The second issue directly lead to guest crash or SMP linux can't boot 
> up AP later if we "stop" or "info cpus" in grub. Though I also sent a 
> patch for BIOS, it's necessary to get correct behavior here.
>   

Going over my backlog it looks like I missed this.  But I think
Marcelo's rework obsoletes this patch?


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [PATCH] KVM: Fix QEmu interrupted HLT emulation

[Yang, Sheng]

On Thursday 11 September 2008 16:50:37 Avi Kivity wrote:
> Yang, Sheng wrote:
> > From: Sheng Yang <sheng.yang@intel.com>
> > Date: Thu, 31 Jul 2008 13:43:58 +0800
> > Subject: [PATCH] KVM: Fix QEmu interrupted HLT emulation
> >
> > QEmu can interrupt VCPU from HLT emulation without setting mp_state to
> > MP_STATE_RUNNABLE, when it kick vcpus which are doing HLT emulation to
> > do something like "stop" or "info cpus". Here are two issues of this
> > behaviour:
> >
> > First, if vcpu exit to QEmu with MP_STATE_HALTED, it would keep in
> > this state later for vcpu_run(), which is eerie...
> >
> > Second, a practical problem: bios load AP boot up code to 0x10000
> > (now), and AP is running HLT there. But later grub load it's stage2
> > code to the same address. Then if the halting vcpu was forced exit to
> > QEmu in grub, and come back for vcpu_run later, it can't execute HLT
> > instruction anymore, just because the bios code is not there,
> > and it would follow a piece of code of grub, which would cause
> > completely chaos...
> >
> > The second issue directly lead to guest crash or SMP linux can't boot
> > up AP later if we "stop" or "info cpus" in grub. Though I also sent a
> > patch for BIOS, it's necessary to get correct behavior here.
>
> Going over my backlog it looks like I missed this.  But I think
> Marcelo's rework obsoletes this patch?

Yeah, long ago... So I also drop this patch.

-- 
regards
Yang, Sheng