[Qemu-devel] [PULL 00/11] Miscellaneous patches for QEMU 2.11-rc2

[Qemu-devel] [PULL 00/11] Miscellaneous patches for QEMU 2.11-rc2

[Paolo Bonzini]

The following changes since commit d24aaf2a2915424962fb101142f28fa4307f4740:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2017-11-06 11:24:14 +0000)

are available in the Git repository at:

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

for you to fetch changes up to 4950b1a766a16bd3feef4c9471b9794e6fe0e1b2:

  fix scripts/update-linux-headers.sh here document (2017-11-15 16:27:01 +0100)

----------------------------------------------------------------
Miscellaneous bugfixes

----------------------------------------------------------------
Dariusz Stojaczyk (1):
      vhost-user-scsi: add missing virtqueue_size param

Dr. David Alan Gilbert (1):
      ioapic/tracing: Remove last DPRINTFs

Emilio G. Cota (1):
      thread-posix: fix qemu_rec_mutex_trylock macro

Gerd Hoffmann (1):
      fix scripts/update-linux-headers.sh here document

Max Reitz (1):
      util/stats64: Fix min/max comparisons

Mike Nawrocki (1):
      Enable 8-byte wide MMIO for 16550 serial devices

Paolo Bonzini (1):
      exec: Do not resolve subpage in mru_section

Pavel Dovgalyuk (2):
      cpu-exec: don't overwrite exception_index
      cpu-exec: avoid cpu_exec_nocache infinite loop with record/replay

Philippe Mathieu-Daudé (1):
      Makefile: simpler/faster "make help"

Wanpeng Li (1):
      target-i386: adds PV_TLB_FLUSH CPUID feature bit

 Makefile                        |  2 +-
 accel/tcg/cpu-exec.c            | 99 ++++++++++++++++++++++++-----------------
 exec.c                          | 12 ++---
 hw/char/serial.c                |  8 +++-
 hw/intc/ioapic.c                | 17 ++-----
 hw/intc/trace-events            |  5 ++-
 hw/scsi/vhost-user-scsi.c       |  2 +
 include/qemu/thread-posix.h     |  2 +-
 scripts/update-linux-headers.sh |  2 +-
 target/i386/cpu.c               |  2 +-
 util/stats64.c                  |  4 +-
 11 files changed, 81 insertions(+), 74 deletions(-)
-- 
2.14.3

[Qemu-devel] [PULL 01/11] Enable 8-byte wide MMIO for 16550 serial devices

[Paolo Bonzini]

From: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu>

Some drivers for the PPMC7400 PowerPC evaluation board accesses the
serial registers through the floating point unit (stfd/ldfd), which is
an 8-byte wide access. This patch enables that behavior.

Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu>
Message-Id: <20171106161039.32596-1-michael.nawrocki@gtri.gatech.edu>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/char/serial.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index 376bd2f240..eb72191ee7 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -1005,7 +1005,7 @@ static void serial_mm_write(void *opaque, hwaddr addr,
                             uint64_t value, unsigned size)
 {
     SerialState *s = opaque;
-    value &= ~0u >> (32 - (size * 8));
+    value &= 255;
     serial_ioport_write(s, addr >> s->it_shift, value, 1);
 }
 
@@ -1014,16 +1014,22 @@ static const MemoryRegionOps serial_mm_ops[3] = {
         .read = serial_mm_read,
         .write = serial_mm_write,
         .endianness = DEVICE_NATIVE_ENDIAN,
+        .valid.max_access_size = 8,
+        .impl.max_access_size = 8,
     },
     [DEVICE_LITTLE_ENDIAN] = {
         .read = serial_mm_read,
         .write = serial_mm_write,
         .endianness = DEVICE_LITTLE_ENDIAN,
+        .valid.max_access_size = 8,
+        .impl.max_access_size = 8,
     },
     [DEVICE_BIG_ENDIAN] = {
         .read = serial_mm_read,
         .write = serial_mm_write,
         .endianness = DEVICE_BIG_ENDIAN,
+        .valid.max_access_size = 8,
+        .impl.max_access_size = 8,
     },
 };
 
-- 
2.14.3

[Qemu-devel] [PULL 02/11] ioapic/tracing: Remove last DPRINTFs

[Paolo Bonzini]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Remove the last few DPRINTFs from hw/intc/ioapic.c and turn
them into tracing.  In one case it's a new trace, in the others
it's just adding a parameter to the existing traces.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20171102180310.24760-1-dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/intc/ioapic.c     | 17 +++--------------
 hw/intc/trace-events |  5 +++--
 2 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 37c4386ae3..36139a4db6 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -35,15 +35,6 @@
 #include "hw/i386/x86-iommu.h"
 #include "trace.h"
 
-//#define DEBUG_IOAPIC
-
-#ifdef DEBUG_IOAPIC
-#define DPRINTF(fmt, ...)                                       \
-    do { printf("ioapic: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...)
-#endif
-
 #define APIC_DELIVERY_MODE_SHIFT 8
 #define APIC_POLARITY_SHIFT 14
 #define APIC_TRIG_MODE_SHIFT 15
@@ -157,7 +148,7 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
      * to GSI 2.  GSI maps to ioapic 1-1.  This is not
      * the cleanest way of doing it but it should work. */
 
-    DPRINTF("%s: %s vec %x\n", __func__, level ? "raise" : "lower", vector);
+    trace_ioapic_set_irq(vector, level);
     if (vector == 0) {
         vector = 2;
     }
@@ -290,11 +281,10 @@ ioapic_mem_read(void *opaque, hwaddr addr, unsigned int size)
                 }
             }
         }
-        DPRINTF("read: %08x = %08x\n", s->ioregsel, val);
         break;
     }
 
-    trace_ioapic_mem_read(addr, size, val);
+    trace_ioapic_mem_read(addr, s->ioregsel, size, val);
 
     return val;
 }
@@ -335,7 +325,7 @@ ioapic_mem_write(void *opaque, hwaddr addr, uint64_t val,
     int index;
 
     addr &= 0xff;
-    trace_ioapic_mem_write(addr, size, val);
+    trace_ioapic_mem_write(addr, s->ioregsel, size, val);
 
     switch (addr) {
     case IOAPIC_IOREGSEL:
@@ -345,7 +335,6 @@ ioapic_mem_write(void *opaque, hwaddr addr, uint64_t val,
         if (size != 4) {
             break;
         }
-        DPRINTF("write: %08x = %08" PRIx64 "\n", s->ioregsel, val);
         switch (s->ioregsel) {
         case IOAPIC_REG_ID:
             s->id = (val >> IOAPIC_ID_SHIFT) & IOAPIC_ID_MASK;
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index b86f242b0f..b298fac7c6 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -18,8 +18,9 @@ apic_mem_writel(uint64_t addr, uint32_t val) "0x%"PRIx64" = 0x%08x"
 ioapic_set_remote_irr(int n) "set remote irr for pin %d"
 ioapic_clear_remote_irr(int n, int vector) "clear remote irr for pin %d vector %d"
 ioapic_eoi_broadcast(int vector) "EOI broadcast for vector %d"
-ioapic_mem_read(uint8_t addr, uint8_t size, uint32_t val) "ioapic mem read addr 0x%"PRIx8" size 0x%"PRIx8" retval 0x%"PRIx32
-ioapic_mem_write(uint8_t addr, uint8_t size, uint32_t val) "ioapic mem write addr 0x%"PRIx8" size 0x%"PRIx8" val 0x%"PRIx32
+ioapic_mem_read(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapic mem read addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8" retval 0x%"PRIx32
+ioapic_mem_write(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapic mem write addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8" val 0x%"PRIx32
+ioapic_set_irq(int vector, int level) "vector: %d level: %d"
 
 # hw/intc/slavio_intctl.c
 slavio_intctl_mem_readl(uint32_t cpu, uint64_t addr, uint32_t ret) "read cpu %d reg 0x%"PRIx64" = 0x%x"
-- 
2.14.3

[Qemu-devel] [PULL 03/11] Makefile: simpler/faster "make help"

[Paolo Bonzini]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Using obscure black magic introduced in eaa2ddbb767 :)

In an out-of-tree directory, running "../configure && make help" will generate
some required files (.mak), then clone some submodules, compile at least
the capstone submodule, generate QMP and Trace files, and finally display
the help.

On an outdated computer (Sun Blade workstation), running "make help" took
more than 5h :) With this patch it took roughly 37min.

Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20171108032052.20029-1-f4bug@amsat.org>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index ec73acfa9a..143ac81736 100644
--- a/Makefile
+++ b/Makefile
@@ -6,7 +6,7 @@ BUILD_DIR=$(CURDIR)
 # Before including a proper config-host.mak, assume we are in the source tree
 SRC_PATH=.
 
-UNCHECKED_GOALS := %clean TAGS cscope ctags docker docker-%
+UNCHECKED_GOALS := %clean TAGS cscope ctags docker docker-% help
 
 # All following code might depend on configuration variables
 ifneq ($(wildcard config-host.mak),)
-- 
2.14.3

[Qemu-devel] [PULL 04/11] thread-posix: fix qemu_rec_mutex_trylock macro

[Paolo Bonzini]

From: "Emilio G. Cota" <cota@braap.org>

We never noticed because it has no users.

Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-Id: <1510273811-13419-1-git-send-email-cota@braap.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/thread-posix.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
index f4296d31c4..f3f47e426f 100644
--- a/include/qemu/thread-posix.h
+++ b/include/qemu/thread-posix.h
@@ -7,7 +7,7 @@
 typedef QemuMutex QemuRecMutex;
 #define qemu_rec_mutex_destroy qemu_mutex_destroy
 #define qemu_rec_mutex_lock qemu_mutex_lock
-#define qemu_rec_mutex_try_lock qemu_mutex_try_lock
+#define qemu_rec_mutex_trylock qemu_mutex_trylock
 #define qemu_rec_mutex_unlock qemu_mutex_unlock
 
 struct QemuMutex {
-- 
2.14.3

[Qemu-devel] [PULL 05/11] target-i386: adds PV_TLB_FLUSH CPUID feature bit

[Paolo Bonzini]

From: Wanpeng Li <wanpeng.li@hotmail.com>

Adds PV_TLB_FLUSH CPUID feature bit.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim KrÄmář <rkrcmar@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
Message-Id: <1510299947-11287-1-git-send-email-wanpeng.li@hotmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6f21a5e518..ecebc5a70a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -347,7 +347,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
         .feat_names = {
             "kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock",
             "kvm-asyncpf", "kvm-steal-time", "kvm-pv-eoi", "kvm-pv-unhalt",
-            NULL, NULL, NULL, NULL,
+            NULL, "kvm-pv-tlb-flush", NULL, NULL,
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
-- 
2.14.3

[Qemu-devel] [PULL 06/11] vhost-user-scsi: add missing virtqueue_size param

[Paolo Bonzini]

From: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>

Commit 5c0919d0 [1] introduced virtqueue_size parameter
for common virtio-scsi path, without updaing the vhost-user-scsi
code. vhost-user-scsi devices right now report size 0 for each vq.

This patch introduces virtqueue_size param to vhost-user-scsi,
that can now be set by the user. However, the most importantly, it
now has a default value of 128 (same as QEMU's virtio-scsi).

[1] 5c0919d0 ("virtio-scsi: Add virtqueue_size parameter
allowing virtqueue size to be set.")

Change-Id: I70e87eab702ebf1196c028dbf17d54fdc0c89a14
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Message-Id: <1510676916-76409-1-git-send-email-dariuszx.stojaczyk@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/vhost-user-scsi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 500fa6a067..f7561e23fa 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -135,6 +135,8 @@ static Property vhost_user_scsi_properties[] = {
     DEFINE_PROP_CHR("chardev", VirtIOSCSICommon, conf.chardev),
     DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, 0),
     DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, 1),
+    DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, conf.virtqueue_size,
+                       128),
     DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, conf.max_sectors,
                        0xFFFF),
     DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, conf.cmd_per_lun, 128),
-- 
2.14.3

[Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Paolo Bonzini]

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

This patch adds a condition before overwriting exception_index fiels.
It is needed when exception_index is already set to some meaningful value.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>

Message-Id: <20171114081812.27640.26372.stgit@pasha-VirtualBox>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/tcg/cpu-exec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 61297f8f4a..0473055a08 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -594,7 +594,9 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
     if (unlikely(atomic_read(&cpu->exit_request)
         || (use_icount && cpu->icount_decr.u16.low + cpu->icount_extra == 0))) {
         atomic_set(&cpu->exit_request, 0);
-        cpu->exception_index = EXCP_INTERRUPT;
+        if (cpu->exception_index == -1) {
+            cpu->exception_index = EXCP_INTERRUPT;
+        }
         return true;
     }
 
-- 
2.14.3

[Qemu-devel] [PULL 08/11] cpu-exec: avoid cpu_exec_nocache infinite loop with record/replay

[Paolo Bonzini]

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

This patch ensures that icount_decr.u32.high is clear before calling
cpu_exec_nocache when exception is pending.  Because the exception is
caused by the first instruction in the block and it cannot be executed
without resetting the flag.

There are two parts in the fix.  First, clear icount_decr.u32.high in
cpu_handle_interrupt (just before processing the "dependent" request,
stored in cpu->interrupt_request or cpu->exit_request) rather than
cpu_loop_exec_tb; this ensures that cpu_handle_exception is always
reached with zero icount_decr.u32.high unless another interrupt has
happened in the meanwhile.

Second, try to cause the exception at the beginning of
cpu_handle_exception, and exit immediately if the TB cannot
execute.  With this change, interrupts are processed and
cpu_exec_nocache can make process.

Signed-off-by: Maria Klimushenkova <maria.klimushenkova@ispras.ru>
Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
Message-Id: <20171114081818.27640.33165.stgit@pasha-VirtualBox>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/tcg/cpu-exec.c | 95 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 54 insertions(+), 41 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 0473055a08..f3de96f346 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -470,48 +470,51 @@ static inline void cpu_handle_debug_exception(CPUState *cpu)
 
 static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
 {
-    if (cpu->exception_index >= 0) {
-        if (cpu->exception_index >= EXCP_INTERRUPT) {
-            /* exit request from the cpu execution loop */
-            *ret = cpu->exception_index;
-            if (*ret == EXCP_DEBUG) {
-                cpu_handle_debug_exception(cpu);
-            }
-            cpu->exception_index = -1;
-            return true;
-        } else {
+    if (cpu->exception_index < 0) {
+#ifndef CONFIG_USER_ONLY
+        if (replay_has_exception()
+               && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
+            /* try to cause an exception pending in the log */
+            cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0, curr_cflags()), true);
+        }
+#endif
+        if (cpu->exception_index < 0) {
+            return false;
+        }
+    }
+
+    if (cpu->exception_index >= EXCP_INTERRUPT) {
+        /* exit request from the cpu execution loop */
+        *ret = cpu->exception_index;
+        if (*ret == EXCP_DEBUG) {
+            cpu_handle_debug_exception(cpu);
+        }
+        cpu->exception_index = -1;
+        return true;
+    } else {
 #if defined(CONFIG_USER_ONLY)
-            /* if user mode only, we simulate a fake exception
-               which will be handled outside the cpu execution
-               loop */
+        /* if user mode only, we simulate a fake exception
+           which will be handled outside the cpu execution
+           loop */
 #if defined(TARGET_I386)
+        CPUClass *cc = CPU_GET_CLASS(cpu);
+        cc->do_interrupt(cpu);
+#endif
+        *ret = cpu->exception_index;
+        cpu->exception_index = -1;
+        return true;
+#else
+        if (replay_exception()) {
             CPUClass *cc = CPU_GET_CLASS(cpu);
+            qemu_mutex_lock_iothread();
             cc->do_interrupt(cpu);
-#endif
-            *ret = cpu->exception_index;
+            qemu_mutex_unlock_iothread();
             cpu->exception_index = -1;
+        } else if (!replay_has_interrupt()) {
+            /* give a chance to iothread in replay mode */
+            *ret = EXCP_INTERRUPT;
             return true;
-#else
-            if (replay_exception()) {
-                CPUClass *cc = CPU_GET_CLASS(cpu);
-                qemu_mutex_lock_iothread();
-                cc->do_interrupt(cpu);
-                qemu_mutex_unlock_iothread();
-                cpu->exception_index = -1;
-            } else if (!replay_has_interrupt()) {
-                /* give a chance to iothread in replay mode */
-                *ret = EXCP_INTERRUPT;
-                return true;
-            }
-#endif
         }
-#ifndef CONFIG_USER_ONLY
-    } else if (replay_has_exception()
-               && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
-        /* try to cause an exception pending in the log */
-        cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0, curr_cflags()), true);
-        *ret = -1;
-        return true;
 #endif
     }
 
@@ -522,6 +525,19 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
                                         TranslationBlock **last_tb)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
+    int32_t insns_left;
+
+    /* Clear the interrupt flag now since we're processing
+     * cpu->interrupt_request and cpu->exit_request.
+     */
+    insns_left = atomic_read(&cpu->icount_decr.u32);
+    atomic_set(&cpu->icount_decr.u16.high, 0);
+    if (unlikely(insns_left < 0)) {
+        /* Ensure the zeroing of icount_decr comes before the next read
+         * of cpu->exit_request or cpu->interrupt_request.
+         */
+        smp_mb();
+    }
 
     if (unlikely(atomic_read(&cpu->interrupt_request))) {
         int interrupt_request;
@@ -620,17 +636,14 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
 
     *last_tb = NULL;
     insns_left = atomic_read(&cpu->icount_decr.u32);
-    atomic_set(&cpu->icount_decr.u16.high, 0);
     if (insns_left < 0) {
         /* Something asked us to stop executing chained TBs; just
          * continue round the main loop. Whatever requested the exit
          * will also 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 zeroing of icount_decr
-         * comes before the next read of cpu->exit_request
-         * or cpu->interrupt_request.
+         * interrupt_request) which will be handled by
+         * cpu_handle_interrupt.  cpu_handle_interrupt will also
+         * clear cpu->icount_decr.u16.high.
          */
-        smp_mb();
         return;
     }
 
-- 
2.14.3

[Qemu-devel] [PULL 09/11] util/stats64: Fix min/max comparisons

[Paolo Bonzini]

From: Max Reitz <mreitz@redhat.com>

stat64_min_slow() and stat64_max_slow() compare the wrong way.  This
makes iotest 136 fail with clang and -m32.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20171114232223.25207-1-mreitz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 util/stats64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/util/stats64.c b/util/stats64.c
index 9968fcceac..389c365a9e 100644
--- a/util/stats64.c
+++ b/util/stats64.c
@@ -91,7 +91,7 @@ bool stat64_min_slow(Stat64 *s, uint64_t value)
     low = atomic_read(&s->low);
 
     orig = ((uint64_t)high << 32) | low;
-    if (orig < value) {
+    if (value < orig) {
         /* We have to set low before high, just like stat64_min reads
          * high before low.  The value may become higher temporarily, but
          * stat64_get does not notice (it takes the lock) and the only ill
@@ -120,7 +120,7 @@ bool stat64_max_slow(Stat64 *s, uint64_t value)
     low = atomic_read(&s->low);
 
     orig = ((uint64_t)high << 32) | low;
-    if (orig > value) {
+    if (value > orig) {
         /* We have to set low before high, just like stat64_max reads
          * high before low.  The value may become lower temporarily, but
          * stat64_get does not notice (it takes the lock) and the only ill
-- 
2.14.3

[Qemu-devel] [PULL 10/11] exec: Do not resolve subpage in mru_section

[Paolo Bonzini]

This fixes a crash caused by picking the wrong memory region in
address_space_lookup_region seen with client code accessing a device
model that uses alias memory regions.  The expensive part of
address_space_lookup_region anyway is phys_page_find; performance-wise
it is okay to repeat the subsequent subpage lookup.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Message-Id: <20171114225941.072707456B5@zero.eik.bme.hu>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/exec.c b/exec.c
index 97a24a875e..3bb9fcf257 100644
--- a/exec.c
+++ b/exec.c
@@ -410,22 +410,16 @@ static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch *d,
 {
     MemoryRegionSection *section = atomic_read(&d->mru_section);
     subpage_t *subpage;
-    bool update;
 
-    if (section && section != &d->map.sections[PHYS_SECTION_UNASSIGNED] &&
-        section_covers_addr(section, addr)) {
-        update = false;
-    } else {
+    if (!section || section == &d->map.sections[PHYS_SECTION_UNASSIGNED] ||
+        !section_covers_addr(section, addr)) {
         section = phys_page_find(d, addr);
-        update = true;
+        atomic_set(&d->mru_section, section);
     }
     if (resolve_subpage && section->mr->subpage) {
         subpage = container_of(section->mr, subpage_t, iomem);
         section = &d->map.sections[subpage->sub_section[SUBPAGE_IDX(addr)]];
     }
-    if (update) {
-        atomic_set(&d->mru_section, section);
-    }
     return section;
 }
 
-- 
2.14.3

[Qemu-devel] [PULL 11/11] fix scripts/update-linux-headers.sh here document

[Paolo Bonzini]

From: Gerd Hoffmann <kraxel@redhat.com>

The minus sign after << causes the shell to strip only
preceding tabs, not spaces.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-Id: <20171110090354.29608-1-kraxel@redhat.com>
Fixes: 40bf8e9aede0f9105a9e1e4aaf17b20aaa55f9a0
Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/update-linux-headers.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index ad80fe3fca..76fd894a77 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -106,7 +106,7 @@ for arch in $ARCHLIST; do
     if [ $arch = x86 ]; then
         cat <<-EOF >"$output/include/standard-headers/asm-x86/hyperv.h"
         /* this is a temporary placeholder until kvm_para.h stops including it */
-        EOF
+EOF
         cp "$tmpdir/include/asm/unistd_32.h" "$output/linux-headers/asm-x86/"
         cp "$tmpdir/include/asm/unistd_x32.h" "$output/linux-headers/asm-x86/"
         cp "$tmpdir/include/asm/unistd_64.h" "$output/linux-headers/asm-x86/"
-- 
2.14.3

Re: [Qemu-devel] [PULL 00/11] Miscellaneous patches for QEMU 2.11-rc2

[Peter Maydell]

On 16 November 2017 at 11:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit d24aaf2a2915424962fb101142f28fa4307f4740:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2017-11-06 11:24:14 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 4950b1a766a16bd3feef4c9471b9794e6fe0e1b2:
>
>   fix scripts/update-linux-headers.sh here document (2017-11-15 16:27:01 +0100)
>
> ----------------------------------------------------------------
> Miscellaneous bugfixes
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Peter Maydell]

On 16 November 2017 at 11:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>
> This patch adds a condition before overwriting exception_index fiels.
> It is needed when exception_index is already set to some meaningful value.
>
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
>
> Message-Id: <20171114081812.27640.26372.stgit@pasha-VirtualBox>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  accel/tcg/cpu-exec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 61297f8f4a..0473055a08 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -594,7 +594,9 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
>      if (unlikely(atomic_read(&cpu->exit_request)
>          || (use_icount && cpu->icount_decr.u16.low + cpu->icount_extra == 0))) {
>          atomic_set(&cpu->exit_request, 0);
> -        cpu->exception_index = EXCP_INTERRUPT;
> +        if (cpu->exception_index == -1) {
> +            cpu->exception_index = EXCP_INTERRUPT;
> +        }
>          return true;
>      }

Hi. This commit breaks booting of Debian on aarch64 virt board.
(repro instructions for creating the image available at:
https://translatedcode.wordpress.com/2017/07/24/installing-debian-on-qemus-64-bit-arm-virt-board/)
The guest kernel never prints anything to the serial port.

Reverting this commit fixes master for me, so I plan to do
that on Monday.

thanks
-- PMM

Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Paolo Bonzini]

On 17/11/2017 21:07, Peter Maydell wrote:
> On 16 November 2017 at 11:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>
>> This patch adds a condition before overwriting exception_index fiels.
>> It is needed when exception_index is already set to some meaningful value.
>>
>> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
>>
>> Message-Id: <20171114081812.27640.26372.stgit@pasha-VirtualBox>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  accel/tcg/cpu-exec.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>> index 61297f8f4a..0473055a08 100644
>> --- a/accel/tcg/cpu-exec.c
>> +++ b/accel/tcg/cpu-exec.c
>> @@ -594,7 +594,9 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
>>      if (unlikely(atomic_read(&cpu->exit_request)
>>          || (use_icount && cpu->icount_decr.u16.low + cpu->icount_extra == 0))) {
>>          atomic_set(&cpu->exit_request, 0);
>> -        cpu->exception_index = EXCP_INTERRUPT;
>> +        if (cpu->exception_index == -1) {
>> +            cpu->exception_index = EXCP_INTERRUPT;
>> +        }
>>          return true;
>>      }
> 
> Hi. This commit breaks booting of Debian on aarch64 virt board.
> (repro instructions for creating the image available at:
> https://translatedcode.wordpress.com/2017/07/24/installing-debian-on-qemus-64-bit-arm-virt-board/)
> The guest kernel never prints anything to the serial port.
> 
> Reverting this commit fixes master for me, so I plan to do
> that on Monday.

Maybe you can also test moving the atomic_set inside the "if".  It does
seem to be a genuine bugfix.

Paolo

Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Peter Maydell]

On 17 November 2017 at 20:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 17/11/2017 21:07, Peter Maydell wrote:
>> Hi. This commit breaks booting of Debian on aarch64 virt board.
>> (repro instructions for creating the image available at:
>> https://translatedcode.wordpress.com/2017/07/24/installing-debian-on-qemus-64-bit-arm-virt-board/)
>> The guest kernel never prints anything to the serial port.
>>
>> Reverting this commit fixes master for me, so I plan to do
>> that on Monday.
>
> Maybe you can also test moving the atomic_set inside the "if".  It does
> seem to be a genuine bugfix.

No, that doesn't help: guest still sits there like a lemon.

thanks
-- PMM

Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Pavel Dovgalyuk]

> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> On 17 November 2017 at 20:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > On 17/11/2017 21:07, Peter Maydell wrote:
> >> Hi. This commit breaks booting of Debian on aarch64 virt board.
> >> (repro instructions for creating the image available at:
> >> https://translatedcode.wordpress.com/2017/07/24/installing-debian-on-qemus-64-bit-arm-virt-
> board/)
> >> The guest kernel never prints anything to the serial port.
> >>
> >> Reverting this commit fixes master for me, so I plan to do
> >> that on Monday.
> >
> > Maybe you can also test moving the atomic_set inside the "if".  It does
> > seem to be a genuine bugfix.
> 
> No, that doesn't help: guest still sits there like a lemon.

Maybe this is a more complex problem?
I tried removing this if and aarch64 still does not work.

Pavel Dovgalyuk

Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Peter Maydell]

On 20 November 2017 at 10:25, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
>> From: Peter Maydell [mailto:peter.maydell@linaro.org]
>> On 17 November 2017 at 20:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> > On 17/11/2017 21:07, Peter Maydell wrote:
>> >> Hi. This commit breaks booting of Debian on aarch64 virt board.
>> >> (repro instructions for creating the image available at:
>> >> https://translatedcode.wordpress.com/2017/07/24/installing-debian-on-qemus-64-bit-arm-virt-
>> board/)
>> >> The guest kernel never prints anything to the serial port.
>> >>
>> >> Reverting this commit fixes master for me, so I plan to do
>> >> that on Monday.
>> >
>> > Maybe you can also test moving the atomic_set inside the "if".  It does
>> > seem to be a genuine bugfix.
>>
>> No, that doesn't help: guest still sits there like a lemon.
>
> Maybe this is a more complex problem?
> I tried removing this if and aarch64 still does not work.

Reverting the commit fixes it for me; I have that going through
build tests and will push the revert later today.

thanks
-- PMM

Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Peter Maydell]

On 20 November 2017 at 11:06, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 20 November 2017 at 10:25, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
>>> From: Peter Maydell [mailto:peter.maydell@linaro.org]
>>> On 17 November 2017 at 20:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> > On 17/11/2017 21:07, Peter Maydell wrote:
>>> >> Hi. This commit breaks booting of Debian on aarch64 virt board.
>>> >> (repro instructions for creating the image available at:
>>> >> https://translatedcode.wordpress.com/2017/07/24/installing-debian-on-qemus-64-bit-arm-virt-
>>> board/)
>>> >> The guest kernel never prints anything to the serial port.
>>> >>
>>> >> Reverting this commit fixes master for me, so I plan to do
>>> >> that on Monday.
>>> >
>>> > Maybe you can also test moving the atomic_set inside the "if".  It does
>>> > seem to be a genuine bugfix.
>>>
>>> No, that doesn't help: guest still sits there like a lemon.
>>
>> Maybe this is a more complex problem?
>> I tried removing this if and aarch64 still does not work.
>
> Reverting the commit fixes it for me; I have that going through
> build tests and will push the revert later today.

Revert pushed to git master.

More generally, this commit seems to assume that QEMU always
does:
 * set exception_index to something
 * handle that
 * clear exception_index to -1

but it's not clear to me that it's actually always the case
that it gets cleared back to -1.

thanks
-- PMM

Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Paolo Bonzini]

On 20/11/2017 13:50, Peter Maydell wrote:
> More generally, this commit seems to assume that QEMU always
> does:
>  * set exception_index to something
>  * handle that
>  * clear exception_index to -1
> 
> but it's not clear to me that it's actually always the case
> that it gets cleared back to -1.

After returning from cpu_handle_interrupt, cpu_exec goes to
cpu_handle_exception which does

    if (cpu->exception_index >= EXCP_INTERRUPT) {
        *ret = cpu->exception_index;
        if (*ret == EXCP_DEBUG) {
            cpu_handle_debug_exception(cpu);
        }
        cpu->exception_index = -1;
        return true;
    } else {
        CPUClass *cc = CPU_GET_CLASS(cpu);
        qemu_mutex_lock_iothread();
        cc->do_interrupt(cpu);
        qemu_mutex_unlock_iothread();
        cpu->exception_index = -1;
    }

    return false;

Does ARM have a case where cc->do_interrupt can longjmp back to the
beginning of cpu_handle_exception?  But I still do not understand why
you don't eventually clear exception_index to -1.  Maybe there should be
an assertion for that before and after cpu_handle_interrupt.

Thanks,

Paolo

Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Pavel Dovgalyuk]

> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> On 20 November 2017 at 11:06, Peter Maydell <peter.maydell@linaro.org> wrote:
> > On 20 November 2017 at 10:25, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
> >>> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> >>> On 17 November 2017 at 20:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>> > On 17/11/2017 21:07, Peter Maydell wrote:
> >>> >> Hi. This commit breaks booting of Debian on aarch64 virt board.
> >>> >> (repro instructions for creating the image available at:
> >>> >> https://translatedcode.wordpress.com/2017/07/24/installing-debian-on-qemus-64-bit-arm-
> virt-
> >>> board/)
> >>> >> The guest kernel never prints anything to the serial port.
> >>> >>
> >>> >> Reverting this commit fixes master for me, so I plan to do
> >>> >> that on Monday.
> >>> >
> >>> > Maybe you can also test moving the atomic_set inside the "if".  It does
> >>> > seem to be a genuine bugfix.
> >>>
> >>> No, that doesn't help: guest still sits there like a lemon.
> >>
> >> Maybe this is a more complex problem?
> >> I tried removing this if and aarch64 still does not work.
> >
> > Reverting the commit fixes it for me; I have that going through
> > build tests and will push the revert later today.
> 
> Revert pushed to git master.
> 
> More generally, this commit seems to assume that QEMU always
> does:
>  * set exception_index to something
>  * handle that
>  * clear exception_index to -1
> 
> but it's not clear to me that it's actually always the case
> that it gets cleared back to -1.

I tried to get some logs with the following code.
It prints that there was an exception 5 and it was overwritten by the standard code.
Fixed code prevents this overwrite.

I guess that one of the following is true:
 - unfixed version misses some exceptions
 - fixed version processes some exceptions twice (e.g., when there is no clear exception)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 280200f..fa810f7 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -605,6 +605,8 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
     /* Finally, check if we need to exit to the main loop.  */
     if (unlikely(atomic_read(&cpu->exit_request)
         || (use_icount && cpu->icount_decr.u16.low + cpu->icount_extra == 0))) 
+        if (cpu->exception_index != -1 && cpu->exception_index != EXCP_INTERRUP
+            qemu_log("overwriting excp_index %x\n", cpu->exception_index);
         atomic_set(&cpu->exit_request, 0);
         cpu->exception_index = EXCP_INTERRUPT;
         return true;

Pavel Dovgalyuk

Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Peter Maydell]

On 9 January 2018 at 13:21, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
> I tried to get some logs with the following code.
> It prints that there was an exception 5 and it was overwritten by the standard code.
> Fixed code prevents this overwrite.
>
> I guess that one of the following is true:
>  - unfixed version misses some exceptions
>  - fixed version processes some exceptions twice (e.g., when there is no clear exception)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 280200f..fa810f7 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -605,6 +605,8 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
>      /* Finally, check if we need to exit to the main loop.  */
>      if (unlikely(atomic_read(&cpu->exit_request)
>          || (use_icount && cpu->icount_decr.u16.low + cpu->icount_extra == 0)))
> +        if (cpu->exception_index != -1 && cpu->exception_index != EXCP_INTERRUP
> +            qemu_log("overwriting excp_index %x\n", cpu->exception_index);
>          atomic_set(&cpu->exit_request, 0);
>          cpu->exception_index = EXCP_INTERRUPT;
>          return true;

This looks like it's just working around whatever is going on
(why should EXCP_INTERRUPT be special?). What we need to do is
find out what's actually happening here...

thanks
-- PMM

Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Pavel Dovgalyuk]

> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> On 9 January 2018 at 13:21, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
> > I tried to get some logs with the following code.
> > It prints that there was an exception 5 and it was overwritten by the standard code.
> > Fixed code prevents this overwrite.
> >
> > I guess that one of the following is true:
> >  - unfixed version misses some exceptions
> >  - fixed version processes some exceptions twice (e.g., when there is no clear exception)
> >
> > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> > index 280200f..fa810f7 100644
> > --- a/accel/tcg/cpu-exec.c
> > +++ b/accel/tcg/cpu-exec.c
> > @@ -605,6 +605,8 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
> >      /* Finally, check if we need to exit to the main loop.  */
> >      if (unlikely(atomic_read(&cpu->exit_request)
> >          || (use_icount && cpu->icount_decr.u16.low + cpu->icount_extra == 0)))
> > +        if (cpu->exception_index != -1 && cpu->exception_index != EXCP_INTERRUP
> > +            qemu_log("overwriting excp_index %x\n", cpu->exception_index);
> >          atomic_set(&cpu->exit_request, 0);
> >          cpu->exception_index = EXCP_INTERRUPT;
> >          return true;
> 
> This looks like it's just working around whatever is going on
> (why should EXCP_INTERRUPT be special?). What we need to do is
> find out what's actually happening here...

The failure cause is in incorrect interrupt processing.
When ARM processes hardware interrupt in arm_cpu_exec_interrupt(),
it executes cs->exception_index = excp_idx;

This assumes, that the exception will be processed later.
But it is processed immediately by calling cc->do_interrupt(cs);
instead of leaving this job to cpu_exec.

I guess these calls should be removed to match the cpu_exec execution pattern.

Pavel Dovgalyuk

Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Peter Maydell]

On 10 January 2018 at 07:04, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
> The failure cause is in incorrect interrupt processing.
> When ARM processes hardware interrupt in arm_cpu_exec_interrupt(),
> it executes cs->exception_index = excp_idx;
>
> This assumes, that the exception will be processed later.
> But it is processed immediately by calling cc->do_interrupt(cs);
> instead of leaving this job to cpu_exec.

That seems fine to me. The code knows it needs to take
an interrupt, it has all the information it needs to do
it, it can just go ahead and call the code that takes
the interrupt. The comment in accel/tcg/cpu-exec.c says:

        /* The target hook has 3 exit conditions:
           False when the interrupt isn't processed,
           True when it is, and we should restart on a new TB,
           and via longjmp via cpu_loop_exit.  */

and here we have processed the interrupt and returned true.
We set exception_index because that's how you tell the
do_interrupt hook which interrupt to deal with.

That is, the pattern that the arm target code assumes for
cs->exception_index is "you don't set this unless you're
about to call do_interrupt; if you do set it then definitely
call do_interrupt and don't do anything much in between".
In that view of the world there's no need to reset it or
check it because nothing is permitted to happen between
"set value" and "call do_interrupt". This is in line with
the way we handle other arm-specific bits of information
associated with the exception, like env->exception.target_el.
Having a long gap between "set value" and "do_interrupt"
is worrying because it means that maybe we might end
up doing something else in that gap that corrupts the
various bits of information associated with the exception,
or something that's not architecturally permitted to
happen at that point.

thanks
-- PMM

Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index

[Pavel Dovgalyuk]

> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> On 10 January 2018 at 07:04, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
> > The failure cause is in incorrect interrupt processing.
> > When ARM processes hardware interrupt in arm_cpu_exec_interrupt(),
> > it executes cs->exception_index = excp_idx;
> >
> > This assumes, that the exception will be processed later.
> > But it is processed immediately by calling cc->do_interrupt(cs);
> > instead of leaving this job to cpu_exec.
> 
> That seems fine to me. The code knows it needs to take
> an interrupt, it has all the information it needs to do
> it, it can just go ahead and call the code that takes
> the interrupt. The comment in accel/tcg/cpu-exec.c says:
> 
>         /* The target hook has 3 exit conditions:
>            False when the interrupt isn't processed,
>            True when it is, and we should restart on a new TB,
>            and via longjmp via cpu_loop_exit.  */
> 
> and here we have processed the interrupt and returned true.
> We set exception_index because that's how you tell the
> do_interrupt hook which interrupt to deal with.
> 
> That is, the pattern that the arm target code assumes for
> cs->exception_index is "you don't set this unless you're
> about to call do_interrupt; if you do set it then definitely
> call do_interrupt and don't do anything much in between".
> In that view of the world there's no need to reset it or
> check it because nothing is permitted to happen between
> "set value" and "call do_interrupt". This is in line with
> the way we handle other arm-specific bits of information
> associated with the exception, like env->exception.target_el.
> Having a long gap between "set value" and "do_interrupt"
> is worrying because it means that maybe we might end
> up doing something else in that gap that corrupts the
> various bits of information associated with the exception,
> or something that's not architecturally permitted to
> happen at that point.

I see.
I found the same pattern in other targets.
But only MIPS resets exception_index after processing
the interrupt. Others do not bother.
Then the following change of cpu_exec should be correct?

            if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
                replay_interrupt();
+               cpu->exception_index = -1;
                *last_tb = NULL;
            }

Pavel Dovgalyuk