Gerd Hoffmann
2008-Aug-22 10:25 UTC
[Xen-devel] [PATCH 0/2] RfC: xenner (aka xen emulation) bits.
Hi folks, These are the very first bits of the xen emulation, codenamed xenner. It brings an xen event channel implementation for qemu. It isn''t very useful alone. This post is just for an early review of the way I''m planing to implement this. The first patch implements the event channels and the switch between libxenctl / qemu internal using function pointers. The second patch makes the backends go through the function pointers. Comments are welcome. cheers, Gerd _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gerd Hoffmann
2008-Aug-22 10:25 UTC
[Xen-devel] [PATCH 1/2] xenner: add event channel implementation.
This adds a xen event channel implementation to qemu, intented to be used by xenner (aka xen emulation). The patch also adds a XenEvtOps struct with function pointers for the xc_evtchn_* family, which is used to switch between libxenctrl and the qemu implementation at runtime. By default libxenctrl is used. --- Makefile.target | 1 + hw/xen_interfaces.h | 27 +++ hw/xen_machine_pv.c | 2 + hw/xenner_libxc_evtchn.c | 396 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 426 insertions(+), 0 deletions(-) create mode 100644 hw/xen_interfaces.h create mode 100644 hw/xenner_libxc_evtchn.c diff --git a/Makefile.target b/Makefile.target index 5c97874..b88fd8f 100644 --- a/Makefile.target +++ b/Makefile.target @@ -521,6 +521,7 @@ endif # xen backend driver support XEN_OBJS := xen_machine_pv.o xen_backend.o xen_devconfig.o xen_domainbuild.o XEN_OBJS += xen_console.o xen_framebuffer.o xen_disk.o xen_nic.o +XEN_OBJS += xenner_libxc_evtchn.o ifeq ($(CONFIG_XEN), yes) OBJS += $(XEN_OBJS) LIBS += $(XEN_LIBS) diff --git a/hw/xen_interfaces.h b/hw/xen_interfaces.h new file mode 100644 index 0000000..869b382 --- /dev/null +++ b/hw/xen_interfaces.h @@ -0,0 +1,27 @@ +#ifndef QEMU_XEN_INTERFACES_H +#define QEMU_XEN_INTERFACES_H 1 + +#include "xen_common.h" + +/* ------------------------------------------------------------- */ +/* xen event channel interface */ + +struct XenEvtOps { + int (*open)(void); + int (*domid)(int xce_handle, int domid); + int (*close)(int xce_handle); + int (*fd)(int xce_handle); + int (*notify)(int xce_handle, evtchn_port_t port); + evtchn_port_or_error_t (*bind_unbound_port)(int xce_handle, int domid); + evtchn_port_or_error_t (*bind_interdomain)(int xce_handle, int domid, + evtchn_port_t remote_port); + evtchn_port_or_error_t (*bind_virq)(int xce_handle, unsigned int virq); + int (*unbind)(int xce_handle, evtchn_port_t port); + evtchn_port_or_error_t (*pending)(int xce_handle); + int (*unmask)(int xce_handle, evtchn_port_t port); +}; +extern struct XenEvtOps xc_evtchn; + +void xenner_evtchn_init(void); + +#endif /* QEMU_XEN_INTERFACES_H */ diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c index 9c67848..5d755f5 100644 --- a/hw/xen_machine_pv.c +++ b/hw/xen_machine_pv.c @@ -26,6 +26,7 @@ #include "boards.h" #include "xen_backend.h" +#include "xen_interfaces.h" #include "xen_domainbuild.h" /* -------------------------------------------------------------------- */ @@ -80,6 +81,7 @@ static int xen_init(void) if (!xen_detect()) { fprintf(stderr, "%s: emulating Xen\n", __FUNCTION__); xen_emulate = 1; + xenner_evtchn_init(); } if (-1 == xen_be_init()) { diff --git a/hw/xenner_libxc_evtchn.c b/hw/xenner_libxc_evtchn.c new file mode 100644 index 0000000..f0c179d --- /dev/null +++ b/hw/xenner_libxc_evtchn.c @@ -0,0 +1,396 @@ +#include <xenctrl.h> + +#include "hw.h" +#include "xen_interfaces.h" + +/* ------------------------------------------------------------- */ + +struct evtpriv; + +struct port { + struct evtpriv *priv; + struct port *peer; + int port; +}; + +struct domain { + int domid; + int refcount; + struct port p[NR_EVENT_CHANNELS]; + TAILQ_ENTRY(domain) list; +}; +static TAILQ_HEAD(domain_head, domain) domains = TAILQ_HEAD_INITIALIZER(domains); + +struct evtpriv { + int fd_read, fd_write; + struct domain *domain; + int ports; + TAILQ_ENTRY(evtpriv) list; +}; +static TAILQ_HEAD(evtpriv_head, evtpriv) privs = TAILQ_HEAD_INITIALIZER(privs); + +static int debug = 0; + +/* ------------------------------------------------------------- */ + +static struct evtpriv *getpriv(int handle) +{ + struct evtpriv *priv; + + TAILQ_FOREACH(priv, &privs, list) { + if (priv->fd_read == handle) + return priv; + } + return NULL; +} + +static struct domain *get_domain(int domid) +{ + struct domain *domain; + + TAILQ_FOREACH(domain, &domains, list) { + if (domain->domid == domid) + goto done; + } + + domain = qemu_mallocz(sizeof(*domain)); + if (NULL == domain) + return NULL; + if (domid) + domain->domid = domid; + TAILQ_INSERT_TAIL(&domains, domain, list); + if (debug) + fprintf(stderr, "xen ev: ?: new domain id %d\n", domain->domid); + +done: + domain->refcount++; + return domain; +} + +static void put_domain(struct domain *domain) +{ + domain->refcount--; + if (domain->refcount) + return; + if (debug) + fprintf(stderr, "xen ev: ?: del domain id %d\n", domain->domid); + TAILQ_REMOVE(&domains, domain, list); + qemu_free(domain); +} + +static struct port *alloc_port(struct evtpriv *priv, char *reason) +{ + struct port *p = NULL; + int i; + + for (i = 1; i < NR_EVENT_CHANNELS; i++) { + if (NULL != priv->domain->p[i].priv) + continue; + p = priv->domain->p+i; + p->port = i; + p->priv = priv; + priv->ports++; + if (debug) + fprintf(stderr, "xen ev:%3d: alloc port %d, domain %d (%s)\n", + priv->fd_read, p->port, priv->domain->domid, reason); + return p; + } + return NULL; +} + +static void bind_port_peer(struct port *p, int domid, int port) +{ + struct domain *domain; + struct port *o; + char *msg = "ok"; + + domain = get_domain(domid); + o = domain->p+port; + if (!o->priv) { + msg = "peer not allocated"; + } else if (o->peer) { + msg = "peer already bound"; + } else if (p->peer) { + msg = "port already bound"; + } else { + o->peer = p; + p->peer = o; + } + if (debug) + fprintf(stderr, "xen ev:%3d: bind port %d domain %d <-> port %d domain %d : %s\n", + p->priv->fd_read, + p->port, p->priv->domain->domid, + port, domid, msg); + put_domain(domain); +} + +static void unbind_port(struct port *p) +{ + struct port *o; + + o = p->peer; + if (o) { + if (debug) + fprintf(stderr,"xen ev:%3d: unbind port %d domain %d <-> port %d domain %d\n", + p->priv->fd_read, + p->port, p->priv->domain->domid, + o->port, o->priv->domain->domid); + o->peer = NULL; + p->peer = NULL; + } +} + +static void notify_send_peer(struct port *peer) +{ + uint32_t evtchn = peer->port; + write(peer->priv->fd_write, &evtchn, sizeof(evtchn)); +} + +static void notify_port(struct port *p) +{ + if (p->peer) { + notify_send_peer(p->peer); + if (debug > 1) + fprintf(stderr, "xen ev:%3d: notify port %d domain %d -> port %d domain %d\n", + p->priv->fd_read, p->port, p->priv->domain->domid, + p->peer->port, p->peer->priv->domain->domid); + } else { + if (debug) + fprintf(stderr,"xen ev:%3d: notify port %d domain %d -> unconnected\n", + p->priv->fd_read, p->port, p->priv->domain->domid); + } +} + +static void unmask_port(struct port *p) +{ + /* nothing to do */ +} + +static void release_port(struct port *p) +{ + if (debug) + fprintf(stderr,"xen ev:%3d: release port %d, domain %d\n", + p->priv->fd_read, p->port, p->priv->domain->domid); + unbind_port(p); + p->priv->ports--; + p->port = 0; + p->priv = 0; +} + +/* ------------------------------------------------------------- */ + +static int qemu_open(void) +{ + struct evtpriv *priv; + int fd[2]; + + priv = qemu_mallocz(sizeof(*priv)); + if (NULL == priv) + goto err; + TAILQ_INSERT_TAIL(&privs, priv, list); + + if (-1 == pipe(fd)) + goto err; + priv->fd_read = fd[0]; + priv->fd_write = fd[1]; + fcntl(priv->fd_read,F_SETFL,O_NONBLOCK); + + priv->domain = get_domain(0); + return priv->fd_read; + +err: + qemu_free(priv); + return -1; +} + +static int qemu_close(int handle) +{ + struct evtpriv *priv = getpriv(handle); + struct port *p; + int i; + + if (!priv) + return -1; + + for (i = 1; i < NR_EVENT_CHANNELS; i++) { + p = priv->domain->p+i; + if (priv != p->priv) + continue; + release_port(p); + } + put_domain(priv->domain); + + close(priv->fd_read); + close(priv->fd_write); + TAILQ_REMOVE(&privs, priv, list); + qemu_free(priv); + return 0; +} + +static int qemu_fd(int handle) +{ + struct evtpriv *priv = getpriv(handle); + + if (!priv) + return -1; + return priv->fd_read; +} + +static int qemu_notify(int handle, evtchn_port_t port) +{ + struct evtpriv *priv = getpriv(handle); + struct port *p; + + if (!priv) + return -1; + if (port >= NR_EVENT_CHANNELS) + return -1; + p = priv->domain->p + port; + notify_port(p); + return -1; +} + +static evtchn_port_or_error_t qemu_bind_unbound_port(int handle, int domid) +{ + struct evtpriv *priv = getpriv(handle); + struct port *p; + + if (!priv) + return -1; + p = alloc_port(priv, "unbound"); + if (!p) + return -1; + return p->port; +} + +static evtchn_port_or_error_t qemu_bind_interdomain(int handle, int domid, + evtchn_port_t remote_port) +{ + struct evtpriv *priv = getpriv(handle); + struct port *p; + + if (!priv) + return -1; + if (remote_port >= NR_EVENT_CHANNELS) + return -1; + p = alloc_port(priv, "interdomain"); + if (!p) + return -1; + bind_port_peer(p, domid, remote_port); + return p->port; +} + +static evtchn_port_or_error_t qemu_bind_virq(int handle, unsigned int virq) +{ + struct evtpriv *priv = getpriv(handle); + struct port *p; + + if (!priv) + return -1; + p = alloc_port(priv, "virq"); + if (!p) + return -1; + /* + * FIXME: port not linked + * (but virq doesn''t go this route anyway I think) + */ + return p->port; +} + +static int qemu_unbind(int handle, evtchn_port_t port) +{ + struct evtpriv *priv = getpriv(handle); + struct port *p; + + if (!priv) + return -1; + if (port >= NR_EVENT_CHANNELS) + return -1; + p = priv->domain->p + port; + unbind_port(p); + release_port(p); + return 0; +} + +static evtchn_port_or_error_t qemu_pending(int handle) +{ + struct evtpriv *priv = getpriv(handle); + uint32_t evtchn; + int rc; + + if (!priv) + return -1; + rc = read(priv->fd_read, &evtchn, sizeof(evtchn)); + if (rc != sizeof(evtchn)) + return -1; + return evtchn; +} + +static int qemu_unmask(int handle, evtchn_port_t port) +{ + struct evtpriv *priv = getpriv(handle); + struct port *p; + + if (!priv) + return -1; + if (port >= NR_EVENT_CHANNELS) + return -1; + p = priv->domain->p + port; + unmask_port(p); + return 0; +} + +static int qemu_domid(int handle, int domid) +{ + struct evtpriv *priv = getpriv(handle); + + if (!priv) + return -1; + if (priv->ports) + return -1; + put_domain(priv->domain); + priv->domain = get_domain(domid); + return 0; +} + +static struct XenEvtOps xenner_evtchn = { + .open = qemu_open, + .domid = qemu_domid, + .close = qemu_close, + .fd = qemu_fd, + .notify = qemu_notify, + .bind_unbound_port = qemu_bind_unbound_port, + .bind_interdomain = qemu_bind_interdomain, + .bind_virq = qemu_bind_virq, + .unbind = qemu_unbind, + .pending = qemu_pending, + .unmask = qemu_unmask, +}; + +/* ------------------------------------------------------------- */ + +static int xc_evtchn_domid(int handle, int domid) +{ + return -1; +} + +struct XenEvtOps xc_evtchn = { + .open = xc_evtchn_open, + .domid = xc_evtchn_domid, + .close = xc_evtchn_close, + .fd = xc_evtchn_fd, + .notify = xc_evtchn_notify, + .bind_unbound_port = xc_evtchn_bind_unbound_port, + .bind_interdomain = xc_evtchn_bind_interdomain, + .bind_virq = xc_evtchn_bind_virq, + .unbind = xc_evtchn_unbind, + .pending = xc_evtchn_pending, + .unmask = xc_evtchn_unmask, +}; + +/* ------------------------------------------------------------- */ + +void xenner_evtchn_init(void) +{ + xc_evtchn = xenner_evtchn; +} -- 1.5.5.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gerd Hoffmann
2008-Aug-22 10:25 UTC
[Xen-devel] [PATCH 2/2] xenner: use evtchn function pointers in the backends.
--- hw/xen_backend.c | 27 ++++++++++++++------------- 1 files changed, 14 insertions(+), 13 deletions(-) diff --git a/hw/xen_backend.c b/hw/xen_backend.c index e926a01..fd07c03 100644 --- a/hw/xen_backend.c +++ b/hw/xen_backend.c @@ -39,6 +39,7 @@ #include "hw.h" #include "qemu-char.h" #include "xen_backend.h" +#include "xen_interfaces.h" /* ------------------------------------------------------------- */ @@ -200,19 +201,19 @@ static struct XenDevice *xen_be_get_xendev(char *type, int dom, int dev, xendev->debug = debug; xendev->local_port = -1; - xendev->evtchndev = xc_evtchn_open(); + xendev->evtchndev = xc_evtchn.open(); if (xendev->evtchndev < 0) { fprintf(stderr, "can''t open evtchn device\n"); qemu_free(xendev); return NULL; } - fcntl(xc_evtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC); + fcntl(xc_evtchn.fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC); if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) { xendev->gnttabdev = xc_gnttab_open(); if (xendev->gnttabdev < 0) { fprintf(stderr, "can''t open gnttab device\n"); - xc_evtchn_close(xendev->evtchndev); + xc_evtchn.close(xendev->evtchndev); qemu_free(xendev); return NULL; } @@ -260,7 +261,7 @@ static struct XenDevice *xen_be_del_xendev(int dom, int dev) } if (xendev->evtchndev >= 0) - xc_evtchn_close(xendev->evtchndev); + xc_evtchn.close(xendev->evtchndev); if (xendev->gnttabdev >= 0) xc_gnttab_close(xendev->gnttabdev); @@ -595,13 +596,13 @@ static void xen_be_evtchn_event(void *opaque) struct XenDevice *xendev = opaque; evtchn_port_t port; - port = xc_evtchn_pending(xendev->evtchndev); + port = xc_evtchn.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, "xc_evtchn.pending returned %d (expected %d)\n", port, xendev->local_port); return; } - xc_evtchn_unmask(xendev->evtchndev, port); + xc_evtchn.unmask(xendev->evtchndev, port); if (xendev->ops->event) xendev->ops->event(xendev); @@ -644,14 +645,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 = xc_evtchn.bind_interdomain (xendev->evtchndev, xendev->dom, xendev->remote_port); if (-1 == xendev->local_port) { - xen_be_printf(xendev, 0, "xc_evtchn_bind_interdomain failed\n"); + xen_be_printf(xendev, 0, "xc_evtchn.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(xc_evtchn.fd(xendev->evtchndev), xen_be_evtchn_event, NULL, xendev); return 0; } @@ -660,15 +661,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(xc_evtchn.fd(xendev->evtchndev), NULL, NULL, NULL); + xc_evtchn.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 xc_evtchn.notify(xendev->evtchndev, xendev->local_port); } /* -- 1.5.5.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony Liguori
2008-Aug-22 13:46 UTC
[Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
Gerd Hoffmann wrote:> This adds a xen event channel implementation to qemu, intented to be > used by xenner (aka xen emulation). > > The patch also adds a XenEvtOps struct with function pointers for the > xc_evtchn_* family, which is used to switch between libxenctrl and the > qemu implementation at runtime. By default libxenctrl is used. >I suppose the QEMU implementation is to eventually eliminate the need for libxc? Do you also plan on doing a XenStore implementation within QEMU?> --- > Makefile.target | 1 + > hw/xen_interfaces.h | 27 +++ > hw/xen_machine_pv.c | 2 + > hw/xenner_libxc_evtchn.c | 396 ++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 426 insertions(+), 0 deletions(-) > create mode 100644 hw/xen_interfaces.h > create mode 100644 hw/xenner_libxc_evtchn.c > > diff --git a/Makefile.target b/Makefile.target > index 5c97874..b88fd8f 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -521,6 +521,7 @@ endif > # xen backend driver support > XEN_OBJS := xen_machine_pv.o xen_backend.o xen_devconfig.o xen_domainbuild.o > XEN_OBJS += xen_console.o xen_framebuffer.o xen_disk.o xen_nic.o > +XEN_OBJS += xenner_libxc_evtchn.o >Historically, we didn''t have directories in QEMU because we used CVS and directories are a nightmare. With the shear number of files being added for Xen, it probably makes sense to stick them all in a directory.> ifeq ($(CONFIG_XEN), yes) > OBJS += $(XEN_OBJS) > LIBS += $(XEN_LIBS) > diff --git a/hw/xen_interfaces.h b/hw/xen_interfaces.h > new file mode 100644 > index 0000000..869b382 > --- /dev/null > +++ b/hw/xen_interfaces.h > @@ -0,0 +1,27 @@ > +#ifndef QEMU_XEN_INTERFACES_H > +#define QEMU_XEN_INTERFACES_H 1 >Minor nit, make sure to have copyrights in all of your files.> +static void bind_port_peer(struct port *p, int domid, int port) > +{ > + struct domain *domain; > + struct port *o; > + char *msg = "ok"; >Should be const char *.> + domain = get_domain(domid); > + o = domain->p+port; > + if (!o->priv) { > + msg = "peer not allocated"; > + } else if (o->peer) { > + msg = "peer already bound"; > + } else if (p->peer) { > + msg = "port already bound"; > + } else { > + o->peer = p; > + p->peer = o; > + } >Watch the whitespace (you''ve got tabs all over).> + if (debug) > + fprintf(stderr, "xen ev:%3d: bind port %d domain %d <-> port %d domain %d : %s\n", > + p->priv->fd_read, > + p->port, p->priv->domain->domid, > + port, domid, msg); > + put_domain(domain); > +} > + > +static void unbind_port(struct port *p) > +{ > + struct port *o; > + > + o = p->peer; > + if (o) { > + if (debug) > + fprintf(stderr,"xen ev:%3d: unbind port %d domain %d <-> port %d domain %d\n", > + p->priv->fd_read, > + p->port, p->priv->domain->domid, > + o->port, o->priv->domain->domid); > + o->peer = NULL; > + p->peer = NULL; > + } > +} > + > +static void notify_send_peer(struct port *peer) > +{ > + uint32_t evtchn = peer->port; > + write(peer->priv->fd_write, &evtchn, sizeof(evtchn)); >Should be unix_write and should deal with errors. Regards, Anthony Liguori _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gerd Hoffmann
2008-Aug-22 15:04 UTC
[Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
Anthony Liguori wrote:> Gerd Hoffmann wrote: >> The patch also adds a XenEvtOps struct with function pointers for the >> xc_evtchn_* family, which is used to switch between libxenctrl and the >> qemu implementation at runtime. By default libxenctrl is used. > > I suppose the QEMU implementation is to eventually eliminate the need > for libxc?Yes, for xen emulation (aka xenner). When running on Xen libxc is still needed of course.> Do you also plan on doing a XenStore implementation within QEMU?Yes.>> @@ -521,6 +521,7 @@ endif >> # xen backend driver support >> XEN_OBJS := xen_machine_pv.o xen_backend.o xen_devconfig.o >> xen_domainbuild.o >> XEN_OBJS += xen_console.o xen_framebuffer.o xen_disk.o xen_nic.o >> +XEN_OBJS += xenner_libxc_evtchn.o > > Historically, we didn''t have directories in QEMU because we used CVS and > directories are a nightmare. With the shear number of files being added > for Xen, it probably makes sense to stick them all in a directory.Hmm, the xen guys vetoed file renames, so I don''t feel like sticking stuff used by xen somewhere else. For the emulation bits aka xenner*.[ch] it should be no problem though. New xenner/ toplevel directory?> Minor nit, make sure to have copyrights in all of your files. > [ ... more review comments ... ]Thanks, I''ll fix it up. cheers, Gerd _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gerd Hoffmann
2008-Aug-25 12:58 UTC
[Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
Hi,>>> XEN_OBJS += xen_console.o xen_framebuffer.o xen_disk.o xen_nic.o >>> +XEN_OBJS += xenner_libxc_evtchn.o >> Historically, we didn''t have directories in QEMU because we used CVS and >> directories are a nightmare. With the shear number of files being added >> for Xen, it probably makes sense to stick them all in a directory. > > Hmm, the xen guys vetoed file renames, so I don''t feel like sticking > stuff used by xen somewhere else. For the emulation bits aka > xenner*.[ch] it should be no problem though. New xenner/ toplevel > directory?Comments on this one? Another idea: Do the initial merge in hw/, and later on, after the qemu-xen tree catched up, move all xen (and xen emulation) bits to another directory. That would keep the scm revision history intact for the Xen guys. I also think we need to organize the source -- especially the header files -- a bit better if we start to separate the code into more subdirectories. And I''d prefer to have some agreement on how the source tree should look like before starting to shuffle around files, so I have to do it only once ... cheers, Gerd _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony Liguori
2008-Aug-25 14:15 UTC
[Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
Gerd Hoffmann wrote:> Hi, > > Comments on this one? > > Another idea: Do the initial merge in hw/, and later on, after the > qemu-xen tree catched up, move all xen (and xen emulation) bits to > another directory. That would keep the scm revision history intact for > the Xen guys. > > I also think we need to organize the source -- especially the header > files -- a bit better if we start to separate the code into more > subdirectories. And I''d prefer to have some agreement on how the source > tree should look like before starting to shuffle around files, so I have > to do it only once ... >Let''s focus on merging and then once it''s merged, we can reorganize files. I still haven''t seen any comments from the Xen guys wrt this patch series. Has there been discussion on xen-devel that I missed? Regards, Anthony Liguori> cheers, > Gerd >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2008-Aug-25 15:12 UTC
Re: [Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
On 25/8/08 15:15, "Anthony Liguori" <anthony@codemonkey.ws> wrote:>> Another idea: Do the initial merge in hw/, and later on, after the >> qemu-xen tree catched up, move all xen (and xen emulation) bits to >> another directory. That would keep the scm revision history intact for >> the Xen guys. >> >> I also think we need to organize the source -- especially the header >> files -- a bit better if we start to separate the code into more >> subdirectories. And I''d prefer to have some agreement on how the source >> tree should look like before starting to shuffle around files, so I have >> to do it only once ... >> > > Let''s focus on merging and then once it''s merged, we can reorganize > files. I still haven''t seen any comments from the Xen guys wrt this > patch series. Has there been discussion on xen-devel that I missed?With regard to this Xenner patch series, I don''t think we have a strong opinion. The changes are not particularly relevant to us. As long as the orthogonal qemu-on-xen modes that we *do* care about can coexist happily alongside, we''ll be happy. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony Liguori
2008-Aug-25 17:57 UTC
Re: [Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
Hi Keir, Keir Fraser wrote:> On 25/8/08 15:15, "Anthony Liguori" <anthony@codemonkey.ws> wrote: > > With regard to this Xenner patch series, I don''t think we have a strong > opinion. The changes are not particularly relevant to us. As long as the > orthogonal qemu-on-xen modes that we *do* care about can coexist happily > alongside, we''ll be happy. >Sorry, I was actually referring to the other qemu-dm PV domain support patch series. That''s the one that I''m looking for input from ya''ll on. Now that 3.3.0 has been released, it should be possible for you guys to consider merging the patches into qemu-dm, right? Regards, Anthony Liguori> -- Keir > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2008-Aug-25 18:42 UTC
Re: [Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
On 25/8/08 18:57, "Anthony Liguori" <anthony@codemonkey.ws> wrote:> Keir Fraser wrote: >> On 25/8/08 15:15, "Anthony Liguori" <anthony@codemonkey.ws> wrote: >> >> With regard to this Xenner patch series, I don''t think we have a strong >> opinion. The changes are not particularly relevant to us. As long as the >> orthogonal qemu-on-xen modes that we *do* care about can coexist happily >> alongside, we''ll be happy. > > Sorry, I was actually referring to the other qemu-dm PV domain support > patch series. That''s the one that I''m looking for input from ya''ll on. > Now that 3.3.0 has been released, it should be possible for you guys to > consider merging the patches into qemu-dm, right?We''ll certainly be sync''ing with upstream qemu now, yes. I''m not sure what Ian Jackson''s plans are with regard to Gerd''s patches. I think he was hoping that Gerd would post patches to xen-devel against his tree (being an existing and actively maintained and tested Xen patchset) and then we would from there submit to upstream. Afaics there''s some workflow or patchflow to be worked out here: I can''t see why we would take Gerd''s patches wholesale when we have a working patchset already. It''s a bank holiday here in the UK today, but I expect Ian will give you his opinion tomorrow! -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gerd Hoffmann
2008-Aug-26 08:27 UTC
Re: [Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
Keir Fraser wrote:> We''ll certainly be sync''ing with upstream qemu now, yes. I''m not sure what > Ian Jackson''s plans are with regard to Gerd''s patches. I think he was hoping > that Gerd would post patches to xen-devel against his tree (being an > existing and actively maintained and tested Xen patchset) and then we would > from there submit to upstream.There are two patchsets out there: #1 for upstream qemu: http://kraxel.fedorapeople.org/patches/qemu-upstream/ #2 for qemu-xen: http://kraxel.fedorapeople.org/patches/qemu-xen/ The patchsets are largely identical, thats why I don''t post both. I usually post only the upstream version and include a pointer to the qemu-xen patches in the intro text for the patchset. Differences come (a) from the fact that qemu-xen is old (these should go away once Ian synced with upstream) and (b) the slightly different ordering to make the qemu-xen patchset bisection-friendly.> Afaics there''s some workflow or patchflow to > be worked out here:Workflow could look like this: (1) Ian merges upstream into qemu-xen. (2) I''ll rebase my patches to the resulting tree. (3) Merge both patchsets, into the trees. That should result in almost identical hw/xen* files in both trees, so we don''t end up with a big mess when Ian merges again.> I can''t see why we would take Gerd''s patches wholesale > when we have a working patchset already.Which patchset you are refering to? As far I know Ian & Samuel are focusing on getting the changes to generic qemu code upstream (such as serial and ide fixes which ran over the qemu-devel list already), not on the xen support bits. cheers, Gerd _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel P. Berrange
2008-Aug-26 08:39 UTC
Re: [Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
On Mon, Aug 25, 2008 at 07:42:36PM +0100, Keir Fraser wrote:> On 25/8/08 18:57, "Anthony Liguori" <anthony@codemonkey.ws> wrote: > > > Keir Fraser wrote: > >> On 25/8/08 15:15, "Anthony Liguori" <anthony@codemonkey.ws> wrote: > >> > >> With regard to this Xenner patch series, I don''t think we have a strong > >> opinion. The changes are not particularly relevant to us. As long as the > >> orthogonal qemu-on-xen modes that we *do* care about can coexist happily > >> alongside, we''ll be happy. > > > > Sorry, I was actually referring to the other qemu-dm PV domain support > > patch series. That''s the one that I''m looking for input from ya''ll on. > > Now that 3.3.0 has been released, it should be possible for you guys to > > consider merging the patches into qemu-dm, right? > > We''ll certainly be sync''ing with upstream qemu now, yes. I''m not sure what > Ian Jackson''s plans are with regard to Gerd''s patches. I think he was hoping > that Gerd would post patches to xen-devel against his tree (being an > existing and actively maintained and tested Xen patchset) and then we would > from there submit to upstream. Afaics there''s some workflow or patchflow to > be worked out here: I can''t see why we would take Gerd''s patches wholesale > when we have a working patchset already.The core benefit of this new patchset is increased featureset. The current Xen fork of QEMU only works for running Xen VMs ontop of the Xen hypervisor, when invoked by XenD. The new patchset should support this existing use case, but in addition allow running of Xen VMs ontop of the Xen hypervisor, *without* needing XenD, and also allow running of Xen VMs ontop of KVM using Xenner. Supporting all three of those use cases with a codebase that is in mainline QEMU would be a significant improvement over the existing patchset both in terms of features & reduced support burden for people developing and distributing Xen/QEMU. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2008-Aug-26 08:40 UTC
Re: [Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
On 26/8/08 09:27, "Gerd Hoffmann" <kraxel@redhat.com> wrote:>> Afaics there''s some workflow or patchflow to >> be worked out here: > > Workflow could look like this: > > (1) Ian merges upstream into qemu-xen. > (2) I''ll rebase my patches to the resulting tree. > (3) Merge both patchsets, into the trees. > > That should result in almost identical hw/xen* files in both trees, so > we don''t end up with a big mess when Ian merges again.That sounds like a plausible avenue. I don''t want to speak for Ian though: he''ll be the one doing most of the work on the qemu-xen side.>> I can''t see why we would take Gerd''s patches wholesale >> when we have a working patchset already. > > Which patchset you are refering to? As far I know Ian & Samuel are > focusing on getting the changes to generic qemu code upstream (such as > serial and ide fixes which ran over the qemu-devel list already), not on > the xen support bits.Sorry, by patchset I meant qemu-xen. Bad confusing terminology on my part. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2008-Aug-26 11:02 UTC
[Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
Gerd Hoffmann writes ("[Qemu-devel] [PATCH 1/2] xenner: add event channel implementation."):> The patch also adds a XenEvtOps struct with function pointers for the > xc_evtchn_* family, which is used to switch between libxenctrl and the > qemu implementation at runtime. By default libxenctrl is used.I don''t understand why it''s necessary to switch between these two modes at runtime. That adds a lot of extra boilerplate and indirection code and probably has some tiny performance effect too. Why not make it a compile-time option ? Even if the xen qemu becomes nearly identical to upstream, version skew issues with the hypervisor, dom0 kernel, and so on will mean that there will have to be a separate qemu branch for xen just for release management purposes and so on. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gerd Hoffmann
2008-Aug-26 13:13 UTC
[Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
Ian Jackson wrote:> Gerd Hoffmann writes ("[Qemu-devel] [PATCH 1/2] xenner: add event channel implementation."): >> The patch also adds a XenEvtOps struct with function pointers for the >> xc_evtchn_* family, which is used to switch between libxenctrl and the >> qemu implementation at runtime. By default libxenctrl is used. > > I don''t understand why it''s necessary to switch between these two > modes at runtime. That adds a lot of extra boilerplate and > indirection code and probably has some tiny performance effect too. > > Why not make it a compile-time option ?IMHO compile time options are a usability nightmare in most cases. Note that in another mail you''ve asked for a command line switch for qemu to enable xen emulation mode even when running on xen. That wouldn''t work any more if we would make this a compile time option. You would have to go and rebuild qemu first. Also distrubutions would have to ship two binaries instead of one. And there are already way to much different qemu binaries today. I want to get that number down, not up. cheers, Gerd _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2008-Aug-26 13:28 UTC
[Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
Gerd Hoffmann writes ("Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation."):> Ian Jackson wrote: > > Why not make it a compile-time option ? > > IMHO compile time options are a usability nightmare in most cases.They''re not a problem if you need different versions in the two cases anyway. That is going to be the case for the foreseeable future. As I explained: there is code in qemu-xen which is essential for operation in the current Xen use model but which upstream don''t think suitable; and of course because of interface skew in the Xen world, it is necessary to have a compatible version of qemu. So an OS installation which is both a Xen/xend dom0 and which also has the ability to run qemu for ordinary emulations, will need two copies of qemu: one for the Xen domUs invoked by xend, and one for command-line invocations.> Also distrubutions would have to ship two binaries instead of one. And > there are already way to much different qemu binaries today. I want to > get that number down, not up.The different binaries are with us for at least the next several years. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gerd Hoffmann
2008-Aug-26 19:34 UTC
[Xen-devel] Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation.
Ian Jackson wrote:> Gerd Hoffmann writes ("Re: [Qemu-devel] [PATCH 1/2] xenner: add event channel implementation."): >> Ian Jackson wrote: >>> Why not make it a compile-time option ? >> IMHO compile time options are a usability nightmare in most cases. > > They''re not a problem if you need different versions in the two cases > anyway. That is going to be the case for the foreseeable future.I''d like to see the very same binary being used some day. I know the day is most likely a few years away, and it wouldn''t work with todays Xen versions because XenD will need changes too to make that happen (see monitor vs xenstore discussion for example). But IMHO that is no reason to give up on that right from the start. cheers, Gerd _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel