diff mbox

[4/4] Make cpu runnable after sipi

Message ID 20090913125521.GQ22885@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gleb Natapov Sept. 13, 2009, 12:55 p.m. UTC 
On Sun, Sep 13, 2009 at 03:05:21PM +0300, Avi Kivity wrote:
> On 09/13/2009 11:30 AM, Gleb Natapov wrote:
> >Signed-off-by: Gleb Natapov<gleb@redhat.com>
> >---
> >  hw/apic.c |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> >
> >diff --git a/hw/apic.c b/hw/apic.c
> >index 11d4759..ac5e7ba 100644
> >--- a/hw/apic.c
> >+++ b/hw/apic.c
> >@@ -534,6 +534,9 @@ void apic_sipi(CPUState *env)
> >                             env->segs[R_CS].limit, env->segs[R_CS].flags);
> >      env->halted = 0;
> >      s->wait_for_sipi = 0;
> >+#ifdef KVM_CAP_MP_STATE
> >+    env->mp_state = KVM_MP_STATE_RUNNABLE;
> >+#endif
> >  }
> 
> -no-kvm-irqchip shouldn't use mp_state at all (since really old
> kernels don't have this ioctl).
> 
Correct. Use patch below instead. mp_state shouldn't be touched if irq
chip is in userspace. I removed pu_synchronize_state() since it is done
before apic_init_reset() is called.

Signed-off-by: Gleb Natapov<gleb@redhat.com>
--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Marcelo Tosatti Sept. 18, 2009, 4:18 p.m. UTC | #1 
On Sun, Sep 13, 2009 at 03:55:21PM +0300, Gleb Natapov wrote:
> On Sun, Sep 13, 2009 at 03:05:21PM +0300, Avi Kivity wrote:
> > On 09/13/2009 11:30 AM, Gleb Natapov wrote:
> > >Signed-off-by: Gleb Natapov<gleb@redhat.com>
> > >---
> > >  hw/apic.c |    3 +++
> > >  1 files changed, 3 insertions(+), 0 deletions(-)
> > >
> > >diff --git a/hw/apic.c b/hw/apic.c
> > >index 11d4759..ac5e7ba 100644
> > >--- a/hw/apic.c
> > >+++ b/hw/apic.c
> > >@@ -534,6 +534,9 @@ void apic_sipi(CPUState *env)
> > >                             env->segs[R_CS].limit, env->segs[R_CS].flags);
> > >      env->halted = 0;
> > >      s->wait_for_sipi = 0;
> > >+#ifdef KVM_CAP_MP_STATE
> > >+    env->mp_state = KVM_MP_STATE_RUNNABLE;
> > >+#endif
> > >  }
> > 
> > -no-kvm-irqchip shouldn't use mp_state at all (since really old
> > kernels don't have this ioctl).
> > 
> Correct. Use patch below instead. mp_state shouldn't be touched if irq
> chip is in userspace. I removed pu_synchronize_state() since it is done
> before apic_init_reset() is called.
> 
> Signed-off-by: Gleb Natapov<gleb@redhat.com>
> diff --git a/hw/apic.c b/hw/apic.c
> index 11d4759..f9fef70 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -488,7 +488,6 @@ void apic_init_reset(CPUState *env)
>      if (!s)
>          return;
>  
> -    cpu_synchronize_state(env);
>      s->tpr = 0;
>      s->spurious_vec = 0xff;
>      s->log_dest = 0;

Gleb,

I've dropped this cleanup hunk. Please send it separately if you like.

Applied the remaining patches too, thanks.

> @@ -509,8 +508,9 @@ void apic_init_reset(CPUState *env)
>  
>      env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP);
>  #ifdef KVM_CAP_MP_STATE
> -    env->mp_state
> -        = env->halted ? KVM_MP_STATE_INIT_RECEIVED : KVM_MP_STATE_RUNNABLE;
> +    if (kvm_irqchip_in_kernel(kvm_context))
> +        env->mp_state
> +            = env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
>  #endif
>  }

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov Sept. 18, 2009, 4:22 p.m. UTC | #2 
On Fri, Sep 18, 2009 at 01:18:18PM -0300, Marcelo Tosatti wrote:
> On Sun, Sep 13, 2009 at 03:55:21PM +0300, Gleb Natapov wrote:
> > On Sun, Sep 13, 2009 at 03:05:21PM +0300, Avi Kivity wrote:
> > > On 09/13/2009 11:30 AM, Gleb Natapov wrote:
> > > >Signed-off-by: Gleb Natapov<gleb@redhat.com>
> > > >---
> > > >  hw/apic.c |    3 +++
> > > >  1 files changed, 3 insertions(+), 0 deletions(-)
> > > >
> > > >diff --git a/hw/apic.c b/hw/apic.c
> > > >index 11d4759..ac5e7ba 100644
> > > >--- a/hw/apic.c
> > > >+++ b/hw/apic.c
> > > >@@ -534,6 +534,9 @@ void apic_sipi(CPUState *env)
> > > >                             env->segs[R_CS].limit, env->segs[R_CS].flags);
> > > >      env->halted = 0;
> > > >      s->wait_for_sipi = 0;
> > > >+#ifdef KVM_CAP_MP_STATE
> > > >+    env->mp_state = KVM_MP_STATE_RUNNABLE;
> > > >+#endif
> > > >  }
> > > 
> > > -no-kvm-irqchip shouldn't use mp_state at all (since really old
> > > kernels don't have this ioctl).
> > > 
> > Correct. Use patch below instead. mp_state shouldn't be touched if irq
> > chip is in userspace. I removed pu_synchronize_state() since it is done
> > before apic_init_reset() is called.
> > 
> > Signed-off-by: Gleb Natapov<gleb@redhat.com>
> > diff --git a/hw/apic.c b/hw/apic.c
> > index 11d4759..f9fef70 100644
> > --- a/hw/apic.c
> > +++ b/hw/apic.c
> > @@ -488,7 +488,6 @@ void apic_init_reset(CPUState *env)
> >      if (!s)
> >          return;
> >  
> > -    cpu_synchronize_state(env);
> >      s->tpr = 0;
> >      s->spurious_vec = 0xff;
> >      s->log_dest = 0;
> 
> Gleb,
> 
> I've dropped this cleanup hunk. Please send it separately if you like.
> 
All callers already do cpu_synchronize_state(). I'll send another path.

> Applied the remaining patches too, thanks.
> 
> > @@ -509,8 +508,9 @@ void apic_init_reset(CPUState *env)
> >  
> >      env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP);
> >  #ifdef KVM_CAP_MP_STATE
> > -    env->mp_state
> > -        = env->halted ? KVM_MP_STATE_INIT_RECEIVED : KVM_MP_STATE_RUNNABLE;
> > +    if (kvm_irqchip_in_kernel(kvm_context))
> > +        env->mp_state
> > +            = env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
> >  #endif
> >  }

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marcelo Tosatti Sept. 18, 2009, 4:23 p.m. UTC | #3 
On Fri, Sep 18, 2009 at 07:22:22PM +0300, Gleb Natapov wrote:
> On Fri, Sep 18, 2009 at 01:18:18PM -0300, Marcelo Tosatti wrote:
> > On Sun, Sep 13, 2009 at 03:55:21PM +0300, Gleb Natapov wrote:
> > > On Sun, Sep 13, 2009 at 03:05:21PM +0300, Avi Kivity wrote:
> > > > On 09/13/2009 11:30 AM, Gleb Natapov wrote:
> > > > >Signed-off-by: Gleb Natapov<gleb@redhat.com>
> > > > >---
> > > > >  hw/apic.c |    3 +++
> > > > >  1 files changed, 3 insertions(+), 0 deletions(-)
> > > > >
> > > > >diff --git a/hw/apic.c b/hw/apic.c
> > > > >index 11d4759..ac5e7ba 100644
> > > > >--- a/hw/apic.c
> > > > >+++ b/hw/apic.c
> > > > >@@ -534,6 +534,9 @@ void apic_sipi(CPUState *env)
> > > > >                             env->segs[R_CS].limit, env->segs[R_CS].flags);
> > > > >      env->halted = 0;
> > > > >      s->wait_for_sipi = 0;
> > > > >+#ifdef KVM_CAP_MP_STATE
> > > > >+    env->mp_state = KVM_MP_STATE_RUNNABLE;
> > > > >+#endif
> > > > >  }
> > > > 
> > > > -no-kvm-irqchip shouldn't use mp_state at all (since really old
> > > > kernels don't have this ioctl).
> > > > 
> > > Correct. Use patch below instead. mp_state shouldn't be touched if irq
> > > chip is in userspace. I removed pu_synchronize_state() since it is done
> > > before apic_init_reset() is called.
> > > 
> > > Signed-off-by: Gleb Natapov<gleb@redhat.com>
> > > diff --git a/hw/apic.c b/hw/apic.c
> > > index 11d4759..f9fef70 100644
> > > --- a/hw/apic.c
> > > +++ b/hw/apic.c
> > > @@ -488,7 +488,6 @@ void apic_init_reset(CPUState *env)
> > >      if (!s)
> > >          return;
> > >  
> > > -    cpu_synchronize_state(env);
> > >      s->tpr = 0;
> > >      s->spurious_vec = 0xff;
> > >      s->log_dest = 0;
> > 
> > Gleb,
> > 
> > I've dropped this cleanup hunk. Please send it separately if you like.
> > 
> All callers already do cpu_synchronize_state(). I'll send another path.

Send to qemu upstream please.

> 
> > Applied the remaining patches too, thanks.
> > 
> > > @@ -509,8 +508,9 @@ void apic_init_reset(CPUState *env)
> > >  
> > >      env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP);
> > >  #ifdef KVM_CAP_MP_STATE
> > > -    env->mp_state
> > > -        = env->halted ? KVM_MP_STATE_INIT_RECEIVED : KVM_MP_STATE_RUNNABLE;
> > > +    if (kvm_irqchip_in_kernel(kvm_context))
> > > +        env->mp_state
> > > +            = env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
> > >  #endif
> > >  }
> 
> --
> 			Gleb.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/hw/apic.c b/hw/apic.c
index 11d4759..f9fef70 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -488,7 +488,6 @@  void apic_init_reset(CPUState *env)
     if (!s)
         return;
 
-    cpu_synchronize_state(env);
     s->tpr = 0;
     s->spurious_vec = 0xff;
     s->log_dest = 0;
@@ -509,8 +508,9 @@  void apic_init_reset(CPUState *env)
 
     env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP);
 #ifdef KVM_CAP_MP_STATE
-    env->mp_state
-        = env->halted ? KVM_MP_STATE_INIT_RECEIVED : KVM_MP_STATE_RUNNABLE;
+    if (kvm_irqchip_in_kernel(kvm_context))
+        env->mp_state
+            = env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
 #endif
 }