[Qemu-devel] [PATCH V5_resend 0/7] nvdimm: support MAP_SYNC for memory-backend-file
Linux 4.15 introduces a new mmap flag MAP_SYNC, which can be used to
guarantee the write persistence to mmap'ed files supporting DAX (e.g.,
files on ext4/xfs file system mounted with '-o dax').
A description of MAP_SYNC and MAP_SHARED_VALIDATE can be found at
https://patchwork.kernel.org/patch/10028151/
In order to make sure that the file metadata is in sync after a fault
while we are writing a shared DAX supporting backend files, this
patch-set enables QEMU to use MAP_SYNC flag for memory-backend-dax-file.
As the DAX vs DMA truncated issue was solved, we refined the code and
send out this feature for the v5 version.
A new auto on/off option 'sync' is added to memory-backend-file:
- on: try to pass MAP_SYNC to mmap(2); if MAP_SYNC is not supported or
'share=off', QEMU will abort
- off: never pass MAP_SYNC to mmap(2)
- auto (default): if MAP_SYNC is supported and 'share=on', work as if
'sync=on'; otherwise, work as if 'sync=off'
Zhang Yi (7):
numa: Fixed the memory leak of numa error message
util/mmap-alloc: switch qemu_ram_mmap() to 'flags' parameter
exec: switch qemu_ram_alloc_from_{file, fd} to the 'flags' parameter
util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap()
util/mmap-alloc: Switch the RAM_SYNC flags to OnOffAuto
hostmem: add more information in error messages
hostmem-file: add 'sync' option
backends/hostmem-file.c | 45 +--
backends/hostmem.c| 8 ---
docs/nvdimm.txt | 20 +++-
exec.c| 9 +++
include/exec/memory.h | 18 ++
include/exec/ram_addr.h | 1 +
include/qemu/mmap-alloc.h | 20 +++-
include/standard-headers/linux/mman.h | 44 ++
numa.c| 1 +
qemu-options.hx | 22 -
util/mmap-alloc.c | 26
util/oslib-posix.c| 4 +++-
12 files changed, 200 insertions(+), 18 deletions(-)
create mode 100644 include/standard-headers/linux/mman.h
--
2.7.4
[Qemu-devel] [PATCH V5_resend 5/7] util/mmap-alloc: Switch the RAM_SYNC flags to OnOffAuto
Signed-off-by: Zhang Yi
A set of RAM_SYNC_ON_OFF_AUTO{AUTO,ON,OFF} flags are added to
qemu_ram_mmap():
- If RAM_SYNC_ON_OFF_AUTO_ON is present, qemu_ram_mmap() will try to pass
MAP_SYNC to mmap(). It will then fail if the host OS or the backend
file do not support MAP_SYNC, or MAP_SYNC is conflict with other
flags.
- If RAM_SYNC_ON_OFF_AUTO_OFF is present, qemu_ram_mmap() will never pass
MAP_SYNC to mmap().
- If RAM_SYNC_ON_OFF_AUTO_AUTO is present, and
* if the host OS and the backend file support MAP_SYNC, and MAP_SYNC
is not conflict with other flags, qemu_ram_mmap() will work as if
RAM_SYNC_ON_OFF_AUTO_ON is present;
* otherwise, qemu_ram_mmap() will work as if RAM_SYNC_ON_OFF_AUTO_OFF is
present.
---
include/exec/memory.h | 9 -
util/mmap-alloc.c | 12 ++--
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 33a4e2c..c74c467 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -127,7 +127,14 @@ typedef struct IOMMUNotifier IOMMUNotifier;
#define RAM_PMEM (1 << 5)
/* RAM can be mmap by a MAP_SYNC flag */
-#define RAM_SYNC (1 << 6)
+#define RAM_SYNC_SHIFT 6
+#define RAM_SYNC_SHIFT_AUTO 7
+
+#define RAM_SYNC_ON_OFF_AUTO_ON (1UL << RAM_SYNC_SHIFT)
+#define RAM_SYNC_ON_OFF_AUTO_OFF (0UL << RAM_SYNC_SHIFT)
+#define RAM_SYNC_ON_OFF_AUTO_AUTO (1UL << RAM_SYNC_SHIFT_AUTO)
+
+#define RAM_SYNC (RAM_SYNC_ON_OFF_AUTO_ON | RAM_SYNC_ON_OFF_AUTO_AUTO)
static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
IOMMUNotifierFlag flags,
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index f411df7..fe9303f 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -111,6 +111,10 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align,
uint32_t flags)
assert(is_power_of_2(align));
/* Always align to host page size */
assert(align >= getpagesize());
+if ((flags & RAM_SYNC_ON_OFF_AUTO_ON) &&
+(!shared || !MAP_SYNC_FLAGS)) {
+return MAP_FAILED;
+}
if ((flags & RAM_SYNC) && shared) {
mmap_xflags |= MAP_SYNC_FLAGS;
}
@@ -123,8 +127,12 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align,
uint32_t flags)
(shared ? MAP_SHARED : MAP_PRIVATE) | mmap_xflags,
fd, 0);
if ((ptr1 == MAP_FAILED) && (mmap_xflags & MAP_SYNC_FLAGS)) {
-mmap_xflags &= ~MAP_SYNC_FLAGS;
-goto retry_mmap_fd;
+if (flags & RAM_SYNC_ON_OFF_AUTO_AUTO) {
+mmap_xflags &= ~MAP_SYNC_FLAGS;
+goto retry_mmap_fd;
+}
+munmap(ptr, total);
+return MAP_FAILED;
}
if (offset > 0) {
--
2.7.4
[Qemu-devel] [PATCH V5_resend 6/7] hostmem: add more information in error messages
When there are multiple memory backends in use, including the object type
name, ID and the property name in the error message can help users to
locate the error.
Signed-off-by: Haozhong Zhang
Signed-off-by: Zhang Yi
---
backends/hostmem-file.c | 6 --
backends/hostmem.c | 8 +---
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index e640749..0dd7a90 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -82,7 +82,8 @@ static void set_mem_path(Object *o, const char *str, Error
**errp)
HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
if (host_memory_backend_mr_inited(backend)) {
-error_setg(errp, "cannot change property value");
+error_setg(errp, "cannot change property 'mem-path' of %s",
+ object_get_typename(o));
return;
}
g_free(fb->mem_path);
@@ -120,7 +121,8 @@ static void file_memory_backend_set_align(Object *o,
Visitor *v,
uint64_t val;
if (host_memory_backend_mr_inited(backend)) {
-error_setg(_err, "cannot change property value");
+error_setg(_err, "cannot change property '%s' of %s",
+ name, object_get_typename(o));
goto out;
}
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 1a89342..e2bcf9f 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -47,7 +47,8 @@ host_memory_backend_set_size(Object *obj, Visitor *v, const
char *name,
uint64_t value;
if (host_memory_backend_mr_inited(backend)) {
-error_setg(_err, "cannot change property value");
+error_setg(_err, "cannot change property %s of %s ",
+ name, object_get_typename(obj));
goto out;
}
@@ -56,8 +57,9 @@ host_memory_backend_set_size(Object *obj, Visitor *v, const
char *name,
goto out;
}
if (!value) {
-error_setg(_err, "Property '%s.%s' doesn't take value '%"
- PRIu64 "'", object_get_typename(obj), name, value);
+error_setg(_err,
+ "property '%s' of %s doesn't take value '%" PRIu64 "'",
+ name, object_get_typename(obj), value);
goto out;
}
backend->size = value;
--
2.7.4
[Qemu-devel] [PATCH V5_resend 2/7] util/mmap-alloc: switch qemu_ram_mmap() to 'flags' parameter
As more flag parameters besides the existing 'shared' are going to be
added to qemu_ram_mmap(), let's switch 'shared' to a 'flags' parameter
in advance, so as to ease the further additions.
Signed-off-by: Haozhong Zhang
Signed-off-by: Zhang Yi
---
exec.c| 3 +--
include/qemu/mmap-alloc.h | 19 ++-
util/mmap-alloc.c | 8 +---
util/oslib-posix.c| 4 +++-
4 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/exec.c b/exec.c
index bb6170d..273f668 100644
--- a/exec.c
+++ b/exec.c
@@ -1859,8 +1859,7 @@ static void *file_ram_alloc(RAMBlock *block,
perror("ftruncate");
}
-area = qemu_ram_mmap(fd, memory, block->mr->align,
- block->flags & RAM_SHARED);
+area = qemu_ram_mmap(fd, memory, block->mr->align, block->flags);
if (area == MAP_FAILED) {
error_setg_errno(errp, errno,
"unable to map backing store for guest RAM");
diff --git a/include/qemu/mmap-alloc.h b/include/qemu/mmap-alloc.h
index 50385e3..6fe6ed4 100644
--- a/include/qemu/mmap-alloc.h
+++ b/include/qemu/mmap-alloc.h
@@ -7,7 +7,24 @@ size_t qemu_fd_getpagesize(int fd);
size_t qemu_mempath_getpagesize(const char *mem_path);
-void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared);
+/**
+ * qemu_ram_mmap: mmap the specified file or device.
+ *
+ * Parameters:
+ * @fd: the file or the device to mmap
+ * @size: the number of bytes to be mmaped
+ * @align: if not zero, specify the alignment of the starting mapping address;
+ * otherwise, the alignment in use will be determined by QEMU.
+ * @flags: specifies additional properties of the mapping, which can be one or
+ * bit-or of following values
+ * - RAM_SHARED: mmap with MAP_SHARED flag
+ * Other bits are ignored.
+ *
+ * Return:
+ * On success, return a pointer to the mapped area.
+ * On failure, return MAP_FAILED.
+ */
+void *qemu_ram_mmap(int fd, size_t size, size_t align, uint32_t flags);
void qemu_ram_munmap(void *ptr, size_t size);
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index fd329ec..8f0a740 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -13,6 +13,7 @@
#include "qemu/osdep.h"
#include "qemu/mmap-alloc.h"
#include "qemu/host-utils.h"
+#include "exec/memory.h"
#define HUGETLBFS_MAGIC 0x958458f6
@@ -75,7 +76,7 @@ size_t qemu_mempath_getpagesize(const char *mem_path)
return getpagesize();
}
-void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared)
+void *qemu_ram_mmap(int fd, size_t size, size_t align, uint32_t flags)
{
/*
* Note: this always allocates at least one extra page of virtual address
@@ -92,11 +93,12 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align, bool
shared)
* anonymous memory is OK.
*/
int anonfd = fd == -1 || qemu_fd_getpagesize(fd) == getpagesize() ? -1 :
fd;
-int flags = anonfd == -1 ? MAP_ANONYMOUS : MAP_NORESERVE;
-void *ptr = mmap(0, total, PROT_NONE, flags | MAP_PRIVATE, anonfd, 0);
+int mmap_flags = anonfd == -1 ? MAP_ANONYMOUS : MAP_NORESERVE;
+void *ptr = mmap(0, total, PROT_NONE, mmap_flags | MAP_PRIVATE, anonfd, 0);
#else
void *ptr = mmap(0, total, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
#endif
+bool shared = flags & RAM_SHARED;
size_t offset;
void *ptr1;
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index fbd0dc8..c28869d 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -203,7 +203,9 @@ void *qemu_memalign(size_t alignment, size_t size)
void *qemu_anon_ram_alloc(size_t size, uint64_t *alignment, bool shared)
{
size_t align = QEMU_VMALLOC_ALIGN;
-void *ptr = qemu_ram_mmap(-1, size, align, shared);
+uint32_t flags = 0;
+flags |= shared;
+void *ptr = qemu_ram_mmap(-1, size, align, flags);
if (ptr == MAP_FAILED) {
return NULL;
--
2.7.4
[Qemu-devel] [PATCH V5_resend 4/7] util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap()
When a file supporting DAX is used as vNVDIMM backend, mmap it with
MAP_SYNC flag in addition can guarantee the persistence of guest write
to the backend file without other QEMU actions (e.g., periodic fsync()
by QEMU).
A set of RAM_SYNC flags are added to qemu_ram_mmap():
Signed-off-by: Haozhong Zhang
Signed-off-by: Zhang Yi
---
exec.c| 2 +-
include/exec/memory.h | 3 +++
include/exec/ram_addr.h | 1 +
include/qemu/mmap-alloc.h | 1 +
include/standard-headers/linux/mman.h | 44 +++
util/mmap-alloc.c | 14 +++
6 files changed, 60 insertions(+), 5 deletions(-)
create mode 100644 include/standard-headers/linux/mman.h
diff --git a/exec.c b/exec.c
index e92a7da..dc4d180 100644
--- a/exec.c
+++ b/exec.c
@@ -2241,7 +2241,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size,
MemoryRegion *mr,
int64_t file_size;
/* Just support these ram flags by now. */
-assert((ram_flags & ~(RAM_SHARED | RAM_PMEM)) == 0);
+assert((ram_flags & ~(RAM_SHARED | RAM_PMEM | RAM_SYNC)) == 0);
if (xen_enabled()) {
error_setg(errp, "-mem-path not supported with Xen");
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 667466b..33a4e2c 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -126,6 +126,9 @@ typedef struct IOMMUNotifier IOMMUNotifier;
/* RAM is a persistent kind memory */
#define RAM_PMEM (1 << 5)
+/* RAM can be mmap by a MAP_SYNC flag */
+#define RAM_SYNC (1 << 6)
+
static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
IOMMUNotifierFlag flags,
hwaddr start, hwaddr end,
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 9ecd911..d239ce7 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -87,6 +87,7 @@ long qemu_getrampagesize(void);
* or bit-or of following values
* - RAM_SHARED: mmap the backing file or device with MAP_SHARED
* - RAM_PMEM: the backend @mem_path or @fd is persistent memory
+ * - RAM_SYNC: mmap with MAP_SYNC flag
* Other bits are ignored.
* @mem_path or @fd: specify the backing file or device
* @errp: pointer to Error*, to store an error if it happens
diff --git a/include/qemu/mmap-alloc.h b/include/qemu/mmap-alloc.h
index 6fe6ed4..1755a8b 100644
--- a/include/qemu/mmap-alloc.h
+++ b/include/qemu/mmap-alloc.h
@@ -18,6 +18,7 @@ size_t qemu_mempath_getpagesize(const char *mem_path);
* @flags: specifies additional properties of the mapping, which can be one or
* bit-or of following values
* - RAM_SHARED: mmap with MAP_SHARED flag
+ * - RAM_SYNC: mmap with MAP_SYNC flag
* Other bits are ignored.
*
* Return:
diff --git a/include/standard-headers/linux/mman.h
b/include/standard-headers/linux/mman.h
new file mode 100644
index 000..ea1fc47
--- /dev/null
+++ b/include/standard-headers/linux/mman.h
@@ -0,0 +1,44 @@
+/*
+ * Definitions of Linux-specific mmap flags.
+ *
+ * Copyright Intel Corporation, 2018
+ *
+ * Author: Haozhong Zhang
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#ifndef _LINUX_MMAN_H
+#define _LINUX_MMAN_H
+
+/*
+ * MAP_SHARED_VALIDATE and MAP_SYNC are introduced in Linux kernel
+ * 4.15, so they may not be defined when compiling on older kernels.
+ */
+#ifdef CONFIG_LINUX
+
+#include
+
+#ifndef MAP_SHARED_VALIDATE
+#define MAP_SHARED_VALIDATE 0x3
+#endif
+
+#ifndef MAP_SYNC
+#define MAP_SYNC 0x8
+#endif
+
+/* MAP_SYNC is only available with MAP_SHARED_VALIDATE. */
+#define MAP_SYNC_FLAGS (MAP_SYNC | MAP_SHARED_VALIDATE)
+
+#else /* !CONFIG_LINUX */
+
+#define MAP_SHARED_VALIDATE 0x0
+#define MAP_SYNC 0x0
+
+#define QEMU_HAS_MAP_SYNC false
+#define MAP_SYNC_FLAGS 0
+
+#endif /* CONFIG_LINUX */
+
+#endif /* !_LINUX_MMAN_H */
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 8f0a740..f411df7 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -14,6 +14,7 @@
#include "qemu/mmap-alloc.h"
#include "qemu/host-utils.h"
#include "exec/memory.h"
+#include "standard-headers/linux/mman.h"
#define HUGETLBFS_MAGIC 0x958458f6
@@ -99,6 +100,7 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align,
uint32_t flags)
void *ptr = mmap(0, total, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
#endif
bool shared = flags & RAM_SHARED;
+int mmap_xflags = 0;
size_t offset;
void *ptr1;
@@ -109,16 +111,20 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align,
uint32_t flags)
assert(is_power_of_2(align));
/* Always align to host page size */
assert(align >= getpagesize());
+if ((flags & RAM_SYNC) && shared) {
+mmap_xflags |=
[Qemu-devel] [PATCH V5_resend 7/7] hostmem-file: add 'sync' option
This option controls whether QEMU mmap(2) the memory backend file with
MAP_SYNC flag, which can fully guarantee the guest write persistence
to the backend, if MAP_SYNC flag is supported by the host kernel
(Linux kernel 4.15 and later) and the backend is a file supporting
DAX (e.g., file on ext4/xfs file system mounted with '-o dax').
It can take one of following values:
- on: try to pass MAP_SYNC to mmap(2); if MAP_SYNC is not supported or
'share=off', QEMU will abort
- off: never pass MAP_SYNC to mmap(2)
- auto (default): if MAP_SYNC is supported and 'share=on', work as if
'sync=on'; otherwise, work as if 'sync=off'
Signed-off-by: Haozhong Zhang
Signed-off-by: Zhang Yi
---
backends/hostmem-file.c | 39 +++
docs/nvdimm.txt | 20 +++-
include/exec/memory.h | 8
qemu-options.hx | 22 +-
4 files changed, 87 insertions(+), 2 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 0dd7a90..73cf181 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -16,6 +16,7 @@
#include "sysemu/hostmem.h"
#include "sysemu/sysemu.h"
#include "qom/object_interfaces.h"
+#include "qapi/qapi-visit.h"
/* hostmem-file.c */
/**
@@ -36,6 +37,7 @@ struct HostMemoryBackendFile {
uint64_t align;
bool discard_data;
bool is_pmem;
+OnOffAuto sync;
};
static void
@@ -62,6 +64,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error
**errp)
path,
backend->size, fb->align,
(backend->share ? RAM_SHARED : 0) |
+ qemu_ram_sync_flags(fb->sync) |
(fb->is_pmem ? RAM_PMEM : 0),
fb->mem_path, errp);
g_free(path);
@@ -136,6 +139,39 @@ static void file_memory_backend_set_align(Object *o,
Visitor *v,
error_propagate(errp, local_err);
}
+static void file_memory_backend_get_sync(
+Object *obj, Visitor *v, const char *name, void *opaque, Error **errp)
+{
+HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj);
+OnOffAuto value = fb->sync;
+
+visit_type_OnOffAuto(v, name, , errp);
+}
+
+static void file_memory_backend_set_sync(
+Object *obj, Visitor *v, const char *name, void *opaque, Error **errp)
+{
+HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj);
+Error *local_err = NULL;
+OnOffAuto value;
+
+if (host_memory_backend_mr_inited(backend)) {
+error_setg(_err, "cannot change property '%s' of %s",
+ name, object_get_typename(obj));
+goto out;
+}
+
+visit_type_OnOffAuto(v, name, , _err);
+if (local_err) {
+goto out;
+}
+fb->sync = value;
+
+ out:
+error_propagate(errp, local_err);
+}
+
static bool file_memory_backend_get_pmem(Object *o, Error **errp)
{
return MEMORY_BACKEND_FILE(o)->is_pmem;
@@ -203,6 +239,9 @@ file_backend_class_init(ObjectClass *oc, void *data)
object_class_property_add_bool(oc, "pmem",
file_memory_backend_get_pmem, file_memory_backend_set_pmem,
_abort);
+object_class_property_add(oc, "sync", "OnOffAuto",
+file_memory_backend_get_sync, file_memory_backend_set_sync,
+NULL, NULL, _abort);
}
static void file_backend_instance_finalize(Object *o)
diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
index 5f158a6..3d89174 100644
--- a/docs/nvdimm.txt
+++ b/docs/nvdimm.txt
@@ -142,11 +142,29 @@ backend of vNVDIMM:
Guest Data Persistence
--
+vNVDIMM is designed and implemented to guarantee the guest data
+persistence on the backends even on the host crash and power
+failures. However, there are still some requirements and limitations
+as explained below.
+
Though QEMU supports multiple types of vNVDIMM backends on Linux,
-currently the only one that can guarantee the guest write persistence
+if MAP_SYNC is not supported by the host kernel and the backends,
+the only backend that can guarantee the guest write persistence
is the device DAX on the real NVDIMM device (e.g., /dev/dax0.0), to
which all guest access do not involve any host-side kernel cache.
+mmap(2) flag MAP_SYNC is added since Linux kernel 4.15. On such
+systems, QEMU can mmap(2) the backend with MAP_SYNC, which can
+guarantee the guest write persistence to vNVDIMM. Besides the host
+kernel support, enabling MAP_SYNC in QEMU also requires:
+
+ - the backend is a file supporting DAX, e.g., a file on an ext4 or
+ xfs file system mounted with '-o dax',
+
+ - 'sync' option of memory-backend-file is not 'off', and
+
+ - 'share' option of memory-backend-file is 'on'.
+
When using other types of backends, it's suggested to set 'unarmed'
option of '-device nvdimm' to 'on', which sets the unarmed flag of the
guest NVDIMM region mapping
[Qemu-devel] [PATCH V5_resend 1/7] numa: Fixed the memory leak of numa error message
object_get_canonical_path_component() returns a string which
must be freed using g_free().
Signed-off-by: Zhang Yi
---
numa.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/numa.c b/numa.c
index 50ec016..3875e1e 100644
--- a/numa.c
+++ b/numa.c
@@ -533,6 +533,7 @@ void memory_region_allocate_system_memory(MemoryRegion *mr,
Object *owner,
error_report("memory backend %s is used multiple times. Each "
"-numa option must use a different memdev value.",
path);
+g_free(path);
exit(1);
}
--
2.7.4
[Qemu-devel] [PATCH V5_resend 3/7] exec: switch qemu_ram_alloc_from_{file, fd} to the 'flags' parameter
As more flag parameters besides the existing 'share' are going to be
added to qemu_ram_alloc_from_{file,fd}(), let's swith 'share' to a
'flags' parameters in advance, so as to ease the further additions.
Signed-off-by: Haozhong Zhang
Signed-off-by: Zhang Yi
---
exec.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/exec.c b/exec.c
index 273f668..e92a7da 100644
--- a/exec.c
+++ b/exec.c
@@ -1810,6 +1810,7 @@ static void *file_ram_alloc(RAMBlock *block,
ram_addr_t memory,
int fd,
bool truncate,
+uint32_t flags,
Error **errp)
{
void *area;
@@ -1859,7 +1860,7 @@ static void *file_ram_alloc(RAMBlock *block,
perror("ftruncate");
}
-area = qemu_ram_mmap(fd, memory, block->mr->align, block->flags);
+area = qemu_ram_mmap(fd, memory, block->mr->align, flags);
if (area == MAP_FAILED) {
error_setg_errno(errp, errno,
"unable to map backing store for guest RAM");
@@ -2278,7 +2279,8 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size,
MemoryRegion *mr,
new_block->used_length = size;
new_block->max_length = size;
new_block->flags = ram_flags;
-new_block->host = file_ram_alloc(new_block, size, fd, !file_size, errp);
+new_block->host = file_ram_alloc(new_block, size, fd, !file_size,
+ram_flags, errp);
if (!new_block->host) {
g_free(new_block);
return NULL;
--
2.7.4
[Qemu-devel] [PATCH v1] tpm: check localities index
From: Prasad J Pandit
While performing mmio device r/w operations, guest could set 'addr'
parameter such that 'locty' index exceeds TPM_TIS_NUM_LOCALITIES=5
after setting new 'locty' via 'tpm_tis_new_active_locality'.
Add check to avoid OOB access.
Reported-by: Cheng Feng
Signed-off-by: Prasad J Pandit
---
hw/tpm/tpm_tis.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
Update: add assert() calls
-> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg00912.html
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 12f5c9a759..d6bf3ceb26 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -293,6 +293,7 @@ static void tpm_tis_request_completed(TPMIf *ti, int ret)
uint8_t locty = s->cmd.locty;
uint8_t l;
+assert(TPM_TIS_IS_VALID_LOCTY(locty));
if (s->cmd.selftest_done) {
for (l = 0; l < TPM_TIS_NUM_LOCALITIES; l++) {
s->loc[locty].sts |= TPM_TIS_STS_SELFTEST_DONE;
@@ -401,6 +402,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
uint32_t avail;
uint8_t v;
+assert(TPM_TIS_IS_VALID_LOCTY(locty));
if (tpm_backend_had_startup_error(s->be_driver)) {
return 0;
}
@@ -523,6 +525,7 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
uint16_t len;
uint32_t mask = (size == 1) ? 0xff : ((size == 2) ? 0x : ~0);
+assert(TPM_TIS_IS_VALID_LOCTY(locty));
trace_tpm_tis_mmio_write(size, addr, val);
if (locty == 4) {
@@ -642,7 +645,7 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
}
}
-if (set_new_locty) {
+if (set_new_locty && TPM_TIS_IS_VALID_LOCTY(active_locty)) {
tpm_tis_new_active_locality(s, active_locty);
}
--
2.17.2
Re: [Qemu-devel] [RFC v1 17/23] riscv: tcg-target: Add direct load and store instructions
On 11/20/18 12:06 AM, Alistair Francis wrote:
> On Fri, Nov 16, 2018 at 9:10 AM Richard Henderson
> wrote:
>>
>> On 11/15/18 11:36 PM, Alistair Francis wrote:
>>> +tcg_out_opc_reg(s, OPC_ADD, base, TCG_GUEST_BASE_REG, addr_regl);
>>
>> Should avoid this when guest_base == 0, which happens fairly regularly for a
>> 64-bit guest.
>>
>>> +/* Prefer to load from offset 0 first, but allow for overlap. */
>>> +if (TCG_TARGET_REG_BITS == 64) {
>>> +tcg_out_opc_imm(s, OPC_LD, lo, base, 0);
>>> +} else {
>>> +tcg_out_opc_imm(s, OPC_LW, lo, base, 0);
>>> +tcg_out_opc_imm(s, OPC_LW, hi, base, 4);
>>> +}
>>
>> Comment sounds like two lines of code that's missing.
>
> I can't figure out what this comment should be for. Why would we want
> to prefer loading with an offset 0?
Perhaps to help the memory controler; perhaps no reason at all.
But "allow for overlap" suggests
} else if (lo != base) {
tcg_out_opc_imm(s, OPC_LW, lo, base, 0);
tcg_out_opc_imm(s, OPC_LW, hi, base, 4);
} else {
tcg_out_opc_imm(s, OPC_LW, hi, base, 4);
tcg_out_opc_imm(s, OPC_LW, lo, base, 0);
}
r~
Re: [Qemu-devel] [RFC v1 20/23] riscv: tcg-target: Add the target init code
On 11/20/18 12:04 AM, Alistair Francis wrote: > On Fri, Nov 16, 2018 at 9:26 AM Richard Henderson > wrote: >> >> On 11/15/18 11:36 PM, Alistair Francis wrote: >>> +tcg_regset_set_reg(s->reserved_regs, TCG_REG_L0); >>> +tcg_regset_set_reg(s->reserved_regs, TCG_REG_L1); >>> +tcg_regset_set_reg(s->reserved_regs, TCG_REG_RA); >> >> Why are these three reserved? > > Do these not need to be? I thought we had to reserve them. The return address, I presume, has been saved by the prologue. I see no reason why it can't be yet another call-clobbered register. As for the other two... what are they supposed to be? r~
Re: [Qemu-devel] [PULL 0/8] Monitor patches for 2018-10-30
On Mon, Nov 19, 2018 at 07:08:13PM +0100, Markus Armbruster wrote:
> Peter Xu writes:
>
> > On Mon, Nov 19, 2018 at 02:17:27PM +0800, Peter Xu wrote:
> >> I reproduced the error with a FreeBSD guest and this change (which
> >> possibly can be squashed into "tests: qmp-test: add queue full test")
> >> worked for me:
> >>
> >> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
> >> index 6989acbca4..83f353db4f 100644
> >> --- a/tests/qmp-test.c
> >> +++ b/tests/qmp-test.c
> >> @@ -281,8 +281,15 @@ static void test_qmp_oob(void)
> >> * will only be able to be handled after the queue is shrinked, so
> >> * it'll be processed only after one existing in-band command
> >> * finishes.
> >> + *
> >> + * NOTE: we need to feed the queue with one extra request to make
> >> + * sure it'll be stuck since when we have sent the Nth request
> >> + * it's possible that we have already popped and processing the
> >> + * 1st request so the Nth request (which could potentially be the
> >> + * [N-1]th element on the queue) might not trigger the
> >> + * monitor-full condition deterministically.
> >> */
> >> -for (i = 1; i <= QMP_REQ_QUEUE_LEN_MAX; i++) {
> >> +for (i = 1; i <= QMP_REQ_QUEUE_LEN_MAX + 1; i++) {
> >> id = g_strdup_printf("queue-blocks-%d", i);
> >> send_cmd_that_blocks(qts, id);
> >> g_free(id);
> >> @@ -291,7 +298,7 @@ static void test_qmp_oob(void)
> >> unblock_blocked_cmd();
> >> recv_cmd_id(qts, "queue-blocks-1");
> >> recv_cmd_id(qts, "oob-1");
> >> -for (i = 2; i <= QMP_REQ_QUEUE_LEN_MAX; i++) {
> >> +for (i = 2; i <= QMP_REQ_QUEUE_LEN_MAX + 1; i++) {
> >> unblock_blocked_cmd();
> >> id = g_strdup_printf("queue-blocks-%d", i);
> >> recv_cmd_id(qts, id);
> >>
> >> So the problem here is that the queue-block-N command might not really
> >> suspend the monitor everytime if we already popped the 1st request,
> >> which will let the N-th request to be (N-1)th, then the parser will
> >> continue to eat the oob command and it could "preempt" the previous
> >> commands.
> >>
> >> Maybe FreeBSD is scheduling the threads in some pattern so it happens
> >> only on FreeBSD and very constantly, but anyway it should be a general
> >> fix to the test program.
>
> I feel particularly dense right now, and need a more detailed analysis
> to understand. Let me try.
>
> Here's what the test does before your fix
>
> Send QMP_REQ_QUEUE_LEN_MAX "blocking" in-band commands
> queue-blocks-1, queue-blocks-2, ... "Blocking" means the command
> blocks until we explicitly unblock it.
>
> Send an out-of-band command oob-1.
>
> Unblock queue-blocks-1.
>
> Receive response to queue-blocks-1.
>
> Receive response to oob-1.
>
> Unblock queue-blocks-2, receive response
> ...
>
> Here's what test test expects QEMU to do:
>
> In I/O thread:
>
> Receive and queue queue-blocks-1, ...
>
> Queue is full, stop receiving commands.
>
> In main thread:
>
> Dequeue queue-blocks-1.
>
> Execute queue-blocks-1, block for a while, finish and send
> response.
>
> In I/O thread:
>
> Queue is no longer full, resume receiving commands.
>
> Receive and execute oob-1, send response
>
> In main thread:
>
> Dequeue queue-blocks-2, execute, block for a while, finish and
> send response.
> ...
>
> No good because the two threads race. On my Linux box the test passes.
> Let's have a closer look how it does that. The appended patch that adds
> another tracepoint. With tracepoints monitor_* and handle_qmp_command
> enabled, I get:
>
> 19271@1542641581.372890:handle_qmp_command mon 0x563e19be4e80 req:
> {"execute": "blockdev-add", "arguments": {"image": {"driver": "null-co"},
> "node-name": "queue-blocks-1", "driver": "blkdebug", "config":
> "/tmp/qmp-test-EZroW3/fifo"}}
>
> I/O thread receives and queues queue-blocks-1.
>
> 19271@1542641581.372954:monitor_qmp_cmd_in_band queue-blocks-1
>
> Main thread dequeues qemu-blocks-1 and starts executing it.
>
> We already lost: the queue won't actually fill up. The test passes
> anyway, but it doesn't actually test "queue full". But why does it
> pass? Let's see.
>
> 19271@1542641581.373211:monitor_qmp_respond queue-blocks-1
>
> Main thread sends response to queue-blocks-1. This means queue-blocks-1
> has been unblocked already. This is possible, because qmp-test sends
> all the commands and then immediately unblocks queue-blocks-1, and the
> main thread can win the race with the I/O thread.
>
> 19271@1542641581.373826:handle_qmp_command mon 0x563e19be4e80 req:
> {"execute": "blockdev-add", "arguments": {"image": {"driver": "null-co"},
> "node-name": "queue-blocks-2", "driver": "blkdebug", "config":
> "/tmp/qmp-test-EZroW3/fifo"}}
> 19271@1542641581.373864:monitor_qmp_cmd_in_band queue-blocks-2
>
Re: [Qemu-devel] SeaBIOS booting time optimization
Hi, > just an update, I enabled the debug prints and I saw two timeouts fired > with a lot > of time lost (~780ms between "init timer" and "Scan for VGA ..."), > putting other prints I discovered that a lot of time is spent in the > tpm_setup(), > during the probe of the 2 TPM devices: > > 00.548869 init timer > 00.549677 ./src/post.c:157 platform_hardware_setup > 00.550182 ./src/hw/tpm_drivers.c:579 tpmhw_probe > 01.300833 WARNING - Timeout at wait_reg8:81! > 01.301388 ./src/hw/tpm_drivers.c:579 tpmhw_probe > 01.331843 WARNING - Timeout at wait_reg8:81! > 01.332316 ./src/post.c:160 platform_hardware_setup > 01.58 Scan for VGA option rom > > Indeed, in the probe of the TPM devices (TIS and CRB) there are timeouts of > 750 ms and 30 ms respectively. > > Then, statically disabling TPM and TCG (CONFIG_TCGBIOS) the time spent > in SeaBIOS goes down from ~846ms to ~56ms: > # SeaBIOS disabling CONFIG_TCGBIOS > BIOS=/home/stefano/repos/seabios/out_notcgbios/bios.bin > qemu_init_end: 40.658371 > fw_start: 40.850395 (+0.192024) > fw_do_boot: 96.750142 (+55.899747) > linux_start_boot: 98.880578 (+2.130436) > > The tpm_setup() is called after the qemu_cfg_init(), so I think we can > disable > this call using some fw_cfg parameters, if we will decide to implement a > runtime > approach. Another possible option is to simply use shorter timeouts on qemu (using runningOnQEMU()), I'd suggest to check with the TPM maintainers whenever that is ok. Background: The drivers in seabios work on both virtual and physical hardware (you can run seabios as coreboot payload on physical hardware). There are quite a few places where are delays and timeouts which are required to work properly on physical hardware. But typically virtual hardware is alot faster. There is -- for example -- no time needed to establish a sata link. Link detection in ahci is instant on qemu. cheers, Gerd
Re: [Qemu-devel] [PATCH for-3.2 02/41] glib-compat: add g_spawn_async_with_fds() fallback
On 2018-11-19 23:50, Samuel Thibault wrote:
> Marc-André Lureau, le mer. 14 nov. 2018 16:36:04 +0400, a ecrit:
>> Signed-off-by: Marc-André Lureau
>
> Reviewed-by: Samuel Thibault
>
> include/glib-compat.h maintainers, may I keep this in my slirp tree, to
> be pushed to master when appropriate?
$ scripts/get_maintainer.pl -f include/glib-compat.h
get_maintainer.pl: No maintainers found [...]
... so I'd say yes, please just go ahead and queue this :-)
Thomas
>> ---
>> include/glib-compat.h | 56 +++
>> 1 file changed, 56 insertions(+)
>>
>> diff --git a/include/glib-compat.h b/include/glib-compat.h
>> index fdf95a255d..8a078c5288 100644
>> --- a/include/glib-compat.h
>> +++ b/include/glib-compat.h
>> @@ -83,6 +83,62 @@ static inline gboolean g_strv_contains_qemu(const gchar
>> *const *strv,
>> }
>> #define g_strv_contains(a, b) g_strv_contains_qemu(a, b)
>>
>> +#if !GLIB_CHECK_VERSION(2, 58, 0)
>> +typedef struct QemuGSpawnFds {
>> +GSpawnChildSetupFunc child_setup;
>> +gpointer user_data;
>> +gint stdin_fd;
>> +gint stdout_fd;
>> +gint stderr_fd;
>> +} QemuGSpawnFds;
>> +
>> +static inline void
>> +qemu_gspawn_fds_setup(gpointer user_data)
>> +{
>> +QemuGSpawnFds *q = (QemuGSpawnFds *)user_data;
>> +
>> +dup2(q->stdin_fd, 0);
>> +dup2(q->stdout_fd, 1);
>> +dup2(q->stderr_fd, 2);
>> +q->child_setup(q->user_data);
>> +}
>> +#endif
>> +
>> +static inline gboolean
>> +g_spawn_async_with_fds_qemu(const gchar *working_directory,
>> +gchar **argv,
>> +gchar **envp,
>> +GSpawnFlags flags,
>> +GSpawnChildSetupFunc child_setup,
>> +gpointer user_data,
>> +GPid *child_pid,
>> +gint stdin_fd,
>> +gint stdout_fd,
>> +gint stderr_fd,
>> +GError **error)
>> +{
>> +#if GLIB_CHECK_VERSION(2, 58, 0)
>> +return g_spawn_async_with_fds(working_directory, argv, envp, flags,
>> + child_setup, user_data,
>> + child_pid, stdin_fd, stdout_fd, stderr_fd,
>> + error);
>> +#else
>> +QemuGSpawnFds setup = {
>> +.child_setup = child_setup,
>> +.user_data = user_data,
>> +.stdin_fd = stdin_fd,
>> +.stdout_fd = stdout_fd,
>> +.stderr_fd = stderr_fd,
>> +};
>> +
>> +return g_spawn_async(working_directory, argv, envp, flags,
>> + qemu_gspawn_fds_setup, ,
>> + child_pid, error);
>> +#endif
>> +}
>> +
>> +#define g_spawn_async_with_fds(wd, argv, env, f, c, d, p, ifd, ofd, efd,
>> err) \
>> +g_spawn_async_with_fds_qemu(wd, argv, env, f, c, d, p, ifd, ofd, efd,
>> err)
>>
>> #if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
>> /*
>> --
>> 2.19.1.708.g4ede3d42df
>>
>
Re: [Qemu-devel] [PATCH] slirp: Enable fork_exec support on Windows
Hi
On Tue, Nov 20, 2018 at 4:57 AM Samuel Thibault
wrote:
>
> g_spawn_async_with_fds is portable on Windows, so we can now enable
> fork_exec support there.
>
> Thanks Daniel P. Berrangé for the notice!
>
> Signed-off-by: Samuel Thibault
Reviewed-by: Marc-André Lureau
> ---
> slirp/misc.c | 14 ++
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/slirp/misc.c b/slirp/misc.c
> index 7972b9b05b..59b4e8f31c 100644
> --- a/slirp/misc.c
> +++ b/slirp/misc.c
> @@ -62,17 +62,6 @@ int add_exec(struct ex_list **ex_ptr, void *chardev, const
> char *cmdline,
> }
>
>
> -#ifdef _WIN32
> -
> -int
> -fork_exec(struct socket *so, const char *ex)
> -{
> -/* not implemented */
> -return 0;
> -}
> -
> -#else
> -
> static int
> slirp_socketpair_with_oob(int sv[2])
> {
> @@ -132,7 +121,9 @@ err:
> static void
> fork_exec_child_setup(gpointer data)
> {
> +#ifndef _WIN32
> setsid();
> +#endif
> }
>
> int
> @@ -177,7 +168,6 @@ fork_exec(struct socket *so, const char *ex)
> qemu_set_nonblock(so->s);
> return 1;
> }
> -#endif
>
> char *slirp_connection_info(Slirp *slirp)
> {
> --
> 2.19.1
>
[Qemu-devel] [PULL 1/1] update seabios to 1.12
Seabios 1.12 has been released yesterday. Update
our snapshot builds to the final release.
git shortlog
Kevin O'Connor (2):
shadow: Rework bios copy code to prevent gcc array-bounds warning
docs: Note v1.12.0 release
Shmuel Eiderman (1):
pvscsi: Scan all 64 possible targets
Signed-off-by: Gerd Hoffmann
---
pc-bios/bios-256k.bin | Bin 262144 -> 262144 bytes
pc-bios/bios.bin | Bin 131072 -> 131072 bytes
pc-bios/vgabios-bochs-display.bin | Bin 27648 -> 27648 bytes
pc-bios/vgabios-cirrus.bin| Bin 38400 -> 38400 bytes
pc-bios/vgabios-qxl.bin | Bin 38912 -> 38912 bytes
pc-bios/vgabios-ramfb.bin | Bin 28160 -> 28160 bytes
pc-bios/vgabios-stdvga.bin| Bin 38912 -> 38912 bytes
pc-bios/vgabios-virtio.bin| Bin 38912 -> 38912 bytes
pc-bios/vgabios-vmware.bin| Bin 38912 -> 38912 bytes
pc-bios/vgabios.bin | Bin 38400 -> 38400 bytes
roms/Makefile | 2 +-
roms/seabios | 2 +-
12 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/pc-bios/bios-256k.bin b/pc-bios/bios-256k.bin
index
72801adc430d83a9ebb4959314015257ac6cb7fe..08ca873ab57ab6e14bb9a5cdf703c6501230f1d3
100644
GIT binary patch
delta 8024
zcmY*e34G1R_WzypOC-{8Ll)U3Tm%V;eMwp>4}#y||D7A{dw(ClJ2Pj_oH=vmtdsO~pLG1+N(BRhH-%`L
zfiDl=j8IKGkMBM{w=m=EK)UKpH;p}`>9jX_%c{wAJ}k9|NwYrjo!mI9FWFopo7;gt
z0NVTx3^0~Yp%iyoD|e<-4yDU~KB4+#m%l!tfn+zD(pelw$1tl%DC-S$5B(Xfn&~
zc@#>?;+;wH6mKMD(tSOkr{hE?(ro#*Vn@Bsq-Er#c{rvg%foE?sYJ0HC?u(~F#EIKpQ9E%%BedKgz
zk*1Nu=3yVCW~471Y$f>7^mhm!ql)Xbp>qC%tH
z4qC6%TT+%wts$m5ztQ{EszibGH}I`#rehMEQ6RB~`c82i=@tuzcHg^8OlI
zOrjjy1LoF?-(DPZvkcu!y{VUM-%DXM-Z-(B<}fXh!9~>2FRuaC-U}eDbOSWj#nvtP
zwut#W==`@x2{j`j_87KCWrx`D3~Djp`q`s2Kw?9@?)o}A$
zWcgpvlfUH9Uo@QVO0(l+fvT=MPJ`)RQuze!gs}FWfX001s!EZ
z361l47ZAr5Gimn29-8Jzkmn^dp61Dz)6lL(^4V$XOJ7RaX~=M;)ILLY+}!CiAWiwQ
z?1GZM;eU>1GaalL9+7PqD4$Nszc13#@?|
z5;Zb=#3}`x=ej+9k;ES16U5OT<;`j}aPw`RpnU4Q`5^vl7jI
zj%&}rrp4@I8*~cv;~fKZ(3XF`(c58tA82fTmre+5oM%M@a2C~>GKp+
z2AF-+Vejag>#(cUvUkWZ${plM^t;e?YCz*8`Z|4ygY3NyJ?$wzWpt6gk^5z|o_>{Y
zZ_r69F{a$4mQ2r#&9~L%)|KG9xcNAF<1XFtk8c9ucDto%*8Hm2(xqDcjKq61l%e)N
zJb+S774L`C0&?l_5PtPPvi>2}qC;}vA+=+zshA$oQ@DGt#}r3BjQGb?$qfSQ@QjYb
z0|h^)7{9xVaGY}MKAPL1z8#KSbDUcw{hw1`p1sJ}_8g9g1{h~vP*RYHCuB-QB)M
z<@_yvriXDMpYPuDBq9HosBLy@g3D{gZLB3PX}K{ZS~j2
zta_ZvbW~2%=g9ik74DD2aK8N2Yrv@^;AahZd9Cv$32P*>0cX-<+1P+T
zrZq;rh7cK~)H9l&(?SXq0pl9D}41~lTlpcV=gvk=lKv@2KUh*bT2`|HxYF)#LY
zP0z^Iw5-l#eL|lE<+hMBjoIw$!tMOuiNM@I@r`5pEzyFTj8^+KCm{FTKjqboKBrt(+{KK(xk
z=RW&?2$y;>G9*vq%XfG&!KXCt%cEg`%ldLE9M0{&+(J2=di}tDUn8|2ZzJR`uMGfG
z+ojI{i1`~?IshCW6Q6>$-59btelOKljMdTdwM({u+QH~M(GwkW>kvxKC2*1bW$PAvn
z$IIzk`Dzr;!19pyp=Sq;Dx>))qSKN$2FH9ZlgDy8MM(Kr!9GGiRCpf(aQ9xHoD
z-gurvgN@D~z^71#WKZCpa8Q>f@O-2<6F!8yY9~8Ba`$L8Cb3LaMa4DV0-a!YHxBIFB@(L4%quH-Z)k$2#EBF0Z
z`O$T0JPEG-mUNrMAwe!58HH8b)tVeF-NjFeW0=gG#F0J`SY<6__t)NFMpY2fwno`lmA#aoPuS)1#a=U
z+_b<3>q*Tt+#l>AjsFN=G!l%KSf`nXXqHZG2bhOU8kge--XNxFj#T;JBhHH+{3g!Y
z4ZPV?m1k)k;L@6~k%jJdxJs=O0==77$tARO=|7pn&7()4-~rKTnrZeApk`mHNI3jm
zsdmO<1z9WKOy*&<)SxLmkl~jH|C{Gm->L9+R(CpAm*Xblj@l2+_XLlQL{7ze!i+
z`>A}PmjBzhCA+#O$F_H0-78Y$TWWFTYEI`WKV#iAs5?D19!-aXY10{eqFf7f`cFYG
z?h%x}){e30<|-m(9y{V)U}sN1+nrUd(0BIqL9o8FD#aK*gLjaxf{iJPwcX2_ZJviE
z(C1)l429vo`6UP?Ux9=5ED!i`)J0OxvowT7y71L`hREDVoW*0z2bmU?4JUt2q;-
z5H3qq9-b_RX7N1u+J06J9~9aFXK@?&-@`RmfN(EJ#uKM^sWOh-O+h^s)J;JvG|@g>
zLGM)t^zyG&|G}NApr;j7ljBx?BWNK2c2%@FUT=$vG%u}w~9ajD`%@xx%
za(?&+bi0jOVVa!{0p@J@2VCp|gV2@=LSKMT(Dr|(zy5oAU{*@%fq#xi
zryLs{nh1tQ)p0uG@*LXTm(H095jcdVd21H0Ligtop6a`bkEQbMr!cNMvh!28?s
zQ>ghzQgaScs)v$2hr^qEtElaQPT2DQmf2PXy5{ck-KR22(%f^bFmGF`E4f0PI5_
zXs!=l2lTtD#*xYs
zbp-B4;6BRtbYa#4z(u8tn)zH1t>wDK8~9TdbUL8u7peY-=x5wC&9*^wfL)aK
z^T6d}={t`@f=!)(QD0%tHmiq-Abe#tfYw|V%!73M$_CYZAp7SbI$P!XJPtK2Rf{AI
zn2mt>O#CzX9Q@3~O#T66+KdnevVFg-_$@(J@QZX=f^79`VO3l
zjIH1~PK=kAf%({NmK<8bp|356MwC8AIb`1=kCyPN+Gj!7CGrBbLrG^PBU`fB(`4ko
z+Tfw=5boq3-o;f+)pNXE+4-AQ~k#mU0xel6FhkuXRUYl=+`Bz0!oBp)sWOZkQ
z0CfEECLE&%sN?76nl?Mnb=i{?`emTZldPqDl)f>Be2FY4>=(dG55IsKBV2$7SMjrO
zQWbT>$7IVg{*tB}UBBWdRBdNhla`0C2;V_`M~zpdt+VPCz{?4MtZW(mD+f~_$@rBwpt@+1%OhAzHonZ|PZ*&?O8CPr
zevF)J`EKrk=I(BO2Tf8w5Ajg1SD6WGWph66e2qNKhf<{*fd%aDUQM~*%$Rog%_y(SvkxI!el6b};%*Ukpk$>)-9(9Z+p!f@
z_jWJrkTX%)!4UO$Q~fZ1@LGyWnZw@cotcm<^AB^K8H!siEhIM-?Qg>|`>G--oTX+I
zU`$@D)vk`F(4rJtszOt#$o=mM)76Xu3>uuja)5U1{
zdKEDbHI_3@Vvpd46J`GqK7@lUJIWt0TCcyj31na-9Oqd~&5XQajzTM?r+5$*NaqsR
za9bHwf_%(hN=vxUpx!;LH+*b=+79Bm>hsG`Csm@RLZylweA6{_!@Jhw8|b>3ZRZ^~
z(aQ-$%5^)_{{k@fsbi-#i-T$SCh7WniZl;6{qj|dlKO^B8qr;)Ws%azky7mfEB
z6a}%;R;LcP9bL>_O`b{
z=qt?uI!gXpf1`VChxuxyVG7GGbuaJ)n2cP&;h#wE1?Y6NoKz#vB^S8^ikifW+zA=~
z{EH}KHpqdCDA_lN*Clw#bYtKp-lXo(`zlB2>J79MNDJkoQY3uw@>MAxLn$`lI^U$R
zGN=rtSiEGH!8OE7K^eDH@vsc07B3xdV2^ZJa07KovY2mj0f?2}d*q{gTn5t_dY_k}d2yeuG+$;u
z;AWL>MM8ds^6~)(%AXIo4jyZsJm7WUZuLWK)=>^Vga;leZjX3VReLQU?=OWWAGIcU
zO5P(>$tJn+h~>2fwK4U?0STU$1&_Ir_f}j}M#;zOt@x1ad(8dupkOZN1LUJ-%Bsg5
zwH24G#Ox`dr<@p~HvoZ_S8>PgD_}J$$1D9qsfqgv8S)fiW14J!%8@B=U35B4MseFP
[Qemu-devel] [PULL 0/1] Seabios 1.12 20181120 patches
The following changes since commit cb968d275c145467c8b385a3618a207ec111eab1: Update version for v3.1.0-rc1 release (2018-11-13 18:16:14 +) are available in the git repository at: git://git.kraxel.org/qemu tags/seabios-1.12-20181120-pull-request for you to fetch changes up to af51dbed380b7eec0c815da1c37b46e57a909ce8: update seabios to 1.12 (2018-11-20 06:57:53 +0100) seabios: update to 1.12-final Gerd Hoffmann (1): update seabios to 1.12 pc-bios/bios-256k.bin | Bin 262144 -> 262144 bytes pc-bios/bios.bin | Bin 131072 -> 131072 bytes pc-bios/vgabios-bochs-display.bin | Bin 27648 -> 27648 bytes pc-bios/vgabios-cirrus.bin| Bin 38400 -> 38400 bytes pc-bios/vgabios-qxl.bin | Bin 38912 -> 38912 bytes pc-bios/vgabios-ramfb.bin | Bin 28160 -> 28160 bytes pc-bios/vgabios-stdvga.bin| Bin 38912 -> 38912 bytes pc-bios/vgabios-virtio.bin| Bin 38912 -> 38912 bytes pc-bios/vgabios-vmware.bin| Bin 38912 -> 38912 bytes pc-bios/vgabios.bin | Bin 38400 -> 38400 bytes roms/Makefile | 2 +- roms/seabios | 2 +- 12 files changed, 2 insertions(+), 2 deletions(-) -- 2.9.3
Re: [Qemu-devel] [PATCH] qom: avoid reporting errors for NULL error object
Hi
On Mon, Nov 19, 2018 at 5:59 PM Daniel P. Berrangé wrote:
>
> When debugging QEMU it is often useful to put a breakpoint on the
> error_setg_internal method impl.
>
> Unfortunately the object_property_add / object_class_property_add
> methods call object_property_find / object_class_property_find methods
> to check if a property exists already before adding the new property.
>
> As a result there are a huge number of calls to error_setg_internal
> on startup of most QEMU commands, making it very painful to set a
> breakpoint on this method.
>
> This puts a minor optimization on the code so that we avoid calling
> error_setg() when errp is NULL. Functionally there's no difference
> since error_setg() is a no-op when errp is NULL, but this lets us
> use breakpoints in GDB in a practical way.
>
> Signed-off-by: Daniel P. Berrangé
Or maybe change the error_setg* macros? After all, if errp is NULL,
it's an error qemu can handle.
> ---
> qom/object.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/qom/object.c b/qom/object.c
> index 547dcf97c3..ddd5e7a30e 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1087,7 +1087,12 @@ ObjectProperty *object_property_find(Object *obj,
> const char *name,
> return prop;
> }
>
> -error_setg(errp, "Property '.%s' not found", name);
> +/* Optimized to avoid calling error_setg if errp == NULL
> + * otherwise every property add call hits error_setg
> + * making it impratical to set breakpoints in GDB */
> +if (errp) {
> +error_setg(errp, "Property '.%s' not found", name);
> +}
> return NULL;
> }
>
> @@ -1133,7 +1138,10 @@ ObjectProperty *object_class_property_find(ObjectClass
> *klass, const char *name,
> }
>
> prop = g_hash_table_lookup(klass->properties, name);
> -if (!prop) {
> +/* Optimized to avoid calling error_setg if errp == NULL
> + * otherwise every property add call hits error_setg
> + * making it impratical to set breakpoints in GDB */
> +if (!prop && errp) {
> error_setg(errp, "Property '.%s' not found", name);
> }
> return prop;
> --
> 2.19.1
>
>
--
Marc-André Lureau
Re: [Qemu-devel] [PATCH for-4.0 v2] virtio: Provide version-specific variants of virtio PCI devices
On Mon, Nov 19, 2018 at 07:47:40PM -0200, Eduardo Habkost wrote: > On Mon, Nov 19, 2018 at 01:07:59PM -0500, Michael S. Tsirkin wrote: > n > > On Mon, Nov 19, 2018 at 11:41:05AM +0100, Cornelia Huck wrote: > > > On Fri, 16 Nov 2018 01:45:51 -0200 > > > Eduardo Habkost wrote: > > > > > > > On Thu, Nov 15, 2018 at 05:29:24PM +0100, Andrea Bolognani wrote: > > > > > > > > One thing that I'm very much not convinced about is the naming, > > > > > specifically leaving the virtio revision out: I get it that we > > > > > Should Never Need™ another major version of the spec, but I'm > > > > > afraid discounting the possibility outright might prove to be > > > > > shortsighted and come back to bite us later, so I'd much rather > > > > > keep it. > > > > That's not the claim. In fact the reverse is true - a major revision can > > come at any point. The claim is that virtio compatibility is not based > > on version numbers. And another claim is that you can trust the > > virtio TC not to overload terminology that it put in the spec. So use > > that and you should be fine. Come up with your own and end up writing > > another spec just to document it. > > > > > > > > > > > > And once that's done, "non-transitional" (while matching the > > > > > language of the spec) starts to look a bit unnecessary when you > > > > > could simply have > > > > > > > > > > virtio-*-pci > > > > > virtio-*-pci-1 > > > > > virtio-*-pci-1-transitional > > > > > > > > > > instead. But I don't feel as strongly about this as I do about > > > > > keeping the virtio revision in the device name :) > > > > > > > > I like that suggestion. Makes the device names more explicit > > > > _and_ shorter. I'll do that in v3. > > > > > > OTOH, that would mean we'd need to introduce new device types if we > > > ever start to support a virtio 2.x standard. My understanding was that > > > we'll want separate device types for transitional and non-transitional > > > for two reasons: the bus which a device can be plugged into, and > > > changing ids. Do we really expect huge changes in a possible 2.x > > > standard that affect virtio-pci only, and not other virtio transports > > > as well? > > > > Yes I think adding a version there is a mistake. > > transitional/legacy/non-transitional are spec terms so > > they are unlikely to change abruptly. OTOH virtio TC can > > just decide next version is 2.0 on a drop of a hat. > > > > And I strongly believe command line users really really do not want all > > this mess. Even adding "pci" is the name confuses people (what are the > > other options?). For command line model=virtio is pretty much perfect. > > So all these names are there primarily for libvirt's benefit. > > And the only input from libvirt devs so far > > has been that it's unclear how does cross version > > migration work. That needs to be addressed in some way. > > What still needs to be addressed? I don't belive you answered Daniel's question. > Just keep the existing device > types on migration. We could make additional promises about > compatibility with the disable-modern and disable-legacy > properties, but I don't see why we need to do it unless we start > deprecating the old device types. Then the answer seems to be in the negative? > > > > > So can we maybe start with a libvirt domain xml patch, with an > > implementation for existing QEMU, get that acked, and then just map it > > back to qemu command line as directly as possible? > > I don't know what you mean here by "libvirt domain XML patch". > > Do you mean solving the problems only on the libvirt side, > keeping the existing device types? Why would we do that? It > would be a hack making the situation even messier. > > If libvirt needs us to provide better interfaces, let's cooperate > with them. I'd like us to avoid having yet another "the problem > must be solved in the other layer first" deadlock here. I mean IIUC libvirt is the solve user that will benefit from this patch. Let's at least get an ack confirming it does make their lives easier. > -- > Eduardo
Re: [Qemu-devel] [PATCH for-4.0 v2] virtio: Provide version-specific variants of virtio PCI devices
On Mon, Nov 19, 2018 at 10:08:42PM -0500, Michael S. Tsirkin wrote: > On Mon, Nov 19, 2018 at 07:47:40PM -0200, Eduardo Habkost wrote: > > On Mon, Nov 19, 2018 at 01:07:59PM -0500, Michael S. Tsirkin wrote: > > n > > > On Mon, Nov 19, 2018 at 11:41:05AM +0100, Cornelia Huck wrote: > > > > On Fri, 16 Nov 2018 01:45:51 -0200 > > > > Eduardo Habkost wrote: > > > > > > > > > On Thu, Nov 15, 2018 at 05:29:24PM +0100, Andrea Bolognani wrote: > > > > > > > > > > One thing that I'm very much not convinced about is the naming, > > > > > > specifically leaving the virtio revision out: I get it that we > > > > > > Should Never Need™ another major version of the spec, but I'm > > > > > > afraid discounting the possibility outright might prove to be > > > > > > shortsighted and come back to bite us later, so I'd much rather > > > > > > keep it. > > > > > > That's not the claim. In fact the reverse is true - a major revision can > > > come at any point. The claim is that virtio compatibility is not based > > > on version numbers. And another claim is that you can trust the > > > virtio TC not to overload terminology that it put in the spec. So use > > > that and you should be fine. Come up with your own and end up writing > > > another spec just to document it. > > > > > > > > > > > > > > > And once that's done, "non-transitional" (while matching the > > > > > > language of the spec) starts to look a bit unnecessary when you > > > > > > could simply have > > > > > > > > > > > > virtio-*-pci > > > > > > virtio-*-pci-1 > > > > > > virtio-*-pci-1-transitional > > > > > > > > > > > > instead. But I don't feel as strongly about this as I do about > > > > > > keeping the virtio revision in the device name :) > > > > > > > > > > I like that suggestion. Makes the device names more explicit > > > > > _and_ shorter. I'll do that in v3. > > > > > > > > OTOH, that would mean we'd need to introduce new device types if we > > > > ever start to support a virtio 2.x standard. My understanding was that > > > > we'll want separate device types for transitional and non-transitional > > > > for two reasons: the bus which a device can be plugged into, and > > > > changing ids. Do we really expect huge changes in a possible 2.x > > > > standard that affect virtio-pci only, and not other virtio transports > > > > as well? > > > > > > Yes I think adding a version there is a mistake. > > > transitional/legacy/non-transitional are spec terms so > > > they are unlikely to change abruptly. OTOH virtio TC can > > > just decide next version is 2.0 on a drop of a hat. > > > > > > And I strongly believe command line users really really do not want all > > > this mess. Even adding "pci" is the name confuses people (what are the > > > other options?). For command line model=virtio is pretty much perfect. > > > So all these names are there primarily for libvirt's benefit. > > > And the only input from libvirt devs so far > > > has been that it's unclear how does cross version > > > migration work. That needs to be addressed in some way. > > > > What still needs to be addressed? > > I don't belive you answered Daniel's question. > > > Just keep the existing device > > types on migration. We could make additional promises about > > compatibility with the disable-modern and disable-legacy > > properties, but I don't see why we need to do it unless we start > > deprecating the old device types. > > Then the answer seems to be in the negative? If the question is about the current state, it's "yes". If it's about promises about the future, then we need to understand what kind of promise is expected. > > > > > > So can we maybe start with a libvirt domain xml patch, with an > > > implementation for existing QEMU, get that acked, and then just map it > > > back to qemu command line as directly as possible? > > > > I don't know what you mean here by "libvirt domain XML patch". > > > > Do you mean solving the problems only on the libvirt side, > > keeping the existing device types? Why would we do that? It > > would be a hack making the situation even messier. > > > > If libvirt needs us to provide better interfaces, let's cooperate > > with them. I'd like us to avoid having yet another "the problem > > must be solved in the other layer first" deadlock here. > > I mean IIUC libvirt is the solve user that will benefit from this patch. > Let's at least get an ack confirming it does make their lives easier. I understand this as an ACK; https://www.mail-archive.com/qemu-devel@nongnu.org/msg567161.html Quoted below: | I don't have a objection from libvirt side. | | Last time, I suggested/discussed this I was not convinced that the benefit | was compelling enough to justify the work across all levels in the stack. | | Apps using the new device model names would either make themselves | incompatible with older libvirt/QEMU, or they would increase their | code complexity & testing matrix by having to support both old & new
[Qemu-devel] [PATCH 2/2] target/xtensa: drop num_[core_]regs from dc232b/dc233c configs
Now that xtensa_count_regs does the right thing, remove manual
initialization of these fields from the affected configurations and let
xtensa_finalize_config initialize them. Add XTREG_END to terminate
register lists.
Signed-off-by: Max Filippov
---
target/xtensa/core-dc232b.c| 2 --
target/xtensa/core-dc232b/gdb-config.inc.c | 1 +
target/xtensa/core-dc233c.c| 2 --
target/xtensa/core-dc233c/gdb-config.inc.c | 1 +
4 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/target/xtensa/core-dc232b.c b/target/xtensa/core-dc232b.c
index 71313378409e..7851bcb63687 100644
--- a/target/xtensa/core-dc232b.c
+++ b/target/xtensa/core-dc232b.c
@@ -40,8 +40,6 @@
static XtensaConfig dc232b __attribute__((unused)) = {
.name = "dc232b",
.gdb_regmap = {
-.num_regs = 120,
-.num_core_regs = 52,
.reg = {
#include "core-dc232b/gdb-config.inc.c"
}
diff --git a/target/xtensa/core-dc232b/gdb-config.inc.c
b/target/xtensa/core-dc232b/gdb-config.inc.c
index 13aba5edecd6..d87168628be8 100644
--- a/target/xtensa/core-dc232b/gdb-config.inc.c
+++ b/target/xtensa/core-dc232b/gdb-config.inc.c
@@ -259,3 +259,4 @@
0, 0, 0, 0, 0, 0)
XTREG(119, 476, 32, 4, 4, 0x000f, 0x0006, -2, 8, 0x0100, a15,
0, 0, 0, 0, 0, 0)
+ XTREG_END
diff --git a/target/xtensa/core-dc233c.c b/target/xtensa/core-dc233c.c
index d701e3f5de07..8853bfd4d08f 100644
--- a/target/xtensa/core-dc233c.c
+++ b/target/xtensa/core-dc233c.c
@@ -40,8 +40,6 @@
static XtensaConfig dc233c __attribute__((unused)) = {
.name = "dc233c",
.gdb_regmap = {
-.num_regs = 121,
-.num_core_regs = 52,
.reg = {
#include "core-dc233c/gdb-config.inc.c"
}
diff --git a/target/xtensa/core-dc233c/gdb-config.inc.c
b/target/xtensa/core-dc233c/gdb-config.inc.c
index b632341b28ec..7e8963227fc0 100644
--- a/target/xtensa/core-dc233c/gdb-config.inc.c
+++ b/target/xtensa/core-dc233c/gdb-config.inc.c
@@ -143,3 +143,4 @@ XTREG(117, 468, 32, 4, 4, 0x000c, 0x0006, -2, 8, 0x0100,
a12, 0, 0, 0, 0
XTREG(118, 472, 32, 4, 4, 0x000d, 0x0006, -2, 8, 0x0100, a13, 0, 0, 0,
0, 0, 0)
XTREG(119, 476, 32, 4, 4, 0x000e, 0x0006, -2, 8, 0x0100, a14, 0, 0, 0,
0, 0, 0)
XTREG(120, 480, 32, 4, 4, 0x000f, 0x0006, -2, 8, 0x0100, a15, 0, 0, 0,
0, 0, 0)
+XTREG_END
--
2.11.0
[Qemu-devel] [PATCH 0/2] target/xtensa fixes for 3.1
Hello, the following two patches fix gdbserver register counting for xtensa-softmmu and xtensa-linux and drops explicit register counters from DC232B and DC233C core configurations. Max Filippov (2): target/xtensa: gdbstub fix register counting target/xtensa: drop num_[core_]regs from dc232b/dc233c configs target/xtensa/core-dc232b.c| 2 -- target/xtensa/core-dc232b/gdb-config.inc.c | 1 + target/xtensa/core-dc233c.c| 2 -- target/xtensa/core-dc233c/gdb-config.inc.c | 1 + target/xtensa/gdbstub.c| 11 --- 5 files changed, 10 insertions(+), 7 deletions(-) -- 2.11.0
[Qemu-devel] [PATCH 1/2] target/xtensa: gdbstub fix register counting
In order to communicate correctly with gdb xtensa gdbstub must provide
expected number of registers in 'g' packet response. xtensa-elf-gdb
expects both nonprivileged and privileged registers. xtensa-linux-gdb
only expects nonprivileged registers. gdb only counts one contiguous
stretch of registers, do the same for the core registers in the
xtensa_count_regs.
With this change qemu-system-xtensa is able to communicate with all
xtensa-elf-gdb versions (versions prior to 8.2 require overlay fixup),
and qemu-xtensa is able to communicate with all xtensa-linux-gdb
versions, except 8.2.
Signed-off-by: Max Filippov
---
target/xtensa/gdbstub.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/target/xtensa/gdbstub.c b/target/xtensa/gdbstub.c
index c9450914c72d..d43bb190c614 100644
--- a/target/xtensa/gdbstub.c
+++ b/target/xtensa/gdbstub.c
@@ -45,15 +45,20 @@ void xtensa_count_regs(const XtensaConfig *config,
unsigned *n_regs, unsigned *n_core_regs)
{
unsigned i;
+bool count_core_regs = true;
for (i = 0; config->gdb_regmap.reg[i].targno >= 0; ++i) {
if (config->gdb_regmap.reg[i].type != xtRegisterTypeTieState &&
config->gdb_regmap.reg[i].type != xtRegisterTypeMapped &&
config->gdb_regmap.reg[i].type != xtRegisterTypeUnmapped) {
++*n_regs;
-if ((config->gdb_regmap.reg[i].flags &
- XTENSA_REGISTER_FLAGS_PRIVILEGED) == 0) {
-++*n_core_regs;
+if (count_core_regs) {
+if ((config->gdb_regmap.reg[i].flags &
+ XTENSA_REGISTER_FLAGS_PRIVILEGED) == 0) {
+++*n_core_regs;
+} else {
+count_core_regs = false;
+}
}
}
}
--
2.11.0
Re: [Qemu-devel] [PATCH for-3.2 38/41] net: do not depend on slirp internals
Philippe Mathieu-Daudé, le mer. 14 nov. 2018 14:21:36 +0100, a ecrit: > > Signed-off-by: Marc-André Lureau > > Reviewed-by: Philippe Mathieu-Daudé > Tested-by: Philippe Mathieu-Daudé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 37/41] slirp: replace ARRAY_SIZE with G_N_ELEMENTS
Philippe Mathieu-Daudé, le mer. 14 nov. 2018 14:17:56 +0100, a ecrit: > On 14/11/18 13:36, Marc-André Lureau wrote: > > Do not require QEMU macro. > > > > Signed-off-by: Marc-André Lureau > > Reviewed-by: Philippe Mathieu-Daudé Daniel P. Berrangé, le mer. 14 nov. 2018 14:15:15 +, a ecrit: > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 36/41] slirp: remove dead TCP_ACK_HACK code
Daniel P. Berrangé, le mer. 14 nov. 2018 14:12:06 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:38PM +0400, Marc-André Lureau wrote: > > Untouched since original introduction in 2004. > > > > Signed-off-by: Marc-André Lureau > > --- > > slirp/tcp_input.c | 23 +-- > > 1 file changed, 1 insertion(+), 22 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 35/41] slirp: NULL is defined by glib (at least)
Daniel P. Berrangé, le mer. 14 nov. 2018 14:11:34 +, a ecrit: > NULL is defined in stddef.h for at least 15 years AFAICT, so > even glib shouldn't bother to define it :-) So I'd suggest > changing $subject to just defer to stddef.h > > On Wed, Nov 14, 2018 at 04:36:37PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp.h | 4 > > 1 file changed, 4 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 33/41] slirp: replace qemu_notify_event() with a callback
Marc-André Lureau, le mer. 14 nov. 2018 16:36:35 +0400, a ecrit:
> Use a "slirp socket" helper function to call the callback when
> sbdrop() returns true.
It's really far from clear what this should be doing :)
I'd say find out how to rephrase 86073017e384 ("slirp: Signal free input
buffer space to io-thread") in a generic way.
Samuel
Re: [Qemu-devel] [PATCH for-3.2 32/41] slirp: remove unused sbflush()
Daniel P. Berrangé, le mer. 14 nov. 2018 14:05:33 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:34PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/sbuf.h | 1 - > > 1 file changed, 1 deletion(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 34/41] slirp: remove #if notdef dead code
Daniel P. Berrangé, le mer. 14 nov. 2018 14:07:38 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:36PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/ip_input.c | 200 -- > > slirp/tcp_input.c | 39 - > > 2 files changed, 239 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 31/41] slirp: add a callback to log guest errors
Marc-André Lureau, le mer. 14 nov. 2018 16:36:33 +0400, a ecrit: > Signed-off-by: Marc-André Lureau Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH 07/22] gpio/puv3_gpio: Convert sysbus initfunction to realize function
On 11/19/18 10:31 PM, Peter Maydell wrote:
On 19 November 2018 at 12:08, Mao Zhongyi
wrote:
Use DeviceClass rather than SysBusDeviceClass in
puv3_gpio_class_init().
Cc: g...@mprc.pku.edu.cn
Signed-off-by: Mao Zhongyi
Signed-off-by: Zhang Shengju
---
hw/gpio/puv3_gpio.c | 28 +---
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/hw/gpio/puv3_gpio.c b/hw/gpio/puv3_gpio.c
index 445afccf9f..bd6fc43aae 100644
--- a/hw/gpio/puv3_gpio.c
+++ b/hw/gpio/puv3_gpio.c
@@ -99,7 +99,7 @@ static const MemoryRegionOps puv3_gpio_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static int puv3_gpio_init(SysBusDevice *dev)
+static void puv3_gpio_realize(DeviceState *dev, Error **errp)
{
PUV3GPIOState *s = PUV3_GPIO(dev);
@@ -107,28 +107,26 @@ static int puv3_gpio_init(SysBusDevice *dev)
s->reg_GPDR = 0;
/* FIXME: these irqs not handled yet */
-sysbus_init_irq(dev, >irq[PUV3_IRQS_GPIOLOW0]);
-sysbus_init_irq(dev, >irq[PUV3_IRQS_GPIOLOW1]);
-sysbus_init_irq(dev, >irq[PUV3_IRQS_GPIOLOW2]);
-sysbus_init_irq(dev, >irq[PUV3_IRQS_GPIOLOW3]);
-sysbus_init_irq(dev, >irq[PUV3_IRQS_GPIOLOW4]);
-sysbus_init_irq(dev, >irq[PUV3_IRQS_GPIOLOW5]);
-sysbus_init_irq(dev, >irq[PUV3_IRQS_GPIOLOW6]);
-sysbus_init_irq(dev, >irq[PUV3_IRQS_GPIOLOW7]);
-sysbus_init_irq(dev, >irq[PUV3_IRQS_GPIOHIGH]);
+sysbus_init_irq(SYS_BUS_DEVICE(dev), >irq[PUV3_IRQS_GPIOLOW0]);
+sysbus_init_irq(SYS_BUS_DEVICE(dev), >irq[PUV3_IRQS_GPIOLOW1]);
+sysbus_init_irq(SYS_BUS_DEVICE(dev), >irq[PUV3_IRQS_GPIOLOW2]);
+sysbus_init_irq(SYS_BUS_DEVICE(dev), >irq[PUV3_IRQS_GPIOLOW3]);
+sysbus_init_irq(SYS_BUS_DEVICE(dev), >irq[PUV3_IRQS_GPIOLOW4]);
+sysbus_init_irq(SYS_BUS_DEVICE(dev), >irq[PUV3_IRQS_GPIOLOW5]);
+sysbus_init_irq(SYS_BUS_DEVICE(dev), >irq[PUV3_IRQS_GPIOLOW6]);
+sysbus_init_irq(SYS_BUS_DEVICE(dev), >irq[PUV3_IRQS_GPIOLOW7]);
+sysbus_init_irq(SYS_BUS_DEVICE(dev), >irq[PUV3_IRQS_GPIOHIGH]);
memory_region_init_io(>iomem, OBJECT(s), _gpio_ops, s,
"puv3_gpio",
PUV3_REGS_OFFSET);
-sysbus_init_mmio(dev, >iomem);
-
-return 0;
+sysbus_init_mmio(SYS_BUS_DEVICE(dev), >iomem);
}
The SYS_BUS_DEVICE() cast is not free (it does type
checking). It's better to do it once, ie
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
and use the variable, if we're going to be using
it several times.
Wow, sure，thanks a lot, will fix it in the next.
mao
thanks
-- PMM
Re: [Qemu-devel] [PATCH for-3.2 30/41] slirp: replace trace functions with DEBUG calls
Marc-André Lureau, le mer. 14 nov. 2018 16:36:32 +0400, a ecrit: > Remove a dependency on QEMU. Use the existing logging facilities. I'm hesitating on this one: this is not really a debugging print (as in qemu debugging), but a real user debugging print. We do want to compile this in without DEBUG defined. Samuel
Re: [Qemu-devel] [PATCH for-3.2 29/41] slirp: improve a bit the debug macros
Marc-André Lureau, le mer. 14 nov. 2018 16:36:31 +0400, a ecrit: > Let them accept multiple arguments. Simplify the inner argument > handling of DEBUG_ARGS/DEBUG_MISC_DEBUG_ERROR. > > Signed-off-by: Marc-André Lureau Applied to my tree, thanks!
[Qemu-devel] [PATCH] usb-host: set ifs.detached as true if kernel driver is not active
If no kernel driver is active, we can already claim and perform I/O on
it without detaching it.
Signed-off-by: linzhecheng
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index f31e9cbbb8..db4ae1e6e8 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -1119,6 +1119,10 @@ static void usb_host_detach_kernel(USBHostDevice *s)
for (i = 0; i < USB_MAX_INTERFACES; i++) {
rc = libusb_kernel_driver_active(s->dh, i);
usb_host_libusb_error("libusb_kernel_driver_active", rc);
+if (rc == 0) {
+s->ifs[i].detached = true;
+continue;
+}
if (rc != 1) {
continue;
}
--
2.12.2.windows.2
Re: [Qemu-devel] [PATCH for-3.2 28/41] slirp: replace error_report() with g_critical()
Marc-André Lureau, le mer. 14 nov. 2018 16:36:30 +0400, a ecrit: > Reduce dependency on QEMU. QEMU could use a custom log handler if it > wants to redirect/filter it. > > Signed-off-by: Marc-André Lureau Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 26/41] slirp: replace compile time DO_KEEPALIVE
Marc-André Lureau, le mer. 14 nov. 2018 16:36:28 +0400, a ecrit: > Use a global variable instead (similar to slirp_debug) > > Signed-off-by: Marc-André Lureau Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 25/41] slirp: replace SIZEOF_CHAR_P with glib equivalent
Philippe Mathieu-Daudé, le mer. 14 nov. 2018 14:14:35 +0100, a ecrit: > On 14/11/18 13:36, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > Reviewed-by: Philippe Mathieu-Daudé Applied to my tree, thanks!
Re: [Qemu-devel] [qemu-s390x] [PATCH 21/22] event-facility: ChangeSysBusDeviceClass *sbdc to SysBusDeviceClass *sbc
On 11/19/18 10:10 PM, Thomas Huth wrote: On 2018-11-19 13:25, Cornelia Huck wrote: On Mon, 19 Nov 2018 20:08:19 +0800 Mao Zhongyi wrote: Most of the SysBusDeviceClass variables are named sbc, and sbdc here is a bit weird, so changing sbdc to keep it consistent with others might look good. A quick git grep also gives sbd and k as variable names, and it is used in a total of two lines which have not been touched since 2013, so... meh. If others like the change, I'm not opposed to merging, though. I think I agree with Cornelia, just changing the variable name because it's named differently in a couple of other places is just unnecessary code churn, which makes "git blame" output harder to read later. I'd suggest to drop this patch, too. Ok, I got it. :) Thanks, Mao Thomas
Re: [Qemu-devel] [PATCH for-3.2 27/41] slirp: remove unused global slirp_instance
Daniel P. Berrangé, le mer. 14 nov. 2018 13:55:18 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:29PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp.h | 2 -- > > 1 file changed, 2 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 21/41] slirp: remove HAVE_SYS_FILIO_H
Daniel P. Berrangé, le mer. 14 nov. 2018 13:52:12 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:23PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp.h| 2 +- > > slirp/slirp_config.h | 6 -- > > 2 files changed, 1 insertion(+), 7 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 24/41] slirp: replace HOST_WORDS_BIGENDIAN with glib equivalent
Philippe Mathieu-Daudé, le mer. 14 nov. 2018 14:14:16 +0100, a ecrit: > On 14/11/18 13:36, Marc-André Lureau wrote: > > One more step towards making the project independent from QEMU. > > > > Signed-off-by: Marc-André Lureau > > Reviewed-by: Philippe Mathieu-Daudé Daniel P. Berrangé, le mer. 14 nov. 2018 13:32:29 +, a ecrit: > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks! Samuel
Re: [Qemu-devel] [PATCH for-3.2 20/41] slirp: remove HAVE_SYS_IOCTL_H
Daniel P. Berrangé, le mer. 14 nov. 2018 13:51:29 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:22PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp.h| 2 +- > > slirp/slirp_config.h | 6 -- > > 2 files changed, 1 insertion(+), 7 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 23/41] slirp: remove unused HAVE_INET_ATON
Daniel P. Berrangé, le mer. 14 nov. 2018 13:54:33 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:25PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp_config.h | 6 -- > > 1 file changed, 6 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 22/41] slirp: remove unused DECLARE_IOVEC
Daniel P. Berrangé, le mer. 14 nov. 2018 13:53:31 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:24PM +0400, Marc-André Lureau wrote: > > It's actually qemu configure CONFIG_IOVEC that is being used. > > That makes it sound like slirp is using CONFIG_IOVEC, but AFAICT > that's only used by QEMU's osdep code. > > > > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp_config.h | 6 -- > > 1 file changed, 6 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 19/41] slirp: remove unused HAVE_SYS_SELECT_H
Daniel P. Berrangé, le mer. 14 nov. 2018 13:51:01 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:21PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/main.h | 4 > > slirp/slirp.h| 4 > > slirp/slirp_config.h | 6 -- > > 3 files changed, 14 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 16/41] slirp: remove unused HAVE_SYS_STROPTS_H
Daniel P. Berrangé, le mer. 14 nov. 2018 13:49:08 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:18PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp.h| 5 - > > slirp/slirp_config.h | 3 --- > > 2 files changed, 8 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 18/41] slirp: remove unused HAVE_SYS_WAIT_H
Daniel P. Berrangé, le mer. 14 nov. 2018 13:50:33 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:20PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp.h| 4 > > slirp/slirp_config.h | 3 --- > > 2 files changed, 7 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 17/41] slirp: remove unused HAVE_ARPA_INET_H
Daniel P. Berrangé, le mer. 14 nov. 2018 13:50:01 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:19PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp_config.h | 6 -- > > 1 file changed, 6 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 15/41] slirp: remove NO_UNIX_SOCKETS
Daniel P. Berrangé, le mer. 14 nov. 2018 13:48:37 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:17PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp.h| 3 --- > > slirp/slirp_config.h | 6 -- > > 2 files changed, 9 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 14/41] slirp: remove unused HAVE_SYS_BITYPES_H
Daniel P. Berrangé, le mer. 14 nov. 2018 13:47:57 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:16PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp.h| 4 > > slirp/slirp_config.h | 3 --- > > 2 files changed, 7 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 13/41] slirp: remove HAVE_SYS_SIGNAL_H
Daniel P. Berrangé, le mer. 14 nov. 2018 13:47:32 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:15PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp.h| 3 --- > > slirp/slirp_config.h | 3 --- > > 2 files changed, 6 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
[Qemu-devel] [PATCH] slirp: Enable fork_exec support on Windows
g_spawn_async_with_fds is portable on Windows, so we can now enable
fork_exec support there.
Thanks Daniel P. Berrangé for the notice!
Signed-off-by: Samuel Thibault
---
slirp/misc.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/slirp/misc.c b/slirp/misc.c
index 7972b9b05b..59b4e8f31c 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -62,17 +62,6 @@ int add_exec(struct ex_list **ex_ptr, void *chardev, const
char *cmdline,
}
-#ifdef _WIN32
-
-int
-fork_exec(struct socket *so, const char *ex)
-{
-/* not implemented */
-return 0;
-}
-
-#else
-
static int
slirp_socketpair_with_oob(int sv[2])
{
@@ -132,7 +121,9 @@ err:
static void
fork_exec_child_setup(gpointer data)
{
+#ifndef _WIN32
setsid();
+#endif
}
int
@@ -177,7 +168,6 @@ fork_exec(struct socket *so, const char *ex)
qemu_set_nonblock(so->s);
return 1;
}
-#endif
char *slirp_connection_info(Slirp *slirp)
{
--
2.19.1
Re: [Qemu-devel] [PATCH for-4.0 v2] virtio: Provide version-specific variants of virtio PCI devices
On Thu, Nov 15, 2018 at 11:50:56AM +0100, Cornelia Huck wrote:
> On Thu, 15 Nov 2018 10:05:59 +
> Daniel P. Berrangé wrote:
>
> > On Wed, Nov 14, 2018 at 09:38:31PM -0200, Eduardo Habkost wrote:
> > > Many of the current virtio-*-pci device types actually represent
> > > 3 different types of devices:
> > > * virtio 1.0 non-transitional devices
> > > * virtio 1.0 transitional devices
> > > * virtio 0.9 ("legacy device" in virtio 1.0 terminology)
> > >
> > > That would be just an annoyance if it didn't break our device/bus
> > > compatibility QMP interfaces. With this multi-purpose device
> > > type, there's no way to tell management software that
> > > transitional devices and legacy devices require a Conventional
> > > PCI bus.
> > >
> > > The multi-purpose device types would also prevent us from telling
> > > management software what's the PCI vendor/device ID for them,
> > > because their PCI IDs change at runtime depending on the bus
> > > where they were plugged.
> > >
> > > This patch adds separate device types for each of those virtio
> > > device flavors:
> > >
> > > - virtio-*-pci: the existing multi-purpose device types
> > > - Configurable using `disable-legacy` and `disable-modern`
> > > properties
> > > - Legacy driver support is automatically enabled/disabled
> > > depending on the bus where it is plugged
> > > - Supports Conventional PCI and PCI Express buses
> > > (but Conventional PCI is incompatible with
> > > disable-legacy=off)
> > > - Changes PCI vendor/device IDs at runtime
> > > - virtio-*-pci-transitional: virtio-1.0 device supporting legacy drivers
>
> It's a virtio-1 (not 1.0) device. Otherwise, I like this terminology
> better.
>
> > > - Supports Conventional PCI buses only, because
> > > it has a PIO BAR
> >
> > Am I right in thinking that this is basically identical
> > to virtio-*-pci, aside from only being valid for PCI
> > buses ?
> >
> > IOW, libvirt can expose this device even if QEMU does
> > not support it, by simply using the existing device
> > type and only ever placing it in a PCI bus ?
> >
> > If libvirt did this compatibility approach, can you
> > confirm this would be live migration state compatible.
> >
> > ie can live migrate virtio-*-pci -> virtio-*-pci-transitional,
> > provided only PCI bus was used.
>
> It also needs to make sure that neither disable-legacy nor
> disable-modern is set. Then this would have a compatible state AFAICS.
>
> >
> > > - virtio-*-pci-non-transitional: modern-only
> > > - Supports both Conventional PCI and PCI Express buses
> >
> > IIUC, libvirt can again provide compatibility with old
> > QEMU by simply using the existing device type and setting
> > disable-legacy ? Can you confirm this would be live
> > migration compatible
> >
> > virtio-*-pci + disable-legacy -> virtio-*pci-non-transitional
>
> I think yes.
This is exactly how it is implemented internally, but I'm not
promising that this will be kept compatible forever. And I
wouldn't like to make that promise unless there's an important
use case for that.
We could easily ensure it will be compatible in pc-4.0 and older,
though. Would that be enough for the use case we have in mind?
--
Eduardo
[Qemu-devel] more serial ports on arm?
Hey guys,
I sort of lost track of the discussion, but what ever happened to
adding an extra serial port to the arm virt machine? I'm still
carrying around the attached patch to run build.wireguard.com and I'd
of course like to see a real solution upstream.
Jason
diff -ru qemu-3.0.0/hw/arm/virt.c qemu-3.0.0-modified/hw/arm/virt.c
--- qemu-3.0.0/hw/arm/virt.c 2018-08-14 21:10:34.0 +0200
+++ qemu-3.0.0-modified/hw/arm/virt.c 2018-09-14 11:48:31.914772294 +0200
@@ -672,13 +672,7 @@
qemu_fdt_setprop(vms->fdt, nodename, "clock-names",
clocknames, sizeof(clocknames));
-if (uart == VIRT_UART) {
-qemu_fdt_setprop_string(vms->fdt, "/chosen", "stdout-path", nodename);
-} else {
-/* Mark as not usable by the normal world */
-qemu_fdt_setprop_string(vms->fdt, nodename, "status", "disabled");
-qemu_fdt_setprop_string(vms->fdt, nodename, "secure-status", "okay");
-}
+qemu_fdt_setprop_string(vms->fdt, "/chosen", "stdout-path", nodename);
g_free(nodename);
}
@@ -1497,11 +1491,11 @@
fdt_add_pmu_nodes(vms);
+create_uart(vms, pic, VIRT_SECURE_UART, sysmem, serial_hd(1));
create_uart(vms, pic, VIRT_UART, sysmem, serial_hd(0));
if (vms->secure) {
create_secure_ram(vms, secure_sysmem);
-create_uart(vms, pic, VIRT_SECURE_UART, secure_sysmem, serial_hd(1));
}
vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64);
Re: [Qemu-devel] [PATCH 22/22] core/sysbus: remove the SysBusDeviceClass::init path
On Mon, Nov 19, 2018 at 09:31:39PM -0200, Eduardo Habkost wrote:
> On Mon, Nov 19, 2018 at 08:08:20PM +0800, Mao Zhongyi wrote:
> > Currently, all sysbus devices have been converted to realize(),
> > so remove this path.
> >
> > Cc: ehabk...@redhat.com
> > Cc: th...@redhat.com
> > Cc: pbonz...@redhat.com
> > Cc: arm...@redhat.com
> > Cc: peter.mayd...@linaro.org
> > Cc: richard.hender...@linaro.org
> > Cc: alistair.fran...@wdc.com
> >
> > Signed-off-by: Mao Zhongyi
> > Signed-off-by: Zhang Shengju
> > ---
> > hw/core/sysbus.c| 15 ---
> > include/hw/sysbus.h | 3 ---
> > 2 files changed, 18 deletions(-)
> >
> > diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> > index 7ac36ad3e7..030ad426c1 100644
> > --- a/hw/core/sysbus.c
> > +++ b/hw/core/sysbus.c
> > @@ -201,20 +201,6 @@ void sysbus_init_ioports(SysBusDevice *dev, uint32_t
> > ioport, uint32_t size)
> > }
> > }
> >
> > -/* TODO remove once all sysbus devices have been converted to realize */
> > -static void sysbus_realize(DeviceState *dev, Error **errp)
> > -{
> > -SysBusDevice *sd = SYS_BUS_DEVICE(dev);
> > -SysBusDeviceClass *sbc = SYS_BUS_DEVICE_GET_CLASS(sd);
> > -
> > -if (!sbc->init) {
> > -return;
> > -}
> > -if (sbc->init(sd) < 0) {
> > -error_setg(errp, "Device initialization failed");
> > -}
> > -}
>
> Nice. :)
>
>
> > -
> > DeviceState *sysbus_create_varargs(const char *name,
> > hwaddr addr, ...)
> > {
> > @@ -327,7 +313,6 @@ MemoryRegion *sysbus_address_space(SysBusDevice *dev)
> > static void sysbus_device_class_init(ObjectClass *klass, void *data)
> > {
> > DeviceClass *k = DEVICE_CLASS(klass);
> > -k->realize = sysbus_realize;
>
> Have you ensured this won't break any subclasses that
> saved the original realize function on a parent_realize field?
> Now they will have parent_realize set to NULL.
>
> Most of them use device_class_set_parent_realize() to implement
> that.
Found one:
$ ./aarch64-softmmu/qemu-system-aarch64 -machine virt,iommu=smmuv3
Segmentation fault (core dumped)
(gdb) bt
#0 0x in ()
#1 0x5596eabe in smmu_base_realize (dev=0x56ac0cb0,
errp=0x7fffc450) at
/home/ehabkost/rh/proj/virt/qemu/hw/arm/smmu-common.c:428
#2 0x55970322 in smmu_realize (d=0x56ac0cb0, errp=0x7fffc4c0)
at /home/ehabkost/rh/proj/virt/qemu/hw/arm/smmuv3.c:1390
#3 0x55ac8f00 in device_set_realized (obj=,
value=, errp=0x7fffc5b0) at hw/core/qdev.c:826
#4 0x55c424a7 in property_set_bool (obj=0x56ac0cb0, v=, name=, opaque=0x56a05b70, errp=0x7fffc5b0) at
qom/object.c:1991
#5 0x55c468df in object_property_set_qobject
(obj=obj@entry=0x56ac0cb0, value=value@entry=0x56ac2520,
name=name@entry=0x55de03f9 "realized", errp=errp@entry=0x7fffc5b0) at
qom/qom-qobject.c:27
#6 0x55c44355 in object_property_set_bool (obj=0x56ac0cb0,
value=, name=0x55de03f9 "realized", errp=0x7fffc5b0) at
qom/object.c:1249
#7 0x55ac7e22 in qdev_init_nofail (dev=dev@entry=0x56ac0cb0) at
hw/core/qdev.c:313
#8 0x5592ef97 in create_smmu (bus=0x569970a0, pic=0x7fffc900,
vms=) at /home/ehabkost/rh/proj/virt/qemu/hw/arm/virt.c:1058
#9 0x5592ef97 in create_pcie (pic=0x7fffc900, vms=)
at /home/ehabkost/rh/proj/virt/qemu/hw/arm/virt.c:1208
#10 0x5592ef97 in machvirt_init (machine=0x5663e9c0) at
/home/ehabkost/rh/proj/virt/qemu/hw/arm/virt.c:1549
#11 0x55acf013 in machine_run_board_init (machine=0x5663e9c0) at
hw/core/machine.c:834
#12 0x5581dab8 in main (argc=, argv=,
envp=) at vl.c:4518
>
> > k->bus_type = TYPE_SYSTEM_BUS;
> > /*
> > * device_add plugs devices into a suitable bus. For "real" buses,
> > diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
> > index 0b59a3b8d6..1aedcf05c9 100644
> > --- a/include/hw/sysbus.h
> > +++ b/include/hw/sysbus.h
> > @@ -38,9 +38,6 @@ typedef struct SysBusDevice SysBusDevice;
> > typedef struct SysBusDeviceClass {
> > /*< private >*/
> > DeviceClass parent_class;
> > -/*< public >*/
> > -
> > -int (*init)(SysBusDevice *dev);
> >
> > /*
> > * Let the sysbus device format its own non-PIO, non-MMIO unit address.
> > --
> > 2.17.1
> >
> >
> >
>
> --
> Eduardo
--
Eduardo
Re: [Qemu-devel] [PATCH 22/22] core/sysbus: remove the SysBusDeviceClass::init path
On Mon, Nov 19, 2018 at 08:08:20PM +0800, Mao Zhongyi wrote:
> Currently, all sysbus devices have been converted to realize(),
> so remove this path.
>
> Cc: ehabk...@redhat.com
> Cc: th...@redhat.com
> Cc: pbonz...@redhat.com
> Cc: arm...@redhat.com
> Cc: peter.mayd...@linaro.org
> Cc: richard.hender...@linaro.org
> Cc: alistair.fran...@wdc.com
>
> Signed-off-by: Mao Zhongyi
> Signed-off-by: Zhang Shengju
> ---
> hw/core/sysbus.c| 15 ---
> include/hw/sysbus.h | 3 ---
> 2 files changed, 18 deletions(-)
>
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index 7ac36ad3e7..030ad426c1 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -201,20 +201,6 @@ void sysbus_init_ioports(SysBusDevice *dev, uint32_t
> ioport, uint32_t size)
> }
> }
>
> -/* TODO remove once all sysbus devices have been converted to realize */
> -static void sysbus_realize(DeviceState *dev, Error **errp)
> -{
> -SysBusDevice *sd = SYS_BUS_DEVICE(dev);
> -SysBusDeviceClass *sbc = SYS_BUS_DEVICE_GET_CLASS(sd);
> -
> -if (!sbc->init) {
> -return;
> -}
> -if (sbc->init(sd) < 0) {
> -error_setg(errp, "Device initialization failed");
> -}
> -}
Nice. :)
> -
> DeviceState *sysbus_create_varargs(const char *name,
> hwaddr addr, ...)
> {
> @@ -327,7 +313,6 @@ MemoryRegion *sysbus_address_space(SysBusDevice *dev)
> static void sysbus_device_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *k = DEVICE_CLASS(klass);
> -k->realize = sysbus_realize;
Have you ensured this won't break any subclasses that
saved the original realize function on a parent_realize field?
Now they will have parent_realize set to NULL.
Most of them use device_class_set_parent_realize() to implement
that.
> k->bus_type = TYPE_SYSTEM_BUS;
> /*
> * device_add plugs devices into a suitable bus. For "real" buses,
> diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
> index 0b59a3b8d6..1aedcf05c9 100644
> --- a/include/hw/sysbus.h
> +++ b/include/hw/sysbus.h
> @@ -38,9 +38,6 @@ typedef struct SysBusDevice SysBusDevice;
> typedef struct SysBusDeviceClass {
> /*< private >*/
> DeviceClass parent_class;
> -/*< public >*/
> -
> -int (*init)(SysBusDevice *dev);
>
> /*
> * Let the sysbus device format its own non-PIO, non-MMIO unit address.
> --
> 2.17.1
>
>
>
--
Eduardo
[Qemu-devel] [Bug 1798451] Re: MMX emulation is missing on HVF Acceleration
** Summary changed: - HVF linux on OSX hangs 2nd time started after adding socket + MMX emulation is missing on HVF Acceleration ** Description changed: - Robs-MacBook-Pro-2:~ robmaskell$ qemu-system-x86_64 --version QEMU emulator version 3.0.0 Host: MacOS - 10.13.6 Model Name: MacBook Pro Model Identifier: MacBookPro14,3 Processor Name: Intel Core i7 Processor Speed:2.8 GHz Number of Processors: 1 Total Number of Cores: 4 L2 Cache (per Core):256 KB L3 Cache: 6 MB Memory: 16 GB Guest OS: Elementary Linux Loki 0.4.1, patched up to date Command used to start QEMU: qemu-system-x86_64 \ -name ElementaryLokiDev \ -machine pc,accel=hvf \ -cpu max \ -smp cpus=2,sockets=2,cores=1,threads=1,maxcpus=2 \ -numa node,nodeid=0 \ -numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=0,socket-id=1 \ -m 8G \ -vga vmware \ -hda e4.qcow2 Symptoms: Started without the -smp / -numa commands to install the OS, then added -smp / -numa and the machine boots and lscpu reports extra cpu as expected. Restart VM and it hangs on startup. Remove -smp / -numa and machine starts again. ** Tags added: hvf x86 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1798451 Title: MMX emulation is missing on HVF Acceleration Status in QEMU: New Bug description: Robs-MacBook-Pro-2:~ robmaskell$ qemu-system-x86_64 --version QEMU emulator version 3.0.0 Host: MacOS - 10.13.6 Model Name: MacBook Pro Model Identifier: MacBookPro14,3 Processor Name: Intel Core i7 Processor Speed:2.8 GHz Number of Processors: 1 Total Number of Cores: 4 L2 Cache (per Core):256 KB L3 Cache: 6 MB Memory: 16 GB Guest OS: Elementary Linux Loki 0.4.1, patched up to date Command used to start QEMU: qemu-system-x86_64 \ -name ElementaryLokiDev \ -machine pc,accel=hvf \ -cpu max \ -smp cpus=2,sockets=2,cores=1,threads=1,maxcpus=2 \ -numa node,nodeid=0 \ -numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=0,socket-id=1 \ -m 8G \ -vga vmware \ -hda e4.qcow2 Symptoms: Started without the -smp / -numa commands to install the OS, then added -smp / -numa and the machine boots and lscpu reports extra cpu as expected. Restart VM and it hangs on startup. Remove -smp / -numa and machine starts again. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1798451/+subscriptions
Re: [Qemu-devel] [PATCH 2/2] numa: Match struct to typedef name
On Thu, Nov 15, 2018 at 03:17:52PM -0600, Eric Blake wrote: > There's no reason to violate our naming conventions by having a > struct with a different name than its typedef. Messed up since > its introduction in commit 8c85901e, but made more obvious when > commit 3bfe5716 promoted it to typedefs.h. > > Signed-off-by: Eric Blake Queued on numa-next, thanks. -- Eduardo
Re: [Qemu-devel] [PATCH for-3.2 03/41] slirp: simplify fork_exec()
On 11/19/18 4:59 PM, Samuel Thibault wrote: Mmm, I don't think any portability issue remains. SO_OOBINLINE is used in other places, the rest is portable. There is the setsid() call which may just not make sense on Windows, but we could just disable it there. I have pushed to https://people.debian.org/~sthibault/qemu.git slirp-2-win what it could look like. I don't have a windows box off hand, could somebody try it to confirm that it builds? You could try 'make docker-test-mingw@fedora' to at least try building for a Windows environment (but that doesn't say much about testing the resulting binary). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH for-3.2 12/41] slirp: remove the disabled readv()/writev() code path
Daniel P. Berrangé, le mer. 14 nov. 2018 13:46:46 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:14PM +0400, Marc-André Lureau wrote: > > The soread() function may be used on datagram sockets, and would > > provide different behaviour if HAVE_READV was set, on empty datagrams. > > This looks like a minor optimization, that never has been a strong > > goal for slirp. > > > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp_config.h | 3 --- > > slirp/socket.c | 15 --- > > 2 files changed, 18 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 11/41] slirp: remove FULL_BOLT
Daniel P. Berrangé, le mer. 14 nov. 2018 13:46:02 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:13PM +0400, Marc-André Lureau wrote: > > Looking at git history, this looks like something from the past, when > > there was a tty layer. Let's remove it. > > > > Signed-off-by: Marc-André Lureau > > --- > > slirp/slirp_config.h | 7 --- > > slirp/if.c | 2 -- > > 2 files changed, 9 deletions(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 10/41] slirp: remove PROBE_CONN dead-code
Philippe Mathieu-Daudé, le mer. 14 nov. 2018 14:12:04 +0100, a ecrit: > On 14/11/18 13:36, Marc-André Lureau wrote: > > Nobody cares for over 14y. Somebody can revert or rewrite if > > interested by that. > > > > Signed-off-by: Marc-André Lureau > > Reviewed-by: Philippe Mathieu-Daudé Daniel P. Berrangé, le mer. 14 nov. 2018 13:33:28 +, a ecrit: > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks! Samuel
Re: [Qemu-devel] [PATCH v2] Acceptance tests: add Linux initrd checking test
On Thu, Nov 15, 2018 at 12:41:09PM -0200, Wainer dos Santos Moschetta wrote: > Hi Eduardo, > > Since you have already queued this patch for 4.0, I would like to know if I > should send a v3 to address Eric's review. Or if you can edit the patch > yourself given the changes are trivial. I can edit it. Thanks for the reminder! -- Eduardo
Re: [Qemu-devel] [PATCH for-3.2 07/41] slirp: add clock_get_ns() callback
Paolo Bonzini, le jeu. 15 nov. 2018 13:54:02 +0100, a ecrit:
> On 14/11/2018 13:36, Marc-André Lureau wrote:
> > Signed-off-by: Marc-André Lureau
> > ---
> > slirp/libslirp.h | 6 ++
> > net/slirp.c | 19 +++
> > slirp/if.c | 2 +-
> > slirp/ip6_icmp.c | 6 --
> > slirp/slirp.c| 11 ++-
> > 5 files changed, 36 insertions(+), 8 deletions(-)
> >
> > diff --git a/slirp/libslirp.h b/slirp/libslirp.h
> > index 16ced2aa0f..fcebcd1e58 100644
> > --- a/slirp/libslirp.h
> > +++ b/slirp/libslirp.h
> > @@ -5,9 +5,15 @@
> >
> > typedef struct Slirp Slirp;
> >
> > +typedef enum SlirpClockType {
> > +SLIRP_CLOCK_VIRTUAL,
> > +SLIRP_CLOCK_REALTIME,
>
> The more I look at this, the more I think that SLIRP_CLOCK_REALTIME is
> wrong and it should also be using SLIRP_CLOCK_VIRTUAL. It's used to put
> a cap on the amount of time the guest has to reply to an ARP request,
> and of course the guest cannot process it if it's paused. So this can
> be simpler, because SlirpClockType can disappear completely.
Mmm, I'd tend to agree indeed.
(it's also used for TCP timers, but the same reasoning applies)
We'd need to check that turning them to VIRTUAL does work fine.
Samuel
Re: [Qemu-devel] [PATCH for-3.2 05/41] slirp: use a callback structure to interface with qemu
Philippe Mathieu-Daudé, le mer. 14 nov. 2018 14:10:15 +0100, a ecrit:
> With the const qualifier:
> Reviewed-by: Philippe Mathieu-Daudé
Stefan Hajnoczi, le mer. 14 nov. 2018 14:30:14 +, a ecrit:
> On Wed, Nov 14, 2018 at 04:36:07PM +0400, Marc-André Lureau wrote:
> > -typedef void (*slirp_output)(void *opaque, const uint8_t *pkt, int
> > pkt_len);
> > +typedef struct SlirpCb {
> > +void (*output)(void *opaque, const uint8_t *pkt, int pkt_len);
> > +} SlirpCb;
>
> Please include doc comments for each SlirpCb callback introduced by this
> series. Otherwise it will be hard for other people to use libslirp.
I have added both and pushed to my tree, thanks!
Samuel
Re: [Qemu-devel] [RFC v1 17/23] riscv: tcg-target: Add direct load and store instructions
On Fri, Nov 16, 2018 at 9:10 AM Richard Henderson
wrote:
>
> On 11/15/18 11:36 PM, Alistair Francis wrote:
> > +tcg_out_opc_reg(s, OPC_ADD, base, TCG_GUEST_BASE_REG, addr_regl);
>
> Should avoid this when guest_base == 0, which happens fairly regularly for a
> 64-bit guest.
>
> > +/* Prefer to load from offset 0 first, but allow for overlap. */
> > +if (TCG_TARGET_REG_BITS == 64) {
> > +tcg_out_opc_imm(s, OPC_LD, lo, base, 0);
> > +} else {
> > +tcg_out_opc_imm(s, OPC_LW, lo, base, 0);
> > +tcg_out_opc_imm(s, OPC_LW, hi, base, 4);
> > +}
>
> Comment sounds like two lines of code that's missing.
I can't figure out what this comment should be for. Why would we want
to prefer loading with an offset 0?
Alistair
>
> > +const TCGMemOp bswap = opc & MO_BSWAP;
> > +
> > +/* TODO: Handle byte swapping */
>
> Should assert rather than emit bad code.
>
> I do still plan to change tcg to allow backends to *not* handle byte swapping
> if they don't want. This will make the i386 and arm32 backends less icky.
>
>
> r~
Re: [Qemu-devel] [RFC v1 20/23] riscv: tcg-target: Add the target init code
On Fri, Nov 16, 2018 at 9:26 AM Richard Henderson wrote: > > On 11/15/18 11:36 PM, Alistair Francis wrote: > > +tcg_regset_set_reg(s->reserved_regs, TCG_REG_L0); > > +tcg_regset_set_reg(s->reserved_regs, TCG_REG_L1); > > +tcg_regset_set_reg(s->reserved_regs, TCG_REG_RA); > > Why are these three reserved? Do these not need to be? I thought we had to reserve them. Alistair > > > r~
Re: [Qemu-devel] [PATCH for-3.2 04/41] slirp: remove unused M_TRAILINGSPACE
Daniel P. Berrangé, le mer. 14 nov. 2018 13:32:57 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:06PM +0400, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau > > --- > > slirp/mbuf.h | 1 - > > 1 file changed, 1 deletion(-) > > Reviewed-by: Daniel P. Berrangé Applied to my tree, thanks!
Re: [Qemu-devel] 3.1.0-rc{0,1} doesn't start
On Mon, Nov 19, 2018 at 04:55:13PM -0500, Bandan Das wrote:
> baldu...@units.it writes:
>
> > hello
> >
> > I'm building qemu from source and happily using it since a bit
> > (2.3.0)
> >
> > Since 3.1.0-rc0 (including latest 3.1.0-rc1) I'm no more able to start
> > qemu, getting:
> >
> > 8<
> > install:115> qemu
> > qemu: error: failed to set MSR 0x10a to 0x0
> > qemu:
> > /home/balducci/tmp/install-us-d/qemu-3.1.0-rc1.d/qemu-3.1.0-rc0/target/i386/kvm.c:2185:
> > kvm_put_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
> > Aborted
> > >8
> >
> I believe the check on whether MSR_IA32_ARCH_CAPABILITIES is present is
> incomplete because it can return 0 for data. Can you try this:
>
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index f524e7d929..4878ffb90b 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -2002,14 +2002,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> #endif
>
> /* If host supports feature MSR, write down. */
> -if (kvm_feature_msrs) {
> -int i;
> -for (i = 0; i < kvm_feature_msrs->nmsrs; i++)
> -if (kvm_feature_msrs->indices[i] == MSR_IA32_ARCH_CAPABILITIES) {
> -kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
> +if (kvm_arch_get_supported_msr_feature(kvm_state,
> MSR_IA32_ARCH_CAPABILITIES)) {
> +kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
>env->features[FEAT_ARCH_CAPABILITIES]);
kvm_arch_get_supported_msr_feature() will return the value of the
MSR on the host side (kvm/x86.c:kvm_get_msr_feature()). Having
it return non-zero doesn't mean KVM's
svm_set_msr(MSR_IA32_ARCH_CAPABILITIES) will work.
If the MSR doesn't work on KVM_SET_MSRS, it is not supposed to
appear on KVM_GET_MSR_INDEX_LIST (even if it appears on
KVM_GET_MSR_FEATURE_INDEX_LIST). QEMU must check
KVM_GET_MSR_INDEX_LIST too before including the MSR on the
KVM_SET_MSRS call.
> -break;
> -}
> }
>
> /*
>
>
> > I have no idea about what the reason might be, apologies.
> >
> > Actually, I have found a recent (2018-10-16) post which might be
> > related to this (it mentions the same error message from qemu):
> > https://lkml.org/lkml/2018/10/16/440; but I'm not in the position to
> > go through. AFAICS, the commit mentioned in the link is present in the
> > 4.19.2 kernel I'm using, so...?
> >
> > I can add that 3.0.0 works nicely (everything else unchanged,
> > including running kernel 4.19.2)
> >
> > OTOH, 3.1.0-rc0 dumps the same error message if I boot into 4.17.14 or
> > 4.18.16 kernels.
> >
> > I enclose my specs hoping that somebody can spot where the problem
> > might be. I will be happy to send any other detail which might be
> > useful.
> >
> > I suspect that this might be some problem on my side, as I couldn't
> > find any similar report (apart some old (qemu-2.8.50) threads, that
> > didn't help)
> >
> >
> > thanks a lot in advance for any hint/help
> >
> > ciao
> > gabriele
> >
> >
> > Here are my specs:
> >
> > #
> > # command to run qemu is:
> > qemu -m 2G /opt/windog \
> > -accel kvm,thread=multi \
> > -netdev user,id=net0,smb=/home/balducci \
> > -device rtl8139,netdev=net0
> >
> > #
> > # qemu build configuration:
> > --prefix=/opt/stow.d/versions/qemu-3.1.0-rc1/usr
> > --libdir=/opt/stow.d/versions/qemu-3.1.0-rc1/usr/lib64
> > --sysconfdir=/opt/stow.d/versions/qemu-3.1.0-rc1/etc
> > --localstatedir=/var/run
> > --docdir=/opt/stow.d/versions/qemu-3.1.0-rc1/usr/share/doc/qemu
> > --target-list=x86_64-softmmu
> > --audio-drv-list=alsa
> >
> >
> > #
> > install:154> uname -sr
> > Linux 4.19.2
> >
> > #
> > install:155> cat /proc/cpuinfo
> > processor : 0
> > vendor_id : AuthenticAMD
> > cpu family : 21
> > model : 48
> > model name : AMD Athlon(tm) X4 860K Quad Core Processor
> > stepping: 1
> > microcode : 0x6003106
> > cpu MHz : 3473.492
> > cache size : 2048 KB
> > physical id : 0
> > siblings: 4
> > core id : 0
> > cpu cores : 2
> > apicid : 16
> > initial apicid : 0
> > fpu : yes
> > fpu_exception : yes
> > cpuid level : 13
> > wp : yes
> > flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca
> > cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt
> > pdpe1gb rdtscp lm constant_tsc rep_good nopl nonstop_tsc cpuid extd_apicid
> > aperfmperf pni pclmulqdq monitor ssse3 fma cx16 sse4_1 sse4_2 popcnt aes
> > xsave avx f16c lahf_lm cmp_legacy svm extapic cr8_legacy abm sse4a
> > misalignsse 3dnowprefetch osvw ibs xop skinit wdt lwp fma4 tce nodeid_msr
> > tbm topoext perfctr_core perfctr_nb bpext ptsc cpb hw_pstate ssbd vmmcall
> > fsgsbase bmi1 xsaveopt arat npt lbrv
Re: [Qemu-devel] [PATCH for-3.2 03/41] slirp: simplify fork_exec()
Hello, Daniel P. Berrangé, le mer. 14 nov. 2018 14:22:34 +, a ecrit: > On Wed, Nov 14, 2018 at 04:36:05PM +0400, Marc-André Lureau wrote: > > Use g_spawn_async_with_fds() to setup the child. > > > > GSpawn handles reaping the child, and closing parent file descriptors. > > The g_spawn* family of APIs is portable to Win32, which the current > fork/exec code in slirp is not. > > So I'm curious if this conversion is sufficient to make this part > of slirp work on Win32, or if further portability problems remain. > If it does work on Win32, then you can also delete the stub > fork_exec() impl the code currently has. Mmm, I don't think any portability issue remains. SO_OOBINLINE is used in other places, the rest is portable. There is the setsid() call which may just not make sense on Windows, but we could just disable it there. I have pushed to https://people.debian.org/~sthibault/qemu.git slirp-2-win what it could look like. I don't have a windows box off hand, could somebody try it to confirm that it builds? Samuel
Re: [Qemu-devel] [PATCH for-3.2 03/41] slirp: simplify fork_exec()
Marc-André Lureau, le mer. 14 nov. 2018 16:36:05 +0400, a ecrit: > Use g_spawn_async_with_fds() to setup the child. > > GSpawn handles reaping the child, and closing parent file descriptors. > > Signed-off-by: Marc-André Lureau Applied to my slirp-2 tree, thanks! Samuel
Re: [Qemu-devel] [PATCH for-3.2 02/41] glib-compat: add g_spawn_async_with_fds() fallback
Marc-André Lureau, le mer. 14 nov. 2018 16:36:04 +0400, a ecrit:
> Signed-off-by: Marc-André Lureau
Reviewed-by: Samuel Thibault
include/glib-compat.h maintainers, may I keep this in my slirp tree, to
be pushed to master when appropriate?
Samuel
> ---
> include/glib-compat.h | 56 +++
> 1 file changed, 56 insertions(+)
>
> diff --git a/include/glib-compat.h b/include/glib-compat.h
> index fdf95a255d..8a078c5288 100644
> --- a/include/glib-compat.h
> +++ b/include/glib-compat.h
> @@ -83,6 +83,62 @@ static inline gboolean g_strv_contains_qemu(const gchar
> *const *strv,
> }
> #define g_strv_contains(a, b) g_strv_contains_qemu(a, b)
>
> +#if !GLIB_CHECK_VERSION(2, 58, 0)
> +typedef struct QemuGSpawnFds {
> +GSpawnChildSetupFunc child_setup;
> +gpointer user_data;
> +gint stdin_fd;
> +gint stdout_fd;
> +gint stderr_fd;
> +} QemuGSpawnFds;
> +
> +static inline void
> +qemu_gspawn_fds_setup(gpointer user_data)
> +{
> +QemuGSpawnFds *q = (QemuGSpawnFds *)user_data;
> +
> +dup2(q->stdin_fd, 0);
> +dup2(q->stdout_fd, 1);
> +dup2(q->stderr_fd, 2);
> +q->child_setup(q->user_data);
> +}
> +#endif
> +
> +static inline gboolean
> +g_spawn_async_with_fds_qemu(const gchar *working_directory,
> +gchar **argv,
> +gchar **envp,
> +GSpawnFlags flags,
> +GSpawnChildSetupFunc child_setup,
> +gpointer user_data,
> +GPid *child_pid,
> +gint stdin_fd,
> +gint stdout_fd,
> +gint stderr_fd,
> +GError **error)
> +{
> +#if GLIB_CHECK_VERSION(2, 58, 0)
> +return g_spawn_async_with_fds(working_directory, argv, envp, flags,
> + child_setup, user_data,
> + child_pid, stdin_fd, stdout_fd, stderr_fd,
> + error);
> +#else
> +QemuGSpawnFds setup = {
> +.child_setup = child_setup,
> +.user_data = user_data,
> +.stdin_fd = stdin_fd,
> +.stdout_fd = stdout_fd,
> +.stderr_fd = stderr_fd,
> +};
> +
> +return g_spawn_async(working_directory, argv, envp, flags,
> + qemu_gspawn_fds_setup, ,
> + child_pid, error);
> +#endif
> +}
> +
> +#define g_spawn_async_with_fds(wd, argv, env, f, c, d, p, ifd, ofd, efd,
> err) \
> +g_spawn_async_with_fds_qemu(wd, argv, env, f, c, d, p, ifd, ofd, efd,
> err)
>
> #if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
> /*
> --
> 2.19.1.708.g4ede3d42df
>
--
Samuel
bien sûr que ça convient mieux à tout le monde
enfin, dans la mesure où tout le monde c'est comme moi
-+- le consensus, c'est facile -+-
Re: [Qemu-devel] [PATCH for-3.2 01/41] slirp: move socket pair creation in helper function
Hello, Marc-André Lureau, le mer. 14 nov. 2018 16:36:03 +0400, a ecrit: > Originally, the patch was fixing a bunch of issues, but Peter beat me > to it with earlier commit "slirp: fork_exec(): create and connect > child socket before fork()". > > Factor out socket pair creation, to simplify the fork_exec() code. > Use the name socketpair_with_oob() since the code is actually similar > to what socketpair() would do, except that it uses TCP sockets, for > SLIRP to be able to call send with MSG_OOB (since SO_OOBINLINE is set, > this could probably be faked instead on regular unix sockets though). > > Signed-off-by: Marc-André Lureau Applied to my tree, thanks! Samuel
Re: [Qemu-devel] 3.1.0-rc{0,1} doesn't start
baldu...@units.it writes:
> hello
>
> I'm building qemu from source and happily using it since a bit
> (2.3.0)
>
> Since 3.1.0-rc0 (including latest 3.1.0-rc1) I'm no more able to start
> qemu, getting:
>
> 8<
> install:115> qemu
> qemu: error: failed to set MSR 0x10a to 0x0
> qemu:
> /home/balducci/tmp/install-us-d/qemu-3.1.0-rc1.d/qemu-3.1.0-rc0/target/i386/kvm.c:2185:
> kvm_put_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
> Aborted
> >8
>
I believe the check on whether MSR_IA32_ARCH_CAPABILITIES is present is
incomplete because it can return 0 for data. Can you try this:
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index f524e7d929..4878ffb90b 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -2002,14 +2002,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
#endif
/* If host supports feature MSR, write down. */
-if (kvm_feature_msrs) {
-int i;
-for (i = 0; i < kvm_feature_msrs->nmsrs; i++)
-if (kvm_feature_msrs->indices[i] == MSR_IA32_ARCH_CAPABILITIES) {
-kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
+if (kvm_arch_get_supported_msr_feature(kvm_state,
MSR_IA32_ARCH_CAPABILITIES)) {
+kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
env->features[FEAT_ARCH_CAPABILITIES]);
-break;
-}
}
/*
> I have no idea about what the reason might be, apologies.
>
> Actually, I have found a recent (2018-10-16) post which might be
> related to this (it mentions the same error message from qemu):
> https://lkml.org/lkml/2018/10/16/440; but I'm not in the position to
> go through. AFAICS, the commit mentioned in the link is present in the
> 4.19.2 kernel I'm using, so...?
>
> I can add that 3.0.0 works nicely (everything else unchanged,
> including running kernel 4.19.2)
>
> OTOH, 3.1.0-rc0 dumps the same error message if I boot into 4.17.14 or
> 4.18.16 kernels.
>
> I enclose my specs hoping that somebody can spot where the problem
> might be. I will be happy to send any other detail which might be
> useful.
>
> I suspect that this might be some problem on my side, as I couldn't
> find any similar report (apart some old (qemu-2.8.50) threads, that
> didn't help)
>
>
> thanks a lot in advance for any hint/help
>
> ciao
> gabriele
>
>
> Here are my specs:
>
> #
> # command to run qemu is:
> qemu -m 2G /opt/windog \
> -accel kvm,thread=multi \
> -netdev user,id=net0,smb=/home/balducci \
> -device rtl8139,netdev=net0
>
> #
> # qemu build configuration:
> --prefix=/opt/stow.d/versions/qemu-3.1.0-rc1/usr
> --libdir=/opt/stow.d/versions/qemu-3.1.0-rc1/usr/lib64
> --sysconfdir=/opt/stow.d/versions/qemu-3.1.0-rc1/etc
> --localstatedir=/var/run
> --docdir=/opt/stow.d/versions/qemu-3.1.0-rc1/usr/share/doc/qemu
> --target-list=x86_64-softmmu
> --audio-drv-list=alsa
>
>
> #
> install:154> uname -sr
> Linux 4.19.2
>
> #
> install:155> cat /proc/cpuinfo
> processor : 0
> vendor_id : AuthenticAMD
> cpu family : 21
> model : 48
> model name : AMD Athlon(tm) X4 860K Quad Core Processor
> stepping: 1
> microcode : 0x6003106
> cpu MHz : 3473.492
> cache size : 2048 KB
> physical id : 0
> siblings: 4
> core id : 0
> cpu cores : 2
> apicid : 16
> initial apicid : 0
> fpu : yes
> fpu_exception : yes
> cpuid level : 13
> wp : yes
> flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca
> cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt
> pdpe1gb rdtscp lm constant_tsc rep_good nopl nonstop_tsc cpuid extd_apicid
> aperfmperf pni pclmulqdq monitor ssse3 fma cx16 sse4_1 sse4_2 popcnt aes
> xsave avx f16c lahf_lm cmp_legacy svm extapic cr8_legacy abm sse4a
> misalignsse 3dnowprefetch osvw ibs xop skinit wdt lwp fma4 tce nodeid_msr tbm
> topoext perfctr_core perfctr_nb bpext ptsc cpb hw_pstate ssbd vmmcall
> fsgsbase bmi1 xsaveopt arat npt lbrv svm_lock nrip_save tsc_scale vmcb_clean
> flushbyasid decodeassists pausefilter pfthreshold overflow_recov
> bugs: fxsave_leak sysret_ss_attrs null_seg spectre_v1 spectre_v2
> spec_store_bypass
> bogomips: 7380.73
> TLB size: 1536 4K pages
> clflush size: 64
> cache_alignment : 64
> address sizes : 48 bits physical, 48 bits virtual
> power management: ts ttp tm 100mhzsteps hwpstate cpb eff_freq_ro [13]
>
> [...cpus 1 2 3 omitted...]
>
>
> #
> install:156> egrep KVM .config-4.19.2
> CONFIG_HAVE_KVM=y
> CONFIG_HAVE_KVM_IRQCHIP=y
> CONFIG_HAVE_KVM_IRQFD=y
> CONFIG_HAVE_KVM_IRQ_ROUTING=y
> CONFIG_HAVE_KVM_EVENTFD=y
> CONFIG_KVM_MMIO=y
> CONFIG_KVM_ASYNC_PF=y
> CONFIG_HAVE_KVM_MSI=y
>
Re: [Qemu-devel] [PATCH for-4.0 v2] virtio: Provide version-specific variants of virtio PCI devices
On Mon, Nov 19, 2018 at 01:07:59PM -0500, Michael S. Tsirkin wrote: n > On Mon, Nov 19, 2018 at 11:41:05AM +0100, Cornelia Huck wrote: > > On Fri, 16 Nov 2018 01:45:51 -0200 > > Eduardo Habkost wrote: > > > > > On Thu, Nov 15, 2018 at 05:29:24PM +0100, Andrea Bolognani wrote: > > > > > > One thing that I'm very much not convinced about is the naming, > > > > specifically leaving the virtio revision out: I get it that we > > > > Should Never Need™ another major version of the spec, but I'm > > > > afraid discounting the possibility outright might prove to be > > > > shortsighted and come back to bite us later, so I'd much rather > > > > keep it. > > That's not the claim. In fact the reverse is true - a major revision can > come at any point. The claim is that virtio compatibility is not based > on version numbers. And another claim is that you can trust the > virtio TC not to overload terminology that it put in the spec. So use > that and you should be fine. Come up with your own and end up writing > another spec just to document it. > > > > > > > > > And once that's done, "non-transitional" (while matching the > > > > language of the spec) starts to look a bit unnecessary when you > > > > could simply have > > > > > > > > virtio-*-pci > > > > virtio-*-pci-1 > > > > virtio-*-pci-1-transitional > > > > > > > > instead. But I don't feel as strongly about this as I do about > > > > keeping the virtio revision in the device name :) > > > > > > I like that suggestion. Makes the device names more explicit > > > _and_ shorter. I'll do that in v3. > > > > OTOH, that would mean we'd need to introduce new device types if we > > ever start to support a virtio 2.x standard. My understanding was that > > we'll want separate device types for transitional and non-transitional > > for two reasons: the bus which a device can be plugged into, and > > changing ids. Do we really expect huge changes in a possible 2.x > > standard that affect virtio-pci only, and not other virtio transports > > as well? > > Yes I think adding a version there is a mistake. > transitional/legacy/non-transitional are spec terms so > they are unlikely to change abruptly. OTOH virtio TC can > just decide next version is 2.0 on a drop of a hat. > > And I strongly believe command line users really really do not want all > this mess. Even adding "pci" is the name confuses people (what are the > other options?). For command line model=virtio is pretty much perfect. > So all these names are there primarily for libvirt's benefit. > And the only input from libvirt devs so far > has been that it's unclear how does cross version > migration work. That needs to be addressed in some way. What still needs to be addressed? Just keep the existing device types on migration. We could make additional promises about compatibility with the disable-modern and disable-legacy properties, but I don't see why we need to do it unless we start deprecating the old device types. > > So can we maybe start with a libvirt domain xml patch, with an > implementation for existing QEMU, get that acked, and then just map it > back to qemu command line as directly as possible? I don't know what you mean here by "libvirt domain XML patch". Do you mean solving the problems only on the libvirt side, keeping the existing device types? Why would we do that? It would be a hack making the situation even messier. If libvirt needs us to provide better interfaces, let's cooperate with them. I'd like us to avoid having yet another "the problem must be solved in the other layer first" deadlock here. -- Eduardo
[Qemu-devel] [RFC PATCH 1/1] target/ppc: support single stepping with KVM HV
The hardware singlestep mechanism in POWER works via a Trace Interrupt
(0xd00) that happens after any instruction executes, whenever MSR_SE =
1 (PowerISA Section 6.5.15 - Trace Interrupt).
However, with kvm_hv, the Trace Interrupt happens inside the guest and
KVM has no visibility of it. Therefore, when the gdbstub uses the
KVM_SET_GUEST_DEBUG ioctl to enable singlestep, KVM simply ignores it.
This patch takes advantage of the Trace Interrupt to perform the step
inside the guest, but uses a breakpoint at the Trace Interrupt handler
to return control to KVM. The exit is treated by KVM as a regular
breakpoint and it returns to the host (and QEMU eventually).
Before signalling GDB, QEMU sets the Next Instruction Pointer to the
instruction following the one being stepped, effectively skipping the
interrupt handler execution and hiding the trace interrupt breakpoint
from GDB.
This approach works with both of GDB's 'scheduler-locking' options
(off, step).
Signed-off-by: Fabiano Rosas
---
accel/kvm/kvm-all.c | 10 +++
exec.c | 1 +
include/sysemu/kvm.h | 4 +++
target/arm/kvm.c | 4 +++
target/i386/kvm.c| 4 +++
target/ppc/kvm.c | 65 +++-
target/s390x/kvm.c | 4 +++
7 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 4880a05399..4fb7199a15 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2313,6 +2313,11 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long
reinject_trap)
return data.err;
}
+void kvm_set_singlestep(CPUState *cs, int enabled)
+{
+kvm_arch_set_singlestep(cs, enabled);
+}
+
int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr,
target_ulong len, int type)
{
@@ -2439,6 +2444,11 @@ int kvm_remove_breakpoint(CPUState *cpu, target_ulong
addr,
void kvm_remove_all_breakpoints(CPUState *cpu)
{
}
+
+void kvm_set_singlestep(CPUState *cs, int enabled)
+{
+}
+
#endif /* !KVM_CAP_SET_GUEST_DEBUG */
static int kvm_set_signal_mask(CPUState *cpu, const sigset_t *sigset)
diff --git a/exec.c b/exec.c
index bb6170dbff..55614822c3 100644
--- a/exec.c
+++ b/exec.c
@@ -1233,6 +1233,7 @@ void cpu_single_step(CPUState *cpu, int enabled)
if (cpu->singlestep_enabled != enabled) {
cpu->singlestep_enabled = enabled;
if (kvm_enabled()) {
+kvm_set_singlestep(cpu, enabled);
kvm_update_guest_debug(cpu, 0);
} else {
/* must flush all the translated code to avoid inconsistencies */
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 97d8d9d0d5..a01a8d58dd 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -259,6 +259,8 @@ int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr,
void kvm_remove_all_breakpoints(CPUState *cpu);
int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap);
+void kvm_set_singlestep(CPUState *cpu, int enabled);
+
int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
int kvm_on_sigbus(int code, void *addr);
@@ -431,6 +433,8 @@ void kvm_arch_remove_all_hw_breakpoints(void);
void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg);
+void kvm_arch_set_singlestep(CPUState *cpu, int enabled);
+
bool kvm_arch_stop_on_emulation_error(CPUState *cpu);
int kvm_check_extension(KVMState *s, unsigned int extension);
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 44dd0ce6ce..dd8e43ab7e 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -670,6 +670,10 @@ int kvm_arch_process_async_events(CPUState *cs)
return 0;
}
+void kvm_arch_set_singlestep(CPUState *cs, int enabled)
+{
+}
+
/* The #ifdef protections are until 32bit headers are imported and can
* be removed once both 32 and 64 bit reach feature parity.
*/
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index f524e7d929..ba56f2ee1f 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -3521,6 +3521,10 @@ static int kvm_handle_debug(X86CPU *cpu,
return ret;
}
+void kvm_arch_set_singlestep(CPUState *cs, int enabled)
+{
+}
+
void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
{
const uint8_t type_code[] = {
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index f81327d6cd..3af5e91997 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -94,6 +94,7 @@ static int cap_ppc_safe_indirect_branch;
static int cap_ppc_nested_kvm_hv;
static uint32_t debug_inst_opcode;
+static target_ulong trace_handler_addr;
/* XXX We have a race condition where we actually have a level triggered
* interrupt, but the infrastructure can't expose that yet, so the guest
@@ -509,6 +510,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, _inst_opcode);
kvmppc_hw_debug_points_init(cenv);
+trace_handler_addr = (cenv->excp_vectors[POWERPC_EXCP_TRACE] |
+ 0xc0004000ull);
+
[Qemu-devel] [RFC PATCH 0/1] single step for KVM HV
Single stepping via GDB/gdbstub is currently not working with KVM HV. When asking for a single step (stepi), KVM simply ignores the request and execution continues. This has the direct effect of breaking GDB's 'step', 'stepi', 'next', 'nexti' commands. The 'continue' command is also affected since continuing right after a breakpoint requires that GDB first perform a single step so that the breakpoint can be re-inserted before continuing - in this case the breakpoint is not re-inserted and it won't hit again. The issue here is that single stepping in POWER makes use of an interrupt (Trace Interrupt [1]) that does not reach the hypervisor, so while the single step would happen if properly triggered, it would not cause an exit to KVM so there would be no way of handing control back to GDB. Aside from that, the guest kernel is not prepared to deal with such an interrupt in kernel mode (when not using KGDB, or some other debugging facility) and it causes an Oops. This series implements a "software single step" approach that makes use of: i) the Trace Interrupt to perform the step inside the guest and ii) a breakpoint at the Trace Interrupt handler address to cause a vm exit (Emulation Assist) so that we can return control to QEMU. With (i), we basically get the single step for free, without having to discover what are the possible targets of instructions that divert execution. With (ii), we hide the single step from the guest and keep all of the step logic in QEMU. This was so far tested with single and multiple vcpus and with GDB scheduler locking on and off [2]. I have not fully explored yet the potential issues when using debuggers simultaneously inside and outside the guest, however I was able to single step the ptrace code while single stepping a userspace program inside the guest with GDB. I'm looking for feedback on the general approach before I develop this further. 1- PowerISA Section 6.5.15 - Trace Interrupt 2- https://sourceware.org/gdb/onlinedocs/gdb/All_002dStop-Mode.html Fabiano Rosas (1): target/ppc: support single stepping with KVM HV accel/kvm/kvm-all.c | 10 +++ exec.c | 1 + include/sysemu/kvm.h | 4 +++ target/arm/kvm.c | 4 +++ target/i386/kvm.c| 4 +++ target/ppc/kvm.c | 65 +++- target/s390x/kvm.c | 4 +++ 7 files changed, 91 insertions(+), 1 deletion(-) -- 2.17.1
Re: [Qemu-devel] [PATCH for-4.0 v2] virtio: Provide version-specific variants of virtio PCI devices
On Mon, Nov 19, 2018 at 07:56:38PM +0100, Cornelia Huck wrote: > On Mon, 19 Nov 2018 13:42:58 -0500 > "Michael S. Tsirkin" wrote: > > > On Mon, Nov 19, 2018 at 07:32:38PM +0100, Cornelia Huck wrote: > > > On Mon, 19 Nov 2018 13:07:59 -0500 > > > "Michael S. Tsirkin" wrote: > > > > > And I strongly believe command line users really really do not want all > > > > this mess. Even adding "pci" is the name confuses people (what are the > > > > other options?). For command line model=virtio is pretty much perfect. > > > > > > I'd argue that it's problematic on platforms where you have different > > > options for virtio transports. What does "model=virtio" mean? Always > > > virtio-pci, if available? Or rather virtio-, with > > > being the best option for the machine? > > > > Most people don't care, for them it's "whatever works". > > > > We have this assumption that if we force a choice then people will > > choose the right thing but in practice they will do what we all do, play > > with it until it kind of works and leave well alone afterwards. > > That's at best - at worst give up and use an easier tool. > > That implies that we (the developers) need to care and make sure that > "model=virtio" gets them the best possible transport (i.e. on s390x, > that would be ccw unless the user explicitly requests pci; I'm not sure > what the situation with mmio is -- probably "use pci whenever > possible"?) I think that's what libvirt already gives us today (I hope.) > > What makes it messy on the pci side is that the "best option" actually > depends on what kind of guest the user wants to run (if the guest is > too old, you're stuck with transitional; if you want to reap the > benefits of PCIe, you need non-transitional...) Yeah, sometimes we can't make everything work out of the box on the default case. But I think in this case we can make the QEMU command-line a bit more usable if we just cover more recent OSes. However, I wish this kind of usability magic didn't automatically imposed us the burden of keeping guest ABI compatibility too. Keeping ABI compatibility on the machine-friendly device types and interfaces is already hard enough. We already have aliases that automatically select a virtio device type at qdev-monitor.c:qdev_alias_table[], and I don't know if they are supposed to keep a stable guest ABI. -- Eduardo
Re: [Qemu-devel] [PATCH v1 6/9] qapi: rewrite string-input-visitor
On 19.11.18 20:51, Markus Armbruster wrote: > Copying Igor and Eduardo for a hostmem.c bug. Search for "core dumped". > > David Hildenbrand writes: > Tests have to be fixed up: - Two BUGs were hardcoded that are fixed now - The string-input-visitor now actually returns a parsed list and not an ordered set. >>> >>> I'd expect this to necessitate an update of callers that expect a set, >>> but... >>> Signed-off-by: David Hildenbrand --- include/qapi/string-input-visitor.h | 4 +- qapi/string-input-visitor.c | 410 tests/test-string-input-visitor.c | 18 +- 3 files changed, 239 insertions(+), 193 deletions(-) >>> >>> ... there's none. >>> >>> Let me know if you need help finding them. I think we tracked them down >>> during the discussion that led to this series. >>> >> >> Indeed, I missed to document that. So here is the outcome: >> >> 1. backends/hostmem.c:host_memory_backend_set_host_nodes() >> >> -> calls visit_type_uint16List(via bitmap) >> -> the code can deal with duplicates/unsorted lists (bitmap_set) > > Yes. > >> Side node: I am not sure if there should be some range checks, but maybe >> the bitmap is large enough hm ... > > Fishy. MAX_NODES is 128. Tinker, tinker, ... > > $ upstream-qemu -nodefaults -object > memory-backend-file,id=mem0,mem-path=x,size=4096,host-nodes=12345 > Segmentation fault (core dumped) > > Igor, Eduardo, this is yours. > > There's another use of visit_type_uint16List() is this file, but it's in > property getter host_memory_backend_get_host_nodes(), and property > getters aren't used with the string input visitor. > >> 2. qapi-visit.c::visit_type_Memdev_members() >> >> -> calls visit_type_uint16List() >> -> I think this never used for input, only for output / freeing > > Yes, it's used by query-memdev with the QObject output visitor to build > the value of @host-nodes. > >> 3. qapi-visit.c::visit_type_NumaNodeOptions_members() >> >> -> calls visit_type_uint16List() >> -> I think this never used for input, only for output / freeing > > It's used for input, but with the opts visitor, see parse_numa(). > >> 4. qapi-visit.c::visit_type_RockerOfDpaGroup_members >> >> -> calls visit_type_uint32List() >> -> I think this never used for input, only for output / freeing > > Yes, it's used by query-rocker-of-dpa-groups with the QObject output > visitor to build the value of @group-ids. > >> 5. qapi-visit.c::visit_type_RxFilterInfo_members() >> >> -> calls visit_type_intList() >> -> I think this never used for input, only for output / freeing > > Yes, it's used by query-rx-filter with the QObject output visitor to > build the value of @vlan-table. > >> 6. numa.c::query_memdev() >> >> -> calls object_property_get_uint16List() >> --> String parsed via visit_type_uint16List() into list > > QOM, hard to understand. > > The value of struct HostMemoryBackend member @host-nodes (a bitmap) is > first converted to a list (sorted, no duplicates) with > host_memory_backend_get_host_nodes() via object_property_get(), then > converted to a string with the string output visitor. The resulting > string is then converted back to a list with the string input visitor. > > Despite the shenanigans going on in the string output visitor, I'd > expect the resulting list to also be sorted and without duplicates. > >> -> qmp_query_memdev() uses this list >> --> Not relevant if unique or sorted > > Depends on the contract of QMP command query-memdev. Here's the > relevant part. > > # @host-nodes: host nodes for its memory policy > > Useless. > > "Sorted, no duplicates" might have become de facto ABI. Not sure. > However, I believe your patch won't affect it, as per the argument I > just made. > >> -> hmp_info_memdev() uses this list >> --> List converted again to a string using string output visitor >> >> -> I don't think unique/sorted is relevant here. > > HMP is not a stable interface. > >> Am I missing anything / is any of my statements wrong? > > Searching the QAPI schema for lists of integers coughs up block latency > histogram stuff, but that's unrelated, as far as I can tell. > > Looks like we're good. I didn't expect that :) > Haha, me too. Will add a short description to the patch message and maybe resend tomorrow! -- Thanks, David / dhildenb
Re: [Qemu-devel] 3.1.0-rc{0,1} doesn't start
On 11/19/2018 01:35 PM, Dr. David Alan Gilbert wrote: * baldu...@units.it (baldu...@units.it) wrote: hi thanks for taking the time to reply Dr. David Alan Gilbert writes: I suspect that this might be some problem on my side, as I couldn't find any similar report (apart some old (qemu-2.8.50) threads, that didn't help) Not necessarily; can you tell me: a) At what point does it fail - immediately when booting the guest? Some time during the boot? Later? b) What guest does it happen on? a) the error happens almost immediately; I mean: when I run qemu from an xterm, it doesn't even popup its window: it just dumps the error message to the terminal and stops b) the guest is an old windows XP OS; but, as I say above, all goes as if qemu doesn't even load the OS image (at least this is my impression) A colleague has confirmed this on his FX-8320 on Fedora 29 with the virt-next repo; so it's nothing that's special about your machine; it's 3.1 that really doesn't like the old AMDs. Also we received a similar report in Fedora bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1651021 Thanks, Cole
Re: [Qemu-devel] [PATCH v1 6/9] qapi: rewrite string-input-visitor
Copying Igor and Eduardo for a hostmem.c bug. Search for "core dumped". David Hildenbrand writes: >>> >>> Tests have to be fixed up: >>> - Two BUGs were hardcoded that are fixed now >>> - The string-input-visitor now actually returns a parsed list and not >>> an ordered set. >> >> I'd expect this to necessitate an update of callers that expect a set, but... >> >>> Signed-off-by: David Hildenbrand >>> --- >>> include/qapi/string-input-visitor.h | 4 +- >>> qapi/string-input-visitor.c | 410 >>> tests/test-string-input-visitor.c | 18 +- >>> 3 files changed, 239 insertions(+), 193 deletions(-) >> >> ... there's none. >> >> Let me know if you need help finding them. I think we tracked them down >> during the discussion that led to this series. >> > > Indeed, I missed to document that. So here is the outcome: > > 1. backends/hostmem.c:host_memory_backend_set_host_nodes() > > -> calls visit_type_uint16List(via bitmap) > -> the code can deal with duplicates/unsorted lists (bitmap_set) Yes. > Side node: I am not sure if there should be some range checks, but maybe > the bitmap is large enough hm ... Fishy. MAX_NODES is 128. Tinker, tinker, ... $ upstream-qemu -nodefaults -object memory-backend-file,id=mem0,mem-path=x,size=4096,host-nodes=12345 Segmentation fault (core dumped) Igor, Eduardo, this is yours. There's another use of visit_type_uint16List() is this file, but it's in property getter host_memory_backend_get_host_nodes(), and property getters aren't used with the string input visitor. > 2. qapi-visit.c::visit_type_Memdev_members() > > -> calls visit_type_uint16List() > -> I think this never used for input, only for output / freeing Yes, it's used by query-memdev with the QObject output visitor to build the value of @host-nodes. > 3. qapi-visit.c::visit_type_NumaNodeOptions_members() > > -> calls visit_type_uint16List() > -> I think this never used for input, only for output / freeing It's used for input, but with the opts visitor, see parse_numa(). > 4. qapi-visit.c::visit_type_RockerOfDpaGroup_members > > -> calls visit_type_uint32List() > -> I think this never used for input, only for output / freeing Yes, it's used by query-rocker-of-dpa-groups with the QObject output visitor to build the value of @group-ids. > 5. qapi-visit.c::visit_type_RxFilterInfo_members() > > -> calls visit_type_intList() > -> I think this never used for input, only for output / freeing Yes, it's used by query-rx-filter with the QObject output visitor to build the value of @vlan-table. > 6. numa.c::query_memdev() > > -> calls object_property_get_uint16List() > --> String parsed via visit_type_uint16List() into list QOM, hard to understand. The value of struct HostMemoryBackend member @host-nodes (a bitmap) is first converted to a list (sorted, no duplicates) with host_memory_backend_get_host_nodes() via object_property_get(), then converted to a string with the string output visitor. The resulting string is then converted back to a list with the string input visitor. Despite the shenanigans going on in the string output visitor, I'd expect the resulting list to also be sorted and without duplicates. > -> qmp_query_memdev() uses this list > --> Not relevant if unique or sorted Depends on the contract of QMP command query-memdev. Here's the relevant part. # @host-nodes: host nodes for its memory policy Useless. "Sorted, no duplicates" might have become de facto ABI. Not sure. However, I believe your patch won't affect it, as per the argument I just made. > -> hmp_info_memdev() uses this list > --> List converted again to a string using string output visitor > > -> I don't think unique/sorted is relevant here. HMP is not a stable interface. > Am I missing anything / is any of my statements wrong? Searching the QAPI schema for lists of integers coughs up block latency histogram stuff, but that's unrelated, as far as I can tell. Looks like we're good. I didn't expect that :) [...]
Re: [Qemu-devel] [PATCH for-4.0 v2] virtio: Provide version-specific variants of virtio PCI devices
On Mon, Nov 19, 2018 at 07:56:38PM +0100, Cornelia Huck wrote: > On Mon, 19 Nov 2018 13:42:58 -0500 > "Michael S. Tsirkin" wrote: > > > On Mon, Nov 19, 2018 at 07:32:38PM +0100, Cornelia Huck wrote: > > > On Mon, 19 Nov 2018 13:07:59 -0500 > > > "Michael S. Tsirkin" wrote: > > > > > And I strongly believe command line users really really do not want all > > > > this mess. Even adding "pci" is the name confuses people (what are the > > > > other options?). For command line model=virtio is pretty much perfect. > > > > > > I'd argue that it's problematic on platforms where you have different > > > options for virtio transports. What does "model=virtio" mean? Always > > > virtio-pci, if available? Or rather virtio-, with > > > being the best option for the machine? > > > > Most people don't care, for them it's "whatever works". > > > > We have this assumption that if we force a choice then people will > > choose the right thing but in practice they will do what we all do, play > > with it until it kind of works and leave well alone afterwards. > > That's at best - at worst give up and use an easier tool. > > That implies that we (the developers) need to care and make sure that > "model=virtio" gets them the best possible transport (i.e. on s390x, > that would be ccw unless the user explicitly requests pci; I'm not sure > what the situation with mmio is -- probably "use pci whenever > possible"?) I think that's what libvirt already gives us today (I hope.) > > What makes it messy on the pci side is that the "best option" actually > depends on what kind of guest the user wants to run (if the guest is > too old, you're stuck with transitional; if you want to reap the > benefits of PCIe, you need non-transitional...) Well it works now - connect it to a bus and it figures out whether it should do transitional or not. You can force transitional in PCIe anyway but then you are limited to about 15 devices - probably sufficient for most people ... -- MST
Re: [Qemu-devel] [PATCH] Acceptance test: add coverage tests for -smp option
On Mon, Nov 19, 2018 at 04:48:00PM -0200, Wainer dos Santos Moschetta wrote:
> On 11/12/2018 02:31 PM, Eduardo Habkost wrote:
> > On Fri, Nov 09, 2018 at 02:58:00PM -0500, Wainer dos Santos Moschetta wrote:
> > > This adds tests for SMP option, by passing -smp with
> > > various combinations of cpus, cores, threads, and sockets
> > > values it checks that invalid topologies are not accepted
> > > as well as missing values are correctly calculated.
> > >
> > > Signed-off-by: Wainer dos Santos Moschetta
> > The test code looks nice to me, but: how exactly do you expect
> > this to be executed?
>
> 'make check-acceptance' executes it with default parameters (i.e. cores=2,
> threads=2, sockets=2, and cpus=8).
>
> It is possible to overwrite the default parameters by using Avocado's -p
> option. For example, 'avocado run -p sockets=1 -p cores=2 -p threads=1
> tests/acceptance/smp_option_coverage.py'.
>
> > Do we have a test runner or multiplexer configuration already
> > able to generate the cores/threads/sockets/cpus parameters?
>
> I did not have any variants file until you asked. Then I realized the
> problems (see inline below) that I will need to address on a v2 patch.
>
> Anyway, the variants YAML file may look like this (adapted from an example
> created by Cleber Rosa):
>
> # cat smp_variants.yaml
> !mux
> min:
> sockets: 1
> cores: 2
> threads: 1
>
> medium:
> sockets: 2
> cores: 2
> threads: 2
>
> max:
> sockets: 2
> cores: 8
> threads: 16
>
> The smp_variants.yaml defines 3 variants (min, medium, and max), each with a
> different SMP topology. You can run the tests as:
>
> # avocado run tests/acceptance/smp_option_coverage.py -m smp_variants.yaml
> JOB ID : 08d9736942e550226de9c3425a9b65c378b6654a
> JOB LOG : /root/avocado/job-results/job-2018-11-19T13.19-08d9736/job.log
> (01/60)
> tests/acceptance/smp_option_coverage.py:SmpOption.test_cpus_eq_maxcpus;min-7e5d:
> PASS (0.04 s)
> < output omitted >
> (60/60)
> tests/acceptance/smp_option_coverage.py:SmpOption.test_cpus_gt_cores_threads_sockets;max-a8e5:
> PASS (0.02 s)
> RESULTS : PASS 60 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 |
> CANCEL 0
> JOB TIME : 6.25 s
>
> If you wish I can distribute that file with this patch too. I can use
> Avocado's test data file mechanism to run the variants as described in:
> https://avocado-framework.readthedocs.io/en/65.0/WritingTests.html#accessing-test-data-files
If a variants file is necessary to make this test case useful, I
would like it to be distributed with the test code, and it should
be used somehow when running "make check-acceptance".
>
> As an alternative I can document all that on docs/devel/testing.rst.
> Whatever you prefer.
>
> Thanks for the review!
>
> - Wainer
>
> >
> > > ---
> > > tests/acceptance/smp_option_coverage.py | 218
> > > 1 file changed, 218 insertions(+)
> > > create mode 100644 tests/acceptance/smp_option_coverage.py
> > >
> > > diff --git a/tests/acceptance/smp_option_coverage.py
> > > b/tests/acceptance/smp_option_coverage.py
> > > new file mode 100644
> > > index 00..ab68d1a67d
> > > --- /dev/null
> > > +++ b/tests/acceptance/smp_option_coverage.py
> > > @@ -0,0 +1,218 @@
> > > +# QEMU -smp option coverage test.
> > > +#
> > > +# Copyright (c) 2018 Red Hat, Inc.
> > > +#
> > > +# Author:
> > > +# Wainer dos Santos Moschetta
> > > +#
> > > +# This work is licensed under the terms of the GNU GPL, version 2 or
> > > +# later. See the COPYING file in the top-level directory.
> > > +
> > > +from functools import reduce
> > > +from avocado.utils.process import run
> > > +
> > > +from avocado_qemu import Test
> > > +
> > > +
> > > +class SmpOption(Test):
> > > +"""
> > > +Launches QEMU with various cpus, cores, threads, sockets, and maxcpus
> > > +combination through -smp option, to check it does not accept invalid
> > > SMP
> > > +topologies as well as it is able to calculate correctly any missing
> > > values.
> > > +
> > > +:avocado: enable
> > > +:avocado: tags=slow,coverage
> > > +"""
> > > +def setUp(self):
> > > +super().setUp()
> > > +self.cores = self.params.get('cores', default=2)
> > > +self.threads = self.params.get('threads', default=2)
> > > +self.sockets = self.params.get('sockets', default=2)
> > > +self.cpus = self.params.get('cpus', default=8)
>
> The self.cpus variable should not be a parameter but rather calculated
> (cores * threads * sockets).
Are you sure? Don't you want to test QEMU behavior when `cpus`
is not `cores*threads*sockets`?
> Also needs to type convert the return of self.params.get from string to
> integer.
You're right.
> [...]
--
Eduardo
Re: [Qemu-devel] [PATCH for-4.0 v2] virtio: Provide version-specific variants of virtio PCI devices
On Mon, 19 Nov 2018 13:42:58 -0500 "Michael S. Tsirkin" wrote: > On Mon, Nov 19, 2018 at 07:32:38PM +0100, Cornelia Huck wrote: > > On Mon, 19 Nov 2018 13:07:59 -0500 > > "Michael S. Tsirkin" wrote: > > > And I strongly believe command line users really really do not want all > > > this mess. Even adding "pci" is the name confuses people (what are the > > > other options?). For command line model=virtio is pretty much perfect. > > > > I'd argue that it's problematic on platforms where you have different > > options for virtio transports. What does "model=virtio" mean? Always > > virtio-pci, if available? Or rather virtio-, with > > being the best option for the machine? > > Most people don't care, for them it's "whatever works". > > We have this assumption that if we force a choice then people will > choose the right thing but in practice they will do what we all do, play > with it until it kind of works and leave well alone afterwards. > That's at best - at worst give up and use an easier tool. That implies that we (the developers) need to care and make sure that "model=virtio" gets them the best possible transport (i.e. on s390x, that would be ccw unless the user explicitly requests pci; I'm not sure what the situation with mmio is -- probably "use pci whenever possible"?) I think that's what libvirt already gives us today (I hope.) What makes it messy on the pci side is that the "best option" actually depends on what kind of guest the user wants to run (if the guest is too old, you're stuck with transitional; if you want to reap the benefits of PCIe, you need non-transitional...)
Re: [Qemu-devel] 3.1.0-rc{0,1} doesn't start
* baldu...@units.it (baldu...@units.it) wrote: > > A colleague has confirmed this on his FX-8320 on Fedora 29 with the > > virt-next repo; so it's nothing that's special about your machine; > > it's 3.1 that really doesn't like the old AMDs. > > ouch! does this mean that I must stop upgrading qemu or do you think > that some development work will be directed towards this "back > compatibility" issue? It'll probably be fixed; thanks to your report I've added it to the known issues part of the 3.1rc list. > (apologies for being totally unable to help with patches/code...I know > zero about all the qemu business) Those bits of the MSRs are pretty specialised as well; so I've asked a few people more familiar with them to jump in. Dave > thanks a lot > ciao > -gabriele -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] 3.1.0-rc{0,1} doesn't start
> A colleague has confirmed this on his FX-8320 on Fedora 29 with the > virt-next repo; so it's nothing that's special about your machine; > it's 3.1 that really doesn't like the old AMDs. ouch! does this mean that I must stop upgrading qemu or do you think that some development work will be directed towards this "back compatibility" issue? (apologies for being totally unable to help with patches/code...I know zero about all the qemu business) thanks a lot ciao -gabriele
Re: [Qemu-devel] [PATCH] Acceptance test: add coverage tests for -smp option
On 11/12/2018 02:31 PM, Eduardo Habkost wrote:
On Fri, Nov 09, 2018 at 02:58:00PM -0500, Wainer dos Santos Moschetta wrote:
This adds tests for SMP option, by passing -smp with
various combinations of cpus, cores, threads, and sockets
values it checks that invalid topologies are not accepted
as well as missing values are correctly calculated.
Signed-off-by: Wainer dos Santos Moschetta
The test code looks nice to me, but: how exactly do you expect
this to be executed?
'make check-acceptance' executes it with default parameters (i.e.
cores=2, threads=2, sockets=2, and cpus=8).
It is possible to overwrite the default parameters by using Avocado's -p
option. For example, 'avocado run -p sockets=1 -p cores=2 -p threads=1
tests/acceptance/smp_option_coverage.py'.
Do we have a test runner or multiplexer configuration already
able to generate the cores/threads/sockets/cpus parameters?
I did not have any variants file until you asked. Then I realized the
problems (see inline below) that I will need to address on a v2 patch.
Anyway, the variants YAML file may look like this (adapted from an
example created by Cleber Rosa):
# cat smp_variants.yaml
!mux
min:
sockets: 1
cores: 2
threads: 1
medium:
sockets: 2
cores: 2
threads: 2
max:
sockets: 2
cores: 8
threads: 16
The smp_variants.yaml defines 3 variants (min, medium, and max), each
with a different SMP topology. You can run the tests as:
# avocado run tests/acceptance/smp_option_coverage.py -m smp_variants.yaml
JOB ID : 08d9736942e550226de9c3425a9b65c378b6654a
JOB LOG : /root/avocado/job-results/job-2018-11-19T13.19-08d9736/job.log
(01/60)
tests/acceptance/smp_option_coverage.py:SmpOption.test_cpus_eq_maxcpus;min-7e5d:
PASS (0.04 s)
< output omitted >
(60/60)
tests/acceptance/smp_option_coverage.py:SmpOption.test_cpus_gt_cores_threads_sockets;max-a8e5:
PASS (0.02 s)
RESULTS : PASS 60 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0
| CANCEL 0
JOB TIME : 6.25 s
If you wish I can distribute that file with this patch too. I can use
Avocado's test data file mechanism to run the variants as described in:
https://avocado-framework.readthedocs.io/en/65.0/WritingTests.html#accessing-test-data-files
As an alternative I can document all that on docs/devel/testing.rst.
Whatever you prefer.
Thanks for the review!
- Wainer
---
tests/acceptance/smp_option_coverage.py | 218
1 file changed, 218 insertions(+)
create mode 100644 tests/acceptance/smp_option_coverage.py
diff --git a/tests/acceptance/smp_option_coverage.py
b/tests/acceptance/smp_option_coverage.py
new file mode 100644
index 00..ab68d1a67d
--- /dev/null
+++ b/tests/acceptance/smp_option_coverage.py
@@ -0,0 +1,218 @@
+# QEMU -smp option coverage test.
+#
+# Copyright (c) 2018 Red Hat, Inc.
+#
+# Author:
+# Wainer dos Santos Moschetta
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later. See the COPYING file in the top-level directory.
+
+from functools import reduce
+from avocado.utils.process import run
+
+from avocado_qemu import Test
+
+
+class SmpOption(Test):
+"""
+Launches QEMU with various cpus, cores, threads, sockets, and maxcpus
+combination through -smp option, to check it does not accept invalid SMP
+topologies as well as it is able to calculate correctly any missing values.
+
+:avocado: enable
+:avocado: tags=slow,coverage
+"""
+def setUp(self):
+super().setUp()
+self.cores = self.params.get('cores', default=2)
+self.threads = self.params.get('threads', default=2)
+self.sockets = self.params.get('sockets', default=2)
+self.cpus = self.params.get('cpus', default=8)
The self.cpus variable should not be a parameter but rather calculated
(cores * threads * sockets).
Also needs to type convert the return of self.params.get from string to
integer.
+
+def get_smp_topology(self):
+"""
+Returns a dict with the id of cores, threads and sockets.
+"""
+res = self.vm.qmp('query-hotpluggable-cpus')['return']
+cpus = [x['props'] for x in res]
+
+return reduce(lambda x, y: {'core-id':
x['core-id'].union([y['core-id']]),
+'thread-id':
x['thread-id'].union([y['thread-id']]),
+'socket-id':
x['socket-id'].union([y['socket-id']])},
+ cpus, {'core-id': set(), 'thread-id': set(),
'socket-id': set()})
+
+@staticmethod
+def build_option(**kwargs):
+"""
+Builds string for the -smp option.
+Use cpus, cores, threads, sockets, maxcpus keys.
+"""
+option_list = []
+if kwargs.get('cpus', None) is not None:
+option_list.append(str(kwargs.get('cpus')))
+for key, val in kwargs.items():
+if key == 'cpus':
+continue
+option_list.append('%s=%s' % (key,
Re: [Qemu-devel] [PATCH] 9p: take write lock on fid path updates
+-- On Thu, 15 Nov 2018, Greg Kurz wrote --+
| Recent commit 5b76ef50f62079a fixed a race where v9fs_co_open2() could
| possibly overwrite a fid path with v9fs_path_copy() while it is being
| accessed by some other thread, ie, use-after-free that can be detected
| by ASAN with a custom 9p client.
|
| It turns out that the same can happen at several locations where
| v9fs_path_copy() is used to set the fid path. The fix is again to
| take the write lock.
|
| Cc: P J P
| Reported-by: zhibin hu
| Signed-off-by: Greg Kurz
| ---
| hw/9pfs/9p.c | 15 +++
| 1 file changed, 15 insertions(+)
|
| diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
| index eef289e394d4..267a25533b77 100644
| --- a/hw/9pfs/9p.c
| +++ b/hw/9pfs/9p.c
| @@ -1391,7 +1391,9 @@ static void coroutine_fn v9fs_walk(void *opaque)
| err = -EINVAL;
| goto out;
| }
| +v9fs_path_write_lock(s);
| v9fs_path_copy(>path, );
| +v9fs_path_unlock(s);
| } else {
| newfidp = alloc_fid(s, newfid);
| if (newfidp == NULL) {
| @@ -2160,6 +2162,7 @@ static void coroutine_fn v9fs_create(void *opaque)
| V9fsString extension;
| int iounit;
| V9fsPDU *pdu = opaque;
| +V9fsState *s = pdu->s;
|
| v9fs_path_init();
| v9fs_string_init();
| @@ -2200,7 +2203,9 @@ static void coroutine_fn v9fs_create(void *opaque)
| if (err < 0) {
| goto out;
| }
| +v9fs_path_write_lock(s);
| v9fs_path_copy(>path, );
| +v9fs_path_unlock(s);
| err = v9fs_co_opendir(pdu, fidp);
| if (err < 0) {
| goto out;
| @@ -2216,7 +2221,9 @@ static void coroutine_fn v9fs_create(void *opaque)
| if (err < 0) {
| goto out;
| }
| +v9fs_path_write_lock(s);
| v9fs_path_copy(>path, );
| +v9fs_path_unlock(s);
| } else if (perm & P9_STAT_MODE_LINK) {
| int32_t ofid = atoi(extension.data);
| V9fsFidState *ofidp = get_fid(pdu, ofid);
| @@ -2234,7 +2241,9 @@ static void coroutine_fn v9fs_create(void *opaque)
| fidp->fid_type = P9_FID_NONE;
| goto out;
| }
| +v9fs_path_write_lock(s);
| v9fs_path_copy(>path, );
| +v9fs_path_unlock(s);
| err = v9fs_co_lstat(pdu, >path, );
| if (err < 0) {
| fidp->fid_type = P9_FID_NONE;
| @@ -2272,7 +2281,9 @@ static void coroutine_fn v9fs_create(void *opaque)
| if (err < 0) {
| goto out;
| }
| +v9fs_path_write_lock(s);
| v9fs_path_copy(>path, );
| +v9fs_path_unlock(s);
| } else if (perm & P9_STAT_MODE_NAMED_PIPE) {
| err = v9fs_co_mknod(pdu, fidp, , fidp->uid, -1,
| 0, S_IFIFO | (perm & 0777), );
| @@ -2283,7 +2294,9 @@ static void coroutine_fn v9fs_create(void *opaque)
| if (err < 0) {
| goto out;
| }
| +v9fs_path_write_lock(s);
| v9fs_path_copy(>path, );
| +v9fs_path_unlock(s);
| } else if (perm & P9_STAT_MODE_SOCKET) {
| err = v9fs_co_mknod(pdu, fidp, , fidp->uid, -1,
| 0, S_IFSOCK | (perm & 0777), );
| @@ -2294,7 +2307,9 @@ static void coroutine_fn v9fs_create(void *opaque)
| if (err < 0) {
| goto out;
| }
| +v9fs_path_write_lock(s);
| v9fs_path_copy(>path, );
| +v9fs_path_unlock(s);
| } else {
| err = v9fs_co_open2(pdu, fidp, , -1,
| omode_to_uflags(mode)|O_CREAT, perm, );
|
Looks okay.
Reviewed-by: Prasad J Pandit
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Re: [Qemu-devel] [PATCH for-4.0 v2] virtio: Provide version-specific variants of virtio PCI devices
On Mon, Nov 19, 2018 at 07:32:38PM +0100, Cornelia Huck wrote: > On Mon, 19 Nov 2018 13:07:59 -0500 > "Michael S. Tsirkin" wrote: > > > On Mon, Nov 19, 2018 at 11:41:05AM +0100, Cornelia Huck wrote: > > > On Fri, 16 Nov 2018 01:45:51 -0200 > > > Eduardo Habkost wrote: > > > > > > > On Thu, Nov 15, 2018 at 05:29:24PM +0100, Andrea Bolognani wrote: > > > > > > And once that's done, "non-transitional" (while matching the > > > > > language of the spec) starts to look a bit unnecessary when you > > > > > could simply have > > > > > > > > > > virtio-*-pci > > > > > virtio-*-pci-1 > > > > > virtio-*-pci-1-transitional > > > > > > > > > > instead. But I don't feel as strongly about this as I do about > > > > > keeping the virtio revision in the device name :) > > > > > > > > I like that suggestion. Makes the device names more explicit > > > > _and_ shorter. I'll do that in v3. > > > > > > OTOH, that would mean we'd need to introduce new device types if we > > > ever start to support a virtio 2.x standard. My understanding was that > > > we'll want separate device types for transitional and non-transitional > > > for two reasons: the bus which a device can be plugged into, and > > > changing ids. Do we really expect huge changes in a possible 2.x > > > standard that affect virtio-pci only, and not other virtio transports > > > as well? > > > > Yes I think adding a version there is a mistake. > > transitional/legacy/non-transitional are spec terms so > > they are unlikely to change abruptly. OTOH virtio TC can > > just decide next version is 2.0 on a drop of a hat. > > I don't really expect that to happen on a drop of a hat, though :) It's > probably more likely when we either have some really major change (and > we messed up if we can't handle that via the virtio compatibility > mechanism), or we go up from 1.x because x is getting too large (won't > happen in the near future.) > > But yes, we should be able to handle further updates without any change > like the ones complicating things now for virtio-pci, as that was > mostly the transition to a proper standard while flushing out some > design problems that you only become aware of later. > > > > > And I strongly believe command line users really really do not want all > > this mess. Even adding "pci" is the name confuses people (what are the > > other options?). For command line model=virtio is pretty much perfect. > > I'd argue that it's problematic on platforms where you have different > options for virtio transports. What does "model=virtio" mean? Always > virtio-pci, if available? Or rather virtio-, with > being the best option for the machine? Most people don't care, for them it's "whatever works". We have this assumption that if we force a choice then people will choose the right thing but in practice they will do what we all do, play with it until it kind of works and leave well alone afterwards. That's at best - at worst give up and use an easier tool. > > So all these names are there primarily for libvirt's benefit. > > And the only input from libvirt devs so far > > has been that it's unclear how does cross version > > migration work. That needs to be addressed in some way. > > > > So can we maybe start with a libvirt domain xml patch, with an > > implementation for existing QEMU, get that acked, and then just map it > > back to qemu command line as directly as possible? > >
Re: [Qemu-devel] SeaBIOS booting time optimization
On Mon, Nov 19, 2018 at 3:15 PM Gerd Hoffmann wrote: > On Mon, Nov 19, 2018 at 01:07:13PM +, Stefan Hajnoczi wrote: > > On Mon, Nov 19, 2018 at 11:42:28AM +0100, Stefano Garzarella wrote: > > > On Mon, Nov 19, 2018 at 9:49 AM Gerd Hoffmann > wrote: > > > > > > > Why at runtime? What is bad with two images? With a runtime option > > > > you probably need some way to enable the "fastboot" hint for seabios, > > > > because some optimizations (like skipping ps/2 initialization) breaks > > > > functionality. So it must be opt-in so you can enable it if you know > > > > your use case is fine with that. But "qemu -boot fast=on" isn't much > > > > different from "qemu -bios seabios-fastboot.bin" after all ... > > > > > > > > > > You are right, but maybe having a single image can be simpler to > distribute, > > > and we can implement something (eg. using fw_cfg) to selectively enable > > > features needed. > > > Anyway, as the first step, I'll try to build a separate image of > SeaBIOS. > > > > Gerd, you may be right that explicit an command-line option like -boot > > fast=on is required. I was hoping SeaBIOS improvements could > > automatically detect cases where no functionality is lost and would not > > require new command-line options for users or management tools (e.g. > > libvirt). > > Will probably be possible to a certain degree, but I expect you can't > get 100% of the qboot savings without something like -boot fast=on. > > > For example, if SeaBIOS sees -kernel, is it necessary to follow the full > > BIOS boot process including loading option ROMs? > > Well, right now -kernel loading is actually implemented by an option rom > ... > > > This way we could avoid scanning PCI devices. > > That has consequences for the acpi tables, because qemu checks where the > firmware mapped the pci bars when generating the tables. > > Also it is not a given that skipping pci initialization in seabios is a > win in the end. The linux kernel has to handle pci bar configuration > then so it might be we just shift around boot times. > > > Admittedly I don't know the answer to how transparent we can make this, > > but I hope Stefano will identify how far we can go. > > Yes, lets see. > Hi guys, just an update, I enabled the debug prints and I saw two timeouts fired with a lot of time lost (~780ms between "init timer" and "Scan for VGA ..."), putting other prints I discovered that a lot of time is spent in the tpm_setup(), during the probe of the 2 TPM devices: 00.548869 init timer 00.549677 ./src/post.c:157 platform_hardware_setup 00.550182 ./src/hw/tpm_drivers.c:579 tpmhw_probe 01.300833 WARNING - Timeout at wait_reg8:81! 01.301388 ./src/hw/tpm_drivers.c:579 tpmhw_probe 01.331843 WARNING - Timeout at wait_reg8:81! 01.332316 ./src/post.c:160 platform_hardware_setup 01.58 Scan for VGA option rom Indeed, in the probe of the TPM devices (TIS and CRB) there are timeouts of 750 ms and 30 ms respectively. Then, statically disabling TPM and TCG (CONFIG_TCGBIOS) the time spent in SeaBIOS goes down from ~846ms to ~56ms: # SeaBIOS disabling CONFIG_TCGBIOS BIOS=/home/stefano/repos/seabios/out_notcgbios/bios.bin qemu_init_end: 40.658371 fw_start: 40.850395 (+0.192024) fw_do_boot: 96.750142 (+55.899747) linux_start_boot: 98.880578 (+2.130436) The tpm_setup() is called after the qemu_cfg_init(), so I think we can disable this call using some fw_cfg parameters, if we will decide to implement a runtime approach. Cheers, Stefano > cheers, > Gerd > > -- Stefano Garzarella Red Hat
Re: [Qemu-devel] 3.1.0-rc{0,1} doesn't start
* baldu...@units.it (baldu...@units.it) wrote:
> hi
>
> thanks for taking the time to reply
>
> Dr. David Alan Gilbert writes:
> > > I suspect that this might be some problem on my side, as I couldn't
> > > find any similar report (apart some old (qemu-2.8.50) threads, that
> > > didn't help)
> >
> > Not necessarily; can you tell me:
> > a) At what point does it fail - immediately when booting the guest?
> > Some time during the boot? Later?
> > b) What guest does it happen on?
>
> a) the error happens almost immediately; I mean: when I run qemu from an
>xterm, it doesn't even popup its window: it just dumps the error
>message to the terminal and stops
> b) the guest is an old windows XP OS; but, as I say above, all goes as
>if qemu doesn't even load the OS image (at least this is my
>impression)
>
A colleague has confirmed this on his FX-8320 on Fedora 29 with the
virt-next repo; so it's nothing that's special about your machine;
it's 3.1 that really doesn't like the old AMDs.
Dave
> Meantime, I have tried to (quick) disable the error
> catching/asserting in i386/kvm.c:
>
> install:41> diff ./qemu-3.1.0-rc1/target/i386/kvm.c.MSR_HACK
> ./qemu-3.1.0-rc1/target/i386/kvm.c
> 2205c2205
> < if (ret < cpu->kvm_msr_buf->nmsrs) {
> ---
> > if (1==0) {
> 2211c2211
> < assert(ret == cpu->kvm_msr_buf->nmsrs);
> ---
> > assert(1==1);
> 2524c2524
> < if (ret < cpu->kvm_msr_buf->nmsrs) {
> ---
> > if (1==0) {
> 2530c2530
> < assert(ret == cpu->kvm_msr_buf->nmsrs);
> ---
> > assert(1==1);
>
> and that makes qemu start and work without apparent problems.
> Of course, that is a crude and risky (I guess) workaround...
>
> thanks again
>
> ciao
> -gabriele
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH for-4.0 v2] virtio: Provide version-specific variants of virtio PCI devices
On Mon, 19 Nov 2018 13:07:59 -0500 "Michael S. Tsirkin" wrote: > On Mon, Nov 19, 2018 at 11:41:05AM +0100, Cornelia Huck wrote: > > On Fri, 16 Nov 2018 01:45:51 -0200 > > Eduardo Habkost wrote: > > > > > On Thu, Nov 15, 2018 at 05:29:24PM +0100, Andrea Bolognani wrote: > > > > And once that's done, "non-transitional" (while matching the > > > > language of the spec) starts to look a bit unnecessary when you > > > > could simply have > > > > > > > > virtio-*-pci > > > > virtio-*-pci-1 > > > > virtio-*-pci-1-transitional > > > > > > > > instead. But I don't feel as strongly about this as I do about > > > > keeping the virtio revision in the device name :) > > > > > > I like that suggestion. Makes the device names more explicit > > > _and_ shorter. I'll do that in v3. > > > > OTOH, that would mean we'd need to introduce new device types if we > > ever start to support a virtio 2.x standard. My understanding was that > > we'll want separate device types for transitional and non-transitional > > for two reasons: the bus which a device can be plugged into, and > > changing ids. Do we really expect huge changes in a possible 2.x > > standard that affect virtio-pci only, and not other virtio transports > > as well? > > Yes I think adding a version there is a mistake. > transitional/legacy/non-transitional are spec terms so > they are unlikely to change abruptly. OTOH virtio TC can > just decide next version is 2.0 on a drop of a hat. I don't really expect that to happen on a drop of a hat, though :) It's probably more likely when we either have some really major change (and we messed up if we can't handle that via the virtio compatibility mechanism), or we go up from 1.x because x is getting too large (won't happen in the near future.) But yes, we should be able to handle further updates without any change like the ones complicating things now for virtio-pci, as that was mostly the transition to a proper standard while flushing out some design problems that you only become aware of later. > > And I strongly believe command line users really really do not want all > this mess. Even adding "pci" is the name confuses people (what are the > other options?). For command line model=virtio is pretty much perfect. I'd argue that it's problematic on platforms where you have different options for virtio transports. What does "model=virtio" mean? Always virtio-pci, if available? Or rather virtio-, with being the best option for the machine? > So all these names are there primarily for libvirt's benefit. > And the only input from libvirt devs so far > has been that it's unclear how does cross version > migration work. That needs to be addressed in some way. > > So can we maybe start with a libvirt domain xml patch, with an > implementation for existing QEMU, get that acked, and then just map it > back to qemu command line as directly as possible? >
Re: [Qemu-devel] [PATCH v5 05/24] hw: acpi: Implement XSDT support for RSDP
On Thu, Nov 08, 2018 at 03:16:23PM +0100, Igor Mammedov wrote:
> On Mon, 5 Nov 2018 02:40:28 +0100
> Samuel Ortiz wrote:
>
> > XSDT is the 64-bit version of the legacy ACPI RSDT (Root System
> > Description Table). RSDT only allow for 32-bit addressses and have thus
> > been deprecated. Since ACPI version 2.0, RSDPs should point at XSDTs and
> > no longer RSDTs, although RSDTs are still supported for backward
> > compatibility.
> >
> > Since version 2.0, RSDPs should add an extended checksum, a complete table
> > length and a version field to the table.
>
> This patch re-implements what arm/virt board already does
> and fixes checksum bug in the later and at the same time
> without a user (within the patch).
>
> I'd suggest redo it a way similar to FADT refactoring
> patch 1: fix checksum bug in virt/arm
> patch 2: update reference tables in test
> patch 3: introduce AcpiRsdpData similar to commit 937d1b587
> (both arm and x86) wich stores all data in hos byte order
> patch 4: convert arm's impl. to build_append_int_noprefix() API (commit
> 5d7a334f7)
>... move out to aml-build.c
> patch 5: reuse generalized arm's build_rsdp() for x86, dropping x86
> specific one
> amending it to generate rev1 variant defined by revision in AcpiRsdpData
> (commit dd1b2037a)
>
> 'make check V=1' shouldn't observe any ACPI tables changes after patch 2.
And your next suggestion is to add patch 6. I guess it's doable but
this will make a single patch a 6 patch series. At this rate this series
will be at 200 patches easily.
Automated checks are cool but hey it's easy to see what changed in a
disassembled table, and we do not update them blindly. So just note in
the comment that there's a table change for ARM and expected files need
to be updated and we should be fine IMHO.
> > Signed-off-by: Samuel Ortiz
> > ---
> > include/hw/acpi/aml-build.h | 3 +++
> > hw/acpi/aml-build.c | 37 +
> > 2 files changed, 40 insertions(+)
> >
> > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> > index c9bcb32d81..3580d0ce90 100644
> > --- a/include/hw/acpi/aml-build.h
> > +++ b/include/hw/acpi/aml-build.h
> > @@ -393,6 +393,9 @@ void
> > build_rsdp(GArray *table_data,
> > BIOSLinker *linker, unsigned rsdt_tbl_offset);
> > void
> > +build_rsdp_xsdt(GArray *table_data,
> > +BIOSLinker *linker, unsigned xsdt_tbl_offset);
> > +void
> > build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
> > const char *oem_id, const char *oem_table_id);
> > void
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index 51b608432f..a030d40674 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -1651,6 +1651,43 @@ build_xsdt(GArray *table_data, BIOSLinker *linker,
> > GArray *table_offsets,
> > (void *)xsdt, "XSDT", xsdt_len, 1, oem_id, oem_table_id);
> > }
> >
> > +/* RSDP pointing at an XSDT */
> > +void
> > +build_rsdp_xsdt(GArray *rsdp_table,
> > +BIOSLinker *linker, unsigned xsdt_tbl_offset)
> > +{
> > +AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
> > +unsigned xsdt_pa_size = sizeof(rsdp->xsdt_physical_address);
> > +unsigned xsdt_pa_offset =
> > +(char *)>xsdt_physical_address - rsdp_table->data;
> > +unsigned xsdt_offset =
> > +(char *)>length - rsdp_table->data;
There's a cleaner way to get at the offsets than pointer math:
1. save rsdp_table length before you push
2. add offset_of for fields
If switching to build_append_int_noprefix then it's even
easier - just save length before you append the int
you intend to patch.
> > +
> > +bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
> > + true /* fseg memory */);
> > +
> > +memcpy(>signature, "RSD PTR ", 8);
> > +memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6);
> > +rsdp->length = cpu_to_le32(sizeof(*rsdp));
> > +/* version 2, we will use the XSDT pointer */
> > +rsdp->revision = 0x02;
> > +
> > +/* Address to be filled by Guest linker */
> > +bios_linker_loader_add_pointer(linker,
> > +ACPI_BUILD_RSDP_FILE, xsdt_pa_offset, xsdt_pa_size,
> > +ACPI_BUILD_TABLE_FILE, xsdt_tbl_offset);
> > +
> > +/* Legacy checksum to be filled by Guest linker */
> > +bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
> > +(char *)rsdp - rsdp_table->data, xsdt_offset,
> > +(char *)>checksum - rsdp_table->data);
> > +
> > +/* Extended checksum to be filled by Guest linker */
> > +bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
> > +(char *)rsdp - rsdp_table->data, sizeof *rsdp,
> > +(char *)>extended_checksum - rsdp_table->data);
> > +}
> > +
> > void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> >
[Qemu-devel] [PATCH v2] target/i386: kvm: add VMX migration blocker
Nested VMX does not support live migration yet. Add a blocker
until that is worked out.
Nested SVM only does not support it, but unfortunately it is
enabled by default for -cpu host so we cannot really disable it.
Signed-off-by: Paolo Bonzini
---
target/i386/kvm.c | 12
1 file changed, 12 insertions(+)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index f524e7d929..27dcca5365 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -854,6 +854,7 @@ static int hyperv_init_vcpu(X86CPU *cpu)
}
static Error *invtsc_mig_blocker;
+static Error *vmx_mig_blocker;
#define KVM_MAX_CPUID_ENTRIES 100
@@ -1246,6 +1247,17 @@ int kvm_arch_init_vcpu(CPUState *cs)
!!(c->ecx & CPUID_EXT_SMX);
}
+if ((env->features[FEAT_1_ECX] & CPUID_EXT_VMX) && !vmx_mig_blocker) {
+error_setg(_mig_blocker,
+ "Nested VMX virtualization does not support live migration
yet");
+r = migrate_add_blocker(vmx_mig_blocker, _err);
+if (local_err) {
+error_report_err(local_err);
+error_free(vmx_mig_blocker);
+return r;
+}
+}
+
if (env->mcg_cap & MCG_LMCE_P) {
has_msr_mcg_ext_ctl = has_msr_feature_control = true;
}
--
2.19.1
Re: [Qemu-devel] [PATCH] target/i386: kvm: add VMX and SVM migration blockers
On 16/11/18 17:56, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonz...@redhat.com) wrote:
>> Nested VMX and SVM do not support live migration yet. Add a blocker
>> until that is worked out.
>>
>> Signed-off-by: Paolo Bonzini
>> ---
>> target/i386/kvm.c | 25 +
>> 1 file changed, 25 insertions(+)
>>
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index db1f4104b6..3b6fbd3f20 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -860,6 +860,8 @@ static int hyperv_init_vcpu(X86CPU *cpu)
>> }
>>
>> static Error *invtsc_mig_blocker;
>> +static Error *vmx_mig_blocker;
>> +static Error *svm_mig_blocker;
>>
>> #define KVM_MAX_CPUID_ENTRIES 100
>>
>> @@ -1250,6 +1252,29 @@ int kvm_arch_init_vcpu(CPUState *cs)
>> if (c) {
>> has_msr_feature_control = !!(c->ecx & CPUID_EXT_VMX) ||
>>!!(c->ecx & CPUID_EXT_SMX);
>> +
>> +}
>> +
>> +if ((env->features[FEAT_1_ECX] & CPUID_EXT_VMX) && !vmx_mig_blocker) {
>> +error_setg(_mig_blocker,
>> + "Nested VMX virtualization does not support live
>> migration yet");
>> +r = migrate_add_blocker(vmx_mig_blocker, _err);
>> +if (local_err) {
>> +error_report_err(local_err);
>> +error_free(vmx_mig_blocker);
>> +return r;
>> +}
>> +}
>> +
>> +if ((env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) &&
>> !svm_mig_blocker) {
>> +error_setg(_mig_blocker,
>> + "Nested SVM virtualization does not support live
>> migration yet");
>> +r = migrate_add_blocker(svm_mig_blocker, _err);
>> +if (local_err) {
>> +error_report_err(local_err);
>> +error_free(svm_mig_blocker);
>> +return r;
>> +}
>
> I think that's OK from a migration point of view; my only worry is if
> people have nesting enabled by default. On AMD isn't it common to have
> it enabled by default in KVM? Especially if using -cpu host ?
Hmm, yeah. I'll send a v2 with only nested VMX.
Paolo
Re: [Qemu-devel] [PATCH] migration: savevm: consult migration blockers
On 16/11/18 18:12, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonz...@redhat.com) wrote:
>> There is really no difference between live migration and savevm, except
>> that savevm does not require bdrv_invalidate_cache to be implemented
>> by all disks. However, it is unlikely that savevm is used with anything
>> except qcow2 disks, so the penalty is small and worth the improvement
>> in catching bad usage of savevm.
>>
>> Only one place was taking care of savevm when adding a migration blocker,
>> and it can be removed.
>
> OK, I'm not sure if it was actually a split between savevm and migration
> or just that the migration blockers were added later.
Just the latter, I think.
>> Signed-off-by: Paolo Bonzini
>> ---
>> migration/savevm.c | 4
>> target/i386/kvm.c | 3 ---
>> 2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index ef707b8c43..1c49776a91 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -2455,6 +2455,10 @@ int save_snapshot(const char *name, Error **errp)
>> struct tm tm;
>> AioContext *aio_context;
>>
>> +if (migration_is_blocked(errp)) {
>> +return false;
>> +}
>> +
>> if (!replay_can_snapshot()) {
>> error_setg(errp, "Record/replay does not allow making snapshot "
>> "right now. Try once more later.");
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index 3b6fbd3f20..d222b68fe4 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -1284,7 +1284,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
>> if (!env->user_tsc_khz) {
>> if ((env->features[FEAT_8000_0007_EDX] & CPUID_APM_INVTSC) &&
>> invtsc_mig_blocker == NULL) {
>> -/* for migration */
>> error_setg(_mig_blocker,
>> "State blocked by non-migratable CPU device"
>> " (invtsc flag)");
>> @@ -1294,8 +1293,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
>> error_free(invtsc_mig_blocker);
>> return r;
>> }
>> -/* for savevm */
>> -vmstate_x86_cpu.unmigratable = 1;
>
> So that means vmstate_x86_cpu can be static now - but why does it live
> in machine.c rather than cpu.c ?
Generally all vmstate lives in machine.c. Just historical reasons, it
was moved out of vl.c (!) many moons ago.
Paolo
Re: [Qemu-devel] [PATCH v5 00/24] ACPI reorganization for hardware-reduced API addition
On Mon, Nov 19, 2018 at 06:14:26PM +0100, Paolo Bonzini wrote: > On 19/11/18 16:31, Igor Mammedov wrote: > > I've tried to give suggestions how to restructure series > > on per patch basis. In my opinion it quite possible to split > > series in several smaller ones and it should really help with > > making series cleaner and easier/faster to review/amend/merge > > vs what we have in v5. > > This is true, on the other hand the series makes sense together and, > even if the patches are more or less independent, they also all follow > the same "plan". For reviewing v6, are you aware of Patchew's series > diff functionality? It can tell you which patches had comments in v5, > reorder patches if applicable, and display deleted and new patches at > the right point in the series. > > v4->v5 is a bit messed up because Samuel probably added a diff order > setup > (https://patchew.org/QEMU/20181101102303.16439-1-sa...@linux.intel.com/diff/20181105014047.26447-1-sa...@linux.intel.com/) > but it's very useful in general. > > Paolo Oh I didn't realize difforder breaks patchew. Or is the problem only if one switches from no order to difforder? -- MST
Re: [Qemu-devel] [PULL 00/10] target-arm queue
On 19 November 2018 at 15:57, Peter Maydell wrote: > Some Arm bugfixes for rc2... > > thanks > -- PMM > > The following changes since commit e6ebbd46b6e539f3613136111977721d212c2812: > > Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging > (2018-11-19 14:31:48 +) > > are available in the Git repository at: > > https://git.linaro.org/people/pmaydell/qemu-arm.git > tags/pull-target-arm-20181119 > > for you to fetch changes up to a00d7f2048c2a1a6a4487ac195c804c78adcf60e: > > MAINTAINERS: list myself as maintainer for various Arm boards (2018-11-19 > 15:55:11 +) > > > target-arm queue: > * various MAINTAINERS file updates > * hw/block/onenand: use qemu_log_mask() for reporting > * hw/block/onenand: Fix off-by-one error allowing out-of-bounds read >on the n800 and n810 machine models > * target/arm: fix smc incorrectly trapping to EL3 when secure is off > * hw/arm/stm32f205: Fix the UART and Timer region size > * target/arm: read ID registers for KVM guests so they can be >used to gate "is feature X present" checks > > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH for-4.0 v2] virtio: Provide version-specific variants of virtio PCI devices
On Mon, Nov 19, 2018 at 11:41:05AM +0100, Cornelia Huck wrote: > On Fri, 16 Nov 2018 01:45:51 -0200 > Eduardo Habkost wrote: > > > On Thu, Nov 15, 2018 at 05:29:24PM +0100, Andrea Bolognani wrote: > > > > One thing that I'm very much not convinced about is the naming, > > > specifically leaving the virtio revision out: I get it that we > > > Should Never Need™ another major version of the spec, but I'm > > > afraid discounting the possibility outright might prove to be > > > shortsighted and come back to bite us later, so I'd much rather > > > keep it. That's not the claim. In fact the reverse is true - a major revision can come at any point. The claim is that virtio compatibility is not based on version numbers. And another claim is that you can trust the virtio TC not to overload terminology that it put in the spec. So use that and you should be fine. Come up with your own and end up writing another spec just to document it. > > > > > > And once that's done, "non-transitional" (while matching the > > > language of the spec) starts to look a bit unnecessary when you > > > could simply have > > > > > > virtio-*-pci > > > virtio-*-pci-1 > > > virtio-*-pci-1-transitional > > > > > > instead. But I don't feel as strongly about this as I do about > > > keeping the virtio revision in the device name :) > > > > I like that suggestion. Makes the device names more explicit > > _and_ shorter. I'll do that in v3. > > OTOH, that would mean we'd need to introduce new device types if we > ever start to support a virtio 2.x standard. My understanding was that > we'll want separate device types for transitional and non-transitional > for two reasons: the bus which a device can be plugged into, and > changing ids. Do we really expect huge changes in a possible 2.x > standard that affect virtio-pci only, and not other virtio transports > as well? Yes I think adding a version there is a mistake. transitional/legacy/non-transitional are spec terms so they are unlikely to change abruptly. OTOH virtio TC can just decide next version is 2.0 on a drop of a hat. And I strongly believe command line users really really do not want all this mess. Even adding "pci" is the name confuses people (what are the other options?). For command line model=virtio is pretty much perfect. So all these names are there primarily for libvirt's benefit. And the only input from libvirt devs so far has been that it's unclear how does cross version migration work. That needs to be addressed in some way. So can we maybe start with a libvirt domain xml patch, with an implementation for existing QEMU, get that acked, and then just map it back to qemu command line as directly as possible? -- MST
