[Qemu-devel] [PATCH] build: add needed missing libraries libm and librt
libm is used in cutils.c, but the library was not specified when linking some binaries, throwing the following error: cutils.o: In function `strtosz_suffix_unit': /home/royger/xen-clean/tools/qemu-xen-dir/cutils.c:354: undefined reference to `__isnan' /home/royger/xen-clean/tools/qemu-xen-dir/cutils.c:357: undefined reference to `modf' collect2: ld returned 1 exit status According to modf man page [0], -lm should be used when linking. librt is used in qemu-time.c, but again the library was not specified at link time, throwing the following error: /home/royger/xen-clean/tools/qemu-xen-dir/qemu-timer.c:597: undefined reference to `timer_gettime' /home/royger/xen-clean/tools/qemu-xen-dir/qemu-timer.c:610: undefined reference to `timer_settime' ../qemu-timer.o: In function `dynticks_start_timer': /home/royger/xen-clean/tools/qemu-xen-dir/qemu-timer.c:565: undefined reference to `timer_create' ../qemu-timer.o: In function `dynticks_stop_timer': /home/royger/xen-clean/tools/qemu-xen-dir/qemu-timer.c:583: undefined reference to `timer_delete' collect2: ld returned 1 exit status According to timer_getttime man page [1], -lrt should be used when linking. [0] http://linux.die.net/man/3/modf [1] http://linux.die.net/man/2/timer_gettime Signed-off-by: Roger Pau Monne --- Makefile|4 ++-- Makefile.target |2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 301c75e..e2c3cd4 100644 --- a/Makefile +++ b/Makefile @@ -34,7 +34,7 @@ configure: ; $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw) -LIBS+=-lz $(LIBS_TOOLS) +LIBS+=-lz -lm -lrt $(LIBS_TOOLS) ifdef BUILD_DOCS DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 QMP/qmp-commands.txt @@ -170,7 +170,7 @@ test-coroutine: test-coroutine.o qemu-timer-common.o async.o $(coroutine-obj-y) $(qapi-obj-y): $(GENERATED_HEADERS) qapi-dir := $(BUILD_DIR)/qapi-generated test-visitor.o test-qmp-commands.o qemu-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir) -qemu-ga$(EXESUF): LIBS = $(LIBS_QGA) +qemu-ga$(EXESUF): LIBS = $(LIBS_QGA) -lm $(qapi-dir)/test-qapi-types.c $(qapi-dir)/test-qapi-types.h :\ $(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py diff --git a/Makefile.target b/Makefile.target index a111521..95d6bc0 100644 --- a/Makefile.target +++ b/Makefile.target @@ -33,6 +33,8 @@ endif PROGS=$(QEMU_PROG) STPFILES= +LIBS+=-lrt + ifndef CONFIG_HAIKU LIBS+=-lm endif -- 1.7.9
[Qemu-devel] Status of query-netdev QMP command
Hello, I've read from the GSoC/2010 that some work was being done creating a query-netdev QMP command: http://wiki.qemu.org/Google_Summer_of_Code_2010/QMP#query-netdev The status says that "mentor has merged it into his tree", but I cannot see this command anywhere upstream, and it will come really handy for what I'm trying to do, do someone know where this has gone? Thanks, Roger.
Re: [Qemu-devel] Status of query-netdev QMP command
Stefan Hajnoczi wrote: On Mon, Jun 18, 2012 at 3:19 PM, Roger Pau Monne wrote: I've read from the GSoC/2010 that some work was being done creating a query-netdev QMP command: http://wiki.qemu.org/Google_Summer_of_Code_2010/QMP#query-netdev The status says that "mentor has merged it into his tree", but I cannot see this command anywhere upstream, and it will come really handy for what I'm trying to do, do someone know where this has gone? I checked qemu.git/master and don't see it either. The HMP "info net" command lists the net devices but I'm not aware of a QMP equivalent. What are you trying to do? On Linux you can pass the name of the tap device you wish to create, and Qemu honors that, but on BSD systems you have no way of creating a tap device with a specific name, they are assigned based on the lowest free number (tap2 for example). I need the query-netdev command in order to get the name of the device that Qemu creates, so I can use it in my scripts afterwards. Roger.
[Qemu-devel] [PATCH RFC 2/3] xen_disk: fix memory leak
On ioreq_release the full ioreq was memset to 0, loosing all the data
and memory allocations inside the QEMUIOVector, which leads to a
memory leak. Create a new function to specifically reset ioreq.
Reported-by: Maik Wessler
Signed-off-by: Roger Pau Monné
Cc: xen-de...@lists.xen.org
Cc: Stefano Stabellini
Cc: Anthony PERARD
---
hw/xen_disk.c | 28 ++--
1 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index a159ee5..1eb485a 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -113,6 +113,31 @@ struct XenBlkDev {
/* - */
+static void ioreq_reset(struct ioreq *ioreq)
+{
+memset(&ioreq->req, 0, sizeof(ioreq->req));
+ioreq->status = 0;
+ioreq->start = 0;
+ioreq->presync = 0;
+ioreq->postsync = 0;
+ioreq->mapped = 0;
+
+memset(ioreq->domids, 0, sizeof(ioreq->domids));
+memset(ioreq->refs, 0, sizeof(ioreq->refs));
+ioreq->prot = 0;
+memset(ioreq->page, 0, sizeof(ioreq->page));
+ioreq->pages = NULL;
+
+ioreq->aio_inflight = 0;
+ioreq->aio_errors = 0;
+
+ioreq->blkdev = NULL;
+memset(&ioreq->list, 0, sizeof(ioreq->list));
+memset(&ioreq->acct, 0, sizeof(ioreq->acct));
+
+qemu_iovec_reset(&ioreq->v);
+}
+
static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
{
struct ioreq *ioreq = NULL;
@@ -130,7 +155,6 @@ static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
/* get one from freelist */
ioreq = QLIST_FIRST(&blkdev->freelist);
QLIST_REMOVE(ioreq, list);
-qemu_iovec_reset(&ioreq->v);
}
QLIST_INSERT_HEAD(&blkdev->inflight, ioreq, list);
blkdev->requests_inflight++;
@@ -154,7 +178,7 @@ static void ioreq_release(struct ioreq *ioreq, bool finish)
struct XenBlkDev *blkdev = ioreq->blkdev;
QLIST_REMOVE(ioreq, list);
-memset(ioreq, 0, sizeof(*ioreq));
+ioreq_reset(ioreq);
ioreq->blkdev = blkdev;
QLIST_INSERT_HEAD(&blkdev->freelist, ioreq, list);
if (finish) {
--
1.7.7.5 (Apple Git-26)
[Qemu-devel] [PATCH RFC 0/3] xen pv disk persistent grants implementation
This series contains two bug fixes for xen_disk (patches 1 & 2) and the implementation of the persistent grants extensions (patch 3), that brings a considerable speed improvement. Thanks for the reviews, Roger.
[Qemu-devel] [PATCH RFC 1/3] xen_disk: handle disk files on ramfs/tmpfs
Files that reside on ramfs or tmpfs cannot be opened with O_DIRECT,
if first call to bdrv_open fails with errno = EINVAL, try a second
call without BDRV_O_NOCACHE.
Signed-off-by: Roger Pau Monné
Cc: xen-de...@lists.xen.org
Cc: Stefano Stabellini
Cc: Anthony PERARD
---
hw/xen_disk.c | 16 +---
1 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index e6bb2f2..a159ee5 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -562,7 +562,7 @@ static void blk_alloc(struct XenDevice *xendev)
static int blk_init(struct XenDevice *xendev)
{
struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
-int index, qflags, info = 0;
+int index, qflags, info = 0, rc;
/* read xenstore entries */
if (blkdev->params == NULL) {
@@ -625,8 +625,18 @@ static int blk_init(struct XenDevice *xendev)
xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
blkdev->bs = bdrv_new(blkdev->dev);
if (blkdev->bs) {
-if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
-bdrv_find_whitelisted_format(blkdev->fileproto)) != 0)
{
+rc = bdrv_open(blkdev->bs, blkdev->filename, qflags,
+bdrv_find_whitelisted_format(blkdev->fileproto));
+if (rc != 0 && errno == EINVAL) {
+/* Files on ramfs or tmpfs cannot be opened with O_DIRECT,
+ * remove the BDRV_O_NOCACHE flag, and try to open
+ * the file again.
+ */
+qflags &= ~BDRV_O_NOCACHE;
+rc = bdrv_open(blkdev->bs, blkdev->filename, qflags,
+bdrv_find_whitelisted_format(blkdev->fileproto));
+}
+if (rc != 0) {
bdrv_delete(blkdev->bs);
blkdev->bs = NULL;
}
--
1.7.7.5 (Apple Git-26)
[Qemu-devel] [PATCH RFC 3/3] xen_disk: add persistent grant support to xen_disk backend
This protocol extension reuses the same set of grant pages for all
transactions between the front/back drivers, avoiding expensive tlb
flushes, grant table lock contention and switches between userspace
and kernel space. The full description of the protocol can be found in
the public blkif.h header.
Speed improvement with 15 guests performing I/O is ~450%.
Signed-off-by: Roger Pau Monné
Cc: xen-de...@lists.xen.org
Cc: Stefano Stabellini
Cc: Anthony PERARD
---
Performance comparison with the previous implementation can be seen in
the followign graph:
http://xenbits.xen.org/people/royger/persistent_read_qemu.png
---
hw/xen_disk.c | 155 ++--
1 files changed, 138 insertions(+), 17 deletions(-)
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 1eb485a..bafeceb 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -52,6 +52,11 @@ static int max_requests = 32;
#define BLOCK_SIZE 512
#define IOCB_COUNT (BLKIF_MAX_SEGMENTS_PER_REQUEST + 2)
+struct persistent_gnt {
+void *page;
+struct XenBlkDev *blkdev;
+};
+
struct ioreq {
blkif_request_t req;
int16_t status;
@@ -69,6 +74,7 @@ struct ioreq {
int prot;
void*page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
void*pages;
+int num_unmap;
/* aio status */
int aio_inflight;
@@ -105,6 +111,12 @@ struct XenBlkDev {
int requests_inflight;
int requests_finished;
+/* Persistent grants extension */
+gbooleanfeature_persistent;
+GTree *persistent_gnts;
+unsigned intpersistent_gnt_c;
+unsigned intmax_grants;
+
/* qemu block driver */
DriveInfo *dinfo;
BlockDriverState*bs;
@@ -138,6 +150,29 @@ static void ioreq_reset(struct ioreq *ioreq)
qemu_iovec_reset(&ioreq->v);
}
+static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
+{
+uint ua = GPOINTER_TO_UINT(a);
+uint ub = GPOINTER_TO_UINT(b);
+return (ua > ub) - (ua < ub);
+}
+
+static void destroy_grant(gpointer pgnt)
+{
+struct persistent_gnt *grant = pgnt;
+XenGnttab gnt = grant->blkdev->xendev.gnttabdev;
+
+if (xc_gnttab_munmap(gnt, grant->page, 1) != 0) {
+xen_be_printf(&grant->blkdev->xendev, 0,
+ "xc_gnttab_munmap failed: %s\n",
+ strerror(errno));
+}
+grant->blkdev->persistent_gnt_c--;
+xen_be_printf(&grant->blkdev->xendev, 3,
+ "unmapped grant %p\n", grant->page);
+g_free(grant);
+}
+
static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
{
struct ioreq *ioreq = NULL;
@@ -266,21 +301,21 @@ static void ioreq_unmap(struct ioreq *ioreq)
XenGnttab gnt = ioreq->blkdev->xendev.gnttabdev;
int i;
-if (ioreq->v.niov == 0 || ioreq->mapped == 0) {
+if (ioreq->num_unmap == 0 || ioreq->mapped == 0) {
return;
}
if (batch_maps) {
if (!ioreq->pages) {
return;
}
-if (xc_gnttab_munmap(gnt, ioreq->pages, ioreq->v.niov) != 0) {
+if (xc_gnttab_munmap(gnt, ioreq->pages, ioreq->num_unmap) != 0) {
xen_be_printf(&ioreq->blkdev->xendev, 0, "xc_gnttab_munmap failed:
%s\n",
strerror(errno));
}
-ioreq->blkdev->cnt_map -= ioreq->v.niov;
+ioreq->blkdev->cnt_map -= ioreq->num_unmap;
ioreq->pages = NULL;
} else {
-for (i = 0; i < ioreq->v.niov; i++) {
+for (i = 0; i < ioreq->num_unmap; i++) {
if (!ioreq->page[i]) {
continue;
}
@@ -298,41 +333,107 @@ static void ioreq_unmap(struct ioreq *ioreq)
static int ioreq_map(struct ioreq *ioreq)
{
XenGnttab gnt = ioreq->blkdev->xendev.gnttabdev;
-int i;
+uint32_t domids[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+uint32_t refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+void *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+int i, j, new_maps = 0;
+struct persistent_gnt *grant;
if (ioreq->v.niov == 0 || ioreq->mapped == 1) {
return 0;
}
-if (batch_maps) {
+if (ioreq->blkdev->feature_persistent) {
+for (i = 0; i < ioreq->v.niov; i++) {
+grant = g_tree_lookup(ioreq->blkdev->persistent_gnts,
+GUINT_TO_POINTER(ioreq->refs[i]));
+
+if (grant != NULL) {
+page[i] = grant->page;
+xen_be_printf(&ioreq->blkdev->xendev, 3,
+ "using persistent-grant %" PRIu32 "\n",
+ ioreq->refs[i]);
+} else {
+/* Add the grant to the list of grants that
+ * should be mapped
+ */
+domids[new_maps] = ioreq->domids[i];
+refs[new_maps] = ioreq->refs[i];
+
[Qemu-devel] [PATCH v2 3/3] serial: poll the serial console with G_IO_HUP
On FreeBSD polling a master pty while the other end is not connected
with G_IO_OUT only results in an endless wait. This is different from
the Linux behaviour, that returns immediately. In order to demonstrate
this, I have the following example code:
http://xenbits.xen.org/people/royger/test_poll.c
When executed on Linux:
$ ./test_poll
In callback
On FreeBSD instead, the callback never gets called:
$ ./test_poll
So, in order to workaround this, poll the source with G_IO_HUP (which
makes the code behave the same way on both Linux and FreeBSD).
Signed-off-by: Roger Pau Monné
Cc: Peter Crosthwaite
Cc: Michael Tokarev
Cc: "Andreas Färber"
Cc: Paolo Bonzini
Cc: xen-de...@lists.xenproject.org
---
Changes since v1:
- Fix other users of qemu_chr_fe_add_watch to use G_IO_HUP.
---
hw/char/serial.c |2 +-
hw/char/virtio-console.c |3 ++-
hw/usb/redirect.c|2 +-
monitor.c|2 +-
4 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/hw/char/serial.c b/hw/char/serial.c
index f4d167f..2a2c9e5 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -246,7 +246,7 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition
cond, void *opaque)
serial_receive1(s, &s->tsr, 1);
} else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) {
if (s->tsr_retry >= 0 && s->tsr_retry < MAX_XMIT_RETRY &&
-qemu_chr_fe_add_watch(s->chr, G_IO_OUT, serial_xmit, s) > 0) {
+qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, serial_xmit, s) >
0) {
s->tsr_retry++;
return FALSE;
}
diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 6c8be0f..38e290a 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -69,7 +69,8 @@ static ssize_t flush_buf(VirtIOSerialPort *port,
if (!k->is_console) {
virtio_serial_throttle_port(port, true);
if (!vcon->watch) {
-vcon->watch = qemu_chr_fe_add_watch(vcon->chr, G_IO_OUT,
+vcon->watch = qemu_chr_fe_add_watch(vcon->chr,
+G_IO_OUT|G_IO_HUP,
chr_write_unblocked, vcon);
}
}
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 287a505..06e757d 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -284,7 +284,7 @@ static int usbredir_write(void *priv, uint8_t *data, int
count)
r = qemu_chr_fe_write(dev->cs, data, count);
if (r < count) {
if (!dev->watch) {
-dev->watch = qemu_chr_fe_add_watch(dev->cs, G_IO_OUT,
+dev->watch = qemu_chr_fe_add_watch(dev->cs, G_IO_OUT|G_IO_HUP,
usbredir_write_unblocked, dev);
}
if (r < 0) {
diff --git a/monitor.c b/monitor.c
index 593679a..ae1c539 100644
--- a/monitor.c
+++ b/monitor.c
@@ -304,7 +304,7 @@ void monitor_flush(Monitor *mon)
mon->outbuf = tmp;
}
if (mon->watch == 0) {
-mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT,
+mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT|G_IO_HUP,
monitor_unblocked, mon);
}
}
--
1.7.7.5 (Apple Git-26)
[Qemu-devel] [PATCH v2 0/3] qemu-freebsd: fixes for running Xen guests
This three patches allow FreeBSD Xen Dom0 to use Qemu (i.e., launch HVM guests). First patch fixes the usage of ENODATA, which doesn't exist on FreeBSD and is replaced with ENOENT instead. The second patch is more controversial probably, since it introduces a FreeBSD specific version of tap_open which behaves like it's Linux counterpart, allowing Qemu to create tap interfaces and rename them. I've decided to just fork the function instead of adding a bunch more of preprocessor code in the original function. Last patch adds G_IO_HUP to calls to qemu_chr_fe_add_watch so they behave the same way on both FreeBSD and Linux (see the commit message for the rationale). Thanks for the review, Roger.
[Qemu-devel] [PATCH v2 1/3] xen: fix usage of ENODATA
ENODATA doesn't exist on FreeBSD, so ENODATA errors returned by the
hypervisor are translated to ENOENT.
Also, the error code is returned in errno if the call returns -1, so
compare the error code with the value in errno instead of the value
returned by the function.
Signed-off-by: Roger Pau Monné
Cc: xen-de...@lists.xenproject.org
Cc: Stefano Stabellini
Cc: Anthony Perard
---
Changes since v1:
- Define ENODATA to ENOENT for platforms that don't have ENODATA.
---
xen-hvm.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/xen-hvm.c b/xen-hvm.c
index a64486c..a414105 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -499,11 +499,14 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
start_addr >> TARGET_PAGE_BITS, npages,
bitmap);
if (rc < 0) {
-if (rc != -ENODATA) {
+#ifndef ENODATA
+#define ENODATA ENOENT
+#endif
+if (errno == ENODATA) {
memory_region_set_dirty(framebuffer, 0, size);
DPRINTF("xen: track_dirty_vram failed (0x" TARGET_FMT_plx
", 0x" TARGET_FMT_plx "): %s\n",
-start_addr, start_addr + size, strerror(-rc));
+start_addr, start_addr + size, strerror(errno));
}
return;
}
--
1.7.7.5 (Apple Git-26)
[Qemu-devel] [PATCH v2 2/3] tap-bsd: implement a FreeBSD only version of tap_open
The current behaviour of tap_open for BSD systems differ greatly from
it's Linux counterpart. Since FreeBSD supports interface renaming and
tap device cloning by opening /dev/tap, implement a FreeBSD specific
version of tap_open that behaves like it's Linux counterpart.
This is specially important for toolstacks that use Qemu (like Xen
libxl), in order to have a unified behaviour across suported
platforms.
Signed-off-by: Roger Pau Monné
Cc: xen-de...@lists.xenproject.org
Cc: Stefano Stabellini
Cc: Anthony Liguori
Cc: Stefan Hajnoczi
---
net/tap-bsd.c | 70 -
1 files changed, 69 insertions(+), 1 deletions(-)
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index 90f8a02..bf91bd0 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -27,12 +27,13 @@
#include "sysemu/sysemu.h"
#include "qemu/error-report.h"
-#ifdef __NetBSD__
+#if defined(__NetBSD__) || defined(__FreeBSD__)
#include
#include
#include
#endif
+#ifndef __FreeBSD__
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
int vnet_hdr_required, int mq_required)
{
@@ -108,6 +109,73 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
return fd;
}
+#else /* __FreeBSD__ */
+
+#define PATH_NET_TAP "/dev/tap"
+
+int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
+ int vnet_hdr_required, int mq_required)
+{
+int fd, s, ret;
+struct ifreq ifr;
+
+TFR(fd = open(PATH_NET_TAP, O_RDWR));
+if (fd < 0) {
+error_report("could not open %s: %s", PATH_NET_TAP, strerror(errno));
+return -1;
+}
+
+memset(&ifr, 0, sizeof(ifr));
+
+ret = ioctl(fd, TAPGIFNAME, (void *)&ifr);
+if (ret < 0) {
+error_report("could not get tap interface name");
+goto error;
+}
+
+if (ifname[0] != '\0') {
+/* User requested the interface to have a specific name */
+s = socket(AF_LOCAL, SOCK_DGRAM, 0);
+if (s < 0) {
+error_report("could not open socket to set interface name");
+goto error;
+}
+ifr.ifr_data = ifname;
+ret = ioctl(s, SIOCSIFNAME, (void *)&ifr);
+close(s);
+if (ret < 0) {
+error_report("could not set tap interface name");
+goto error;
+}
+} else {
+pstrcpy(ifname, ifname_size, ifr.ifr_name);
+}
+
+if (*vnet_hdr) {
+/* BSD doesn't have IFF_VNET_HDR */
+*vnet_hdr = 0;
+
+if (vnet_hdr_required && !*vnet_hdr) {
+error_report("vnet_hdr=1 requested, but no kernel "
+ "support for IFF_VNET_HDR available");
+goto error;
+}
+}
+if (mq_required) {
+error_report("mq_required requested, but not kernel support"
+ "for IFF_MULTI_QUEUE available");
+goto error;
+}
+
+fcntl(fd, F_SETFL, O_NONBLOCK);
+return fd;
+
+error:
+close(fd);
+return -1;
+}
+#endif /* __FreeBSD__ */
+
int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
{
return 0;
--
1.7.7.5 (Apple Git-26)
[Qemu-devel] [PATCH] block: char devices on FreeBSD are not behind a pager
Acknowledge this and forcefully set BDRV_O_NOCACHE and O_DIRECT in order to
force QEMU to use aligned buffers.
Signed-off-by: Roger Pau Monné
Cc: Kevin Wolf
Cc: Stefan Hajnoczi
---
block/raw-posix.c | 12
1 file changed, 12 insertions(+)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 86ce4f2..63841dd 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -472,6 +472,18 @@ static int raw_open_common(BlockDriverState *bs, QDict
*options,
}
#endif
}
+#ifdef __FreeBSD__
+if (S_ISCHR(st.st_mode)) {
+/*
+ * The file is a char device (disk), which on FreeBSD isn't behind
+ * a pager, so set BDRV_O_NOCACHE unconditionally. This is needed
+ * so Qemu makes sure all IO operations on the device are aligned
+ * to sector size, or else FreeBSD will reject them with EINVAL.
+ */
+bs->open_flags |= BDRV_O_NOCACHE;
+s->open_flags |= O_DIRECT;
+}
+#endif
#ifdef CONFIG_XFS
if (platform_test_xfs_fd(s->fd)) {
--
1.9.3 (Apple Git-50)
[Qemu-devel] [PATCH v2] block: char devices on FreeBSD are not behind a pager
Introduce a new flag to mark devices that require requests to be aligned and
replace the usage of BDRV_O_NOCACHE and O_DIRECT with this flag when
appropriate.
If a character device is used as a backend on a FreeBSD host set this flag
unconditionally.
Signed-off-by: Roger Pau Monné
Cc: Kevin Wolf
Cc: Stefan Hajnoczi
---
Changes since v1:
- Intead of appending BDRV_O_NOCACHE and O_DIRECT when a char dev is used
on FreeBSD introduce a new flag that is used to mark if a device needs
requests to be aligned.
---
block/raw-posix.c | 25 -
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 86ce4f2..b5361f7 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -150,6 +150,7 @@ typedef struct BDRVRawState {
bool has_discard:1;
bool has_write_zeroes:1;
bool discard_zeroes:1;
+bool needs_alignement:1;
#ifdef CONFIG_FIEMAP
bool skip_fiemap;
#endif
@@ -230,7 +231,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int
fd, Error **errp)
/* For /dev/sg devices the alignment is not really used.
With buffered I/O, we don't have any restrictions. */
-if (bs->sg || !(s->open_flags & O_DIRECT)) {
+if (bs->sg || !s->needs_alignement) {
bs->request_alignment = 1;
s->buf_align = 1;
return;
@@ -446,6 +447,8 @@ static int raw_open_common(BlockDriverState *bs, QDict
*options,
s->has_discard = true;
s->has_write_zeroes = true;
+if ((bs->open_flags & BDRV_O_NOCACHE) != 0)
+s->needs_alignement = true;
if (fstat(s->fd, &st) < 0) {
error_setg_errno(errp, errno, "Could not stat file");
@@ -472,6 +475,17 @@ static int raw_open_common(BlockDriverState *bs, QDict
*options,
}
#endif
}
+#ifdef __FreeBSD__
+if (S_ISCHR(st.st_mode)) {
+/*
+ * The file is a char device (disk), which on FreeBSD isn't behind
+ * a pager, so force all requests to be aligned. This is needed
+ * so Qemu makes sure all IO operations on the device are aligned
+ * to sector size, or else FreeBSD will reject them with EINVAL.
+ */
+s->needs_alignement = true;
+}
+#endif
#ifdef CONFIG_XFS
if (platform_test_xfs_fd(s->fd)) {
@@ -1076,11 +1090,12 @@ static BlockDriverAIOCB
*raw_aio_submit(BlockDriverState *bs,
return NULL;
/*
- * If O_DIRECT is used the buffer needs to be aligned on a sector
- * boundary. Check if this is the case or tell the low-level
- * driver that it needs to copy the buffer.
+ * Check if the underlying device requires requests to be aligned,
+ * and if the request we are trying to submit is aligned or not.
+ * If this is the case tell the low-level driver that it needs
+ * to copy the buffer.
*/
-if ((bs->open_flags & BDRV_O_NOCACHE)) {
+if (s->needs_alignement) {
if (!bdrv_qiov_is_aligned(bs, qiov)) {
type |= QEMU_AIO_MISALIGNED;
#ifdef CONFIG_LINUX_AIO
--
1.9.3 (Apple Git-50)
[Qemu-devel] [PATCH v3] block: char devices on FreeBSD are not behind a pager
Introduce a new flag to mark devices that require requests to be aligned and
replace the usage of BDRV_O_NOCACHE and O_DIRECT with this flag when
appropriate.
If a character device is used as a backend on a FreeBSD host set this flag
unconditionally.
Signed-off-by: Roger Pau Monné
Reviewed-by: Max Reitz
Cc: Kevin Wolf
Cc: Stefan Hajnoczi
---
Changes since v2:
- s/alignement/alignment/.
- Don't make needs_alignment a bit field.
- Add braces to single statement conditional block.
Changes since v1:
- Intead of appending BDRV_O_NOCACHE and O_DIRECT when a char dev is used
on FreeBSD introduce a new flag that is used to mark if a device needs
requests to be aligned.
---
block/raw-posix.c | 26 +-
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 86ce4f2..afa4b01 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -150,6 +150,7 @@ typedef struct BDRVRawState {
bool has_discard:1;
bool has_write_zeroes:1;
bool discard_zeroes:1;
+bool needs_alignment;
#ifdef CONFIG_FIEMAP
bool skip_fiemap;
#endif
@@ -230,7 +231,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int
fd, Error **errp)
/* For /dev/sg devices the alignment is not really used.
With buffered I/O, we don't have any restrictions. */
-if (bs->sg || !(s->open_flags & O_DIRECT)) {
+if (bs->sg || !s->needs_alignment) {
bs->request_alignment = 1;
s->buf_align = 1;
return;
@@ -446,6 +447,9 @@ static int raw_open_common(BlockDriverState *bs, QDict
*options,
s->has_discard = true;
s->has_write_zeroes = true;
+if ((bs->open_flags & BDRV_O_NOCACHE) != 0) {
+s->needs_alignment = true;
+}
if (fstat(s->fd, &st) < 0) {
error_setg_errno(errp, errno, "Could not stat file");
@@ -472,6 +476,17 @@ static int raw_open_common(BlockDriverState *bs, QDict
*options,
}
#endif
}
+#ifdef __FreeBSD__
+if (S_ISCHR(st.st_mode)) {
+/*
+ * The file is a char device (disk), which on FreeBSD isn't behind
+ * a pager, so force all requests to be aligned. This is needed
+ * so QEMU makes sure all IO operations on the device are aligned
+ * to sector size, or else FreeBSD will reject them with EINVAL.
+ */
+s->needs_alignment = true;
+}
+#endif
#ifdef CONFIG_XFS
if (platform_test_xfs_fd(s->fd)) {
@@ -1076,11 +1091,12 @@ static BlockDriverAIOCB
*raw_aio_submit(BlockDriverState *bs,
return NULL;
/*
- * If O_DIRECT is used the buffer needs to be aligned on a sector
- * boundary. Check if this is the case or tell the low-level
- * driver that it needs to copy the buffer.
+ * Check if the underlying device requires requests to be aligned,
+ * and if the request we are trying to submit is aligned or not.
+ * If this is the case tell the low-level driver that it needs
+ * to copy the buffer.
*/
-if ((bs->open_flags & BDRV_O_NOCACHE)) {
+if (s->needs_alignment) {
if (!bdrv_qiov_is_aligned(bs, qiov)) {
type |= QEMU_AIO_MISALIGNED;
#ifdef CONFIG_LINUX_AIO
--
1.9.3 (Apple Git-50)
[Qemu-devel] [PATCH 1/2] build: check if libm is needed in configure
Remove the hardcoded use of libm and instead rely on configure to
check for it. It is needed at least for qemu-ga and qemu-system.
Signed-off-by: Roger Pau Monne
---
Makefile.target |4
configure | 14 ++
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/Makefile.target b/Makefile.target
index a111521..1bfd419 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -33,10 +33,6 @@ endif
PROGS=$(QEMU_PROG)
STPFILES=
-ifndef CONFIG_HAIKU
-LIBS+=-lm
-endif
-
config-target.h: config-target.h-timestamp
config-target.h-timestamp: config-target.mak
diff --git a/configure b/configure
index b113f60..7bcd547 100755
--- a/configure
+++ b/configure
@@ -2447,6 +2447,20 @@ elif compile_prog "" "-lrt" ; then
LIBS="-lrt $LIBS"
fi
+##
+# Do we need libm
+cat > $TMPC <
+int main(void) { double a, b; return modf(a, &b);}
+EOF
+
+if compile_prog "" "" ; then
+ :
+elif compile_prog "" "-lm" ; then
+ LIBS="-lm $LIBS"
+ libs_qga="-lm $libs_qga"
+fi
+
if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \
"$aix" != "yes" -a "$haiku" != "yes" ; then
libs_softmmu="-lutil $libs_softmmu"
--
1.7.9
[Qemu-devel] [PATCH 2/2] build: replace librt check function
Replace clock_gettime with timer_gettime, since at least under
uclibc 0.9.33 the clock_getttime function can be used without linking
against librt (although the manual page states the opposite).
Signed-off-by: Roger Pau Monne
---
configure |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/configure b/configure
index 7bcd547..fb99632 100755
--- a/configure
+++ b/configure
@@ -2438,7 +2438,8 @@ fi
cat > $TMPC <
#include
-int main(void) { clockid_t id; return clock_gettime(id, NULL); }
+int main(void) { timer_t tid; struct itimerspec it; \
+ return timer_gettime(tid, &it); }
EOF
if compile_prog "" "" ; then
--
1.7.9
[Qemu-devel] [PATCH 1/3] build: replace librt check function
Replace clock_gettime with timer_gettime, since at least under
uclibc 0.9.33 the clock_getttime function can be used without linking
against librt (although the manual page states the opposite).
Signed-off-by: Roger Pau Monne
---
configure |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/configure b/configure
index f9d5330..68eb3fa 100755
--- a/configure
+++ b/configure
@@ -2513,7 +2513,8 @@ fi
cat > $TMPC <
#include
-int main(void) { return clock_gettime(CLOCK_REALTIME, NULL); }
+int main(void) { timer_t tid; struct itimerspec it; \
+ return timer_gettime(tid, &it); }
EOF
if compile_prog "" "" ; then
--
1.7.9
[Qemu-devel] [PATCH 3/3] build: check if libm is needed in configure
Remove the hardcoded use of libm and instead rely on configure to
check for it. It is needed at least for qemu-ga and qemu-system.
Signed-off-by: Roger Pau Monne
---
Makefile.target |4
configure | 14 ++
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/Makefile.target b/Makefile.target
index 68a5641..c230aff 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -42,10 +42,6 @@ PROGS+=$(QEMU_PROGW)
endif
STPFILES=
-ifndef CONFIG_HAIKU
-LIBS+=-lm
-endif
-
config-target.h: config-target.h-timestamp
config-target.h-timestamp: config-target.mak
diff --git a/configure b/configure
index 790d495..b0cb175 100755
--- a/configure
+++ b/configure
@@ -2524,6 +2524,20 @@ elif compile_prog "" "-lrt" ; then
libs_qga="-lrt $libs_qga"
fi
+##
+# Do we need libm
+cat > $TMPC <
+int main(void) { double a, b; return modf(a, &b);}
+EOF
+
+if compile_prog "" "" ; then
+ :
+elif compile_prog "" "-lm" ; then
+ LIBS="-lm $LIBS"
+ libs_qga="-lm $libs_qga"
+fi
+
if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \
"$aix" != "yes" -a "$haiku" != "yes" ; then
libs_softmmu="-lutil $libs_softmmu"
--
1.7.9
[Qemu-devel] [PATCH 2/3] build: add librt to libs_qga
librt is needed to link qemu-ga. Signed-off-by: Roger Pau Monne --- configure |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/configure b/configure index 68eb3fa..790d495 100755 --- a/configure +++ b/configure @@ -2521,6 +2521,7 @@ if compile_prog "" "" ; then : elif compile_prog "" "-lrt" ; then LIBS="-lrt $LIBS" + libs_qga="-lrt $libs_qga" fi if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \ -- 1.7.9
[Qemu-devel] [PATCH 1/3] build: replace librt check function
Replace clock_gettime with timer_gettime, since at least under
uclibc 0.9.33 the clock_getttime function can be used without linking
against librt (although the manual page states the opposite).
Signed-off-by: Roger Pau Monne
---
configure |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/configure b/configure
index f9d5330..68eb3fa 100755
--- a/configure
+++ b/configure
@@ -2513,7 +2513,8 @@ fi
cat > $TMPC <
#include
-int main(void) { return clock_gettime(CLOCK_REALTIME, NULL); }
+int main(void) { timer_t tid; struct itimerspec it; \
+ return timer_gettime(tid, &it); }
EOF
if compile_prog "" "" ; then
--
1.7.9
[Qemu-devel] [PATCH 2/3] build: add librt to libs_qga
librt is needed to link qemu-ga. Signed-off-by: Roger Pau Monne --- configure |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/configure b/configure index 68eb3fa..790d495 100755 --- a/configure +++ b/configure @@ -2521,6 +2521,7 @@ if compile_prog "" "" ; then : elif compile_prog "" "-lrt" ; then LIBS="-lrt $LIBS" + libs_qga="-lrt $libs_qga" fi if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \ -- 1.7.9
[Qemu-devel] [PATCH 3/3] build: check if libm is needed in configure
Remove the hardcoded use of libm and instead rely on configure to
check for it. It is needed at least for qemu-ga and qemu-system.
Signed-off-by: Roger Pau Monne
---
Makefile.target |4
configure | 14 ++
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/Makefile.target b/Makefile.target
index 68a5641..c230aff 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -42,10 +42,6 @@ PROGS+=$(QEMU_PROGW)
endif
STPFILES=
-ifndef CONFIG_HAIKU
-LIBS+=-lm
-endif
-
config-target.h: config-target.h-timestamp
config-target.h-timestamp: config-target.mak
diff --git a/configure b/configure
index 790d495..b0cb175 100755
--- a/configure
+++ b/configure
@@ -2524,6 +2524,20 @@ elif compile_prog "" "-lrt" ; then
libs_qga="-lrt $libs_qga"
fi
+##
+# Do we need libm
+cat > $TMPC <
+int main(void) { double a, b; return modf(a, &b);}
+EOF
+
+if compile_prog "" "" ; then
+ :
+elif compile_prog "" "-lm" ; then
+ LIBS="-lm $LIBS"
+ libs_qga="-lm $libs_qga"
+fi
+
if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \
"$aix" != "yes" -a "$haiku" != "yes" ; then
libs_softmmu="-lutil $libs_softmmu"
--
1.7.9
[Qemu-devel] [PATCH 2/2] tap-bsd: implement a FreeBSD only version of tap_open
The current behaviour of tap_open for BSD systems differ greatly from
it's Linux counterpart. Since FreeBSD supports interface renaming and
tap device cloning by opening /dev/tap, implement a FreeBSD specific
version of tap_open that behaves like it's Linux counterpart.
This is specially important for toolstacks that use Qemu (like Xen
libxl), in order to have a unified behaviour across suported
platforms.
Signed-off-by: Roger Pau Monné
Cc: xen-de...@lists.xenproject.org
Cc: Stefano Stabellini
Cc: Anthony Liguori
Cc: Stefan Hajnoczi
---
net/tap-bsd.c | 70 -
1 files changed, 69 insertions(+), 1 deletions(-)
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index 90f8a02..bf91bd0 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -27,12 +27,13 @@
#include "sysemu/sysemu.h"
#include "qemu/error-report.h"
-#ifdef __NetBSD__
+#if defined(__NetBSD__) || defined(__FreeBSD__)
#include
#include
#include
#endif
+#ifndef __FreeBSD__
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
int vnet_hdr_required, int mq_required)
{
@@ -108,6 +109,73 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
return fd;
}
+#else /* __FreeBSD__ */
+
+#define PATH_NET_TAP "/dev/tap"
+
+int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
+ int vnet_hdr_required, int mq_required)
+{
+int fd, s, ret;
+struct ifreq ifr;
+
+TFR(fd = open(PATH_NET_TAP, O_RDWR));
+if (fd < 0) {
+error_report("could not open %s: %s", PATH_NET_TAP, strerror(errno));
+return -1;
+}
+
+memset(&ifr, 0, sizeof(ifr));
+
+ret = ioctl(fd, TAPGIFNAME, (void *)&ifr);
+if (ret < 0) {
+error_report("could not get tap interface name");
+goto error;
+}
+
+if (ifname[0] != '\0') {
+/* User requested the interface to have a specific name */
+s = socket(AF_LOCAL, SOCK_DGRAM, 0);
+if (s < 0) {
+error_report("could not open socket to set interface name");
+goto error;
+}
+ifr.ifr_data = ifname;
+ret = ioctl(s, SIOCSIFNAME, (void *)&ifr);
+close(s);
+if (ret < 0) {
+error_report("could not set tap interface name");
+goto error;
+}
+} else {
+pstrcpy(ifname, ifname_size, ifr.ifr_name);
+}
+
+if (*vnet_hdr) {
+/* BSD doesn't have IFF_VNET_HDR */
+*vnet_hdr = 0;
+
+if (vnet_hdr_required && !*vnet_hdr) {
+error_report("vnet_hdr=1 requested, but no kernel "
+ "support for IFF_VNET_HDR available");
+goto error;
+}
+}
+if (mq_required) {
+error_report("mq_required requested, but not kernel support"
+ "for IFF_MULTI_QUEUE available");
+goto error;
+}
+
+fcntl(fd, F_SETFL, O_NONBLOCK);
+return fd;
+
+error:
+close(fd);
+return -1;
+}
+#endif /* __FreeBSD__ */
+
int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
{
return 0;
--
1.7.7.5 (Apple Git-26)
[Qemu-devel] [PATCH 0/2] qemu-freebsd: fixes for running Xen guests
This two patches allow FreeBSD Xen Dom0 to use Qemu (i.e., launch HVM guests). First patch fixes the usage of ENODATA, which doesn't exist on FreeBSD and is replaced with ENOENT instead. The second patch is more controversial probably, since it introduces a FreeBSD specific version of tap_open which behaves like it's Linux counterpart, allowing Qemu to create tap interfaces and rename them. I've decided to just fork the function instead of adding a bunch more of preprocessor code in the original function. Thanks for the review, Roger.
[Qemu-devel] [PATCH 1/2] xen: fix usage of ENODATA
ENODATA doesn't exist on FreeBSD, so ENODATA errors returned by the
hypervisor are translated to ENOENT.
Also, the error code is returned in errno if the call returns -1, so
compare the error code with the value in errno instead of the value
returned by the function.
Signed-off-by: Roger Pau Monné
Cc: xen-de...@lists.xenproject.org
Cc: Stefano Stabellini
Cc: Anthony Perard
---
xen-all.c |8 ++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/xen-all.c b/xen-all.c
index 0b4d934..4026b7b 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -499,11 +499,15 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
start_addr >> TARGET_PAGE_BITS, npages,
bitmap);
if (rc < 0) {
-if (rc != -ENODATA) {
+#ifdef ENODATA
+if (errno == ENODATA) {
+#else
+if (errno == ENOENT) {
+#endif
memory_region_set_dirty(framebuffer, 0, size);
DPRINTF("xen: track_dirty_vram failed (0x" TARGET_FMT_plx
", 0x" TARGET_FMT_plx "): %s\n",
-start_addr, start_addr + size, strerror(-rc));
+start_addr, start_addr + size, strerror(errno));
}
return;
}
--
1.7.7.5 (Apple Git-26)
[Qemu-devel] [PATCH] serial: poll the serial console with G_IO_HUP
On FreeBSD polling a master pty while the other end is not connected
with G_IO_OUT only results in an endless wait. This is different from
the Linux behaviour, that returns immediately. In order to demonstrate
this, I have the following example code:
http://xenbits.xen.org/people/royger/test_poll.c
When executed on Linux:
$ ./test_poll
In callback
On FreeBSD instead, the callback never gets called:
$ ./test_poll
So, in order to workaround this, poll the source with G_IO_HUP (which
makes the code behave the same way on both Linux and FreeBSD).
Signed-off-by: Roger Pau Monné
Cc: Peter Crosthwaite
Cc: Michael Tokarev
Cc: "Andreas Färber"
Cc: Paolo Bonzini
Cc: xen-de...@lists.xenproject.org
---
hw/char/serial.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 6025592..ab9c40f 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -243,7 +243,7 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition
cond, void *opaque)
serial_receive1(s, &s->tsr, 1);
} else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) {
if (s->tsr_retry >= 0 && s->tsr_retry < MAX_XMIT_RETRY &&
-qemu_chr_fe_add_watch(s->chr, G_IO_OUT, serial_xmit, s) > 0) {
+qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, serial_xmit, s) >
0) {
s->tsr_retry++;
return FALSE;
}
--
1.7.7.5 (Apple Git-26)
[Qemu-devel] [PATCH] xen_disk: fix unmapping of persistent grants
This patch fixes two issues with persistent grants and the disk PV backend
(Qdisk):
- Don't use batch mappings when using persistent grants, doing so prevents
unmapping single grants (the whole area has to be unmapped at once).
- Unmap persistent grants before switching to the closed state, so the
frontend can also free them.
Signed-off-by: Roger Pau Monné
Reported-and-Tested-by: George Dunlap
Cc: Stefano Stabellini
Cc: Kevin Wolf
Cc: Stefan Hajnoczi
Cc: George Dunlap
---
hw/block/xen_disk.c | 35 ---
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 231e9a7..1300c0a 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -43,8 +43,6 @@
/* - */
-static int batch_maps = 0;
-
static int max_requests = 32;
/* - */
@@ -105,6 +103,7 @@ struct XenBlkDev {
blkif_back_rings_t rings;
int more_work;
int cnt_map;
+boolbatch_maps;
/* request lists */
QLIST_HEAD(inflight_head, ioreq) inflight;
@@ -309,7 +308,7 @@ static void ioreq_unmap(struct ioreq *ioreq)
if (ioreq->num_unmap == 0 || ioreq->mapped == 0) {
return;
}
-if (batch_maps) {
+if (ioreq->blkdev->batch_maps) {
if (!ioreq->pages) {
return;
}
@@ -386,7 +385,7 @@ static int ioreq_map(struct ioreq *ioreq)
new_maps = ioreq->v.niov;
}
-if (batch_maps && new_maps) {
+if (ioreq->blkdev->batch_maps && new_maps) {
ioreq->pages = xc_gnttab_map_grant_refs
(gnt, new_maps, domids, refs, ioreq->prot);
if (ioreq->pages == NULL) {
@@ -433,7 +432,7 @@ static int ioreq_map(struct ioreq *ioreq)
*/
grant = g_malloc0(sizeof(*grant));
new_maps--;
-if (batch_maps) {
+if (ioreq->blkdev->batch_maps) {
grant->page = ioreq->pages + (new_maps) * XC_PAGE_SIZE;
} else {
grant->page = ioreq->page[new_maps];
@@ -718,7 +717,9 @@ static void blk_alloc(struct XenDevice *xendev)
QLIST_INIT(&blkdev->freelist);
blkdev->bh = qemu_bh_new(blk_bh, blkdev);
if (xen_mode != XEN_EMULATE) {
-batch_maps = 1;
+blkdev->batch_maps = TRUE;
+} else {
+blkdev->batch_maps = FALSE;
}
if (xc_gnttab_set_max_grants(xendev->gnttabdev,
MAX_GRANTS(max_requests, BLKIF_MAX_SEGMENTS_PER_REQUEST)) < 0) {
@@ -923,6 +924,13 @@ static int blk_connect(struct XenDevice *xendev)
} else {
blkdev->feature_persistent = !!pers;
}
+if (blkdev->feature_persistent) {
+/*
+ * Disable batch maps, since that would prevent unmapping
+ * single persistent grants.
+ */
+blkdev->batch_maps = FALSE;
+}
blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
if (blkdev->xendev.protocol) {
@@ -1000,6 +1008,16 @@ static void blk_disconnect(struct XenDevice *xendev)
blkdev->cnt_map--;
blkdev->sring = NULL;
}
+
+/*
+ * Unmap persistent grants before switching to the closed state
+ * so the frontend can free them.
+ */
+if (blkdev->feature_persistent) {
+g_tree_destroy(blkdev->persistent_gnts);
+assert(blkdev->persistent_gnt_count == 0);
+blkdev->feature_persistent = FALSE;
+}
}
static int blk_free(struct XenDevice *xendev)
@@ -1011,11 +1029,6 @@ static int blk_free(struct XenDevice *xendev)
blk_disconnect(xendev);
}
-/* Free persistent grants */
-if (blkdev->feature_persistent) {
-g_tree_destroy(blkdev->persistent_gnts);
-}
-
while (!QLIST_EMPTY(&blkdev->freelist)) {
ioreq = QLIST_FIRST(&blkdev->freelist);
QLIST_REMOVE(ioreq, list);
--
1.9.3 (Apple Git-50)
[Qemu-devel] [PATCH v2 for-xen-4.5] xen_disk: fix unmapping of persistent grants
This patch fixes two issues with persistent grants and the disk PV backend
(Qdisk):
- Keep track of memory regions where persistent grants have been mapped
since we need to unmap them as a whole. It is not possible to unmap a
single grant if it has been batch-mapped.
- Unmap persistent grants before switching to the closed state, so the
frontend can also free them.
Signed-off-by: Roger Pau Monné
Reported-by: George Dunlap
Cc: Stefano Stabellini
Cc: Kevin Wolf
Cc: Stefan Hajnoczi
Cc: George Dunlap
Cc: Konrad Rzeszutek Wilk
---
Xen release exception: this is obviously an important bug-fix that should be
included in 4.5. Without this fix, trying to do a block-detach of a Qdisk
from a running guest can lead to guest crash depending on the blkfront
implementation.
---
Changes since v1:
- Instead of disabling batch mappings when using persistent grants, keep
track of the persistently mapped regions and unmap them on disconnection.
This prevents unmapping single persistent grants, but the current
implementation doesn't require it.
This new version is slightly faster than the previous one, the following
test results are in IOPS from 20 runs of a block-attach, fio run,
block-detach test loop:
x batch
+ no_batch
+--+
| + + xx + + + x xx x |
|+ + x x+ *+++ * +x* +++x* x xx x *|
| |_A_M__||
||A_M___| |
+--+
N Min MaxMedian AvgStddev
x 20 52941 63822 62396 61180.15 2673.6497
+ 20 49967 63849 60322 59116.9 3456.3455
Difference at 95.0% confidence
-2063.25 +/- 1977.66
-3.37242% +/- 3.23252%
(Student's t, pooled s = 3089.88)
---
hw/block/xen_disk.c | 77 -
1 file changed, 64 insertions(+), 13 deletions(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 231e9a7..75bdc16 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -59,6 +59,13 @@ struct PersistentGrant {
typedef struct PersistentGrant PersistentGrant;
+struct PersistentRegion {
+void *addr;
+int num;
+};
+
+typedef struct PersistentRegion PersistentRegion;
+
struct ioreq {
blkif_request_t req;
int16_t status;
@@ -118,6 +125,7 @@ struct XenBlkDev {
gbooleanfeature_discard;
gbooleanfeature_persistent;
GTree *persistent_gnts;
+GSList *persistent_regions;
unsigned intpersistent_gnt_count;
unsigned intmax_grants;
@@ -164,19 +172,41 @@ static gint int_cmp(gconstpointer a, gconstpointer b,
gpointer user_data)
static void destroy_grant(gpointer pgnt)
{
PersistentGrant *grant = pgnt;
-XenGnttab gnt = grant->blkdev->xendev.gnttabdev;
-if (xc_gnttab_munmap(gnt, grant->page, 1) != 0) {
-xen_be_printf(&grant->blkdev->xendev, 0,
- "xc_gnttab_munmap failed: %s\n",
- strerror(errno));
-}
grant->blkdev->persistent_gnt_count--;
xen_be_printf(&grant->blkdev->xendev, 3,
- "unmapped grant %p\n", grant->page);
+ "removed grant %p\n", grant->page);
g_free(grant);
}
+static void add_persistent_region(struct XenBlkDev *blkdev, void *addr, int
num)
+{
+PersistentRegion *region;
+
+region = g_malloc0(sizeof(*region));
+region->addr = addr;
+region->num = num;
+blkdev->persistent_regions = g_slist_append(blkdev->persistent_regions,
+region);
+}
+
+static void remove_persistent_region(gpointer data, gpointer dev)
+{
+PersistentRegion *region = data;
+struct XenBlkDev *blkdev = dev;
+XenGnttab gnt = blkdev->xendev.gnttabdev;
+
+if (xc_gnttab_munmap(gnt, region->addr, region->num) != 0) {
+xen_be_printf(&blkdev->xendev, 0,
+ "xc_gnttab_munmap region %p failed: %s\n",
+ region->addr, strerror(errno));
+}
+xen_be_printf(&blkdev->xendev, 3,
+ "unmapped grant region %p with %d pages\n",
+ region->addr, region->num);
+g_free(region);
+}
+
static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
{
struct ioreq *ioreq = NULL;
@@ -421,7 +451,17 @@ static int ioreq_map(struct ioreq *ioreq)
}
}
}
-if (ioreq->blkdev->feature_persistent) {
+if (ioreq->blkdev->feature_persistent && new_maps &&
+(!batch_maps || (ioreq->blkdev->persistent_gnt_count + new_maps <=
+ioreq->blkdev->max_grants))) {
+/*
+ * If we are using p
[Qemu-devel] [PATCH v3 for-xen-4.5] xen_disk: fix unmapping of persistent grants
This patch fixes two issues with persistent grants and the disk PV backend
(Qdisk):
- Keep track of memory regions where persistent grants have been mapped
since we need to unmap them as a whole. It is not possible to unmap a
single grant if it has been batch-mapped. A new check has also been added
to make sure persistent grants are only used if the whole mapped region
can be persistently mapped in the batch_maps case.
- Unmap persistent grants before switching to the closed state, so the
frontend can also free them.
Signed-off-by: Roger Pau Monné
Reported-by: George Dunlap
Cc: Stefano Stabellini
Cc: Kevin Wolf
Cc: Stefan Hajnoczi
Cc: George Dunlap
Cc: Konrad Rzeszutek Wilk
---
Xen release exception: this is obviously an important bug-fix that should be
included in 4.5. Without this fix, trying to do a block-detach of a Qdisk
from a running guest can lead to guest crash depending on the blkfront
implementation.
---
Changea since v2:
- Expand the commit message to mention the newly added check.
- Don't use persistent_regions in the !batch_maps case, we can use the
previous implementation which works fine in that case.
Changes since v1:
- Instead of disabling batch mappings when using persistent grants, keep
track of the persistently mapped regions and unmap them on disconnection.
This prevents unmapping single persistent grants, but the current
implementation doesn't require it.
This new version is slightly faster than the previous one, the following
test results are in IOPS from 20 runs of a block-attach, fio run,
block-detach test loop:
x batch
+ no_batch
+--+
| + + xx + + + x xx x |
|+ + x x+ *+++ * +x* +++x* x xx x *|
| |_A_M__||
||A_M___| |
+--+
N Min MaxMedian AvgStddev
x 20 52941 63822 62396 61180.15 2673.6497
+ 20 49967 63849 60322 59116.9 3456.3455
Difference at 95.0% confidence
-2063.25 +/- 1977.66
-3.37242% +/- 3.23252%
(Student's t, pooled s = 3089.88)
---
hw/block/xen_disk.c | 72 -
1 file changed, 66 insertions(+), 6 deletions(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 231e9a7..21842a0 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -59,6 +59,13 @@ struct PersistentGrant {
typedef struct PersistentGrant PersistentGrant;
+struct PersistentRegion {
+void *addr;
+int num;
+};
+
+typedef struct PersistentRegion PersistentRegion;
+
struct ioreq {
blkif_request_t req;
int16_t status;
@@ -118,6 +125,7 @@ struct XenBlkDev {
gbooleanfeature_discard;
gbooleanfeature_persistent;
GTree *persistent_gnts;
+GSList *persistent_regions;
unsigned intpersistent_gnt_count;
unsigned intmax_grants;
@@ -177,6 +185,23 @@ static void destroy_grant(gpointer pgnt)
g_free(grant);
}
+static void remove_persistent_region(gpointer data, gpointer dev)
+{
+PersistentRegion *region = data;
+struct XenBlkDev *blkdev = dev;
+XenGnttab gnt = blkdev->xendev.gnttabdev;
+
+if (xc_gnttab_munmap(gnt, region->addr, region->num) != 0) {
+xen_be_printf(&blkdev->xendev, 0,
+ "xc_gnttab_munmap region %p failed: %s\n",
+ region->addr, strerror(errno));
+}
+xen_be_printf(&blkdev->xendev, 3,
+ "unmapped grant region %p with %d pages\n",
+ region->addr, region->num);
+g_free(region);
+}
+
static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
{
struct ioreq *ioreq = NULL;
@@ -343,6 +368,7 @@ static int ioreq_map(struct ioreq *ioreq)
void *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
int i, j, new_maps = 0;
PersistentGrant *grant;
+PersistentRegion *region;
/* domids and refs variables will contain the information necessary
* to map the grants that are needed to fulfill this request.
*
@@ -421,7 +447,22 @@ static int ioreq_map(struct ioreq *ioreq)
}
}
}
-if (ioreq->blkdev->feature_persistent) {
+if (ioreq->blkdev->feature_persistent && new_maps != 0 &&
+(!batch_maps || (ioreq->blkdev->persistent_gnt_count + new_maps <=
+ioreq->blkdev->max_grants))) {
+/*
+ * If we are using persistent grants and batch mappings only
+ * add the new maps to the list of persistent grants if the whole
+ * area can be persistently mapped.
+ */
+if (batch_maps) {
+
[Qemu-devel] [PATCH QEMU 2/2] xen: fix usage of xc_domain_create in domain builder
Due to the addition of HVMlite and the requirement to always provide a valid
xc_domain_configuration_t, xc_domain_create now always takes an arch domain
config, which can be NULL in order to mimic previous behaviour.
Signed-off-by: Roger Pau Monné
---
Cc: Stefano Stabellini
Cc: qemu-devel@nongnu.org
---
hw/xenpv/xen_domainbuild.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
index c0ab753..a737908 100644
--- a/hw/xenpv/xen_domainbuild.c
+++ b/hw/xenpv/xen_domainbuild.c
@@ -234,7 +234,7 @@ int xen_domain_build_pv(const char *kernel, const char
*ramdisk,
int rc;
memcpy(uuid, qemu_uuid, sizeof(uuid));
-rc = xc_domain_create(xen_xc, ssidref, uuid, flags, &xen_domid);
+rc = xc_domain_create(xen_xc, ssidref, uuid, flags, &xen_domid, NULL);
if (rc < 0) {
fprintf(stderr, "xen: xc_domain_create() failed\n");
goto err;
--
1.9.5 (Apple Git-50.3)
[Qemu-devel] [PATCH QEMU v2 2/2] xen: fix usage of xc_domain_create in domain builder
Due to the addition of HVMlite and the requirement to always provide a valid
xc_domain_configuration_t, xc_domain_create now always takes an arch domain
config, which can be NULL in order to mimic previous behaviour.
Add a small stub called xen_domain_create that encapsulates the correct call
to xc_domain_create depending on the libxc version detected.
Signed-off-by: Roger Pau Monné
---
Cc: Stefano Stabellini
Cc: qemu-devel@nongnu.org
---
Changes since v1:
- Add a compat layer to support previous libxc versions.
- Add machinery to detect current Xen unstable (4.7).
---
configure | 17 +
hw/xenpv/xen_domainbuild.c | 2 +-
include/hw/xen/xen_common.h | 12
3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 779623a..001982f 100755
--- a/configure
+++ b/configure
@@ -1863,6 +1863,23 @@ EOF
elif
cat > $TMPC <
+#include
+int main(void) {
+ xc_interface *xc = NULL;
+ xen_domain_handle_t handle;
+ xc_domain_create(xc, 0, handle, 0, NULL, NULL);
+ return 0;
+}
+EOF
+ compile_prog "" "$xen_libs"
+then
+xen_ctrl_version=470
+xen=yes
+
+ # Xen 4.6
+ elif
+ cat > $TMPC <
#include
#include
#include
diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
index c0ab753..07814f6 100644
--- a/hw/xenpv/xen_domainbuild.c
+++ b/hw/xenpv/xen_domainbuild.c
@@ -234,7 +234,7 @@ int xen_domain_build_pv(const char *kernel, const char
*ramdisk,
int rc;
memcpy(uuid, qemu_uuid, sizeof(uuid));
-rc = xc_domain_create(xen_xc, ssidref, uuid, flags, &xen_domid);
+rc = xen_domain_create(xen_xc, ssidref, uuid, flags, &xen_domid, NULL);
if (rc < 0) {
fprintf(stderr, "xen: xc_domain_create() failed\n");
goto err;
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 5923290..e5ba732 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -417,4 +417,16 @@ static inline int xen_set_ioreq_server_state(XenXC xc,
domid_t dom,
#endif
+static inline int xen_domain_create(XenXC xc, uint32_t ssidref,
+xen_domain_handle_t handle, uint32_t flags,
+uint32_t *pdomid,
+xc_domain_configuration_t *config)
+{
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 470
+return xc_domain_create(xc, ssidref, handle, flags, pdomid);
+#else
+return xc_domain_create(xc, ssidref, handle, flags, pdomid, config);
+#endif
+}
+
#endif /* QEMU_HW_XEN_COMMON_H */
--
1.9.5 (Apple Git-50.3)
[Qemu-devel] [PATCH QEMU v3 2/2] xen: fix usage of xc_domain_create in domain builder
Due to the addition of HVMlite and the requirement to always provide a valid
xc_domain_configuration_t, xc_domain_create now always takes an arch domain
config, which can be NULL in order to mimic previous behaviour.
Add a small stub called xen_domain_create that encapsulates the correct call
to xc_domain_create depending on the libxc version detected.
Signed-off-by: Roger Pau Monné
---
Cc: Stefano Stabellini
Cc: qemu-devel@nongnu.org
---
Changes since v2:
- Drop the last parameter to xen_create_domain since it's always NULL ATM
and was causing build problems with older Xen versions.
- Create two different inline functions instead of putting ifdefs inside of
a single one.
Changes since v1:
- Add a compat layer to support previous libxc versions.
- Add machinery to detect current Xen unstable (4.7).
---
configure | 17 +
hw/xenpv/xen_domainbuild.c | 2 +-
include/hw/xen/xen_common.h | 16
3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 779623a..001982f 100755
--- a/configure
+++ b/configure
@@ -1863,6 +1863,23 @@ EOF
elif
cat > $TMPC <
+#include
+int main(void) {
+ xc_interface *xc = NULL;
+ xen_domain_handle_t handle;
+ xc_domain_create(xc, 0, handle, 0, NULL, NULL);
+ return 0;
+}
+EOF
+ compile_prog "" "$xen_libs"
+then
+xen_ctrl_version=470
+xen=yes
+
+ # Xen 4.6
+ elif
+ cat > $TMPC <
#include
#include
#include
diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
index c0ab753..ac0e5ac 100644
--- a/hw/xenpv/xen_domainbuild.c
+++ b/hw/xenpv/xen_domainbuild.c
@@ -234,7 +234,7 @@ int xen_domain_build_pv(const char *kernel, const char
*ramdisk,
int rc;
memcpy(uuid, qemu_uuid, sizeof(uuid));
-rc = xc_domain_create(xen_xc, ssidref, uuid, flags, &xen_domid);
+rc = xen_domain_create(xen_xc, ssidref, uuid, flags, &xen_domid);
if (rc < 0) {
fprintf(stderr, "xen: xc_domain_create() failed\n");
goto err;
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 5923290..04bf6da 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -417,4 +417,20 @@ static inline int xen_set_ioreq_server_state(XenXC xc,
domid_t dom,
#endif
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 470
+static inline int xen_domain_create(XenXC xc, uint32_t ssidref,
+xen_domain_handle_t handle, uint32_t flags,
+uint32_t *pdomid)
+{
+return xc_domain_create(xc, ssidref, handle, flags, pdomid);
+}
+#else
+static inline int xen_domain_create(XenXC xc, uint32_t ssidref,
+xen_domain_handle_t handle, uint32_t flags,
+uint32_t *pdomid)
+{
+return xc_domain_create(xc, ssidref, handle, flags, pdomid, NULL);
+}
+#endif
+
#endif /* QEMU_HW_XEN_COMMON_H */
--
1.9.5 (Apple Git-50.3)
[Qemu-devel] [PATCH v2] xen_disk: add persistent grant support to xen_disk backend
This protocol extension reuses the same set of grant pages for all
transactions between the front/back drivers, avoiding expensive tlb
flushes, grant table lock contention and switches between userspace
and kernel space. The full description of the protocol can be found in
the public blkif.h header.
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob_plain;f=xen/include/public/io/blkif.h
Speed improvement with 15 guests performing I/O is ~450%.
Signed-off-by: Roger Pau Monné
Cc: xen-de...@lists.xen.org
Cc: Stefano Stabellini
Cc: Anthony PERARD
---
Performance comparison with the previous implementation can be seen in
the followign graph:
http://xenbits.xen.org/people/royger/persistent_read_qemu.png
---
Changes since v1:
* Fix coding style issues.
* Add more comments regarding how ioreq_map works.
---
hw/xen_disk.c | 172 +++--
1 files changed, 155 insertions(+), 17 deletions(-)
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 1eb485a..cddd12a 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -52,6 +52,13 @@ static int max_requests = 32;
#define BLOCK_SIZE 512
#define IOCB_COUNT (BLKIF_MAX_SEGMENTS_PER_REQUEST + 2)
+struct PersistentGrant {
+void *page;
+struct XenBlkDev *blkdev;
+};
+
+typedef struct PersistentGrant PersistentGrant;
+
struct ioreq {
blkif_request_t req;
int16_t status;
@@ -69,6 +76,7 @@ struct ioreq {
int prot;
void*page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
void*pages;
+int num_unmap;
/* aio status */
int aio_inflight;
@@ -105,6 +113,12 @@ struct XenBlkDev {
int requests_inflight;
int requests_finished;
+/* Persistent grants extension */
+gbooleanfeature_persistent;
+GTree *persistent_gnts;
+unsigned intpersistent_gnt_count;
+unsigned intmax_grants;
+
/* qemu block driver */
DriveInfo *dinfo;
BlockDriverState*bs;
@@ -138,6 +152,29 @@ static void ioreq_reset(struct ioreq *ioreq)
qemu_iovec_reset(&ioreq->v);
}
+static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
+{
+uint ua = GPOINTER_TO_UINT(a);
+uint ub = GPOINTER_TO_UINT(b);
+return (ua > ub) - (ua < ub);
+}
+
+static void destroy_grant(gpointer pgnt)
+{
+PersistentGrant *grant = pgnt;
+XenGnttab gnt = grant->blkdev->xendev.gnttabdev;
+
+if (xc_gnttab_munmap(gnt, grant->page, 1) != 0) {
+xen_be_printf(&grant->blkdev->xendev, 0,
+ "xc_gnttab_munmap failed: %s\n",
+ strerror(errno));
+}
+grant->blkdev->persistent_gnt_count--;
+xen_be_printf(&grant->blkdev->xendev, 3,
+ "unmapped grant %p\n", grant->page);
+g_free(grant);
+}
+
static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
{
struct ioreq *ioreq = NULL;
@@ -266,21 +303,21 @@ static void ioreq_unmap(struct ioreq *ioreq)
XenGnttab gnt = ioreq->blkdev->xendev.gnttabdev;
int i;
-if (ioreq->v.niov == 0 || ioreq->mapped == 0) {
+if (ioreq->num_unmap == 0 || ioreq->mapped == 0) {
return;
}
if (batch_maps) {
if (!ioreq->pages) {
return;
}
-if (xc_gnttab_munmap(gnt, ioreq->pages, ioreq->v.niov) != 0) {
+if (xc_gnttab_munmap(gnt, ioreq->pages, ioreq->num_unmap) != 0) {
xen_be_printf(&ioreq->blkdev->xendev, 0, "xc_gnttab_munmap failed:
%s\n",
strerror(errno));
}
-ioreq->blkdev->cnt_map -= ioreq->v.niov;
+ioreq->blkdev->cnt_map -= ioreq->num_unmap;
ioreq->pages = NULL;
} else {
-for (i = 0; i < ioreq->v.niov; i++) {
+for (i = 0; i < ioreq->num_unmap; i++) {
if (!ioreq->page[i]) {
continue;
}
@@ -298,41 +335,121 @@ static void ioreq_unmap(struct ioreq *ioreq)
static int ioreq_map(struct ioreq *ioreq)
{
XenGnttab gnt = ioreq->blkdev->xendev.gnttabdev;
-int i;
+uint32_t domids[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+uint32_t refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+void *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+int i, j, new_maps = 0;
+PersistentGrant *grant;
+/* domids and refs variables will contain the information necessary
+ * to map the grants that are needed to fulfill this request.
+ *
+ * After mapping the needed grants, the page array will contain the
+ * memory address of each granted page in the order specified in ioreq
+ * (disregarding if it's a persistent grant or not).
+ */
if (ioreq->v.niov == 0 || ioreq->mapped == 1) {
return 0;
}
-if (batch_maps) {
+if (ioreq->blkdev->feature_persistent) {
+for (i = 0; i < ioreq->v.niov; i++) {
+grant = g_tree_lookup(ioreq->blkdev->persistent_gnts,
+
[Qemu-devel] [PATCH] audio: fix bug in mixeng_template.h build on NetBSD
This is a bug fix for rc1, although I think this bug has been present for a long time. NetBSD has typedefs and defines of types, so all the types specified in mixeng.c IN_T define (int8_t, uint8_t...) got expanded to __int8_t, __uint8_t, and the construction of types in mixeng_template.h failed. audio/mixeng.c:150:17: error: 'conv_natural_uint8_t_to_mono' undeclared here (not in a function) [...] audio/mixeng_template.h:114:1: warning: 'conv_natural___uint8_t_to_mono' defined but not used [...] Undef those types, so we can safely compile. This is safe even if the types are not defined. Cc: Vassili Karpov (malc) Signed-off-by: Roger Pau Monne --- audio/mixeng_template.h | 12 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/audio/mixeng_template.h b/audio/mixeng_template.h index e644c23..33e6b61 100644 --- a/audio/mixeng_template.h +++ b/audio/mixeng_template.h @@ -27,6 +27,18 @@ * dec++'ified by Dscho */ +/* + * Remove definitions of types, to prevent expansion in "glue" macro. + * This is needed at least for NetBSD, but any operating system that + * has those defines will probably cause trouble. + */ +#undef int8_t +#undef uint8_t +#undef int16_t +#undef uint16_t +#undef int32_t +#undef uint32_t + #ifndef SIGNED #define HALF (IN_MAX >> 1) #endif -- 1.7.7.5 (Apple Git-26)
Re: [Qemu-devel] [PATCH] audio: fix bug in mixeng_template.h build on NetBSD
malc escribió: On Fri, 11 May 2012, Roger Pau Monne wrote: This is a bug fix for rc1, although I think this bug has been present for a long time. If there's a bug than it's within NetBSD itself, this issue has been discussed few times (at least twice it hink) in the past, please search the ML archives. [..snip..] Hello, I've found http://copilotco.com/mail-archives/qemu.2008/msg11157.html and http://lists.gnu.org/archive/html/qemu-devel/2009-09/msg00255.html But none of them seems to reach a conclusion about how to solve this. The standard regarding stdint.h doesn't forbid having this types defined as both typedefs and preprocessor macros, so I don't think this is a NetBSD bug. As far as I can see, we have at least three ways of solving this: - Undef the macros. - Use something like concat(conv_natural_, uint8_t, _to_mono) (as done on the second thread I've posted). - Pass two separate arguments; instead of using: #define IN_T uint8_t use something like: #define BSIZE 8 #define ITYPE uint But this will probably introduce quite some modifications. Anyway, how do you think it's best to solve this? Regards, Roger.
[Qemu-devel] [PATCH] audio: split IN_T into two separate constants
Split IN_T into BSIZE and ITYPE, to avoid expansion if the OS has
defined macros for the intX_t and uintX_t types. The IN_T constant is
then defined in mixeng_template.h so it can be used by the
functions/macros on this header file.
This change has been tested successfully under Debian Linux and NetBSD
6.0BETA.
Cc: Vassili Karpov (malc)
Signed-off-by: Roger Pau Monne
---
audio/mixeng.c | 36
audio/mixeng_template.h |4 +++-
2 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/audio/mixeng.c b/audio/mixeng.c
index 5446be6..02a9d9f 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -33,7 +33,8 @@
#define ENDIAN_CONVERT(v) (v)
/* Signed 8 bit */
-#define IN_T int8_t
+#define BSIZE 8
+#define ITYPE int
#define IN_MIN SCHAR_MIN
#define IN_MAX SCHAR_MAX
#define SIGNED
@@ -42,25 +43,29 @@
#undef SIGNED
#undef IN_MAX
#undef IN_MIN
-#undef IN_T
+#undef BSIZE
+#undef ITYPE
#undef SHIFT
/* Unsigned 8 bit */
-#define IN_T uint8_t
+#define BSIZE 8
+#define ITYPE uint
#define IN_MIN 0
#define IN_MAX UCHAR_MAX
#define SHIFT 8
#include "mixeng_template.h"
#undef IN_MAX
#undef IN_MIN
-#undef IN_T
+#undef BSIZE
+#undef ITYPE
#undef SHIFT
#undef ENDIAN_CONVERT
#undef ENDIAN_CONVERSION
/* Signed 16 bit */
-#define IN_T int16_t
+#define BSIZE 16
+#define ITYPE int
#define IN_MIN SHRT_MIN
#define IN_MAX SHRT_MAX
#define SIGNED
@@ -78,11 +83,13 @@
#undef SIGNED
#undef IN_MAX
#undef IN_MIN
-#undef IN_T
+#undef BSIZE
+#undef ITYPE
#undef SHIFT
/* Unsigned 16 bit */
-#define IN_T uint16_t
+#define BSIZE 16
+#define ITYPE uint
#define IN_MIN 0
#define IN_MAX USHRT_MAX
#define SHIFT 16
@@ -98,11 +105,13 @@
#undef ENDIAN_CONVERSION
#undef IN_MAX
#undef IN_MIN
-#undef IN_T
+#undef BSIZE
+#undef ITYPE
#undef SHIFT
/* Signed 32 bit */
-#define IN_T int32_t
+#define BSIZE 32
+#define ITYPE int
#define IN_MIN INT32_MIN
#define IN_MAX INT32_MAX
#define SIGNED
@@ -120,11 +129,13 @@
#undef SIGNED
#undef IN_MAX
#undef IN_MIN
-#undef IN_T
+#undef BSIZE
+#undef ITYPE
#undef SHIFT
/* Unsigned 32 bit */
-#define IN_T uint32_t
+#define BSIZE 32
+#define ITYPE uint
#define IN_MIN 0
#define IN_MAX UINT32_MAX
#define SHIFT 32
@@ -140,7 +151,8 @@
#undef ENDIAN_CONVERSION
#undef IN_MAX
#undef IN_MIN
-#undef IN_T
+#undef BSIZE
+#undef ITYPE
#undef SHIFT
t_sample *mixeng_conv[2][2][2][3] = {
diff --git a/audio/mixeng_template.h b/audio/mixeng_template.h
index e644c23..30849a6 100644
--- a/audio/mixeng_template.h
+++ b/audio/mixeng_template.h
@@ -31,7 +31,8 @@
#define HALF (IN_MAX >> 1)
#endif
-#define ET glue (ENDIAN_CONVERSION, glue (_, IN_T))
+#define ET glue (ENDIAN_CONVERSION, glue (glue (glue (_, ITYPE), BSIZE), _t))
+#define IN_T glue (glue (ITYPE, BSIZE), _t)
#ifdef FLOAT_MIXENG
static mixeng_real inline glue (conv_, ET) (IN_T v)
@@ -150,3 +151,4 @@ static void glue (glue (clip_, ET), _from_mono)
#undef ET
#undef HALF
+#undef IN_T
--
1.7.7.5 (Apple Git-26)
Re: [Qemu-devel] [PATCH] audio: split IN_T into two separate constants
malc wrote: On Fri, 18 May 2012, Roger Pau Monne wrote: Split IN_T into BSIZE and ITYPE, to avoid expansion if the OS has defined macros for the intX_t and uintX_t types. The IN_T constant is then defined in mixeng_template.h so it can be used by the functions/macros on this header file. This change has been tested successfully under Debian Linux and NetBSD 6.0BETA. Applied. Thanks!
[Qemu-devel] [PATCH] qemu-xen: make use of xenstore relative paths
Qemu has several hardcoded xenstore paths that are only valid on Dom0.
Attempts to launch a Qemu instance (to act as a userspace backend for
PV disks) will fail because Qemu is not able to access those paths
when running on a domain different than Dom0.
Instead make the xenstore paths relative to the domain where Qemu is
actually running.
Signed-off-by: Roger Pau Monné
Cc: xen-de...@lists.xenproject.org
Cc: Anthony PERARD
Cc: Stefano Stabellini
---
hw/xen_backend.c | 19 ++-
xen-all.c|2 +-
2 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/hw/xen_backend.c b/hw/xen_backend.c
index 008cdb3..e220606 100644
--- a/hw/xen_backend.c
+++ b/hw/xen_backend.c
@@ -205,7 +205,6 @@ static struct XenDevice *xen_be_get_xendev(const char
*type, int dom, int dev,
struct XenDevOps *ops)
{
struct XenDevice *xendev;
-char *dom0;
xendev = xen_be_find_xendev(type, dom, dev);
if (xendev) {
@@ -219,12 +218,10 @@ static struct XenDevice *xen_be_get_xendev(const char
*type, int dom, int dev,
xendev->dev = dev;
xendev->ops = ops;
-dom0 = xs_get_domain_path(xenstore, 0);
-snprintf(xendev->be, sizeof(xendev->be), "%s/backend/%s/%d/%d",
- dom0, xendev->type, xendev->dom, xendev->dev);
+snprintf(xendev->be, sizeof(xendev->be), "backend/%s/%d/%d",
+ xendev->type, xendev->dom, xendev->dev);
snprintf(xendev->name, sizeof(xendev->name), "%s-%d",
xendev->type, xendev->dev);
-free(dom0);
xendev->debug = debug;
xendev->local_port = -1;
@@ -570,14 +567,12 @@ static int xenstore_scan(const char *type, int dom,
struct XenDevOps *ops)
{
struct XenDevice *xendev;
char path[XEN_BUFSIZE], token[XEN_BUFSIZE];
-char **dev = NULL, *dom0;
+char **dev = NULL;
unsigned int cdev, j;
/* setup watch */
-dom0 = xs_get_domain_path(xenstore, 0);
snprintf(token, sizeof(token), "be:%p:%d:%p", type, dom, ops);
-snprintf(path, sizeof(path), "%s/backend/%s/%d", dom0, type, dom);
-free(dom0);
+snprintf(path, sizeof(path), "backend/%s/%d", type, dom);
if (!xs_watch(xenstore, path, token)) {
xen_be_printf(NULL, 0, "xen be: watching backend path (%s) failed\n",
path);
return -1;
@@ -603,12 +598,10 @@ static void xenstore_update_be(char *watch, char *type,
int dom,
struct XenDevOps *ops)
{
struct XenDevice *xendev;
-char path[XEN_BUFSIZE], *dom0, *bepath;
+char path[XEN_BUFSIZE], *bepath;
unsigned int len, dev;
-dom0 = xs_get_domain_path(xenstore, 0);
-len = snprintf(path, sizeof(path), "%s/backend/%s/%d", dom0, type, dom);
-free(dom0);
+len = snprintf(path, sizeof(path), "backend/%s/%d", type, dom);
if (strncmp(path, watch, len) != 0) {
return;
}
diff --git a/xen-all.c b/xen-all.c
index 15be8ed..99666f9 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -967,7 +967,7 @@ static void xenstore_record_dm_state(struct xs_handle *xs,
const char *state)
exit(1);
}
-snprintf(path, sizeof (path), "/local/domain/0/device-model/%u/state",
xen_domid);
+snprintf(path, sizeof (path), "device-model/%u/state", xen_domid);
if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) {
fprintf(stderr, "error recording dm state\n");
exit(1);
--
1.7.7.5 (Apple Git-26)
[Qemu-devel] [PATCH v2] qemu/xen: make use of xenstore relative paths
Qemu has several hardcoded xenstore paths that are only valid on Dom0.
Attempts to launch a Qemu instance (to act as a userspace backend for
PV disks) will fail because Qemu is not able to access those paths
when running on a domain different than Dom0.
Instead make the xenstore paths relative to the domain where Qemu is
actually running.
Signed-off-by: Roger Pau Monné
Reviewed-by: Anthony PERARD
Cc: xen-de...@lists.xenproject.org
Cc: Anthony PERARD
Cc: Stefano Stabellini
---
Changes since v1:
* Update paths to match upstream Qemu.
---
hw/xen/xen_backend.c | 19 ++-
xen-all.c|2 +-
2 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index d82ce5d..197795f 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -205,7 +205,6 @@ static struct XenDevice *xen_be_get_xendev(const char
*type, int dom, int dev,
struct XenDevOps *ops)
{
struct XenDevice *xendev;
-char *dom0;
xendev = xen_be_find_xendev(type, dom, dev);
if (xendev) {
@@ -219,12 +218,10 @@ static struct XenDevice *xen_be_get_xendev(const char
*type, int dom, int dev,
xendev->dev = dev;
xendev->ops = ops;
-dom0 = xs_get_domain_path(xenstore, 0);
-snprintf(xendev->be, sizeof(xendev->be), "%s/backend/%s/%d/%d",
- dom0, xendev->type, xendev->dom, xendev->dev);
+snprintf(xendev->be, sizeof(xendev->be), "backend/%s/%d/%d",
+ xendev->type, xendev->dom, xendev->dev);
snprintf(xendev->name, sizeof(xendev->name), "%s-%d",
xendev->type, xendev->dev);
-free(dom0);
xendev->debug = debug;
xendev->local_port = -1;
@@ -570,14 +567,12 @@ static int xenstore_scan(const char *type, int dom,
struct XenDevOps *ops)
{
struct XenDevice *xendev;
char path[XEN_BUFSIZE], token[XEN_BUFSIZE];
-char **dev = NULL, *dom0;
+char **dev = NULL;
unsigned int cdev, j;
/* setup watch */
-dom0 = xs_get_domain_path(xenstore, 0);
snprintf(token, sizeof(token), "be:%p:%d:%p", type, dom, ops);
-snprintf(path, sizeof(path), "%s/backend/%s/%d", dom0, type, dom);
-free(dom0);
+snprintf(path, sizeof(path), "backend/%s/%d", type, dom);
if (!xs_watch(xenstore, path, token)) {
xen_be_printf(NULL, 0, "xen be: watching backend path (%s) failed\n",
path);
return -1;
@@ -603,12 +598,10 @@ static void xenstore_update_be(char *watch, char *type,
int dom,
struct XenDevOps *ops)
{
struct XenDevice *xendev;
-char path[XEN_BUFSIZE], *dom0, *bepath;
+char path[XEN_BUFSIZE], *bepath;
unsigned int len, dev;
-dom0 = xs_get_domain_path(xenstore, 0);
-len = snprintf(path, sizeof(path), "%s/backend/%s/%d", dom0, type, dom);
-free(dom0);
+len = snprintf(path, sizeof(path), "backend/%s/%d", type, dom);
if (strncmp(path, watch, len) != 0) {
return;
}
diff --git a/xen-all.c b/xen-all.c
index 839f14f..0504c45 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -948,7 +948,7 @@ static void xenstore_record_dm_state(struct xs_handle *xs,
const char *state)
exit(1);
}
-snprintf(path, sizeof (path), "/local/domain/0/device-model/%u/state",
xen_domid);
+snprintf(path, sizeof (path), "device-model/%u/state", xen_domid);
if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) {
fprintf(stderr, "error recording dm state\n");
exit(1);
--
1.7.7.5 (Apple Git-26)
[Qemu-devel] [PATCH] xen-mapcache: use MAP_FIXED flag so the mmap address hint is always honored
Or if it's not possible to honor the hinted address an error is returned
instead. This makes it easier to spot the actual failure, instead of
failing later on when the caller of xen_remap_bucket realizes the
mapping has not been created at the requested address.
Also note that at least on FreeBSD using MAP_FIXED will cause mmap to
try harder to honor the passed address.
Signed-off-by: Roger Pau Monné
---
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: Igor Druzhinin
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc: Eduardo Habkost
Cc: "Michael S. Tsirkin"
Cc: Marcel Apfelbaum
Cc: xen-de...@lists.xenproject.org
---
hw/i386/xen/xen-mapcache.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
index 349f72d00c..0e2870b320 100644
--- a/hw/i386/xen/xen-mapcache.c
+++ b/hw/i386/xen/xen-mapcache.c
@@ -185,8 +185,14 @@ static void xen_remap_bucket(MapCacheEntry *entry,
}
if (!dummy) {
+/*
+ * If the caller has requested the mapping at a specific address use
+ * MAP_FIXED to make sure it's honored. Note that with MAP_FIXED at
+ * least FreeBSD will try harder to honor the passed address.
+ */
vaddr_base = xenforeignmemory_map2(xen_fmem, xen_domid, vaddr,
- PROT_READ | PROT_WRITE, 0,
+ PROT_READ | PROT_WRITE,
+ vaddr ? MAP_FIXED : 0,
nb_pfn, pfns, err);
if (vaddr_base == NULL) {
perror("xenforeignmemory_map2");
--
2.17.2 (Apple Git-113)
Re: [Qemu-devel] [PATCH 1/3] xen-disk: only advertize feature-persistent if grant copy is not available
On Wed, Jun 21, 2017 at 11:40:00AM +0100, Paul Durrant wrote: > > -Original Message- > > From: Qemu-devel [mailto:qemu-devel- > > bounces+paul.durrant=citrix@nongnu.org] On Behalf Of Paul Durrant > > Sent: 21 June 2017 10:36 > > To: Roger Pau Monne ; Stefano Stabellini > > > > Cc: Kevin Wolf ; qemu-bl...@nongnu.org; qemu- > > de...@nongnu.org; Max Reitz ; Anthony Perard > > ; xen-de...@lists.xenproject.org > > Subject: Re: [Qemu-devel] [PATCH 1/3] xen-disk: only advertize feature- > > persistent if grant copy is not available > > > > > -Original Message- > > > From: Roger Pau Monne > > > Sent: 21 June 2017 10:18 > > > To: Stefano Stabellini > > > Cc: Paul Durrant ; xen- > > de...@lists.xenproject.org; > > > qemu-devel@nongnu.org; qemu-bl...@nongnu.org; Anthony Perard > > > ; Kevin Wolf ; Max > > Reitz > > > > > > Subject: Re: [PATCH 1/3] xen-disk: only advertize feature-persistent if > > grant > > > copy is not available > > > > > > On Tue, Jun 20, 2017 at 03:19:33PM -0700, Stefano Stabellini wrote: > > > > On Tue, 20 Jun 2017, Paul Durrant wrote: > > > > > If grant copy is available then it will always be used in preference > > > > > to > > > > > persistent maps. In this case feature-persistent should not be > > advertized > > > > > to the frontend, otherwise it may needlessly copy data into > > > > > persistently > > > > > granted buffers. > > > > > > > > > > Signed-off-by: Paul Durrant > > > > > > > > CC'ing Roger. > > > > > > > > It is true that using feature-persistent together with grant copies is a > > > > a very bad idea. > > > > > > > > But this change enstablishes an explicit preference of > > > > feature_grant_copy over feature-persistent in the xen_disk backend. It > > > > is not obvious to me that it should be the case. > > > > > > > > Why is feature_grant_copy (without feature-persistent) better than > > > > feature-persistent (without feature_grant_copy)? Shouldn't we simply > > > > avoid grant copies to copy data to persistent grants? > > > > > > When using persistent grants the frontend must always copy data from > > > the buffer to the persistent grant, there's no way to avoid this. > > > > > > Using grant_copy we move the copy from the frontend to the backend, > > > which means the CPU time of the copy is accounted to the backend. This > > > is not ideal, but IMHO it's better than persistent grants because it > > > avoids keeping a pool of mapped grants that consume memory and make > > > the code more complex. > > > > > > Do you have some performance data showing the difference between > > > persistent grants vs grant copy? > > > > > > > No, but I can get some :-) > > > > For a little background... I've been trying to push throughput of fio > > running in > > a debian stretch guest on my skull canyon NUC. When I started out, I was > > getting ~100MBbs. When I finished, with this patch, the IOThreads one, the > > multi-page ring one and a bit of hackery to turn off all the aio flushes > > that > > seem to occur even if the image is opened with O_DIRECT, I was getting > > ~960Mbps... which is about line rate for the SSD in the in NUC. > > > > So, I'll force use of persistent grants on and see what sort of throughput I > > get. > > A quick test with grant copy forced off (causing persistent grants to be > used)... My VM is debian stretch using a 16 page shared ring from blkfront. > The image backing xvdb is a fully inflated 10G qcow2. > > root@dhcp-237-70:~# fio --randrepeat=1 --ioengine=libaio --direct=0 > --gtod_reduce=1 --name=test --filename=/dev/xvdb --bs=512k --iodepth=64 > --size=10G --readwrite=randwrite --ramp_time=4 > test: (g=0): rw=randwrite, bs=512K-512K/512K-512K/512K-512K, ioengine=libaio, > iodepth=64 > fio-2.16 > Starting 1 process > Jobs: 1 (f=1): [w(1)] [70.6% done] [0KB/539.4MB/0KB /s] [0/1078/0 iops] [eta > 00m:05s] > test: (groupid=0, jobs=1): err= 0: pid=633: Wed Jun 21 06:26:06 2017 > write: io=6146.6MB, bw=795905KB/s, iops=1546, runt= 7908msec > cpu : usr=2.07%, sys=34.00%, ctx=4490, majf=0, minf=1 > IO depths: 1=0.1%, 2=0.1%, 4=0.1%, 8=0.1%, 16=0.1%, 32=0.3%, >=64=166.9% > submit: 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0
[Qemu-devel] [PATCH QEMU] xen/pt: allow QEMU to request MSI unmasking at bind time
When a MSIX interrupt is bound to a guest using
xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is
left masked by default.
This causes problems with guests that first configure interrupts and
clean the per-entry MSIX table mask bit and afterwards enable MSIX
globally. In such scenario the Xen internal msixtbl handlers would not
detect the unmasking of MSIX entries because vectors are not yet
registered since MSIX is not enabled, and vectors would be left
masked.
Introduce a new flag in the gflags field to signal Xen whether a MSIX
interrupt should be unmasked after being bound.
Signed-off-by: Roger Pau Monné
Reported-by: Andreas Kinzler
---
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Jan Beulich
Cc: qemu-devel@nongnu.org
---
hw/xen/xen_pt_msi.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index ff9a79f5d2..c00ac2fd7d 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -24,6 +24,7 @@
#define XEN_PT_GFLAGS_SHIFT_DM 9
#define XEN_PT_GFLAGSSHIFT_DELIV_MODE 12
#define XEN_PT_GFLAGSSHIFT_TRG_MODE 15
+#define XEN_PT_GFLAGSSHIFT_UNMASKED 16
#define latch(fld) latch[PCI_MSIX_ENTRY_##fld / sizeof(uint32_t)]
@@ -155,7 +156,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
int pirq,
bool is_msix,
int msix_entry,
- int *old_pirq)
+ int *old_pirq,
+ bool masked)
{
PCIDevice *d = &s->dev;
uint8_t gvec = msi_vector(data);
@@ -171,6 +173,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
table_addr = s->msix->mmio_base_addr;
}
+gflags |= masked ? 0 : (1u << XEN_PT_GFLAGSSHIFT_UNMASKED);
+
rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec,
pirq, gflags, table_addr);
@@ -274,7 +278,7 @@ int xen_pt_msi_update(XenPCIPassthroughState *s)
{
XenPTMSI *msi = s->msi;
return msi_msix_update(s, msi_addr64(msi), msi->data, msi->pirq,
- false, 0, &msi->pirq);
+ false, 0, &msi->pirq, false);
}
void xen_pt_msi_disable(XenPCIPassthroughState *s)
@@ -355,7 +359,8 @@ static int xen_pt_msix_update_one(XenPCIPassthroughState
*s, int entry_nr,
}
rc = msi_msix_update(s, entry->addr, entry->data, pirq, true,
- entry_nr, &entry->pirq);
+ entry_nr, &entry->pirq,
+ vec_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT);
if (!rc) {
entry->updated = false;
--
2.11.0 (Apple Git-81)
Re: [Qemu-devel] [PATCH QEMU] xen/pt: allow QEMU to request MSI unmasking at bind time
On Thu, Aug 24, 2017 at 03:54:21AM -0600, Jan Beulich wrote:
> >>> On 24.08.17 at 11:47, wrote:
> > @@ -274,7 +278,7 @@ int xen_pt_msi_update(XenPCIPassthroughState *s)
> > {
> > XenPTMSI *msi = s->msi;
> > return msi_msix_update(s, msi_addr64(msi), msi->data, msi->pirq,
> > - false, 0, &msi->pirq);
> > + false, 0, &msi->pirq, false);
> > }
>
> I don't think this is correct when the device has mask bits.
Right, I thought I modified this. I've already changed
pt_irq_create_bind so that the original behavior is keep if the unmask
bit is not set. In this case this should be 'true' in order to keep
the previous behavior, which was correct for MSI.
It also makes me wonder whether it would be better to change the name
of the parameter to "unmask", so that false -> no change to mask, true
-> unconditionally unmask.
Thanks, Roger.
Re: [Qemu-devel] [PATCH QEMU] xen/pt: allow QEMU to request MSI unmasking at bind time
On Thu, Aug 24, 2017 at 04:13:58AM -0600, Jan Beulich wrote:
> >>> On 24.08.17 at 12:06, wrote:
> > On Thu, Aug 24, 2017 at 03:54:21AM -0600, Jan Beulich wrote:
> >> >>> On 24.08.17 at 11:47, wrote:
> >> > @@ -274,7 +278,7 @@ int xen_pt_msi_update(XenPCIPassthroughState *s)
> >> > {
> >> > XenPTMSI *msi = s->msi;
> >> > return msi_msix_update(s, msi_addr64(msi), msi->data, msi->pirq,
> >> > - false, 0, &msi->pirq);
> >> > + false, 0, &msi->pirq, false);
> >> > }
> >>
> >> I don't think this is correct when the device has mask bits.
> >
> > Right, I thought I modified this. I've already changed
> > pt_irq_create_bind so that the original behavior is keep if the unmask
> > bit is not set. In this case this should be 'true' in order to keep
> > the previous behavior, which was correct for MSI.
>
> Wouldn't you want to pass the state of the mask bit here,
> rather than uniformly hard coding true or false?
Yes, I think so. I've overlooked the MSI code because I thought we
allowed QEMU to directly write to the mask register, but that's not
true, it's trapped by Xen.
Thanks, Roger.
[Qemu-devel] [PATCH QEMU v2] xen/pt: allow QEMU to request MSI unmasking at bind time
When a MSI interrupt is bound to a guest using
xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is
left masked by default.
This causes problems with guests that first configure interrupts and
clean the per-entry MSIX table mask bit and afterwards enable MSIX
globally. In such scenario the Xen internal msixtbl handlers would not
detect the unmasking of MSIX entries because vectors are not yet
registered since MSIX is not enabled, and vectors would be left
masked.
Introduce a new flag in the gflags field to signal Xen whether a MSI
interrupt should be unmasked after being bound.
This also requires to track the mask register for MSI interrupts, so
QEMU can also notify to Xen whether the MSI interrupt should be bound
masked or unmasked
Signed-off-by: Roger Pau Monné
Reported-by: Andreas Kinzler
---
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Jan Beulich
Cc: qemu-devel@nongnu.org
---
Changes since v1:
- Track MSI mask bits.
- Notify Xen of whether MSI interrupts should be unmasked after
binding, instead of hardcoding it.
---
hw/xen/xen_pt.h | 1 +
hw/xen/xen_pt_config_init.c | 19 +--
hw/xen/xen_pt_msi.c | 13 ++---
3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 191d9caea1..aa39a9aa5f 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -180,6 +180,7 @@ typedef struct XenPTMSI {
uint32_t addr_hi; /* guest message upper address */
uint16_t data; /* guest message data */
uint32_t ctrl_offset; /* saved control offset */
+uint32_t mask; /* guest mask bits */
int pirq; /* guest pirq corresponding */
bool initialized; /* when guest MSI is initialized */
bool mapped; /* when pirq is mapped */
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 1f04ec5eec..9c724eeb62 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -1315,6 +1315,21 @@ static int
xen_pt_msgdata_reg_write(XenPCIPassthroughState *s,
return 0;
}
+static int xen_pt_mask_reg_write(XenPCIPassthroughState *s, XenPTReg
*cfg_entry,
+ uint32_t *val, uint32_t dev_value,
+ uint32_t valid_mask)
+{
+int rc;
+
+rc = xen_pt_long_reg_write(s, cfg_entry, val, dev_value, valid_mask);
+if (rc)
+return rc;
+
+s->msi->mask = *val;
+
+return 0;
+}
+
/* MSI Capability Structure reg static information table */
static XenPTRegInfo xen_pt_emu_reg_msi[] = {
/* Next Pointer reg */
@@ -1393,7 +1408,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
.emu_mask = 0x,
.init = xen_pt_mask_reg_init,
.u.dw.read = xen_pt_long_reg_read,
-.u.dw.write = xen_pt_long_reg_write,
+.u.dw.write = xen_pt_mask_reg_write,
},
/* Mask reg (if PCI_MSI_FLAGS_MASKBIT set, for 64-bit devices) */
{
@@ -1404,7 +1419,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
.emu_mask = 0x,
.init = xen_pt_mask_reg_init,
.u.dw.read = xen_pt_long_reg_read,
-.u.dw.write = xen_pt_long_reg_write,
+.u.dw.write = xen_pt_mask_reg_write,
},
/* Pending reg (if PCI_MSI_FLAGS_MASKBIT set, for 32-bit devices) */
{
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index ff9a79f5d2..6d1e3bdeb4 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -24,6 +24,7 @@
#define XEN_PT_GFLAGS_SHIFT_DM 9
#define XEN_PT_GFLAGSSHIFT_DELIV_MODE 12
#define XEN_PT_GFLAGSSHIFT_TRG_MODE 15
+#define XEN_PT_GFLAGSSHIFT_UNMASKED 16
#define latch(fld) latch[PCI_MSIX_ENTRY_##fld / sizeof(uint32_t)]
@@ -155,7 +156,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
int pirq,
bool is_msix,
int msix_entry,
- int *old_pirq)
+ int *old_pirq,
+ bool masked)
{
PCIDevice *d = &s->dev;
uint8_t gvec = msi_vector(data);
@@ -171,6 +173,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
table_addr = s->msix->mmio_base_addr;
}
+gflags |= masked ? 0 : (1u << XEN_PT_GFLAGSSHIFT_UNMASKED);
+
rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec,
pirq, gflags, table_addr);
@@ -273,8 +277,10 @@ int xen_pt_msi_setup(XenPCIPassthroughState *s)
int xen_pt_msi_update(XenPCIPassthroughState *s)
{
XenPTMSI *msi = s->msi;
+
+/* Current MSI emulation in QEMU only supports 1 vector */
return msi_msix_update(s, msi_addr64(msi), msi->data, msi->pirq,
- false, 0, &msi->pirq);
+ false, 0, &msi->pirq, msi->mask & 1);
}
void xen_pt_msi_disable(XenPCIPassthroughState *s)
@@ -355,7 +361,8 @@ static int xen_pt_msix_update_one(XenPCIP
Re: [Qemu-devel] [PATCH QEMU v2] xen/pt: allow QEMU to request MSI unmasking at bind time
On Thu, Aug 24, 2017 at 07:09:08AM -0600, Jan Beulich wrote: > >>> On 24.08.17 at 14:19, wrote: > > When a MSI interrupt is bound to a guest using > > xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is > > left masked by default. > > > > This causes problems with guests that first configure interrupts and > > clean the per-entry MSIX table mask bit and afterwards enable MSIX > > globally. In such scenario the Xen internal msixtbl handlers would not > > detect the unmasking of MSIX entries because vectors are not yet > > registered since MSIX is not enabled, and vectors would be left > > masked. > > > > Introduce a new flag in the gflags field to signal Xen whether a MSI > > interrupt should be unmasked after being bound. > > > > This also requires to track the mask register for MSI interrupts, so > > QEMU can also notify to Xen whether the MSI interrupt should be bound > > masked or unmasked > > > > Signed-off-by: Roger Pau Monné > > Reported-by: Andreas Kinzler > > Reviewed-by: Jan Beulich Thanks, Roger.
[Qemu-devel] [PATCH QEMU v3] xen/pt: allow QEMU to request MSI unmasking at bind time
When a MSI interrupt is bound to a guest using
xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is
left masked by default.
This causes problems with guests that first configure interrupts and
clean the per-entry MSIX table mask bit and afterwards enable MSIX
globally. In such scenario the Xen internal msixtbl handlers would not
detect the unmasking of MSIX entries because vectors are not yet
registered since MSIX is not enabled, and vectors would be left
masked.
Introduce a new flag in the gflags field to signal Xen whether a MSI
interrupt should be unmasked after being bound.
This also requires to track the mask register for MSI interrupts, so
QEMU can also notify to Xen whether the MSI interrupt should be bound
masked or unmasked
Signed-off-by: Roger Pau Monné
Reviewed-by: Jan Beulich
Reported-by: Andreas Kinzler
---
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Jan Beulich
Cc: qemu-devel@nongnu.org
---
Changes since v2:
- Fix coding style.
Changes since v1:
- Track MSI mask bits.
- Notify Xen of whether MSI interrupts should be unmasked after
binding, instead of hardcoding it.
---
hw/xen/xen_pt.h | 1 +
hw/xen/xen_pt_config_init.c | 19 +--
hw/xen/xen_pt_msi.c | 13 ++---
3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 191d9caea1..aa39a9aa5f 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -180,6 +180,7 @@ typedef struct XenPTMSI {
uint32_t addr_hi; /* guest message upper address */
uint16_t data; /* guest message data */
uint32_t ctrl_offset; /* saved control offset */
+uint32_t mask; /* guest mask bits */
int pirq; /* guest pirq corresponding */
bool initialized; /* when guest MSI is initialized */
bool mapped; /* when pirq is mapped */
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 1f04ec5eec..9c724eeb62 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -1315,6 +1315,21 @@ static int
xen_pt_msgdata_reg_write(XenPCIPassthroughState *s,
return 0;
}
+static int xen_pt_mask_reg_write(XenPCIPassthroughState *s, XenPTReg
*cfg_entry,
+ uint32_t *val, uint32_t dev_value,
+ uint32_t valid_mask)
+{
+int rc;
+
+rc = xen_pt_long_reg_write(s, cfg_entry, val, dev_value, valid_mask);
+if (rc)
+return rc;
+
+s->msi->mask = *val;
+
+return 0;
+}
+
/* MSI Capability Structure reg static information table */
static XenPTRegInfo xen_pt_emu_reg_msi[] = {
/* Next Pointer reg */
@@ -1393,7 +1408,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
.emu_mask = 0x,
.init = xen_pt_mask_reg_init,
.u.dw.read = xen_pt_long_reg_read,
-.u.dw.write = xen_pt_long_reg_write,
+.u.dw.write = xen_pt_mask_reg_write,
},
/* Mask reg (if PCI_MSI_FLAGS_MASKBIT set, for 64-bit devices) */
{
@@ -1404,7 +1419,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
.emu_mask = 0x,
.init = xen_pt_mask_reg_init,
.u.dw.read = xen_pt_long_reg_read,
-.u.dw.write = xen_pt_long_reg_write,
+.u.dw.write = xen_pt_mask_reg_write,
},
/* Pending reg (if PCI_MSI_FLAGS_MASKBIT set, for 32-bit devices) */
{
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index ff9a79f5d2..6d1e3bdeb4 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -24,6 +24,7 @@
#define XEN_PT_GFLAGS_SHIFT_DM 9
#define XEN_PT_GFLAGSSHIFT_DELIV_MODE 12
#define XEN_PT_GFLAGSSHIFT_TRG_MODE 15
+#define XEN_PT_GFLAGSSHIFT_UNMASKED 16
#define latch(fld) latch[PCI_MSIX_ENTRY_##fld / sizeof(uint32_t)]
@@ -155,7 +156,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
int pirq,
bool is_msix,
int msix_entry,
- int *old_pirq)
+ int *old_pirq,
+ bool masked)
{
PCIDevice *d = &s->dev;
uint8_t gvec = msi_vector(data);
@@ -171,6 +173,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
table_addr = s->msix->mmio_base_addr;
}
+gflags |= masked ? 0 : (1u << XEN_PT_GFLAGSSHIFT_UNMASKED);
+
rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec,
pirq, gflags, table_addr);
@@ -273,8 +277,10 @@ int xen_pt_msi_setup(XenPCIPassthroughState *s)
int xen_pt_msi_update(XenPCIPassthroughState *s)
{
XenPTMSI *msi = s->msi;
+
+/* Current MSI emulation in QEMU only supports 1 vector */
return msi_msix_update(s, msi_addr64(msi), msi->data, msi->pirq,
- false, 0, &msi->pirq);
+ false, 0, &msi->pirq, msi->mask & 1);
}
void xen_pt_msi_disable(XenPCIPassthroughState
[Qemu-devel] [PATCH QEMU v4] xen/pt: allow QEMU to request MSI unmasking at bind time
When a MSI interrupt is bound to a guest using
xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is
left masked by default.
This causes problems with guests that first configure interrupts and
clean the per-entry MSIX table mask bit and afterwards enable MSIX
globally. In such scenario the Xen internal msixtbl handlers would not
detect the unmasking of MSIX entries because vectors are not yet
registered since MSIX is not enabled, and vectors would be left
masked.
Introduce a new flag in the gflags field to signal Xen whether a MSI
interrupt should be unmasked after being bound.
This also requires to track the mask register for MSI interrupts, so
QEMU can also notify to Xen whether the MSI interrupt should be bound
masked or unmasked
Signed-off-by: Roger Pau Monné
Reviewed-by: Jan Beulich
Reported-by: Andreas Kinzler
---
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Jan Beulich
Cc: qemu-devel@nongnu.org
---
Changes since v2:
- Fix coding style.
Changes since v1:
- Track MSI mask bits.
- Notify Xen of whether MSI interrupts should be unmasked after
binding, instead of hardcoding it.
---
hw/xen/xen_pt.h | 1 +
hw/xen/xen_pt_config_init.c | 20 ++--
hw/xen/xen_pt_msi.c | 13 ++---
3 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 191d9caea1..aa39a9aa5f 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -180,6 +180,7 @@ typedef struct XenPTMSI {
uint32_t addr_hi; /* guest message upper address */
uint16_t data; /* guest message data */
uint32_t ctrl_offset; /* saved control offset */
+uint32_t mask; /* guest mask bits */
int pirq; /* guest pirq corresponding */
bool initialized; /* when guest MSI is initialized */
bool mapped; /* when pirq is mapped */
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 1f04ec5eec..a3ce33e78b 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -1315,6 +1315,22 @@ static int
xen_pt_msgdata_reg_write(XenPCIPassthroughState *s,
return 0;
}
+static int xen_pt_mask_reg_write(XenPCIPassthroughState *s, XenPTReg
*cfg_entry,
+ uint32_t *val, uint32_t dev_value,
+ uint32_t valid_mask)
+{
+int rc;
+
+rc = xen_pt_long_reg_write(s, cfg_entry, val, dev_value, valid_mask);
+if (rc) {
+return rc;
+}
+
+s->msi->mask = *val;
+
+return 0;
+}
+
/* MSI Capability Structure reg static information table */
static XenPTRegInfo xen_pt_emu_reg_msi[] = {
/* Next Pointer reg */
@@ -1393,7 +1409,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
.emu_mask = 0x,
.init = xen_pt_mask_reg_init,
.u.dw.read = xen_pt_long_reg_read,
-.u.dw.write = xen_pt_long_reg_write,
+.u.dw.write = xen_pt_mask_reg_write,
},
/* Mask reg (if PCI_MSI_FLAGS_MASKBIT set, for 64-bit devices) */
{
@@ -1404,7 +1420,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
.emu_mask = 0x,
.init = xen_pt_mask_reg_init,
.u.dw.read = xen_pt_long_reg_read,
-.u.dw.write = xen_pt_long_reg_write,
+.u.dw.write = xen_pt_mask_reg_write,
},
/* Pending reg (if PCI_MSI_FLAGS_MASKBIT set, for 32-bit devices) */
{
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index ff9a79f5d2..6d1e3bdeb4 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -24,6 +24,7 @@
#define XEN_PT_GFLAGS_SHIFT_DM 9
#define XEN_PT_GFLAGSSHIFT_DELIV_MODE 12
#define XEN_PT_GFLAGSSHIFT_TRG_MODE 15
+#define XEN_PT_GFLAGSSHIFT_UNMASKED 16
#define latch(fld) latch[PCI_MSIX_ENTRY_##fld / sizeof(uint32_t)]
@@ -155,7 +156,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
int pirq,
bool is_msix,
int msix_entry,
- int *old_pirq)
+ int *old_pirq,
+ bool masked)
{
PCIDevice *d = &s->dev;
uint8_t gvec = msi_vector(data);
@@ -171,6 +173,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
table_addr = s->msix->mmio_base_addr;
}
+gflags |= masked ? 0 : (1u << XEN_PT_GFLAGSSHIFT_UNMASKED);
+
rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec,
pirq, gflags, table_addr);
@@ -273,8 +277,10 @@ int xen_pt_msi_setup(XenPCIPassthroughState *s)
int xen_pt_msi_update(XenPCIPassthroughState *s)
{
XenPTMSI *msi = s->msi;
+
+/* Current MSI emulation in QEMU only supports 1 vector */
return msi_msix_update(s, msi_addr64(msi), msi->data, msi->pirq,
- false, 0, &msi->pirq);
+ false, 0, &msi->pirq, msi->mask & 1);
}
void xen_pt_msi_disable(XenPCIPassth
[PATCH] qemu-user: fix build with LLVM lld 10
lld 10.0.0 introduced a new linker option --image-base equivalent to the GNU -Ttext-segment one, hence use it when available. This fixes the build of QEMU on systems using lld 10 or greater. Signed-off-by: Dimitry Andric Signed-off-by: Roger Pau Monné --- Cc: Laurent Vivier Cc: Richard Henderson Cc: "Philippe Mathieu-Daudé" Cc: Paolo Bonzini Cc: "Daniel P. Berrangé" --- configure | 42 +++--- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/configure b/configure index da09c35895..92d57d84fa 100755 --- a/configure +++ b/configure @@ -6514,27 +6514,31 @@ if ( [ "$linux_user" = yes ] || [ "$bsd_user" = yes ] ) && [ "$pie" = no ]; then cat > $TMPC &1; then -error_exit \ -"We need to link the QEMU user mode binaries at a" \ -"specific text address. Unfortunately your linker" \ -"doesn't support either the -Ttext-segment option or" \ -"printing the default linker script with --verbose." \ -"If you don't want the user mode binaries, pass the" \ -"--disable-user option to configure." - fi + textseg_ldflags="-Wl,-Ttext-segment=$textseg_addr" + if ! compile_prog "" "$textseg_ldflags"; then +# In case ld does not support -Ttext-segment, edit the default linker +# script via sed to set the .text start addr. This is needed on FreeBSD +# at least. +if ! $ld --verbose >/dev/null 2>&1; then + error_exit \ + "We need to link the QEMU user mode binaries at a" \ + "specific text address. Unfortunately your linker" \ + "doesn't support either the --image-base or -Ttext-segment" \ + "options or printing the default linker script with" \ + "--verbose. If you don't want the user mode binaries," \ + "pass the --disable-user option to configure." +fi - $ld --verbose | sed \ --e '1,/==/d' \ --e '/==/,$d' \ --e "s/[.] = [0-9a-fx]* [+] SIZEOF_HEADERS/. = $textseg_addr + SIZEOF_HEADERS/" \ --e "s/__executable_start = [0-9a-fx]*/__executable_start = $textseg_addr/" > config-host.ld - textseg_ldflags="-Wl,-T../config-host.ld" +$ld --verbose | sed \ + -e '1,/==/d' \ + -e '/==/,$d' \ + -e "s/[.] = [0-9a-fx]* [+] SIZEOF_HEADERS/. = $textseg_addr + SIZEOF_HEADERS/" \ + -e "s/__executable_start = [0-9a-fx]*/__executable_start = $textseg_addr/" > config-host.ld +textseg_ldflags="-Wl,-T../config-host.ld" + fi fi fi fi -- 2.26.0
[PATCH] xen: fix build without pci passthrough
has_igd_gfx_passthru is only available when QEMU is built with
CONFIG_XEN_PCI_PASSTHROUGH, and hence shouldn't be used in common
code without checking if it's available.
Fixes: 46472d82322d0 ('xen: convert "-machine igd-passthru" to an accelerator
property')
Signed-off-by: Roger Pau Monné
---
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: xen-de...@lists.xenproject.org
---
hw/xen/xen-common.c | 4
hw/xen/xen_pt.h | 7 +++
2 files changed, 11 insertions(+)
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index a15070f7f6..c800862419 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -127,6 +127,7 @@ static void xen_change_state_handler(void *opaque, int
running,
}
}
+#ifdef CONFIG_XEN_PCI_PASSTHROUGH
static bool xen_get_igd_gfx_passthru(Object *obj, Error **errp)
{
return has_igd_gfx_passthru;
@@ -136,6 +137,7 @@ static void xen_set_igd_gfx_passthru(Object *obj, bool
value, Error **errp)
{
has_igd_gfx_passthru = value;
}
+#endif
static void xen_setup_post(MachineState *ms, AccelState *accel)
{
@@ -197,11 +199,13 @@ static void xen_accel_class_init(ObjectClass *oc, void
*data)
compat_props_add(ac->compat_props, compat, G_N_ELEMENTS(compat));
+#ifdef CONFIG_XEN_PCI_PASSTHROUGH
object_class_property_add_bool(oc, "igd-passthru",
xen_get_igd_gfx_passthru, xen_set_igd_gfx_passthru,
&error_abort);
object_class_property_set_description(oc, "igd-passthru",
"Set on/off to enable/disable igd passthrou", &error_abort);
+#endif
}
#define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen")
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 179775db7b..660dd8a008 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -1,6 +1,7 @@
#ifndef XEN_PT_H
#define XEN_PT_H
+#include "qemu/osdep.h"
#include "hw/xen/xen_common.h"
#include "hw/pci/pci.h"
#include "xen-host-pci-device.h"
@@ -322,7 +323,13 @@ extern void *pci_assign_dev_load_option_rom(PCIDevice *dev,
unsigned int domain,
unsigned int bus, unsigned int
slot,
unsigned int function);
+
+#ifdef CONFIG_XEN_PCI_PASSTHROUGH
extern bool has_igd_gfx_passthru;
+#else
+# define has_igd_gfx_passthru false
+#endif
+
static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
{
return (has_igd_gfx_passthru
--
2.26.2
[PATCH v2] xen: fix build without pci passthrough
has_igd_gfx_passthru is only available when QEMU is built with
CONFIG_XEN_PCI_PASSTHROUGH, and hence shouldn't be used in common
code without checking if it's available.
Fixes: 46472d82322d0 ('xen: convert "-machine igd-passthru" to an accelerator
property')
Signed-off-by: Roger Pau Monné
---
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: xen-de...@lists.xenproject.org
---
Changes since v1:
- Do not include osdep in header file.
- Always add the setters/getters of igd-passthru, report an error
when attempting to set igd-passthru without built in
pci-passthrough support.
---
hw/xen/xen-common.c | 4
hw/xen/xen_pt.h | 6 ++
2 files changed, 10 insertions(+)
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index 70564cc952..d758770da0 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -134,7 +134,11 @@ static bool xen_get_igd_gfx_passthru(Object *obj, Error
**errp)
static void xen_set_igd_gfx_passthru(Object *obj, bool value, Error **errp)
{
+#ifdef CONFIG_XEN_PCI_PASSTHROUGH
has_igd_gfx_passthru = value;
+#else
+error_setg(errp, "Xen PCI passthrough support not built in");
+#endif
}
static void xen_setup_post(MachineState *ms, AccelState *accel)
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 179775db7b..7430235a27 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -322,7 +322,13 @@ extern void *pci_assign_dev_load_option_rom(PCIDevice *dev,
unsigned int domain,
unsigned int bus, unsigned int
slot,
unsigned int function);
+
+#ifdef CONFIG_XEN_PCI_PASSTHROUGH
extern bool has_igd_gfx_passthru;
+#else
+# define has_igd_gfx_passthru false
+#endif
+
static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
{
return (has_igd_gfx_passthru
--
2.26.2
[Qemu-devel] [PATCH v2] xen-mapcache: use MAP_FIXED flag so the mmap address hint is always honored
Or if it's not possible to honor the hinted address an error is returned
instead. This makes it easier to spot the actual failure, instead of
failing later on when the caller of xen_remap_bucket realizes the
mapping has not been created at the requested address.
Also note that at least on FreeBSD using MAP_FIXED will cause mmap to
try harder to honor the passed address.
Signed-off-by: Roger Pau Monné
---
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: Igor Druzhinin
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc: Eduardo Habkost
Cc: "Michael S. Tsirkin"
Cc: Marcel Apfelbaum
Cc: xen-de...@lists.xenproject.org
---
Changes since v1:
- Use MAP_FIXED for the dummy mmap call also if a specific virtual
address is requested.
---
hw/i386/xen/xen-mapcache.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
index 349f72d00c..23de5517db 100644
--- a/hw/i386/xen/xen-mapcache.c
+++ b/hw/i386/xen/xen-mapcache.c
@@ -185,8 +185,13 @@ static void xen_remap_bucket(MapCacheEntry *entry,
}
if (!dummy) {
+/*
+ * If the caller has requested the mapping at a specific address use
+ * MAP_FIXED to make sure it's honored.
+ */
vaddr_base = xenforeignmemory_map2(xen_fmem, xen_domid, vaddr,
- PROT_READ | PROT_WRITE, 0,
+ PROT_READ | PROT_WRITE,
+ vaddr ? MAP_FIXED : 0,
nb_pfn, pfns, err);
if (vaddr_base == NULL) {
perror("xenforeignmemory_map2");
@@ -198,7 +203,8 @@ static void xen_remap_bucket(MapCacheEntry *entry,
* mapping immediately due to certain circumstances (i.e. on resume
now)
*/
vaddr_base = mmap(vaddr, size, PROT_READ | PROT_WRITE,
- MAP_ANON | MAP_SHARED, -1, 0);
+ MAP_ANON | MAP_SHARED | (vaddr ? MAP_FIXED : 0),
+ -1, 0);
if (vaddr_base == MAP_FAILED) {
perror("mmap");
exit(-1);
--
2.17.2 (Apple Git-113)
[Qemu-devel] [PATCH v3] xen-mapcache: use MAP_FIXED flag so the mmap address hint is always honored
Or if it's not possible to honor the hinted address an error is returned
instead. This makes it easier to spot the actual failure, instead of
failing later on when the caller of xen_remap_bucket realizes the
mapping has not been created at the requested address.
Also note that at least on FreeBSD using MAP_FIXED will cause mmap to
try harder to honor the passed address.
Signed-off-by: Roger Pau Monné
Reviewed-by: Paul Durrant
---
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: Igor Druzhinin
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc: Eduardo Habkost
Cc: "Michael S. Tsirkin"
Cc: Marcel Apfelbaum
Cc: xen-de...@lists.xenproject.org
---
Changes since v2:
- Move comment position.
Changes since v1:
- Use MAP_FIXED for the dummy mmap call also if a specific virtual
address is requested.
---
hw/i386/xen/xen-mapcache.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
index 349f72d00c..254759f776 100644
--- a/hw/i386/xen/xen-mapcache.c
+++ b/hw/i386/xen/xen-mapcache.c
@@ -184,9 +184,14 @@ static void xen_remap_bucket(MapCacheEntry *entry,
pfns[i] = (address_index << (MCACHE_BUCKET_SHIFT-XC_PAGE_SHIFT)) + i;
}
+/*
+ * If the caller has requested the mapping at a specific address use
+ * MAP_FIXED to make sure it's honored.
+ */
if (!dummy) {
vaddr_base = xenforeignmemory_map2(xen_fmem, xen_domid, vaddr,
- PROT_READ | PROT_WRITE, 0,
+ PROT_READ | PROT_WRITE,
+ vaddr ? MAP_FIXED : 0,
nb_pfn, pfns, err);
if (vaddr_base == NULL) {
perror("xenforeignmemory_map2");
@@ -198,7 +203,8 @@ static void xen_remap_bucket(MapCacheEntry *entry,
* mapping immediately due to certain circumstances (i.e. on resume
now)
*/
vaddr_base = mmap(vaddr, size, PROT_READ | PROT_WRITE,
- MAP_ANON | MAP_SHARED, -1, 0);
+ MAP_ANON | MAP_SHARED | (vaddr ? MAP_FIXED : 0),
+ -1, 0);
if (vaddr_base == MAP_FAILED) {
perror("mmap");
exit(-1);
--
2.17.2 (Apple Git-113)
[PATCH 0/2] xen: error handling and FreeBSD compatibility fixes
Hello, First patch fixes some error handling paths that incorrectly used error_prepend() in the Xen console driver. Second patch removes usage of the 'm' character in scanf directives, as it's not supported on FreeBSD (see usages of "%ms"). Thanks, Roger. Roger Pau Monné (2): xen/console: fix error handling in xen_console_device_create() xen: do not use '%ms' scanf specifier hw/char/xen_console.c | 15 +++ hw/xen/xen-bus.c | 7 +-- 2 files changed, 16 insertions(+), 6 deletions(-) -- 2.46.0
[PATCH 2/2] xen: do not use '%ms' scanf specifier
The 'm' parameter used to request auto-allocation of the destination variable
is not supported on FreeBSD, and as such leads to failures to parse.
What's more, the current usage of '%ms' with xs_node_scanf() is pointless, as
it just leads to a double allocation of the same string. Instead use
qemu_xen_xs_read() to read the whole xenstore node.
Fixes: a783f8ad4ec9 ('xen: add a mechanism to automatically create
XenDevice-s...')
Fixes: 9b7737469080 ('hw/xen: update Xen console to XenDevice model')
Signed-off-by: Roger Pau Monné
---
Cc: Stefano Stabellini
Cc: Anthony PERARD
Cc: Paul Durrant
Cc: "Edgar E. Iglesias"
Cc: "Marc-André Lureau"
Cc: Paolo Bonzini
Cc: xen-de...@lists.xenproject.org
---
hw/char/xen_console.c | 11 +--
hw/xen/xen-bus.c | 7 +--
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index af706c7ef440..18afd214c2f6 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -531,6 +531,7 @@ static void xen_console_device_create(XenBackendInstance
*backend,
const char *name = xen_backend_get_name(backend);
unsigned long number;
char *fe = NULL, *type = NULL, *output = NULL;
+const char *node_path;
char label[32];
XenDevice *xendev = NULL;
XenConsole *con;
@@ -550,7 +551,10 @@ static void xen_console_device_create(XenBackendInstance
*backend,
goto fail;
}
-if (xs_node_scanf(xsh, XBT_NULL, fe, "type", errp, "%ms", &type) != 1) {
+node_path = g_strdup_printf("%s/type", fe);
+type = qemu_xen_xs_read(xsh, XBT_NULL, node_path, NULL);
+g_free(node_path);
+if (!type) {
error_setg(errp, "failed to read console device type: ");
goto fail;
}
@@ -568,7 +572,10 @@ static void xen_console_device_create(XenBackendInstance
*backend,
snprintf(label, sizeof(label), "xencons%ld", number);
-if (xs_node_scanf(xsh, XBT_NULL, fe, "output", NULL, "%ms", &output) == 1)
{
+node_path = g_strdup_printf("%s/output", fe);
+output = qemu_xen_xs_read(xsh, XBT_NULL, node_path, NULL);
+g_free(node_path);
+if (!output) {
/*
* FIXME: sure we want to support implicit
* muxed monitors here?
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index adfc4efad035..9be807649e77 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -142,6 +142,7 @@ again:
opts = qdict_new();
for (i = 0; i < n; i++) {
+const char *node_path;
char *val;
/*
@@ -156,8 +157,10 @@ again:
!strcmp(key[i], "hotplug-status"))
continue;
-if (xs_node_scanf(xenbus->xsh, tid, path, key[i], NULL, "%ms",
- &val) == 1) {
+node_path = g_strdup_printf("%s/%s", path, key[i]);
+val = qemu_xen_xs_read(xenbus->xsh, tid, node_path, NULL);
+g_free(node_path);
+if (val) {
qdict_put_str(opts, key[i], val);
free(val);
}
--
2.46.0
[PATCH 1/2] xen/console: fix error handling in xen_console_device_create()
The usage of error_prepend() in some of the error contexts of
xen_console_device_create() is incorrect, as `errp` hasn't been initialized.
This leads to the following segmentation fault on error paths resulting from
xenstore reads:
Program terminated with signal SIGSEGV, Segmentation fault.
Address not mapped to object.
fmt=0x15c4dfeade42 "failed to read console device type: ",
ap=0x15cd0165ab50)
at ../qemu-xen-dir-remote/util/error.c:142
142 g_string_append(newmsg, (*errp)->msg);
[...]
(gdb) bt
(errp=0x15cd0165ae10, fmt=0x15c4dfeade42 "failed to read console device
type: ", ap=0x15cd0165ab50) at ../qemu-xen-dir-remote/util/error.c:142
(errp=0x15cd0165ae10, fmt=0x15c4dfeade42 "failed to read console device
type: ")
at ../qemu-xen-dir-remote/util/error.c:152
(backend=0x43944de00660, opts=0x43944c929000, errp=0x15cd0165ae10)
at ../qemu-xen-dir-remote/hw/char/xen_console.c:555
Replace usages of error_prepend() with error_setg() where appropriate.
Fixes: 9b7737469080 ('hw/xen: update Xen console to XenDevice model')
Signed-off-by: Roger Pau Monné
---
Cc: Stefano Stabellini
Cc: Anthony PERARD
Cc: Paul Durrant
Cc: "Edgar E. Iglesias"
Cc: "Marc-André Lureau"
Cc: Paolo Bonzini
Cc: xen-de...@lists.xenproject.org
---
hw/char/xen_console.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index ef0c2912efa1..af706c7ef440 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -551,7 +551,7 @@ static void xen_console_device_create(XenBackendInstance
*backend,
}
if (xs_node_scanf(xsh, XBT_NULL, fe, "type", errp, "%ms", &type) != 1) {
-error_prepend(errp, "failed to read console device type: ");
+error_setg(errp, "failed to read console device type: ");
goto fail;
}
@@ -582,7 +582,7 @@ static void xen_console_device_create(XenBackendInstance
*backend,
} else if (number) {
cd = serial_hd(number);
if (!cd) {
-error_prepend(errp, "console: No serial device #%ld found: ",
+error_setg(errp, "console: No serial device #%ld found: ",
number);
goto fail;
}
--
2.46.0
[PATCH v2 2/2] xen: do not use '%ms' scanf specifier
The 'm' parameter used to request auto-allocation of the destination variable
is not supported on FreeBSD, and as such leads to failures to parse.
What's more, the current usage of '%ms' with xs_node_scanf() is pointless, as
it just leads to a double allocation of the same string. Instead use
xs_node_read() to read the whole xenstore node.
Fixes: a783f8ad4ec9 ('xen: add a mechanism to automatically create
XenDevice-s...')
Fixes: 9b7737469080 ('hw/xen: update Xen console to XenDevice model')
Signed-off-by: Roger Pau Monné
---
Changes since v2:
- New version of xs_node_read().
- Fix usage of %ms in xen-block.c
Changes since v1:
- Introduce xs_node_read() helper.
- Merge with errp fixes.
---
Cc: Stefano Stabellini
Cc: Anthony PERARD
Cc: Paul Durrant
Cc: "Edgar E. Iglesias"
Cc: Kevin Wolf
Cc: Hanna Reitz
Cc: "Marc-André Lureau"
Cc: Paolo Bonzini
Cc: xen-de...@lists.xenproject.org
Cc: qemu-bl...@nongnu.org
---
hw/block/xen-block.c | 3 ++-
hw/char/xen_console.c| 6 --
hw/xen/xen-bus.c | 14 --
include/hw/xen/xen-bus.h | 1 +
4 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 306d38927cf4..034a18b70e28 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -239,7 +239,8 @@ static void xen_block_connect(XenDevice *xendev, Error
**errp)
return;
}
-if (xen_device_frontend_scanf(xendev, "protocol", "%ms", &str) != 1) {
+str = xen_device_frontend_read(xendev, "protocol");
+if (!str) {
/* x86 defaults to the 32-bit protocol even for 64-bit guests. */
if (object_dynamic_cast(OBJECT(qdev_get_machine()), "x86-machine")) {
protocol = BLKIF_PROTOCOL_X86_32;
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index ef0c2912efa1..989e75fef88f 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -550,7 +550,8 @@ static void xen_console_device_create(XenBackendInstance
*backend,
goto fail;
}
-if (xs_node_scanf(xsh, XBT_NULL, fe, "type", errp, "%ms", &type) != 1) {
+type = xs_node_read(xsh, XBT_NULL, NULL, errp, "%s/%s", fe, "type");
+if (!type) {
error_prepend(errp, "failed to read console device type: ");
goto fail;
}
@@ -568,7 +569,8 @@ static void xen_console_device_create(XenBackendInstance
*backend,
snprintf(label, sizeof(label), "xencons%ld", number);
-if (xs_node_scanf(xsh, XBT_NULL, fe, "output", NULL, "%ms", &output) == 1)
{
+output = xs_node_read(xsh, XBT_NULL, NULL, errp, "%s/%s", fe, "output");
+if (output) {
/*
* FIXME: sure we want to support implicit
* muxed monitors here?
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index adfc4efad035..85b92cded4e2 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -156,8 +156,8 @@ again:
!strcmp(key[i], "hotplug-status"))
continue;
-if (xs_node_scanf(xenbus->xsh, tid, path, key[i], NULL, "%ms",
- &val) == 1) {
+val = xs_node_read(xenbus->xsh, tid, NULL, NULL, "%s/%s", path,
key[i]);
+if (val) {
qdict_put_str(opts, key[i], val);
free(val);
}
@@ -650,6 +650,16 @@ int xen_device_frontend_scanf(XenDevice *xendev, const
char *key,
return rc;
}
+char *xen_device_frontend_read(XenDevice *xendev, const char *key)
+{
+XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev)));
+
+g_assert(xenbus->xsh);
+
+return xs_node_read(xenbus->xsh, XBT_NULL, NULL, NULL, "%s/%s",
+xendev->frontend_path, key);;
+}
+
static void xen_device_frontend_set_state(XenDevice *xendev,
enum xenbus_state state,
bool publish)
diff --git a/include/hw/xen/xen-bus.h b/include/hw/xen/xen-bus.h
index 38d40afa3798..2adb2af83919 100644
--- a/include/hw/xen/xen-bus.h
+++ b/include/hw/xen/xen-bus.h
@@ -91,6 +91,7 @@ void xen_device_frontend_printf(XenDevice *xendev, const char
*key,
int xen_device_frontend_scanf(XenDevice *xendev, const char *key,
const char *fmt, ...)
G_GNUC_SCANF(3, 4);
+char *xen_device_frontend_read(XenDevice *xendev, const char *key);
void xen_device_set_max_grant_refs(XenDevice *xendev, unsigned int nr_refs,
Error **errp);
--
2.46.0
[PATCH v2 1/2] hw/xen: Add xs_node_read() helper function
From: David Woodhouse
This returns the full contents of the node, having created the node path
from the printf-style format string provided in its arguments.
This will save various callers from having to do so for themselves (and
from using xs_node_scanf() with the non-portable %ms format string.
Signed-off-by: David Woodhouse
[remove double newline and constify trace parameters]
Signed-off-by: Roger Pau Monné
---
Cc: Stefano Stabellini
Cc: Anthony PERARD
Cc: Paul Durrant
Cc: "Edgar E. Iglesias"
Cc: xen-de...@lists.xenproject.org
---
hw/xen/trace-events | 1 +
hw/xen/xen-bus-helper.c | 22 ++
include/hw/xen/xen-bus-helper.h | 4
3 files changed, 27 insertions(+)
diff --git a/hw/xen/trace-events b/hw/xen/trace-events
index a07fe41c6d3b..461dee7b239f 100644
--- a/hw/xen/trace-events
+++ b/hw/xen/trace-events
@@ -39,6 +39,7 @@ xs_node_create(const char *node) "%s"
xs_node_destroy(const char *node) "%s"
xs_node_vprintf(char *path, char *value) "%s %s"
xs_node_vscanf(char *path, char *value) "%s %s"
+xs_node_read(const char *path, const char *value) "%s %s"
xs_node_watch(char *path) "%s"
xs_node_unwatch(char *path) "%s"
diff --git a/hw/xen/xen-bus-helper.c b/hw/xen/xen-bus-helper.c
index b2b2cc9c5d5e..0fba7946c55e 100644
--- a/hw/xen/xen-bus-helper.c
+++ b/hw/xen/xen-bus-helper.c
@@ -142,6 +142,28 @@ int xs_node_scanf(struct qemu_xs_handle *h,
xs_transaction_t tid,
return rc;
}
+char *xs_node_read(struct qemu_xs_handle *h, xs_transaction_t tid,
+ unsigned int *len, Error **errp,
+ const char *node_fmt, ...)
+{
+char *path, *value;
+va_list ap;
+
+va_start(ap, node_fmt);
+path = g_strdup_vprintf(node_fmt, ap);
+va_end(ap);
+
+value = qemu_xen_xs_read(h, tid, path, len);
+trace_xs_node_read(path, value);
+if (!value) {
+error_setg_errno(errp, errno, "failed to read from '%s'", path);
+}
+
+g_free(path);
+
+return value;
+}
+
struct qemu_xs_watch *xs_node_watch(struct qemu_xs_handle *h, const char *node,
const char *key, xs_watch_fn fn,
void *opaque, Error **errp)
diff --git a/include/hw/xen/xen-bus-helper.h b/include/hw/xen/xen-bus-helper.h
index d8dcc2f0107d..6478d25be5e6 100644
--- a/include/hw/xen/xen-bus-helper.h
+++ b/include/hw/xen/xen-bus-helper.h
@@ -37,6 +37,10 @@ int xs_node_scanf(struct qemu_xs_handle *h,
xs_transaction_t tid,
const char *node, const char *key, Error **errp,
const char *fmt, ...)
G_GNUC_SCANF(6, 7);
+char *xs_node_read(struct qemu_xs_handle *h, xs_transaction_t tid,
+ unsigned int *len, Error **errp,
+ const char *node_fmt, ...)
+G_GNUC_PRINTF(5, 6);
/* Watch node/key unless node is empty, in which case watch key */
struct qemu_xs_watch *xs_node_watch(struct qemu_xs_handle *h, const char *node,
--
2.46.0
[PATCH v2 0/2] xen: error handling and FreeBSD compatibility fixes
Hello, First patch from David introduces a new helper to fetch xenstore nodes, while second patch removes the usage of scanf related functions with the "%ms" format specifier, as it's not supported by the FreeBSD scanf libc implementation. Thanks, Roger. David Woodhouse (1): hw/xen: Add xs_node_read() helper function Roger Pau Monné (1): xen: do not use '%ms' scanf specifier hw/block/xen-block.c| 3 ++- hw/char/xen_console.c | 14 -- hw/xen/trace-events | 1 + hw/xen/xen-bus-helper.c | 22 ++ hw/xen/xen-bus.c| 14 -- include/hw/xen/xen-bus-helper.h | 4 include/hw/xen/xen-bus.h| 1 + 7 files changed, 50 insertions(+), 9 deletions(-) -- 2.46.0
[PATCH v2] xen: do not use '%ms' scanf specifier
The 'm' parameter used to request auto-allocation of the destination variable
is not supported on FreeBSD, and as such leads to failures to parse.
What's more, the current usage of '%ms' with xs_node_scanf() is pointless, as
it just leads to a double allocation of the same string. Instead introduce and
use xs_node_read() to read the whole xenstore node.
Additionally fix the errors paths to not use error_prepend(), as that could
lead to a segmentation fault because xs_node_scanf() only initializes errp when
returning a value smaller than 0:
Program terminated with signal SIGSEGV, Segmentation fault.
Address not mapped to object.
fmt=0x15c4dfeade42 "failed to read console device type: ",
ap=0x15cd0165ab50)
at ../qemu-xen-dir-remote/util/error.c:142
142 g_string_append(newmsg, (*errp)->msg);
[...]
(gdb) bt
(errp=0x15cd0165ae10, fmt=0x15c4dfeade42 "failed to read console device
type: ", ap=0x15cd0165ab50) at ../qemu-xen-dir-remote/util/error.c:142
(errp=0x15cd0165ae10, fmt=0x15c4dfeade42 "failed to read console device
type: ")
at ../qemu-xen-dir-remote/util/error.c:152
(backend=0x43944de00660, opts=0x43944c929000, errp=0x15cd0165ae10)
at ../qemu-xen-dir-remote/hw/char/xen_console.c:555
With the change to use xs_node_read() instead of xs_node_scanf() errp will
never be initialized, and hence error_setg() should be used unconditionally.
Fixes: a783f8ad4ec9 ('xen: add a mechanism to automatically create
XenDevice-s...')
Fixes: 9b7737469080 ('hw/xen: update Xen console to XenDevice model')
Signed-off-by: Roger Pau Monné
---
Changes since v1:
- Introduce xs_node_read() helper.
- Merge with errp fixes.
---
Cc: Stefano Stabellini
Cc: Anthony PERARD
Cc: Paul Durrant
Cc: "Edgar E. Iglesias"
Cc: "Marc-André Lureau"
Cc: Paolo Bonzini
Cc: xen-de...@lists.xenproject.org
---
hw/char/xen_console.c | 12 +++-
hw/xen/xen-bus-helper.c | 12
hw/xen/xen-bus.c| 4 ++--
include/hw/xen/xen-bus-helper.h | 2 ++
4 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index ef0c2912efa1..a3591df1af2e 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -550,8 +550,9 @@ static void xen_console_device_create(XenBackendInstance
*backend,
goto fail;
}
-if (xs_node_scanf(xsh, XBT_NULL, fe, "type", errp, "%ms", &type) != 1) {
-error_prepend(errp, "failed to read console device type: ");
+type = xs_node_read(xsh, XBT_NULL, fe, "type");
+if (!type) {
+error_setg(errp, "failed to read console device type");
goto fail;
}
@@ -568,7 +569,8 @@ static void xen_console_device_create(XenBackendInstance
*backend,
snprintf(label, sizeof(label), "xencons%ld", number);
-if (xs_node_scanf(xsh, XBT_NULL, fe, "output", NULL, "%ms", &output) == 1)
{
+output = xs_node_read(xsh, XBT_NULL, fe, "output");
+if (!output) {
/*
* FIXME: sure we want to support implicit
* muxed monitors here?
@@ -582,8 +584,8 @@ static void xen_console_device_create(XenBackendInstance
*backend,
} else if (number) {
cd = serial_hd(number);
if (!cd) {
-error_prepend(errp, "console: No serial device #%ld found: ",
- number);
+error_setg(errp, "console: No serial device #%ld found",
+ number);
goto fail;
}
} else {
diff --git a/hw/xen/xen-bus-helper.c b/hw/xen/xen-bus-helper.c
index b2b2cc9c5d5e..115c5b1a8ce8 100644
--- a/hw/xen/xen-bus-helper.c
+++ b/hw/xen/xen-bus-helper.c
@@ -142,6 +142,18 @@ int xs_node_scanf(struct qemu_xs_handle *h,
xs_transaction_t tid,
return rc;
}
+char *xs_node_read(struct qemu_xs_handle *h, xs_transaction_t tid,
+ const char *node, const char *key)
+{
+char *path = (strlen(node) != 0) ? g_strdup_printf("%s/%s", node, key)
+ : g_strdup(key);
+char *value = qemu_xen_xs_read(h, tid, path, NULL);
+
+g_free(path);
+
+return value;
+}
+
struct qemu_xs_watch *xs_node_watch(struct qemu_xs_handle *h, const char *node,
const char *key, xs_watch_fn fn,
void *opaque, Error **errp)
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index adfc4efad035..aaede5d9ecb2 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -156,8 +156,8 @@ again:
!strcmp(key[i], "hotplug-status"))
continue;
-if (xs_node_scanf(xenbus->xsh, tid, path, key[i], NULL, "%ms",
- &val) == 1) {
+val = xs_node_read(xenbus->xsh, tid, path, key[i]);
+if (val) {
qdict_put_str(opts, key[i], val);
free(val);
}
diff --git a/include/hw/xen/xen-bus-helper.h b/include/hw/xen/xen-bus-helper.h
index d8dcc2f0107d..79f0787332ed 100644
--- a/include/h
