* [Qemu-devel] [PATCH v4 0/<VARIOUS>] Begin to disentangle libxenctrl and provide some stable libraries
@ 2015-10-21 15:22 Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 0/9] " Ian Campbell
0 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2015-10-21 15:22 UTC (permalink / raw)
To: xen-devel
Cc: Wei Liu, Stefano Stabellini, Ian Jackson, qemu-devel,
minios-devel, samuel.thibault, Roger Pau Monne
In <1431963008.4944.80.camel@citrix.com> I proposed stabilising some
parts of the libxenctrl API/ABI by disaggregating into separate
libraries.
This is v4 of that set of series against:
xen
qemu-xen
qemu-xen-traditional
mini-os
NB: Samuel+minios-devel will only get the mini-os side and Stefano+qemu
-devel the qemu-xen side.
The code in for all repos can be found in:
git://xenbits.xen.org/people/ianc/libxenctrl-split/xen.git v4
git://xenbits.xen.org/people/ianc/libxenctrl-split/qemu-xen.git v4
git://xenbits.xen.org/people/ianc/libxenctrl-split/qemu-xen-traditional.git v4
git://xenbits.xen.org/people/ianc/libxenctrl-split/mini-os.git v4
The tip of the xen.git branch contains an extra patch hacking Config.mk
to point to all the others above, which should get the correct things for
the HEAD of the branch, but not further back in time.
The new libraries here are:
* libxentoollog: Common logging infrastructure
* libxenevtchn: Userspace access to evtchns (via /dev/xen/evtchn etc)
* libxengnttab: Userspace access to grant tables (via /dev/xen/gnt??? etc)
* libxencall: Making hypercalls (i.e. the IOCTL_PRIVCMD_HYPERCALL type
functionality)
* libxenforeignmemory: Privileged mappings of foreign memory
(IOCTL_PRIVCMD_MMAP et al)
The first three were actually pretty distinct within libxenctrl already and
have not changed in quite some time.
Although the other two are somewhat new they are based on top of long
standing stable ioctls, which gives me some confidence.
Nonetheless I would appreciate extra review of at least the interface
headers of all of these with a particular eye to the suitability of these
interfaces being maintained in an ABI (_B_, not _P_) stable way going
forward.
Still to come would be libraries for specific out of tree purposes
(device model, kexec), which would be adding new library at the same
level as libxc I think, rather than underneath, i.e. also using the
libraries split out here, but hopefully not libxenctrl itself.
The new libraries use linker version-scripts to hopefully make future
ABI changes be possible in a compatible way.
Since last time I have:
* Addressed various review comments:
* Addressed feedback from Stefano on the qemu-xen series (and this
version now goes to qemu-devel too)
* Switched the foreign mapping interfaces to use size_t for the number
of pages.
* Fixed the callers of xenforeignmemory_unmap (should have been pages,
but everywhere was passing bytes like the previous munmap case)
* HACK patch in xen.git now updates Config.mk instead of .config
The whole thing has been build tested on Linux (incl stubdoms), and on
FreeBSD. I have runtime tested older versions on Linux but my test boxes
are currently in some netherworld having been moved to a different colo.
Neither NetBSD nor Solaris have been tested at all. It's certainly not
impossible that I've not got the #includes in the new files quite right.
http://xenbits.xen.org/people/ianc/libxenctrl-split/v4.html is the document
I've been using to try and track what I'm doing. It may not be all that
useful. The history of it is in the v4-with-doc branch of the xen.git
linked to above.
Ian.
_______________________________________________
Minios-devel mailing list
Minios-devel@lists.xenproject.org
http://lists.xenproject.org/cgi-bin/mailman/listinfo/minios-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH QEMU-XEN v4 0/9] Begin to disentangle libxenctrl and provide some stable libraries
2015-10-21 15:22 [Qemu-devel] [PATCH v4 0/<VARIOUS>] Begin to disentangle libxenctrl and provide some stable libraries Ian Campbell
@ 2015-10-21 15:23 ` Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 1/9] xen_console: correctly cleanup primary console on teardown Ian Campbell
` (8 more replies)
0 siblings, 9 replies; 25+ messages in thread
From: Ian Campbell @ 2015-10-21 15:23 UTC (permalink / raw)
To: ian.jackson, wei.liu2, xen-devel
Cc: Ian Campbell, qemu-devel, stefano.stabellini
We intend to stabilise some parts of the libxenctrl interface by
splitting out some functionality into separate stable libraries.
This is the qemu-xen part of the first phase of that change.
This mail is (or is intended to be) a reply to a "0/<VARIOUS>"
super-intro mail covering all of the related patch series and which
contains more details.
Ian Campbell (9):
xen_console: correctly cleanup primary console on teardown.
xen: Switch to libxenevtchn interface for compat shims.
xen: Switch to libxengnttab interface for compat shims.
xen: Switch uses of xc_map_foreign_range into xc_map_foreign_bulk
xen: Switch uses of xc_map_foreign_pages into xc_map_foreign_bulk
xen: Switch uses of xc_map_foreign_bulk to use libxenforeignmemory
API.
xen: Use stable library interfaces when they are available.
xen: domainbuild: reopen libxenctrl interface after forking for domain
watcher.
xen: make it possible to build without the Xen PV domain builder
configure | 72 +++++++++++++++++++++++
hw/block/xen_disk.c | 38 ++++++------
hw/char/xen_console.c | 20 +++----
hw/display/xenfb.c | 22 ++++---
hw/net/xen_nic.c | 16 ++---
hw/xen/xen_backend.c | 44 +++++++-------
hw/xenpv/Makefile.objs | 4 +-
hw/xenpv/xen_domainbuild.c | 9 ++-
hw/xenpv/xen_machine_pv.c | 14 +++--
include/hw/xen/xen_backend.h | 5 +-
include/hw/xen/xen_common.h | 135 +++++++++++++++++++++++++++++++++----------
xen-common.c | 6 ++
xen-hvm.c | 53 +++++++++--------
xen-mapcache.c | 6 +-
14 files changed, 309 insertions(+), 135 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH QEMU-XEN v4 1/9] xen_console: correctly cleanup primary console on teardown.
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 0/9] " Ian Campbell
@ 2015-10-21 15:23 ` Ian Campbell
2015-10-22 16:46 ` Stefano Stabellini
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 2/9] xen: Switch to libxenevtchn interface for compat shims Ian Campbell
` (7 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2015-10-21 15:23 UTC (permalink / raw)
To: ian.jackson, wei.liu2, xen-devel
Cc: Ian Campbell, qemu-devel, stefano.stabellini
All of the work in con_disconnect applies to the primary console case
(when xendev->dev is NULL). Therefore remove the early check and bail
and allow it to fall through. All of the existing code is correctly
conditional already.
The ->dev and ->gnttabdev handles are either both set or neither. For
consistency with con_initialise() with to the former here too.
With this con_initialise and con_disconnect now mirror each other.
Fix up a hard tab in the function while editing.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: New patch based on feedback to "xen: Switch uses of
xc_map_foreign_bulk to use libxenforeignmemory API."
---
hw/char/xen_console.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index eb7f450..63ade33 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -265,9 +265,6 @@ static void con_disconnect(struct XenDevice *xendev)
{
struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
- if (!xendev->dev) {
- return;
- }
if (con->chr) {
qemu_chr_add_handlers(con->chr, NULL, NULL, NULL, NULL);
qemu_chr_fe_release(con->chr);
@@ -275,12 +272,12 @@ static void con_disconnect(struct XenDevice *xendev)
xen_be_unbind_evtchn(&con->xendev);
if (con->sring) {
- if (!xendev->gnttabdev) {
+ if (!xendev->dev) {
munmap(con->sring, XC_PAGE_SIZE);
} else {
xc_gnttab_munmap(xendev->gnttabdev, con->sring, 1);
}
- con->sring = NULL;
+ con->sring = NULL;
}
}
--
2.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH QEMU-XEN v4 2/9] xen: Switch to libxenevtchn interface for compat shims.
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 0/9] " Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 1/9] xen_console: correctly cleanup primary console on teardown Ian Campbell
@ 2015-10-21 15:23 ` Ian Campbell
2015-10-23 11:06 ` Stefano Stabellini
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 3/9] xen: Switch to libxengnttab " Ian Campbell
` (6 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2015-10-21 15:23 UTC (permalink / raw)
To: ian.jackson, wei.liu2, xen-devel
Cc: Ian Campbell, qemu-devel, stefano.stabellini
In Xen 4.7 we are refactoring parts libxenctrl into a number of
separate libraries which will provide backward and forward API and ABI
compatiblity.
One such library will be libxenevtchn which provides access to event
channels.
In preparation for this switch the compatibility layer in xen_common.h
(which support building with older versions of Xen) to use what will
be the new library API. This means that the evtchn shim will disappear
for versions of Xen which include libxenevtchn.
To simplify things for the <= 4.0.0 support we wrap the int fd in a
malloc(sizeof int) such that the handle is always a pointer. This
leads to less typedef headaches and the need for
XC_HANDLER_INITIAL_VALUE etc for these interfaces.
Build tested with 4.0 and 4.5.
Note that this patch does not add any support for actually using
libxenevtchn, it just adjusts the existing shims.
Note that xc_evtchn_alloc_unbound functionality remains in libxenctrl,
since that functionality is not exposed by /dev/xen/evtchn.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: Ran checkpatch, fixed all errors
Allocate correct size for handle (i.e. not size of the ptr)
---
hw/xen/xen_backend.c | 31 ++++++++++++++++---------------
include/hw/xen/xen_backend.h | 2 +-
include/hw/xen/xen_common.h | 44 ++++++++++++++++++++++++++++++++++----------
xen-hvm.c | 25 +++++++++++++------------
4 files changed, 64 insertions(+), 38 deletions(-)
diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index b2cb22b..342ec9b 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -243,19 +243,19 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev,
xendev->debug = debug;
xendev->local_port = -1;
- xendev->evtchndev = xen_xc_evtchn_open(NULL, 0);
- if (xendev->evtchndev == XC_HANDLER_INITIAL_VALUE) {
+ xendev->evtchndev = xenevtchn_open(NULL, 0);
+ if (xendev->evtchndev == NULL) {
xen_be_printf(NULL, 0, "can't open evtchn device\n");
g_free(xendev);
return NULL;
}
- fcntl(xc_evtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
+ fcntl(xenevtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
xendev->gnttabdev = xen_xc_gnttab_open(NULL, 0);
if (xendev->gnttabdev == XC_HANDLER_INITIAL_VALUE) {
xen_be_printf(NULL, 0, "can't open gnttab device\n");
- xc_evtchn_close(xendev->evtchndev);
+ xenevtchn_close(xendev->evtchndev);
g_free(xendev);
return NULL;
}
@@ -306,8 +306,8 @@ static struct XenDevice *xen_be_del_xendev(int dom, int dev)
g_free(xendev->fe);
}
- if (xendev->evtchndev != XC_HANDLER_INITIAL_VALUE) {
- xc_evtchn_close(xendev->evtchndev);
+ if (xendev->evtchndev != NULL) {
+ xenevtchn_close(xendev->evtchndev);
}
if (xendev->gnttabdev != XC_HANDLER_INITIAL_VALUE) {
xc_gnttab_close(xendev->gnttabdev);
@@ -691,13 +691,14 @@ static void xen_be_evtchn_event(void *opaque)
struct XenDevice *xendev = opaque;
evtchn_port_t port;
- port = xc_evtchn_pending(xendev->evtchndev);
+ port = xenevtchn_pending(xendev->evtchndev);
if (port != xendev->local_port) {
- xen_be_printf(xendev, 0, "xc_evtchn_pending returned %d (expected %d)\n",
+ xen_be_printf(xendev, 0,
+ "xenevtchn_pending returned %d (expected %d)\n",
port, xendev->local_port);
return;
}
- xc_evtchn_unmask(xendev->evtchndev, port);
+ xenevtchn_unmask(xendev->evtchndev, port);
if (xendev->ops->event) {
xendev->ops->event(xendev);
@@ -742,14 +743,14 @@ int xen_be_bind_evtchn(struct XenDevice *xendev)
if (xendev->local_port != -1) {
return 0;
}
- xendev->local_port = xc_evtchn_bind_interdomain
+ xendev->local_port = xenevtchn_bind_interdomain
(xendev->evtchndev, xendev->dom, xendev->remote_port);
if (xendev->local_port == -1) {
- xen_be_printf(xendev, 0, "xc_evtchn_bind_interdomain failed\n");
+ xen_be_printf(xendev, 0, "xenevtchn_bind_interdomain failed\n");
return -1;
}
xen_be_printf(xendev, 2, "bind evtchn port %d\n", xendev->local_port);
- qemu_set_fd_handler(xc_evtchn_fd(xendev->evtchndev),
+ qemu_set_fd_handler(xenevtchn_fd(xendev->evtchndev),
xen_be_evtchn_event, NULL, xendev);
return 0;
}
@@ -759,15 +760,15 @@ void xen_be_unbind_evtchn(struct XenDevice *xendev)
if (xendev->local_port == -1) {
return;
}
- qemu_set_fd_handler(xc_evtchn_fd(xendev->evtchndev), NULL, NULL, NULL);
- xc_evtchn_unbind(xendev->evtchndev, xendev->local_port);
+ qemu_set_fd_handler(xenevtchn_fd(xendev->evtchndev), NULL, NULL, NULL);
+ xenevtchn_unbind(xendev->evtchndev, xendev->local_port);
xen_be_printf(xendev, 2, "unbind evtchn port %d\n", xendev->local_port);
xendev->local_port = -1;
}
int xen_be_send_notify(struct XenDevice *xendev)
{
- return xc_evtchn_notify(xendev->evtchndev, xendev->local_port);
+ return xenevtchn_notify(xendev->evtchndev, xendev->local_port);
}
/*
diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
index 3b4125e..a90314f 100644
--- a/include/hw/xen/xen_backend.h
+++ b/include/hw/xen/xen_backend.h
@@ -46,7 +46,7 @@ struct XenDevice {
int remote_port;
int local_port;
- XenEvtchn evtchndev;
+ xenevtchn_handle *evtchndev;
XenGnttab gnttabdev;
struct XenDevOps *ops;
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 5923290..4dc2ee6 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -39,17 +39,38 @@ static inline void *xc_map_foreign_bulk(int xc_handle, uint32_t dom, int prot,
#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 410
typedef int XenXC;
-typedef int XenEvtchn;
+typedef int xenevtchn_handle;
typedef int XenGnttab;
# define XC_INTERFACE_FMT "%i"
# define XC_HANDLER_INITIAL_VALUE -1
-static inline XenEvtchn xen_xc_evtchn_open(void *logger,
- unsigned int open_flags)
+static inline xenevtchn_handle *xenevtchn_open(void *logger,
+ unsigned int open_flags)
{
- return xc_evtchn_open();
+ xenevtchn_handle *h = malloc(sizeof(*h));
+ if (!h) {
+ return NULL;
+ }
+ *h = xc_evtchn_open();
+ if (*h == -1) {
+ free(h);
+ h = NULL;
+ }
+ return h;
}
+static inline int xenevtchn_close(xenevtchn_handle *h)
+{
+ int rc = xc_evtchn_close(*h);
+ free(h);
+ return rc;
+}
+#define xenevtchn_fd(h) xc_evtchn_fd(*h)
+#define xenevtchn_pending(h) xc_evtchn_pending(*h)
+#define xenevtchn_notify(h, p) xc_evtchn_notify(*h, p)
+#define xenevtchn_bind_interdomain(h, d, p) xc_evtchn_bind_interdomain(*h, d, p)
+#define xenevtchn_unmask(h, p) xc_evtchn_unmask(*h, p)
+#define xenevtchn_unbind(h, p) xc_evtchn_unmask(*h, p)
static inline XenGnttab xen_xc_gnttab_open(void *logger,
unsigned int open_flags)
@@ -108,17 +129,20 @@ static inline void xs_close(struct xs_handle *xsh)
#else
typedef xc_interface *XenXC;
-typedef xc_evtchn *XenEvtchn;
+typedef xc_evtchn xenevtchn_handle;
typedef xc_gnttab *XenGnttab;
# define XC_INTERFACE_FMT "%p"
# define XC_HANDLER_INITIAL_VALUE NULL
-static inline XenEvtchn xen_xc_evtchn_open(void *logger,
- unsigned int open_flags)
-{
- return xc_evtchn_open(logger, open_flags);
-}
+#define xenevtchn_open(l, f) xc_evtchn_open(l, f);
+#define xenevtchn_close(h) xc_evtchn_close(h)
+#define xenevtchn_fd(h) xc_evtchn_fd(h)
+#define xenevtchn_pending(h) xc_evtchn_pending(h)
+#define xenevtchn_notify(h, p) xc_evtchn_notify(h, p)
+#define xenevtchn_bind_interdomain(h, d, p) xc_evtchn_bind_interdomain(h, d, p)
+#define xenevtchn_unmask(h, p) xc_evtchn_unmask(h, p)
+#define xenevtchn_unbind(h, p) xc_evtchn_unbind(h, p)
static inline XenGnttab xen_xc_gnttab_open(void *logger,
unsigned int open_flags)
diff --git a/xen-hvm.c b/xen-hvm.c
index 3a7fd58..4470d58 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -109,7 +109,7 @@ typedef struct XenIOState {
/* evtchn local port for buffered io */
evtchn_port_t bufioreq_local_port;
/* the evtchn fd for polling */
- XenEvtchn xce_handle;
+ xenevtchn_handle *xce_handle;
/* which vcpu we are serving */
int send_vcpu;
@@ -709,7 +709,7 @@ static ioreq_t *cpu_get_ioreq(XenIOState *state)
int i;
evtchn_port_t port;
- port = xc_evtchn_pending(state->xce_handle);
+ port = xenevtchn_pending(state->xce_handle);
if (port == state->bufioreq_local_port) {
timer_mod(state->buffered_io_timer,
BUFFER_IO_MAX_DELAY + qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
@@ -728,7 +728,7 @@ static ioreq_t *cpu_get_ioreq(XenIOState *state)
}
/* unmask the wanted port again */
- xc_evtchn_unmask(state->xce_handle, port);
+ xenevtchn_unmask(state->xce_handle, port);
/* get the io packet from shared memory */
state->send_vcpu = i;
@@ -1014,7 +1014,7 @@ static void handle_buffered_io(void *opaque)
BUFFER_IO_MAX_DELAY + qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
} else {
timer_del(state->buffered_io_timer);
- xc_evtchn_unmask(state->xce_handle, state->bufioreq_local_port);
+ xenevtchn_unmask(state->xce_handle, state->bufioreq_local_port);
}
}
@@ -1058,7 +1058,8 @@ static void cpu_handle_ioreq(void *opaque)
}
req->state = STATE_IORESP_READY;
- xc_evtchn_notify(state->xce_handle, state->ioreq_local_port[state->send_vcpu]);
+ xenevtchn_notify(state->xce_handle,
+ state->ioreq_local_port[state->send_vcpu]);
}
}
@@ -1066,8 +1067,8 @@ static void xen_main_loop_prepare(XenIOState *state)
{
int evtchn_fd = -1;
- if (state->xce_handle != XC_HANDLER_INITIAL_VALUE) {
- evtchn_fd = xc_evtchn_fd(state->xce_handle);
+ if (state->xce_handle != NULL) {
+ evtchn_fd = xenevtchn_fd(state->xce_handle);
}
state->buffered_io_timer = timer_new_ms(QEMU_CLOCK_REALTIME, handle_buffered_io,
@@ -1105,7 +1106,7 @@ static void xen_exit_notifier(Notifier *n, void *data)
{
XenIOState *state = container_of(n, XenIOState, exit);
- xc_evtchn_close(state->xce_handle);
+ xenevtchn_close(state->xce_handle);
xs_daemon_close(state->xenstore);
}
@@ -1174,8 +1175,8 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
state = g_malloc0(sizeof (XenIOState));
- state->xce_handle = xen_xc_evtchn_open(NULL, 0);
- if (state->xce_handle == XC_HANDLER_INITIAL_VALUE) {
+ state->xce_handle = xenevtchn_open(NULL, 0);
+ if (state->xce_handle == NULL) {
perror("xen: event channel open");
return -1;
}
@@ -1255,7 +1256,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
/* FIXME: how about if we overflow the page here? */
for (i = 0; i < max_cpus; i++) {
- rc = xc_evtchn_bind_interdomain(state->xce_handle, xen_domid,
+ rc = xenevtchn_bind_interdomain(state->xce_handle, xen_domid,
xen_vcpu_eport(state->shared_page, i));
if (rc == -1) {
fprintf(stderr, "shared evtchn %d bind error %d\n", i, errno);
@@ -1264,7 +1265,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
state->ioreq_local_port[i] = rc;
}
- rc = xc_evtchn_bind_interdomain(state->xce_handle, xen_domid,
+ rc = xenevtchn_bind_interdomain(state->xce_handle, xen_domid,
bufioreq_evtchn);
if (rc == -1) {
fprintf(stderr, "buffered evtchn bind error %d\n", errno);
--
2.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH QEMU-XEN v4 3/9] xen: Switch to libxengnttab interface for compat shims.
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 0/9] " Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 1/9] xen_console: correctly cleanup primary console on teardown Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 2/9] xen: Switch to libxenevtchn interface for compat shims Ian Campbell
@ 2015-10-21 15:23 ` Ian Campbell
2015-10-23 11:06 ` Stefano Stabellini
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 4/9] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_bulk Ian Campbell
` (5 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2015-10-21 15:23 UTC (permalink / raw)
To: ian.jackson, wei.liu2, xen-devel
Cc: Ian Campbell, qemu-devel, stefano.stabellini
In Xen 4.7 we are refactoring parts libxenctrl into a number of
separate libraries which will provide backward and forward API and ABI
compatiblity.
One such library will be libxengnttab which provides access to grant
tables.
In preparation for this switch the compatibility layer in xen_common.h
(which support building with older versions of Xen) to use what will
be the new library API. This means that the gnttab shim will disappear
for versions of Xen which include libxengnttab.
To simplify things for the <= 4.0.0 support we wrap the int fd in a
malloc(sizeof int) such that the handle is always a pointer. This
leads to less typedef headaches and the need for
XC_HANDLER_INITIAL_VALUE etc for these interfaces.
Build tested with 4.0 and 4.5.
Note that this patch does not add any support for actually using
libxengnttab, it just adjusts the existing shims.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: Ran checkpatch, fixed all errors
Allocate correct size for handle (i.e. not size of the ptr)
Rebase onto "xen_console: correctly cleanup primary console on
teardown."
---
hw/block/xen_disk.c | 38 ++++++++++++++++++++------------------
hw/char/xen_console.c | 4 ++--
hw/net/xen_nic.c | 16 ++++++++--------
hw/xen/xen_backend.c | 10 +++++-----
include/hw/xen/xen_backend.h | 2 +-
include/hw/xen/xen_common.h | 42 ++++++++++++++++++++++++++++++++----------
6 files changed, 68 insertions(+), 44 deletions(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 21842a0..15413f6 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -172,11 +172,11 @@ 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;
+ xengnttab_handle *gnt = grant->blkdev->xendev.gnttabdev;
- if (xc_gnttab_munmap(gnt, grant->page, 1) != 0) {
+ if (xengnttab_munmap(gnt, grant->page, 1) != 0) {
xen_be_printf(&grant->blkdev->xendev, 0,
- "xc_gnttab_munmap failed: %s\n",
+ "xengnttab_munmap failed: %s\n",
strerror(errno));
}
grant->blkdev->persistent_gnt_count--;
@@ -189,11 +189,11 @@ static void remove_persistent_region(gpointer data, gpointer dev)
{
PersistentRegion *region = data;
struct XenBlkDev *blkdev = dev;
- XenGnttab gnt = blkdev->xendev.gnttabdev;
+ xengnttab_handle *gnt = blkdev->xendev.gnttabdev;
- if (xc_gnttab_munmap(gnt, region->addr, region->num) != 0) {
+ if (xengnttab_munmap(gnt, region->addr, region->num) != 0) {
xen_be_printf(&blkdev->xendev, 0,
- "xc_gnttab_munmap region %p failed: %s\n",
+ "xengnttab_munmap region %p failed: %s\n",
region->addr, strerror(errno));
}
xen_be_printf(&blkdev->xendev, 3,
@@ -328,7 +328,7 @@ err:
static void ioreq_unmap(struct ioreq *ioreq)
{
- XenGnttab gnt = ioreq->blkdev->xendev.gnttabdev;
+ xengnttab_handle *gnt = ioreq->blkdev->xendev.gnttabdev;
int i;
if (ioreq->num_unmap == 0 || ioreq->mapped == 0) {
@@ -338,8 +338,9 @@ static void ioreq_unmap(struct ioreq *ioreq)
if (!ioreq->pages) {
return;
}
- if (xc_gnttab_munmap(gnt, ioreq->pages, ioreq->num_unmap) != 0) {
- xen_be_printf(&ioreq->blkdev->xendev, 0, "xc_gnttab_munmap failed: %s\n",
+ if (xengnttab_munmap(gnt, ioreq->pages, ioreq->num_unmap) != 0) {
+ xen_be_printf(&ioreq->blkdev->xendev, 0,
+ "xengnttab_munmap failed: %s\n",
strerror(errno));
}
ioreq->blkdev->cnt_map -= ioreq->num_unmap;
@@ -349,8 +350,9 @@ static void ioreq_unmap(struct ioreq *ioreq)
if (!ioreq->page[i]) {
continue;
}
- if (xc_gnttab_munmap(gnt, ioreq->page[i], 1) != 0) {
- xen_be_printf(&ioreq->blkdev->xendev, 0, "xc_gnttab_munmap failed: %s\n",
+ if (xengnttab_munmap(gnt, ioreq->page[i], 1) != 0) {
+ xen_be_printf(&ioreq->blkdev->xendev, 0,
+ "xengnttab_munmap failed: %s\n",
strerror(errno));
}
ioreq->blkdev->cnt_map--;
@@ -362,7 +364,7 @@ static void ioreq_unmap(struct ioreq *ioreq)
static int ioreq_map(struct ioreq *ioreq)
{
- XenGnttab gnt = ioreq->blkdev->xendev.gnttabdev;
+ xengnttab_handle *gnt = ioreq->blkdev->xendev.gnttabdev;
uint32_t domids[BLKIF_MAX_SEGMENTS_PER_REQUEST];
uint32_t refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
void *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
@@ -413,7 +415,7 @@ static int ioreq_map(struct ioreq *ioreq)
}
if (batch_maps && new_maps) {
- ioreq->pages = xc_gnttab_map_grant_refs
+ ioreq->pages = xengnttab_map_grant_refs
(gnt, new_maps, domids, refs, ioreq->prot);
if (ioreq->pages == NULL) {
xen_be_printf(&ioreq->blkdev->xendev, 0,
@@ -429,7 +431,7 @@ static int ioreq_map(struct ioreq *ioreq)
ioreq->blkdev->cnt_map += new_maps;
} else if (new_maps) {
for (i = 0; i < new_maps; i++) {
- ioreq->page[i] = xc_gnttab_map_grant_ref
+ ioreq->page[i] = xengnttab_map_grant_ref
(gnt, domids[i], refs[i], ioreq->prot);
if (ioreq->page[i] == NULL) {
xen_be_printf(&ioreq->blkdev->xendev, 0,
@@ -762,9 +764,9 @@ static void blk_alloc(struct XenDevice *xendev)
if (xen_mode != XEN_EMULATE) {
batch_maps = 1;
}
- if (xc_gnttab_set_max_grants(xendev->gnttabdev,
+ if (xengnttab_set_max_grants(xendev->gnttabdev,
MAX_GRANTS(max_requests, BLKIF_MAX_SEGMENTS_PER_REQUEST)) < 0) {
- xen_be_printf(xendev, 0, "xc_gnttab_set_max_grants failed: %s\n",
+ xen_be_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
strerror(errno));
}
}
@@ -976,7 +978,7 @@ static int blk_connect(struct XenDevice *xendev)
}
}
- blkdev->sring = xc_gnttab_map_grant_ref(blkdev->xendev.gnttabdev,
+ blkdev->sring = xengnttab_map_grant_ref(blkdev->xendev.gnttabdev,
blkdev->xendev.dom,
blkdev->ring_ref,
PROT_READ | PROT_WRITE);
@@ -1041,7 +1043,7 @@ static void blk_disconnect(struct XenDevice *xendev)
xen_be_unbind_evtchn(&blkdev->xendev);
if (blkdev->sring) {
- xc_gnttab_munmap(blkdev->xendev.gnttabdev, blkdev->sring, 1);
+ xengnttab_munmap(blkdev->xendev.gnttabdev, blkdev->sring, 1);
blkdev->cnt_map--;
blkdev->sring = NULL;
}
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 63ade33..8c06c19 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -233,7 +233,7 @@ static int con_initialise(struct XenDevice *xendev)
PROT_READ|PROT_WRITE,
con->ring_ref);
} else {
- con->sring = xc_gnttab_map_grant_ref(xendev->gnttabdev, con->xendev.dom,
+ con->sring = xengnttab_map_grant_ref(xendev->gnttabdev, con->xendev.dom,
con->ring_ref,
PROT_READ|PROT_WRITE);
}
@@ -275,7 +275,7 @@ static void con_disconnect(struct XenDevice *xendev)
if (!xendev->dev) {
munmap(con->sring, XC_PAGE_SIZE);
} else {
- xc_gnttab_munmap(xendev->gnttabdev, con->sring, 1);
+ xengnttab_munmap(xendev->gnttabdev, con->sring, 1);
}
con->sring = NULL;
}
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index 63918ae..778de28 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -169,7 +169,7 @@ static void net_tx_packets(struct XenNetDev *netdev)
(txreq.flags & NETTXF_more_data) ? " more_data" : "",
(txreq.flags & NETTXF_extra_info) ? " extra_info" : "");
- page = xc_gnttab_map_grant_ref(netdev->xendev.gnttabdev,
+ page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
netdev->xendev.dom,
txreq.gref, PROT_READ);
if (page == NULL) {
@@ -191,7 +191,7 @@ static void net_tx_packets(struct XenNetDev *netdev)
qemu_send_packet(qemu_get_queue(netdev->nic),
page + txreq.offset, txreq.size);
}
- xc_gnttab_munmap(netdev->xendev.gnttabdev, page, 1);
+ xengnttab_munmap(netdev->xendev.gnttabdev, page, 1);
net_tx_response(netdev, &txreq, NETIF_RSP_OKAY);
}
if (!netdev->tx_work) {
@@ -283,7 +283,7 @@ static ssize_t net_rx_packet(NetClientState *nc, const uint8_t *buf, size_t size
memcpy(&rxreq, RING_GET_REQUEST(&netdev->rx_ring, rc), sizeof(rxreq));
netdev->rx_ring.req_cons = ++rc;
- page = xc_gnttab_map_grant_ref(netdev->xendev.gnttabdev,
+ page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
netdev->xendev.dom,
rxreq.gref, PROT_WRITE);
if (page == NULL) {
@@ -293,7 +293,7 @@ static ssize_t net_rx_packet(NetClientState *nc, const uint8_t *buf, size_t size
return -1;
}
memcpy(page + NET_IP_ALIGN, buf, size);
- xc_gnttab_munmap(netdev->xendev.gnttabdev, page, 1);
+ xengnttab_munmap(netdev->xendev.gnttabdev, page, 1);
net_rx_response(netdev, &rxreq, NETIF_RSP_OKAY, NET_IP_ALIGN, size, 0);
return size;
@@ -366,11 +366,11 @@ static int net_connect(struct XenDevice *xendev)
return -1;
}
- netdev->txs = xc_gnttab_map_grant_ref(netdev->xendev.gnttabdev,
+ netdev->txs = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
netdev->xendev.dom,
netdev->tx_ring_ref,
PROT_READ | PROT_WRITE);
- netdev->rxs = xc_gnttab_map_grant_ref(netdev->xendev.gnttabdev,
+ netdev->rxs = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
netdev->xendev.dom,
netdev->rx_ring_ref,
PROT_READ | PROT_WRITE);
@@ -398,11 +398,11 @@ static void net_disconnect(struct XenDevice *xendev)
xen_be_unbind_evtchn(&netdev->xendev);
if (netdev->txs) {
- xc_gnttab_munmap(netdev->xendev.gnttabdev, netdev->txs, 1);
+ xengnttab_munmap(netdev->xendev.gnttabdev, netdev->txs, 1);
netdev->txs = NULL;
}
if (netdev->rxs) {
- xc_gnttab_munmap(netdev->xendev.gnttabdev, netdev->rxs, 1);
+ xengnttab_munmap(netdev->xendev.gnttabdev, netdev->rxs, 1);
netdev->rxs = NULL;
}
if (netdev->nic) {
diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index 342ec9b..f988b95 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -252,15 +252,15 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev,
fcntl(xenevtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
- xendev->gnttabdev = xen_xc_gnttab_open(NULL, 0);
- if (xendev->gnttabdev == XC_HANDLER_INITIAL_VALUE) {
+ xendev->gnttabdev = xengnttab_open(NULL, 0);
+ if (xendev->gnttabdev == NULL) {
xen_be_printf(NULL, 0, "can't open gnttab device\n");
xenevtchn_close(xendev->evtchndev);
g_free(xendev);
return NULL;
}
} else {
- xendev->gnttabdev = XC_HANDLER_INITIAL_VALUE;
+ xendev->gnttabdev = NULL;
}
QTAILQ_INSERT_TAIL(&xendevs, xendev, next);
@@ -309,8 +309,8 @@ static struct XenDevice *xen_be_del_xendev(int dom, int dev)
if (xendev->evtchndev != NULL) {
xenevtchn_close(xendev->evtchndev);
}
- if (xendev->gnttabdev != XC_HANDLER_INITIAL_VALUE) {
- xc_gnttab_close(xendev->gnttabdev);
+ if (xendev->gnttabdev != NULL) {
+ xengnttab_close(xendev->gnttabdev);
}
QTAILQ_REMOVE(&xendevs, xendev, next);
diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
index a90314f..8e8857b 100644
--- a/include/hw/xen/xen_backend.h
+++ b/include/hw/xen/xen_backend.h
@@ -47,7 +47,7 @@ struct XenDevice {
int local_port;
xenevtchn_handle *evtchndev;
- XenGnttab gnttabdev;
+ xengnttab_handle *gnttabdev;
struct XenDevOps *ops;
QTAILQ_ENTRY(XenDevice) next;
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 4dc2ee6..db62df4 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -40,7 +40,7 @@ static inline void *xc_map_foreign_bulk(int xc_handle, uint32_t dom, int prot,
typedef int XenXC;
typedef int xenevtchn_handle;
-typedef int XenGnttab;
+typedef int xengnttab_handle;
# define XC_INTERFACE_FMT "%i"
# define XC_HANDLER_INITIAL_VALUE -1
@@ -72,11 +72,31 @@ static inline int xenevtchn_close(xenevtchn_handle *h)
#define xenevtchn_unmask(h, p) xc_evtchn_unmask(*h, p)
#define xenevtchn_unbind(h, p) xc_evtchn_unmask(*h, p)
-static inline XenGnttab xen_xc_gnttab_open(void *logger,
- unsigned int open_flags)
+static inline xengnttab_handle *xengnttab_open(void *logger,
+ unsigned int open_flags)
{
- return xc_gnttab_open();
+ xengnttab_handle *h = malloc(sizeof(*h));
+ if (!h) {
+ return NULL;
+ }
+ *h = xc_gnttab_open();
+ if (*h == -1) {
+ free(h);
+ h = NULL;
+ }
+ return h;
}
+static inline int xengnttab_close(xengnttab_handle *h)
+{
+ int rc = xc_gnttab_close(*h);
+ free(h);
+ return rc;
+}
+#define xengnttab_set_max_grants(h, n) xc_gnttab_set_max_grants(*h, n)
+#define xengnttab_map_grant_ref(h, d, r, p) xc_gnttab_map_grant_ref(*h, d, r, p)
+#define xengnttab_map_grant_refs(h, c, d, r, p) \
+ xc_gnttab_map_grant_refs(*h, c, d, r, p)
+#define xengnttab_munmap(h, a, n) xc_gnttab_munmap(*h, a, n)
static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
unsigned int open_flags)
@@ -130,7 +150,7 @@ static inline void xs_close(struct xs_handle *xsh)
typedef xc_interface *XenXC;
typedef xc_evtchn xenevtchn_handle;
-typedef xc_gnttab *XenGnttab;
+typedef xc_gnttab xengnttab_handle;
# define XC_INTERFACE_FMT "%p"
# define XC_HANDLER_INITIAL_VALUE NULL
@@ -144,11 +164,13 @@ typedef xc_gnttab *XenGnttab;
#define xenevtchn_unmask(h, p) xc_evtchn_unmask(h, p)
#define xenevtchn_unbind(h, p) xc_evtchn_unbind(h, p)
-static inline XenGnttab xen_xc_gnttab_open(void *logger,
- unsigned int open_flags)
-{
- return xc_gnttab_open(logger, open_flags);
-}
+#define xengnttab_open(l, f) xc_gnttab_open(l, f)
+#define xengnttab_close(h) xc_gnttab_close(h)
+#define xengnttab_set_max_grants(h, n) xc_gnttab_set_max_grants(h, n)
+#define xengnttab_map_grant_ref(h, d, r, p) xc_gnttab_map_grant_ref(h, d, r, p)
+#define xengnttab_munmap(h, a, n) xc_gnttab_munmap(h, a, n)
+#define xengnttab_map_grant_refs(h, c, d, r, p) \
+ xc_gnttab_map_grant_refs(h, c, d, r, p)
static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
unsigned int open_flags)
--
2.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH QEMU-XEN v4 4/9] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_bulk
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 0/9] " Ian Campbell
` (2 preceding siblings ...)
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 3/9] xen: Switch to libxengnttab " Ian Campbell
@ 2015-10-21 15:23 ` Ian Campbell
2015-10-23 11:07 ` Stefano Stabellini
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 5/9] xen: Switch uses of xc_map_foreign_pages " Ian Campbell
` (4 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2015-10-21 15:23 UTC (permalink / raw)
To: ian.jackson, wei.liu2, xen-devel
Cc: Ian Campbell, qemu-devel, stefano.stabellini
In Xen 4.7 we are refactoring parts libxenctrl into a number of
separate libraries which will provide backward and forward API and ABI
compatiblity.
One such library will be libxenforeignmemory which provides access to
privileged foreign mappings and which will provide an interface
equivalent to xc_map_foreign_bulk.
In preparation for this switch all uses of xc_map_foreign_range to
xc_map_foreign_bulk. This is trivial because size was always
XC_PAGE_SIZE so the necessary adjustments are trivial:
* Pass &mfn (an array of length 1) instead of mfn. The function
takes a pointer to const, so there is no possibily of mfn changing
due to this change.
* Add a local err variable to receive the errors and pass &err
(again, an array of length 1)
* Adjust any existing logging to include err too
* Pass nr_pages=1 instead of size=XC_PAGE_SIZE
There is one wrinkle in xen_console.c:con_initialise() where
con->ring_ref is an int but can in some code paths (when !xendev->dev)
be treated as an mfn. I think this is an existing latent truncation
hazard on platforms where xen_pfn_t is 64-bit and int is 32-bit (e.g.
amd64, both arm* variants). I'm unsure under what circumstances
xendev->dev can be NULL or if anything elsewhere ensures the value
fits into an int. For now I just use a temporary xen_pfn_t to in
effect upcast the pointer from int* to xen_pfn_t*.
Build tested with 4.0 and 4.5.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: Ran checkpatch, fixed all errors
Mention the const-ness of the mfn array in the commit log
---
hw/char/xen_console.c | 9 +++++----
hw/display/xenfb.c | 6 +++---
xen-hvm.c | 30 ++++++++++++++++--------------
3 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 8c06c19..11c6472 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -219,6 +219,7 @@ static int con_initialise(struct XenDevice *xendev)
{
struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
int limit;
+ int err;
if (xenstore_read_int(con->console, "ring-ref", &con->ring_ref) == -1)
return -1;
@@ -228,10 +229,10 @@ static int con_initialise(struct XenDevice *xendev)
con->buffer.max_capacity = limit;
if (!xendev->dev) {
- con->sring = xc_map_foreign_range(xen_xc, con->xendev.dom,
- XC_PAGE_SIZE,
- PROT_READ|PROT_WRITE,
- con->ring_ref);
+ xen_pfn_t mfn = con->ring_ref;
+ con->sring = xc_map_foreign_bulk(xen_xc, con->xendev.dom,
+ PROT_READ|PROT_WRITE,
+ &mfn, &err, 1);
} else {
con->sring = xengnttab_map_grant_ref(xendev->gnttabdev, con->xendev.dom,
con->ring_ref,
diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index 8a61e95..10cefed 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -94,6 +94,7 @@ struct XenFB {
static int common_bind(struct common *c)
{
uint64_t mfn;
+ int err;
if (xenstore_read_fe_uint64(&c->xendev, "page-ref", &mfn) == -1)
return -1;
@@ -102,9 +103,8 @@ static int common_bind(struct common *c)
if (xenstore_read_fe_int(&c->xendev, "event-channel", &c->xendev.remote_port) == -1)
return -1;
- c->page = xc_map_foreign_range(xen_xc, c->xendev.dom,
- XC_PAGE_SIZE,
- PROT_READ | PROT_WRITE, mfn);
+ c->page = xc_map_foreign_bulk(xen_xc, c->xendev.dom,
+ PROT_READ | PROT_WRITE, &mfn, &err, 1);
if (c->page == NULL)
return -1;
diff --git a/xen-hvm.c b/xen-hvm.c
index 4470d58..0c84e30 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -1167,7 +1167,7 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
MemoryRegion **ram_memory)
{
- int i, rc;
+ int i, rc, map_err;
xen_pfn_t ioreq_pfn;
xen_pfn_t bufioreq_pfn;
evtchn_port_t bufioreq_evtchn;
@@ -1214,33 +1214,35 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
DPRINTF("buffered io page at pfn %lx\n", bufioreq_pfn);
DPRINTF("buffered io evtchn is %x\n", bufioreq_evtchn);
- state->shared_page = xc_map_foreign_range(xen_xc, xen_domid, XC_PAGE_SIZE,
- PROT_READ|PROT_WRITE, ioreq_pfn);
+ state->shared_page = xc_map_foreign_bulk(xen_xc, xen_domid,
+ PROT_READ|PROT_WRITE,
+ &ioreq_pfn, &map_err, 1);
if (state->shared_page == NULL) {
- hw_error("map shared IO page returned error %d handle=" XC_INTERFACE_FMT,
- errno, xen_xc);
+ hw_error(
+ "map shared IO page returned error %d(%d) handle=" XC_INTERFACE_FMT,
+ errno, map_err, xen_xc);
}
rc = xen_get_vmport_regs_pfn(xen_xc, xen_domid, &ioreq_pfn);
if (!rc) {
DPRINTF("shared vmport page at pfn %lx\n", ioreq_pfn);
state->shared_vmport_page =
- xc_map_foreign_range(xen_xc, xen_domid, XC_PAGE_SIZE,
- PROT_READ|PROT_WRITE, ioreq_pfn);
+ xc_map_foreign_bulk(xen_xc, xen_domid, PROT_READ|PROT_WRITE,
+ &ioreq_pfn, &map_err, 1);
if (state->shared_vmport_page == NULL) {
- hw_error("map shared vmport IO page returned error %d handle="
- XC_INTERFACE_FMT, errno, xen_xc);
+ hw_error("map shared vmport IO page returned error %d (%d) handle="
+ XC_INTERFACE_FMT, errno, map_err, xen_xc);
}
} else if (rc != -ENOSYS) {
hw_error("get vmport regs pfn returned error %d, rc=%d", errno, rc);
}
- state->buffered_io_page = xc_map_foreign_range(xen_xc, xen_domid,
- XC_PAGE_SIZE,
- PROT_READ|PROT_WRITE,
- bufioreq_pfn);
+ state->buffered_io_page = xc_map_foreign_bulk(xen_xc, xen_domid,
+ PROT_READ|PROT_WRITE,
+ &bufioreq_pfn,
+ &map_err, 1);
if (state->buffered_io_page == NULL) {
- hw_error("map buffered IO page returned error %d", errno);
+ hw_error("map buffered IO page returned error %d (%d)", errno, map_err);
}
/* Note: cpus is empty at this point in init */
--
2.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH QEMU-XEN v4 5/9] xen: Switch uses of xc_map_foreign_pages into xc_map_foreign_bulk
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 0/9] " Ian Campbell
` (3 preceding siblings ...)
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 4/9] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_bulk Ian Campbell
@ 2015-10-21 15:23 ` Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 6/9] xen: Switch uses of xc_map_foreign_bulk to use libxenforeignmemory API Ian Campbell
` (3 subsequent siblings)
8 siblings, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2015-10-21 15:23 UTC (permalink / raw)
To: ian.jackson, wei.liu2, xen-devel
Cc: Ian Campbell, qemu-devel, stefano.stabellini
In Xen 4.7 we are refactoring parts libxenctrl into a number of
separate libraries which will provide backward and forward API and ABI
compatiblity.
One such library will be libxenforeignmemory which provides access to
privileged foreign mappings and which will provide an interface
equivalent to xc_map_foreign_bulk.
In preparation for this switch both uses of xc_map_foreign_pages
(which both happen to be in xenfb_map_fb) to xc_map_foreign_bulk. This
simply requires allocating and passing a new err array (the same one
for both calls).
Build tested with 4.0 and 4.5.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
v4: Fix indentation
---
hw/display/xenfb.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index 10cefed..b0ac1e6 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -428,6 +428,7 @@ static int xenfb_map_fb(struct XenFB *xenfb)
int n_fbdirs;
xen_pfn_t *pgmfns = NULL;
xen_pfn_t *fbmfns = NULL;
+ int *errs = NULL;
void *map, *pd;
int mode, ret = -1;
@@ -487,17 +488,18 @@ static int xenfb_map_fb(struct XenFB *xenfb)
pgmfns = g_malloc0(sizeof(xen_pfn_t) * n_fbdirs);
fbmfns = g_malloc0(sizeof(xen_pfn_t) * xenfb->fbpages);
+ errs = g_malloc0(sizeof(int) * n_fbdirs);
xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd);
- map = xc_map_foreign_pages(xen_xc, xenfb->c.xendev.dom,
- PROT_READ, pgmfns, n_fbdirs);
+ map = xc_map_foreign_bulk(xen_xc, xenfb->c.xendev.dom,
+ PROT_READ, pgmfns, errs, n_fbdirs);
if (map == NULL)
goto out;
xenfb_copy_mfns(mode, xenfb->fbpages, fbmfns, map);
munmap(map, n_fbdirs * XC_PAGE_SIZE);
- xenfb->pixels = xc_map_foreign_pages(xen_xc, xenfb->c.xendev.dom,
- PROT_READ, fbmfns, xenfb->fbpages);
+ xenfb->pixels = xc_map_foreign_bulk(xen_xc, xenfb->c.xendev.dom,
+ PROT_READ, fbmfns, errs, xenfb->fbpages);
if (xenfb->pixels == NULL)
goto out;
@@ -506,6 +508,7 @@ static int xenfb_map_fb(struct XenFB *xenfb)
out:
g_free(pgmfns);
g_free(fbmfns);
+ g_free(errs);
return ret;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH QEMU-XEN v4 6/9] xen: Switch uses of xc_map_foreign_bulk to use libxenforeignmemory API.
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 0/9] " Ian Campbell
` (4 preceding siblings ...)
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 5/9] xen: Switch uses of xc_map_foreign_pages " Ian Campbell
@ 2015-10-21 15:23 ` Ian Campbell
2015-10-23 11:06 ` Stefano Stabellini
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 7/9] xen: Use stable library interfaces when they are available Ian Campbell
` (2 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2015-10-21 15:23 UTC (permalink / raw)
To: ian.jackson, wei.liu2, xen-devel
Cc: Ian Campbell, qemu-devel, stefano.stabellini
In Xen 4.7 we are refactoring parts libxenctrl into a number of
separate libraries which will provide backward and forward API and ABI
compatiblity.
One such library will be libxenforeignmemory which provides access to
privileged foreign mappings and which will provide an interface
equivalent to xc_map_foreign_bulk.
In preparation for adding support for libxenforeignmemory add support
to the <=4.0 and <=4.6 compat code in xen_common.h to allow us to
switch to using the new API. These shims will disappear for versions
of Xen which include libxenforeignmemory.
Since libxenforeignmemory will have its own handle type but for <= 4.6
the functionality is provided by using a libxenctrl handle we
introduce a new global xen_fmem alongside the existing xen_xc. In fact
we make xen_fmem a pointer to the existing xen_xc, which then works
correctly with both <=4.0 (xc handle is an int) and <=4.6 (xc handle
is a pointer). In the latter case xen_fmem is actually a double
indirect pointer, but it all falls out in the wash.
Unlike libxenctrl libxenforeignmemory has an explicit unmap function,
rather than just specifying that munmap should be used, so the unmap
paths are updated to use xenforeignmemory_unmap, which is a shim for
munmap on these versions of xen. The mappings in xen-hvm.c do not
appear to be unmapped (which makes sense for a qemu-dm process)
In fb_disconnect this results in a change from simply mmap over the
existing mapping (with an implciit munmap) to expliclty unmapping with
xenforeignmemory_unmap and then mapping the required anonymous memory
in the same hole. I don't think this is a problem since any other
thread which was racily touching this region would already be running
the risk of hitting the mapping halfway through the call. If this is
thought to be a problem then we could consider adding an extra API to
the libxenforeignmemory interface to replace a foreign mapping with
anonymous shared memory, but I'd prefer not to.
Build tested with 4.0 and 4.5.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
I noticed in xen_console.c that the decision to use a foreign
privileged memory mapping vs a grant dev is made using different
variables in con_initialise vs con_disconnect. The former uses
xendev->dev while the latter uses xendev->gnttabdev. Is this a latent
bug?
v4: Rebase onto "xen_console: correctly cleanup primary console on
teardown."
xenforeignmemory_unmap takes pages not bytes
Compat wrapper for xenforeignmemory_open instead of ifdef in code.
Run check patch and fix most issues. I did not fix:
ERROR: do not initialise globals to 0 or NULL
+xenforeignmemory_handle *xen_fmem = NULL;
=> This is consistent with all of the existing declarations.
ERROR: need consistent spacing around '*' (ctx:WxV)
+typedef xc_interface *xenforeignmemory_handle;
=> I think this is a false +ve since this is a pointer "*" not a multiple "*".
---
hw/char/xen_console.c | 8 ++++----
hw/display/xenfb.c | 15 ++++++++-------
hw/xen/xen_backend.c | 3 ++-
include/hw/xen/xen_backend.h | 1 +
include/hw/xen/xen_common.h | 12 ++++++++++++
xen-common.c | 6 ++++++
xen-hvm.c | 18 +++++++++---------
xen-mapcache.c | 6 +++---
8 files changed, 45 insertions(+), 24 deletions(-)
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 11c6472..24f3a40 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -230,9 +230,9 @@ static int con_initialise(struct XenDevice *xendev)
if (!xendev->dev) {
xen_pfn_t mfn = con->ring_ref;
- con->sring = xc_map_foreign_bulk(xen_xc, con->xendev.dom,
- PROT_READ|PROT_WRITE,
- &mfn, &err, 1);
+ con->sring = xenforeignmemory_map(xen_fmem, con->xendev.dom,
+ PROT_READ|PROT_WRITE,
+ &mfn, &err, 1);
} else {
con->sring = xengnttab_map_grant_ref(xendev->gnttabdev, con->xendev.dom,
con->ring_ref,
@@ -274,7 +274,7 @@ static void con_disconnect(struct XenDevice *xendev)
if (con->sring) {
if (!xendev->dev) {
- munmap(con->sring, XC_PAGE_SIZE);
+ xenforeignmemory_unmap(xen_fmem, con->sring, 1);
} else {
xengnttab_munmap(xendev->gnttabdev, con->sring, 1);
}
diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index b0ac1e6..a5ddb60 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -103,8 +103,8 @@ static int common_bind(struct common *c)
if (xenstore_read_fe_int(&c->xendev, "event-channel", &c->xendev.remote_port) == -1)
return -1;
- c->page = xc_map_foreign_bulk(xen_xc, c->xendev.dom,
- PROT_READ | PROT_WRITE, &mfn, &err, 1);
+ c->page = xenforeignmemory_map(xen_fmem, c->xendev.dom,
+ PROT_READ | PROT_WRITE, &mfn, &err, 1);
if (c->page == NULL)
return -1;
@@ -119,7 +119,7 @@ static void common_unbind(struct common *c)
{
xen_be_unbind_evtchn(&c->xendev);
if (c->page) {
- munmap(c->page, XC_PAGE_SIZE);
+ xenforeignmemory_unmap(xen_fmem, c->page, 1);
c->page = NULL;
}
}
@@ -491,14 +491,14 @@ static int xenfb_map_fb(struct XenFB *xenfb)
errs = g_malloc0(sizeof(int) * n_fbdirs);
xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd);
- map = xc_map_foreign_bulk(xen_xc, xenfb->c.xendev.dom,
- PROT_READ, pgmfns, errs, n_fbdirs);
+ map = xenforeignmemory_map(xen_fmem, xenfb->c.xendev.dom,
+ PROT_READ, pgmfns, errs, n_fbdirs);
if (map == NULL)
goto out;
xenfb_copy_mfns(mode, xenfb->fbpages, fbmfns, map);
- munmap(map, n_fbdirs * XC_PAGE_SIZE);
+ xenforeignmemory_unmap(xen_fmem, map, n_fbdirs);
- xenfb->pixels = xc_map_foreign_bulk(xen_xc, xenfb->c.xendev.dom,
+ xenfb->pixels = xenforeignmemory_map(xen_fmem, xenfb->c.xendev.dom,
PROT_READ, fbmfns, errs, xenfb->fbpages);
if (xenfb->pixels == NULL)
goto out;
@@ -907,6 +907,7 @@ static void fb_disconnect(struct XenDevice *xendev)
* Replacing the framebuffer with anonymous shared memory
* instead. This releases the guest pages and keeps qemu happy.
*/
+ xenforeignmemory_unmap(xen_fmem, fb->pixels, fb->fbpages);
fb->pixels = mmap(fb->pixels, fb->fbpages * XC_PAGE_SIZE,
PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANON,
-1, 0);
diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index f988b95..30ffcb4 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -45,6 +45,7 @@
/* public */
XenXC xen_xc = XC_HANDLER_INITIAL_VALUE;
+xenforeignmemory_handle *xen_fmem = NULL;
struct xs_handle *xenstore = NULL;
const char *xen_protocol;
@@ -719,7 +720,7 @@ int xen_be_init(void)
goto err;
}
- if (xen_xc == XC_HANDLER_INITIAL_VALUE) {
+ if (xen_xc == XC_HANDLER_INITIAL_VALUE || xen_fmem == NULL) {
/* Check if xen_init() have been called */
goto err;
}
diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
index 8e8857b..e0d52ee 100644
--- a/include/hw/xen/xen_backend.h
+++ b/include/hw/xen/xen_backend.h
@@ -57,6 +57,7 @@ struct XenDevice {
/* variables */
extern XenXC xen_xc;
+extern xenforeignmemory_handle *xen_fmem;
extern struct xs_handle *xenstore;
extern const char *xen_protocol;
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index db62df4..2a5f27a 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -41,6 +41,7 @@ static inline void *xc_map_foreign_bulk(int xc_handle, uint32_t dom, int prot,
typedef int XenXC;
typedef int xenevtchn_handle;
typedef int xengnttab_handle;
+typedef int xenforeignmemory_handle;
# define XC_INTERFACE_FMT "%i"
# define XC_HANDLER_INITIAL_VALUE -1
@@ -104,6 +105,11 @@ static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
return xc_interface_open();
}
+#define xenforeignmemory_open(l, f) &xen_xc
+#define xenforeignmemory_map(h, d, p, a, e, n) \
+ xc_map_foreign_bulk(*h, d, p, a, e, n)
+#define xenforeignmemory_unmap(h, p, s) munmap(p, s * XC_PAGE_SIZE)
+
static inline int xc_fd(int xen_xc)
{
return xen_xc;
@@ -149,6 +155,7 @@ static inline void xs_close(struct xs_handle *xsh)
#else
typedef xc_interface *XenXC;
+typedef xc_interface *xenforeignmemory_handle;
typedef xc_evtchn xenevtchn_handle;
typedef xc_gnttab xengnttab_handle;
@@ -178,6 +185,11 @@ static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
return xc_interface_open(logger, dombuild_logger, open_flags);
}
+#define xenforeignmemory_open(l, f) &xen_xc
+#define xenforeignmemory_map(h, d, p, a, e, n) \
+ xc_map_foreign_bulk(*h, d, p, a, e, n)
+#define xenforeignmemory_unmap(h, p, s) munmap(p, s * XC_PAGE_SIZE)
+
/* FIXME There is now way to have the xen fd */
static inline int xc_fd(xc_interface *xen_xc)
{
diff --git a/xen-common.c b/xen-common.c
index 56359ca..dd8619b 100644
--- a/xen-common.c
+++ b/xen-common.c
@@ -117,6 +117,12 @@ static int xen_init(MachineState *ms)
xen_be_printf(NULL, 0, "can't open xen interface\n");
return -1;
}
+ xen_fmem = xenforeignmemory_open(0, 0);
+ if (xen_fmem == NULL) {
+ xen_be_printf(NULL, 0, "can't open xen fmem interface\n");
+ xc_interface_close(xen_xc);
+ return -1;
+ }
qemu_add_vm_change_state_handler(xen_change_state_handler, NULL);
return 0;
diff --git a/xen-hvm.c b/xen-hvm.c
index 0c84e30..8fe7397 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -1214,9 +1214,9 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
DPRINTF("buffered io page at pfn %lx\n", bufioreq_pfn);
DPRINTF("buffered io evtchn is %x\n", bufioreq_evtchn);
- state->shared_page = xc_map_foreign_bulk(xen_xc, xen_domid,
- PROT_READ|PROT_WRITE,
- &ioreq_pfn, &map_err, 1);
+ state->shared_page = xenforeignmemory_map(xen_fmem, xen_domid,
+ PROT_READ|PROT_WRITE,
+ &ioreq_pfn, &map_err, 1);
if (state->shared_page == NULL) {
hw_error(
"map shared IO page returned error %d(%d) handle=" XC_INTERFACE_FMT,
@@ -1227,8 +1227,8 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
if (!rc) {
DPRINTF("shared vmport page at pfn %lx\n", ioreq_pfn);
state->shared_vmport_page =
- xc_map_foreign_bulk(xen_xc, xen_domid, PROT_READ|PROT_WRITE,
- &ioreq_pfn, &map_err, 1);
+ xenforeignmemory_map(xen_fmem, xen_domid, PROT_READ|PROT_WRITE,
+ &ioreq_pfn, &map_err, 1);
if (state->shared_vmport_page == NULL) {
hw_error("map shared vmport IO page returned error %d (%d) handle="
XC_INTERFACE_FMT, errno, map_err, xen_xc);
@@ -1237,10 +1237,10 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
hw_error("get vmport regs pfn returned error %d, rc=%d", errno, rc);
}
- state->buffered_io_page = xc_map_foreign_bulk(xen_xc, xen_domid,
- PROT_READ|PROT_WRITE,
- &bufioreq_pfn,
- &map_err, 1);
+ state->buffered_io_page = xenforeignmemory_map(xen_fmem, xen_domid,
+ PROT_READ|PROT_WRITE,
+ &bufioreq_pfn,
+ &map_err, 1);
if (state->buffered_io_page == NULL) {
hw_error("map buffered IO page returned error %d (%d)", errno, map_err);
}
diff --git a/xen-mapcache.c b/xen-mapcache.c
index 66da1a6..7d4953c 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -169,10 +169,10 @@ static void xen_remap_bucket(MapCacheEntry *entry,
pfns[i] = (address_index << (MCACHE_BUCKET_SHIFT-XC_PAGE_SHIFT)) + i;
}
- vaddr_base = xc_map_foreign_bulk(xen_xc, xen_domid, PROT_READ|PROT_WRITE,
- pfns, err, nb_pfn);
+ vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid, PROT_READ|PROT_WRITE,
+ pfns, err, nb_pfn);
if (vaddr_base == NULL) {
- perror("xc_map_foreign_bulk");
+ perror("xenforeignmemory_map");
exit(-1);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH QEMU-XEN v4 7/9] xen: Use stable library interfaces when they are available.
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 0/9] " Ian Campbell
` (5 preceding siblings ...)
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 6/9] xen: Switch uses of xc_map_foreign_bulk to use libxenforeignmemory API Ian Campbell
@ 2015-10-21 15:23 ` Ian Campbell
2015-10-23 11:31 ` Stefano Stabellini
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 8/9] xen: domainbuild: reopen libxenctrl interface after forking for domain watcher Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder Ian Campbell
8 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2015-10-21 15:23 UTC (permalink / raw)
To: ian.jackson, wei.liu2, xen-devel
Cc: Ian Campbell, qemu-devel, stefano.stabellini
In Xen 4.7 we are refactoring parts libxenctrl into a number of
separate libraries which will provide backward and forward API and ABI
compatiblity.
Specifically libxenevtchn, libxengnttab and libxenforeignmemory.
Previous patches have already laid the groundwork for using these by
switching the existing compatibility shims to reflect the intefaces to
these libraries.
So all which remains is to update configure to detect the libraries
and enable their use. Although they are notionally independent we take
an all or nothing approach to the three libraries since they were
added at the same time.
The only non-obvious bit is that we now open a proper xenforeignmemory
handle for xen_fmem instead of reusing the xen_xc handle.
Build tested with 4.0, 4.5 and the patches targetting 4.7 which adds
these libraries.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: xenforeignmemory_open is now a compat wrapper, so no ifdef.
Simplify configury by asserting that interface version 470 will
always have the libraries (lack of them makes it 460).
Ran checkpatch and fixed everything except:
ERROR: need consistent spacing around '*' (ctx:WxV)
+typedef xc_interface *XenXC;
Which I think is a false +ve.
simplify configury
---
configure | 55 +++++++++++++++++++++++++++++++++++++++++++++
include/hw/xen/xen_common.h | 38 +++++++++++++++++++++++++++++--
2 files changed, 91 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index 779623a..fe0a39d 100755
--- a/configure
+++ b/configure
@@ -1840,6 +1840,7 @@ fi
if test "$xen" != "no" ; then
xen_libs="-lxenstore -lxenctrl -lxenguest"
+ xen_stable_libs="-lxenforeignmemory -lxengnttab -lxenevtchn"
# First we test whether Xen headers and libraries are available.
# If no, we are done and there is no Xen support.
@@ -1862,6 +1863,57 @@ EOF
# Xen unstable
elif
cat > $TMPC <<EOF &&
+/*
+ * If we have stable libs the we don't want the libxc compat
+ * layers, regardless of what CFLAGS we may have been given.
+ */
+#undef XC_WANT_COMPAT_EVTCHN_API
+#undef XC_WANT_COMPAT_GNTTAB_API
+#undef XC_WANT_COMPAT_MAP_FOREIGN_API
+#include <xenctrl.h>
+#include <xenstore.h>
+#include <xenevtchn.h>
+#include <xengnttab.h>
+#include <xenforeignmemory.h>
+#include <stdint.h>
+#include <xen/hvm/hvm_info_table.h>
+#if !defined(HVM_MAX_VCPUS)
+# error HVM_MAX_VCPUS not defined
+#endif
+int main(void) {
+ xc_interface *xc;
+ xenforeignmemory_handle *xfmem;
+ xenevtchn_handle *xe;
+ xengnttab_handle *xg;
+
+ xs_daemon_open();
+
+ xc = xc_interface_open(0, 0, 0);
+ xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
+ xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
+ xc_hvm_inject_msi(xc, 0, 0xf0000000, 0x00000000);
+ xc_hvm_create_ioreq_server(xc, 0, HVM_IOREQSRV_BUFIOREQ_ATOMIC, NULL);
+
+ xfmem = xenforeignmemory_open(0, 0);
+ xenforeignmemory_map(xfmem, 0, 0, 0, 0, 0);
+
+ xe = xenevtchn_open(0, 0);
+ xenevtchn_fd(xe);
+
+ xg = xengnttab_open(0, 0);
+ xengnttab_map_grant_ref(xg, 0, 0, 0);
+
+ return 0;
+}
+EOF
+ compile_prog "" "$xen_libs $xen_stable_libs"
+ then
+ xen_ctrl_version=470
+ xen=yes
+
+ # Xen 4.6
+ elif
+ cat > $TMPC <<EOF &&
#include <xenctrl.h>
#include <xenstore.h>
#include <stdint.h>
@@ -2037,6 +2089,9 @@ EOF
fi
if test "$xen" = yes; then
+ if test $xen_ctrl_version -ge 470 ; then
+ libs_softmmu="$xen_stable_libs $libs_softmmu"
+ fi
libs_softmmu="$xen_libs $libs_softmmu"
fi
fi
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 2a5f27a..38293b4 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -6,6 +6,17 @@
#include <stddef.h>
#include <inttypes.h>
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 470
+/*
+ * If we have new enough libxenctrl then we do not want/need these compat
+ * interfaces, despite what the user supplied cflags might say. They
+ * must be undefined before including xenctrl.h
+ */
+#undef XC_WANT_COMPAT_EVTCHN_API
+#undef XC_WANT_COMPAT_GNTTAB_API
+#undef XC_WANT_COMPAT_MAP_FOREIGN_API
+#endif
+
#include <xenctrl.h>
#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 420
# include <xs.h>
@@ -151,8 +162,8 @@ static inline void xs_close(struct xs_handle *xsh)
}
-/* Xen 4.1 */
-#else
+/* Xen 4.1 thru 4.6 */
+#elif CONFIG_XEN_CTRL_INTERFACE_VERSION < 470
typedef xc_interface *XenXC;
typedef xc_interface *xenforeignmemory_handle;
@@ -195,6 +206,29 @@ static inline int xc_fd(xc_interface *xen_xc)
{
return -1;
}
+#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 470 */
+
+typedef xc_interface *XenXC;
+
+# define XC_INTERFACE_FMT "%p"
+# define XC_HANDLER_INITIAL_VALUE NULL
+
+#include <xenevtchn.h>
+#include <xengnttab.h>
+#include <xenforeignmemory.h>
+
+static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
+ unsigned int open_flags)
+{
+ return xc_interface_open(logger, dombuild_logger, open_flags);
+}
+
+/* FIXME There is now way to have the xen fd */
+static inline int xc_fd(xc_interface *xen_xc)
+{
+ return -1;
+}
+
#endif
/* Xen before 4.2 */
--
2.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH QEMU-XEN v4 8/9] xen: domainbuild: reopen libxenctrl interface after forking for domain watcher.
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 0/9] " Ian Campbell
` (6 preceding siblings ...)
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 7/9] xen: Use stable library interfaces when they are available Ian Campbell
@ 2015-10-21 15:23 ` Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder Ian Campbell
8 siblings, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2015-10-21 15:23 UTC (permalink / raw)
To: ian.jackson, wei.liu2, xen-devel
Cc: Ian Campbell, qemu-devel, stefano.stabellini
Using an existing libxenctrl handle after a fork was never
particularly safe (especially if foreign mappings existed at the time
of the fork) and the xc fd has been unavailable for many releases.
Reopen the handle after fork and therefore do away with xc_fd().
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
The fact that xc_fd hasn't been useful since at least Xen 4.1 makes me
question the utility of this domainbuild in QEMU. Perhaps we should
just nuke it?
---
hw/xenpv/xen_domainbuild.c | 9 ++++++---
include/hw/xen/xen_common.h | 17 -----------------
2 files changed, 6 insertions(+), 20 deletions(-)
diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
index c0ab753..3e8422f 100644
--- a/hw/xenpv/xen_domainbuild.c
+++ b/hw/xenpv/xen_domainbuild.c
@@ -174,12 +174,15 @@ static int xen_domain_watcher(void)
for (i = 3; i < n; i++) {
if (i == fd[0])
continue;
- if (i == xc_fd(xen_xc)) {
- continue;
- }
close(i);
}
+ /*
+ * Reopen xc interface, since the original is unsafe after fork
+ * and was closed above.
+ */
+ xen_xc = xc_interface_open(0, 0, 0);
+
/* ignore term signals */
signal(SIGINT, SIG_IGN);
signal(SIGTERM, SIG_IGN);
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 38293b4..4b4b50d 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -121,12 +121,6 @@ static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
xc_map_foreign_bulk(*h, d, p, a, e, n)
#define xenforeignmemory_unmap(h, p, s) munmap(p, s * XC_PAGE_SIZE)
-static inline int xc_fd(int xen_xc)
-{
- return xen_xc;
-}
-
-
static inline int xc_domain_populate_physmap_exact
(XenXC xc_handle, uint32_t domid, unsigned long nr_extents,
unsigned int extent_order, unsigned int mem_flags, xen_pfn_t *extent_start)
@@ -201,11 +195,6 @@ static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
xc_map_foreign_bulk(*h, d, p, a, e, n)
#define xenforeignmemory_unmap(h, p, s) munmap(p, s * XC_PAGE_SIZE)
-/* FIXME There is now way to have the xen fd */
-static inline int xc_fd(xc_interface *xen_xc)
-{
- return -1;
-}
#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 470 */
typedef xc_interface *XenXC;
@@ -223,12 +212,6 @@ static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
return xc_interface_open(logger, dombuild_logger, open_flags);
}
-/* FIXME There is now way to have the xen fd */
-static inline int xc_fd(xc_interface *xen_xc)
-{
- return -1;
-}
-
#endif
/* Xen before 4.2 */
--
2.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 0/9] " Ian Campbell
` (7 preceding siblings ...)
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 8/9] xen: domainbuild: reopen libxenctrl interface after forking for domain watcher Ian Campbell
@ 2015-10-21 15:23 ` Ian Campbell
2015-10-22 11:07 ` Ian Campbell
2015-10-23 11:12 ` Stefano Stabellini
8 siblings, 2 replies; 25+ messages in thread
From: Ian Campbell @ 2015-10-21 15:23 UTC (permalink / raw)
To: ian.jackson, wei.liu2, xen-devel
Cc: Ian Campbell, qemu-devel, stefano.stabellini
Until the previous patch this relied on xc_fd(), which was only
implemented for Xen 4.0 and earlier.
Given this wasn't working since Xen 4.0 I have marked this as disabled
by default.
Removing this support drops the use of a bunch of symbols from
libxenctrl, specifically:
- xc_domain_create
- xc_domain_destroy
- xc_domain_getinfo
- xc_domain_max_vcpus
- xc_domain_setmaxmem
- xc_domain_unpause
- xc_evtchn_alloc_unbound
- xc_linux_build
This is another step towards only using Xen libraries which provide a
stable inteface.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: Fixed all checkpatch errors.
Disabled by default.
---
configure | 17 +++++++++++++++++
hw/xenpv/Makefile.objs | 4 +++-
hw/xenpv/xen_machine_pv.c | 14 ++++++++++----
3 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/configure b/configure
index fe0a39d..9eab587 100755
--- a/configure
+++ b/configure
@@ -910,6 +910,10 @@ for opt do
;;
--enable-xen-pci-passthrough) xen_pci_passthrough="yes"
;;
+ --disable-xen-pv-domain-build) xen_pv_domain_build="no"
+ ;;
+ --enable-xen-pv-domain-build) xen_pv_domain_build="yes"
+ ;;
--disable-brlapi) brlapi="no"
;;
--enable-brlapi) brlapi="yes"
@@ -2113,6 +2117,15 @@ if test "$xen_pci_passthrough" != "no"; then
fi
fi
+if test "$xen_pv_domain_build" != "no"; then
+ if test "$xen_pv_domain_build" = "yes" &&
+ test "$xen" != "yes"; then
+ error_exit "User requested Xen PV domain builder support" \
+ "which requires Xen support."
+ fi
+ xen_pv_domain_build=no
+fi
+
##########################################
# libtool probe
@@ -4393,6 +4406,7 @@ fi
echo "xen support $xen"
if test "$xen" = "yes" ; then
echo "xen ctrl version $xen_ctrl_version"
+ echo "pv dom build $xen_pv_domain_build"
fi
echo "brlapi support $brlapi"
echo "bluez support $bluez"
@@ -4725,6 +4739,9 @@ fi
if test "$xen" = "yes" ; then
echo "CONFIG_XEN_BACKEND=y" >> $config_host_mak
echo "CONFIG_XEN_CTRL_INTERFACE_VERSION=$xen_ctrl_version" >> $config_host_mak
+ if test "$xen_pv_domain_build" = "yes" ; then
+ echo "CONFIG_XEN_PV_DOMAIN_BUILD=y" >> $config_host_mak
+ fi
fi
if test "$linux_aio" = "yes" ; then
echo "CONFIG_LINUX_AIO=y" >> $config_host_mak
diff --git a/hw/xenpv/Makefile.objs b/hw/xenpv/Makefile.objs
index 49f6e9e..bbf5873 100644
--- a/hw/xenpv/Makefile.objs
+++ b/hw/xenpv/Makefile.objs
@@ -1,2 +1,4 @@
# Xen PV machine support
-obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
+obj-$(CONFIG_XEN) += xen_machine_pv.o
+# Xen PV machine builder support
+obj-$(CONFIG_XEN_PV_DOMAIN_BUILD) += xen_domainbuild.o
diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c
index 2e545d2..e5b3698 100644
--- a/hw/xenpv/xen_machine_pv.c
+++ b/hw/xenpv/xen_machine_pv.c
@@ -30,9 +30,6 @@
static void xen_init_pv(MachineState *machine)
{
- const char *kernel_filename = machine->kernel_filename;
- const char *kernel_cmdline = machine->kernel_cmdline;
- const char *initrd_filename = machine->initrd_filename;
DriveInfo *dinfo;
int i;
@@ -46,13 +43,22 @@ static void xen_init_pv(MachineState *machine)
case XEN_ATTACH:
/* nothing to do, xend handles everything */
break;
- case XEN_CREATE:
+ case XEN_CREATE: {
+#ifdef CONFIG_XEN_PV_DOMAIN_BUILD
+ const char *kernel_filename = machine->kernel_filename;
+ const char *kernel_cmdline = machine->kernel_cmdline;
+ const char *initrd_filename = machine->initrd_filename;
if (xen_domain_build_pv(kernel_filename, initrd_filename,
kernel_cmdline) < 0) {
fprintf(stderr, "xen pv domain creation failed\n");
exit(1);
}
+#else
+ fprintf(stderr, "xen pv domain creation not supported\n");
+ exit(1);
+#endif
break;
+ }
case XEN_EMULATE:
fprintf(stderr, "xen emulation not implemented (yet)\n");
exit(1);
--
2.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder Ian Campbell
@ 2015-10-22 11:07 ` Ian Campbell
2015-10-23 11:12 ` Stefano Stabellini
1 sibling, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2015-10-22 11:07 UTC (permalink / raw)
To: ian.jackson, wei.liu2, xen-devel; +Cc: qemu-devel, stefano.stabellini
On Wed, 2015-10-21 at 16:23 +0100, Ian Campbell wrote:
> [...]
> v4: Fixed all checkpatch errors.
> Disabled by default.
I botched this and it is no longer possible to turn it on. (I wonder if
anyone would have noticed in practice...)
> @@ -2113,6 +2117,15 @@ if test "$xen_pci_passthrough" != "no"; then
> fi
> fi
>
> +if test "$xen_pv_domain_build" != "no"; then
> + if test "$xen_pv_domain_build" = "yes" &&
> + test "$xen" != "yes"; then
> + error_exit "User requested Xen PV domain builder support" \
> + "which requires Xen support."
> + fi
> + xen_pv_domain_build=no
> +fi
This is bogus.
v5 will have this incremental fix in it:
diff --git a/configure b/configure
index b9c5d68..24cfd35 100755
--- a/configure
+++ b/configure
@@ -2117,13 +2117,13 @@ if test "$xen_pci_passthrough" != "no"; then
fi
fi
-if test "$xen_pv_domain_build" != "no"; then
- if test "$xen_pv_domain_build" = "yes" &&
- test "$xen" != "yes"; then
- error_exit "User requested Xen PV domain builder support" \
- "which requires Xen support."
- fi
- xen_pv_domain_build=no
+if test "$xen_pv_domain_build" = "yes"; then
+ if test "$xen" != "yes"; then
+ error_exit "User requested Xen PV domain builder support" \
+ "which requires Xen support."
+ fi
+else
+ xen_pv_domain_build=no
fi
##########################################
Which is a bit easier to grok with -b:
diff --git a/configure b/configure
index b9c5d68..24cfd35 100755
--- a/configure
+++ b/configure
@@ -2117,12 +2117,12 @@ if test "$xen_pci_passthrough" != "no"; then
fi
fi
-if test "$xen_pv_domain_build" != "no"; then
- if test "$xen_pv_domain_build" = "yes" &&
- test "$xen" != "yes"; then
+if test "$xen_pv_domain_build" = "yes"; then
+ if test "$xen" != "yes"; then
error_exit "User requested Xen PV domain builder support" \
"which requires Xen support."
fi
+else
xen_pv_domain_build=no
fi
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 1/9] xen_console: correctly cleanup primary console on teardown.
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 1/9] xen_console: correctly cleanup primary console on teardown Ian Campbell
@ 2015-10-22 16:46 ` Stefano Stabellini
0 siblings, 0 replies; 25+ messages in thread
From: Stefano Stabellini @ 2015-10-22 16:46 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, stefano.stabellini, ian.jackson, qemu-devel, xen-devel
On Wed, 21 Oct 2015, Ian Campbell wrote:
> All of the work in con_disconnect applies to the primary console case
> (when xendev->dev is NULL). Therefore remove the early check and bail
> and allow it to fall through. All of the existing code is correctly
> conditional already.
>
> The ->dev and ->gnttabdev handles are either both set or neither. For
> consistency with con_initialise() with to the former here too.
>
> With this con_initialise and con_disconnect now mirror each other.
>
> Fix up a hard tab in the function while editing.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> v4: New patch based on feedback to "xen: Switch uses of
> xc_map_foreign_bulk to use libxenforeignmemory API."
> ---
> hw/char/xen_console.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index eb7f450..63ade33 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -265,9 +265,6 @@ static void con_disconnect(struct XenDevice *xendev)
> {
> struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
>
> - if (!xendev->dev) {
> - return;
> - }
> if (con->chr) {
> qemu_chr_add_handlers(con->chr, NULL, NULL, NULL, NULL);
> qemu_chr_fe_release(con->chr);
> @@ -275,12 +272,12 @@ static void con_disconnect(struct XenDevice *xendev)
> xen_be_unbind_evtchn(&con->xendev);
>
> if (con->sring) {
> - if (!xendev->gnttabdev) {
> + if (!xendev->dev) {
> munmap(con->sring, XC_PAGE_SIZE);
> } else {
> xc_gnttab_munmap(xendev->gnttabdev, con->sring, 1);
> }
> - con->sring = NULL;
> + con->sring = NULL;
> }
> }
>
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 2/9] xen: Switch to libxenevtchn interface for compat shims.
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 2/9] xen: Switch to libxenevtchn interface for compat shims Ian Campbell
@ 2015-10-23 11:06 ` Stefano Stabellini
0 siblings, 0 replies; 25+ messages in thread
From: Stefano Stabellini @ 2015-10-23 11:06 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, stefano.stabellini, ian.jackson, qemu-devel, xen-devel
On Wed, 21 Oct 2015, Ian Campbell wrote:
> In Xen 4.7 we are refactoring parts libxenctrl into a number of
> separate libraries which will provide backward and forward API and ABI
> compatiblity.
>
> One such library will be libxenevtchn which provides access to event
> channels.
>
> In preparation for this switch the compatibility layer in xen_common.h
> (which support building with older versions of Xen) to use what will
> be the new library API. This means that the evtchn shim will disappear
> for versions of Xen which include libxenevtchn.
>
> To simplify things for the <= 4.0.0 support we wrap the int fd in a
> malloc(sizeof int) such that the handle is always a pointer. This
> leads to less typedef headaches and the need for
> XC_HANDLER_INITIAL_VALUE etc for these interfaces.
>
> Build tested with 4.0 and 4.5.
>
> Note that this patch does not add any support for actually using
> libxenevtchn, it just adjusts the existing shims.
>
> Note that xc_evtchn_alloc_unbound functionality remains in libxenctrl,
> since that functionality is not exposed by /dev/xen/evtchn.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> v4: Ran checkpatch, fixed all errors
> Allocate correct size for handle (i.e. not size of the ptr)
> ---
> hw/xen/xen_backend.c | 31 ++++++++++++++++---------------
> include/hw/xen/xen_backend.h | 2 +-
> include/hw/xen/xen_common.h | 44 ++++++++++++++++++++++++++++++++++----------
> xen-hvm.c | 25 +++++++++++++------------
> 4 files changed, 64 insertions(+), 38 deletions(-)
>
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> index b2cb22b..342ec9b 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -243,19 +243,19 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev,
> xendev->debug = debug;
> xendev->local_port = -1;
>
> - xendev->evtchndev = xen_xc_evtchn_open(NULL, 0);
> - if (xendev->evtchndev == XC_HANDLER_INITIAL_VALUE) {
> + xendev->evtchndev = xenevtchn_open(NULL, 0);
> + if (xendev->evtchndev == NULL) {
> xen_be_printf(NULL, 0, "can't open evtchn device\n");
> g_free(xendev);
> return NULL;
> }
> - fcntl(xc_evtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
> + fcntl(xenevtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
>
> if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
> xendev->gnttabdev = xen_xc_gnttab_open(NULL, 0);
> if (xendev->gnttabdev == XC_HANDLER_INITIAL_VALUE) {
> xen_be_printf(NULL, 0, "can't open gnttab device\n");
> - xc_evtchn_close(xendev->evtchndev);
> + xenevtchn_close(xendev->evtchndev);
> g_free(xendev);
> return NULL;
> }
> @@ -306,8 +306,8 @@ static struct XenDevice *xen_be_del_xendev(int dom, int dev)
> g_free(xendev->fe);
> }
>
> - if (xendev->evtchndev != XC_HANDLER_INITIAL_VALUE) {
> - xc_evtchn_close(xendev->evtchndev);
> + if (xendev->evtchndev != NULL) {
> + xenevtchn_close(xendev->evtchndev);
> }
> if (xendev->gnttabdev != XC_HANDLER_INITIAL_VALUE) {
> xc_gnttab_close(xendev->gnttabdev);
> @@ -691,13 +691,14 @@ static void xen_be_evtchn_event(void *opaque)
> struct XenDevice *xendev = opaque;
> evtchn_port_t port;
>
> - port = xc_evtchn_pending(xendev->evtchndev);
> + port = xenevtchn_pending(xendev->evtchndev);
> if (port != xendev->local_port) {
> - xen_be_printf(xendev, 0, "xc_evtchn_pending returned %d (expected %d)\n",
> + xen_be_printf(xendev, 0,
> + "xenevtchn_pending returned %d (expected %d)\n",
> port, xendev->local_port);
> return;
> }
> - xc_evtchn_unmask(xendev->evtchndev, port);
> + xenevtchn_unmask(xendev->evtchndev, port);
>
> if (xendev->ops->event) {
> xendev->ops->event(xendev);
> @@ -742,14 +743,14 @@ int xen_be_bind_evtchn(struct XenDevice *xendev)
> if (xendev->local_port != -1) {
> return 0;
> }
> - xendev->local_port = xc_evtchn_bind_interdomain
> + xendev->local_port = xenevtchn_bind_interdomain
> (xendev->evtchndev, xendev->dom, xendev->remote_port);
> if (xendev->local_port == -1) {
> - xen_be_printf(xendev, 0, "xc_evtchn_bind_interdomain failed\n");
> + xen_be_printf(xendev, 0, "xenevtchn_bind_interdomain failed\n");
> return -1;
> }
> xen_be_printf(xendev, 2, "bind evtchn port %d\n", xendev->local_port);
> - qemu_set_fd_handler(xc_evtchn_fd(xendev->evtchndev),
> + qemu_set_fd_handler(xenevtchn_fd(xendev->evtchndev),
> xen_be_evtchn_event, NULL, xendev);
> return 0;
> }
> @@ -759,15 +760,15 @@ void xen_be_unbind_evtchn(struct XenDevice *xendev)
> if (xendev->local_port == -1) {
> return;
> }
> - qemu_set_fd_handler(xc_evtchn_fd(xendev->evtchndev), NULL, NULL, NULL);
> - xc_evtchn_unbind(xendev->evtchndev, xendev->local_port);
> + qemu_set_fd_handler(xenevtchn_fd(xendev->evtchndev), NULL, NULL, NULL);
> + xenevtchn_unbind(xendev->evtchndev, xendev->local_port);
> xen_be_printf(xendev, 2, "unbind evtchn port %d\n", xendev->local_port);
> xendev->local_port = -1;
> }
>
> int xen_be_send_notify(struct XenDevice *xendev)
> {
> - return xc_evtchn_notify(xendev->evtchndev, xendev->local_port);
> + return xenevtchn_notify(xendev->evtchndev, xendev->local_port);
> }
>
> /*
> diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
> index 3b4125e..a90314f 100644
> --- a/include/hw/xen/xen_backend.h
> +++ b/include/hw/xen/xen_backend.h
> @@ -46,7 +46,7 @@ struct XenDevice {
> int remote_port;
> int local_port;
>
> - XenEvtchn evtchndev;
> + xenevtchn_handle *evtchndev;
> XenGnttab gnttabdev;
>
> struct XenDevOps *ops;
> diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> index 5923290..4dc2ee6 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -39,17 +39,38 @@ static inline void *xc_map_foreign_bulk(int xc_handle, uint32_t dom, int prot,
> #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 410
>
> typedef int XenXC;
> -typedef int XenEvtchn;
> +typedef int xenevtchn_handle;
> typedef int XenGnttab;
>
> # define XC_INTERFACE_FMT "%i"
> # define XC_HANDLER_INITIAL_VALUE -1
>
> -static inline XenEvtchn xen_xc_evtchn_open(void *logger,
> - unsigned int open_flags)
> +static inline xenevtchn_handle *xenevtchn_open(void *logger,
> + unsigned int open_flags)
> {
> - return xc_evtchn_open();
> + xenevtchn_handle *h = malloc(sizeof(*h));
> + if (!h) {
> + return NULL;
> + }
> + *h = xc_evtchn_open();
> + if (*h == -1) {
> + free(h);
> + h = NULL;
> + }
> + return h;
> }
> +static inline int xenevtchn_close(xenevtchn_handle *h)
> +{
> + int rc = xc_evtchn_close(*h);
> + free(h);
> + return rc;
> +}
> +#define xenevtchn_fd(h) xc_evtchn_fd(*h)
> +#define xenevtchn_pending(h) xc_evtchn_pending(*h)
> +#define xenevtchn_notify(h, p) xc_evtchn_notify(*h, p)
> +#define xenevtchn_bind_interdomain(h, d, p) xc_evtchn_bind_interdomain(*h, d, p)
> +#define xenevtchn_unmask(h, p) xc_evtchn_unmask(*h, p)
> +#define xenevtchn_unbind(h, p) xc_evtchn_unmask(*h, p)
>
> static inline XenGnttab xen_xc_gnttab_open(void *logger,
> unsigned int open_flags)
> @@ -108,17 +129,20 @@ static inline void xs_close(struct xs_handle *xsh)
> #else
>
> typedef xc_interface *XenXC;
> -typedef xc_evtchn *XenEvtchn;
> +typedef xc_evtchn xenevtchn_handle;
> typedef xc_gnttab *XenGnttab;
>
> # define XC_INTERFACE_FMT "%p"
> # define XC_HANDLER_INITIAL_VALUE NULL
>
> -static inline XenEvtchn xen_xc_evtchn_open(void *logger,
> - unsigned int open_flags)
> -{
> - return xc_evtchn_open(logger, open_flags);
> -}
> +#define xenevtchn_open(l, f) xc_evtchn_open(l, f);
> +#define xenevtchn_close(h) xc_evtchn_close(h)
> +#define xenevtchn_fd(h) xc_evtchn_fd(h)
> +#define xenevtchn_pending(h) xc_evtchn_pending(h)
> +#define xenevtchn_notify(h, p) xc_evtchn_notify(h, p)
> +#define xenevtchn_bind_interdomain(h, d, p) xc_evtchn_bind_interdomain(h, d, p)
> +#define xenevtchn_unmask(h, p) xc_evtchn_unmask(h, p)
> +#define xenevtchn_unbind(h, p) xc_evtchn_unbind(h, p)
>
> static inline XenGnttab xen_xc_gnttab_open(void *logger,
> unsigned int open_flags)
> diff --git a/xen-hvm.c b/xen-hvm.c
> index 3a7fd58..4470d58 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -109,7 +109,7 @@ typedef struct XenIOState {
> /* evtchn local port for buffered io */
> evtchn_port_t bufioreq_local_port;
> /* the evtchn fd for polling */
> - XenEvtchn xce_handle;
> + xenevtchn_handle *xce_handle;
> /* which vcpu we are serving */
> int send_vcpu;
>
> @@ -709,7 +709,7 @@ static ioreq_t *cpu_get_ioreq(XenIOState *state)
> int i;
> evtchn_port_t port;
>
> - port = xc_evtchn_pending(state->xce_handle);
> + port = xenevtchn_pending(state->xce_handle);
> if (port == state->bufioreq_local_port) {
> timer_mod(state->buffered_io_timer,
> BUFFER_IO_MAX_DELAY + qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
> @@ -728,7 +728,7 @@ static ioreq_t *cpu_get_ioreq(XenIOState *state)
> }
>
> /* unmask the wanted port again */
> - xc_evtchn_unmask(state->xce_handle, port);
> + xenevtchn_unmask(state->xce_handle, port);
>
> /* get the io packet from shared memory */
> state->send_vcpu = i;
> @@ -1014,7 +1014,7 @@ static void handle_buffered_io(void *opaque)
> BUFFER_IO_MAX_DELAY + qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
> } else {
> timer_del(state->buffered_io_timer);
> - xc_evtchn_unmask(state->xce_handle, state->bufioreq_local_port);
> + xenevtchn_unmask(state->xce_handle, state->bufioreq_local_port);
> }
> }
>
> @@ -1058,7 +1058,8 @@ static void cpu_handle_ioreq(void *opaque)
> }
>
> req->state = STATE_IORESP_READY;
> - xc_evtchn_notify(state->xce_handle, state->ioreq_local_port[state->send_vcpu]);
> + xenevtchn_notify(state->xce_handle,
> + state->ioreq_local_port[state->send_vcpu]);
> }
> }
>
> @@ -1066,8 +1067,8 @@ static void xen_main_loop_prepare(XenIOState *state)
> {
> int evtchn_fd = -1;
>
> - if (state->xce_handle != XC_HANDLER_INITIAL_VALUE) {
> - evtchn_fd = xc_evtchn_fd(state->xce_handle);
> + if (state->xce_handle != NULL) {
> + evtchn_fd = xenevtchn_fd(state->xce_handle);
> }
>
> state->buffered_io_timer = timer_new_ms(QEMU_CLOCK_REALTIME, handle_buffered_io,
> @@ -1105,7 +1106,7 @@ static void xen_exit_notifier(Notifier *n, void *data)
> {
> XenIOState *state = container_of(n, XenIOState, exit);
>
> - xc_evtchn_close(state->xce_handle);
> + xenevtchn_close(state->xce_handle);
> xs_daemon_close(state->xenstore);
> }
>
> @@ -1174,8 +1175,8 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
>
> state = g_malloc0(sizeof (XenIOState));
>
> - state->xce_handle = xen_xc_evtchn_open(NULL, 0);
> - if (state->xce_handle == XC_HANDLER_INITIAL_VALUE) {
> + state->xce_handle = xenevtchn_open(NULL, 0);
> + if (state->xce_handle == NULL) {
> perror("xen: event channel open");
> return -1;
> }
> @@ -1255,7 +1256,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
>
> /* FIXME: how about if we overflow the page here? */
> for (i = 0; i < max_cpus; i++) {
> - rc = xc_evtchn_bind_interdomain(state->xce_handle, xen_domid,
> + rc = xenevtchn_bind_interdomain(state->xce_handle, xen_domid,
> xen_vcpu_eport(state->shared_page, i));
> if (rc == -1) {
> fprintf(stderr, "shared evtchn %d bind error %d\n", i, errno);
> @@ -1264,7 +1265,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> state->ioreq_local_port[i] = rc;
> }
>
> - rc = xc_evtchn_bind_interdomain(state->xce_handle, xen_domid,
> + rc = xenevtchn_bind_interdomain(state->xce_handle, xen_domid,
> bufioreq_evtchn);
> if (rc == -1) {
> fprintf(stderr, "buffered evtchn bind error %d\n", errno);
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 3/9] xen: Switch to libxengnttab interface for compat shims.
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 3/9] xen: Switch to libxengnttab " Ian Campbell
@ 2015-10-23 11:06 ` Stefano Stabellini
2015-10-23 11:15 ` Ian Campbell
2015-10-23 12:42 ` Ian Campbell
0 siblings, 2 replies; 25+ messages in thread
From: Stefano Stabellini @ 2015-10-23 11:06 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, stefano.stabellini, ian.jackson, qemu-devel, xen-devel
On Wed, 21 Oct 2015, Ian Campbell wrote:
> In Xen 4.7 we are refactoring parts libxenctrl into a number of
> separate libraries which will provide backward and forward API and ABI
> compatiblity.
>
> One such library will be libxengnttab which provides access to grant
> tables.
>
> In preparation for this switch the compatibility layer in xen_common.h
> (which support building with older versions of Xen) to use what will
> be the new library API. This means that the gnttab shim will disappear
> for versions of Xen which include libxengnttab.
>
> To simplify things for the <= 4.0.0 support we wrap the int fd in a
> malloc(sizeof int) such that the handle is always a pointer. This
> leads to less typedef headaches and the need for
> XC_HANDLER_INITIAL_VALUE etc for these interfaces.
>
> Build tested with 4.0 and 4.5.
>
> Note that this patch does not add any support for actually using
> libxengnttab, it just adjusts the existing shims.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
The patch looks OK but doesn't apply cleanly to master, please rebase.
After fixing it up, it fails my 4.0 build test (I probably made a
mistake).
> v4: Ran checkpatch, fixed all errors
> Allocate correct size for handle (i.e. not size of the ptr)
> Rebase onto "xen_console: correctly cleanup primary console on
> teardown."
> ---
> hw/block/xen_disk.c | 38 ++++++++++++++++++++------------------
> hw/char/xen_console.c | 4 ++--
> hw/net/xen_nic.c | 16 ++++++++--------
> hw/xen/xen_backend.c | 10 +++++-----
> include/hw/xen/xen_backend.h | 2 +-
> include/hw/xen/xen_common.h | 42 ++++++++++++++++++++++++++++++++----------
> 6 files changed, 68 insertions(+), 44 deletions(-)
>
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 21842a0..15413f6 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -172,11 +172,11 @@ 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;
> + xengnttab_handle *gnt = grant->blkdev->xendev.gnttabdev;
>
> - if (xc_gnttab_munmap(gnt, grant->page, 1) != 0) {
> + if (xengnttab_munmap(gnt, grant->page, 1) != 0) {
> xen_be_printf(&grant->blkdev->xendev, 0,
> - "xc_gnttab_munmap failed: %s\n",
> + "xengnttab_munmap failed: %s\n",
> strerror(errno));
> }
> grant->blkdev->persistent_gnt_count--;
> @@ -189,11 +189,11 @@ static void remove_persistent_region(gpointer data, gpointer dev)
> {
> PersistentRegion *region = data;
> struct XenBlkDev *blkdev = dev;
> - XenGnttab gnt = blkdev->xendev.gnttabdev;
> + xengnttab_handle *gnt = blkdev->xendev.gnttabdev;
>
> - if (xc_gnttab_munmap(gnt, region->addr, region->num) != 0) {
> + if (xengnttab_munmap(gnt, region->addr, region->num) != 0) {
> xen_be_printf(&blkdev->xendev, 0,
> - "xc_gnttab_munmap region %p failed: %s\n",
> + "xengnttab_munmap region %p failed: %s\n",
> region->addr, strerror(errno));
> }
> xen_be_printf(&blkdev->xendev, 3,
> @@ -328,7 +328,7 @@ err:
>
> static void ioreq_unmap(struct ioreq *ioreq)
> {
> - XenGnttab gnt = ioreq->blkdev->xendev.gnttabdev;
> + xengnttab_handle *gnt = ioreq->blkdev->xendev.gnttabdev;
> int i;
>
> if (ioreq->num_unmap == 0 || ioreq->mapped == 0) {
> @@ -338,8 +338,9 @@ static void ioreq_unmap(struct ioreq *ioreq)
> if (!ioreq->pages) {
> return;
> }
> - if (xc_gnttab_munmap(gnt, ioreq->pages, ioreq->num_unmap) != 0) {
> - xen_be_printf(&ioreq->blkdev->xendev, 0, "xc_gnttab_munmap failed: %s\n",
> + if (xengnttab_munmap(gnt, ioreq->pages, ioreq->num_unmap) != 0) {
> + xen_be_printf(&ioreq->blkdev->xendev, 0,
> + "xengnttab_munmap failed: %s\n",
> strerror(errno));
> }
> ioreq->blkdev->cnt_map -= ioreq->num_unmap;
> @@ -349,8 +350,9 @@ static void ioreq_unmap(struct ioreq *ioreq)
> if (!ioreq->page[i]) {
> continue;
> }
> - if (xc_gnttab_munmap(gnt, ioreq->page[i], 1) != 0) {
> - xen_be_printf(&ioreq->blkdev->xendev, 0, "xc_gnttab_munmap failed: %s\n",
> + if (xengnttab_munmap(gnt, ioreq->page[i], 1) != 0) {
> + xen_be_printf(&ioreq->blkdev->xendev, 0,
> + "xengnttab_munmap failed: %s\n",
> strerror(errno));
> }
> ioreq->blkdev->cnt_map--;
> @@ -362,7 +364,7 @@ static void ioreq_unmap(struct ioreq *ioreq)
>
> static int ioreq_map(struct ioreq *ioreq)
> {
> - XenGnttab gnt = ioreq->blkdev->xendev.gnttabdev;
> + xengnttab_handle *gnt = ioreq->blkdev->xendev.gnttabdev;
> uint32_t domids[BLKIF_MAX_SEGMENTS_PER_REQUEST];
> uint32_t refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
> void *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
> @@ -413,7 +415,7 @@ static int ioreq_map(struct ioreq *ioreq)
> }
>
> if (batch_maps && new_maps) {
> - ioreq->pages = xc_gnttab_map_grant_refs
> + ioreq->pages = xengnttab_map_grant_refs
> (gnt, new_maps, domids, refs, ioreq->prot);
> if (ioreq->pages == NULL) {
> xen_be_printf(&ioreq->blkdev->xendev, 0,
> @@ -429,7 +431,7 @@ static int ioreq_map(struct ioreq *ioreq)
> ioreq->blkdev->cnt_map += new_maps;
> } else if (new_maps) {
> for (i = 0; i < new_maps; i++) {
> - ioreq->page[i] = xc_gnttab_map_grant_ref
> + ioreq->page[i] = xengnttab_map_grant_ref
> (gnt, domids[i], refs[i], ioreq->prot);
> if (ioreq->page[i] == NULL) {
> xen_be_printf(&ioreq->blkdev->xendev, 0,
> @@ -762,9 +764,9 @@ static void blk_alloc(struct XenDevice *xendev)
> if (xen_mode != XEN_EMULATE) {
> batch_maps = 1;
> }
> - if (xc_gnttab_set_max_grants(xendev->gnttabdev,
> + if (xengnttab_set_max_grants(xendev->gnttabdev,
> MAX_GRANTS(max_requests, BLKIF_MAX_SEGMENTS_PER_REQUEST)) < 0) {
> - xen_be_printf(xendev, 0, "xc_gnttab_set_max_grants failed: %s\n",
> + xen_be_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
> strerror(errno));
> }
> }
> @@ -976,7 +978,7 @@ static int blk_connect(struct XenDevice *xendev)
> }
> }
>
> - blkdev->sring = xc_gnttab_map_grant_ref(blkdev->xendev.gnttabdev,
> + blkdev->sring = xengnttab_map_grant_ref(blkdev->xendev.gnttabdev,
> blkdev->xendev.dom,
> blkdev->ring_ref,
> PROT_READ | PROT_WRITE);
> @@ -1041,7 +1043,7 @@ static void blk_disconnect(struct XenDevice *xendev)
> xen_be_unbind_evtchn(&blkdev->xendev);
>
> if (blkdev->sring) {
> - xc_gnttab_munmap(blkdev->xendev.gnttabdev, blkdev->sring, 1);
> + xengnttab_munmap(blkdev->xendev.gnttabdev, blkdev->sring, 1);
> blkdev->cnt_map--;
> blkdev->sring = NULL;
> }
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index 63ade33..8c06c19 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -233,7 +233,7 @@ static int con_initialise(struct XenDevice *xendev)
> PROT_READ|PROT_WRITE,
> con->ring_ref);
> } else {
> - con->sring = xc_gnttab_map_grant_ref(xendev->gnttabdev, con->xendev.dom,
> + con->sring = xengnttab_map_grant_ref(xendev->gnttabdev, con->xendev.dom,
> con->ring_ref,
> PROT_READ|PROT_WRITE);
> }
> @@ -275,7 +275,7 @@ static void con_disconnect(struct XenDevice *xendev)
> if (!xendev->dev) {
> munmap(con->sring, XC_PAGE_SIZE);
> } else {
> - xc_gnttab_munmap(xendev->gnttabdev, con->sring, 1);
> + xengnttab_munmap(xendev->gnttabdev, con->sring, 1);
> }
> con->sring = NULL;
> }
> diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
> index 63918ae..778de28 100644
> --- a/hw/net/xen_nic.c
> +++ b/hw/net/xen_nic.c
> @@ -169,7 +169,7 @@ static void net_tx_packets(struct XenNetDev *netdev)
> (txreq.flags & NETTXF_more_data) ? " more_data" : "",
> (txreq.flags & NETTXF_extra_info) ? " extra_info" : "");
>
> - page = xc_gnttab_map_grant_ref(netdev->xendev.gnttabdev,
> + page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> netdev->xendev.dom,
> txreq.gref, PROT_READ);
> if (page == NULL) {
> @@ -191,7 +191,7 @@ static void net_tx_packets(struct XenNetDev *netdev)
> qemu_send_packet(qemu_get_queue(netdev->nic),
> page + txreq.offset, txreq.size);
> }
> - xc_gnttab_munmap(netdev->xendev.gnttabdev, page, 1);
> + xengnttab_munmap(netdev->xendev.gnttabdev, page, 1);
> net_tx_response(netdev, &txreq, NETIF_RSP_OKAY);
> }
> if (!netdev->tx_work) {
> @@ -283,7 +283,7 @@ static ssize_t net_rx_packet(NetClientState *nc, const uint8_t *buf, size_t size
> memcpy(&rxreq, RING_GET_REQUEST(&netdev->rx_ring, rc), sizeof(rxreq));
> netdev->rx_ring.req_cons = ++rc;
>
> - page = xc_gnttab_map_grant_ref(netdev->xendev.gnttabdev,
> + page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> netdev->xendev.dom,
> rxreq.gref, PROT_WRITE);
> if (page == NULL) {
> @@ -293,7 +293,7 @@ static ssize_t net_rx_packet(NetClientState *nc, const uint8_t *buf, size_t size
> return -1;
> }
> memcpy(page + NET_IP_ALIGN, buf, size);
> - xc_gnttab_munmap(netdev->xendev.gnttabdev, page, 1);
> + xengnttab_munmap(netdev->xendev.gnttabdev, page, 1);
> net_rx_response(netdev, &rxreq, NETIF_RSP_OKAY, NET_IP_ALIGN, size, 0);
>
> return size;
> @@ -366,11 +366,11 @@ static int net_connect(struct XenDevice *xendev)
> return -1;
> }
>
> - netdev->txs = xc_gnttab_map_grant_ref(netdev->xendev.gnttabdev,
> + netdev->txs = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> netdev->xendev.dom,
> netdev->tx_ring_ref,
> PROT_READ | PROT_WRITE);
> - netdev->rxs = xc_gnttab_map_grant_ref(netdev->xendev.gnttabdev,
> + netdev->rxs = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> netdev->xendev.dom,
> netdev->rx_ring_ref,
> PROT_READ | PROT_WRITE);
> @@ -398,11 +398,11 @@ static void net_disconnect(struct XenDevice *xendev)
> xen_be_unbind_evtchn(&netdev->xendev);
>
> if (netdev->txs) {
> - xc_gnttab_munmap(netdev->xendev.gnttabdev, netdev->txs, 1);
> + xengnttab_munmap(netdev->xendev.gnttabdev, netdev->txs, 1);
> netdev->txs = NULL;
> }
> if (netdev->rxs) {
> - xc_gnttab_munmap(netdev->xendev.gnttabdev, netdev->rxs, 1);
> + xengnttab_munmap(netdev->xendev.gnttabdev, netdev->rxs, 1);
> netdev->rxs = NULL;
> }
> if (netdev->nic) {
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> index 342ec9b..f988b95 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -252,15 +252,15 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev,
> fcntl(xenevtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
>
> if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
> - xendev->gnttabdev = xen_xc_gnttab_open(NULL, 0);
> - if (xendev->gnttabdev == XC_HANDLER_INITIAL_VALUE) {
> + xendev->gnttabdev = xengnttab_open(NULL, 0);
> + if (xendev->gnttabdev == NULL) {
> xen_be_printf(NULL, 0, "can't open gnttab device\n");
> xenevtchn_close(xendev->evtchndev);
> g_free(xendev);
> return NULL;
> }
> } else {
> - xendev->gnttabdev = XC_HANDLER_INITIAL_VALUE;
> + xendev->gnttabdev = NULL;
> }
>
> QTAILQ_INSERT_TAIL(&xendevs, xendev, next);
> @@ -309,8 +309,8 @@ static struct XenDevice *xen_be_del_xendev(int dom, int dev)
> if (xendev->evtchndev != NULL) {
> xenevtchn_close(xendev->evtchndev);
> }
> - if (xendev->gnttabdev != XC_HANDLER_INITIAL_VALUE) {
> - xc_gnttab_close(xendev->gnttabdev);
> + if (xendev->gnttabdev != NULL) {
> + xengnttab_close(xendev->gnttabdev);
> }
>
> QTAILQ_REMOVE(&xendevs, xendev, next);
> diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
> index a90314f..8e8857b 100644
> --- a/include/hw/xen/xen_backend.h
> +++ b/include/hw/xen/xen_backend.h
> @@ -47,7 +47,7 @@ struct XenDevice {
> int local_port;
>
> xenevtchn_handle *evtchndev;
> - XenGnttab gnttabdev;
> + xengnttab_handle *gnttabdev;
>
> struct XenDevOps *ops;
> QTAILQ_ENTRY(XenDevice) next;
> diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> index 4dc2ee6..db62df4 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -40,7 +40,7 @@ static inline void *xc_map_foreign_bulk(int xc_handle, uint32_t dom, int prot,
>
> typedef int XenXC;
> typedef int xenevtchn_handle;
> -typedef int XenGnttab;
> +typedef int xengnttab_handle;
>
> # define XC_INTERFACE_FMT "%i"
> # define XC_HANDLER_INITIAL_VALUE -1
> @@ -72,11 +72,31 @@ static inline int xenevtchn_close(xenevtchn_handle *h)
> #define xenevtchn_unmask(h, p) xc_evtchn_unmask(*h, p)
> #define xenevtchn_unbind(h, p) xc_evtchn_unmask(*h, p)
>
> -static inline XenGnttab xen_xc_gnttab_open(void *logger,
> - unsigned int open_flags)
> +static inline xengnttab_handle *xengnttab_open(void *logger,
> + unsigned int open_flags)
> {
> - return xc_gnttab_open();
> + xengnttab_handle *h = malloc(sizeof(*h));
> + if (!h) {
> + return NULL;
> + }
> + *h = xc_gnttab_open();
> + if (*h == -1) {
> + free(h);
> + h = NULL;
> + }
> + return h;
> }
> +static inline int xengnttab_close(xengnttab_handle *h)
> +{
> + int rc = xc_gnttab_close(*h);
> + free(h);
> + return rc;
> +}
> +#define xengnttab_set_max_grants(h, n) xc_gnttab_set_max_grants(*h, n)
> +#define xengnttab_map_grant_ref(h, d, r, p) xc_gnttab_map_grant_ref(*h, d, r, p)
> +#define xengnttab_map_grant_refs(h, c, d, r, p) \
> + xc_gnttab_map_grant_refs(*h, c, d, r, p)
> +#define xengnttab_munmap(h, a, n) xc_gnttab_munmap(*h, a, n)
>
> static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
> unsigned int open_flags)
> @@ -130,7 +150,7 @@ static inline void xs_close(struct xs_handle *xsh)
>
> typedef xc_interface *XenXC;
> typedef xc_evtchn xenevtchn_handle;
> -typedef xc_gnttab *XenGnttab;
> +typedef xc_gnttab xengnttab_handle;
>
> # define XC_INTERFACE_FMT "%p"
> # define XC_HANDLER_INITIAL_VALUE NULL
> @@ -144,11 +164,13 @@ typedef xc_gnttab *XenGnttab;
> #define xenevtchn_unmask(h, p) xc_evtchn_unmask(h, p)
> #define xenevtchn_unbind(h, p) xc_evtchn_unbind(h, p)
>
> -static inline XenGnttab xen_xc_gnttab_open(void *logger,
> - unsigned int open_flags)
> -{
> - return xc_gnttab_open(logger, open_flags);
> -}
> +#define xengnttab_open(l, f) xc_gnttab_open(l, f)
> +#define xengnttab_close(h) xc_gnttab_close(h)
> +#define xengnttab_set_max_grants(h, n) xc_gnttab_set_max_grants(h, n)
> +#define xengnttab_map_grant_ref(h, d, r, p) xc_gnttab_map_grant_ref(h, d, r, p)
> +#define xengnttab_munmap(h, a, n) xc_gnttab_munmap(h, a, n)
> +#define xengnttab_map_grant_refs(h, c, d, r, p) \
> + xc_gnttab_map_grant_refs(h, c, d, r, p)
>
> static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
> unsigned int open_flags)
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 6/9] xen: Switch uses of xc_map_foreign_bulk to use libxenforeignmemory API.
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 6/9] xen: Switch uses of xc_map_foreign_bulk to use libxenforeignmemory API Ian Campbell
@ 2015-10-23 11:06 ` Stefano Stabellini
0 siblings, 0 replies; 25+ messages in thread
From: Stefano Stabellini @ 2015-10-23 11:06 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, stefano.stabellini, ian.jackson, qemu-devel, xen-devel
On Wed, 21 Oct 2015, Ian Campbell wrote:
> In Xen 4.7 we are refactoring parts libxenctrl into a number of
> separate libraries which will provide backward and forward API and ABI
> compatiblity.
>
> One such library will be libxenforeignmemory which provides access to
> privileged foreign mappings and which will provide an interface
> equivalent to xc_map_foreign_bulk.
>
> In preparation for adding support for libxenforeignmemory add support
> to the <=4.0 and <=4.6 compat code in xen_common.h to allow us to
> switch to using the new API. These shims will disappear for versions
> of Xen which include libxenforeignmemory.
>
> Since libxenforeignmemory will have its own handle type but for <= 4.6
> the functionality is provided by using a libxenctrl handle we
> introduce a new global xen_fmem alongside the existing xen_xc. In fact
> we make xen_fmem a pointer to the existing xen_xc, which then works
> correctly with both <=4.0 (xc handle is an int) and <=4.6 (xc handle
> is a pointer). In the latter case xen_fmem is actually a double
> indirect pointer, but it all falls out in the wash.
>
> Unlike libxenctrl libxenforeignmemory has an explicit unmap function,
> rather than just specifying that munmap should be used, so the unmap
> paths are updated to use xenforeignmemory_unmap, which is a shim for
> munmap on these versions of xen. The mappings in xen-hvm.c do not
> appear to be unmapped (which makes sense for a qemu-dm process)
>
> In fb_disconnect this results in a change from simply mmap over the
> existing mapping (with an implciit munmap) to expliclty unmapping with
^ implicit
> xenforeignmemory_unmap and then mapping the required anonymous memory
> in the same hole. I don't think this is a problem since any other
> thread which was racily touching this region would already be running
> the risk of hitting the mapping halfway through the call. If this is
> thought to be a problem then we could consider adding an extra API to
> the libxenforeignmemory interface to replace a foreign mapping with
> anonymous shared memory, but I'd prefer not to.
>
> Build tested with 4.0 and 4.5.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> I noticed in xen_console.c that the decision to use a foreign
> privileged memory mapping vs a grant dev is made using different
> variables in con_initialise vs con_disconnect. The former uses
> xendev->dev while the latter uses xendev->gnttabdev. Is this a latent
> bug?
>
> v4: Rebase onto "xen_console: correctly cleanup primary console on
> teardown."
>
> xenforeignmemory_unmap takes pages not bytes
>
> Compat wrapper for xenforeignmemory_open instead of ifdef in code.
>
> Run check patch and fix most issues. I did not fix:
>
> ERROR: do not initialise globals to 0 or NULL
> +xenforeignmemory_handle *xen_fmem = NULL;
>
> => This is consistent with all of the existing declarations.
>
> ERROR: need consistent spacing around '*' (ctx:WxV)
> +typedef xc_interface *xenforeignmemory_handle;
>
> => I think this is a false +ve since this is a pointer "*" not a multiple "*".
> ---
> hw/char/xen_console.c | 8 ++++----
> hw/display/xenfb.c | 15 ++++++++-------
> hw/xen/xen_backend.c | 3 ++-
> include/hw/xen/xen_backend.h | 1 +
> include/hw/xen/xen_common.h | 12 ++++++++++++
> xen-common.c | 6 ++++++
> xen-hvm.c | 18 +++++++++---------
> xen-mapcache.c | 6 +++---
> 8 files changed, 45 insertions(+), 24 deletions(-)
>
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index 11c6472..24f3a40 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -230,9 +230,9 @@ static int con_initialise(struct XenDevice *xendev)
>
> if (!xendev->dev) {
> xen_pfn_t mfn = con->ring_ref;
> - con->sring = xc_map_foreign_bulk(xen_xc, con->xendev.dom,
> - PROT_READ|PROT_WRITE,
> - &mfn, &err, 1);
> + con->sring = xenforeignmemory_map(xen_fmem, con->xendev.dom,
> + PROT_READ|PROT_WRITE,
> + &mfn, &err, 1);
> } else {
> con->sring = xengnttab_map_grant_ref(xendev->gnttabdev, con->xendev.dom,
> con->ring_ref,
> @@ -274,7 +274,7 @@ static void con_disconnect(struct XenDevice *xendev)
>
> if (con->sring) {
> if (!xendev->dev) {
> - munmap(con->sring, XC_PAGE_SIZE);
> + xenforeignmemory_unmap(xen_fmem, con->sring, 1);
> } else {
> xengnttab_munmap(xendev->gnttabdev, con->sring, 1);
> }
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index b0ac1e6..a5ddb60 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -103,8 +103,8 @@ static int common_bind(struct common *c)
> if (xenstore_read_fe_int(&c->xendev, "event-channel", &c->xendev.remote_port) == -1)
> return -1;
>
> - c->page = xc_map_foreign_bulk(xen_xc, c->xendev.dom,
> - PROT_READ | PROT_WRITE, &mfn, &err, 1);
> + c->page = xenforeignmemory_map(xen_fmem, c->xendev.dom,
> + PROT_READ | PROT_WRITE, &mfn, &err, 1);
> if (c->page == NULL)
> return -1;
>
> @@ -119,7 +119,7 @@ static void common_unbind(struct common *c)
> {
> xen_be_unbind_evtchn(&c->xendev);
> if (c->page) {
> - munmap(c->page, XC_PAGE_SIZE);
> + xenforeignmemory_unmap(xen_fmem, c->page, 1);
> c->page = NULL;
> }
> }
> @@ -491,14 +491,14 @@ static int xenfb_map_fb(struct XenFB *xenfb)
> errs = g_malloc0(sizeof(int) * n_fbdirs);
>
> xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd);
> - map = xc_map_foreign_bulk(xen_xc, xenfb->c.xendev.dom,
> - PROT_READ, pgmfns, errs, n_fbdirs);
> + map = xenforeignmemory_map(xen_fmem, xenfb->c.xendev.dom,
> + PROT_READ, pgmfns, errs, n_fbdirs);
> if (map == NULL)
> goto out;
> xenfb_copy_mfns(mode, xenfb->fbpages, fbmfns, map);
> - munmap(map, n_fbdirs * XC_PAGE_SIZE);
> + xenforeignmemory_unmap(xen_fmem, map, n_fbdirs);
>
> - xenfb->pixels = xc_map_foreign_bulk(xen_xc, xenfb->c.xendev.dom,
> + xenfb->pixels = xenforeignmemory_map(xen_fmem, xenfb->c.xendev.dom,
> PROT_READ, fbmfns, errs, xenfb->fbpages);
> if (xenfb->pixels == NULL)
> goto out;
> @@ -907,6 +907,7 @@ static void fb_disconnect(struct XenDevice *xendev)
> * Replacing the framebuffer with anonymous shared memory
> * instead. This releases the guest pages and keeps qemu happy.
> */
> + xenforeignmemory_unmap(xen_fmem, fb->pixels, fb->fbpages);
> fb->pixels = mmap(fb->pixels, fb->fbpages * XC_PAGE_SIZE,
> PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANON,
> -1, 0);
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> index f988b95..30ffcb4 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -45,6 +45,7 @@
>
> /* public */
> XenXC xen_xc = XC_HANDLER_INITIAL_VALUE;
> +xenforeignmemory_handle *xen_fmem = NULL;
> struct xs_handle *xenstore = NULL;
> const char *xen_protocol;
>
> @@ -719,7 +720,7 @@ int xen_be_init(void)
> goto err;
> }
>
> - if (xen_xc == XC_HANDLER_INITIAL_VALUE) {
> + if (xen_xc == XC_HANDLER_INITIAL_VALUE || xen_fmem == NULL) {
> /* Check if xen_init() have been called */
> goto err;
> }
> diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
> index 8e8857b..e0d52ee 100644
> --- a/include/hw/xen/xen_backend.h
> +++ b/include/hw/xen/xen_backend.h
> @@ -57,6 +57,7 @@ struct XenDevice {
>
> /* variables */
> extern XenXC xen_xc;
> +extern xenforeignmemory_handle *xen_fmem;
> extern struct xs_handle *xenstore;
> extern const char *xen_protocol;
>
> diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> index db62df4..2a5f27a 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -41,6 +41,7 @@ static inline void *xc_map_foreign_bulk(int xc_handle, uint32_t dom, int prot,
> typedef int XenXC;
> typedef int xenevtchn_handle;
> typedef int xengnttab_handle;
> +typedef int xenforeignmemory_handle;
>
> # define XC_INTERFACE_FMT "%i"
> # define XC_HANDLER_INITIAL_VALUE -1
> @@ -104,6 +105,11 @@ static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
> return xc_interface_open();
> }
>
> +#define xenforeignmemory_open(l, f) &xen_xc
> +#define xenforeignmemory_map(h, d, p, a, e, n) \
> + xc_map_foreign_bulk(*h, d, p, a, e, n)
> +#define xenforeignmemory_unmap(h, p, s) munmap(p, s * XC_PAGE_SIZE)
> +
> static inline int xc_fd(int xen_xc)
> {
> return xen_xc;
> @@ -149,6 +155,7 @@ static inline void xs_close(struct xs_handle *xsh)
> #else
>
> typedef xc_interface *XenXC;
> +typedef xc_interface *xenforeignmemory_handle;
> typedef xc_evtchn xenevtchn_handle;
> typedef xc_gnttab xengnttab_handle;
>
> @@ -178,6 +185,11 @@ static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
> return xc_interface_open(logger, dombuild_logger, open_flags);
> }
>
> +#define xenforeignmemory_open(l, f) &xen_xc
> +#define xenforeignmemory_map(h, d, p, a, e, n) \
> + xc_map_foreign_bulk(*h, d, p, a, e, n)
> +#define xenforeignmemory_unmap(h, p, s) munmap(p, s * XC_PAGE_SIZE)
> +
> /* FIXME There is now way to have the xen fd */
> static inline int xc_fd(xc_interface *xen_xc)
> {
> diff --git a/xen-common.c b/xen-common.c
> index 56359ca..dd8619b 100644
> --- a/xen-common.c
> +++ b/xen-common.c
> @@ -117,6 +117,12 @@ static int xen_init(MachineState *ms)
> xen_be_printf(NULL, 0, "can't open xen interface\n");
> return -1;
> }
> + xen_fmem = xenforeignmemory_open(0, 0);
> + if (xen_fmem == NULL) {
> + xen_be_printf(NULL, 0, "can't open xen fmem interface\n");
> + xc_interface_close(xen_xc);
> + return -1;
> + }
> qemu_add_vm_change_state_handler(xen_change_state_handler, NULL);
>
> return 0;
> diff --git a/xen-hvm.c b/xen-hvm.c
> index 0c84e30..8fe7397 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -1214,9 +1214,9 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> DPRINTF("buffered io page at pfn %lx\n", bufioreq_pfn);
> DPRINTF("buffered io evtchn is %x\n", bufioreq_evtchn);
>
> - state->shared_page = xc_map_foreign_bulk(xen_xc, xen_domid,
> - PROT_READ|PROT_WRITE,
> - &ioreq_pfn, &map_err, 1);
> + state->shared_page = xenforeignmemory_map(xen_fmem, xen_domid,
> + PROT_READ|PROT_WRITE,
> + &ioreq_pfn, &map_err, 1);
> if (state->shared_page == NULL) {
> hw_error(
> "map shared IO page returned error %d(%d) handle=" XC_INTERFACE_FMT,
> @@ -1227,8 +1227,8 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> if (!rc) {
> DPRINTF("shared vmport page at pfn %lx\n", ioreq_pfn);
> state->shared_vmport_page =
> - xc_map_foreign_bulk(xen_xc, xen_domid, PROT_READ|PROT_WRITE,
> - &ioreq_pfn, &map_err, 1);
> + xenforeignmemory_map(xen_fmem, xen_domid, PROT_READ|PROT_WRITE,
> + &ioreq_pfn, &map_err, 1);
> if (state->shared_vmport_page == NULL) {
> hw_error("map shared vmport IO page returned error %d (%d) handle="
> XC_INTERFACE_FMT, errno, map_err, xen_xc);
> @@ -1237,10 +1237,10 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> hw_error("get vmport regs pfn returned error %d, rc=%d", errno, rc);
> }
>
> - state->buffered_io_page = xc_map_foreign_bulk(xen_xc, xen_domid,
> - PROT_READ|PROT_WRITE,
> - &bufioreq_pfn,
> - &map_err, 1);
> + state->buffered_io_page = xenforeignmemory_map(xen_fmem, xen_domid,
> + PROT_READ|PROT_WRITE,
> + &bufioreq_pfn,
> + &map_err, 1);
> if (state->buffered_io_page == NULL) {
> hw_error("map buffered IO page returned error %d (%d)", errno, map_err);
> }
> diff --git a/xen-mapcache.c b/xen-mapcache.c
> index 66da1a6..7d4953c 100644
> --- a/xen-mapcache.c
> +++ b/xen-mapcache.c
> @@ -169,10 +169,10 @@ static void xen_remap_bucket(MapCacheEntry *entry,
> pfns[i] = (address_index << (MCACHE_BUCKET_SHIFT-XC_PAGE_SHIFT)) + i;
> }
>
> - vaddr_base = xc_map_foreign_bulk(xen_xc, xen_domid, PROT_READ|PROT_WRITE,
> - pfns, err, nb_pfn);
> + vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid, PROT_READ|PROT_WRITE,
> + pfns, err, nb_pfn);
> if (vaddr_base == NULL) {
> - perror("xc_map_foreign_bulk");
> + perror("xenforeignmemory_map");
> exit(-1);
> }
>
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 4/9] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_bulk
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 4/9] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_bulk Ian Campbell
@ 2015-10-23 11:07 ` Stefano Stabellini
0 siblings, 0 replies; 25+ messages in thread
From: Stefano Stabellini @ 2015-10-23 11:07 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, stefano.stabellini, ian.jackson, qemu-devel, xen-devel
On Wed, 21 Oct 2015, Ian Campbell wrote:
> In Xen 4.7 we are refactoring parts libxenctrl into a number of
> separate libraries which will provide backward and forward API and ABI
> compatiblity.
>
> One such library will be libxenforeignmemory which provides access to
> privileged foreign mappings and which will provide an interface
> equivalent to xc_map_foreign_bulk.
>
> In preparation for this switch all uses of xc_map_foreign_range to
> xc_map_foreign_bulk. This is trivial because size was always
> XC_PAGE_SIZE so the necessary adjustments are trivial:
>
> * Pass &mfn (an array of length 1) instead of mfn. The function
> takes a pointer to const, so there is no possibily of mfn changing
> due to this change.
> * Add a local err variable to receive the errors and pass &err
> (again, an array of length 1)
> * Adjust any existing logging to include err too
> * Pass nr_pages=1 instead of size=XC_PAGE_SIZE
>
> There is one wrinkle in xen_console.c:con_initialise() where
> con->ring_ref is an int but can in some code paths (when !xendev->dev)
> be treated as an mfn. I think this is an existing latent truncation
> hazard on platforms where xen_pfn_t is 64-bit and int is 32-bit (e.g.
> amd64, both arm* variants). I'm unsure under what circumstances
> xendev->dev can be NULL or if anything elsewhere ensures the value
> fits into an int. For now I just use a temporary xen_pfn_t to in
> effect upcast the pointer from int* to xen_pfn_t*.
>
> Build tested with 4.0 and 4.5.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> v4: Ran checkpatch, fixed all errors
> Mention the const-ness of the mfn array in the commit log
> ---
> hw/char/xen_console.c | 9 +++++----
> hw/display/xenfb.c | 6 +++---
> xen-hvm.c | 30 ++++++++++++++++--------------
> 3 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index 8c06c19..11c6472 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -219,6 +219,7 @@ static int con_initialise(struct XenDevice *xendev)
> {
> struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
> int limit;
> + int err;
>
> if (xenstore_read_int(con->console, "ring-ref", &con->ring_ref) == -1)
> return -1;
> @@ -228,10 +229,10 @@ static int con_initialise(struct XenDevice *xendev)
> con->buffer.max_capacity = limit;
>
> if (!xendev->dev) {
> - con->sring = xc_map_foreign_range(xen_xc, con->xendev.dom,
> - XC_PAGE_SIZE,
> - PROT_READ|PROT_WRITE,
> - con->ring_ref);
> + xen_pfn_t mfn = con->ring_ref;
> + con->sring = xc_map_foreign_bulk(xen_xc, con->xendev.dom,
> + PROT_READ|PROT_WRITE,
> + &mfn, &err, 1);
> } else {
> con->sring = xengnttab_map_grant_ref(xendev->gnttabdev, con->xendev.dom,
> con->ring_ref,
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 8a61e95..10cefed 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -94,6 +94,7 @@ struct XenFB {
> static int common_bind(struct common *c)
> {
> uint64_t mfn;
> + int err;
>
> if (xenstore_read_fe_uint64(&c->xendev, "page-ref", &mfn) == -1)
> return -1;
> @@ -102,9 +103,8 @@ static int common_bind(struct common *c)
> if (xenstore_read_fe_int(&c->xendev, "event-channel", &c->xendev.remote_port) == -1)
> return -1;
>
> - c->page = xc_map_foreign_range(xen_xc, c->xendev.dom,
> - XC_PAGE_SIZE,
> - PROT_READ | PROT_WRITE, mfn);
> + c->page = xc_map_foreign_bulk(xen_xc, c->xendev.dom,
> + PROT_READ | PROT_WRITE, &mfn, &err, 1);
> if (c->page == NULL)
> return -1;
>
> diff --git a/xen-hvm.c b/xen-hvm.c
> index 4470d58..0c84e30 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -1167,7 +1167,7 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
> int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> MemoryRegion **ram_memory)
> {
> - int i, rc;
> + int i, rc, map_err;
> xen_pfn_t ioreq_pfn;
> xen_pfn_t bufioreq_pfn;
> evtchn_port_t bufioreq_evtchn;
> @@ -1214,33 +1214,35 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> DPRINTF("buffered io page at pfn %lx\n", bufioreq_pfn);
> DPRINTF("buffered io evtchn is %x\n", bufioreq_evtchn);
>
> - state->shared_page = xc_map_foreign_range(xen_xc, xen_domid, XC_PAGE_SIZE,
> - PROT_READ|PROT_WRITE, ioreq_pfn);
> + state->shared_page = xc_map_foreign_bulk(xen_xc, xen_domid,
> + PROT_READ|PROT_WRITE,
> + &ioreq_pfn, &map_err, 1);
> if (state->shared_page == NULL) {
> - hw_error("map shared IO page returned error %d handle=" XC_INTERFACE_FMT,
> - errno, xen_xc);
> + hw_error(
> + "map shared IO page returned error %d(%d) handle=" XC_INTERFACE_FMT,
> + errno, map_err, xen_xc);
> }
>
> rc = xen_get_vmport_regs_pfn(xen_xc, xen_domid, &ioreq_pfn);
> if (!rc) {
> DPRINTF("shared vmport page at pfn %lx\n", ioreq_pfn);
> state->shared_vmport_page =
> - xc_map_foreign_range(xen_xc, xen_domid, XC_PAGE_SIZE,
> - PROT_READ|PROT_WRITE, ioreq_pfn);
> + xc_map_foreign_bulk(xen_xc, xen_domid, PROT_READ|PROT_WRITE,
> + &ioreq_pfn, &map_err, 1);
> if (state->shared_vmport_page == NULL) {
> - hw_error("map shared vmport IO page returned error %d handle="
> - XC_INTERFACE_FMT, errno, xen_xc);
> + hw_error("map shared vmport IO page returned error %d (%d) handle="
> + XC_INTERFACE_FMT, errno, map_err, xen_xc);
> }
> } else if (rc != -ENOSYS) {
> hw_error("get vmport regs pfn returned error %d, rc=%d", errno, rc);
> }
>
> - state->buffered_io_page = xc_map_foreign_range(xen_xc, xen_domid,
> - XC_PAGE_SIZE,
> - PROT_READ|PROT_WRITE,
> - bufioreq_pfn);
> + state->buffered_io_page = xc_map_foreign_bulk(xen_xc, xen_domid,
> + PROT_READ|PROT_WRITE,
> + &bufioreq_pfn,
> + &map_err, 1);
> if (state->buffered_io_page == NULL) {
> - hw_error("map buffered IO page returned error %d", errno);
> + hw_error("map buffered IO page returned error %d (%d)", errno, map_err);
> }
>
> /* Note: cpus is empty at this point in init */
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder Ian Campbell
2015-10-22 11:07 ` Ian Campbell
@ 2015-10-23 11:12 ` Stefano Stabellini
2015-10-23 11:19 ` Ian Campbell
1 sibling, 1 reply; 25+ messages in thread
From: Stefano Stabellini @ 2015-10-23 11:12 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, stefano.stabellini, ian.jackson, qemu-devel, xen-devel
On Wed, 21 Oct 2015, Ian Campbell wrote:
> Until the previous patch this relied on xc_fd(), which was only
> implemented for Xen 4.0 and earlier.
>
> Given this wasn't working since Xen 4.0 I have marked this as disabled
> by default.
>
> Removing this support drops the use of a bunch of symbols from
> libxenctrl, specifically:
>
> - xc_domain_create
> - xc_domain_destroy
> - xc_domain_getinfo
> - xc_domain_max_vcpus
> - xc_domain_setmaxmem
> - xc_domain_unpause
> - xc_evtchn_alloc_unbound
> - xc_linux_build
>
> This is another step towards only using Xen libraries which provide a
> stable inteface.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> v4: Fixed all checkpatch errors.
> Disabled by default.
> ---
> configure | 17 +++++++++++++++++
> hw/xenpv/Makefile.objs | 4 +++-
> hw/xenpv/xen_machine_pv.c | 14 ++++++++++----
> 3 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/configure b/configure
> index fe0a39d..9eab587 100755
> --- a/configure
> +++ b/configure
> @@ -910,6 +910,10 @@ for opt do
> ;;
> --enable-xen-pci-passthrough) xen_pci_passthrough="yes"
> ;;
> + --disable-xen-pv-domain-build) xen_pv_domain_build="no"
> + ;;
> + --enable-xen-pv-domain-build) xen_pv_domain_build="yes"
> + ;;
> --disable-brlapi) brlapi="no"
> ;;
> --enable-brlapi) brlapi="yes"
> @@ -2113,6 +2117,15 @@ if test "$xen_pci_passthrough" != "no"; then
> fi
> fi
>
> +if test "$xen_pv_domain_build" != "no"; then
> + if test "$xen_pv_domain_build" = "yes" &&
> + test "$xen" != "yes"; then
> + error_exit "User requested Xen PV domain builder support" \
> + "which requires Xen support."
> + fi
> + xen_pv_domain_build=no
> +fi
Can we simplify this to:
if test "$xen_pv_domain_build" = "yes" &&
test "$xen" != "yes"; then
error_exit "User requested Xen PV domain builder support" \
"which requires Xen support."
fi
fi
and move xen_pv_domain_build=no at the beginning of the file?
> ##########################################
> # libtool probe
>
> @@ -4393,6 +4406,7 @@ fi
> echo "xen support $xen"
> if test "$xen" = "yes" ; then
> echo "xen ctrl version $xen_ctrl_version"
> + echo "pv dom build $xen_pv_domain_build"
> fi
> echo "brlapi support $brlapi"
> echo "bluez support $bluez"
> @@ -4725,6 +4739,9 @@ fi
> if test "$xen" = "yes" ; then
> echo "CONFIG_XEN_BACKEND=y" >> $config_host_mak
> echo "CONFIG_XEN_CTRL_INTERFACE_VERSION=$xen_ctrl_version" >> $config_host_mak
> + if test "$xen_pv_domain_build" = "yes" ; then
> + echo "CONFIG_XEN_PV_DOMAIN_BUILD=y" >> $config_host_mak
> + fi
> fi
> if test "$linux_aio" = "yes" ; then
> echo "CONFIG_LINUX_AIO=y" >> $config_host_mak
> diff --git a/hw/xenpv/Makefile.objs b/hw/xenpv/Makefile.objs
> index 49f6e9e..bbf5873 100644
> --- a/hw/xenpv/Makefile.objs
> +++ b/hw/xenpv/Makefile.objs
> @@ -1,2 +1,4 @@
> # Xen PV machine support
> -obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
> +obj-$(CONFIG_XEN) += xen_machine_pv.o
> +# Xen PV machine builder support
> +obj-$(CONFIG_XEN_PV_DOMAIN_BUILD) += xen_domainbuild.o
> diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c
> index 2e545d2..e5b3698 100644
> --- a/hw/xenpv/xen_machine_pv.c
> +++ b/hw/xenpv/xen_machine_pv.c
> @@ -30,9 +30,6 @@
>
> static void xen_init_pv(MachineState *machine)
> {
> - const char *kernel_filename = machine->kernel_filename;
> - const char *kernel_cmdline = machine->kernel_cmdline;
> - const char *initrd_filename = machine->initrd_filename;
> DriveInfo *dinfo;
> int i;
>
> @@ -46,13 +43,22 @@ static void xen_init_pv(MachineState *machine)
> case XEN_ATTACH:
> /* nothing to do, xend handles everything */
> break;
> - case XEN_CREATE:
> + case XEN_CREATE: {
> +#ifdef CONFIG_XEN_PV_DOMAIN_BUILD
> + const char *kernel_filename = machine->kernel_filename;
> + const char *kernel_cmdline = machine->kernel_cmdline;
> + const char *initrd_filename = machine->initrd_filename;
> if (xen_domain_build_pv(kernel_filename, initrd_filename,
> kernel_cmdline) < 0) {
> fprintf(stderr, "xen pv domain creation failed\n");
> exit(1);
> }
> +#else
> + fprintf(stderr, "xen pv domain creation not supported\n");
> + exit(1);
> +#endif
> break;
> + }
> case XEN_EMULATE:
> fprintf(stderr, "xen emulation not implemented (yet)\n");
> exit(1);
Please add a default case with an error and place the XEN_CREATE
entirely within the ifdef CONFIG_XEN_PV_DOMAIN_BUILD.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 3/9] xen: Switch to libxengnttab interface for compat shims.
2015-10-23 11:06 ` Stefano Stabellini
@ 2015-10-23 11:15 ` Ian Campbell
2015-10-23 12:42 ` Ian Campbell
1 sibling, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2015-10-23 11:15 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: wei.liu2, ian.jackson, qemu-devel, xen-devel
On Fri, 2015-10-23 at 12:06 +0100, Stefano Stabellini wrote:
> On Wed, 21 Oct 2015, Ian Campbell wrote:
> > In Xen 4.7 we are refactoring parts libxenctrl into a number of
> > separate libraries which will provide backward and forward API and ABI
> > compatiblity.
> >
> > One such library will be libxengnttab which provides access to grant
> > tables.
> >
> > In preparation for this switch the compatibility layer in xen_common.h
> > (which support building with older versions of Xen) to use what will
> > be the new library API. This means that the gnttab shim will disappear
> > for versions of Xen which include libxengnttab.
> >
> > To simplify things for the <= 4.0.0 support we wrap the int fd in a
> > malloc(sizeof int) such that the handle is always a pointer. This
> > leads to less typedef headaches and the need for
> > XC_HANDLER_INITIAL_VALUE etc for these interfaces.
> >
> > Build tested with 4.0 and 4.5.
> >
> > Note that this patch does not add any support for actually using
> > libxengnttab, it just adjusts the existing shims.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>
> The patch looks OK but doesn't apply cleanly to master, please rebase.
> After fixing it up, it fails my 4.0 build test (I probably made a
> mistake).
It was based on:
commit 816609b2841297925a223ec377c336360e044ee5
Author: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue Jun 9 21:08:47 2015 +0200
spice-display: fix segfault in qemu_spice_create_update
Ah, I'm pointing at our xenbits qemu upstream tree (master branch, not even
upstream-tested) not the qemu.org one. I'll update to base on proper
mainline for v5 next time.
(I thought things were awfully quite on the incoming patches front when I
ran git remote update!)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder
2015-10-23 11:12 ` Stefano Stabellini
@ 2015-10-23 11:19 ` Ian Campbell
2015-10-23 11:35 ` Stefano Stabellini
0 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2015-10-23 11:19 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: wei.liu2, ian.jackson, qemu-devel, xen-devel
On Fri, 2015-10-23 at 12:12 +0100, Stefano Stabellini wrote:
> @@ -2113,6 +2117,15 @@ if test "$xen_pci_passthrough" != "no"; then
> > fi
> > fi
> >
> > +if test "$xen_pv_domain_build" != "no"; then
> > + if test "$xen_pv_domain_build" = "yes" &&
> > + test "$xen" != "yes"; then
> > + error_exit "User requested Xen PV domain builder support" \
> > + "which requires Xen support."
> > + fi
> > + xen_pv_domain_build=no
> > +fi
>
> Can we simplify this to:
>
> if test "$xen_pv_domain_build" = "yes" &&
> test "$xen" != "yes"; then
> error_exit "User requested Xen PV domain builder support" \
> "which requires Xen support."
> fi
> fi
>
> and move xen_pv_domain_build=no at the beginning of the file?
I think so, I hadn't noticed the precedent for doing so further up in the
file.
Just to check, is this still your preference after my earlier reply-to-self
explaining why the code above is utter rubbish and proposing a different
simplified version?
@@ -46,13 +43,22 @@ static void xen_init_pv(MachineState *machine)
> > case XEN_ATTACH:
> > /* nothing to do, xend handles everything */
> > break;
> > - case XEN_CREATE:
> > + case XEN_CREATE: {
> > +#ifdef CONFIG_XEN_PV_DOMAIN_BUILD
> > + const char *kernel_filename = machine->kernel_filename;
> > + const char *kernel_cmdline = machine->kernel_cmdline;
> > + const char *initrd_filename = machine->initrd_filename;
> > if (xen_domain_build_pv(kernel_filename, initrd_filename,
> > kernel_cmdline) < 0) {
> > fprintf(stderr, "xen pv domain creation failed\n");
> > exit(1);
> > }
> > +#else
> > + fprintf(stderr, "xen pv domain creation not supported\n");
> > + exit(1);
> > +#endif
> > break;
> > + }
> > case XEN_EMULATE:
> > fprintf(stderr, "xen emulation not implemented (yet)\n");
> > exit(1);
>
> Please add a default case with an error and place the XEN_CREATE
> entirely within the ifdef CONFIG_XEN_PV_DOMAIN_BUILD.
Will do.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 7/9] xen: Use stable library interfaces when they are available.
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 7/9] xen: Use stable library interfaces when they are available Ian Campbell
@ 2015-10-23 11:31 ` Stefano Stabellini
2015-10-23 12:25 ` Ian Campbell
0 siblings, 1 reply; 25+ messages in thread
From: Stefano Stabellini @ 2015-10-23 11:31 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, stefano.stabellini, ian.jackson, qemu-devel, xen-devel
On Wed, 21 Oct 2015, Ian Campbell wrote:
> In Xen 4.7 we are refactoring parts libxenctrl into a number of
> separate libraries which will provide backward and forward API and ABI
> compatiblity.
>
> Specifically libxenevtchn, libxengnttab and libxenforeignmemory.
>
> Previous patches have already laid the groundwork for using these by
> switching the existing compatibility shims to reflect the intefaces to
> these libraries.
>
> So all which remains is to update configure to detect the libraries
> and enable their use. Although they are notionally independent we take
> an all or nothing approach to the three libraries since they were
> added at the same time.
>
> The only non-obvious bit is that we now open a proper xenforeignmemory
> handle for xen_fmem instead of reusing the xen_xc handle.
>
> Build tested with 4.0, 4.5 and the patches targetting 4.7 which adds
> these libraries.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> v4: xenforeignmemory_open is now a compat wrapper, so no ifdef.
>
> Simplify configury by asserting that interface version 470 will
> always have the libraries (lack of them makes it 460).
>
> Ran checkpatch and fixed everything except:
>
> ERROR: need consistent spacing around '*' (ctx:WxV)
> +typedef xc_interface *XenXC;
>
> Which I think is a false +ve.
>
> simplify configury
> ---
> configure | 55 +++++++++++++++++++++++++++++++++++++++++++++
> include/hw/xen/xen_common.h | 38 +++++++++++++++++++++++++++++--
> 2 files changed, 91 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index 779623a..fe0a39d 100755
> --- a/configure
> +++ b/configure
> @@ -1840,6 +1840,7 @@ fi
>
> if test "$xen" != "no" ; then
> xen_libs="-lxenstore -lxenctrl -lxenguest"
> + xen_stable_libs="-lxenforeignmemory -lxengnttab -lxenevtchn"
>
> # First we test whether Xen headers and libraries are available.
> # If no, we are done and there is no Xen support.
> @@ -1862,6 +1863,57 @@ EOF
> # Xen unstable
> elif
> cat > $TMPC <<EOF &&
> +/*
> + * If we have stable libs the we don't want the libxc compat
> + * layers, regardless of what CFLAGS we may have been given.
> + */
> +#undef XC_WANT_COMPAT_EVTCHN_API
> +#undef XC_WANT_COMPAT_GNTTAB_API
> +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
> +#include <xenctrl.h>
> +#include <xenstore.h>
> +#include <xenevtchn.h>
> +#include <xengnttab.h>
> +#include <xenforeignmemory.h>
> +#include <stdint.h>
> +#include <xen/hvm/hvm_info_table.h>
> +#if !defined(HVM_MAX_VCPUS)
> +# error HVM_MAX_VCPUS not defined
> +#endif
> +int main(void) {
> + xc_interface *xc;
> + xenforeignmemory_handle *xfmem;
> + xenevtchn_handle *xe;
> + xengnttab_handle *xg;
> +
> + xs_daemon_open();
> +
> + xc = xc_interface_open(0, 0, 0);
> + xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
> + xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
> + xc_hvm_inject_msi(xc, 0, 0xf0000000, 0x00000000);
> + xc_hvm_create_ioreq_server(xc, 0, HVM_IOREQSRV_BUFIOREQ_ATOMIC, NULL);
> +
> + xfmem = xenforeignmemory_open(0, 0);
> + xenforeignmemory_map(xfmem, 0, 0, 0, 0, 0);
> +
> + xe = xenevtchn_open(0, 0);
> + xenevtchn_fd(xe);
> +
> + xg = xengnttab_open(0, 0);
> + xengnttab_map_grant_ref(xg, 0, 0, 0);
> +
> + return 0;
> +}
> +EOF
> + compile_prog "" "$xen_libs $xen_stable_libs"
> + then
> + xen_ctrl_version=470
> + xen=yes
this is good
> +
> + # Xen 4.6
> + elif
> + cat > $TMPC <<EOF &&
> #include <xenctrl.h>
> #include <xenstore.h>
> #include <stdint.h>
> @@ -2037,6 +2089,9 @@ EOF
> fi
>
> if test "$xen" = yes; then
> + if test $xen_ctrl_version -ge 470 ; then
> + libs_softmmu="$xen_stable_libs $libs_softmmu"
tab
> + fi
> libs_softmmu="$xen_libs $libs_softmmu"
> fi
> fi
> diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> index 2a5f27a..38293b4 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -6,6 +6,17 @@
> #include <stddef.h>
> #include <inttypes.h>
>
> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 470
> +/*
> + * If we have new enough libxenctrl then we do not want/need these compat
> + * interfaces, despite what the user supplied cflags might say. They
> + * must be undefined before including xenctrl.h
> + */
> +#undef XC_WANT_COMPAT_EVTCHN_API
> +#undef XC_WANT_COMPAT_GNTTAB_API
> +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
> +#endif
Can we always do the #under given that earlier libxenctrl versions will
simple ignore them? I am asking because I would prefer to avoid
introducing another ifdef here outside the sequence if ifdefs already
present below.
> #include <xenctrl.h>
> #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 420
> # include <xs.h>
> @@ -151,8 +162,8 @@ static inline void xs_close(struct xs_handle *xsh)
> }
>
>
> -/* Xen 4.1 */
> -#else
> +/* Xen 4.1 thru 4.6 */
> +#elif CONFIG_XEN_CTRL_INTERFACE_VERSION < 470
>
> typedef xc_interface *XenXC;
> typedef xc_interface *xenforeignmemory_handle;
> @@ -195,6 +206,29 @@ static inline int xc_fd(xc_interface *xen_xc)
> {
> return -1;
> }
> +#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 470 */
> +
> +typedef xc_interface *XenXC;
> +
> +# define XC_INTERFACE_FMT "%p"
> +# define XC_HANDLER_INITIAL_VALUE NULL
> +
> +#include <xenevtchn.h>
> +#include <xengnttab.h>
> +#include <xenforeignmemory.h>
> +
> +static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
> + unsigned int open_flags)
> +{
> + return xc_interface_open(logger, dombuild_logger, open_flags);
> +}
> +
> +/* FIXME There is now way to have the xen fd */
^ no way
> +static inline int xc_fd(xc_interface *xen_xc)
> +{
> + return -1;
> +}
> +
> #endif
>
> /* Xen before 4.2 */
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder
2015-10-23 11:19 ` Ian Campbell
@ 2015-10-23 11:35 ` Stefano Stabellini
2015-10-23 12:23 ` Ian Campbell
0 siblings, 1 reply; 25+ messages in thread
From: Stefano Stabellini @ 2015-10-23 11:35 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, xen-devel, ian.jackson, qemu-devel, Stefano Stabellini
On Fri, 23 Oct 2015, Ian Campbell wrote:
> On Fri, 2015-10-23 at 12:12 +0100, Stefano Stabellini wrote:
> > @@ -2113,6 +2117,15 @@ if test "$xen_pci_passthrough" != "no"; then
> > > fi
> > > fi
> > >
> > > +if test "$xen_pv_domain_build" != "no"; then
> > > + if test "$xen_pv_domain_build" = "yes" &&
> > > + test "$xen" != "yes"; then
> > > + error_exit "User requested Xen PV domain builder support" \
> > > + "which requires Xen support."
> > > + fi
> > > + xen_pv_domain_build=no
> > > +fi
> >
> > Can we simplify this to:
> >
> > if test "$xen_pv_domain_build" = "yes" &&
> > test "$xen" != "yes"; then
> > error_exit "User requested Xen PV domain builder support" \
> > "which requires Xen support."
> > fi
> > fi
> >
> > and move xen_pv_domain_build=no at the beginning of the file?
>
> I think so, I hadn't noticed the precedent for doing so further up in the
> file.
>
> Just to check, is this still your preference after my earlier reply-to-self
> explaining why the code above is utter rubbish and proposing a different
> simplified version?
The simplified check is OK, but I would still prefer if you moved
xen_pv_domain_build=no at the beginning of the file.
> @@ -46,13 +43,22 @@ static void xen_init_pv(MachineState *machine)
> > > case XEN_ATTACH:
> > > /* nothing to do, xend handles everything */
> > > break;
> > > - case XEN_CREATE:
> > > + case XEN_CREATE: {
> > > +#ifdef CONFIG_XEN_PV_DOMAIN_BUILD
> > > + const char *kernel_filename = machine->kernel_filename;
> > > + const char *kernel_cmdline = machine->kernel_cmdline;
> > > + const char *initrd_filename = machine->initrd_filename;
> > > if (xen_domain_build_pv(kernel_filename, initrd_filename,
> > > kernel_cmdline) < 0) {
> > > fprintf(stderr, "xen pv domain creation failed\n");
> > > exit(1);
> > > }
> > > +#else
> > > + fprintf(stderr, "xen pv domain creation not supported\n");
> > > + exit(1);
> > > +#endif
> > > break;
> > > + }
> > > case XEN_EMULATE:
> > > fprintf(stderr, "xen emulation not implemented (yet)\n");
> > > exit(1);
> >
> > Please add a default case with an error and place the XEN_CREATE
> > entirely within the ifdef CONFIG_XEN_PV_DOMAIN_BUILD.
>
> Will do.
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder
2015-10-23 11:35 ` Stefano Stabellini
@ 2015-10-23 12:23 ` Ian Campbell
0 siblings, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2015-10-23 12:23 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: wei.liu2, ian.jackson, qemu-devel, xen-devel
On Fri, 2015-10-23 at 12:35 +0100, Stefano Stabellini wrote:
> On Fri, 23 Oct 2015, Ian Campbell wrote:
> > On Fri, 2015-10-23 at 12:12 +0100, Stefano Stabellini wrote:
> > > @@ -2113,6 +2117,15 @@ if test "$xen_pci_passthrough" != "no"; then
> > > > fi
> > > > fi
> > > >
> > > > +if test "$xen_pv_domain_build" != "no"; then
> > > > + if test "$xen_pv_domain_build" = "yes" &&
> > > > + test "$xen" != "yes"; then
> > > > + error_exit "User requested Xen PV domain builder support" \
> > > > + "which requires Xen support."
> > > > + fi
> > > > + xen_pv_domain_build=no
> > > > +fi
> > >
> > > Can we simplify this to:
> > >
> > > if test "$xen_pv_domain_build" = "yes" &&
> > > test "$xen" != "yes"; then
> > > error_exit "User requested Xen PV domain builder support" \
> > > "which requires Xen support."
> > > fi
> > > fi
> > >
> > > and move xen_pv_domain_build=no at the beginning of the file?
> >
> > I think so, I hadn't noticed the precedent for doing so further up in
> > the
> > file.
> >
> > Just to check, is this still your preference after my earlier reply-to
> > -self
> > explaining why the code above is utter rubbish and proposing a
> > different
> > simplified version?
>
> The simplified check is OK, but I would still prefer if you moved
> xen_pv_domain_build=no at the beginning of the file.
Ack, I'll do that then.
>
>
> > @@ -46,13 +43,22 @@ static void xen_init_pv(MachineState *machine)
> > > > case XEN_ATTACH:
> > > > /* nothing to do, xend handles everything */
> > > > break;
> > > > - case XEN_CREATE:
> > > > + case XEN_CREATE: {
> > > > +#ifdef CONFIG_XEN_PV_DOMAIN_BUILD
> > > > + const char *kernel_filename = machine->kernel_filename;
> > > > + const char *kernel_cmdline = machine->kernel_cmdline;
> > > > + const char *initrd_filename = machine->initrd_filename;
> > > > if (xen_domain_build_pv(kernel_filename, initrd_filename,
> > > > kernel_cmdline) < 0) {
> > > > fprintf(stderr, "xen pv domain creation failed\n");
> > > > exit(1);
> > > > }
> > > > +#else
> > > > + fprintf(stderr, "xen pv domain creation not supported\n");
> > > > + exit(1);
> > > > +#endif
> > > > break;
> > > > + }
> > > > case XEN_EMULATE:
> > > > fprintf(stderr, "xen emulation not implemented (yet)\n");
> > > > exit(1);
> > >
> > > Please add a default case with an error and place the XEN_CREATE
> > > entirely within the ifdef CONFIG_XEN_PV_DOMAIN_BUILD.
> >
> > Will do.
> >
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 7/9] xen: Use stable library interfaces when they are available.
2015-10-23 11:31 ` Stefano Stabellini
@ 2015-10-23 12:25 ` Ian Campbell
0 siblings, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2015-10-23 12:25 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: wei.liu2, ian.jackson, qemu-devel, xen-devel
On Fri, 2015-10-23 at 12:31 +0100, Stefano Stabellini wrote:
> > diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> > index 2a5f27a..38293b4 100644
> > --- a/include/hw/xen/xen_common.h
> > +++ b/include/hw/xen/xen_common.h
> > @@ -6,6 +6,17 @@
> > #include <stddef.h>
> > #include <inttypes.h>
> >
> > +#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 470
> > +/*
> > + * If we have new enough libxenctrl then we do not want/need these compat
> > + * interfaces, despite what the user supplied cflags might say. They
> > + * must be undefined before including xenctrl.h
> > + */
> > +#undef XC_WANT_COMPAT_EVTCHN_API
> > +#undef XC_WANT_COMPAT_GNTTAB_API
> > +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
> > +#endif
>
> Can we always do the #under given that earlier libxenctrl versions will
> simple ignore them? I am asking because I would prefer to avoid
> introducing another ifdef here outside the sequence if ifdefs already
> present below.
Ah yes, we can indeed.
Ian.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH QEMU-XEN v4 3/9] xen: Switch to libxengnttab interface for compat shims.
2015-10-23 11:06 ` Stefano Stabellini
2015-10-23 11:15 ` Ian Campbell
@ 2015-10-23 12:42 ` Ian Campbell
1 sibling, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2015-10-23 12:42 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: wei.liu2, ian.jackson, qemu-devel, xen-devel
On Fri, 2015-10-23 at 12:06 +0100, Stefano Stabellini wrote:
> On Wed, 21 Oct 2015, Ian Campbell wrote:
> > In Xen 4.7 we are refactoring parts libxenctrl into a number of
> > separate libraries which will provide backward and forward API and ABI
> > compatiblity.
> >
> > One such library will be libxengnttab which provides access to grant
> > tables.
> >
> > In preparation for this switch the compatibility layer in xen_common.h
> > (which support building with older versions of Xen) to use what will
> > be the new library API. This means that the gnttab shim will disappear
> > for versions of Xen which include libxengnttab.
> >
> > To simplify things for the <= 4.0.0 support we wrap the int fd in a
> > malloc(sizeof int) such that the handle is always a pointer. This
> > leads to less typedef headaches and the need for
> > XC_HANDLER_INITIAL_VALUE etc for these interfaces.
> >
> > Build tested with 4.0 and 4.5.
> >
> > Note that this patch does not add any support for actually using
> > libxengnttab, it just adjusts the existing shims.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>
> The patch looks OK but doesn't apply cleanly to master, please rebase.
> After fixing it up, it fails my 4.0 build test (I probably made a
> mistake).
There was a new xc_gnttab_munmap which needed converting.
BTW, contrary to what every single commit message here says I'm actually
build testing on: xen with my other patches from this mega series, 4.6,
4.1, 4.0, 4.5, 4.4, 4.3, 4.2 and on my dev xen with pvbuilder forced to on.
I'm just going to drop all these "Build tested.." from the commit messages.
(the strange version ordering is so I test the different API classes first
and fail early on problems and then fill in the gaps).
Ian.
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2015-10-23 12:42 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-21 15:22 [Qemu-devel] [PATCH v4 0/<VARIOUS>] Begin to disentangle libxenctrl and provide some stable libraries Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 0/9] " Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 1/9] xen_console: correctly cleanup primary console on teardown Ian Campbell
2015-10-22 16:46 ` Stefano Stabellini
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 2/9] xen: Switch to libxenevtchn interface for compat shims Ian Campbell
2015-10-23 11:06 ` Stefano Stabellini
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 3/9] xen: Switch to libxengnttab " Ian Campbell
2015-10-23 11:06 ` Stefano Stabellini
2015-10-23 11:15 ` Ian Campbell
2015-10-23 12:42 ` Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 4/9] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_bulk Ian Campbell
2015-10-23 11:07 ` Stefano Stabellini
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 5/9] xen: Switch uses of xc_map_foreign_pages " Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 6/9] xen: Switch uses of xc_map_foreign_bulk to use libxenforeignmemory API Ian Campbell
2015-10-23 11:06 ` Stefano Stabellini
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 7/9] xen: Use stable library interfaces when they are available Ian Campbell
2015-10-23 11:31 ` Stefano Stabellini
2015-10-23 12:25 ` Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 8/9] xen: domainbuild: reopen libxenctrl interface after forking for domain watcher Ian Campbell
2015-10-21 15:23 ` [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder Ian Campbell
2015-10-22 11:07 ` Ian Campbell
2015-10-23 11:12 ` Stefano Stabellini
2015-10-23 11:19 ` Ian Campbell
2015-10-23 11:35 ` Stefano Stabellini
2015-10-23 12:23 ` Ian Campbell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).