[Qemu-devel] [PULL 00/23] Misc patches for 2017-02-16

[Qemu-devel] [PULL 00/23] Misc patches for 2017-02-16

[Paolo Bonzini]

The following changes since commit 5dae13cd71f0755a1395b5a4cde635b8a6ee3f58:

  Merge remote-tracking branch 'remotes/rth/tags/pull-or-20170214' into staging (2017-02-14 09:55:48 +0000)

are available in the git repository at:


  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 395d41a549f69f4ec603afa3d3ade1e2005777d5:

  target-i386: correctly propagate retaddr into SVM helpers (2017-02-16 15:30:49 +0100)

----------------------------------------------------------------
* GUEST_PANICKED improvements (Anton)
* vCont gdbstub rewrite (Claudio)
* Fix CPU creation with -device (Liyang)
* Logging fixes for pty chardevs (Ed)
* Makefile "move if changed" fix (Lin)
* First part of cpu_exec refactoring (me)
* SVM emulation fix (me)
* apic_delivered fix (Pavel)
* "info ioapic" fix (Peter)
* qemu-nbd socket activation (Richard)
* QOMification of mcf_uart (Thomas)

----------------------------------------------------------------
Alberto Garcia (1):
      qemu-doc: Clarify that -vga std is now the default

Anton Nefedov (4):
      qemu-char: socket backend: disconnect on write error
      i386/cpu: add crash-information QOM property
      report guest crash information in GUEST_PANICKED event
      vl: log available guest crash information

Claudio Imbrenda (2):
      move vm_start to cpus.c
      gdbstub: Fix vCont behaviour

Dou Liyang (1):
      vl: Move the cpu_synchronize_all_post_init() after generic devices initialization

Ed Swierk (1):
      char: drop data written to a disconnected pty

Lin Ma (1):
      Makefile: avoid leaving the temporary QEMU_PKGVERSION header file

Paolo Bonzini (7):
      test-vmstate: remove yield_until_fd_readable
      cpu-exec: fix icount out-of-bounds access
      cpu-exec: tighten barrier on TCG_EXIT_REQUESTED
      cpu-exec: avoid cpu_loop_exit in cpu_handle_interrupt
      cpu-exec: avoid repeated sigsetjmp on interrupts
      cpu-exec: remove outermost infinite loop
      target-i386: correctly propagate retaddr into SVM helpers

Pavel Dovgalyuk (1):
      apic: reset apic_delivered global variable on machine reset

Peter Xu (3):
      kvm/ioapic: dump real object instead of a fake one
      ioapic: fix error report value of def version
      kvm/ioapic: correct kvm ioapic version

Richard W.M. Jones (1):
      qemu-nbd: Implement socket activation.

Thomas Huth (1):
      hw/char/mcf_uart: QOMify the ColdFire UART

 Makefile                  |   6 +-
 chardev/char-pty.c        |   2 +-
 chardev/char-socket.c     |  10 +++
 cpu-exec.c                |  86 ++++++++++---------
 cpus.c                    |  42 ++++++++++
 gdbstub.c                 | 209 +++++++++++++++++++++++++++++++++++-----------
 hw/char/mcf_uart.c        | 102 ++++++++++++++++------
 hw/i386/kvm/ioapic.c      |  13 ++-
 hw/intc/apic_common.c     |   2 +
 hw/intc/ioapic.c          |   6 +-
 hw/m68k/mcf5208.c         |   6 +-
 hw/misc/pvpanic.c         |   2 +-
 hw/ppc/spapr_rtas.c       |   3 +-
 include/exec/exec-all.h   |   1 +
 include/hw/m68k/mcf.h     |   6 +-
 include/qom/cpu.h         |  10 +++
 include/sysemu/sysemu.h   |   4 +-
 kvm-all.c                 |   3 +-
 qapi-schema.json          |  24 ++++++
 qapi/event.json           |   6 +-
 qemu-nbd.c                | 172 ++++++++++++++++++++++++++++++++++++--
 qemu-options.hx           |   4 +-
 qom/cpu.c                 |  11 +++
 target/i386/cpu.c         |  51 +++++++++++
 target/i386/cpu.h         |   5 +-
 target/i386/excp_helper.c |  11 +--
 target/i386/helper.h      |   1 -
 target/i386/misc_helper.c |  24 +++---
 target/i386/seg_helper.c  |   6 +-
 target/i386/svm_helper.c  |  65 +++++++-------
 target/s390x/kvm.c        |   4 +-
 tests/test-vmstate.c      |  11 ---
 vl.c                      |  56 +++++--------
 33 files changed, 711 insertions(+), 253 deletions(-)
-- 
1.8.3.1

[Qemu-devel] [PULL 01/23] kvm/ioapic: dump real object instead of a fake one

[Paolo Bonzini]

From: Peter Xu <peterx@redhat.com>

When we do "info ioapic" for kvm ioapic, we were building up a temporary
ioapic object. Let's fetch the real one and update correspond to the
real object as well.

This fixes printing uninitialized version field in
ioapic_print_redtbl().

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <1486106298-3699-2-git-send-email-peterx@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/kvm/ioapic.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index 8eb2c7a..716d59a 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -114,11 +114,11 @@ static void kvm_ioapic_put(IOAPICCommonState *s)
 
 void kvm_ioapic_dump_state(Monitor *mon, const QDict *qdict)
 {
-    IOAPICCommonState s;
+    IOAPICCommonState *s = IOAPIC_COMMON(object_resolve_path("ioapic", NULL));
 
-    kvm_ioapic_get(&s);
-
-    ioapic_print_redtbl(mon, &s);
+    assert(s);
+    kvm_ioapic_get(s);
+    ioapic_print_redtbl(mon, s);
 }
 
 static void kvm_ioapic_reset(DeviceState *dev)
-- 
1.8.3.1

[Qemu-devel] [PULL 02/23] ioapic: fix error report value of def version

[Paolo Bonzini]

From: Peter Xu <peterx@redhat.com>

It should be 0x20, rather than 0x11.

Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <1486106298-3699-3-git-send-email-peterx@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/intc/ioapic.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 9047b89..37c4386 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -408,13 +408,15 @@ static void ioapic_machine_done_notify(Notifier *notifier, void *data)
 #endif
 }
 
+#define IOAPIC_VER_DEF 0x20
+
 static void ioapic_realize(DeviceState *dev, Error **errp)
 {
     IOAPICCommonState *s = IOAPIC_COMMON(dev);
 
     if (s->version != 0x11 && s->version != 0x20) {
         error_report("IOAPIC only supports version 0x11 or 0x20 "
-                     "(default: 0x11).");
+                     "(default: 0x%x).", IOAPIC_VER_DEF);
         exit(1);
     }
 
@@ -429,7 +431,7 @@ static void ioapic_realize(DeviceState *dev, Error **errp)
 }
 
 static Property ioapic_properties[] = {
-    DEFINE_PROP_UINT8("version", IOAPICCommonState, version, 0x20),
+    DEFINE_PROP_UINT8("version", IOAPICCommonState, version, IOAPIC_VER_DEF),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.8.3.1

[Qemu-devel] [PULL 03/23] kvm/ioapic: correct kvm ioapic version

[Paolo Bonzini]

From: Peter Xu <peterx@redhat.com>

Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <1486106298-3699-4-git-send-email-peterx@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/kvm/ioapic.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index 716d59a..98ca480 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -143,6 +143,11 @@ static void kvm_ioapic_realize(DeviceState *dev, Error **errp)
     IOAPICCommonState *s = IOAPIC_COMMON(dev);
 
     memory_region_init_reservation(&s->io_memory, NULL, "kvm-ioapic", 0x1000);
+    /*
+     * KVM ioapic only supports 0x11 now. This will only be used when
+     * we want to dump ioapic version.
+     */
+    s->version = 0x11;
 
     qdev_init_gpio_in(dev, kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 04/23] test-vmstate: remove yield_until_fd_readable

[Paolo Bonzini]

The function is not needed anymore now that migration is built on
top of QIOChannel.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/test-vmstate.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index d0dd390..39f338a4 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -33,17 +33,6 @@
 static char temp_file[] = "/tmp/vmst.test.XXXXXX";
 static int temp_fd;
 
-/* Fake yield_until_fd_readable() implementation so we don't have to pull the
- * coroutine code as dependency.
- */
-void yield_until_fd_readable(int fd)
-{
-    fd_set fds;
-    FD_ZERO(&fds);
-    FD_SET(fd, &fds);
-    select(fd + 1, &fds, NULL, NULL, NULL);
-}
-
 
 /* Duplicate temp_fd and seek to the beginning of the file */
 static QEMUFile *open_test_file(bool write)
-- 
1.8.3.1

[Qemu-devel] [PULL 05/23] qemu-char: socket backend: disconnect on write error

[Paolo Bonzini]

From: Anton Nefedov <anton.nefedov@virtuozzo.com>

Socket backend read handler should normally perform a disconnect, however
the read handler may not get a chance to run if the frontend is not ready
(qemu_chr_be_can_write() == 0).

This means that in virtio-serial frontend case if
 - the host has disconnected (giving EPIPE on socket write)
 - and the guest has disconnected (-> frontend not ready -> backend
   will not read)
 - and there is still data (frontend->backend) to flush (has to be a really
   tricky timing but nevertheless, we have observed the case in production)

This results in virtio-serial trying to flush this data continiously forming
a busy loop.

Solution: react on write error in the socket write handler.
errno is not reliable after qio_channel_writev_full(), so we may not get
the exact EPIPE, so disconnect on any error but QIO_CHANNEL_ERR_BLOCK which
io_channel_send_full() converts to errno EAGAIN.
We must not disconnect right away though, there still may be data to read
(see 4bf1cb0).

Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Daniel P. Berrange <berrange@redhat.com>
CC: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1486045589-8074-1-git-send-email-den@openvz.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 chardev/char-socket.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 4068dc5..865c527 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -97,6 +97,9 @@ static gboolean tcp_chr_accept(QIOChannel *chan,
                                GIOCondition cond,
                                void *opaque);
 
+static int tcp_chr_read_poll(void *opaque);
+static void tcp_chr_disconnect(Chardev *chr);
+
 /* Called with chr_write_lock held.  */
 static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len)
 {
@@ -114,6 +117,13 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len)
             s->write_msgfds_num = 0;
         }
 
+        if (ret < 0 && errno != EAGAIN) {
+            if (tcp_chr_read_poll(chr) <= 0) {
+                tcp_chr_disconnect(chr);
+                return len;
+            } /* else let the read handler finish it properly */
+        }
+
         return ret;
     } else {
         /* XXX: indicate an error ? */
-- 
1.8.3.1

[Qemu-devel] [PULL 06/23] apic: reset apic_delivered global variable on machine reset

[Paolo Bonzini]

From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>

This patch adds call to apic_reset_irq_delivered when the virtual
machine is reset.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
Message-Id: <20170131114054.276.62201.stgit@PASHA-ISP>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/intc/apic_common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 6ce8ef7..7a6e771 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -251,6 +251,8 @@ static void apic_reset_common(DeviceState *dev)
     s->apicbase = APIC_DEFAULT_ADDRESS | bsp | MSR_IA32_APICBASE_ENABLE;
     s->id = s->initial_apic_id;
 
+    apic_reset_irq_delivered();
+
     s->vapic_paddr = 0;
     info->vapic_base_update(s);
 
-- 
1.8.3.1

[Qemu-devel] [PULL 07/23] char: drop data written to a disconnected pty

[Paolo Bonzini]

From: Ed Swierk <eswierk@skyportsystems.com>

When a serial port writes data to a pty that's disconnected, drop the
data and return the length dropped. This avoids triggering pointless
retries in callers like the 16550A serial_xmit(), and causes
qemu_chr_fe_write() to write all data to the log file, rather than
logging only while a pty client like virsh console happens to be
connected.

Signed-off-by: Ed Swierk <eswierk@skyportsystems.com>
Message-Id: <1485870329-79428-1-git-send-email-eswierk@skyportsystems.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 chardev/char-pty.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 27eb85f..ecf2c7a 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -129,7 +129,7 @@ static int char_pty_chr_write(Chardev *chr, const uint8_t *buf, int len)
         /* guest sends data, check for (re-)connect */
         pty_chr_update_read_handler_locked(chr);
         if (!s->connected) {
-            return 0;
+            return len;
         }
     }
     return io_channel_send(s->ioc, buf, len);
-- 
1.8.3.1

[Qemu-devel] [PULL 08/23] move vm_start to cpus.c

[Paolo Bonzini]

From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>

This patch:

* moves vm_start to cpus.c.
* exports qemu_vmstop_requested, since it's needed by vm_start.
* extracts vm_prepare_start from vm_start; it does what vm_start did,
  except restarting the cpus.
* vm_start now calls vm_prepare_start and then restarts the cpus.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
Message-Id: <1487092068-16562-2-git-send-email-imbrenda@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c                  | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/sysemu/sysemu.h |  2 ++
 vl.c                    | 30 +-----------------------------
 3 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/cpus.c b/cpus.c
index 71a82e5..0bcb5b5 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1578,6 +1578,48 @@ int vm_stop(RunState state)
     return do_vm_stop(state);
 }
 
+/**
+ * Prepare for (re)starting the VM.
+ * Returns -1 if the vCPUs are not to be restarted (e.g. if they are already
+ * running or in case of an error condition), 0 otherwise.
+ */
+int vm_prepare_start(void)
+{
+    RunState requested;
+    int res = 0;
+
+    qemu_vmstop_requested(&requested);
+    if (runstate_is_running() && requested == RUN_STATE__MAX) {
+        return -1;
+    }
+
+    /* Ensure that a STOP/RESUME pair of events is emitted if a
+     * vmstop request was pending.  The BLOCK_IO_ERROR event, for
+     * example, according to documentation is always followed by
+     * the STOP event.
+     */
+    if (runstate_is_running()) {
+        qapi_event_send_stop(&error_abort);
+        res = -1;
+    } else {
+        replay_enable_events();
+        cpu_enable_ticks();
+        runstate_set(RUN_STATE_RUNNING);
+        vm_state_notify(1, RUN_STATE_RUNNING);
+    }
+
+    /* We are sending this now, but the CPUs will be resumed shortly later */
+    qapi_event_send_resume(&error_abort);
+    return res;
+}
+
+void vm_start(void)
+{
+    if (!vm_prepare_start()) {
+        resume_all_vcpus();
+    }
+}
+
 /* does a state transition even if the VM is already stopped,
    current state is forgotten forever */
 int vm_stop_force_state(RunState state)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 4d50694..eda5241 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -37,6 +37,7 @@ void vm_state_notify(int running, RunState state);
 #define VMRESET_REPORT   true
 
 void vm_start(void);
+int vm_prepare_start(void);
 int vm_stop(RunState state);
 int vm_stop_force_state(RunState state);
 
@@ -60,6 +61,7 @@ void qemu_register_powerdown_notifier(Notifier *notifier);
 void qemu_system_debug_request(void);
 void qemu_system_vmstop_request(RunState reason);
 void qemu_system_vmstop_request_prepare(void);
+bool qemu_vmstop_requested(RunState *r);
 int qemu_shutdown_requested_get(void);
 int qemu_reset_requested_get(void);
 void qemu_system_killed(int signal, pid_t pid);
diff --git a/vl.c b/vl.c
index b4eaf03..0d866ad 100644
--- a/vl.c
+++ b/vl.c
@@ -724,7 +724,7 @@ StatusInfo *qmp_query_status(Error **errp)
     return info;
 }
 
-static bool qemu_vmstop_requested(RunState *r)
+bool qemu_vmstop_requested(RunState *r)
 {
     qemu_mutex_lock(&vmstop_lock);
     *r = vmstop_requested;
@@ -745,34 +745,6 @@ void qemu_system_vmstop_request(RunState state)
     qemu_notify_event();
 }
 
-void vm_start(void)
-{
-    RunState requested;
-
-    qemu_vmstop_requested(&requested);
-    if (runstate_is_running() && requested == RUN_STATE__MAX) {
-        return;
-    }
-
-    /* Ensure that a STOP/RESUME pair of events is emitted if a
-     * vmstop request was pending.  The BLOCK_IO_ERROR event, for
-     * example, according to documentation is always followed by
-     * the STOP event.
-     */
-    if (runstate_is_running()) {
-        qapi_event_send_stop(&error_abort);
-    } else {
-        replay_enable_events();
-        cpu_enable_ticks();
-        runstate_set(RUN_STATE_RUNNING);
-        vm_state_notify(1, RUN_STATE_RUNNING);
-        resume_all_vcpus();
-    }
-
-    qapi_event_send_resume(&error_abort);
-}
-
-
 /***********************************************************/
 /* real time host monotonic timer */
 
-- 
1.8.3.1

[Qemu-devel] [PULL 09/23] gdbstub: Fix vCont behaviour

[Paolo Bonzini]

From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>

When GDB issues a "vCont", QEMU was not handling it correctly when
multiple VCPUs are active.
For vCont, for each thread (VCPU), it can be specified whether to
single step, continue or stop that thread. The default is to stop a
thread.
However, when (for example) "vCont;s:2" is issued, all VCPUs continue
to run, although all but VCPU nr 2 are to be stopped.

This patch completely rewrites the vCont parsing code.

Please note that this improvement only works in system emulation mode,
when in userspace emulation mode the old behaviour is preserved.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
Message-Id: <1487092068-16562-3-git-send-email-imbrenda@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 gdbstub.c | 209 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 162 insertions(+), 47 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 755a8e3..9911153 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -387,6 +387,60 @@ static inline void gdb_continue(GDBState *s)
 #endif
 }
 
+/*
+ * Resume execution, per CPU actions. For user-mode emulation it's
+ * equivalent to gdb_continue.
+ */
+static int gdb_continue_partial(GDBState *s, char *newstates)
+{
+    CPUState *cpu;
+    int res = 0;
+#ifdef CONFIG_USER_ONLY
+    /*
+     * This is not exactly accurate, but it's an improvement compared to the
+     * previous situation, where only one CPU would be single-stepped.
+     */
+    CPU_FOREACH(cpu) {
+        if (newstates[cpu->cpu_index] == 's') {
+            cpu_single_step(cpu, sstep_flags);
+        }
+    }
+    s->running_state = 1;
+#else
+    int flag = 0;
+
+    if (!runstate_needs_reset()) {
+        if (vm_prepare_start()) {
+            return 0;
+        }
+
+        CPU_FOREACH(cpu) {
+            switch (newstates[cpu->cpu_index]) {
+            case 0:
+            case 1:
+                break; /* nothing to do here */
+            case 's':
+                cpu_single_step(cpu, sstep_flags);
+                cpu_resume(cpu);
+                flag = 1;
+                break;
+            case 'c':
+                cpu_resume(cpu);
+                flag = 1;
+                break;
+            default:
+                res = -1;
+                break;
+            }
+        }
+    }
+    if (flag) {
+        qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true);
+    }
+#endif
+    return res;
+}
+
 static void put_buffer(GDBState *s, const uint8_t *buf, int len)
 {
 #ifdef CONFIG_USER_ONLY
@@ -785,6 +839,107 @@ static int is_query_packet(const char *p, const char *query, char separator)
         (p[query_len] == '\0' || p[query_len] == separator);
 }
 
+/**
+ * gdb_handle_vcont - Parses and handles a vCont packet.
+ * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
+ *         a format error, 0 on success.
+ */
+static int gdb_handle_vcont(GDBState *s, const char *p)
+{
+    int res, idx, signal = 0;
+    char cur_action;
+    char *newstates;
+    unsigned long tmp;
+    CPUState *cpu;
+#ifdef CONFIG_USER_ONLY
+    int max_cpus = 1; /* global variable max_cpus exists only in system mode */
+
+    CPU_FOREACH(cpu) {
+        max_cpus = max_cpus <= cpu->cpu_index ? cpu->cpu_index + 1 : max_cpus;
+    }
+#endif
+    /* uninitialised CPUs stay 0 */
+    newstates = g_new0(char, max_cpus);
+
+    /* mark valid CPUs with 1 */
+    CPU_FOREACH(cpu) {
+        newstates[cpu->cpu_index] = 1;
+    }
+
+    /*
+     * res keeps track of what error we are returning, with -ENOTSUP meaning
+     * that the command is unknown or unsupported, thus returning an empty
+     * packet, while -EINVAL and -ERANGE cause an E22 packet, due to invalid,
+     *  or incorrect parameters passed.
+     */
+    res = 0;
+    while (*p) {
+        if (*p++ != ';') {
+            res = -ENOTSUP;
+            goto out;
+        }
+
+        cur_action = *p++;
+        if (cur_action == 'C' || cur_action == 'S') {
+            cur_action = tolower(cur_action);
+            res = qemu_strtoul(p + 1, &p, 16, &tmp);
+            if (res) {
+                goto out;
+            }
+            signal = gdb_signal_to_target(tmp);
+        } else if (cur_action != 'c' && cur_action != 's') {
+            /* unknown/invalid/unsupported command */
+            res = -ENOTSUP;
+            goto out;
+        }
+        /* thread specification. special values: (none), -1 = all; 0 = any */
+        if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
+            if (*p == ':') {
+                p += 3;
+            }
+            for (idx = 0; idx < max_cpus; idx++) {
+                if (newstates[idx] == 1) {
+                    newstates[idx] = cur_action;
+                }
+            }
+        } else if (*p == ':') {
+            p++;
+            res = qemu_strtoul(p, &p, 16, &tmp);
+            if (res) {
+                goto out;
+            }
+            idx = tmp;
+            /* 0 means any thread, so we pick the first valid CPU */
+            if (!idx) {
+                idx = cpu_index(first_cpu);
+            }
+
+            /*
+             * If we are in user mode, the thread specified is actually a
+             * thread id, and not an index. We need to find the actual
+             * CPU first, and only then we can use its index.
+             */
+            cpu = find_cpu(idx);
+            /* invalid CPU/thread specified */
+            if (!idx || !cpu) {
+                res = -EINVAL;
+                goto out;
+            }
+            /* only use if no previous match occourred */
+            if (newstates[cpu->cpu_index] == 1) {
+                newstates[cpu->cpu_index] = cur_action;
+            }
+        }
+    }
+    s->signal = signal;
+    gdb_continue_partial(s, newstates);
+
+out:
+    g_free(newstates);
+
+    return res;
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -830,60 +985,20 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         return RS_IDLE;
     case 'v':
         if (strncmp(p, "Cont", 4) == 0) {
-            int res_signal, res_thread;
-
             p += 4;
             if (*p == '?') {
                 put_packet(s, "vCont;c;C;s;S");
                 break;
             }
-            res = 0;
-            res_signal = 0;
-            res_thread = 0;
-            while (*p) {
-                int action, signal;
-
-                if (*p++ != ';') {
-                    res = 0;
-                    break;
-                }
-                action = *p++;
-                signal = 0;
-                if (action == 'C' || action == 'S') {
-                    signal = gdb_signal_to_target(strtoul(p, (char **)&p, 16));
-                    if (signal == -1) {
-                        signal = 0;
-                    }
-                } else if (action != 'c' && action != 's') {
-                    res = 0;
-                    break;
-                }
-                thread = 0;
-                if (*p == ':') {
-                    thread = strtoull(p+1, (char **)&p, 16);
-                }
-                action = tolower(action);
-                if (res == 0 || (res == 'c' && action == 's')) {
-                    res = action;
-                    res_signal = signal;
-                    res_thread = thread;
-                }
-            }
+
+            res = gdb_handle_vcont(s, p);
+
             if (res) {
-                if (res_thread != -1 && res_thread != 0) {
-                    cpu = find_cpu(res_thread);
-                    if (cpu == NULL) {
-                        put_packet(s, "E22");
-                        break;
-                    }
-                    s->c_cpu = cpu;
-                }
-                if (res == 's') {
-                    cpu_single_step(s->c_cpu, sstep_flags);
+                if ((res == -EINVAL) || (res == -ERANGE)) {
+                    put_packet(s, "E22");
+                    break;
                 }
-                s->signal = res_signal;
-                gdb_continue(s);
-                return RS_IDLE;
+                goto unknown_command;
             }
             break;
         } else {
-- 
1.8.3.1

[Qemu-devel] [PULL 10/23] hw/char/mcf_uart: QOMify the ColdFire UART

[Paolo Bonzini]

From: Thomas Huth <huth@tuxfamily.org>

Use type_init() etc. to adapt the ColdFire UART
to the latest QEMU device conventions.

Signed-off-by: Thomas Huth <huth@tuxfamily.org>
Message-Id: <1485586582-6490-1-git-send-email-huth@tuxfamily.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/char/mcf_uart.c    | 102 +++++++++++++++++++++++++++++++++++++-------------
 hw/m68k/mcf5208.c     |   6 +--
 include/hw/m68k/mcf.h |   6 +--
 3 files changed, 81 insertions(+), 33 deletions(-)

diff --git a/hw/char/mcf_uart.c b/hw/char/mcf_uart.c
index 80c380e..e69672f 100644
--- a/hw/char/mcf_uart.c
+++ b/hw/char/mcf_uart.c
@@ -7,12 +7,15 @@
  */
 #include "qemu/osdep.h"
 #include "hw/hw.h"
+#include "hw/sysbus.h"
 #include "hw/m68k/mcf.h"
 #include "sysemu/char.h"
 #include "exec/address-spaces.h"
 #include "qapi/error.h"
 
 typedef struct {
+    SysBusDevice parent_obj;
+
     MemoryRegion iomem;
     uint8_t mr[2];
     uint8_t sr;
@@ -30,6 +33,9 @@ typedef struct {
     CharBackend chr;
 } mcf_uart_state;
 
+#define TYPE_MCF_UART "mcf-uart"
+#define MCF_UART(obj) OBJECT_CHECK(mcf_uart_state, (obj), TYPE_MCF_UART)
+
 /* UART Status Register bits.  */
 #define MCF_UART_RxRDY  0x01
 #define MCF_UART_FFULL  0x02
@@ -220,8 +226,10 @@ void mcf_uart_write(void *opaque, hwaddr addr,
     mcf_uart_update(s);
 }
 
-static void mcf_uart_reset(mcf_uart_state *s)
+static void mcf_uart_reset(DeviceState *dev)
 {
+    mcf_uart_state *s = MCF_UART(dev);
+
     s->fifo_len = 0;
     s->mr[0] = 0;
     s->mr[1] = 0;
@@ -275,36 +283,80 @@ static void mcf_uart_receive(void *opaque, const uint8_t *buf, int size)
     mcf_uart_push_byte(s, buf[0]);
 }
 
-void *mcf_uart_init(qemu_irq irq, Chardev *chr)
-{
-    mcf_uart_state *s;
-
-    s = g_malloc0(sizeof(mcf_uart_state));
-    s->irq = irq;
-    if (chr) {
-        qemu_chr_fe_init(&s->chr, chr, &error_abort);
-        qemu_chr_fe_set_handlers(&s->chr, mcf_uart_can_receive,
-                                 mcf_uart_receive, mcf_uart_event,
-                                 s, NULL, true);
-    }
-    mcf_uart_reset(s);
-    return s;
-}
-
 static const MemoryRegionOps mcf_uart_ops = {
     .read = mcf_uart_read,
     .write = mcf_uart_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-void mcf_uart_mm_init(MemoryRegion *sysmem,
-                      hwaddr base,
-                      qemu_irq irq,
-                      Chardev *chr)
+static void mcf_uart_instance_init(Object *obj)
+{
+    SysBusDevice *dev = SYS_BUS_DEVICE(obj);
+    mcf_uart_state *s = MCF_UART(dev);
+
+    memory_region_init_io(&s->iomem, obj, &mcf_uart_ops, s, "uart", 0x40);
+    sysbus_init_mmio(dev, &s->iomem);
+
+    sysbus_init_irq(dev, &s->irq);
+}
+
+static void mcf_uart_realize(DeviceState *dev, Error **errp)
+{
+    mcf_uart_state *s = MCF_UART(dev);
+
+    qemu_chr_fe_set_handlers(&s->chr, mcf_uart_can_receive, mcf_uart_receive,
+                             mcf_uart_event, s, NULL, true);
+}
+
+static Property mcf_uart_properties[] = {
+    DEFINE_PROP_CHR("chardev", mcf_uart_state, chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void mcf_uart_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = mcf_uart_realize;
+    dc->reset = mcf_uart_reset;
+    dc->props = mcf_uart_properties;
+    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+}
+
+static const TypeInfo mcf_uart_info = {
+    .name          = TYPE_MCF_UART,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(mcf_uart_state),
+    .instance_init = mcf_uart_instance_init,
+    .class_init    = mcf_uart_class_init,
+};
+
+static void mcf_uart_register(void)
+{
+    type_register_static(&mcf_uart_info);
+}
+
+type_init(mcf_uart_register)
+
+void *mcf_uart_init(qemu_irq irq, Chardev *chrdrv)
+{
+    DeviceState  *dev;
+
+    dev = qdev_create(NULL, TYPE_MCF_UART);
+    if (chrdrv) {
+        qdev_prop_set_chr(dev, "chardev", chrdrv);
+    }
+    qdev_init_nofail(dev);
+
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
+
+    return dev;
+}
+
+void mcf_uart_mm_init(hwaddr base, qemu_irq irq, Chardev *chrdrv)
 {
-    mcf_uart_state *s;
+    DeviceState  *dev;
 
-    s = mcf_uart_init(irq, chr);
-    memory_region_init_io(&s->iomem, NULL, &mcf_uart_ops, s, "uart", 0x40);
-    memory_region_add_subregion(sysmem, base, &s->iomem);
+    dev = mcf_uart_init(irq, chrdrv);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
 }
diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index bad1d33..6563518 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -255,9 +255,9 @@ static void mcf5208evb_init(MachineState *machine)
     /* Internal peripherals.  */
     pic = mcf_intc_init(address_space_mem, 0xfc048000, cpu);
 
-    mcf_uart_mm_init(address_space_mem, 0xfc060000, pic[26], serial_hds[0]);
-    mcf_uart_mm_init(address_space_mem, 0xfc064000, pic[27], serial_hds[1]);
-    mcf_uart_mm_init(address_space_mem, 0xfc068000, pic[28], serial_hds[2]);
+    mcf_uart_mm_init(0xfc060000, pic[26], serial_hds[0]);
+    mcf_uart_mm_init(0xfc064000, pic[27], serial_hds[1]);
+    mcf_uart_mm_init(0xfc068000, pic[28], serial_hds[2]);
 
     mcf5208_sys_init(address_space_mem, pic);
 
diff --git a/include/hw/m68k/mcf.h b/include/hw/m68k/mcf.h
index 9a0bcfa..0db49c5 100644
--- a/include/hw/m68k/mcf.h
+++ b/include/hw/m68k/mcf.h
@@ -4,17 +4,13 @@
 
 #include "target/m68k/cpu-qom.h"
 
-struct MemoryRegion;
-
 /* mcf_uart.c */
 uint64_t mcf_uart_read(void *opaque, hwaddr addr,
                        unsigned size);
 void mcf_uart_write(void *opaque, hwaddr addr,
                     uint64_t val, unsigned size);
 void *mcf_uart_init(qemu_irq irq, Chardev *chr);
-void mcf_uart_mm_init(struct MemoryRegion *sysmem,
-                      hwaddr base,
-                      qemu_irq irq, Chardev *chr);
+void mcf_uart_mm_init(hwaddr base, qemu_irq irq, Chardev *chr);
 
 /* mcf_intc.c */
 qemu_irq *mcf_intc_init(struct MemoryRegion *sysmem,
-- 
1.8.3.1

[Qemu-devel] [PULL 11/23] cpu-exec: fix icount out-of-bounds access

[Paolo Bonzini]

When icount is active, tb_add_jump is surprisingly called with an
out of bounds basic block index.  I have no idea how that can work,
but it does not seem like a good idea.  Clear *last_tb for all
TB_EXIT_ICOUNT_EXPIRED cases, even when all you have to do is
refill icount_extra.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-exec.c              | 7 ++++---
 include/exec/exec-all.h | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 57583f1..1f7d217 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -542,7 +542,7 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
 
     trace_exec_tb(tb, tb->pc);
     ret = cpu_tb_exec(cpu, tb);
-    *last_tb = (TranslationBlock *)(ret & ~TB_EXIT_MASK);
+    tb = (TranslationBlock *)(ret & ~TB_EXIT_MASK);
     *tb_exit = ret & TB_EXIT_MASK;
     switch (*tb_exit) {
     case TB_EXIT_REQUESTED:
@@ -566,6 +566,7 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
         abort();
 #else
         int insns_left = cpu->icount_decr.u32;
+        *last_tb = NULL;
         if (cpu->icount_extra && insns_left >= 0) {
             /* Refill decrementer and continue execution.  */
             cpu->icount_extra += insns_left;
@@ -575,17 +576,17 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
         } else {
             if (insns_left > 0) {
                 /* Execute remaining instructions.  */
-                cpu_exec_nocache(cpu, insns_left, *last_tb, false);
+                cpu_exec_nocache(cpu, insns_left, tb, false);
                 align_clocks(sc, cpu);
             }
             cpu->exception_index = EXCP_INTERRUPT;
-            *last_tb = NULL;
             cpu_loop_exit(cpu);
         }
         break;
 #endif
     }
     default:
+        *last_tb = tb;
         break;
     }
 }
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index bbc9478..21ab7bf 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -318,6 +318,7 @@ static inline void tb_set_jmp_target(TranslationBlock *tb,
 static inline void tb_add_jump(TranslationBlock *tb, int n,
                                TranslationBlock *tb_next)
 {
+    assert(n < ARRAY_SIZE(tb->jmp_list_next));
     if (tb->jmp_list_next[n]) {
         /* Another thread has already done this while we were
          * outside of the lock; nothing to do in this case */
-- 
1.8.3.1

[Qemu-devel] [PULL 12/23] cpu-exec: tighten barrier on TCG_EXIT_REQUESTED

[Paolo Bonzini]

This seems to have worked just fine so far on weakly-ordered
architectures, but I don't see anything that prevents the
reordering from:

    store 1 to exit_request
    store 1 to tcg_exit_req
                                 load tcg_exit_req
                                 store 0 to tcg_exit_req
                                 load exit_request
                                 store 0 to exit_request
    store 1 to exit_request
    store 1 to tcg_exit_req

to this:

    store 1 to exit_request
    store 1 to tcg_exit_req
                                 load tcg_exit_req
                                 load exit_request
    store 1 to exit_request
    store 1 to tcg_exit_req
                                 store 0 to tcg_exit_req
                                 store 0 to exit_request

therefore losing a request.  It's possible that other memory barriers
(e.g. in rcu_read_unlock) are hiding it, but better safe than
sorry.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-exec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 1f7d217..d50625b 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -552,11 +552,11 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
          * have set something else (eg exit_request or
          * interrupt_request) which we will handle
          * next time around the loop.  But we need to
-         * ensure the tcg_exit_req read in generated code
+         * ensure the zeroing of tcg_exit_req (see cpu_tb_exec)
          * comes before the next read of cpu->exit_request
          * or cpu->interrupt_request.
          */
-        smp_rmb();
+        smp_mb();
         *last_tb = NULL;
         break;
     case TB_EXIT_ICOUNT_EXPIRED:
-- 
1.8.3.1

[Qemu-devel] [PULL 13/23] cpu-exec: avoid cpu_loop_exit in cpu_handle_interrupt

[Paolo Bonzini]

The siglongjmp goes straight back to the beginning of cpu_exec's
outermost loop.  We do not need a siglongjmp, we can simply
leave the inner TB execution loop.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-exec.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index d50625b..ed2fbc6 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -461,7 +461,7 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
     return false;
 }
 
-static inline void cpu_handle_interrupt(CPUState *cpu,
+static inline bool cpu_handle_interrupt(CPUState *cpu,
                                         TranslationBlock **last_tb)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
@@ -475,7 +475,7 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
         if (interrupt_request & CPU_INTERRUPT_DEBUG) {
             cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
             cpu->exception_index = EXCP_DEBUG;
-            cpu_loop_exit(cpu);
+            return true;
         }
         if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {
             /* Do nothing */
@@ -484,7 +484,7 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
             cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;
             cpu->halted = 1;
             cpu->exception_index = EXCP_HLT;
-            cpu_loop_exit(cpu);
+            return true;
         }
 #if defined(TARGET_I386)
         else if (interrupt_request & CPU_INTERRUPT_INIT) {
@@ -494,13 +494,13 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
             cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0);
             do_cpu_init(x86_cpu);
             cpu->exception_index = EXCP_HALTED;
-            cpu_loop_exit(cpu);
+            return true;
         }
 #else
         else if (interrupt_request & CPU_INTERRUPT_RESET) {
             replay_interrupt();
             cpu_reset(cpu);
-            cpu_loop_exit(cpu);
+            return true;
         }
 #endif
         /* The target hook has 3 exit conditions:
@@ -526,8 +526,10 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
     if (unlikely(atomic_read(&cpu->exit_request) || replay_has_interrupt())) {
         atomic_set(&cpu->exit_request, 0);
         cpu->exception_index = EXCP_INTERRUPT;
-        cpu_loop_exit(cpu);
+        return true;
     }
+
+    return false;
 }
 
 static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
@@ -625,7 +627,7 @@ int cpu_exec(CPUState *cpu)
     for(;;) {
         /* prepare setjmp context for exception handling */
         if (sigsetjmp(cpu->jmp_env, 0) == 0) {
-            TranslationBlock *tb, *last_tb = NULL;
+            TranslationBlock *last_tb = NULL;
             int tb_exit = 0;
 
             /* if an exception is pending, we execute it here */
@@ -633,14 +635,13 @@ int cpu_exec(CPUState *cpu)
                 break;
             }
 
-            for(;;) {
-                cpu_handle_interrupt(cpu, &last_tb);
-                tb = tb_find(cpu, last_tb, tb_exit);
+            while (!cpu_handle_interrupt(cpu, &last_tb)) {
+                TranslationBlock *tb = tb_find(cpu, last_tb, tb_exit);
                 cpu_loop_exec_tb(cpu, tb, &last_tb, &tb_exit, &sc);
                 /* Try to align the host and virtual clocks
                    if the guest is in advance */
                 align_clocks(&sc, cpu);
-            } /* for(;;) */
+            }
         } else {
 #if defined(__clang__) || !QEMU_GNUC_PREREQ(4, 6)
             /* Some compilers wrongly smash all local variables after
-- 
1.8.3.1

[Qemu-devel] [PULL 14/23] cpu-exec: avoid repeated sigsetjmp on interrupts

[Paolo Bonzini]

The sigsetjmp only needs to be prepared once for the whole execution
of cpu_exec.  This patch takes care of the "== 0" side, using a
nested loop so that cpu_handle_interrupt goes straight back to
cpu_handle_exception without doing another sigsetjmp.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-exec.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index ed2fbc6..865015c 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -627,21 +627,21 @@ int cpu_exec(CPUState *cpu)
     for(;;) {
         /* prepare setjmp context for exception handling */
         if (sigsetjmp(cpu->jmp_env, 0) == 0) {
-            TranslationBlock *last_tb = NULL;
-            int tb_exit = 0;
-
             /* if an exception is pending, we execute it here */
-            if (cpu_handle_exception(cpu, &ret)) {
-                break;
+            while (!cpu_handle_exception(cpu, &ret)) {
+                TranslationBlock *last_tb = NULL;
+                int tb_exit = 0;
+
+                while (!cpu_handle_interrupt(cpu, &last_tb)) {
+                    TranslationBlock *tb = tb_find(cpu, last_tb, tb_exit);
+                    cpu_loop_exec_tb(cpu, tb, &last_tb, &tb_exit, &sc);
+                    /* Try to align the host and virtual clocks
+                       if the guest is in advance */
+                    align_clocks(&sc, cpu);
+                }
             }
+            break;
 
-            while (!cpu_handle_interrupt(cpu, &last_tb)) {
-                TranslationBlock *tb = tb_find(cpu, last_tb, tb_exit);
-                cpu_loop_exec_tb(cpu, tb, &last_tb, &tb_exit, &sc);
-                /* Try to align the host and virtual clocks
-                   if the guest is in advance */
-                align_clocks(&sc, cpu);
-            }
         } else {
 #if defined(__clang__) || !QEMU_GNUC_PREREQ(4, 6)
             /* Some compilers wrongly smash all local variables after
-- 
1.8.3.1

[Qemu-devel] [PULL 15/23] cpu-exec: remove outermost infinite loop

[Paolo Bonzini]

Reorganize the sigsetjmp so that the restart case falls through
to cpu_handle_exception and the execution loop.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-exec.c | 58 +++++++++++++++++++++++++++-------------------------------
 1 file changed, 27 insertions(+), 31 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 865015c..b8ebb5c 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -624,41 +624,37 @@ int cpu_exec(CPUState *cpu)
      */
     init_delay_params(&sc, cpu);
 
-    for(;;) {
-        /* prepare setjmp context for exception handling */
-        if (sigsetjmp(cpu->jmp_env, 0) == 0) {
-            /* if an exception is pending, we execute it here */
-            while (!cpu_handle_exception(cpu, &ret)) {
-                TranslationBlock *last_tb = NULL;
-                int tb_exit = 0;
-
-                while (!cpu_handle_interrupt(cpu, &last_tb)) {
-                    TranslationBlock *tb = tb_find(cpu, last_tb, tb_exit);
-                    cpu_loop_exec_tb(cpu, tb, &last_tb, &tb_exit, &sc);
-                    /* Try to align the host and virtual clocks
-                       if the guest is in advance */
-                    align_clocks(&sc, cpu);
-                }
-            }
-            break;
-
-        } else {
+    /* prepare setjmp context for exception handling */
+    if (sigsetjmp(cpu->jmp_env, 0) != 0) {
 #if defined(__clang__) || !QEMU_GNUC_PREREQ(4, 6)
-            /* Some compilers wrongly smash all local variables after
-             * siglongjmp. There were bug reports for gcc 4.5.0 and clang.
-             * Reload essential local variables here for those compilers.
-             * Newer versions of gcc would complain about this code (-Wclobbered). */
-            cpu = current_cpu;
-            cc = CPU_GET_CLASS(cpu);
+        /* Some compilers wrongly smash all local variables after
+         * siglongjmp. There were bug reports for gcc 4.5.0 and clang.
+         * Reload essential local variables here for those compilers.
+         * Newer versions of gcc would complain about this code (-Wclobbered). */
+        cpu = current_cpu;
+        cc = CPU_GET_CLASS(cpu);
 #else /* buggy compiler */
-            /* Assert that the compiler does not smash local variables. */
-            g_assert(cpu == current_cpu);
-            g_assert(cc == CPU_GET_CLASS(cpu));
+        /* Assert that the compiler does not smash local variables. */
+        g_assert(cpu == current_cpu);
+        g_assert(cc == CPU_GET_CLASS(cpu));
 #endif /* buggy compiler */
-            cpu->can_do_io = 1;
-            tb_lock_reset();
+        cpu->can_do_io = 1;
+        tb_lock_reset();
+    }
+
+    /* if an exception is pending, we execute it here */
+    while (!cpu_handle_exception(cpu, &ret)) {
+        TranslationBlock *last_tb = NULL;
+        int tb_exit = 0;
+
+        while (!cpu_handle_interrupt(cpu, &last_tb)) {
+            TranslationBlock *tb = tb_find(cpu, last_tb, tb_exit);
+            cpu_loop_exec_tb(cpu, tb, &last_tb, &tb_exit, &sc);
+            /* Try to align the host and virtual clocks
+               if the guest is in advance */
+            align_clocks(&sc, cpu);
         }
-    } /* for(;;) */
+    }
 
     cc->cpu_exec_exit(cpu);
     rcu_read_unlock();
-- 
1.8.3.1

[Qemu-devel] [PULL 16/23] qemu-doc: Clarify that -vga std is now the default

[Paolo Bonzini]

From: Alberto Garcia <berto@igalia.com>

The QEMU manual page states that Cirrus Logic is the default video
card if the user doesn't specify any. However this is not true since
QEMU 2.2.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20170127094154.19778-1-berto@igalia.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-options.hx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index ac036b4..5633d39 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1194,12 +1194,12 @@ Select type of VGA card to emulate. Valid values for @var{type} are
 Cirrus Logic GD5446 Video card. All Windows versions starting from
 Windows 95 should recognize and use this graphic card. For optimal
 performances, use 16 bit color depth in the guest and the host OS.
-(This one is the default)
+(This card was the default before QEMU 2.2)
 @item std
 Standard VGA card with Bochs VBE extensions.  If your guest OS
 supports the VESA 2.0 VBE extensions (e.g. Windows XP) and if you want
 to use high resolution modes (>= 1280x1024x16) then you should use
-this option.
+this option. (This card is the default since QEMU 2.2)
 @item vmware
 VMWare SVGA-II compatible adapter. Use it if you have sufficiently
 recent XFree86/XOrg server or Windows guest with a driver for this
-- 
1.8.3.1

[Qemu-devel] [PULL 17/23] qemu-nbd: Implement socket activation.

[Paolo Bonzini]

From: "Richard W.M. Jones" <rjones@redhat.com>

Socket activation (sometimes known as systemd socket activation)
allows an Internet superserver to pass a pre-opened listening socket
to the process, instead of having qemu-nbd open a socket itself.  This
is done via the LISTEN_FDS and LISTEN_PID environment variables, and a
standard file descriptor range.

This change partially implements socket activation for qemu-nbd.  If
the environment variables are set correctly, then socket activation
will happen automatically, otherwise everything works as before.  The
limitation is that LISTEN_FDS must be 1.

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Message-Id: <20170204100317.32425-2-rjones@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-nbd.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 163 insertions(+), 9 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index c734f62..e4fede6 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -463,6 +463,135 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp)
     return creds;
 }
 
+static void setup_address_and_port(const char **address, const char **port)
+{
+    if (*address == NULL) {
+        *address = "0.0.0.0";
+    }
+
+    if (*port == NULL) {
+        *port = stringify(NBD_DEFAULT_PORT);
+    }
+}
+
+#define FIRST_SOCKET_ACTIVATION_FD 3 /* defined by systemd ABI */
+
+#ifndef _WIN32
+/*
+ * Check if socket activation was requested via use of the
+ * LISTEN_FDS and LISTEN_PID environment variables.
+ *
+ * Returns 0 if no socket activation, or the number of FDs.
+ */
+static unsigned int check_socket_activation(void)
+{
+    const char *s;
+    unsigned long pid;
+    unsigned long nr_fds;
+    unsigned int i;
+    int fd;
+    int err;
+
+    s = getenv("LISTEN_PID");
+    if (s == NULL) {
+        return 0;
+    }
+    err = qemu_strtoul(s, NULL, 10, &pid);
+    if (err) {
+        if (verbose) {
+            fprintf(stderr, "malformed %s environment variable (ignored)\n",
+                    "LISTEN_PID");
+        }
+        return 0;
+    }
+    if (pid != getpid()) {
+        if (verbose) {
+            fprintf(stderr, "%s was not for us (ignored)\n",
+                    "LISTEN_PID");
+        }
+        return 0;
+    }
+
+    s = getenv("LISTEN_FDS");
+    if (s == NULL) {
+        return 0;
+    }
+    err = qemu_strtoul(s, NULL, 10, &nr_fds);
+    if (err) {
+        if (verbose) {
+            fprintf(stderr, "malformed %s environment variable (ignored)\n",
+                    "LISTEN_FDS");
+        }
+        return 0;
+    }
+    assert(nr_fds <= UINT_MAX);
+
+    /* A limitation of current qemu-nbd is that it can only listen on
+     * a single socket.  When that limitation is lifted, we can change
+     * this function to allow LISTEN_FDS > 1, and remove the assertion
+     * in the main function below.
+     */
+    if (nr_fds > 1) {
+        error_report("qemu-nbd does not support socket activation with %s > 1",
+                     "LISTEN_FDS");
+        exit(EXIT_FAILURE);
+    }
+
+    /* So these are not passed to any child processes we might start. */
+    unsetenv("LISTEN_FDS");
+    unsetenv("LISTEN_PID");
+
+    /* So the file descriptors don't leak into child processes. */
+    for (i = 0; i < nr_fds; ++i) {
+        fd = FIRST_SOCKET_ACTIVATION_FD + i;
+        if (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1) {
+            /* If we cannot set FD_CLOEXEC then it probably means the file
+             * descriptor is invalid, so socket activation has gone wrong
+             * and we should exit.
+             */
+            error_report("Socket activation failed: "
+                         "invalid file descriptor fd = %d: %m",
+                         fd);
+            exit(EXIT_FAILURE);
+        }
+    }
+
+    return (unsigned int) nr_fds;
+}
+
+#else /* !_WIN32 */
+static unsigned int check_socket_activation(void)
+{
+    return 0;
+}
+#endif
+
+/*
+ * Check socket parameters compatibility when socket activation is used.
+ */
+static const char *socket_activation_validate_opts(const char *device,
+                                                   const char *sockpath,
+                                                   const char *address,
+                                                   const char *port)
+{
+    if (device != NULL) {
+        return "NBD device can't be set when using socket activation";
+    }
+
+    if (sockpath != NULL) {
+        return "Unix socket can't be set when using socket activation";
+    }
+
+    if (address != NULL) {
+        return "The interface can't be set when using socket activation";
+    }
+
+    if (port != NULL) {
+        return "TCP port number can't be set when using socket activation";
+    }
+
+    return NULL;
+}
 
 int main(int argc, char **argv)
 {
@@ -471,7 +600,7 @@ int main(int argc, char **argv)
     off_t dev_offset = 0;
     uint16_t nbdflags = 0;
     bool disconnect = false;
-    const char *bindto = "0.0.0.0";
+    const char *bindto = NULL;
     const char *port = NULL;
     char *sockpath = NULL;
     char *device = NULL;
@@ -533,6 +662,7 @@ int main(int argc, char **argv)
     char *trace_file = NULL;
     bool fork_process = false;
     int old_stderr = -1;
+    unsigned socket_activation;
 
     /* The client thread uses SIGTERM to interrupt the server.  A signal
      * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -751,6 +881,19 @@ int main(int argc, char **argv)
     trace_init_file(trace_file);
     qemu_set_log(LOG_TRACE);
 
+    socket_activation = check_socket_activation();
+    if (socket_activation == 0) {
+        setup_address_and_port(&bindto, &port);
+    } else {
+        /* Using socket activation - check user didn't use -p etc. */
+        const char *err_msg = socket_activation_validate_opts(device, sockpath,
+                                                              bindto, port);
+        if (err_msg != NULL) {
+            error_report("%s", err_msg);
+            exit(EXIT_FAILURE);
+        }
+    }
+
     if (tlscredsid) {
         if (sockpath) {
             error_report("TLS is only supported with IPv4/IPv6");
@@ -855,7 +998,25 @@ int main(int argc, char **argv)
         snprintf(sockpath, 128, SOCKET_PATH, basename(device));
     }
 
-    saddr = nbd_build_socket_address(sockpath, bindto, port);
+    if (socket_activation == 0) {
+        server_ioc = qio_channel_socket_new();
+        saddr = nbd_build_socket_address(sockpath, bindto, port);
+        if (qio_channel_socket_listen_sync(server_ioc, saddr, &local_err) < 0) {
+            object_unref(OBJECT(server_ioc));
+            error_report_err(local_err);
+            return 1;
+        }
+    } else {
+        /* See comment in check_socket_activation above. */
+        assert(socket_activation == 1);
+        server_ioc = qio_channel_socket_new_fd(FIRST_SOCKET_ACTIVATION_FD,
+                                               &local_err);
+        if (server_ioc == NULL) {
+            error_report("Failed to use socket activation: %s",
+                         error_get_pretty(local_err));
+            exit(EXIT_FAILURE);
+        }
+    }
 
     if (qemu_init_main_loop(&local_err)) {
         error_report_err(local_err);
@@ -950,13 +1111,6 @@ int main(int argc, char **argv)
         exit(EXIT_FAILURE);
     }
 
-    server_ioc = qio_channel_socket_new();
-    if (qio_channel_socket_listen_sync(server_ioc, saddr, &local_err) < 0) {
-        object_unref(OBJECT(server_ioc));
-        error_report_err(local_err);
-        return 1;
-    }
-
     if (device) {
         int ret;
 
-- 
1.8.3.1

[Qemu-devel] [PULL 18/23] vl: Move the cpu_synchronize_all_post_init() after generic devices initialization

[Paolo Bonzini]

From: Dou Liyang <douly.fnst@cn.fujitsu.com>

At the Qemu initialization, we call the cpu_synchronize_all_post_init()
to synchronize All CPU states to KVM in the ./vl.c::main().

Currently, it is called before we initialize the CPUs, which is created
by "-device" command and parsed by generic devices initialization, So,
these CPUs may be ignored to synchronize.

The patch moves the cpu_synchronize_all_post_init func after generic
devices initialization to make sure that all the CPUs can be included.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Message-Id: <1485916178-17838-1-git-send-email-douly.fnst@cn.fujitsu.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 0d866ad..a05671d 100644
--- a/vl.c
+++ b/vl.c
@@ -4462,8 +4462,6 @@ int main(int argc, char **argv, char **envp)
 
     audio_init();
 
-    cpu_synchronize_all_post_init();
-
     if (hax_enabled()) {
         hax_sync_vcpus();
     }
@@ -4489,6 +4487,8 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    cpu_synchronize_all_post_init();
+
     numa_post_machine_init();
 
     rom_reset_order_override();
-- 
1.8.3.1

[Qemu-devel] [PULL 19/23] Makefile: avoid leaving the temporary QEMU_PKGVERSION header file

[Paolo Bonzini]

From: Lin Ma <lma@suse.com>

By commit 67a1de0d, When we perform 'git pull && make && sudo make install',
In 'make' stage a qemu-version.h.tmp will be generated. If the content of
qemu-version.h.tmp and qemu-version.h aren't consistent, The qemu-version.h.tmp
will be renamed to qemu-version.h. Because of the target FORCE, The same action
will be do again in 'make install' stage.

In 'make install' stage, If there is no qemu-version.h.tmp exists and we run
'make install' with sudo, The owner and group of new qemu-version.h.tmp will be
privileged user/group. When we run 'make' next time, qemu-version.h.tmp can't
be overwritten because of permission issue.

This patch removed qemu-version.h.tmp after build to fix this issue.

Signed-off-by: Lin Ma <lma@suse.com>
Message-Id: <20170215024030.23895-1-lma@suse.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index b993741..830fa5a 100644
--- a/Makefile
+++ b/Makefile
@@ -299,7 +299,11 @@ qemu-version.h: FORCE
 				printf '""\n'; \
 			fi; \
 		fi) > $@.tmp)
-	$(call quiet-command, cmp -s $@ $@.tmp || mv $@.tmp $@)
+	$(call quiet-command, if ! cmp -s $@ $@.tmp; then \
+	  mv $@.tmp $@; \
+	 else \
+	  rm $@.tmp; \
+	 fi)
 
 config-host.h: config-host.h-timestamp
 config-host.h-timestamp: config-host.mak
-- 
1.8.3.1

[Qemu-devel] [PULL 20/23] i386/cpu: add crash-information QOM property

[Paolo Bonzini]

From: Anton Nefedov <anton.nefedov@virtuozzo.com>

Windows reports BSOD parameters through Hyper-V crash MSRs. This
information is very useful for initial crash analysis and thus
it would be nice to have a way to fetch it.

Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Message-Id: <1487053524-18674-2-git-send-email-den@openvz.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 kvm-all.c         |  1 +
 qapi-schema.json  | 24 ++++++++++++++++++++++++
 target/i386/cpu.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+)

diff --git a/kvm-all.c b/kvm-all.c
index a27c880..64f46c8 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2000,6 +2000,7 @@ int kvm_cpu_exec(CPUState *cpu)
                 ret = EXCP_INTERRUPT;
                 break;
             case KVM_SYSTEM_EVENT_CRASH:
+                kvm_cpu_synchronize_state(cpu);
                 qemu_mutex_lock_iothread();
                 qemu_system_guest_panicked();
                 qemu_mutex_unlock_iothread();
diff --git a/qapi-schema.json b/qapi-schema.json
index 5edb08d..baa0d26 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -5871,6 +5871,30 @@
   'data': [ 'pause', 'poweroff' ] }
 
 ##
+# @GuestPanicInformation:
+#
+# Information about a guest panic
+#
+# Since: 2.9
+##
+{'union': 'GuestPanicInformation',
+ 'data': { 'hyper-v': 'GuestPanicInformationHyperV' } }
+
+##
+# @GuestPanicInformationHyperV:
+#
+# Hyper-V specific guest panic information (HV crash MSRs)
+#
+# Since: 2.9
+##
+{'struct': 'GuestPanicInformationHyperV',
+ 'data': { 'arg1': 'uint64',
+           'arg2': 'uint64',
+           'arg3': 'uint64',
+           'arg4': 'uint64',
+           'arg5': 'uint64' } }
+
+##
 # @rtc-reset-reinjection:
 #
 # This command will reset the RTC interrupt reinjection backlog.
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index eb49980..71aa91f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3495,6 +3495,53 @@ static void x86_cpu_register_feature_bit_props(X86CPU *cpu,
     x86_cpu_register_bit_prop(cpu, name, &cpu->env.features[w], bitnr);
 }
 
+static GuestPanicInformation *x86_cpu_get_crash_info(CPUState *cs)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+    GuestPanicInformation *panic_info = NULL;
+
+    if (env->features[FEAT_HYPERV_EDX] & HV_X64_GUEST_CRASH_MSR_AVAILABLE) {
+        GuestPanicInformationHyperV *panic_info_hv =
+            g_malloc0(sizeof(GuestPanicInformationHyperV));
+        panic_info = g_malloc0(sizeof(GuestPanicInformation));
+
+        panic_info->type = GUEST_PANIC_INFORMATION_KIND_HYPER_V;
+        panic_info->u.hyper_v.data = panic_info_hv;
+
+        assert(HV_X64_MSR_CRASH_PARAMS >= 5);
+        panic_info_hv->arg1 = env->msr_hv_crash_params[0];
+        panic_info_hv->arg2 = env->msr_hv_crash_params[1];
+        panic_info_hv->arg3 = env->msr_hv_crash_params[2];
+        panic_info_hv->arg4 = env->msr_hv_crash_params[3];
+        panic_info_hv->arg5 = env->msr_hv_crash_params[4];
+    }
+
+    return panic_info;
+}
+static void x86_cpu_get_crash_info_qom(Object *obj, Visitor *v,
+                                       const char *name, void *opaque,
+                                       Error **errp)
+{
+    CPUState *cs = CPU(obj);
+    GuestPanicInformation *panic_info;
+
+    if (!cs->crash_occurred) {
+        error_setg(errp, "No crash occured");
+        return;
+    }
+
+    panic_info = x86_cpu_get_crash_info(cs);
+    if (panic_info == NULL) {
+        error_setg(errp, "No crash information");
+        return;
+    }
+
+    visit_type_GuestPanicInformation(v, "crash-information", &panic_info,
+                                     errp);
+    qapi_free_GuestPanicInformation(panic_info);
+}
+
 static void x86_cpu_initfn(Object *obj)
 {
     CPUState *cs = CPU(obj);
@@ -3530,6 +3577,9 @@ static void x86_cpu_initfn(Object *obj)
                         x86_cpu_get_feature_words,
                         NULL, NULL, (void *)cpu->filtered_features, NULL);
 
+    object_property_add(obj, "crash-information", "GuestPanicInformation",
+                        x86_cpu_get_crash_info_qom, NULL, NULL, NULL, NULL);
+
     cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
 
     for (w = 0; w < FEATURE_WORDS; w++) {
-- 
1.8.3.1

[Qemu-devel] [PULL 21/23] report guest crash information in GUEST_PANICKED event

[Paolo Bonzini]

From: Anton Nefedov <anton.nefedov@virtuozzo.com>

it's not very convenient to use the crash-information property interface,
so provide a CPU class callback to get the guest crash information, and pass
that information in the event

Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Message-Id: <1487053524-18674-3-git-send-email-den@openvz.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/misc/pvpanic.c       |  2 +-
 hw/ppc/spapr_rtas.c     |  3 ++-
 include/qom/cpu.h       | 10 ++++++++++
 include/sysemu/sysemu.h |  2 +-
 kvm-all.c               |  2 +-
 qapi/event.json         |  6 ++++--
 qom/cpu.c               | 11 +++++++++++
 target/i386/cpu.c       |  1 +
 target/s390x/kvm.c      |  4 ++--
 vl.c                    | 11 ++++++++---
 10 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index 0ac1e6a..57da7f2 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -42,7 +42,7 @@ static void handle_event(int event)
     }
 
     if (event & PVPANIC_PANICKED) {
-        qemu_system_guest_panicked();
+        qemu_system_guest_panicked(NULL);
         return;
     }
 }
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index bb19944..619f32c 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -334,7 +334,8 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
 {
     target_ulong ret = 0;
 
-    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, &error_abort);
+    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, false, NULL,
+                                   &error_abort);
 
     rtas_st(rets, 0, ret);
 }
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 45bcf21..f69b240 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -158,6 +158,7 @@ typedef struct CPUClass {
                            uint8_t *buf, int len, bool is_write);
     void (*dump_state)(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
                        int flags);
+    GuestPanicInformation* (*get_crash_info)(CPUState *cpu);
     void (*dump_statistics)(CPUState *cpu, FILE *f,
                             fprintf_function cpu_fprintf, int flags);
     int64_t (*get_arch_id)(CPUState *cpu);
@@ -472,6 +473,15 @@ int cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
                              void *opaque);
 
 /**
+ * cpu_get_crash_info:
+ * @cpu: The CPU to get crash information for
+ *
+ * Gets the previously saved crash information.
+ * Caller is responsible for freeing the data.
+ */
+GuestPanicInformation *cpu_get_crash_info(CPUState *cpu);
+
+/**
  * CPUDumpFlags:
  * @CPU_DUMP_CODE:
  * @CPU_DUMP_FPU: dump FPU register state, not just integer
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index eda5241..576c7ce 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -66,7 +66,7 @@ int qemu_shutdown_requested_get(void);
 int qemu_reset_requested_get(void);
 void qemu_system_killed(int signal, pid_t pid);
 void qemu_system_reset(bool report);
-void qemu_system_guest_panicked(void);
+void qemu_system_guest_panicked(GuestPanicInformation *info);
 size_t qemu_target_page_bits(void);
 
 void qemu_add_exit_notifier(Notifier *notify);
diff --git a/kvm-all.c b/kvm-all.c
index 64f46c8..0c94637 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2002,7 +2002,7 @@ int kvm_cpu_exec(CPUState *cpu)
             case KVM_SYSTEM_EVENT_CRASH:
                 kvm_cpu_synchronize_state(cpu);
                 qemu_mutex_lock_iothread();
-                qemu_system_guest_panicked();
+                qemu_system_guest_panicked(cpu_get_crash_info(cpu));
                 qemu_mutex_unlock_iothread();
                 ret = 0;
                 break;
diff --git a/qapi/event.json b/qapi/event.json
index 7bf539b..970ff02 100644
--- a/qapi/event.json
+++ b/qapi/event.json
@@ -488,7 +488,9 @@
 #
 # @action: action that has been taken, currently always "pause"
 #
-# Since: 1.5
+# @info: optional information about a panic
+#
+# Since: 1.5 (@info since 2.9)
 #
 # Example:
 #
@@ -497,7 +499,7 @@
 #
 ##
 { 'event': 'GUEST_PANICKED',
-  'data': { 'action': 'GuestPanicAction' } }
+  'data': { 'action': 'GuestPanicAction', '*info': 'GuestPanicInformation' } }
 
 ##
 # @QUORUM_FAILURE:
diff --git a/qom/cpu.c b/qom/cpu.c
index 0e19b1a..ed87c50 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -218,6 +218,17 @@ static bool cpu_common_exec_interrupt(CPUState *cpu, int int_req)
     return false;
 }
 
+GuestPanicInformation *cpu_get_crash_info(CPUState *cpu)
+{
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+    GuestPanicInformation *res = NULL;
+
+    if (cc->get_crash_info) {
+        res = cc->get_crash_info(cpu);
+    }
+    return res;
+}
+
 void cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
                     int flags)
 {
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 71aa91f..fd7add2 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3734,6 +3734,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     cc->do_interrupt = x86_cpu_do_interrupt;
     cc->cpu_exec_interrupt = x86_cpu_exec_interrupt;
     cc->dump_state = x86_cpu_dump_state;
+    cc->get_crash_info = x86_cpu_get_crash_info;
     cc->set_pc = x86_cpu_set_pc;
     cc->synchronize_from_tb = x86_cpu_synchronize_from_tb;
     cc->gdb_read_register = x86_cpu_gdb_read_register;
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 6ed3876..2536780 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1864,7 +1864,7 @@ static void unmanageable_intercept(S390CPU *cpu, const char *str, int pswoffset)
                  str, cs->cpu_index, ldq_phys(cs->as, cpu->env.psa + pswoffset),
                  ldq_phys(cs->as, cpu->env.psa + pswoffset + 8));
     s390_cpu_halt(cpu);
-    qemu_system_guest_panicked();
+    qemu_system_guest_panicked(NULL);
 }
 
 static int handle_intercept(S390CPU *cpu)
@@ -1897,7 +1897,7 @@ static int handle_intercept(S390CPU *cpu)
                 if (is_special_wait_psw(cs)) {
                     qemu_system_shutdown_request();
                 } else {
-                    qemu_system_guest_panicked();
+                    qemu_system_guest_panicked(NULL);
                 }
             }
             r = EXCP_HALTED;
diff --git a/vl.c b/vl.c
index a05671d..5993270 100644
--- a/vl.c
+++ b/vl.c
@@ -1679,18 +1679,23 @@ void qemu_system_reset(bool report)
     cpu_synchronize_all_post_reset();
 }
 
-void qemu_system_guest_panicked(void)
+void qemu_system_guest_panicked(GuestPanicInformation *info)
 {
     if (current_cpu) {
         current_cpu->crash_occurred = true;
     }
-    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, &error_abort);
+    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE,
+                                   !!info, info, &error_abort);
     vm_stop(RUN_STATE_GUEST_PANICKED);
     if (!no_shutdown) {
         qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_POWEROFF,
-                                       &error_abort);
+                                       !!info, info, &error_abort);
         qemu_system_shutdown_request();
     }
+
+    if (info) {
+        qapi_free_GuestPanicInformation(info);
+    }
 }
 
 void qemu_system_reset_request(void)
-- 
1.8.3.1

[Qemu-devel] [PULL 22/23] vl: log available guest crash information

[Paolo Bonzini]

From: Anton Nefedov <anton.nefedov@virtuozzo.com>

There is a suitable log mask for the purpose.

Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Message-Id: <1487053524-18674-4-git-send-email-den@openvz.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/vl.c b/vl.c
index 5993270..27d9829 100644
--- a/vl.c
+++ b/vl.c
@@ -1681,6 +1681,8 @@ void qemu_system_reset(bool report)
 
 void qemu_system_guest_panicked(GuestPanicInformation *info)
 {
+    qemu_log_mask(LOG_GUEST_ERROR, "Guest crashed\n");
+
     if (current_cpu) {
         current_cpu->crash_occurred = true;
     }
@@ -1694,6 +1696,15 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
     }
 
     if (info) {
+        if (info->type == GUEST_PANIC_INFORMATION_KIND_HYPER_V) {
+            qemu_log_mask(LOG_GUEST_ERROR, "HV crash parameters: (%#"PRIx64
+                          " %#"PRIx64" %#"PRIx64" %#"PRIx64" %#"PRIx64")\n",
+                          info->u.hyper_v.data->arg1,
+                          info->u.hyper_v.data->arg2,
+                          info->u.hyper_v.data->arg3,
+                          info->u.hyper_v.data->arg4,
+                          info->u.hyper_v.data->arg5);
+        }
         qapi_free_GuestPanicInformation(info);
     }
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 23/23] target-i386: correctly propagate retaddr into SVM helpers

[Paolo Bonzini]

Commit 2afbdf8 ("target-i386: exception handling for memory helpers",
2015-09-15) changed tlb_fill's cpu_restore_state+raise_exception_err
to raise_exception_err_ra.  After this change, the cpu_restore_state
and raise_exception_err's cpu_loop_exit are merged into
raise_exception_err_ra's cpu_loop_exit_restore.

This actually fixed some bugs, but when SVM is enabled there is a
second path from raise_exception_err_ra to cpu_loop_exit.  This is
the VMEXIT path, and now cpu_vmexit is called without a
cpu_restore_state before.

The fix is to pass the retaddr to cpu_vmexit (via
cpu_svm_check_intercept_param).  All helpers can now use GETPC() to pass
the correct retaddr, too.

Cc: qemu-stable@nongnu.org
Fixes: 2afbdf84807d673eb682cb78158e11cdacbf4673
Reported-by: Alexander Boettcher <alexander.boettcher@genode-labs.com>
Tested-by: Alexander Boettcher <alexander.boettcher@genode-labs.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-exec.c                |  2 +-
 target/i386/cpu.h         |  5 ++--
 target/i386/excp_helper.c | 11 ++++----
 target/i386/helper.h      |  1 -
 target/i386/misc_helper.c | 24 ++++++++---------
 target/i386/seg_helper.c  |  6 ++---
 target/i386/svm_helper.c  | 65 ++++++++++++++++++++++-------------------------
 7 files changed, 56 insertions(+), 58 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index b8ebb5c..142a586 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -491,7 +491,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
             X86CPU *x86_cpu = X86_CPU(cpu);
             CPUArchState *env = &x86_cpu->env;
             replay_interrupt();
-            cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0);
+            cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0, 0);
             do_cpu_init(x86_cpu);
             cpu->exception_index = EXCP_HALTED;
             return true;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 4d788d5..cfb1ce0 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1621,8 +1621,9 @@ void helper_lock_init(void);
 
 /* svm_helper.c */
 void cpu_svm_check_intercept_param(CPUX86State *env1, uint32_t type,
-                                   uint64_t param);
-void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1);
+                                   uint64_t param, uint64_t retaddr);
+void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1,
+                uintptr_t retaddr);
 
 /* seg_helper.c */
 void do_interrupt_x86_hardirq(CPUX86State *env, int intno, int is_hw);
diff --git a/target/i386/excp_helper.c b/target/i386/excp_helper.c
index f0dc499..923f1af 100644
--- a/target/i386/excp_helper.c
+++ b/target/i386/excp_helper.c
@@ -39,7 +39,8 @@ void helper_raise_exception(CPUX86State *env, int exception_index)
  * needed. It should only be called, if this is not an interrupt.
  * Returns the new exception number.
  */
-static int check_exception(CPUX86State *env, int intno, int *error_code)
+static int check_exception(CPUX86State *env, int intno, int *error_code,
+                           uint64_t retaddr)
 {
     int first_contributory = env->old_exception == 0 ||
                               (env->old_exception >= 10 &&
@@ -53,7 +54,7 @@ static int check_exception(CPUX86State *env, int intno, int *error_code)
 #if !defined(CONFIG_USER_ONLY)
     if (env->old_exception == EXCP08_DBLE) {
         if (env->hflags & HF_SVMI_MASK) {
-            cpu_vmexit(env, SVM_EXIT_SHUTDOWN, 0); /* does not return */
+            cpu_vmexit(env, SVM_EXIT_SHUTDOWN, 0, retaddr); /* does not return */
         }
 
         qemu_log_mask(CPU_LOG_RESET, "Triple fault\n");
@@ -93,10 +94,10 @@ static void QEMU_NORETURN raise_interrupt2(CPUX86State *env, int intno,
 
     if (!is_int) {
         cpu_svm_check_intercept_param(env, SVM_EXIT_EXCP_BASE + intno,
-                                      error_code);
-        intno = check_exception(env, intno, &error_code);
+                                      error_code, retaddr);
+        intno = check_exception(env, intno, &error_code, retaddr);
     } else {
-        cpu_svm_check_intercept_param(env, SVM_EXIT_SWINT, 0);
+        cpu_svm_check_intercept_param(env, SVM_EXIT_SWINT, 0, retaddr);
     }
 
     cs->exception_index = intno;
diff --git a/target/i386/helper.h b/target/i386/helper.h
index 4c1aaff..6fb8fb9 100644
--- a/target/i386/helper.h
+++ b/target/i386/helper.h
@@ -99,7 +99,6 @@ DEF_HELPER_2(inl, tl, env, i32)
 DEF_HELPER_FLAGS_4(bpt_io, TCG_CALL_NO_WG, void, env, i32, i32, tl)
 
 DEF_HELPER_3(svm_check_intercept_param, void, env, i32, i64)
-DEF_HELPER_3(vmexit, void, env, i32, i64)
 DEF_HELPER_4(svm_check_io, void, env, i32, i32, i32)
 DEF_HELPER_3(vmrun, void, env, int, int)
 DEF_HELPER_1(vmmcall, void, env)
diff --git a/target/i386/misc_helper.c b/target/i386/misc_helper.c
index 5029efe..ca2ea09 100644
--- a/target/i386/misc_helper.c
+++ b/target/i386/misc_helper.c
@@ -101,7 +101,7 @@ void helper_cpuid(CPUX86State *env)
 {
     uint32_t eax, ebx, ecx, edx;
 
-    cpu_svm_check_intercept_param(env, SVM_EXIT_CPUID, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_CPUID, 0, GETPC());
 
     cpu_x86_cpuid(env, (uint32_t)env->regs[R_EAX], (uint32_t)env->regs[R_ECX],
                   &eax, &ebx, &ecx, &edx);
@@ -125,7 +125,7 @@ target_ulong helper_read_crN(CPUX86State *env, int reg)
 {
     target_ulong val;
 
-    cpu_svm_check_intercept_param(env, SVM_EXIT_READ_CR0 + reg, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_READ_CR0 + reg, 0, GETPC());
     switch (reg) {
     default:
         val = env->cr[reg];
@@ -143,7 +143,7 @@ target_ulong helper_read_crN(CPUX86State *env, int reg)
 
 void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
 {
-    cpu_svm_check_intercept_param(env, SVM_EXIT_WRITE_CR0 + reg, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_WRITE_CR0 + reg, 0, GETPC());
     switch (reg) {
     case 0:
         cpu_x86_update_cr0(env, t0);
@@ -179,7 +179,7 @@ void helper_invlpg(CPUX86State *env, target_ulong addr)
 {
     X86CPU *cpu = x86_env_get_cpu(env);
 
-    cpu_svm_check_intercept_param(env, SVM_EXIT_INVLPG, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_INVLPG, 0, GETPC());
     tlb_flush_page(CPU(cpu), addr);
 }
 
@@ -190,7 +190,7 @@ void helper_rdtsc(CPUX86State *env)
     if ((env->cr[4] & CR4_TSD_MASK) && ((env->hflags & HF_CPL_MASK) != 0)) {
         raise_exception_ra(env, EXCP0D_GPF, GETPC());
     }
-    cpu_svm_check_intercept_param(env, SVM_EXIT_RDTSC, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_RDTSC, 0, GETPC());
 
     val = cpu_get_tsc(env) + env->tsc_offset;
     env->regs[R_EAX] = (uint32_t)(val);
@@ -208,7 +208,7 @@ void helper_rdpmc(CPUX86State *env)
     if ((env->cr[4] & CR4_PCE_MASK) && ((env->hflags & HF_CPL_MASK) != 0)) {
         raise_exception_ra(env, EXCP0D_GPF, GETPC());
     }
-    cpu_svm_check_intercept_param(env, SVM_EXIT_RDPMC, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_RDPMC, 0, GETPC());
 
     /* currently unimplemented */
     qemu_log_mask(LOG_UNIMP, "x86: unimplemented rdpmc\n");
@@ -228,7 +228,7 @@ void helper_wrmsr(CPUX86State *env)
 {
     uint64_t val;
 
-    cpu_svm_check_intercept_param(env, SVM_EXIT_MSR, 1);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_MSR, 1, GETPC());
 
     val = ((uint32_t)env->regs[R_EAX]) |
         ((uint64_t)((uint32_t)env->regs[R_EDX]) << 32);
@@ -388,7 +388,7 @@ void helper_rdmsr(CPUX86State *env)
 {
     uint64_t val;
 
-    cpu_svm_check_intercept_param(env, SVM_EXIT_MSR, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_MSR, 0, GETPC());
 
     switch ((uint32_t)env->regs[R_ECX]) {
     case MSR_IA32_SYSENTER_CS:
@@ -557,7 +557,7 @@ void helper_hlt(CPUX86State *env, int next_eip_addend)
 {
     X86CPU *cpu = x86_env_get_cpu(env);
 
-    cpu_svm_check_intercept_param(env, SVM_EXIT_HLT, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_HLT, 0, GETPC());
     env->eip += next_eip_addend;
 
     do_hlt(cpu);
@@ -569,7 +569,7 @@ void helper_monitor(CPUX86State *env, target_ulong ptr)
         raise_exception_ra(env, EXCP0D_GPF, GETPC());
     }
     /* XXX: store address? */
-    cpu_svm_check_intercept_param(env, SVM_EXIT_MONITOR, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_MONITOR, 0, GETPC());
 }
 
 void helper_mwait(CPUX86State *env, int next_eip_addend)
@@ -580,7 +580,7 @@ void helper_mwait(CPUX86State *env, int next_eip_addend)
     if ((uint32_t)env->regs[R_ECX] != 0) {
         raise_exception_ra(env, EXCP0D_GPF, GETPC());
     }
-    cpu_svm_check_intercept_param(env, SVM_EXIT_MWAIT, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_MWAIT, 0, GETPC());
     env->eip += next_eip_addend;
 
     cpu = x86_env_get_cpu(env);
@@ -597,7 +597,7 @@ void helper_pause(CPUX86State *env, int next_eip_addend)
 {
     X86CPU *cpu = x86_env_get_cpu(env);
 
-    cpu_svm_check_intercept_param(env, SVM_EXIT_PAUSE, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_PAUSE, 0, GETPC());
     env->eip += next_eip_addend;
 
     do_pause(cpu);
diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c
index d24574d..5c845dc 100644
--- a/target/i386/seg_helper.c
+++ b/target/i386/seg_helper.c
@@ -1335,7 +1335,7 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
     } else if (env->hflags2 & HF2_GIF_MASK) {
         if ((interrupt_request & CPU_INTERRUPT_SMI) &&
             !(env->hflags & HF_SMM_MASK)) {
-            cpu_svm_check_intercept_param(env, SVM_EXIT_SMI, 0);
+            cpu_svm_check_intercept_param(env, SVM_EXIT_SMI, 0, 0);
             cs->interrupt_request &= ~CPU_INTERRUPT_SMI;
             do_smm_enter(cpu);
             ret = true;
@@ -1356,7 +1356,7 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
                      (env->eflags & IF_MASK &&
                       !(env->hflags & HF_INHIBIT_IRQ_MASK))))) {
             int intno;
-            cpu_svm_check_intercept_param(env, SVM_EXIT_INTR, 0);
+            cpu_svm_check_intercept_param(env, SVM_EXIT_INTR, 0, 0);
             cs->interrupt_request &= ~(CPU_INTERRUPT_HARD |
                                        CPU_INTERRUPT_VIRQ);
             intno = cpu_get_pic_interrupt(env);
@@ -1372,7 +1372,7 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
                    !(env->hflags & HF_INHIBIT_IRQ_MASK)) {
             int intno;
             /* FIXME: this should respect TPR */
-            cpu_svm_check_intercept_param(env, SVM_EXIT_VINTR, 0);
+            cpu_svm_check_intercept_param(env, SVM_EXIT_VINTR, 0, 0);
             intno = x86_ldl_phys(cs, env->vm_vmcb
                              + offsetof(struct vmcb, control.int_vector));
             qemu_log_mask(CPU_LOG_TB_IN_ASM,
diff --git a/target/i386/svm_helper.c b/target/i386/svm_helper.c
index 210f6aa..5fecf77 100644
--- a/target/i386/svm_helper.c
+++ b/target/i386/svm_helper.c
@@ -60,11 +60,8 @@ void helper_invlpga(CPUX86State *env, int aflag)
 {
 }
 
-void helper_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1)
-{
-}
-
-void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1)
+void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1,
+                uint64_t retaddr)
 {
 }
 
@@ -74,7 +71,7 @@ void helper_svm_check_intercept_param(CPUX86State *env, uint32_t type,
 }
 
 void cpu_svm_check_intercept_param(CPUX86State *env, uint32_t type,
-                                   uint64_t param)
+                                   uint64_t param, uint64_t retaddr)
 {
 }
 
@@ -130,7 +127,7 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
     uint32_t event_inj;
     uint32_t int_ctl;
 
-    cpu_svm_check_intercept_param(env, SVM_EXIT_VMRUN, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_VMRUN, 0, GETPC());
 
     if (aflag == 2) {
         addr = env->regs[R_EAX];
@@ -355,7 +352,7 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
 
 void helper_vmmcall(CPUX86State *env)
 {
-    cpu_svm_check_intercept_param(env, SVM_EXIT_VMMCALL, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_VMMCALL, 0, GETPC());
     raise_exception(env, EXCP06_ILLOP);
 }
 
@@ -364,7 +361,7 @@ void helper_vmload(CPUX86State *env, int aflag)
     CPUState *cs = CPU(x86_env_get_cpu(env));
     target_ulong addr;
 
-    cpu_svm_check_intercept_param(env, SVM_EXIT_VMLOAD, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_VMLOAD, 0, GETPC());
 
     if (aflag == 2) {
         addr = env->regs[R_EAX];
@@ -404,7 +401,7 @@ void helper_vmsave(CPUX86State *env, int aflag)
     CPUState *cs = CPU(x86_env_get_cpu(env));
     target_ulong addr;
 
-    cpu_svm_check_intercept_param(env, SVM_EXIT_VMSAVE, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_VMSAVE, 0, GETPC());
 
     if (aflag == 2) {
         addr = env->regs[R_EAX];
@@ -445,19 +442,19 @@ void helper_vmsave(CPUX86State *env, int aflag)
 
 void helper_stgi(CPUX86State *env)
 {
-    cpu_svm_check_intercept_param(env, SVM_EXIT_STGI, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_STGI, 0, GETPC());
     env->hflags2 |= HF2_GIF_MASK;
 }
 
 void helper_clgi(CPUX86State *env)
 {
-    cpu_svm_check_intercept_param(env, SVM_EXIT_CLGI, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_CLGI, 0, GETPC());
     env->hflags2 &= ~HF2_GIF_MASK;
 }
 
 void helper_skinit(CPUX86State *env)
 {
-    cpu_svm_check_intercept_param(env, SVM_EXIT_SKINIT, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_SKINIT, 0, GETPC());
     /* XXX: not implemented */
     raise_exception(env, EXCP06_ILLOP);
 }
@@ -467,7 +464,7 @@ void helper_invlpga(CPUX86State *env, int aflag)
     X86CPU *cpu = x86_env_get_cpu(env);
     target_ulong addr;
 
-    cpu_svm_check_intercept_param(env, SVM_EXIT_INVLPGA, 0);
+    cpu_svm_check_intercept_param(env, SVM_EXIT_INVLPGA, 0, GETPC());
 
     if (aflag == 2) {
         addr = env->regs[R_EAX];
@@ -480,8 +477,8 @@ void helper_invlpga(CPUX86State *env, int aflag)
     tlb_flush_page(CPU(cpu), addr);
 }
 
-void helper_svm_check_intercept_param(CPUX86State *env, uint32_t type,
-                                      uint64_t param)
+void cpu_svm_check_intercept_param(CPUX86State *env, uint32_t type,
+                                   uint64_t param, uint64_t retaddr)
 {
     CPUState *cs = CPU(x86_env_get_cpu(env));
 
@@ -491,27 +488,27 @@ void helper_svm_check_intercept_param(CPUX86State *env, uint32_t type,
     switch (type) {
     case SVM_EXIT_READ_CR0 ... SVM_EXIT_READ_CR0 + 8:
         if (env->intercept_cr_read & (1 << (type - SVM_EXIT_READ_CR0))) {
-            helper_vmexit(env, type, param);
+            cpu_vmexit(env, type, param, retaddr);
         }
         break;
     case SVM_EXIT_WRITE_CR0 ... SVM_EXIT_WRITE_CR0 + 8:
         if (env->intercept_cr_write & (1 << (type - SVM_EXIT_WRITE_CR0))) {
-            helper_vmexit(env, type, param);
+            cpu_vmexit(env, type, param, retaddr);
         }
         break;
     case SVM_EXIT_READ_DR0 ... SVM_EXIT_READ_DR0 + 7:
         if (env->intercept_dr_read & (1 << (type - SVM_EXIT_READ_DR0))) {
-            helper_vmexit(env, type, param);
+            cpu_vmexit(env, type, param, retaddr);
         }
         break;
     case SVM_EXIT_WRITE_DR0 ... SVM_EXIT_WRITE_DR0 + 7:
         if (env->intercept_dr_write & (1 << (type - SVM_EXIT_WRITE_DR0))) {
-            helper_vmexit(env, type, param);
+            cpu_vmexit(env, type, param, retaddr);
         }
         break;
     case SVM_EXIT_EXCP_BASE ... SVM_EXIT_EXCP_BASE + 31:
         if (env->intercept_exceptions & (1 << (type - SVM_EXIT_EXCP_BASE))) {
-            helper_vmexit(env, type, param);
+            cpu_vmexit(env, type, param, retaddr);
         }
         break;
     case SVM_EXIT_MSR:
@@ -538,28 +535,28 @@ void helper_svm_check_intercept_param(CPUX86State *env, uint32_t type,
                 t0 %= 8;
                 break;
             default:
-                helper_vmexit(env, type, param);
+                cpu_vmexit(env, type, param, retaddr);
                 t0 = 0;
                 t1 = 0;
                 break;
             }
             if (x86_ldub_phys(cs, addr + t1) & ((1 << param) << t0)) {
-                helper_vmexit(env, type, param);
+                cpu_vmexit(env, type, param, retaddr);
             }
         }
         break;
     default:
         if (env->intercept & (1ULL << (type - SVM_EXIT_INTR))) {
-            helper_vmexit(env, type, param);
+            cpu_vmexit(env, type, param, retaddr);
         }
         break;
     }
 }
 
-void cpu_svm_check_intercept_param(CPUX86State *env, uint32_t type,
-                                   uint64_t param)
+void helper_svm_check_intercept_param(CPUX86State *env, uint32_t type,
+                                      uint64_t param)
 {
-    helper_svm_check_intercept_param(env, type, param);
+    cpu_svm_check_intercept_param(env, type, param, GETPC());
 }
 
 void helper_svm_check_io(CPUX86State *env, uint32_t port, uint32_t param,
@@ -578,17 +575,22 @@ void helper_svm_check_io(CPUX86State *env, uint32_t port, uint32_t param,
             x86_stq_phys(cs,
                      env->vm_vmcb + offsetof(struct vmcb, control.exit_info_2),
                      env->eip + next_eip_addend);
-            helper_vmexit(env, SVM_EXIT_IOIO, param | (port << 16));
+            cpu_vmexit(env, SVM_EXIT_IOIO, param | (port << 16), GETPC());
         }
     }
 }
 
 /* Note: currently only 32 bits of exit_code are used */
-void helper_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1)
+void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1,
+                uintptr_t retaddr)
 {
     CPUState *cs = CPU(x86_env_get_cpu(env));
     uint32_t int_ctl;
 
+    if (retaddr) {
+        cpu_restore_state(cs, retaddr);
+    }
+
     qemu_log_mask(CPU_LOG_TB_IN_ASM, "vmexit(%08x, %016" PRIx64 ", %016"
                   PRIx64 ", " TARGET_FMT_lx ")!\n",
                   exit_code, exit_info_1,
@@ -766,9 +768,4 @@ void helper_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1)
     cpu_loop_exit(cs);
 }
 
-void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1)
-{
-    helper_vmexit(env, exit_code, exit_info_1);
-}
-
 #endif
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 21/23] report guest crash information in GUEST_PANICKED event

[Eric Blake]

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

On 02/16/2017 08:31 AM, Paolo Bonzini wrote:
> From: Anton Nefedov <anton.nefedov@virtuozzo.com>
> 
> it's not very convenient to use the crash-information property interface,
> so provide a CPU class callback to get the guest crash information, and pass
> that information in the event
> 
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Message-Id: <1487053524-18674-3-git-send-email-den@openvz.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

> +++ b/qapi/event.json
> @@ -488,7 +488,9 @@
>  #
>  # @action: action that has been taken, currently always "pause"
>  #
> -# Since: 1.5
> +# @info: optional information about a panic
> +#
> +# Since: 1.5 (@info since 2.9)

I had a (late) comment on this being unusual formatting, but it can be
fixed in a followup patch if there is no other reason to respin the pull
request.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PULL 00/23] Misc patches for 2017-02-16

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PULL 00/23] Misc patches for 2017-02-16
Message-id: 1487255507-106654-1-git-send-email-pbonzini@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1487255507-106654-1-git-send-email-pbonzini@redhat.com -> patchew/1487255507-106654-1-git-send-email-pbonzini@redhat.com
Switched to a new branch 'test'
a842dba target-i386: correctly propagate retaddr into SVM helpers
36f0d76 vl: log available guest crash information
5951e60 report guest crash information in GUEST_PANICKED event
e3d0196 i386/cpu: add crash-information QOM property
b22a547 Makefile: avoid leaving the temporary QEMU_PKGVERSION header file
1ac46cf vl: Move the cpu_synchronize_all_post_init() after generic devices initialization
ad5c282 qemu-nbd: Implement socket activation.
511652d qemu-doc: Clarify that -vga std is now the default
7ab8e56 cpu-exec: remove outermost infinite loop
8a13214 cpu-exec: avoid repeated sigsetjmp on interrupts
03e16ce cpu-exec: avoid cpu_loop_exit in cpu_handle_interrupt
4ff5c14 cpu-exec: tighten barrier on TCG_EXIT_REQUESTED
942cdfa cpu-exec: fix icount out-of-bounds access
91aa9ee hw/char/mcf_uart: QOMify the ColdFire UART
f437c15 gdbstub: Fix vCont behaviour
239bca2 move vm_start to cpus.c
5aab44b char: drop data written to a disconnected pty
f50cc4e apic: reset apic_delivered global variable on machine reset
d4b7bdc qemu-char: socket backend: disconnect on write error
26e8792 test-vmstate: remove yield_until_fd_readable
8394c0b kvm/ioapic: correct kvm ioapic version
30cb597 ioapic: fix error report value of def version
342745d kvm/ioapic: dump real object instead of a fake one

=== OUTPUT BEGIN ===
Checking PATCH 1/23: kvm/ioapic: dump real object instead of a fake one...
Checking PATCH 2/23: ioapic: fix error report value of def version...
Checking PATCH 3/23: kvm/ioapic: correct kvm ioapic version...
Checking PATCH 4/23: test-vmstate: remove yield_until_fd_readable...
Checking PATCH 5/23: qemu-char: socket backend: disconnect on write error...
Checking PATCH 6/23: apic: reset apic_delivered global variable on machine reset...
Checking PATCH 7/23: char: drop data written to a disconnected pty...
Checking PATCH 8/23: move vm_start to cpus.c...
Checking PATCH 9/23: gdbstub: Fix vCont behaviour...
Checking PATCH 10/23: hw/char/mcf_uart: QOMify the ColdFire UART...
Checking PATCH 11/23: cpu-exec: fix icount out-of-bounds access...
Checking PATCH 12/23: cpu-exec: tighten barrier on TCG_EXIT_REQUESTED...
ERROR: memory barrier without comment
#51: FILE: cpu-exec.c:559:
+        smp_mb();

total: 1 errors, 0 warnings, 13 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 13/23: cpu-exec: avoid cpu_loop_exit in cpu_handle_interrupt...
Checking PATCH 14/23: cpu-exec: avoid repeated sigsetjmp on interrupts...
Checking PATCH 15/23: cpu-exec: remove outermost infinite loop...
WARNING: line over 80 characters
#51: FILE: cpu-exec.c:633:
+         * Newer versions of gcc would complain about this code (-Wclobbered). */

total: 0 errors, 1 warnings, 68 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 16/23: qemu-doc: Clarify that -vga std is now the default...
Checking PATCH 17/23: qemu-nbd: Implement socket activation....
Checking PATCH 18/23: vl: Move the cpu_synchronize_all_post_init() after generic devices initialization...
Checking PATCH 19/23: Makefile: avoid leaving the temporary QEMU_PKGVERSION header file...
Checking PATCH 20/23: i386/cpu: add crash-information QOM property...
Checking PATCH 21/23: report guest crash information in GUEST_PANICKED event...
Checking PATCH 22/23: vl: log available guest crash information...
Checking PATCH 23/23: target-i386: correctly propagate retaddr into SVM helpers...
WARNING: line over 80 characters
#76: FILE: target/i386/excp_helper.c:57:
+            cpu_vmexit(env, SVM_EXIT_SHUTDOWN, 0, retaddr); /* does not return */

total: 0 errors, 1 warnings, 368 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PULL 21/23] report guest crash information in GUEST_PANICKED event

[Denis V. Lunev]

On 02/16/2017 07:07 PM, Eric Blake wrote:
> On 02/16/2017 08:31 AM, Paolo Bonzini wrote:
>> From: Anton Nefedov <anton.nefedov@virtuozzo.com>
>>
>> it's not very convenient to use the crash-information property interface,
>> so provide a CPU class callback to get the guest crash information, and pass
>> that information in the event
>>
>> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> Message-Id: <1487053524-18674-3-git-send-email-den@openvz.org>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> +++ b/qapi/event.json
>> @@ -488,7 +488,9 @@
>>  #
>>  # @action: action that has been taken, currently always "pause"
>>  #
>> -# Since: 1.5
>> +# @info: optional information about a panic
>> +#
>> +# Since: 1.5 (@info since 2.9)
> I had a (late) comment on this being unusual formatting, but it can be
> fixed in a followup patch if there is no other reason to respin the pull
> request.
>
OK, we will send the follow up.

Anton, pls.

Den

[Qemu-devel] [PATCH] qmp-events: fix GUEST_PANICKED description formatting

[Anton Nefedov]

Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
---
 qapi/event.json | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qapi/event.json b/qapi/event.json
index 970ff02..e02852c 100644
--- a/qapi/event.json
+++ b/qapi/event.json
@@ -488,9 +488,9 @@
 #
 # @action: action that has been taken, currently always "pause"
 #
-# @info: optional information about a panic
+# @info: #optional information about a panic (since 2.9)
 #
-# Since: 1.5 (@info since 2.9)
+# Since: 1.5
 #
 # Example:
 #
-- 
2.7.4

Re: [Qemu-devel] [PATCH] qmp-events: fix GUEST_PANICKED description formatting

[Eric Blake]

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

On 02/16/2017 10:30 AM, Anton Nefedov wrote:
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> ---
>  qapi/event.json | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi/event.json b/qapi/event.json
> index 970ff02..e02852c 100644
> --- a/qapi/event.json
> +++ b/qapi/event.json
> @@ -488,9 +488,9 @@
>  #
>  # @action: action that has been taken, currently always "pause"
>  #
> -# @info: optional information about a panic
> +# @info: #optional information about a panic (since 2.9)
>  #
> -# Since: 1.5 (@info since 2.9)
> +# Since: 1.5

My comment on v4 also pointed out a useless conditional that should be
fixed at the same time:

https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg03361.html

And it's probably better to submit this patch as a top-level thread
rather than buried in-reply-to a pull request.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PULL 00/23] Misc patches for 2017-02-16

[Peter Maydell]

On 16 February 2017 at 14:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit 5dae13cd71f0755a1395b5a4cde635b8a6ee3f58:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-or-20170214' into staging (2017-02-14 09:55:48 +0000)
>
> are available in the git repository at:
>
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 395d41a549f69f4ec603afa3d3ade1e2005777d5:
>
>   target-i386: correctly propagate retaddr into SVM helpers (2017-02-16 15:30:49 +0100)
>
> ----------------------------------------------------------------
> * GUEST_PANICKED improvements (Anton)
> * vCont gdbstub rewrite (Claudio)
> * Fix CPU creation with -device (Liyang)
> * Logging fixes for pty chardevs (Ed)
> * Makefile "move if changed" fix (Lin)
> * First part of cpu_exec refactoring (me)
> * SVM emulation fix (me)
> * apic_delivered fix (Pavel)
> * "info ioapic" fix (Peter)
> * qemu-nbd socket activation (Richard)
> * QOMification of mcf_uart (Thomas)

Fails to build on 32-bit ARM:

/home/petmay01/qemu/target/i386/svm_helper.c:63:6: error: conflicting
types for 'cpu_v
mexit'
 void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1,
      ^
In file included from /home/petmay01/qemu/target/i386/svm_helper.c:21:0:
/home/petmay01/qemu/target/i386/cpu.h:1625:6: note: previous
declaration of 'cpu_vmexit' was here
 void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1,
      ^
/home/petmay01/qemu/rules.mak:69: recipe for target
'target/i386/svm_helper.o' failed

The problem is in the bit of the prototype the error message
doesn't quote:

Prototype in cpu.h:
  void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1,
                  uintptr_t retaddr);

svm_helper.c stub version;
  void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1,
                  uint64_t retaddr)

svm_helper.c real version:
  void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1,
                  uintptr_t retaddr)

On 32-bit uintptr_t and uint64_t aren't the same thing.

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/23] Misc patches for 2017-02-16

[Paolo Bonzini]

> The problem is in the bit of the prototype the error message
> doesn't quote:
> 
> Prototype in cpu.h:
>   void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t
>   exit_info_1,
>                   uintptr_t retaddr);
> 
> svm_helper.c stub version;
>   void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t
>   exit_info_1,
>                   uint64_t retaddr)
> 
> svm_helper.c real version:
>   void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1,
>                   uintptr_t retaddr)
> 
> On 32-bit uintptr_t and uint64_t aren't the same thing.

Saw the same on mingw, must have forgotten to "commit --amend".

Paolo

Re: [Qemu-devel] [PULL 09/23] gdbstub: Fix vCont behaviour

[Alex Bennée]

Paolo Bonzini <pbonzini@redhat.com> writes:

> From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>
> When GDB issues a "vCont", QEMU was not handling it correctly when
> multiple VCPUs are active.
> For vCont, for each thread (VCPU), it can be specified whether to
> single step, continue or stop that thread. The default is to stop a
> thread.
<snip>
>
> +/**
> + * gdb_handle_vcont - Parses and handles a vCont packet.
> + * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
> + *         a format error, 0 on success.
> + */
> +static int gdb_handle_vcont(GDBState *s, const char *p)
> +{
> +    int res, idx, signal = 0;
> +    char cur_action;
> +    char *newstates;
> +    unsigned long tmp;
> +    CPUState *cpu;
> +#ifdef CONFIG_USER_ONLY
> +    int max_cpus = 1; /* global variable max_cpus exists only in system mode */
> +
> +    CPU_FOREACH(cpu) {
> +        max_cpus = max_cpus <= cpu->cpu_index ? cpu->cpu_index + 1 : max_cpus;
> +    }
> +#endif
> +    /* uninitialised CPUs stay 0 */
> +    newstates = g_new0(char, max_cpus);
> +
> +    /* mark valid CPUs with 1 */
> +    CPU_FOREACH(cpu) {
> +        newstates[cpu->cpu_index] = 1;
> +    }
> +
> +    /*
> +     * res keeps track of what error we are returning, with -ENOTSUP meaning
> +     * that the command is unknown or unsupported, thus returning an empty
> +     * packet, while -EINVAL and -ERANGE cause an E22 packet, due to invalid,
> +     *  or incorrect parameters passed.
> +     */
> +    res = 0;
> +    while (*p) {
> +        if (*p++ != ';') {
> +            res = -ENOTSUP;
> +            goto out;
> +        }
> +
> +        cur_action = *p++;
> +        if (cur_action == 'C' || cur_action == 'S') {
> +            cur_action = tolower(cur_action);
> +            res = qemu_strtoul(p + 1, &p, 16, &tmp);
> +            if (res) {
> +                goto out;
> +            }
> +            signal = gdb_signal_to_target(tmp);
> +        } else if (cur_action != 'c' && cur_action != 's') {
> +            /* unknown/invalid/unsupported command */
> +            res = -ENOTSUP;
> +            goto out;
> +        }
> +        /* thread specification. special values: (none), -1 = all; 0 = any */
> +        if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
> +            if (*p == ':') {
> +                p += 3;
> +            }
> +            for (idx = 0; idx < max_cpus; idx++) {
> +                if (newstates[idx] == 1) {
> +                    newstates[idx] = cur_action;
> +                }
> +            }
> +        } else if (*p == ':') {
> +            p++;
> +            res = qemu_strtoul(p, &p, 16, &tmp);
> +            if (res) {
> +                goto out;
> +            }
> +            idx = tmp;
> +            /* 0 means any thread, so we pick the first valid CPU */
> +            if (!idx) {
> +                idx = cpu_index(first_cpu);
> +            }
> +
> +            /*
> +             * If we are in user mode, the thread specified is actually a
> +             * thread id, and not an index. We need to find the actual
> +             * CPU first, and only then we can use its index.
> +             */
> +            cpu = find_cpu(idx);
> +            /* invalid CPU/thread specified */
> +            if (!idx || !cpu) {
> +                res = -EINVAL;
> +                goto out;
> +            }

This fails on a packet like vCont;C04:0;c where we do find a cpu but it
happens to have a internal cpu_index of 0.

I'm sending a patch.

--
Alex Bennée

Re: [Qemu-devel] [PULL 09/23] gdbstub: Fix vCont behaviour

[Jan Kiszka]

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

On 2017-02-16 15:31, Paolo Bonzini wrote:
> From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> 
> When GDB issues a "vCont", QEMU was not handling it correctly when
> multiple VCPUs are active.
> For vCont, for each thread (VCPU), it can be specified whether to
> single step, continue or stop that thread. The default is to stop a
> thread.
> However, when (for example) "vCont;s:2" is issued, all VCPUs continue
> to run, although all but VCPU nr 2 are to be stopped.
> 
> This patch completely rewrites the vCont parsing code.
> 
> Please note that this improvement only works in system emulation mode,
> when in userspace emulation mode the old behaviour is preserved.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> Message-Id: <1487092068-16562-3-git-send-email-imbrenda@linux.vnet.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  gdbstub.c | 209 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 162 insertions(+), 47 deletions(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index 755a8e3..9911153 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -387,6 +387,60 @@ static inline void gdb_continue(GDBState *s)
>  #endif
>  }
>  
> +/*
> + * Resume execution, per CPU actions. For user-mode emulation it's
> + * equivalent to gdb_continue.
> + */
> +static int gdb_continue_partial(GDBState *s, char *newstates)
> +{
> +    CPUState *cpu;
> +    int res = 0;
> +#ifdef CONFIG_USER_ONLY
> +    /*
> +     * This is not exactly accurate, but it's an improvement compared to the
> +     * previous situation, where only one CPU would be single-stepped.
> +     */
> +    CPU_FOREACH(cpu) {
> +        if (newstates[cpu->cpu_index] == 's') {
> +            cpu_single_step(cpu, sstep_flags);
> +        }
> +    }
> +    s->running_state = 1;
> +#else
> +    int flag = 0;
> +
> +    if (!runstate_needs_reset()) {
> +        if (vm_prepare_start()) {
> +            return 0;
> +        }
> +
> +        CPU_FOREACH(cpu) {
> +            switch (newstates[cpu->cpu_index]) {
> +            case 0:
> +            case 1:
> +                break; /* nothing to do here */
> +            case 's':
> +                cpu_single_step(cpu, sstep_flags);
> +                cpu_resume(cpu);
> +                flag = 1;
> +                break;
> +            case 'c':
> +                cpu_resume(cpu);
> +                flag = 1;
> +                break;
> +            default:
> +                res = -1;
> +                break;
> +            }
> +        }
> +    }
> +    if (flag) {
> +        qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true);
> +    }
> +#endif
> +    return res;
> +}
> +
>  static void put_buffer(GDBState *s, const uint8_t *buf, int len)
>  {
>  #ifdef CONFIG_USER_ONLY
> @@ -785,6 +839,107 @@ static int is_query_packet(const char *p, const char *query, char separator)
>          (p[query_len] == '\0' || p[query_len] == separator);
>  }
>  
> +/**
> + * gdb_handle_vcont - Parses and handles a vCont packet.
> + * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
> + *         a format error, 0 on success.
> + */
> +static int gdb_handle_vcont(GDBState *s, const char *p)
> +{
> +    int res, idx, signal = 0;
> +    char cur_action;
> +    char *newstates;
> +    unsigned long tmp;
> +    CPUState *cpu;
> +#ifdef CONFIG_USER_ONLY
> +    int max_cpus = 1; /* global variable max_cpus exists only in system mode */
> +
> +    CPU_FOREACH(cpu) {
> +        max_cpus = max_cpus <= cpu->cpu_index ? cpu->cpu_index + 1 : max_cpus;
> +    }
> +#endif
> +    /* uninitialised CPUs stay 0 */
> +    newstates = g_new0(char, max_cpus);
> +
> +    /* mark valid CPUs with 1 */
> +    CPU_FOREACH(cpu) {
> +        newstates[cpu->cpu_index] = 1;
> +    }
> +
> +    /*
> +     * res keeps track of what error we are returning, with -ENOTSUP meaning
> +     * that the command is unknown or unsupported, thus returning an empty
> +     * packet, while -EINVAL and -ERANGE cause an E22 packet, due to invalid,
> +     *  or incorrect parameters passed.
> +     */
> +    res = 0;
> +    while (*p) {
> +        if (*p++ != ';') {
> +            res = -ENOTSUP;
> +            goto out;
> +        }
> +
> +        cur_action = *p++;
> +        if (cur_action == 'C' || cur_action == 'S') {
> +            cur_action = tolower(cur_action);
> +            res = qemu_strtoul(p + 1, &p, 16, &tmp);
> +            if (res) {
> +                goto out;
> +            }
> +            signal = gdb_signal_to_target(tmp);
> +        } else if (cur_action != 'c' && cur_action != 's') {
> +            /* unknown/invalid/unsupported command */
> +            res = -ENOTSUP;
> +            goto out;
> +        }
> +        /* thread specification. special values: (none), -1 = all; 0 = any */
> +        if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
> +            if (*p == ':') {
> +                p += 3;
> +            }
> +            for (idx = 0; idx < max_cpus; idx++) {
> +                if (newstates[idx] == 1) {
> +                    newstates[idx] = cur_action;
> +                }
> +            }
> +        } else if (*p == ':') {
> +            p++;
> +            res = qemu_strtoul(p, &p, 16, &tmp);
> +            if (res) {
> +                goto out;
> +            }
> +            idx = tmp;
> +            /* 0 means any thread, so we pick the first valid CPU */
> +            if (!idx) {
> +                idx = cpu_index(first_cpu);
> +            }
> +
> +            /*
> +             * If we are in user mode, the thread specified is actually a
> +             * thread id, and not an index. We need to find the actual
> +             * CPU first, and only then we can use its index.
> +             */
> +            cpu = find_cpu(idx);
> +            /* invalid CPU/thread specified */
> +            if (!idx || !cpu) {
> +                res = -EINVAL;
> +                goto out;
> +            }
> +            /* only use if no previous match occourred */
> +            if (newstates[cpu->cpu_index] == 1) {
> +                newstates[cpu->cpu_index] = cur_action;
> +            }
> +        }
> +    }
> +    s->signal = signal;
> +    gdb_continue_partial(s, newstates);
> +
> +out:
> +    g_free(newstates);
> +
> +    return res;
> +}
> +
>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>  {
>      CPUState *cpu;
> @@ -830,60 +985,20 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          return RS_IDLE;
>      case 'v':
>          if (strncmp(p, "Cont", 4) == 0) {
> -            int res_signal, res_thread;
> -
>              p += 4;
>              if (*p == '?') {
>                  put_packet(s, "vCont;c;C;s;S");
>                  break;
>              }
> -            res = 0;
> -            res_signal = 0;
> -            res_thread = 0;
> -            while (*p) {
> -                int action, signal;
> -
> -                if (*p++ != ';') {
> -                    res = 0;
> -                    break;
> -                }
> -                action = *p++;
> -                signal = 0;
> -                if (action == 'C' || action == 'S') {
> -                    signal = gdb_signal_to_target(strtoul(p, (char **)&p, 16));
> -                    if (signal == -1) {
> -                        signal = 0;
> -                    }
> -                } else if (action != 'c' && action != 's') {
> -                    res = 0;
> -                    break;
> -                }
> -                thread = 0;
> -                if (*p == ':') {
> -                    thread = strtoull(p+1, (char **)&p, 16);
> -                }
> -                action = tolower(action);
> -                if (res == 0 || (res == 'c' && action == 's')) {
> -                    res = action;
> -                    res_signal = signal;
> -                    res_thread = thread;
> -                }
> -            }
> +
> +            res = gdb_handle_vcont(s, p);
> +
>              if (res) {
> -                if (res_thread != -1 && res_thread != 0) {
> -                    cpu = find_cpu(res_thread);
> -                    if (cpu == NULL) {
> -                        put_packet(s, "E22");
> -                        break;
> -                    }
> -                    s->c_cpu = cpu;
> -                }
> -                if (res == 's') {
> -                    cpu_single_step(s->c_cpu, sstep_flags);
> +                if ((res == -EINVAL) || (res == -ERANGE)) {
> +                    put_packet(s, "E22");
> +                    break;
>                  }
> -                s->signal = res_signal;
> -                gdb_continue(s);
> -                return RS_IDLE;
> +                goto unknown_command;
>              }
>              break;
>          } else {
> 

Seems like no one is doing guest debugging with kvm on x86 except me,
and I'm only doing it too infrequently now: This one broke that use case
for SMP guests long ago. How was it tested?

To reproduce the bug: set up an x86-64 guest kernel with > 1 core, break
on some prominent syscall entry (e.g. sys_execve), continue the guest on
hit and it will quickly lock up, even after disabling the breakpoint
again. Kernel version doesn't matter (was my first guess), gdb is
7.7.50.20140604-cvs (OpenSUSE) here.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PULL 09/23] gdbstub: Fix vCont behaviour

[Jan Kiszka]

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

On 2018-02-17 09:56, Jan Kiszka wrote:
> On 2017-02-16 15:31, Paolo Bonzini wrote:
>> From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>>
>> When GDB issues a "vCont", QEMU was not handling it correctly when
>> multiple VCPUs are active.
>> For vCont, for each thread (VCPU), it can be specified whether to
>> single step, continue or stop that thread. The default is to stop a
>> thread.
>> However, when (for example) "vCont;s:2" is issued, all VCPUs continue
>> to run, although all but VCPU nr 2 are to be stopped.
>>
>> This patch completely rewrites the vCont parsing code.
>>
>> Please note that this improvement only works in system emulation mode,
>> when in userspace emulation mode the old behaviour is preserved.
>>
>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>> Message-Id: <1487092068-16562-3-git-send-email-imbrenda@linux.vnet.ibm.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  gdbstub.c | 209 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>>  1 file changed, 162 insertions(+), 47 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index 755a8e3..9911153 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -387,6 +387,60 @@ static inline void gdb_continue(GDBState *s)
>>  #endif
>>  }
>>  
>> +/*
>> + * Resume execution, per CPU actions. For user-mode emulation it's
>> + * equivalent to gdb_continue.
>> + */
>> +static int gdb_continue_partial(GDBState *s, char *newstates)
>> +{
>> +    CPUState *cpu;
>> +    int res = 0;
>> +#ifdef CONFIG_USER_ONLY
>> +    /*
>> +     * This is not exactly accurate, but it's an improvement compared to the
>> +     * previous situation, where only one CPU would be single-stepped.
>> +     */
>> +    CPU_FOREACH(cpu) {
>> +        if (newstates[cpu->cpu_index] == 's') {
>> +            cpu_single_step(cpu, sstep_flags);
>> +        }
>> +    }
>> +    s->running_state = 1;
>> +#else
>> +    int flag = 0;
>> +
>> +    if (!runstate_needs_reset()) {
>> +        if (vm_prepare_start()) {
>> +            return 0;
>> +        }
>> +
>> +        CPU_FOREACH(cpu) {
>> +            switch (newstates[cpu->cpu_index]) {
>> +            case 0:
>> +            case 1:
>> +                break; /* nothing to do here */
>> +            case 's':
>> +                cpu_single_step(cpu, sstep_flags);
>> +                cpu_resume(cpu);
>> +                flag = 1;
>> +                break;
>> +            case 'c':
>> +                cpu_resume(cpu);
>> +                flag = 1;
>> +                break;
>> +            default:
>> +                res = -1;
>> +                break;
>> +            }
>> +        }
>> +    }
>> +    if (flag) {
>> +        qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true);
>> +    }
>> +#endif
>> +    return res;
>> +}
>> +
>>  static void put_buffer(GDBState *s, const uint8_t *buf, int len)
>>  {
>>  #ifdef CONFIG_USER_ONLY
>> @@ -785,6 +839,107 @@ static int is_query_packet(const char *p, const char *query, char separator)
>>          (p[query_len] == '\0' || p[query_len] == separator);
>>  }
>>  
>> +/**
>> + * gdb_handle_vcont - Parses and handles a vCont packet.
>> + * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
>> + *         a format error, 0 on success.
>> + */
>> +static int gdb_handle_vcont(GDBState *s, const char *p)
>> +{
>> +    int res, idx, signal = 0;
>> +    char cur_action;
>> +    char *newstates;
>> +    unsigned long tmp;
>> +    CPUState *cpu;
>> +#ifdef CONFIG_USER_ONLY
>> +    int max_cpus = 1; /* global variable max_cpus exists only in system mode */
>> +
>> +    CPU_FOREACH(cpu) {
>> +        max_cpus = max_cpus <= cpu->cpu_index ? cpu->cpu_index + 1 : max_cpus;
>> +    }
>> +#endif
>> +    /* uninitialised CPUs stay 0 */
>> +    newstates = g_new0(char, max_cpus);
>> +
>> +    /* mark valid CPUs with 1 */
>> +    CPU_FOREACH(cpu) {
>> +        newstates[cpu->cpu_index] = 1;
>> +    }
>> +
>> +    /*
>> +     * res keeps track of what error we are returning, with -ENOTSUP meaning
>> +     * that the command is unknown or unsupported, thus returning an empty
>> +     * packet, while -EINVAL and -ERANGE cause an E22 packet, due to invalid,
>> +     *  or incorrect parameters passed.
>> +     */
>> +    res = 0;
>> +    while (*p) {
>> +        if (*p++ != ';') {
>> +            res = -ENOTSUP;
>> +            goto out;
>> +        }
>> +
>> +        cur_action = *p++;
>> +        if (cur_action == 'C' || cur_action == 'S') {
>> +            cur_action = tolower(cur_action);
>> +            res = qemu_strtoul(p + 1, &p, 16, &tmp);
>> +            if (res) {
>> +                goto out;
>> +            }
>> +            signal = gdb_signal_to_target(tmp);
>> +        } else if (cur_action != 'c' && cur_action != 's') {
>> +            /* unknown/invalid/unsupported command */
>> +            res = -ENOTSUP;
>> +            goto out;
>> +        }
>> +        /* thread specification. special values: (none), -1 = all; 0 = any */
>> +        if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
>> +            if (*p == ':') {
>> +                p += 3;
>> +            }
>> +            for (idx = 0; idx < max_cpus; idx++) {
>> +                if (newstates[idx] == 1) {
>> +                    newstates[idx] = cur_action;
>> +                }
>> +            }
>> +        } else if (*p == ':') {
>> +            p++;
>> +            res = qemu_strtoul(p, &p, 16, &tmp);
>> +            if (res) {
>> +                goto out;
>> +            }
>> +            idx = tmp;
>> +            /* 0 means any thread, so we pick the first valid CPU */
>> +            if (!idx) {
>> +                idx = cpu_index(first_cpu);
>> +            }
>> +
>> +            /*
>> +             * If we are in user mode, the thread specified is actually a
>> +             * thread id, and not an index. We need to find the actual
>> +             * CPU first, and only then we can use its index.
>> +             */
>> +            cpu = find_cpu(idx);
>> +            /* invalid CPU/thread specified */
>> +            if (!idx || !cpu) {
>> +                res = -EINVAL;
>> +                goto out;
>> +            }
>> +            /* only use if no previous match occourred */
>> +            if (newstates[cpu->cpu_index] == 1) {
>> +                newstates[cpu->cpu_index] = cur_action;
>> +            }
>> +        }
>> +    }
>> +    s->signal = signal;
>> +    gdb_continue_partial(s, newstates);
>> +
>> +out:
>> +    g_free(newstates);
>> +
>> +    return res;
>> +}
>> +
>>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>>  {
>>      CPUState *cpu;
>> @@ -830,60 +985,20 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>>          return RS_IDLE;
>>      case 'v':
>>          if (strncmp(p, "Cont", 4) == 0) {
>> -            int res_signal, res_thread;
>> -
>>              p += 4;
>>              if (*p == '?') {
>>                  put_packet(s, "vCont;c;C;s;S");
>>                  break;
>>              }
>> -            res = 0;
>> -            res_signal = 0;
>> -            res_thread = 0;
>> -            while (*p) {
>> -                int action, signal;
>> -
>> -                if (*p++ != ';') {
>> -                    res = 0;
>> -                    break;
>> -                }
>> -                action = *p++;
>> -                signal = 0;
>> -                if (action == 'C' || action == 'S') {
>> -                    signal = gdb_signal_to_target(strtoul(p, (char **)&p, 16));
>> -                    if (signal == -1) {
>> -                        signal = 0;
>> -                    }
>> -                } else if (action != 'c' && action != 's') {
>> -                    res = 0;
>> -                    break;
>> -                }
>> -                thread = 0;
>> -                if (*p == ':') {
>> -                    thread = strtoull(p+1, (char **)&p, 16);
>> -                }
>> -                action = tolower(action);
>> -                if (res == 0 || (res == 'c' && action == 's')) {
>> -                    res = action;
>> -                    res_signal = signal;
>> -                    res_thread = thread;
>> -                }
>> -            }
>> +
>> +            res = gdb_handle_vcont(s, p);
>> +
>>              if (res) {
>> -                if (res_thread != -1 && res_thread != 0) {
>> -                    cpu = find_cpu(res_thread);
>> -                    if (cpu == NULL) {
>> -                        put_packet(s, "E22");
>> -                        break;
>> -                    }
>> -                    s->c_cpu = cpu;
>> -                }
>> -                if (res == 's') {
>> -                    cpu_single_step(s->c_cpu, sstep_flags);
>> +                if ((res == -EINVAL) || (res == -ERANGE)) {
>> +                    put_packet(s, "E22");
>> +                    break;
>>                  }
>> -                s->signal = res_signal;
>> -                gdb_continue(s);
>> -                return RS_IDLE;
>> +                goto unknown_command;
>>              }
>>              break;
>>          } else {
>>
> 
> Seems like no one is doing guest debugging with kvm on x86 except me,
> and I'm only doing it too infrequently now: This one broke that use case
> for SMP guests long ago. How was it tested?
> 
> To reproduce the bug: set up an x86-64 guest kernel with > 1 core, break
> on some prominent syscall entry (e.g. sys_execve), continue the guest on
> hit and it will quickly lock up, even after disabling the breakpoint
> again. Kernel version doesn't matter (was my first guess), gdb is
> 7.7.50.20140604-cvs (OpenSUSE) here.
> 

I case it helps:

GNU gdb (GDB) 7.7.50.20140604-cvs
Copyright (C) 2014 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from vmlinux...done.
(gdb) tar rem :1234
Remote debugging using :1234
0xffffffff81719d51 in native_safe_halt () at ../arch/x86/include/asm/irqflags.h:54
warning: Source file is more recent than executable.
54              asm volatile("hlt": : :"memory");
(gdb) c
Continuing.
^C
Program received signal SIGINT, Interrupt.
0xffffffff81719d51 in native_safe_halt () at ../arch/x86/include/asm/irqflags.h:54
54              asm volatile("hlt": : :"memory");
(gdb) set debug remote 1
(gdb) b sys_execve
Sending packet: $Hg4#e3...Ack
Packet received: OK
Sending packet: $g#67...Ack
Packet received: c023ef390088ffff000000000000000044be080000c9ffff00f70000000000000100000000000000c023ef390088ffff0000000000000000c8be080000c9ffff0100000000000000010000000000000010be080000c9ffff089a0082ffffffffc023ef390088ffffc023ef390088ffff0300000000000000c023ef390088ffff519d7181ffffffff0602000010000000180000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f03000000000000000000000000000000000000000000000000000000000000000000000000000000000000ff000000ffffffffffffffffffffffff00ffffff70617273655f616e645f657865637574414d4500000000000000000000000000000000000000000031000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000801f0000
Sending packet: $mffffffff811a3800,40#f3...Ack
Packet received: 5b5d415ce9aa65ebffe8a2e265004883ec10488974240848891424e84c620000488b14244889c7488b742408e8cdfdffff48984883c410c3e873e26500554489
Sending packet: $mffffffff811a3809,1#c9...Ack
Packet received: e8
Breakpoint 1 at 0xffffffff811a3809: file ../fs/exec.c, line 1923.
(gdb) c
Continuing.
Sending packet: $Z0,ffffffff811a3809,1#12...Ack
Packet received: OK
Packet Z0 (software-breakpoint) is supported
Sending packet: $vCont;c#a8...Ack
Packet received: T05thread:01;
Sending packet: $g#67...Ack
Packet received: 3b0000000000000058bfc00000c9ffff60f1d015ff7f0000e0f073000000000090b673000000000090436b00000000003b0000000000000030bfc00000c9ffff40bd73000000000060b37300000000000000000000000000000000000000000000a852370088ffff00000000000000000000000000000000000000000000000009381a81ffffffff0202000010000000180000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f03000000000000000000000000000000000000000000000000000000000000ff000000000000000000000000ffffff00000000000000000000000000000000ff000000000000000000000000ffffff000000000000000000ff00ffffffffff000000000000000031000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000801f0000
Sending packet: $Hg4#e3...Ack
Packet received: OK
Sending packet: $g#67...Ack
Packet received: c023ef390088ffff000000000000000001000000000000004013da3f0088ffff0100000000000000c023ef390088ffff0000000000000000c8be080000c9ffff0100000000000000010000000000000080bd080000c9ffff88af0383ffffffffc023ef390088ffffc023ef390088ffff0300000000000000c023ef390088ffff519d7181ffffffff0602000010000000180000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f0300000001000000000000000000000000000000000000000000000000000000000000000000000000ffffffffffff000000000000000000000000000000000000000000000000000000000000000000000000ff00000000000000ff00000074726163743d2f646275732d7666732d00000000008064400000000000000000fefdfdfdfdfded3f0000000000000000000000000000f03f0000000000000000000000000000944000000000000000000000000000008e400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000a01f0000
Sending packet: $z0,ffffffff811a3809,1#32...Ack
Packet received: OK

Sending packet: $Hg1#e0...Ack
Packet received: OK
Sending packet: $mffffc90000c0bf30,8#83...Ack
Packet received: ae180081ffffffff
Sending packet: $mffffc90000c0bf30,8#83...Ack
Packet received: ae180081ffffffff
Breakpoint 1, SyS_execve (filename=7029648, argv=7583376, envp=7598304) at ../fs/exec.c:1923
warning: Source file is more recent than executable.
(gdb) c
Continuing.
Sending packet: $vCont;s:1#23...Ack
Packet received: T05thread:01;
Sending packet: $g#67...Ack
Packet received: 0020000000c9ffff46000000000000000c3fc03f0088ffff8287ac5600000000f1a60381fffffffff1a60381ffffffff000000006622ab56383fc03f0088ffff01000000000000000100000000000000d83ec03f0088ffff88af0383ffffffff18bec13f0088ffff98bec13f0088ffff18bfc13f0088ffff0a180000000000004da70381ffffffff4600000010000000180000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f03000000000000000000000000000000000000000000000000000000000000ff000000000000000000000000ffffff00000000000000000000000000000000ff000000000000000000000000ffffff000000000000000000ff00ffffffffff000000000000000031000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000801f0000
Sending packet: $Z0,ffffffff811a3809,1#12...Ack
Packet received: OK
Sending packet: $vCont;c#a8...Ack
Packet received: T05thread:01;
Sending packet: $g#67...Ack
Packet received: 3b0000000000000058bfc00000c9ffff60f1d015ff7f0000e0f073000000000090b673000000000090436b00000000003b0000000000000030bfc00000c9ffff40bd73000000000060b37300000000000000000000000000000000000000000000a852370088ffff00000000000000000000000000000000000000000000000009381a81ffffffff0203000010000000180000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f03000000000000000000000000000000000000000000000000000000000000ff000000000000000000000000ffffff00000000000000000000000000000000ff000000000000000000000000ffffff000000000000000000ff00ffffffffff000000000000000031000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000801f0000
Sending packet: $Hg4#e3...Ack
Packet received: OK
Sending packet: $g#67...Ack
Packet received: c023ef390088ffff000000000000000001000000000000004013da3f0088ffff0100000000000000c023ef390088ffff0000000000000000c8be080000c9ffff0100000000000000010000000000000080bd080000c9ffff88af0383ffffffffc023ef390088ffffc023ef390088ffff0300000000000000c023ef390088ffff519d7181ffffffff0602000010000000180000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f0300000001000000000000000000000000000000000000000000000000000000000000000000000000ffffffffffff000000000000000000000000000000000000000000000000000000000000000000000000ff00000000000000ff00000074726163743d2f646275732d7666732d00000000008064400000000000000000fefdfdfdfdfded3f0000000000000000000000000000f03f0000000000000000000000000000944000000000000000000000000000008e400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000a01f0000
Sending packet: $z0,ffffffff811a3809,1#32...Ack
Packet received: OK

Sending packet: $Hg1#e0...Ack
Packet received: OK
Sending packet: $mffffc90000c0bf30,8#83...Ack
Packet received: ae180081ffffffff
Sending packet: $mffffc90000c0bf30,8#83...Ack
Packet received: ae180081ffffffff
Breakpoint 1, SyS_execve (filename=7029648, argv=7583376, envp=7598304) at ../fs/exec.c:1923
(gdb) c
Continuing.
Sending packet: $vCont;s:1#23...Ack

...and now the guest is dead. I can still interrupt it, but it's
otherwise not working properly.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PULL 09/23] gdbstub: Fix vCont behaviour

[Alex Bennée]

Jan Kiszka <jan.kiszka@web.de> writes:

> On 2018-02-17 09:56, Jan Kiszka wrote:
>> On 2017-02-16 15:31, Paolo Bonzini wrote:
>>> From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>>>
>>> When GDB issues a "vCont", QEMU was not handling it correctly when
>>> multiple VCPUs are active.
>>> For vCont, for each thread (VCPU), it can be specified whether to
>>> single step, continue or stop that thread. The default is to stop a
>>> thread.
>>> However, when (for example) "vCont;s:2" is issued, all VCPUs continue
>>> to run, although all but VCPU nr 2 are to be stopped.
>>>
>>> This patch completely rewrites the vCont parsing code.
>>>
>>> Please note that this improvement only works in system emulation mode,
>>> when in userspace emulation mode the old behaviour is preserved.
>>>
>>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>>> Message-Id: <1487092068-16562-3-git-send-email-imbrenda@linux.vnet.ibm.com>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>  gdbstub.c | 209 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>>>  1 file changed, 162 insertions(+), 47 deletions(-)
>>>
<snip>
>>
>> Seems like no one is doing guest debugging with kvm on x86 except me,
>> and I'm only doing it too infrequently now: This one broke that use case
>> for SMP guests long ago. How was it tested?
>>
>> To reproduce the bug: set up an x86-64 guest kernel with > 1 core, break
>> on some prominent syscall entry (e.g. sys_execve), continue the guest on
>> hit and it will quickly lock up, even after disabling the breakpoint
>> again. Kernel version doesn't matter (was my first guess), gdb is
>> 7.7.50.20140604-cvs (OpenSUSE) here.

I thought I fixed this with 5a6a1ad181c658b810041d852b290ac836965aca

FWIW I do periodically test ARM TCG and KVM guest debug using:

  tests/guest-debug/test-gdbstub.py

But we are missing a nice integration to get an appropriate guest image
to automate this process. If we can fix that we should be able to turn
on the test as part of make check.


--
Alex Bennée

Re: [Qemu-devel] [PULL 09/23] gdbstub: Fix vCont behaviour

[Jan Kiszka]

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

On 2018-02-17 14:27, Alex Bennée wrote:
> 
> Jan Kiszka <jan.kiszka@web.de> writes:
> 
>> On 2018-02-17 09:56, Jan Kiszka wrote:
>>> On 2017-02-16 15:31, Paolo Bonzini wrote:
>>>> From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>>>>
>>>> When GDB issues a "vCont", QEMU was not handling it correctly when
>>>> multiple VCPUs are active.
>>>> For vCont, for each thread (VCPU), it can be specified whether to
>>>> single step, continue or stop that thread. The default is to stop a
>>>> thread.
>>>> However, when (for example) "vCont;s:2" is issued, all VCPUs continue
>>>> to run, although all but VCPU nr 2 are to be stopped.
>>>>
>>>> This patch completely rewrites the vCont parsing code.
>>>>
>>>> Please note that this improvement only works in system emulation mode,
>>>> when in userspace emulation mode the old behaviour is preserved.
>>>>
>>>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>>>> Message-Id: <1487092068-16562-3-git-send-email-imbrenda@linux.vnet.ibm.com>
>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> ---
>>>>  gdbstub.c | 209 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>>>>  1 file changed, 162 insertions(+), 47 deletions(-)
>>>>
> <snip>
>>>
>>> Seems like no one is doing guest debugging with kvm on x86 except me,
>>> and I'm only doing it too infrequently now: This one broke that use case
>>> for SMP guests long ago. How was it tested?
>>>
>>> To reproduce the bug: set up an x86-64 guest kernel with > 1 core, break
>>> on some prominent syscall entry (e.g. sys_execve), continue the guest on
>>> hit and it will quickly lock up, even after disabling the breakpoint
>>> again. Kernel version doesn't matter (was my first guess), gdb is
>>> 7.7.50.20140604-cvs (OpenSUSE) here.
> 
> I thought I fixed this with 5a6a1ad181c658b810041d852b290ac836965aca
> 
> FWIW I do periodically test ARM TCG and KVM guest debug using:
> 
>   tests/guest-debug/test-gdbstub.py
> 
> But we are missing a nice integration to get an appropriate guest image
> to automate this process. If we can fix that we should be able to turn
> on the test as part of make check.
> 

If that test above is extended with more interesting setups, that should
be enough. E.g., you can reproduce this issue by running qemu with -smp
4 and the following test modifications.

diff --git a/tests/guest-debug/test-gdbstub.py b/tests/guest-debug/test-gdbstub.py
index 31ba6c943a..a55782fa9a 100644
--- a/tests/guest-debug/test-gdbstub.py
+++ b/tests/guest-debug/test-gdbstub.py
@@ -15,6 +15,7 @@ def report(cond, msg):
         print ("PASS: %s" % (msg))
     else:
         print ("FAIL: %s" % (msg))
+        global failcount
         failcount += 1
 
 
@@ -33,6 +34,7 @@ def check_break(sym_name):
     bp = gdb.Breakpoint(sym_name)
 
     gdb.execute("c")
+    gdb.execute("c 100")
 
     # hopefully we came back
     end_pc = gdb.parse_and_eval('$pc')
@@ -138,12 +140,12 @@ def run_test():
     # Can't set this up until we are in the kernel proper
     # if we make it to run_init_process we've over-run and
     # one of the tests failed
-    print ("Setup catch-all for run_init_process")
-    cbp = CatchBreakpoint("run_init_process")
-    cpb2 = CatchBreakpoint("try_to_run_init_process")
+    #print ("Setup catch-all for run_init_process")
+    #cbp = CatchBreakpoint("run_init_process")
+    #cpb2 = CatchBreakpoint("try_to_run_init_process")
 
     print ("Checking Normal breakpoint works")
-    break_ok = check_break("wait_for_completion")
+    break_ok = check_break("SyS_execve")
     report(break_ok, "break @ wait_for_completion")
 
     print ("Checking watchpoint works")


With -smp 1, check_break succeeds.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PULL 09/23] gdbstub: Fix vCont behaviour

[Claudio Imbrenda]

On Sat, 17 Feb 2018 10:07:38 +0100
Jan Kiszka <jan.kiszka@web.de> wrote:

[...]

> > Seems like no one is doing guest debugging with kvm on x86 except
> > me, and I'm only doing it too infrequently now: This one broke that
> > use case for SMP guests long ago. How was it tested?
> > 
> > To reproduce the bug: set up an x86-64 guest kernel with > 1 core,
> > break on some prominent syscall entry (e.g. sys_execve), continue
> > the guest on hit and it will quickly lock up, even after disabling
> > the breakpoint again. Kernel version doesn't matter (was my first
> > guess), gdb is 7.7.50.20140604-cvs (OpenSUSE) here.

[...]

> Sending packet: $Hg1#e0...Ack
> Packet received: OK
> Sending packet: $mffffc90000c0bf30,8#83...Ack
> Packet received: ae180081ffffffff
> Sending packet: $mffffc90000c0bf30,8#83...Ack
> Packet received: ae180081ffffffff
> Breakpoint 1, SyS_execve (filename=7029648, argv=7583376,
> envp=7598304) at ../fs/exec.c:1923 (gdb) c
> Continuing.
> Sending packet: $vCont;s:1#23...Ack
> 
> ...and now the guest is dead. I can still interrupt it, but it's
> otherwise not working properly.

I tried, but I could not reproduce this bug neither on s390x nor on
amd64. in both cases I used recent enough versions of qemu so that
they have the patch you mentioned, and qemu was started with several
cpus (I tried 64 on s390x, and 4 on amd64).

in particular on amd64:
host: 4.4.0-112-generic (ubuntu 16.04)
QEMU version 2.10.0  (vanilla from git)
gdb (Ubuntu 7.11.1-0ubuntu1~16.5) 7.11.1

I also used the gdb script you provided but everything worked both with
-smp 1 and with -smp 4

the only issue I had was that I had to disable kaslr in the guest to be
able to do anything, but that does not seem to be the problem you have.

my primary suspicion at this point would be an issue in KVM, and not
in qemu. have you tried running it without KVM? what is the version of
qemu and kernel in the host?


Claudio

Re: [Qemu-devel] [PULL 09/23] gdbstub: Fix vCont behaviour

[Jan Kiszka]

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

On 2018-02-19 19:15, Claudio Imbrenda wrote:
> On Sat, 17 Feb 2018 10:07:38 +0100
> Jan Kiszka <jan.kiszka@web.de> wrote:
> 
> [...]
> 
>>> Seems like no one is doing guest debugging with kvm on x86 except
>>> me, and I'm only doing it too infrequently now: This one broke that
>>> use case for SMP guests long ago. How was it tested?
>>>
>>> To reproduce the bug: set up an x86-64 guest kernel with > 1 core,
>>> break on some prominent syscall entry (e.g. sys_execve), continue
>>> the guest on hit and it will quickly lock up, even after disabling
>>> the breakpoint again. Kernel version doesn't matter (was my first
>>> guess), gdb is 7.7.50.20140604-cvs (OpenSUSE) here.
> 
> [...]
> 
>> Sending packet: $Hg1#e0...Ack
>> Packet received: OK
>> Sending packet: $mffffc90000c0bf30,8#83...Ack
>> Packet received: ae180081ffffffff
>> Sending packet: $mffffc90000c0bf30,8#83...Ack
>> Packet received: ae180081ffffffff
>> Breakpoint 1, SyS_execve (filename=7029648, argv=7583376,
>> envp=7598304) at ../fs/exec.c:1923 (gdb) c
>> Continuing.
>> Sending packet: $vCont;s:1#23...Ack
>>
>> ...and now the guest is dead. I can still interrupt it, but it's
>> otherwise not working properly.
> 
> I tried, but I could not reproduce this bug neither on s390x nor on
> amd64. in both cases I used recent enough versions of qemu so that
> they have the patch you mentioned, and qemu was started with several
> cpus (I tried 64 on s390x, and 4 on amd64).
> 
> in particular on amd64:
> host: 4.4.0-112-generic (ubuntu 16.04)
> QEMU version 2.10.0  (vanilla from git)
> gdb (Ubuntu 7.11.1-0ubuntu1~16.5) 7.11.1
> 
> I also used the gdb script you provided but everything worked both with
> -smp 1 and with -smp 4
> 
> the only issue I had was that I had to disable kaslr in the guest to be
> able to do anything, but that does not seem to be the problem you have.
> 
> my primary suspicion at this point would be an issue in KVM, and not
> in qemu. have you tried running it without KVM? what is the version of
> qemu and kernel in the host?

Yes, debugging works without KVM. But that is a completely different
setup for various reasons (no parallelism, no guest-visible
modifications by soft-breakpoints and continuations). But I would not
exclude that the issue is caused by KVM, and your patch just surfaced it.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]