[Qemu-devel] [PATCH 0/2] cpu->exception_index fixes

[Qemu-devel] [PATCH 0/2] cpu->exception_index fixes

[Paolo Bonzini]

Two fixes for cpu->exception_index:

- breakage of linux-user from commit e511b4d

- wrong subsection definition in 2.2 (see patch 2)

Paolo

Paolo Bonzini (2):
  cpu: initialize cpu->exception_index on reset
  exec: change default exception_index value for migration to -1

 cpus.c    | 3 ---
 exec.c    | 4 ++--
 qom/cpu.c | 1 +
 3 files changed, 3 insertions(+), 5 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] cpu: initialize cpu->exception_index on reset

[Paolo Bonzini]

This unbreaks linux-user (broken by e511b4d, cpu-exec: reset exception_index
correctly, 2014-11-26).

Reported-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c    | 3 ---
 qom/cpu.c | 1 +
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/cpus.c b/cpus.c
index 1b5168a..2edb5cd 100644
--- a/cpus.c
+++ b/cpus.c
@@ -940,7 +940,6 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
     qemu_mutex_lock(&qemu_global_mutex);
     qemu_thread_get_self(cpu->thread);
     cpu->thread_id = qemu_get_thread_id();
-    cpu->exception_index = -1;
     cpu->can_do_io = 1;
     current_cpu = cpu;
 
@@ -982,7 +981,6 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
     qemu_mutex_lock_iothread();
     qemu_thread_get_self(cpu->thread);
     cpu->thread_id = qemu_get_thread_id();
-    cpu->exception_index = -1;
     cpu->can_do_io = 1;
 
     sigemptyset(&waitset);
@@ -1026,7 +1024,6 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
     CPU_FOREACH(cpu) {
         cpu->thread_id = qemu_get_thread_id();
         cpu->created = true;
-        cpu->exception_index = -1;
         cpu->can_do_io = 1;
     }
     qemu_cond_signal(&qemu_cpu_cond);
diff --git a/qom/cpu.c b/qom/cpu.c
index 79d2228..9c68fa4 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -249,6 +249,7 @@ static void cpu_common_reset(CPUState *cpu)
     cpu->icount_extra = 0;
     cpu->icount_decr.u32 = 0;
     cpu->can_do_io = 0;
+    cpu->exception_index = -1;
     memset(cpu->tb_jmp_cache, 0, TB_JMP_CACHE_SIZE * sizeof(void *));
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] exec: change default exception_index value for migration to -1

[Paolo Bonzini]

In QEMU 2.2 the exception_index value was added to the migration stream
through a subsection.  The default was set to 0, which is wrong and
should have been -1.

However, 2.2 does not have commit e511b4d (cpu-exec: reset exception_index
correctly, 2014-11-26), hence in 2.2 the exception_index is never used
and is set to -1 on the next call to cpu_exec.  So we can change the
migration stream to make the default -1.  The effects are:

- 2.2.1 -> 2.2.0: cpu->exception_index set incorrectly to 0 if it
were -1 on the source; then reset to -1 in cpu_exec.  This is TCG
only; KVM does not use exception_index.

- 2.2.0 -> 2.2.1: cpu->exception_index set incorrectly to -1 if it
were 0 on the source; but it would be reset to -1 in cpu_exec anyway.
This is TCG only; KVM does not use exception_index.

- 2.2.1 -> 2.1: two bugs fixed: 1) can migrate backwards if
cpu->exception_index is set to -1; 2) should not migrate backwards
(but 2.2.0 allows it) if cpu->exception_index is set to 0

- 2.2.0 -> 2.3.0: 2.2.0 will send the subsection unnecessarily if
exception_index is -1, but that is not a problem.  2.3.0 will set
cpu->exception_index to -1 if it is 0 on the source, but this would
be anyway a problem for 2.2.0 -> 2.2.x migration (due to lack of
commit e511b4d in 2.2.x) so we can ignore it

- 2.2.1 -> 2.3.0: everything works.

In addition, play it safe and never send the subsection unless TCG
is in use.  KVM does not use exception_index (PPC KVM stores values
in it for use in the subsequent call to ppc_cpu_do_interrupt, but
does not need it as soon as kvm_handle_debug returns).  Xen and
qtest do not run any code for the CPU at all.

Reported-by: Igor Mammedov <imammedo@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index 963481a..c2ed10a 100644
--- a/exec.c
+++ b/exec.c
@@ -434,7 +434,7 @@ static int cpu_common_pre_load(void *opaque)
 {
     CPUState *cpu = opaque;
 
-    cpu->exception_index = 0;
+    cpu->exception_index = -1;
 
     return 0;
 }
@@ -443,7 +443,7 @@ static bool cpu_common_exception_index_needed(void *opaque)
 {
     CPUState *cpu = opaque;
 
-    return cpu->exception_index != 0;
+    return tcg_enabled() && cpu->exception_index != -1;
 }
 
 static const VMStateDescription vmstate_cpu_common_exception_index = {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/2] cpu->exception_index fixes

[Laurent Desnogues]

Hello,

On Fri, Dec 19, 2014 at 12:53 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Two fixes for cpu->exception_index:
>
> - breakage of linux-user from commit e511b4d

Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com>

Thanks,

Laurent

> - wrong subsection definition in 2.2 (see patch 2)
>
> Paolo
>
> Paolo Bonzini (2):
>   cpu: initialize cpu->exception_index on reset
>   exec: change default exception_index value for migration to -1
>
>  cpus.c    | 3 ---
>  exec.c    | 4 ++--
>  qom/cpu.c | 1 +
>  3 files changed, 3 insertions(+), 5 deletions(-)
>
> --
> 1.8.3.1
>
>

Re: [Qemu-devel] [PATCH 1/2] cpu: initialize cpu->exception_index on reset

[Eduardo Habkost]

On Fri, Dec 19, 2014 at 12:53:13PM +0100, Paolo Bonzini wrote:
> This unbreaks linux-user (broken by e511b4d, cpu-exec: reset exception_index
> correctly, 2014-11-26).
> 
> Reported-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Fixes the problem. Thanks!

Tested-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/2] cpu->exception_index fixes

[Peter Maydell]

On 19 December 2014 at 11:53, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Two fixes for cpu->exception_index:
>
> - breakage of linux-user from commit e511b4d
>
> - wrong subsection definition in 2.2 (see patch 2)
>
> Paolo
>
> Paolo Bonzini (2):
>   cpu: initialize cpu->exception_index on reset
>   exec: change default exception_index value for migration to -1

Thanks; applied to master as a bugfix. I have also updated my
pull-buildtest script to do a basic "check linux-user isn't totally
busted" step :-) [using the linux-user test tarball on the wiki].

-- PMM