From b9afaba891458d3e1f3f853a8cb6880498ed34e3 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 27 Mar 2017 17:50:05 +0100 Subject: [PATCH 1/6] iscsi: drop unused IscsiAIOCB.qiov field The IscsiAIOCB.qiov field has been unused since commit 063c3378a9e3c25cc0afac3c72e4823d0621e352 ("block/iscsi: introduce bdrv_co_{readv, writev, flush_to_disk}") back in 2013. Signed-off-by: Stefan Hajnoczi Message-Id: <20170327165005.22038-1-stefanha@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Paolo Bonzini --- block/iscsi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/block/iscsi.c b/block/iscsi.c index 75d890538e..716e74abba 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -103,7 +103,6 @@ typedef struct IscsiTask { typedef struct IscsiAIOCB { BlockAIOCB common; - QEMUIOVector *qiov; QEMUBH *bh; IscsiLun *iscsilun; struct scsi_task *task; From c7f15bc93661a36fec9fd0107654cf44d06c5992 Mon Sep 17 00:00:00 2001 From: Tejaswini Poluri Date: Tue, 28 Mar 2017 12:49:43 +0530 Subject: [PATCH 2/6] target-i386: fix "info lapic" segfault on isapc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Start QEMU with "qemu-system-x86_64 -nographic -M isapc -serial none-monitor stdio" and enter "info lapic" at the monitor prompt ⇒ Segmentation fault Signed-off-by: Tejaswini Poluri Message-Id: <1490685583-16987-1-git-send-email-tejaswinipoluri3@gmail.com> Signed-off-by: Paolo Bonzini --- target/i386/helper.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/target/i386/helper.c b/target/i386/helper.c index e2af3404f2..f11cac63a1 100644 --- a/target/i386/helper.c +++ b/target/i386/helper.c @@ -326,6 +326,10 @@ void x86_cpu_dump_local_apic_state(CPUState *cs, FILE *f, { X86CPU *cpu = X86_CPU(cs); APICCommonState *s = APIC_COMMON(cpu->apic_state); + if (!s) { + cpu_fprintf(f, "local apic state not available\n"); + return; + } uint32_t *lvt = s->lvt; cpu_fprintf(f, "dumping local APIC state for CPU %-2u\n\n", From cb9a05a4f169347f85e345a2abe42c45e1d3157b Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 30 Mar 2017 12:28:12 -0500 Subject: [PATCH 3/6] ipmi: Fix macro issues Macro parameters should almost always have () around them when used. llvm reported an error on this. Remove redundant parenthesis and put parenthesis around the entire macros with assignments in case they are used in an expression. Remove some unused macros. Reported in https://bugs.launchpad.net/bugs/1651167 Signed-off-by: Corey Minyard Reviewed-by: Eric Blake Message-Id: <1490894892-8055-1-git-send-email-minyard@acm.org> Signed-off-by: Paolo Bonzini --- hw/ipmi/isa_ipmi_bt.c | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c index 1c69cb33f8..2fcc3d2e7c 100644 --- a/hw/ipmi/isa_ipmi_bt.c +++ b/hw/ipmi/isa_ipmi_bt.c @@ -37,40 +37,30 @@ #define IPMI_BT_HBUSY_BIT 6 #define IPMI_BT_BBUSY_BIT 7 -#define IPMI_BT_CLR_WR_MASK (1 << IPMI_BT_CLR_WR_BIT) #define IPMI_BT_GET_CLR_WR(d) (((d) >> IPMI_BT_CLR_WR_BIT) & 0x1) -#define IPMI_BT_SET_CLR_WR(d, v) (d) = (((d) & ~IPMI_BT_CLR_WR_MASK) | \ - (((v & 1) << IPMI_BT_CLR_WR_BIT))) -#define IPMI_BT_CLR_RD_MASK (1 << IPMI_BT_CLR_RD_BIT) #define IPMI_BT_GET_CLR_RD(d) (((d) >> IPMI_BT_CLR_RD_BIT) & 0x1) -#define IPMI_BT_SET_CLR_RD(d, v) (d) = (((d) & ~IPMI_BT_CLR_RD_MASK) | \ - (((v & 1) << IPMI_BT_CLR_RD_BIT))) -#define IPMI_BT_H2B_ATN_MASK (1 << IPMI_BT_H2B_ATN_BIT) #define IPMI_BT_GET_H2B_ATN(d) (((d) >> IPMI_BT_H2B_ATN_BIT) & 0x1) -#define IPMI_BT_SET_H2B_ATN(d, v) (d) = (((d) & ~IPMI_BT_H2B_ATN_MASK) | \ - (((v & 1) << IPMI_BT_H2B_ATN_BIT))) #define IPMI_BT_B2H_ATN_MASK (1 << IPMI_BT_B2H_ATN_BIT) #define IPMI_BT_GET_B2H_ATN(d) (((d) >> IPMI_BT_B2H_ATN_BIT) & 0x1) -#define IPMI_BT_SET_B2H_ATN(d, v) (d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ - (((v & 1) << IPMI_BT_B2H_ATN_BIT))) +#define IPMI_BT_SET_B2H_ATN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ + (((v) & 1) << IPMI_BT_B2H_ATN_BIT))) #define IPMI_BT_SMS_ATN_MASK (1 << IPMI_BT_SMS_ATN_BIT) #define IPMI_BT_GET_SMS_ATN(d) (((d) >> IPMI_BT_SMS_ATN_BIT) & 0x1) -#define IPMI_BT_SET_SMS_ATN(d, v) (d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ - (((v & 1) << IPMI_BT_SMS_ATN_BIT))) +#define IPMI_BT_SET_SMS_ATN(d, v) ((d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ + (((v) & 1) << IPMI_BT_SMS_ATN_BIT))) #define IPMI_BT_HBUSY_MASK (1 << IPMI_BT_HBUSY_BIT) #define IPMI_BT_GET_HBUSY(d) (((d) >> IPMI_BT_HBUSY_BIT) & 0x1) -#define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ - (((v & 1) << IPMI_BT_HBUSY_BIT))) +#define IPMI_BT_SET_HBUSY(d, v) ((d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ + (((v) & 1) << IPMI_BT_HBUSY_BIT))) #define IPMI_BT_BBUSY_MASK (1 << IPMI_BT_BBUSY_BIT) -#define IPMI_BT_GET_BBUSY(d) (((d) >> IPMI_BT_BBUSY_BIT) & 0x1) -#define IPMI_BT_SET_BBUSY(d, v) (d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ - (((v & 1) << IPMI_BT_BBUSY_BIT))) +#define IPMI_BT_SET_BBUSY(d, v) ((d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ + (((v) & 1) << IPMI_BT_BBUSY_BIT))) /* Mask register */ @@ -79,13 +69,13 @@ #define IPMI_BT_B2H_IRQ_EN_MASK (1 << IPMI_BT_B2H_IRQ_EN_BIT) #define IPMI_BT_GET_B2H_IRQ_EN(d) (((d) >> IPMI_BT_B2H_IRQ_EN_BIT) & 0x1) -#define IPMI_BT_SET_B2H_IRQ_EN(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) | \ - (((v & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) +#define IPMI_BT_SET_B2H_IRQ_EN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) |\ + (((v) & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) #define IPMI_BT_B2H_IRQ_MASK (1 << IPMI_BT_B2H_IRQ_BIT) #define IPMI_BT_GET_B2H_IRQ(d) (((d) >> IPMI_BT_B2H_IRQ_BIT) & 0x1) -#define IPMI_BT_SET_B2H_IRQ(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ - (((v & 1) << IPMI_BT_B2H_IRQ_BIT))) +#define IPMI_BT_SET_B2H_IRQ(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ + (((v) & 1) << IPMI_BT_B2H_IRQ_BIT))) typedef struct IPMIBT { IPMIBmc *bmc; From cc1e13913916f755fd2dc5041b8d4bf25d3ea88e Mon Sep 17 00:00:00 2001 From: yaolujing Date: Sat, 1 Apr 2017 08:15:09 +0800 Subject: [PATCH 4/6] nbd: fix memory leak on socket_connect failed When TCP connection fails between nbd server and client, the local var, sioc, memory leak. This patch fixes the memory leak. Signed-off-by: yaolujing Reviewed-by: Eric Blake Message-Id: <1491005709-29989-1-git-send-email-yaolujing@huawei.com> Signed-off-by: Paolo Bonzini --- block/nbd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/nbd.c b/block/nbd.c index 1b832c2132..3150712f0c 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -311,6 +311,7 @@ static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr, saddr, &local_err); if (local_err) { + object_unref(OBJECT(sioc)); error_propagate(errp, local_err); return NULL; } From 90c4fe5fc517a045e7a7cf2f23472e114042ca29 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 3 Apr 2017 13:41:28 +0200 Subject: [PATCH 5/6] exec: revert MemoryRegionCache MemoryRegionCache did not know about virtio support for IOMMUs (because the two features were developed at the same time). Revert MemoryRegionCache to "normal" address_space_* operations for 2.9, as it is simpler than undoing the virtio patches. Signed-off-by: Paolo Bonzini --- exec.c | 64 ++++++++----------------------------------- include/exec/memory.h | 10 +++---- 2 files changed, 15 insertions(+), 59 deletions(-) diff --git a/exec.c b/exec.c index e57a8a2178..c97ef4a8da 100644 --- a/exec.c +++ b/exec.c @@ -3236,75 +3236,33 @@ int64_t address_space_cache_init(MemoryRegionCache *cache, hwaddr len, bool is_write) { - hwaddr l, xlat; - MemoryRegion *mr; - void *ptr; - - assert(len > 0); - - l = len; - mr = address_space_translate(as, addr, &xlat, &l, is_write); - if (!memory_access_is_direct(mr, is_write)) { - return -EINVAL; - } - - l = address_space_extend_translation(as, addr, len, mr, xlat, l, is_write); - ptr = qemu_ram_ptr_length(mr->ram_block, xlat, &l); - - cache->xlat = xlat; - cache->is_write = is_write; - cache->mr = mr; - cache->ptr = ptr; - cache->len = l; - memory_region_ref(cache->mr); - - return l; + cache->len = len; + cache->as = as; + cache->xlat = addr; + return len; } void address_space_cache_invalidate(MemoryRegionCache *cache, hwaddr addr, hwaddr access_len) { - assert(cache->is_write); - invalidate_and_set_dirty(cache->mr, addr + cache->xlat, access_len); } void address_space_cache_destroy(MemoryRegionCache *cache) { - if (!cache->mr) { - return; - } - - if (xen_enabled()) { - xen_invalidate_map_cache_entry(cache->ptr); - } - memory_region_unref(cache->mr); - cache->mr = NULL; -} - -/* Called from RCU critical section. This function has the same - * semantics as address_space_translate, but it only works on a - * predefined range of a MemoryRegion that was mapped with - * address_space_cache_init. - */ -static inline MemoryRegion *address_space_translate_cached( - MemoryRegionCache *cache, hwaddr addr, hwaddr *xlat, - hwaddr *plen, bool is_write) -{ - assert(addr < cache->len && *plen <= cache->len - addr); - *xlat = addr + cache->xlat; - return cache->mr; + cache->as = NULL; } #define ARG1_DECL MemoryRegionCache *cache #define ARG1 cache #define SUFFIX _cached -#define TRANSLATE(...) address_space_translate_cached(cache, __VA_ARGS__) +#define TRANSLATE(addr, ...) \ + address_space_translate(cache->as, cache->xlat + (addr), __VA_ARGS__) #define IS_DIRECT(mr, is_write) true -#define MAP_RAM(mr, ofs) (cache->ptr + (ofs - cache->xlat)) -#define INVALIDATE(mr, ofs, len) ((void)0) -#define RCU_READ_LOCK() ((void)0) -#define RCU_READ_UNLOCK() ((void)0) +#define MAP_RAM(mr, ofs) qemu_map_ram_ptr((mr)->ram_block, ofs) +#define INVALIDATE(mr, ofs, len) invalidate_and_set_dirty(mr, ofs, len) +#define RCU_READ_LOCK() rcu_read_lock() +#define RCU_READ_UNLOCK() rcu_read_unlock() #include "memory_ldst.inc.c" /* virtual memory access for debug (includes writing to ROM) */ diff --git a/include/exec/memory.h b/include/exec/memory.h index e39256ad03..f20b191793 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -1426,13 +1426,11 @@ void stq_be_phys(AddressSpace *as, hwaddr addr, uint64_t val); struct MemoryRegionCache { hwaddr xlat; - void *ptr; hwaddr len; - MemoryRegion *mr; - bool is_write; + AddressSpace *as; }; -#define MEMORY_REGION_CACHE_INVALID ((MemoryRegionCache) { .mr = NULL }) +#define MEMORY_REGION_CACHE_INVALID ((MemoryRegionCache) { .as = NULL }) /* address_space_cache_init: prepare for repeated access to a physical * memory region @@ -1688,7 +1686,7 @@ address_space_read_cached(MemoryRegionCache *cache, hwaddr addr, void *buf, int len) { assert(addr < cache->len && len <= cache->len - addr); - memcpy(buf, cache->ptr + addr, len); + address_space_read(cache->as, cache->xlat + addr, MEMTXATTRS_UNSPECIFIED, buf, len); } /** @@ -1704,7 +1702,7 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr, void *buf, int len) { assert(addr < cache->len && len <= cache->len - addr); - memcpy(cache->ptr + addr, buf, len); + address_space_write(cache->as, cache->xlat + addr, MEMTXATTRS_UNSPECIFIED, buf, len); } #endif From ecbddbb106114f90008024b4e6c3ba1c38d7ca0e Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Fri, 31 Mar 2017 21:51:33 +0100 Subject: [PATCH 6/6] main-loop: Acquire main_context lock around os_host_main_loop_wait. When running virt-rescue the serial console hangs from time to time. Virt-rescue runs an ordinary Linux kernel "appliance", but there is only a single idle process running inside, so the qemu main loop is largely idle. With virt-rescue >= 1.37 you may be able to observe the hang by doing: $ virt-rescue -e ^] --scratch > while true; do ls -l /usr/bin; done The hang in virt-rescue can be resolved by pressing a key on the serial console. Possibly with the same root cause, we also observed hangs during very early boot of regular Linux VMs with a serial console. Those hangs are extremely rare, but you may be able to observe them by running this command on baremetal for a sufficiently long time: $ while libguestfs-test-tool -t 60 >& /tmp/log ; do echo -n . ; done (Check in /tmp/log that the failure was caused by a hang during early boot, and not some other reason) During investigation of this bug, Paolo Bonzini wrote: > glib is expecting QEMU to use g_main_context_acquire around accesses to > GMainContext. However QEMU is not doing that, instead it is taking its > own mutex. So we should add g_main_context_acquire and > g_main_context_release in the two implementations of > os_host_main_loop_wait; these should undo the effect of Frediano's > glib patch. This patch exactly implements Paolo's suggestion in that paragraph. This fixes the serial console hang in my testing, across 3 different physical machines (AMD, Intel Core i7 and Intel Xeon), over many hours of automated testing. I wasn't able to reproduce the early boot hangs (but as noted above, these are extremely rare in any case). Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1435432 Reported-by: Richard W.M. Jones Tested-by: Richard W.M. Jones Signed-off-by: Richard W.M. Jones Message-Id: <20170331205133.23906-1-rjones@redhat.com> [Paolo: this is actually a glib bug: recent glib versions are also expecting g_main_context_acquire around g_poll---but that is not documented and probably not even intended]. Signed-off-by: Paolo Bonzini --- util/main-loop.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/util/main-loop.c b/util/main-loop.c index 4534c89308..19cad6b8b6 100644 --- a/util/main-loop.c +++ b/util/main-loop.c @@ -218,9 +218,12 @@ static void glib_pollfds_poll(void) static int os_host_main_loop_wait(int64_t timeout) { + GMainContext *context = g_main_context_default(); int ret; static int spin_counter; + g_main_context_acquire(context); + glib_pollfds_fill(&timeout); /* If the I/O thread is very busy or we are incorrectly busy waiting in @@ -256,6 +259,9 @@ static int os_host_main_loop_wait(int64_t timeout) } glib_pollfds_poll(); + + g_main_context_release(context); + return ret; } #else @@ -412,12 +418,15 @@ static int os_host_main_loop_wait(int64_t timeout) fd_set rfds, wfds, xfds; int nfds; + g_main_context_acquire(context); + /* XXX: need to suppress polling by better using win32 events */ ret = 0; for (pe = first_polling_entry; pe != NULL; pe = pe->next) { ret |= pe->func(pe->opaque); } if (ret != 0) { + g_main_context_release(context); return ret; } @@ -472,6 +481,8 @@ static int os_host_main_loop_wait(int64_t timeout) g_main_context_dispatch(context); } + g_main_context_release(context); + return select_ret || g_poll_ret; } #endif