Re: [PATCH v2 01/19] tcg: Enhance flush_icache_range with separate data pointer

[Joelle van Dyne]

s->code_ptr and s->code_buf are 4 byte pointers on aarch64 so the
cache flush is off by a factor of 4

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 44b923f5fe..2c4b66965b 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -4325,7 +4325,8 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)

 /* flush instruction cache */
 flush_idcache_range((uintptr_t)tcg_mirror_rw_to_rx(s->code_buf),
-(uintptr_t)s->code_buf, s->code_ptr - s->code_buf);
+(uintptr_t)s->code_buf,
+(uintptr_t)s->code_ptr - (uintptr_t)s->code_buf);

 return tcg_current_code_size(s);
 }

With this and the other changes, split JIT works fine on iOS.

-j

On Thu, Oct 29, 2020 at 5:49 PM Richard Henderson
 wrote:
>
> We are shortly going to have a split rw/rx jit buffer.  Depending
> on the host, we need to flush the dcache at the rw data pointer and
> flush the icache at the rx code pointer.
>
> For now, the two passed pointers are identical, so there is no
> effective change in behaviour.
>
> Signed-off-by: Richard Henderson 
> ---
>  tcg/aarch64/tcg-target.h |  9 +++--
>  tcg/arm/tcg-target.h |  8 ++--
>  tcg/i386/tcg-target.h|  3 ++-
>  tcg/mips/tcg-target.h|  8 ++--
>  tcg/ppc/tcg-target.h |  2 +-
>  tcg/riscv/tcg-target.h   |  8 ++--
>  tcg/s390/tcg-target.h|  3 ++-
>  tcg/sparc/tcg-target.h   |  8 +---
>  tcg/tci/tcg-target.h |  3 ++-
>  softmmu/physmem.c|  9 -
>  tcg/tcg.c|  5 +++--
>  tcg/aarch64/tcg-target.c.inc |  2 +-
>  tcg/mips/tcg-target.c.inc|  2 +-
>  tcg/ppc/tcg-target.c.inc | 21 +++--
>  tcg/sparc/tcg-target.c.inc   |  4 ++--
>  15 files changed, 63 insertions(+), 32 deletions(-)
>
> diff --git a/tcg/aarch64/tcg-target.h b/tcg/aarch64/tcg-target.h
> index 663dd0b95e..d0a6a059b7 100644
> --- a/tcg/aarch64/tcg-target.h
> +++ b/tcg/aarch64/tcg-target.h
> @@ -148,9 +148,14 @@ typedef enum {
>  #define TCG_TARGET_DEFAULT_MO (0)
>  #define TCG_TARGET_HAS_MEMORY_BSWAP 1
>
> -static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
> +/* Flush the dcache at RW, and the icache at RX, as necessary. */
> +static inline void flush_idcache_range(uintptr_t rx, uintptr_t rw, size_t 
> len)
>  {
> -__builtin___clear_cache((char *)start, (char *)stop);
> +/* TODO: Copy this from gcc to avoid 4 loops instead of 2. */
> +if (rw != rx) {
> +__builtin___clear_cache((char *)rw, (char *)(rw + len));
> +}
> +__builtin___clear_cache((char *)rx, (char *)(rx + len));
>  }
>
>  void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t);
> diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
> index 17e771374d..fa88b24e43 100644
> --- a/tcg/arm/tcg-target.h
> +++ b/tcg/arm/tcg-target.h
> @@ -134,9 +134,13 @@ enum {
>  #define TCG_TARGET_DEFAULT_MO (0)
>  #define TCG_TARGET_HAS_MEMORY_BSWAP 1
>
> -static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
> +/* Flush the dcache at RW, and the icache at RX, as necessary. */
> +static inline void flush_idcache_range(uintptr_t rx, uintptr_t rw, size_t 
> len)
>  {
> -__builtin___clear_cache((char *) start, (char *) stop);
> +if (rw != rx) {
> +__builtin___clear_cache((char *)rw, (char *)(rw + len));
> +}
> +__builtin___clear_cache((char *)rx, (char *)(rx + len));
>  }
>
>  /* not defined -- call should be eliminated at compile time */
> diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
> index abd4ac7fc0..8323e72639 100644
> --- a/tcg/i386/tcg-target.h
> +++ b/tcg/i386/tcg-target.h
> @@ -206,7 +206,8 @@ extern bool have_avx2;
>  #define TCG_TARGET_extract_i64_valid(ofs, len) \
>  (((ofs) == 8 && (len) == 8) || ((ofs) + (len)) == 32)
>
> -static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
> +/* Flush the dcache at RW, and the icache at RX, as necessary. */
> +static inline void flush_idcache_range(uintptr_t rx, uintptr_t rw, size_t 
> len)
>  {
>  }
>
> diff --git a/tcg/mips/tcg-target.h b/tcg/mips/tcg-target.h
> index c6b091d849..47b1226ee9 100644
> --- a/tcg/mips/tcg-target.h
> +++ b/tcg/mips/tcg-target.h
> @@ -207,9 +207,13 @@ extern bool use_mips32r2_instructions;
>  #define TCG_TARGET_DEFAULT_MO (0)
>  #define TCG_TARGET_HAS_MEMORY_BSWAP 1
>
> -static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
> +/* Flush the dcache at RW, and the icache at RX, as necessary. */
> +static inline void flush_idcache_range(uintptr_t rx, uintptr_t rw, size_t 
> len)
>  {
> -cacheflush ((void *)start, stop-start, ICACHE);
> +if (rx != rw) {
> +cacheflush((void *)rw, len, DCACHE);
> +}
> +cacheflush((void *)rx, len, ICACHE);
>  }
>
>  void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t);
> diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
> index be10363956..fbb6dc1b47 100644
> --- a/tcg/ppc/tcg-target.h
> +++ b/tcg/ppc/tcg-t

Re: [PATCH v2 14/19] RFC: accel/tcg: Support split-rwx for darwin/iOS with vm_remap

[Joelle van Dyne]

There's a compiler warning:

warning: incompatible pointer to integer conversion assigning to
'mach_vm_address_t' (aka 'unsigned long long') from 'void *'
[-Wint-conversion]
buf_rw = tcg_ctx->code_gen_buffer;

I changed it to
buf_rw = (mach_vm_address_t)tcg_ctx->code_gen_buffer;

Also, MAP_JIT doesn't work with the split mapping (it needs the same
entitlements that allows for RWX mapping) so I made the following
changes

@@ -1088,15 +1094,11 @@ static bool alloc_code_gen_buffer(size_t size,
int mirror, Error **errp)
 return true;
 }
 #else
-static bool alloc_code_gen_buffer_anon(size_t size, int prot, Error **errp)
+static bool alloc_code_gen_buffer_anon(size_t size, int prot, int
flags, Error **errp)
 {
-int flags = MAP_PRIVATE | MAP_ANONYMOUS;
 void *buf;

-#ifdef CONFIG_DARWIN
-/* Applicable to both iOS and macOS (Apple Silicon). */
-flags |= MAP_JIT;
-#endif
+flags |= MAP_PRIVATE | MAP_ANONYMOUS;

 buf = mmap(NULL, size, prot, flags, -1, 0);
 if (buf == MAP_FAILED) {
@@ -1211,7 +1213,7 @@ static bool
alloc_code_gen_buffer_mirror_vmremap(size_t size, Error **errp)
 vm_prot_t cur_prot, max_prot;

 /* Map the read-write portion via normal anon memory. */
-if (!alloc_code_gen_buffer_anon(size, PROT_READ | PROT_WRITE, errp)) {
+if (!alloc_code_gen_buffer_anon(size, PROT_READ | PROT_WRITE, 0, errp)) {
 return false;
 }

@@ -1263,6 +1265,8 @@ static bool alloc_code_gen_buffer_mirror(size_t
size, Error **errp)

 static bool alloc_code_gen_buffer(size_t size, int mirror, Error **errp)
 {
+int flags = 0;
+
 if (mirror) {
 Error *local_err = NULL;
 if (alloc_code_gen_buffer_mirror(size, &local_err)) {
@@ -1283,8 +1287,11 @@ static bool alloc_code_gen_buffer(size_t size,
int mirror, Error **errp)
 /* The tcg interpreter does not need execute permission. */
 prot = PROT_READ | PROT_WRITE;
 #endif
+#ifdef CONFIG_DARWIN
+flags |= MAP_JIT;
+#endif

-return alloc_code_gen_buffer_anon(size, prot, errp);
+return alloc_code_gen_buffer_anon(size, prot, flags, errp);
 }
 #endif /* USE_STATIC_CODE_GEN_BUFFER, WIN32, POSIX */

With this in addition to the iOS host patches, I was able to run it on
the iPad but am getting random crashes that I am continuing to debug.

-j

On Thu, Oct 29, 2020 at 5:49 PM Richard Henderson
 wrote:
>
> Cribbed from code posted by Joelle van Dyne ,
> and rearranged to a cleaner structure.  Completely untested.
>
> Signed-off-by: Richard Henderson 
> ---
>  accel/tcg/translate-all.c | 68 ++-
>  1 file changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 3e69ebd1d3..bf8263fdb4 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1093,6 +1093,11 @@ static bool alloc_code_gen_buffer_anon(size_t size, 
> int prot, Error **errp)
>  int flags = MAP_PRIVATE | MAP_ANONYMOUS;
>  void *buf;
>
> +#ifdef CONFIG_DARWIN
> +/* Applicable to both iOS and macOS (Apple Silicon). */
> +flags |= MAP_JIT;
> +#endif
> +
>  buf = mmap(NULL, size, prot, flags, -1, 0);
>  if (buf == MAP_FAILED) {
>  error_setg_errno(errp, errno,
> @@ -1182,13 +1187,74 @@ static bool alloc_code_gen_buffer_mirror_memfd(size_t 
> size, Error **errp)
>  qemu_madvise(buf_rx, size, QEMU_MADV_HUGEPAGE);
>  return true;
>  }
> -#endif
> +#endif /* CONFIG_LINUX */
> +
> +#ifdef CONFIG_DARWIN
> +#include 
> +
> +extern kern_return_t mach_vm_remap(vm_map_t target_task,
> +   mach_vm_address_t *target_address,
> +   mach_vm_size_t size,
> +   mach_vm_offset_t mask,
> +   int flags,
> +   vm_map_t src_task,
> +   mach_vm_address_t src_address,
> +   boolean_t copy,
> +   vm_prot_t *cur_protection,
> +   vm_prot_t *max_protection,
> +   vm_inherit_t inheritance);
> +
> +static bool alloc_code_gen_buffer_mirror_vmremap(size_t size, Error **errp)
> +{
> +kern_return_t ret;
> +mach_vm_address_t buf_rw, buf_rx;
> +vm_prot_t cur_prot, max_prot;
> +
> +/* Map the read-write portion via normal anon memory. */
> +if (!alloc_code_gen_buffer_anon(size, PROT_READ | PROT_WRITE, errp)) {
> +return false;
> +}
> +
> +buf_rw = tcg_ctx->code_gen_buffer;
> +buf_rx = 0;
> +ret = mach_vm_remap(mach_task_self(),
> +&buf_rx,
> +size,
> +0,
> +VM_FLAGS_ANYWHERE | VM_FLAGS_RANDOM_ADDR,
> +mach_task_self(),
> +buf_rw,
> +false,
> +&cur_prot,
> +  

Re: [PATCH v2 18/19] tcg/aarch64: Implement flush_idcache_range manually

[Joelle van Dyne]

Unfortunately this crashes on iOS/Apple Silicon macOS.

(lldb) bt
* thread #19, stop reason = EXC_BAD_INSTRUCTION (code=1, subcode=0xd53b002a)
  * frame #0: 0x0001169501e0
libqemu-x86_64-softmmu.utm.dylib`tcg_prologue_init + 760
...
(lldb) x/i 0x0001169501e0
->  0x1169501e0: 0xd53b002a   mrsx10, CTR_EL0

I was able to fix it by adding

#ifdef CONFIG_DARWIN
extern void sys_icache_invalidate(void *start, size_t len);
extern void sys_dcache_flush(void *start, size_t len);

void flush_idcache_range(uintptr_t rx, uintptr_t rw, size_t len)
{
sys_dcache_flush((void *)rw, len);
sys_icache_invalidate((void *)rx, len);
}
#else
...
#endif

Another thing, for x86 (and maybe other archs), the icache is cache
coherent but does it apply if we are aliasing the memory address? I
think in that case, it's like we're doing a DMA right and still need
to do flushing+invalidating?

-j

On Thu, Oct 29, 2020 at 5:49 PM Richard Henderson
 wrote:
>
> Copy the single pointer implementation from libgcc and modify it to
> support the double pointer interface we require.  This halves the
> number of cache operations required when split-rwx is enabled.
>
> Signed-off-by: Richard Henderson 
> ---
>  tcg/aarch64/tcg-target.h | 11 +---
>  tcg/aarch64/tcg-target.c.inc | 53 
>  2 files changed, 54 insertions(+), 10 deletions(-)
>
> diff --git a/tcg/aarch64/tcg-target.h b/tcg/aarch64/tcg-target.h
> index fa64058d43..e62d38ba55 100644
> --- a/tcg/aarch64/tcg-target.h
> +++ b/tcg/aarch64/tcg-target.h
> @@ -148,16 +148,7 @@ typedef enum {
>  #define TCG_TARGET_DEFAULT_MO (0)
>  #define TCG_TARGET_HAS_MEMORY_BSWAP 1
>
> -/* Flush the dcache at RW, and the icache at RX, as necessary. */
> -static inline void flush_idcache_range(uintptr_t rx, uintptr_t rw, size_t 
> len)
> -{
> -/* TODO: Copy this from gcc to avoid 4 loops instead of 2. */
> -if (rw != rx) {
> -__builtin___clear_cache((char *)rw, (char *)(rw + len));
> -}
> -__builtin___clear_cache((char *)rx, (char *)(rx + len));
> -}
> -
> +void flush_idcache_range(uintptr_t rx, uintptr_t rw, size_t len);
>  void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t, uintptr_t);
>
>  #ifdef CONFIG_SOFTMMU
> diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
> index bd888bc66d..5e8f3faad2 100644
> --- a/tcg/aarch64/tcg-target.c.inc
> +++ b/tcg/aarch64/tcg-target.c.inc
> @@ -2968,3 +2968,56 @@ void tcg_register_jit(const void *buf, size_t buf_size)
>  {
>  tcg_register_jit_int(buf, buf_size, &debug_frame, sizeof(debug_frame));
>  }
> +
> +/*
> + * Flush the dcache at RW, and the icache at RX, as necessary.
> + * This is a copy of gcc's __aarch64_sync_cache_range, modified
> + * to fit this three-operand interface.
> + */
> +void flush_idcache_range(uintptr_t rx, uintptr_t rw, size_t len)
> +{
> +const unsigned CTR_IDC = 1u << 28;
> +const unsigned CTR_DIC = 1u << 29;
> +static unsigned int cache_info;
> +uintptr_t icache_lsize, dcache_lsize, p;
> +
> +if (!cache_info) {
> +/*
> + * CTR_EL0 [3:0] contains log2 of icache line size in words.
> + * CTR_EL0 [19:16] contains log2 of dcache line size in words.
> + */
> +asm volatile("mrs\t%0, ctr_el0" : "=r"(cache_info));
> +}
> +
> +icache_lsize = 4 << extract32(cache_info, 0, 4);
> +dcache_lsize = 4 << extract32(cache_info, 16, 4);
> +
> +/*
> + * If CTR_EL0.IDC is enabled, Data cache clean to the Point of 
> Unification
> + * is not required for instruction to data coherence.
> + */
> +if (!(cache_info & CTR_IDC)) {
> +/*
> + * Loop over the address range, clearing one cache line at once.
> + * Data cache must be flushed to unification first to make sure
> + * the instruction cache fetches the updated data.
> + */
> +for (p = rw & -dcache_lsize; p < rw + len; p += dcache_lsize) {
> +asm volatile("dc\tcvau, %0" : : "r" (p) : "memory");
> +}
> +asm volatile("dsb\tish" : : : "memory");
> +}
> +
> +/*
> + * If CTR_EL0.DIC is enabled, Instruction cache cleaning to the Point
> + * of Unification is not required for instruction to data coherence.
> + */
> +if (!(cache_info & CTR_DIC)) {
> +for (p = rx & -icache_lsize; p < rx + len; p += icache_lsize) {
> +asm volatile("ic\tivau, %0" : : "r"(p) : "memory");
> +}
> +asm volatile ("dsb\tish" : : : "memory");
> +}
> +
> +asm volatile("isb" : : : "memory");
> +}
> --
> 2.25.1
>

[Bug 1890333] Re: [OSS-Fuzz] Issue 26693: qemu:qemu-fuzz-i386-target-generic-fuzz-xhci: Index-out-of-bounds in xhci_runtime_write Assertion failure in address_space_stw_le_cached through virtio-* devi

[Alexander Bulekov]

OSS-Fuzz Report: https://bugs.chromium.org/p/oss-
fuzz/issues/detail?id=26797

=== Reproducer (build with --enable-sanitizers) ===
cat << EOF | ./qemu-system-i386 -display none \
-machine accel=qtest, -m 512M -machine q35 \
-device virtio-blk,drive=disk0 \
-drive file=null-co://,id=disk0,if=none,format=raw \
-qtest stdio
outl 0xcf8 0x80001889
outl 0xcfc 0x1000
outl 0xcf8 0x80001897
outl 0xcf8 0x80001890
outl 0xcfc 0x4
outl 0xcf8 0x800018ff
outl 0xcf8 0x80001897
inb 0xcfc
outl 0xcf8 0x8000188a
outl 0xcfc 0xd4624
outl 0xcf8 0x80001897
outl 0xcf8 0x80001806
outl 0xcf8 0x80001897
outl 0xcfc 0xff6ca0ba
outl 0xcf8 0x8000188c
outw 0xcfc 0x14
outl 0xcf8 0x80001897
outl 0xcf8 0x8000185a
outl 0xcf8 0x80001897
outl 0xcfc 0x5f6c6346
inb 0xcfc
outl 0xcf8 0x80001802
outl 0xcfc 0x65a6055
outl 0xcf8 0x80001897
inb 0xcfc
outl 0xcf8 0x80001889
outl 0xcfc 0x1869
outl 0xcf8 0x80001812
outl 0xcf8 0x80001897
outl 0xcfc 0x5f6c6346
outl 0xcf8 0x8000188c
outw 0xcfc 0x24
outl 0xcf8 0x80001890
outl 0xcf8 0x80001897
outl 0xcfc 0x1
outl 0xcf8 0x80001892
outl 0xcfc 0x1ff04
outl 0xcf8 0x8000188c
outw 0xcfc 0x1c
outl 0xcf8 0x80001890
outl 0xcfc 0x1
outl 0xcf8 0x80001897
outl 0xcfc 0xfd467562
outl 0xcf8 0x8000188a
outl 0xcfc 0x245a5546
outl 0xcf8 0x80001890
outl 0xcf8 0x80001897
inb 0xcfc
outl 0xcf8 0x8000188c
outw 0xcfc 0x14
outl 0xcf8 0x80001897
outl 0xcf8 0x80001806
outl 0xcf8 0x80001889
outl 0xcfc 0x1869
outl 0xcf8 0x80001812
outl 0xcf8 0x80001897
outl 0xcfc 0x6c6346
outl 0xcf8 0x8000188c
outw 0xcfc 0x14
outl 0xcf8 0x80001890
outl 0xcf8 0x80001897
outl 0xcfc 0x1ff04
EOF

=== Stack Trace ===

qemu-fuzz-i386-target-generic-fuzz-virtio-blk: 
/src/qemu/include/exec/memory_ldst_cached.h.inc:88: void 
address_space_stw_le_cached(MemoryRegionCache *, hwaddr, uint32_t, MemTxAttrs, 
MemTxResult *): Assertion `addr < cache->len && 2 <= cache->len - addr' failed.
==46== ERROR: libFuzzer: deadly signal
#0 0x55deb7b59e61 in __sanitizer_print_stack_trace 
/src/llvm-project/compiler-rt/lib/asan/asan_stack.cpp:86:3
#1 0x55deb7aa1158 in fuzzer::PrintStackTrace() 
/src/llvm-project/compiler-rt/lib/fuzzer/FuzzerUtil.cpp:210:5
#2 0x55deb7a87053 in fuzzer::Fuzzer::CrashCallback() 
/src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:233:3
#3 0x7fccd310638f in libpthread.so.0
#4 0x7fccd273e437 in gsignal
#5 0x7fccd2740039 in abort
#6 0x7fccd2736be6 in libc.so.6
#7 0x7fccd2736c91 in __assert_fail
#8 0x55deb8416ba3 in address_space_stw_le_cached 
/src/qemu/include/exec/memory_ldst_cached.h.inc:88:5
#9 0x55deb8416a40 in stw_le_phys_cached 
/src/qemu/include/exec/memory_ldst_phys.h.inc:121:5
#10 0x55deb8416a13 in virtio_stw_phys_cached 
/src/qemu/include/hw/virtio/virtio-access.h:196:9
#11 0x55deb8416899 in vring_set_avail_event /src/qemu/hw/virtio/virtio.c:428:5
#12 0x55deb8406ba8 in virtio_queue_split_set_notification 
/src/qemu/hw/virtio/virtio.c:437:9
#13 0x55deb84067a2 in virtio_queue_set_notification 
/src/qemu/hw/virtio/virtio.c:498:9
#14 0x55deb84755d3 in virtio_blk_handle_vq 
/src/qemu/hw/block/virtio-blk.c:795:13
#15 0x55deb84916ce in virtio_blk_data_plane_handle_output 
/src/qemu/hw/block/dataplane/virtio-blk.c:165:12
#16 0x55deb841afaf in virtio_queue_notify_aio_vq 
/src/qemu/hw/virtio/virtio.c:2325:15
#17 0x55deb8415adb in virtio_queue_host_notifier_aio_read 
/src/qemu/hw/virtio/virtio.c:3529:9
#18 0x55deb892af84 in aio_dispatch_handler /src/qemu/util/aio-posix.c:329:9
#19 0x55deb8929b8a in aio_dispatch_handlers /src/qemu/util/aio-posix.c:372:20
#20 0x55deb8929ac0 in aio_dispatch /src/qemu/util/aio-posix.c:382:5
#21 0x55deb893ae2c in aio_ctx_dispatch /src/qemu/util/async.c:306:5
#22 0x7fccd3868196 in g_main_context_dispatch
#23 0x55deb8947fed in glib_pollfds_poll /src/qemu/util/main-loop.c:221:9
#24 0x55deb8947264 in os_host_main_loop_wait /src/qemu/util/main-loop.c:244:5
#25 0x55deb8946f25 in main_loop_wait /src/qemu/util/main-loop.c:520:11
#26 0x55deb7b8806a in flush_events /src/qemu/tests/qtest/fuzz/fuzz.c:48:9
#27 0x55deb7b85a32 in generic_fuzz 
/src/qemu/tests/qtest/fuzz/generic_fuzz.c:681:17
#28 0x55deb7b88450 in LLVMFuzzerTestOneInput 
/src/qemu/tests/qtest/fuzz/fuzz.c:150:5
#29 0x55deb7a887c1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, 
unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:595:15
#30 0x55deb7a73892 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned 
long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:323:6
#31 0x55deb7a7994e in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char 
const*, unsigned long)) 
/src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:852:9
#32 0x55deb7aa1932 in main 
/src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#33 0x7fccd272983f in __libc_start_main
#34 0x55deb7a4eb38 in _start

** Summary changed:

- Assertion failure in address_space_stw_le_cached through virtio-* devices
+ [OSS-Fuzz] Issue 26693: qemu:qemu-fuzz-i386-target-generic-fuzz-xhci: 
Index-out-of-bounds in xhci_runtime_write Assertion failure i

[Bug 1902394] [NEW] Guest stuck in Paused state right after created It

[Tuan Anh]

Public bug reported:

Im using Centos 8 . I have try to use many Distribution such as :
Centos, Ubuntum, Debian,.. on the guest but still all the the VM get
into paused state immidiately after using virt-install ( I have tried
using virt-manager too )

CPU INFO :
Architecture:x86_64
CPU op-mode(s):  32-bit, 64-bit
Byte Order:  Little Endian
CPU(s):  8
On-line CPU(s) list: 0-7
Thread(s) per core:  1
Core(s) per socket:  1
Socket(s):   8
NUMA node(s):1
Vendor ID:   GenuineIntel
CPU family:  6
Model:   85
Model name:  Intel(R) Xeon(R) Silver 4214 CPU @ 2.20GHz
Stepping:7
CPU MHz: 2199.998
BogoMIPS:4399.99
Virtualization:  VT-x
Hypervisor vendor:   KVM
Virtualization type: full
L1d cache:   32K
L1i cache:   32K
L2 cache:4096K
L3 cache:16384K
NUMA node0 CPU(s):   0-7
Flags:   fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
cmov pat pse36 clflush mmx fxsr sse sse2 ss syscall nx pdpe1gb rdtscp lm 
constant_tsc arch_perfmon rep_good nopl xtopology cpuid tsc_known_freq pni 
pclmulqdq vmx ssse3 fma cx16 pcid sse4_1 sse4_2 x2apic movbe popcnt 
tsc_deadline_timer aes xsave avx f16c rdrand hypervisor lahf_lm abm 
3dnowprefetch invpcid_single pti ssbd ibrs ibpb tpr_shadow vnmi flexpriority 
ept vpid fsgsbase tsc_adjust bmi1 hle avx2 smep bmi2 erms invpcid rtm mpx 
avx512f rdseed adx smap clflushopt clwb avx512cd xsaveopt xsavec xgetbv1 arat

VM Log :

2020-10-31 08:29:51.737+: starting up libvirt version: 4.5.0, package: 
42.module_el8.2.0+320+13f867d7 (CentOS Buildsys , 
2020-05-28-17:13:31, ), qemu version: 
2.12.0qemu-kvm-2.12.0-99.module_el8.2.0+524+f765f7e0.4, kernel: 
4.18.0-193.28.1.el8_2.x86_64, hostname: interns.novalocal
LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin 
QEMU_AUDIO_DRV=spice /usr/libexec/qemu-kvm -name guest=cirros,debug-threads=on 
-S -object 
secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-18-cirros/master-key.aes
 -machine pc-i440fx-rhel7.6.0,accel=kvm,usb=off,vmport=off,dump-guest-core=off 
-cpu 
Cascadelake-Server,ss=on,hypervisor=on,tsc-adjust=on,arch-capabilities=on,ibpb=on,skip-l1dfl-vmentry=on,invpcid=off,avx512dq=off,avx512bw=off,avx512vl=off,pku=off,avx512vnni=off,pdpe1gb=off
 -m 1024 -realtime mlock=off -smp 1,sockets=1,cores=1,threads=1 -uuid 
ef9573a3-a02d-4ef0-86cb-e38da7b7b20d -no-user-config -nodefaults -chardev 
socket,id=charmonitor,fd=29,server,nowait -mon 
chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew -global 
kvm-pit.lost_tick_policy=delay -no-hpet -no-shutdown -global 
PIIX4_PM.disable_s3=1 -global PIIX4_PM.disable_s4=1 -boot strict=on -device 
ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x5.0x7 -device 
ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5 
-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x5.0x1 
-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2 
-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 -drive 
file=/home/kvm/cirros-0.3.0-x86_64-disk.img,format=qcow2,if=none,id=drive-ide0-0-0
 -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 
-netdev tap,fd=31,id=hostnet0 -device 
e1000,netdev=hostnet0,id=net0,mac=52:54:00:c3:32:b0,bus=pci.0,addr=0x3 -chardev 
pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev 
spicevmc,id=charchannel0,name=vdagent -device 
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
 -device usb-tablet,id=input0,bus=usb.0,port=1 -spice 
port=5900,addr=127.0.0.1,disable-ticketing,image-compression=off,seamless-migration=on
 -device 
qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pci.0,addr=0x2
 -device intel-hda,id=sound0,bus=pci.0,addr=0x4 -device 
hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -chardev 
spicevmc,id=charredir0,name=usbredir -device 
usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=2 -chardev 
spicevmc,id=charredir1,name=usbredir -device 
usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=3 -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x7 -sandbox 
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny -msg 
timestamp=on
2020-10-31T08:29:51.815604Z qemu-kvm: -chardev pty,id=charserial0: char device 
redirected to /dev/pts/1 (label charserial0)
KVM: exception 0 exit (error code 0x0)
EAX= EBX= ECX= EDX=00050656
ESI= EDI= EBP= ESP=
EIP=fff0 EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =   9300
CS =f000   9b00
SS =   9300
DS =   9300
FS =   9300
GS =   9300
LDT=   8200
TR =  

Re: Out-of-Process Device Emulation session at KVM Forum 2020

[Michael S. Tsirkin]

On Fri, Oct 30, 2020 at 11:13:59AM +, Stefan Hajnoczi wrote:
> > > 3. The device can save/load opaque blobs. This is the initial VFIO
> > > approach.
> >
> >
> > I still don't get why it must be opaque.
> 
> If the device state format needs to be in the VMM then each device
> needs explicit enablement in each VMM (QEMU, cloud-hypervisor, etc).

And QEMU cares why exactly?

> Let's invert the question: why does the VMM need to understand the
> device state of a _passthrough_ device?

To support cross version migration and compatibility checks.
This problem is harder than it appears, I don't think vendors
will do a good job of it without any guidance and standards.

-- 
MST

Re: [PULL v2 01/16] tests/9pfs: fix test dir for parallel tests

[Christian Schoenebeck]

On Samstag, 31. Oktober 2020 21:34:31 CET Peter Maydell wrote:
> On Sat, 31 Oct 2020 at 13:20, Christian Schoenebeck
> 
>  wrote:
> > On Freitag, 30. Oktober 2020 13:07:03 CET Christian Schoenebeck wrote:
> > > Use mkdtemp() to generate a unique directory for the 9p 'local' tests.
> > > 
> > > This fixes occasional 9p test failures when running 'make check -jN' if
> > > QEMU was compiled for multiple target architectures, because the
> > > individual
> > > architecture's test suites would run in parallel and interfere with each
> > > other's data as the test directory was previously hard coded and hence
> > > the
> > > same directory was used by all of them simultaniously.
> > > 
> > > This also requires a change how the test directory is created and
> > > deleted:
> > > As the test path is now randomized and virtio_9p_register_nodes() being
> > > called in a somewhat undeterministic way, that's no longer an
> > > appropriate
> > > place to create and remove the test directory. Use a constructor and
> > > destructor function for creating and removing the test directory
> > > instead.
> > > Unfortunately libqos currently does not support setup/teardown callbacks
> > > to handle this more cleanly.
> > 
> > Peter, please ignore this PR. This patch needs rework:
> OK. As a general rule you need to make "please drop this PR"
> requests as replies to the top level cover letter, though --
> otherwise it's pot luck whether I happen to notice them or not.
> 
> thanks
> -- PMM

Okay, got it.

Best regards,
Christian Schoenebeck

Re: [PULL v2 01/16] tests/9pfs: fix test dir for parallel tests

[Peter Maydell]

On Sat, 31 Oct 2020 at 13:20, Christian Schoenebeck
 wrote:
>
> On Freitag, 30. Oktober 2020 13:07:03 CET Christian Schoenebeck wrote:
> > Use mkdtemp() to generate a unique directory for the 9p 'local' tests.
> >
> > This fixes occasional 9p test failures when running 'make check -jN' if
> > QEMU was compiled for multiple target architectures, because the individual
> > architecture's test suites would run in parallel and interfere with each
> > other's data as the test directory was previously hard coded and hence the
> > same directory was used by all of them simultaniously.
> >
> > This also requires a change how the test directory is created and deleted:
> > As the test path is now randomized and virtio_9p_register_nodes() being
> > called in a somewhat undeterministic way, that's no longer an appropriate
> > place to create and remove the test directory. Use a constructor and
> > destructor function for creating and removing the test directory instead.
> > Unfortunately libqos currently does not support setup/teardown callbacks
> > to handle this more cleanly.
>
> Peter, please ignore this PR. This patch needs rework:

OK. As a general rule you need to make "please drop this PR"
requests as replies to the top level cover letter, though --
otherwise it's pot luck whether I happen to notice them or not.

thanks
-- PMM

Re: [PULL 0/5] Misc next patches

[Peter Maydell]

On Thu, 29 Oct 2020 at 10:04, Daniel P. Berrangé  wrote:
>
> The following changes since commit bbc48d2bcb9711614fbe751c2c5ae13e172fbca8:
>
>   Merge remote-tracking branch 'remotes/philmd-gitlab/tags/renesas-20201027' =
> into staging (2020-10-28 16:25:31 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/berrange/qemu tags/misc-next-pull-request
>
> for you to fetch changes up to dfc00eb7dea43bfb6d4a2ba38c4f6bc9745f3729:
>
>   util: include the target address in socket connect failures (2020-10-29 09:=
> 57:37 +)
>
> 
> Misc fixes
>
>  * Improve socket cnnection failure error reporting
>  * Fix LGPL version number
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM

Re: [PULL 00/16] migration queue

[Christian Schoenebeck]

On Samstag, 31. Oktober 2020 18:46:11 CET Peter Xu wrote:
> On Sat, Oct 31, 2020 at 05:26:28PM +, Peter Maydell wrote:
> > On Sat, 31 Oct 2020 at 16:12, Christian Schoenebeck
> > 
> >  wrote:
> > > On Montag, 26. Oktober 2020 17:19:36 CET Dr. David Alan Gilbert (git) 
wrote:
> > > > 
> > > > migration pull: 2020-10-26
> > > > 
> > > > Another go at Peter's postcopy fixes
> > > > 
> > > > Cleanups from Bihong Yu and Peter Maydell.
> > > > 
> > > > Signed-off-by: Dr. David Alan Gilbert 
> > > 
> > > May it be possible that this PR introduced a lockup of the qtests that I
> > > am
> > > encountering in this week's upstream revisions?
> > 
> > If you try the patches Peter Xu attached to this thread
> > does the lockup go away ?
> > 
> > https://lore.kernel.org/qemu-devel/20201030135350.GA588069@xz-x1/
> > 
> > (I'm also seeing intermittent hangs, for some reason almost always
> > on s390x host.)
> 
> It would be good to know exactly which test hanged.  If it's migration-test
> then it's very possible.

It's run-test-144 that does not return; according to Makefile.mtest that's 
migration-test, so chances are high that it's indeed introduced by this PR.

> The race above patch(es) tried to fix should logically be reproducable on
> all archs, not s390x only.
> 
> Thanks,

Yes, it's i386 here that locks up.

I'm running the loop with your patches now, so far so good, let's see if it's 
still alive tomorrow.

Best regards,
Christian Schoenebeck

Re: [PULL v3 00/22] Build system + misc changes for 2020-10-16

[Paolo Bonzini]

Yup, it works only with --sphix-build which obviously is how I tested it...
I will include a fix in my next pull request (I don't really have anything
planned, but something will most likely pop up).

Paolo

Il sab 31 ott 2020, 16:46 Peter Maydell  ha
scritto:

> On Sat, 17 Oct 2020 at 15:50, Paolo Bonzini  wrote:
> > 
> > * Drop ninjatool and just require ninja (Paolo)
> > * Fix docs build under msys2 (Yonggang)
> > * HAX snafu fix (Claudio)
> > * Disable signal handlers during fuzzing (Alex)
> > * Miscellaneous fixes (Bruce, Greg)
> >
> > Yonggang Luo (3):
> >   docs: Fix Sphinx configuration for msys2/mingw
> >   meson: Move the detection logic for sphinx to meson
> >   cirrus: Enable doc build on msys2/mingw
>
> I've just noticed that there seems to be a minor bug with
> the new sphinx detection logic: if the Sphinx is the
> wrong version then it prints:
>
> Program sphinx-build found: YES
> ../../docs/meson.build:30: WARNING:  exists but it is either too old
> or uses too old a Python version
>
> ie it hasn't actually managed to substitute in the
> program name, so there's just a double-space after
> WARNING: instead...
>
> thanks
> -- PMM
>
>

Re: [PULL 00/16] migration queue

[Peter Xu]

On Sat, Oct 31, 2020 at 05:26:28PM +, Peter Maydell wrote:
> On Sat, 31 Oct 2020 at 16:12, Christian Schoenebeck
>  wrote:
> >
> > On Montag, 26. Oktober 2020 17:19:36 CET Dr. David Alan Gilbert (git) wrote:
> > > 
> > > migration pull: 2020-10-26
> > >
> > > Another go at Peter's postcopy fixes
> > >
> > > Cleanups from Bihong Yu and Peter Maydell.
> > >
> > > Signed-off-by: Dr. David Alan Gilbert 
> > >
> >
> > May it be possible that this PR introduced a lockup of the qtests that I am
> > encountering in this week's upstream revisions?
> 
> If you try the patches Peter Xu attached to this thread
> does the lockup go away ?
> 
> https://lore.kernel.org/qemu-devel/20201030135350.GA588069@xz-x1/
> 
> (I'm also seeing intermittent hangs, for some reason almost always
> on s390x host.)

It would be good to know exactly which test hanged.  If it's migration-test
then it's very possible.

The race above patch(es) tried to fix should logically be reproducable on all
archs, not s390x only.

Thanks,

-- 
Peter Xu

Re: [PULL 00/16] migration queue

[Peter Maydell]

On Sat, 31 Oct 2020 at 16:12, Christian Schoenebeck
 wrote:
>
> On Montag, 26. Oktober 2020 17:19:36 CET Dr. David Alan Gilbert (git) wrote:
> > 
> > migration pull: 2020-10-26
> >
> > Another go at Peter's postcopy fixes
> >
> > Cleanups from Bihong Yu and Peter Maydell.
> >
> > Signed-off-by: Dr. David Alan Gilbert 
> >
>
> May it be possible that this PR introduced a lockup of the qtests that I am
> encountering in this week's upstream revisions?

If you try the patches Peter Xu attached to this thread
does the lockup go away ?

https://lore.kernel.org/qemu-devel/20201030135350.GA588069@xz-x1/

(I'm also seeing intermittent hangs, for some reason almost always
on s390x host.)

thanks
-- PMM

Re: [PATCH v3] qom: code hardening - have bound checking while looping with integer value

[Ani Sinha]

On Thu, Oct 15, 2020 at 10:22 PM Eduardo Habkost  wrote:
>
> On Mon, Sep 21, 2020 at 03:03:25PM +0530, Ani Sinha wrote:
> > Object property insertion code iterates over an integer to get an unused
> > index that can be used as an unique name for an object property. This loop
> > increments the integer value indefinitely. Although very unlikely, this can
> > still cause an integer overflow.
> > In this change, we fix the above code by checking against INT16_MAX and 
> > making
> > sure that the interger index does not overflow beyond that value. If no
> > available index is found, the code would cause an assertion failure. This
> > assertion failure is necessary because the callers of the function do not 
> > check
> > the return value for NULL.
> >
> > Signed-off-by: Ani Sinha 
> > Reviewed-by: Daniel P. Berrangé 
>
> Queued on machine-next, thanks!  My apologies for the delay.

Any idea when this will be pulled?

Re: [PULL 00/16] migration queue

[Christian Schoenebeck]

On Montag, 26. Oktober 2020 17:19:36 CET Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> The following changes since commit a46e72710566eea0f90f9c673a0f02da0064acce:
> 
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20201026' into
> staging (2020-10-26 14:50:03 +)
> 
> are available in the Git repository at:
> 
>   git://github.com/dagrh/qemu.git tags/pull-migration-20201026a
> 
> for you to fetch changes up to a47295014de56e108f359ec859d5499b851f62b8:
> 
>   migration-test: Only hide error if !QTEST_LOG (2020-10-26 16:15:04 +)
> 
> 
> migration pull: 2020-10-26
> 
> Another go at Peter's postcopy fixes
> 
> Cleanups from Bihong Yu and Peter Maydell.
> 
> Signed-off-by: Dr. David Alan Gilbert 
> 

May it be possible that this PR introduced a lockup of the qtests that I am 
encountering in this week's upstream revisions?

PASS 25 qtest-x86_64/bios-tables-test /x86_64/acpi/microvm/pcie

Looking for expected file 'tests/data/acpi/microvm/FACP.rtc'
Looking for expected file 'tests/data/acpi/microvm/FACP'
Using expected file 'tests/data/acpi/microvm/FACP'
Looking for expected file 'tests/data/acpi/microvm/APIC.rtc'
Looking for expected file 'tests/data/acpi/microvm/APIC'
Using expected file 'tests/data/acpi/microvm/APIC'
Looking for expected file 'tests/data/acpi/microvm/DSDT.rtc'
Using expected file 'tests/data/acpi/microvm/DSDT.rtc'
PASS 24 qtest-i386/bios-tables-test /i386/acpi/microvm/rtc
PASS 15 qtest-i386/migration-test /i386/migration/multifd/tcp/cancel
PASS 16 qtest-i386/migration-test /i386/migration/multifd/tcp/zlib
^Cmake: *** [Makefile.mtest:1169: run-test-144] Interrupt

I tried to bisect the causing commit and came up to this PR merge. But I'm not 
absolutely sure it really is, because it sometimes takes 2 hours or more to 
trigger the lockup, so the relevant commit may as well have slipped during 
bisection.

Last week's revisions run here for >24h without issues, right now I get 
lockups of test runs after ~3min .. ~2h when running the qtests in an endless 
loop:

#/bin/sh
i=0
while true; do
  i=$[$i+1]
  echo '**'
  echo "* RUN $i *"
  echo '**'
  make check-qtest -j32 V=1
  if [[ "$?" -ne 0 ]]; then
echo "Aborted after $i runs due to failure"
break
  fi
done

> 
> Bihong Yu (9):
>   migration: Do not use C99 // comments
>   migration: Don't use '#' flag of printf format
>   migration: Add spaces around operator
>   migration: Open brace '{' following struct go on the same line
>   migration: Add braces {} for if statement
>   migration: Do not initialise statics and globals to 0 or NULL
>   migration: Open brace '{' following function declarations go on the
> next line migration: Delete redundant spaces
>   migration: using trace_ to replace DPRINTF
> 
> Peter Maydell (1):
>   migration: Drop unused VMSTATE_FLOAT64 support
> 
> Peter Xu (6):
>   migration: Pass incoming state into qemu_ufd_copy_ioctl()
>   migration: Introduce migrate_send_rp_message_req_pages()
>   migration: Maintain postcopy faulted addresses
>   migration: Sync requested pages after postcopy recovery
>   migration/postcopy: Release fd before going into 'postcopy-pause'
>   migration-test: Only hide error if !QTEST_LOG
> 
>  include/migration/vmstate.h  | 13 --
>  migration/block.c| 40 ++---
>  migration/migration.c| 59
> +- migration/migration.h|
> 24 ++---
>  migration/page_cache.c   | 13 +++---
>  migration/postcopy-ram.c | 27 +++-
>  migration/ram.c  | 14 +-
>  migration/rdma.c |  7 ++---
>  migration/savevm.c   | 61
> ++-- migration/trace-events   |
> 16 
>  migration/vmstate-types.c| 26 ---
>  migration/vmstate.c  | 10 
>  tests/qtest/migration-test.c |  6 -
>  13 files changed, 213 insertions(+), 103 deletions(-)

Re: [PATCH qemu v10] spapr: Implement Open Firmware client interface

[Greg Kurz]

On Sat, 31 Oct 2020 16:53:24 +1100
Alexey Kardashevskiy  wrote:

> Has anyone at least tried this, or everybody is busy  KVMforuming? :)
> 

Yeah virtualKVMforuming :)

I had tried when you posted this v10 but I didn't get far at the time,
because the kernel I was passing didn't want to boot and then I got
distracted by more urgent work... so I just gave a try on my laptop 
(fedora32 with gcc-10.2.1-6.fc32.x86_64) and:

[36/945] Compiling C object 
libqemu-ppc64-softmmu.fa.p/hw_ppc_spapr_of_client.c.o
FAILED: libqemu-ppc64-softmmu.fa.p/hw_ppc_spapr_of_client.c.o 
cc -Ilibqemu-ppc64-softmmu.fa.p -I. -I../.. -Itarget/ppc -I../../target/ppc 
-Itrace -Iqapi -Iui -Iui/shader -I/usr/include/capstone -I/usr/include/pixman-1 
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fdiagnostics-color=auto 
-pipe -Wall -Winvalid-pch -Werror -std=gnu99 -O2 -g -U_FORTIFY_SOURCE 
-D_FORTIFY_SOURCE=2 -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef 
-Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
-Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security 
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs 
-Wendif-labels -Wexpansion-to-defined -Wno-missing-include-dirs 
-Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -isystem 
/home/greg/Work/qemu/qemu-ppc/linux-headers -isystem linux-headers -iquote 
/home/greg/Work/qemu/qemu-ppc/tcg/i386 -iquote . -iquote 
/home/greg/Work/qemu/qemu-ppc -iquote /home/greg/Work/qemu/qemu-ppc/accel/tcg 
-iquote /home/greg/Work/qemu/qemu-ppc/include -iquote 
/home/greg/Work/qemu/qemu-ppc/disas/libvixl -pthread -fPIC 
-isystem../../linux-headers -isystemlinux-headers -DNEED_CPU_H 
'-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' 
'-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ 
libqemu-ppc64-softmmu.fa.p/hw_ppc_spapr_of_client.c.o -MF 
libqemu-ppc64-softmmu.fa.p/hw_ppc_spapr_of_client.c.o.d -o 
libqemu-ppc64-softmmu.fa.p/hw_ppc_spapr_of_client.c.o -c 
../../hw/ppc/spapr_of_client.c
../../hw/ppc/spapr_of_client.c: In function ‘prop_format’:
../../hw/ppc/spapr_of_client.c:131:29: error: comparison is always false due to 
limited range of data type [-Werror=type-limits]
  131 | if (*c < 0x20 || *c >= 0x80) {
  | ^~

Fixed by:

-const char *c;
+const unsigned char *c;

../../hw/ppc/spapr_of_client.c: In function ‘spapr_h_of_client’:
../../hw/ppc/spapr_of_client.c:793:35: error: taking address of packed member 
of ‘struct prom_args’ may result in an unaligned pointer value 
[-Werror=address-of-packed-member]
  793 |   &pargs.args[nargs + 1]);
  |   ^~
../../hw/ppc/spapr_of_client.c:800:38: error: taking address of packed member 
of ‘struct prom_args’ may result in an unaligned pointer value 
[-Werror=address-of-packed-member]
  800 |  &pargs.args[nargs + 1]);
  |  ^~
cc1: all warnings being treated as errors

Fixed by dropping QEMU_PACKED and ensuring we don't have unwanted padding:

-} QEMU_PACKED;
+};
+
+QEMU_BUILD_BUG_ON(sizeof(struct prom_args) != 13 * 4);

I'll resume my experiments later :)

Cheers,

--
Greg

Re: [PULL v3 00/22] Build system + misc changes for 2020-10-16

[Peter Maydell]

On Sat, 17 Oct 2020 at 15:50, Paolo Bonzini  wrote:
> 
> * Drop ninjatool and just require ninja (Paolo)
> * Fix docs build under msys2 (Yonggang)
> * HAX snafu fix (Claudio)
> * Disable signal handlers during fuzzing (Alex)
> * Miscellaneous fixes (Bruce, Greg)
>
> Yonggang Luo (3):
>   docs: Fix Sphinx configuration for msys2/mingw
>   meson: Move the detection logic for sphinx to meson
>   cirrus: Enable doc build on msys2/mingw

I've just noticed that there seems to be a minor bug with
the new sphinx detection logic: if the Sphinx is the
wrong version then it prints:

Program sphinx-build found: YES
../../docs/meson.build:30: WARNING:  exists but it is either too old
or uses too old a Python version

ie it hasn't actually managed to substitute in the
program name, so there's just a double-space after
WARNING: instead...

thanks
-- PMM

Re: [PATCH] util: Remove redundant checks in the openpty()

[Peter Maydell]

On Sat, 31 Oct 2020 at 11:04, AlexChen  wrote:
>
> As we can see from the following function call stack, the amaster and the 
> aslave
> cannot be NULL: char_pty_open() -> qemu_openpty_raw() -> openpty().
> In addition, the amaster and the aslave has been dereferenced at the beginning
> of the openpty(). So the checks on amaster and aslave in the openpty() are 
> redundant.
>
> Reported-by: Euler Robot 
> Signed-off-by: Alex Chen 

This function is trying to match the BSD/glibc openpty()
function, so the thing to check here is not QEMU's specific
current usage but the API specification for openpty():
https://www.gnu.org/software/libc/manual/html_node/Pseudo_002dTerminal-Pairs.html
https://www.freebsd.org/cgi/man.cgi?query=openpty

The spec says that name, termp and winp can all be
NULL, but it doesn't say this for amaster and aslave,
so indeed the change in this patch is the correct one.

> ---
>  util/qemu-openpty.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
> index eb17f5b0bc..427f43a769 100644
> --- a/util/qemu-openpty.c
> +++ b/util/qemu-openpty.c
> @@ -80,10 +80,9 @@ static int openpty(int *amaster, int *aslave, char *name,
>  (termp != NULL && tcgetattr(sfd, termp) < 0))
>  goto err;
>
> -if (amaster)
> -*amaster = mfd;
> -if (aslave)
> -*aslave = sfd;
> +*amaster = mfd;
> +*aslave = sfd;
> +
>  if (winp)
>  ioctl(sfd, TIOCSWINSZ, winp);

Reviewed-by: Peter Maydell 

though you might like to mention in the commit message that
the openpty() API doesn't allow NULL amaster or aslave
arguments.

thanks
-- PMM

Re: [PATCH 0/2] docs: Fix building with Sphinx 3.2

[Paolo Bonzini]

On 30/10/20 21:02, Peter Maydell wrote:
> Having actually looked at the current state of the kernel's
> kernel-doc script I see somebody has already done the
> necessary updates for Sphinx 3 compatibility. So we have
> two choices for 5.2:
>  * take this patch 1 as a minimal fix
>  * do the sync of the kernel's version of the script
> 
> I'll have a look at how painful or otherwise the sync is, I guess.
> We only use the kerneldoc stuff in the 'devel' manual which
> is not really userfacing anyway.

Oh nice! I had a slightly older Linux checkout and it wasn't there yet.
 I think you can go ahead with these two patches, I'll take care of the
sync for 6.0.

Here are our two patches.

https://lore.kernel.org/linux-doc/20201030144713.201372-1-pbonz...@redhat.com/T/#t

Re: [PULL v2 00/32] VFIO updates 2020-10-28 (for QEMU 5.2 soft-freeze)

[Peter Maydell]

On Wed, 28 Oct 2020 at 16:42, Alex Williamson
 wrote:
>
> The following changes since commit 33dc9914eac581dea9bdea35dcda4d542531d66a:
>
>   Revert series: virtiofsd: Announce submounts to the guest (2020-10-28 
> 13:17:32 +)
>
> are available in the Git repository at:
>
>   git://github.com/awilliam/qemu-vfio.git tags/vfio-update-20201028.0
>
> for you to fetch changes up to 83d64f2efe383f1f70e180cf1579d3bbe2fbcdf5:
>
>   vfio: fix incorrect print type (2020-10-28 10:30:37 -0600)
>
> 
> VFIO update 2020-10-28
>
>  * Migration support (Kirti Wankhede)
>  * s390 DMA limiting (Matthew Rosato)
>  * zPCI hardware info (Matthew Rosato)
>  * Lock guard (Amey Narkhede)
>  * Print fixes (Zhengui li)
>  * Warning/build fixes
>

Hi; this fails to build on OSX and the BSDs:

../../hw/s390x/s390-pci-vfio.c:13:10: fatal error: 'linux/vfio.h' file not found
#include 
 ^~

fails differently but on the same file on windows builds:

../../hw/s390x/s390-pci-vfio.c:12:23: fatal error: sys/ioctl.h: No
such file or directory

and has this error on 32-bit hosts:

../../hw/vfio/common.c: In function 'vfio_dma_unmap_bitmap':
../../hw/vfio/common.c:414:48: error: passing argument 1 of
'cpu_physical_memory_set_dirty_lebitmap' from incompatible pointer
type [-Werror=incompatible-pointer-types]
 cpu_physical_memory_set_dirty_lebitmap((uint64_t *)bitmap->data,
^
In file included from ../../hw/vfio/common.c:32:0:
/home/peter.maydell/qemu/include/exec/ram_addr.h:337:20: note:
expected 'long unsigned int *' but argument is of type 'uint64_t *
{aka long long unsigned int *}'
 static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned
long *bitmap,
^~
../../hw/vfio/common.c: In function 'vfio_get_dirty_bitmap':
../../hw/vfio/common.c:1008:44: error: passing argument 1 of
'cpu_physical_memory_set_dirty_lebitmap' from incompatible pointer
type [-Werror=incompatible-pointer-types]
 cpu_physical_memory_set_dirty_lebitmap((uint64_t *)range->bitmap.data,
^
In file included from ../../hw/vfio/common.c:32:0:
/home/peter.maydell/qemu/include/exec/ram_addr.h:337:20: note:
expected 'long unsigned int *' but argument is of type 'uint64_t *
{aka long long unsigned int *}'
 static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned
long *bitmap,
^~

thanks
-- PMM

Re: [PULL 00/10] qemu-sparc queue 20201028

[Peter Maydell]

On Wed, 28 Oct 2020 at 08:24, Mark Cave-Ayland
 wrote:
>
> The following changes since commit cfc1105649947f03134294a2448ce2b2e117456f:
>
>   Merge remote-tracking branch 
> 'remotes/philmd-gitlab/tags/acceptance-testing-20201026' into staging 
> (2020-10-27 16:58:39 +)
>
> are available in the Git repository at:
>
>   git://github.com/mcayland/qemu.git tags/qemu-sparc-20201028
>
> for you to fetch changes up to 0980307e705b5677d9b4158a0a0346abf5041f33:
>
>   hw/pci-host/sabre: Simplify code initializing variable once (2020-10-28 
> 07:59:26 +)
>
> 
> qemu-sparc queue
>
> 
> Mark Cave-Ayland (6):
>   sparc32-dma: use object_initialize_child() for espdma and ledma child 
> objects
>   sparc32-ledma: use object_initialize_child() for lance child object
>   sparc32-espdma: use object_initialize_child() for esp child object
>   sparc32-ledma: don't reference nd_table directly within the device
>   sabre: don't call sysbus_mmio_map() in sabre_realize()
>   sabre: increase number of PCI bus IRQs from 32 to 64
>
> Philippe Mathieu-Daudé (4):
>   hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and 
> blitter
>   hw/pci-host/sabre: Update documentation link
>   hw/pci-host/sabre: Remove superfluous address range check
>   hw/pci-host/sabre: Simplify code initializing variable once


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM

Re: [PATCH 0/2] KVM: Introduce ioeventfd read support

[Paolo Bonzini]

On 20/10/20 19:00, Amey Narkhede wrote:
> The first patch updates linux headers to
> add ioeventfd read support while the
> second patch can be used to test the
> ioeventfd read feature with kvm-unit-test
> which reads from specified guest addres.
> Make sure the address provided in
> kvm_set_ioeventfd_read matches with address
> in x86/ioeventfd_read test in kvm-unit-tests.
> 
> Amey Narkhede (2):
>   linux-headers: Add support for reads in ioeventfd
>   kvm: Add ioeventfd read test code
> 
>  accel/kvm/kvm-all.c   | 55 +++
>  linux-headers/linux/kvm.h |  5 +++-
>  2 files changed, 59 insertions(+), 1 deletion(-)
> 

Hi,

in what occasions is this useful?

Paolo

Re: [RFC PATCH-for-5.2] exec: Remove dead code (CID 1432876)

[Paolo Bonzini]

On 30/10/20 16:37, Philippe Mathieu-Daudé wrote:
> We removed the global_locking field in commit 4174495408a,
> leaving dead code around the 'unlocked' variable. Remove it
> to fix the DEADCODE issue reported by Coverity (CID 1432876).
> 
> Fixes: 4174495408a ("exec: Remove MemoryRegion::global_locking field")
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  softmmu/physmem.c | 10 +-
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index a9adedb9f82..0b31be29282 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -2723,22 +2723,14 @@ static int memory_access_size(MemoryRegion *mr, 
> unsigned l, hwaddr addr)
>  
>  static bool prepare_mmio_access(MemoryRegion *mr)
>  {
> -bool unlocked = !qemu_mutex_iothread_locked();
>  bool release_lock = false;
>  
> -if (unlocked) {
> +if (!qemu_mutex_iothread_locked()) {
>  qemu_mutex_lock_iothread();
> -unlocked = false;
>  release_lock = true;
>  }
>  if (mr->flush_coalesced_mmio) {
> -if (unlocked) {
> -qemu_mutex_lock_iothread();
> -}
>  qemu_flush_coalesced_mmio_buffer();
> -if (unlocked) {
> -qemu_mutex_unlock_iothread();
> -}
>  }
>  
>  return release_lock;
> 

Queued, thanks.

Paolo

[Bug 1902365] [NEW] 3x 100% host CPU core usage while virtual machine is in idle

[Germano Massullo]

Public bug reported:

My Fedora 33 machine "top" command shows qemu-system-x86_64 process
using ~300% CPU, that means 3x CPU cores at 100%. Since the virtual
machine (named CentOS 8) is almost in idle (top command inside the VM
shows ~0% CPU usage), there must be something wrong. I attach qemu
process GDB backtrace, and virtual machine libvirt XML

Host details:
libvirt-6.6.0-2.fc33.x86_64
qemu-system-x86-5.1.0-5.fc33.x86_64
virt-manager-3.1.0-1.fc33.noarch
kernel 5.8.16-300.fc33.x86_64
CPU: AMD Ryzen 5 3600

# gdb qemu-system-x86_64 405756
GNU gdb (GDB) Fedora 9.2-7.fc33
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
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-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
.
Find the GDB manual and other documentation resources online at:
.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from qemu-system-x86_64...
Reading symbols from .gnu_debugdata for /usr/bin/qemu-system-x86_64...
(No debugging symbols found in .gnu_debugdata for /usr/bin/qemu-system-x86_64)
Attaching to program: /usr/bin/qemu-system-x86_64, process 405756
[New LWP 405788]
[New LWP 405798]
[New LWP 405799]
[New LWP 405800]
[New LWP 405801]
[New LWP 405802]
[New LWP 405804]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
0x7f549d0bdb0e in ppoll () from target:/lib64/libc.so.6
(gdb) set height 0
(gdb) set print elements 0
(gdb) set print frame-arguments all
(gdb) thread apply all backtrace

Thread 8 (Thread 0x7f53837ff640 (LWP 405804)):
#0  0x7f549d0bda0f in poll () from target:/lib64/libc.so.6
#1  0x7f549e4c2d1e in g_main_context_iterate.constprop () from 
target:/lib64/libglib-2.0.so.0
#2  0x7f549e4716ab in g_main_loop_run () from target:/lib64/libglib-2.0.so.0
#3  0x7f549dcfcc66 in red_worker_main.lto_priv () from 
target:/lib64/libspice-server.so.1
#4  0x7f549d19c3f9 in start_thread () from target:/lib64/libpthread.so.0
#5  0x7f549d0c8b03 in clone () from target:/lib64/libc.so.6

Thread 7 (Thread 0x7f5390dfd640 (LWP 405802)):
#0  0x7f549d0bf58b in ioctl () from target:/lib64/libc.so.6
#1  0x55a60728ec87 in kvm_vcpu_ioctl ()
#2  0x55a60728edc1 in kvm_cpu_exec ()
#3  0x55a60734dc04 in qemu_kvm_cpu_thread_fn ()
#4  0x55a6076dc0ff in qemu_thread_start ()
#5  0x7f549d19c3f9 in start_thread () from target:/lib64/libpthread.so.0
#6  0x7f549d0c8b03 in clone () from target:/lib64/libc.so.6

Thread 6 (Thread 0x7f53915fe640 (LWP 405801)):
#0  0x7f549d0bf58b in ioctl () from target:/lib64/libc.so.6
#1  0x55a60728ec87 in kvm_vcpu_ioctl ()
#2  0x55a60728edc1 in kvm_cpu_exec ()
#3  0x55a60734dc04 in qemu_kvm_cpu_thread_fn ()
#4  0x55a6076dc0ff in qemu_thread_start ()
#5  0x7f549d19c3f9 in start_thread () from target:/lib64/libpthread.so.0
#6  0x7f549d0c8b03 in clone () from target:/lib64/libc.so.6

Thread 5 (Thread 0x7f5391dff640 (LWP 405800)):
#0  0x7f549d0bf58b in ioctl () from target:/lib64/libc.so.6
#1  0x55a60728ec87 in kvm_vcpu_ioctl ()
#2  0x55a60728edc1 in kvm_cpu_exec ()
#3  0x55a60734dc04 in qemu_kvm_cpu_thread_fn ()
#4  0x55a6076dc0ff in qemu_thread_start ()
#5  0x7f549d19c3f9 in start_thread () from target:/lib64/libpthread.so.0
#6  0x7f549d0c8b03 in clone () from target:/lib64/libc.so.6

Thread 4 (Thread 0x7f54988b7640 (LWP 405799)):
#0  0x7f549d0bf58b in ioctl () from target:/lib64/libc.so.6
#1  0x55a60728ec87 in kvm_vcpu_ioctl ()
#2  0x55a60728edc1 in kvm_cpu_exec ()
#3  0x55a60734dc04 in qemu_kvm_cpu_thread_fn ()
#4  0x55a6076dc0ff in qemu_thread_start ()
#5  0x7f549d19c3f9 in start_thread () from target:/lib64/libpthread.so.0
#6  0x7f549d0c8b03 in clone () from target:/lib64/libc.so.6

Thread 3 (Thread 0x7f549917b640 (LWP 405798)):
#0  0x7f549d0bda0f in poll () from target:/lib64/libc.so.6
#1  0x7f549e4c2d1e in g_main_context_iterate.constprop () from 
target:/lib64/libglib-2.0.so.0
#2  0x7f549e4716ab in g_main_loop_run () from target:/lib64/libglib-2.0.so.0
#3  0x55a6073c4c81 in iothread_run ()
#4  0x55a6076dc0ff in qemu_thread_start ()
#5  0x7f549d19c3f9 in start_thread () from target:/lib64/libpthread.so.0
#6  0x7f549d0c8b03 in clone () from target:/lib64/libc.so.6

Thread 2 (Thread 0x7f549b93a640 (LWP 405788)):
#0  0x7f549d0c350d in syscall () from target:/lib64/libc.so.6
#1  0x55a6076dce9a in qemu_event_wait ()
#2  0x55a6076e56ca in call_rcu_thread ()
#3  0x55a6076dc0ff in qemu_thread_start ()
#4  0x000

Re: [PATCH 2/2] block: assert that permission commit sets same permissions

[Vladimir Sementsov-Ogievskiy]

31.10.2020 15:35, Vladimir Sementsov-Ogievskiy wrote:

On permission update commit we must set same permissions as on _check_.
Let's add assertions. Next step may be to drop permission parameters
from _set_.

Note that prior to previous commit, fixing bdrv_drop_intermediate(),
new assertion in bdrv_child_set_perm() crashes on iotests 30 and 40.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  block.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index bd9f4e534b..0f4da59a6c 100644
--- a/block.c
+++ b/block.c
@@ -2105,9 +2105,10 @@ static void bdrv_abort_perm_update(BlockDriverState *bs)
  }
  }
  
-static void bdrv_set_perm(BlockDriverState *bs, uint64_t cumulative_perms,

-  uint64_t cumulative_shared_perms)
+static void bdrv_set_perm(BlockDriverState *bs, uint64_t _cumulative_perms,
+  uint64_t _cumulative_shared_perms)
  {
+uint64_t cumulative_perms, cumulative_shared_perms;
  BlockDriver *drv = bs->drv;
  BdrvChild *c;
  
@@ -2115,6 +2116,10 @@ static void bdrv_set_perm(BlockDriverState *bs, uint64_t cumulative_perms,

  return;
  }
  
+bdrv_get_cumulative_perm(bs, &cumulative_perms, &cumulative_shared_perms);

+assert(_cumulative_perms == cumulative_perms);
+assert(_cumulative_shared_perms == cumulative_shared_perms);
+
  /* Update this node */
  if (drv->bdrv_set_perm) {
  drv->bdrv_set_perm(bs, cumulative_perms, cumulative_shared_perms);
@@ -2301,6 +2306,8 @@ static void bdrv_child_set_perm(BdrvChild *c, uint64_t 
perm, uint64_t shared)
  
  c->has_backup_perm = false;
  
+assert(c->perm == perm);

+assert(c->shared_perm == shared);
  c->perm = perm;
  c->shared_perm = shared;
  



squash:

@@ -2308,8 +2308,6 @@ static void bdrv_child_set_perm(BdrvChild *c, uint64_t 
perm, uint64_t shared)
 
 assert(c->perm == perm);

 assert(c->shared_perm == shared);
-c->perm = perm;
-c->shared_perm = shared;
 
 bdrv_get_cumulative_perm(c->bs, &cumulative_perms,

  &cumulative_shared_perms);



--
Best regards,
Vladimir

Re: [PULL v2 01/16] tests/9pfs: fix test dir for parallel tests

[Christian Schoenebeck]

On Freitag, 30. Oktober 2020 13:07:03 CET Christian Schoenebeck wrote:
> Use mkdtemp() to generate a unique directory for the 9p 'local' tests.
> 
> This fixes occasional 9p test failures when running 'make check -jN' if
> QEMU was compiled for multiple target architectures, because the individual
> architecture's test suites would run in parallel and interfere with each
> other's data as the test directory was previously hard coded and hence the
> same directory was used by all of them simultaniously.
> 
> This also requires a change how the test directory is created and deleted:
> As the test path is now randomized and virtio_9p_register_nodes() being
> called in a somewhat undeterministic way, that's no longer an appropriate
> place to create and remove the test directory. Use a constructor and
> destructor function for creating and removing the test directory instead.
> Unfortunately libqos currently does not support setup/teardown callbacks
> to handle this more cleanly.

Peter, please ignore this PR. This patch needs rework:

ERROR:../tests/qtest/test-x86-cpuid-compat.c:208:test_plus_minus: stdout of
child process (/x86/cpuid/parsing-plus-minus/subprocess [34856]) failed to
match: 

stdout was:

# mkdir('/home/travis/build/cschoenebeck/qemu/build/qtest-9p-local-PwY2nQ')
failed: File exists

ERROR qtest-x86_64/test-x86-cpuid-compat - Bail out! ERROR:../tests/qtest/
test-x86-cpuid-compat.c:208:test_plus_minus: stdout of child process (/x86/
cpuid/parsing-plus-minus/subprocess [34856]) failed to match:

make: *** [Makefile.mtest:1793: run-test-222] Error 1

https://travis-ci.org/github/cschoenebeck/qemu/jobs/740199494

> 
> Signed-off-by: Christian Schoenebeck 
> Tested-by: Greg Kurz 
> Reviewed-by: Greg Kurz 
> Message-Id:
>  .com> Signed-off-by: Christian Schoenebeck 
> ---
>  tests/qtest/libqos/virtio-9p.c | 25 +++--
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qtest/libqos/virtio-9p.c b/tests/qtest/libqos/virtio-9p.c
> index d43647b3b7..6b22fa0e9a 100644
> --- a/tests/qtest/libqos/virtio-9p.c
> +++ b/tests/qtest/libqos/virtio-9p.c
> @@ -35,7 +35,12 @@ static char *concat_path(const char* a, const char* b)
>  static void init_local_test_path(void)
>  {
>  char *pwd = g_get_current_dir();
> -local_test_path = concat_path(pwd, "qtest-9p-local");
> +char *template = concat_path(pwd, "qtest-9p-local-XX");
> +local_test_path = mkdtemp(template);
> +if (!local_test_path) {
> +g_test_message("mkdtemp('%s') failed: %s", template,
> strerror(errno)); +}
> +g_assert(local_test_path);
>  g_free(pwd);
>  }
> 
> @@ -246,11 +251,6 @@ static void virtio_9p_register_nodes(void)
>  const char *str_simple = "fsdev=fsdev0,mount_tag=" MOUNT_TAG;
>  const char *str_addr = "fsdev=fsdev0,addr=04.0,mount_tag=" MOUNT_TAG;
> 
> -/* make sure test dir for the 'local' tests exists and is clean */
> -init_local_test_path();
> -remove_local_test_dir();
> -create_local_test_dir();
> -
>  QPCIAddress addr = {
>  .devfn = QPCI_DEVFN(4, 0),
>  };
> @@ -278,3 +278,16 @@ static void virtio_9p_register_nodes(void)
>  }
> 
>  libqos_init(virtio_9p_register_nodes);
> +
> +static void __attribute__((constructor)) construct_virtio_9p(void)
> +{
> +/* make sure test dir for the 'local' tests exists */
> +init_local_test_path();
> +create_local_test_dir();
> +}

I'm not sure yet what happens there exactly. One problem that I can see is 
that this constructor function is executed before main() is entered, it 
generates a random test path as global variable, creates that test directory, 
then main() is entered and qost-test.c might now call g_test_trap_subprocess() 
(depending on the config) which would fork the process for the individual test 
cases, probably ending up with a shared test dir path unintentionally. So this 
constructor solution is probably wrong anyway.

But that does not explain what I'm seeing in above's CI error: that error 
message suggests that create_local_test_dir() was called in parallel with the 
same test path. A GCC constructor function being called again on fork() is not 
an expected behaviour to me, but assuming it does on some system, the address 
space should have deviated at this point and hence each subprocess should end 
up with overwriting the global test path variable with their own test 
directory path in this case.

It's too late for touching libqos for this, so I hope we can find a temporary 
workaround for this problem for 5.2 release. For instance by using a TLS 
(__thread) variable instead of a regular global variable for the test path, 
but I would still like to understand the precise (mis)behaviour observed 
there.

> +
> +static void __attribute__((destructor)) destruct_virtio_9p(void)
> +{
> +/* remove previously created test dir when test suite completed */
> +remove_local_test_dir();
> +}

[PATCH 2/2] block: assert that permission commit sets same permissions

[Vladimir Sementsov-Ogievskiy]

On permission update commit we must set same permissions as on _check_.
Let's add assertions. Next step may be to drop permission parameters
from _set_.

Note that prior to previous commit, fixing bdrv_drop_intermediate(),
new assertion in bdrv_child_set_perm() crashes on iotests 30 and 40.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index bd9f4e534b..0f4da59a6c 100644
--- a/block.c
+++ b/block.c
@@ -2105,9 +2105,10 @@ static void bdrv_abort_perm_update(BlockDriverState *bs)
 }
 }
 
-static void bdrv_set_perm(BlockDriverState *bs, uint64_t cumulative_perms,
-  uint64_t cumulative_shared_perms)
+static void bdrv_set_perm(BlockDriverState *bs, uint64_t _cumulative_perms,
+  uint64_t _cumulative_shared_perms)
 {
+uint64_t cumulative_perms, cumulative_shared_perms;
 BlockDriver *drv = bs->drv;
 BdrvChild *c;
 
@@ -2115,6 +2116,10 @@ static void bdrv_set_perm(BlockDriverState *bs, uint64_t 
cumulative_perms,
 return;
 }
 
+bdrv_get_cumulative_perm(bs, &cumulative_perms, &cumulative_shared_perms);
+assert(_cumulative_perms == cumulative_perms);
+assert(_cumulative_shared_perms == cumulative_shared_perms);
+
 /* Update this node */
 if (drv->bdrv_set_perm) {
 drv->bdrv_set_perm(bs, cumulative_perms, cumulative_shared_perms);
@@ -2301,6 +2306,8 @@ static void bdrv_child_set_perm(BdrvChild *c, uint64_t 
perm, uint64_t shared)
 
 c->has_backup_perm = false;
 
+assert(c->perm == perm);
+assert(c->shared_perm == shared);
 c->perm = perm;
 c->shared_perm = shared;
 
-- 
2.21.3

[PATCH 1/2] block: make bdrv_drop_intermediate() less wrong

[Vladimir Sementsov-Ogievskiy]

First, permission update loop tries to do iterations transactionally,
but the whole update is not transactional: nobody roll-back successful
loop iterations when some iteration fails.

Second, in the iteration we have nested permission update:
c->klass->update_filename may point to bdrv_child_cb_update_filename()
which calls bdrv_backing_update_filename(), which may do node reopen to
RW.

Permission update system is not prepared to nested updates, at least it
has intermediate permission-update state stored in BdrvChild
structures: has_backup_perm, backup_perm and backup_shared_perm.

So, let's first do bdrv_replace_node() (which is more transactional
than open-coded update in bdrv_drop_intermediate()) and then call
update_filename() in separate. We still do not rollback changes in case
of update_filename() failure but it's not much worse than pre-patch
behavior.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block.c | 36 +++-
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/block.c b/block.c
index 9538af4884..bd9f4e534b 100644
--- a/block.c
+++ b/block.c
@@ -4958,36 +4958,30 @@ int bdrv_drop_intermediate(BlockDriverState *top, 
BlockDriverState *base,
 backing_file_str = base->filename;
 }
 
-QLIST_FOREACH_SAFE(c, &top->parents, next_parent, next) {
-/* Check whether we are allowed to switch c from top to base */
-GSList *ignore_children = g_slist_prepend(NULL, c);
-ret = bdrv_check_update_perm(base, NULL, c->perm, c->shared_perm,
- ignore_children, NULL, &local_err);
-g_slist_free(ignore_children);
-if (ret < 0) {
-error_report_err(local_err);
-goto exit;
-}
+bdrv_replace_node(top, base, &local_err);
+if (local_err) {
+error_report_err(local_err);
+goto exit;
+}
 
-/* If so, update the backing file path in the image file */
+QLIST_FOREACH_SAFE(c, &base->parents, next_parent, next) {
 if (c->klass->update_filename) {
 ret = c->klass->update_filename(c, base, backing_file_str,
 &local_err);
 if (ret < 0) {
-bdrv_abort_perm_update(base);
+/*
+ * TODO: Actually, we want to rollback all previous iterations
+ * of this loop, and (which is almost impossible) previous
+ * bdrv_replace_node()...
+ *
+ * Note, that c->klass->update_filename may lead to permission
+ * update, so it's a bad idea to call it inside permission
+ * update transaction of bdrv_replace_node.
+ */
 error_report_err(local_err);
 goto exit;
 }
 }
-
-/*
- * Do the actual switch in the in-memory graph.
- * Completes bdrv_check_update_perm() transaction internally.
- * c->frozen is false, we have checked that above.
- */
-bdrv_ref(base);
-bdrv_replace_child(c, base);
-bdrv_unref(top);
 }
 
 if (update_inherits_from) {
-- 
2.21.3

[PATCH 0/2] Fix nested permission update

[Vladimir Sementsov-Ogievskiy]

Hi! With help of some assertions (patch 2) I've found that
bdrv_drop_intermediate() do nested permission update which in my opinion
may lead to unpredictable behavior.

Vladimir Sementsov-Ogievskiy (2):
  block: make bdrv_drop_intermediate() less wrong
  block: assert that permission commit sets same permissions

 block.c | 47 ---
 1 file changed, 24 insertions(+), 23 deletions(-)

-- 
2.21.3

[PATCH] util: Remove redundant checks in the openpty()

[AlexChen]

As we can see from the following function call stack, the amaster and the aslave
cannot be NULL: char_pty_open() -> qemu_openpty_raw() -> openpty().
In addition, the amaster and the aslave has been dereferenced at the beginning
of the openpty(). So the checks on amaster and aslave in the openpty() are 
redundant.

Reported-by: Euler Robot 
Signed-off-by: Alex Chen 
---
 util/qemu-openpty.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
index eb17f5b0bc..427f43a769 100644
--- a/util/qemu-openpty.c
+++ b/util/qemu-openpty.c
@@ -80,10 +80,9 @@ static int openpty(int *amaster, int *aslave, char *name,
 (termp != NULL && tcgetattr(sfd, termp) < 0))
 goto err;

-if (amaster)
-*amaster = mfd;
-if (aslave)
-*aslave = sfd;
+*amaster = mfd;
+*aslave = sfd;
+
 if (winp)
 ioctl(sfd, TIOCSWINSZ, winp);

-- 
2.19.1

Re: --enable-xen on gitlab CI? (was Re: [PATCH 09/36] qdev: Make qdev_get_prop_ptr() get Object* arg)

[Thomas Huth]

On 30/10/2020 18.13, Paolo Bonzini wrote:
> On 30/10/20 12:35, Eduardo Habkost wrote:
>>
>> What is necessary to make sure we have a CONFIG_XEN=y job in
>> gitlab CI?  Maybe just including xen-devel in some of the
>> container images is enough?
> 
> Fedora already has it, but build-system-fedora does not include
> x86_64-softmmu.

Eduardo, could you try to add xen-devel to the centos8 container? If that
does not work, we can still move the x86_64-softmmu target to the fedora
pipeline instead.

 Thomas

Re: [PULL 0/3] tcg patch queue

[Peter Maydell]

On Tue, 27 Oct 2020 at 16:51, Richard Henderson
 wrote:
>
> The following changes since commit 4a74626970ab4ea475263d155b10fb75c9af0b33:
>
>   Merge remote-tracking branch 
> 'remotes/stefanha-gitlab/tags/tracing-pull-request' into staging (2020-10-27 
> 11:28:46 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20201027
>
> for you to fetch changes up to 1d705e8a5bbfe36294081baa45ab68a9ad987f33:
>
>   accel/tcg: Add CPU_LOG_EXEC tracing for cpu_io_recompile() (2020-10-27 
> 09:48:07 -0700)
>
> 
> Optimize across branches.
> Add logging for cpu_io_recompile.
>
> 
> Peter Maydell (1):
>   accel/tcg: Add CPU_LOG_EXEC tracing for cpu_io_recompile()
>
> Richard Henderson (2):
>   tcg: Do not kill globals at conditional branches
>   tcg/optimize: Flush data at labels not TCG_OPF_BB_END


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM

Re: [PATCH] hw/display/exynos4210_fimd: Fix potential NULL pointer dereference

[Peter Maydell]

On Sat, 31 Oct 2020 at 02:57, AlexChen  wrote:
>
> On 2020/10/30 22:28, Peter Maydell wrote:
> > On Fri, 30 Oct 2020 at 10:23, AlexChen  wrote:
> >>
> >> In exynos4210_fimd_update(), the pointer s is dereferenced before
> >> being check if it is valid, which may lead to NULL pointer dereference.
> >> So move the assignment to global_width after checking that the s is valid
> >>
> >> Reported-by: Euler Robot 
> >> Signed-off-by: Alex Chen 
> >> ---
> >>  hw/display/exynos4210_fimd.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
> >> index 4c16e1f5a0..a1179d2f89 100644
> >> --- a/hw/display/exynos4210_fimd.c
> >> +++ b/hw/display/exynos4210_fimd.c
> >> @@ -1275,12 +1275,12 @@ static void exynos4210_fimd_update(void *opaque)
> >>  bool blend = false;
> >>  uint8_t *host_fb_addr;
> >>  bool is_dirty = false;
> >> -const int global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 
> >> 1;
> >>
> >>  if (!s || !s->console || !s->enabled ||
> >>  surface_bits_per_pixel(qemu_console_surface(s->console)) == 0) {
> >>  return;
> >>  }
> >> +const int global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 
> >> 1;
> >>  exynos4210_update_resolution(s);
> >>  surface = qemu_console_surface(s->console);
> >
> > Yep, this is a bug, but QEMU's coding style doesn't permit
> > variable declarations in the middle of functions. You need
> > to split the declaration (which stays where it is) and the
> > initialization (which can moved down below the !s test.
> >
> Thans for your review. I have also considered this modification to be 
> incompatible with
> the QEMU's coding style. But the type of global_width is const int which 
> cannot be
> assigned once they are defined.
> Could we define the global_width as int, such as this modification:
>
> diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
> index 4c16e1f5a0..34a960a976 100644
> --- a/hw/display/exynos4210_fimd.c
> +++ b/hw/display/exynos4210_fimd.c
> @@ -1275,12 +1275,14 @@ static void exynos4210_fimd_update(void *opaque)
>  bool blend = false;
>  uint8_t *host_fb_addr;
>  bool is_dirty = false;
> -const int global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;
> +int global_width;
>
>  if (!s || !s->console || !s->enabled ||
>  surface_bits_per_pixel(qemu_console_surface(s->console)) == 0) {
>  return;
>  }
> +
> +global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;
>  exynos4210_update_resolution(s);
>  surface = qemu_console_surface(s->console);

Yes, I think that would be fine.

thanks
-- PMM

Re: [PATCH 33/36] tests: Use static properties at check-qom-proplist test case

[Marc-André Lureau]

On Fri, Oct 30, 2020 at 2:28 AM Eduardo Habkost  wrote:

> Use static properties for the bool and string properties used at
> check-qom-proplist.
>
> Signed-off-by: Eduardo Habkost 
> ---
> Cc: Paolo Bonzini 
> Cc: "Daniel P. Berrangé" 
> Cc: Eduardo Habkost 
> Cc: qemu-devel@nongnu.org
> ---
>  tests/check-qom-proplist.c | 61 +-
>  1 file changed, 8 insertions(+), 53 deletions(-)
>
> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> index 1b76581980..94ad6631c0 100644
> --- a/tests/check-qom-proplist.c
> +++ b/tests/check-qom-proplist.c
> @@ -26,6 +26,8 @@
>  #include "qemu/option.h"
>  #include "qemu/config-file.h"
>  #include "qom/object_interfaces.h"
> +#include "qom/static-property.h"
> +#include "qom/static-property-internal.h"
>

>
>  #define TYPE_DUMMY "qemu-dummy"
> @@ -68,24 +70,6 @@ struct DummyObjectClass {
>  };
>
>
> -static void dummy_set_bv(Object *obj,
> - bool value,
> - Error **errp)
> -{
> -DummyObject *dobj = DUMMY_OBJECT(obj);
> -
> -dobj->bv = value;
> -}
> -
> -static bool dummy_get_bv(Object *obj,
> - Error **errp)
> -{
> -DummyObject *dobj = DUMMY_OBJECT(obj);
> -
> -return dobj->bv;
> -}
> -
> -
>  static void dummy_set_av(Object *obj,
>   int value,
>   Error **errp)
> @@ -103,39 +87,20 @@ static int dummy_get_av(Object *obj,
>  return dobj->av;
>  }
>
> +static Property bv_prop =
> +DEFINE_PROP_BOOL("bv", DummyObject, bv, false);
>
> -static void dummy_set_sv(Object *obj,
> - const char *value,
> - Error **errp)
> -{
> -DummyObject *dobj = DUMMY_OBJECT(obj);
> -
> -g_free(dobj->sv);
> -dobj->sv = g_strdup(value);
> -}
> -
> -static char *dummy_get_sv(Object *obj,
> -  Error **errp)
> -{
> -DummyObject *dobj = DUMMY_OBJECT(obj);
> -
> -return g_strdup(dobj->sv);
> -}
> -
> +static Property sv_prop =
> +DEFINE_PROP_STRING("sv", DummyObject, sv);
>
>  static void dummy_init(Object *obj)
>  {
> -object_property_add_bool(obj, "bv",
> - dummy_get_bv,
> - dummy_set_bv);
> +object_property_add_static(obj, &bv_prop, NULL);
>

Ok for testing internal functions.. hopefully it won't serve as an example!

 }
>
> -
>  static void dummy_class_init(ObjectClass *cls, void *data)
>  {
> -object_class_property_add_str(cls, "sv",
> -  dummy_get_sv,
> -  dummy_set_sv);
> +object_class_property_add_static(cls, &sv_prop, NULL);
>  object_class_property_add_enum(cls, "av",
> "DummyAnimal",
> &dummy_animal_map,
> @@ -143,21 +108,11 @@ static void dummy_class_init(ObjectClass *cls, void
> *data)
> dummy_set_av);
>  }
>
> -
> -static void dummy_finalize(Object *obj)
> -{
> -DummyObject *dobj = DUMMY_OBJECT(obj);
> -
> -g_free(dobj->sv);
> -}
> -
> -
>  static const TypeInfo dummy_info = {
>  .name  = TYPE_DUMMY,
>  .parent= TYPE_OBJECT,
>  .instance_size = sizeof(DummyObject),
>  .instance_init = dummy_init,
> -.instance_finalize = dummy_finalize,
>  .class_size = sizeof(DummyObjectClass),
>  .class_init = dummy_class_init,
>  .interfaces = (InterfaceInfo[]) {
> --
> 2.28.0
>

Reviewed-by: Marc-André Lureau 

-- 
Marc-André Lureau

Re: [PATCH 32/36] qdev: Move base property types to qom/property-types.c

[Marc-André Lureau]

On Fri, Oct 30, 2020 at 2:26 AM Eduardo Habkost  wrote:

> Move all property types from qdev-properties.c to
> qom/property-types.c.
>
> Signed-off-by: Eduardo Habkost 
>

Reviewed-by: Marc-André Lureau 

-- 
Marc-André Lureau