Patchwork temporary fix for on_vcpu

login
register
mail settings
Submitter Glauber Costa
Date Sept. 11, 2009, 2:41 p.m.
Message ID <1252680092-5208-1-git-send-email-glommer@redhat.com>
Download mbox | patch
Permalink /patch/33465/
State Superseded
Headers show

Comments

Glauber Costa - Sept. 11, 2009, 2:41 p.m.
Recent changes made on_vcpu hit the abort() path, even with the IO thread
disabled. This is because cpu_single_env is no longer set when we call this
function. Although the correct fix is a little bit more complicated that that,
the recent thread in which I proposed qemu_queue_work (which fixes that, btw),
is likely to go on a quite different direction.

So for the benefit of those using guest debugging, I'm proposing this simple
fix in the interim.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm-all.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Jan Kiszka - Sept. 11, 2009, 3:56 p.m.
Glauber Costa wrote:
> Recent changes made on_vcpu hit the abort() path, even with the IO thread
> disabled. This is because cpu_single_env is no longer set when we call this
> function. Although the correct fix is a little bit more complicated that that,
> the recent thread in which I proposed qemu_queue_work (which fixes that, btw),
> is likely to go on a quite different direction.
> 
> So for the benefit of those using guest debugging, I'm proposing this simple
> fix in the interim.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  kvm-all.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index df4e849..2c24440 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -902,11 +902,15 @@ void kvm_setup_guest_memory(void *start, size_t size)
>  #ifdef KVM_CAP_SET_GUEST_DEBUG
>  static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
>  {
> +#ifdef CONFIG_IOTHREAD
>      if (env == cpu_single_env) {
>          func(data);
>          return;
>      }
>      abort();
> +#else
> +   func(data);

spaces++ :) But the workaround works.

> +#endif
>  }
>  
>  struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env,

Unless there is hope to fix kvm in iothread mode soon, we should issue a
warning or even disable kvm support in that setup. That is particularly
important for 0.11-stable.

Jan
Glauber Costa - Sept. 11, 2009, 4:12 p.m.
On Fri, Sep 11, 2009 at 05:56:08PM +0200, Jan Kiszka wrote:
> Glauber Costa wrote:
> > Recent changes made on_vcpu hit the abort() path, even with the IO thread
> > disabled. This is because cpu_single_env is no longer set when we call this
> > function. Although the correct fix is a little bit more complicated that that,
> > the recent thread in which I proposed qemu_queue_work (which fixes that, btw),
> > is likely to go on a quite different direction.
> > 
> > So for the benefit of those using guest debugging, I'm proposing this simple
> > fix in the interim.
> > 
> > Signed-off-by: Glauber Costa <glommer@redhat.com>
> > ---
> >  kvm-all.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/kvm-all.c b/kvm-all.c
> > index df4e849..2c24440 100644
> > --- a/kvm-all.c
> > +++ b/kvm-all.c
> > @@ -902,11 +902,15 @@ void kvm_setup_guest_memory(void *start, size_t size)
> >  #ifdef KVM_CAP_SET_GUEST_DEBUG
> >  static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
> >  {
> > +#ifdef CONFIG_IOTHREAD
> >      if (env == cpu_single_env) {
> >          func(data);
> >          return;
> >      }
> >      abort();
> > +#else
> > +   func(data);
> 
> spaces++ :) But the workaround works.
> 
> > +#endif
> >  }
> >  
> >  struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env,
> 
> Unless there is hope to fix kvm in iothread mode soon, we should issue a
> warning or even disable kvm support in that setup. That is particularly
> important for 0.11-stable.
As I said, with the fixes I sent recently, it should work pretty well.
Jan Kiszka - Sept. 11, 2009, 4:18 p.m.
Glauber Costa wrote:
> On Fri, Sep 11, 2009 at 05:56:08PM +0200, Jan Kiszka wrote:
>> Glauber Costa wrote:
>>> Recent changes made on_vcpu hit the abort() path, even with the IO thread
>>> disabled. This is because cpu_single_env is no longer set when we call this
>>> function. Although the correct fix is a little bit more complicated that that,
>>> the recent thread in which I proposed qemu_queue_work (which fixes that, btw),
>>> is likely to go on a quite different direction.
>>>
>>> So for the benefit of those using guest debugging, I'm proposing this simple
>>> fix in the interim.
>>>
>>> Signed-off-by: Glauber Costa <glommer@redhat.com>
>>> ---
>>>  kvm-all.c |    4 ++++
>>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> index df4e849..2c24440 100644
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -902,11 +902,15 @@ void kvm_setup_guest_memory(void *start, size_t size)
>>>  #ifdef KVM_CAP_SET_GUEST_DEBUG
>>>  static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
>>>  {
>>> +#ifdef CONFIG_IOTHREAD
>>>      if (env == cpu_single_env) {
>>>          func(data);
>>>          return;
>>>      }
>>>      abort();
>>> +#else
>>> +   func(data);
>> spaces++ :) But the workaround works.
>>
>>> +#endif
>>>  }
>>>  
>>>  struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env,
>> Unless there is hope to fix kvm in iothread mode soon, we should issue a
>> warning or even disable kvm support in that setup. That is particularly
>> important for 0.11-stable.
> As I said, with the fixes I sent recently, it should work pretty well.

You are referring to the workqueue things? Or "do proper cpu_self check"
and the corresponding 2/2 (which I do not find in the archives)?

Jan
Glauber Costa - Sept. 11, 2009, 6:35 p.m.
On Fri, Sep 11, 2009 at 06:18:02PM +0200, Jan Kiszka wrote:
> Glauber Costa wrote:
> > On Fri, Sep 11, 2009 at 05:56:08PM +0200, Jan Kiszka wrote:
> >> Glauber Costa wrote:
> >>> Recent changes made on_vcpu hit the abort() path, even with the IO thread
> >>> disabled. This is because cpu_single_env is no longer set when we call this
> >>> function. Although the correct fix is a little bit more complicated that that,
> >>> the recent thread in which I proposed qemu_queue_work (which fixes that, btw),
> >>> is likely to go on a quite different direction.
> >>>
> >>> So for the benefit of those using guest debugging, I'm proposing this simple
> >>> fix in the interim.
> >>>
> >>> Signed-off-by: Glauber Costa <glommer@redhat.com>
> >>> ---
> >>>  kvm-all.c |    4 ++++
> >>>  1 files changed, 4 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/kvm-all.c b/kvm-all.c
> >>> index df4e849..2c24440 100644
> >>> --- a/kvm-all.c
> >>> +++ b/kvm-all.c
> >>> @@ -902,11 +902,15 @@ void kvm_setup_guest_memory(void *start, size_t size)
> >>>  #ifdef KVM_CAP_SET_GUEST_DEBUG
> >>>  static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
> >>>  {
> >>> +#ifdef CONFIG_IOTHREAD
> >>>      if (env == cpu_single_env) {
> >>>          func(data);
> >>>          return;
> >>>      }
> >>>      abort();
> >>> +#else
> >>> +   func(data);
> >> spaces++ :) But the workaround works.
> >>
> >>> +#endif
> >>>  }
> >>>  
> >>>  struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env,
> >> Unless there is hope to fix kvm in iothread mode soon, we should issue a
> >> warning or even disable kvm support in that setup. That is particularly
> >> important for 0.11-stable.
> > As I said, with the fixes I sent recently, it should work pretty well.
> 
> You are referring to the workqueue things? Or "do proper cpu_self check"
> and the corresponding 2/2 (which I do not find in the archives)?
The later.
The 1/2 numbering is by bad. It is the way it was in my personal tree. The
other patch is already sent a while ago. Not sure what anthony did of it.

If anthony prefer, I can resend both to make it easier to pick

Patch

diff --git a/kvm-all.c b/kvm-all.c
index df4e849..2c24440 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -902,11 +902,15 @@  void kvm_setup_guest_memory(void *start, size_t size)
 #ifdef KVM_CAP_SET_GUEST_DEBUG
 static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
 {
+#ifdef CONFIG_IOTHREAD
     if (env == cpu_single_env) {
         func(data);
         return;
     }
     abort();
+#else
+   func(data);
+#endif
 }
 
 struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env,