diff mbox

[1/4] avoid halted state for in kernel irqchip

Message ID 1244053544-12074-2-git-send-email-glommer@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Glauber Costa June 3, 2009, 6:25 p.m. UTC
This patch is part of a series that tries to provide
a kvm-free apic implementation. In the last interation,
Jan pointed out that halted state management with in kernel
irqchip gets quite messy. I don't disagree.

It broke this series specifically, as init IPIs had the
halted state set. The solution would be to rework halted
state management. But I'm too lazy. Besides, gleb told me
he would do it, which makes it his problem, not mine. ;-)

This patch can be used to bypass the problem entirely:
if kvm apic do not call apic_init_ipi, but a version of it
that does everything but mangling around with cpu states,
the problem becomes a non issue, and my glorious series
can be applied.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 hw/apic.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

Comments

Gleb Natapov June 3, 2009, 7:50 p.m. UTC | #1
On Wed, Jun 03, 2009 at 02:25:41PM -0400, Glauber Costa wrote:
> This patch is part of a series that tries to provide
> a kvm-free apic implementation. In the last interation,
> Jan pointed out that halted state management with in kernel
> irqchip gets quite messy. I don't disagree.
> 
> It broke this series specifically, as init IPIs had the
> halted state set. The solution would be to rework halted
> state management. But I'm too lazy. Besides, gleb told me
> he would do it, which makes it his problem, not mine. ;-)
> 
> This patch can be used to bypass the problem entirely:
> if kvm apic do not call apic_init_ipi, but a version of it
> that does everything but mangling around with cpu states,
> the problem becomes a non issue, and my glorious series
> can be applied.
> 
May be I miss something, but I don't see apic_init_ipi_state() called
outside of apic_init_ipi() in this patch series, so what is the point of
this patch?

> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  hw/apic.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/apic.c b/hw/apic.c
> index 86aa6b6..862289d 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -443,8 +443,7 @@ static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
>      }
>  }
>  
> -
> -static void apic_init_ipi(APICState *s)
> +static void apic_init_ipi_state(APICState *s)
>  {
>      int i;
>  
> @@ -466,6 +465,11 @@ static void apic_init_ipi(APICState *s)
>      s->next_time = 0;
>  
>      cpu_reset(s->cpu_env);
> +}
> +
> +static void apic_init_ipi(APICState *s)
> +{
> +    apic_init_ipi_state(s);
>  
>      if (!(s->apicbase & MSR_IA32_APICBASE_BSP) &&
>          (!kvm_enabled() || !qemu_kvm_irqchip_in_kernel()))
> -- 
> 1.5.6.6
> 
> --
> 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.
--
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
Glauber Costa June 3, 2009, 9:10 p.m. UTC | #2
On Wed, Jun 03, 2009 at 10:50:59PM +0300, Gleb Natapov wrote:
> On Wed, Jun 03, 2009 at 02:25:41PM -0400, Glauber Costa wrote:
> > This patch is part of a series that tries to provide
> > a kvm-free apic implementation. In the last interation,
> > Jan pointed out that halted state management with in kernel
> > irqchip gets quite messy. I don't disagree.
> > 
> > It broke this series specifically, as init IPIs had the
> > halted state set. The solution would be to rework halted
> > state management. But I'm too lazy. Besides, gleb told me
> > he would do it, which makes it his problem, not mine. ;-)
> > 
> > This patch can be used to bypass the problem entirely:
> > if kvm apic do not call apic_init_ipi, but a version of it
> > that does everything but mangling around with cpu states,
> > the problem becomes a non issue, and my glorious series
> > can be applied.
> > 
> May be I miss something, but I don't see apic_init_ipi_state() called
> outside of apic_init_ipi() in this patch series, so what is the point of
> this patch?
it should be called from patch 3.

But I made a mistake, and in fact it is not. My bad.

> 
> > Signed-off-by: Glauber Costa <glommer@redhat.com>
> > ---
> >  hw/apic.c |    8 ++++++--
> >  1 files changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/apic.c b/hw/apic.c
> > index 86aa6b6..862289d 100644
> > --- a/hw/apic.c
> > +++ b/hw/apic.c
> > @@ -443,8 +443,7 @@ static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
> >      }
> >  }
> >  
> > -
> > -static void apic_init_ipi(APICState *s)
> > +static void apic_init_ipi_state(APICState *s)
> >  {
> >      int i;
> >  
> > @@ -466,6 +465,11 @@ static void apic_init_ipi(APICState *s)
> >      s->next_time = 0;
> >  
> >      cpu_reset(s->cpu_env);
> > +}
> > +
> > +static void apic_init_ipi(APICState *s)
> > +{
> > +    apic_init_ipi_state(s);
> >  
> >      if (!(s->apicbase & MSR_IA32_APICBASE_BSP) &&
> >          (!kvm_enabled() || !qemu_kvm_irqchip_in_kernel()))
> > -- 
> > 1.5.6.6
> > 
> > --
> > 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.
--
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 86aa6b6..862289d 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -443,8 +443,7 @@  static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
     }
 }
 
-
-static void apic_init_ipi(APICState *s)
+static void apic_init_ipi_state(APICState *s)
 {
     int i;
 
@@ -466,6 +465,11 @@  static void apic_init_ipi(APICState *s)
     s->next_time = 0;
 
     cpu_reset(s->cpu_env);
+}
+
+static void apic_init_ipi(APICState *s)
+{
+    apic_init_ipi_state(s);
 
     if (!(s->apicbase & MSR_IA32_APICBASE_BSP) &&
         (!kvm_enabled() || !qemu_kvm_irqchip_in_kernel()))