* [Qemu-devel] [PATCH 1/7] xen: groundwork for xen support
2008-08-04 15:50 [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Gerd Hoffmann
@ 2008-08-04 15:50 ` Gerd Hoffmann
2008-08-04 16:34 ` Blue Swirl
2008-08-04 17:35 ` Anthony Liguori
2008-08-04 15:50 ` [Qemu-devel] [PATCH 2/7] xen: backend driver core Gerd Hoffmann
` (6 subsequent siblings)
7 siblings, 2 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 15:50 UTC (permalink / raw)
To: qemu-devel, xen-devel; +Cc: Gerd Hoffmann
- configure script and build system changes.
- wind up new machine type.
- add -domid command line option.
- allow xenpv machines run without disk and kernel specified
by adding a nodisk_ok field to QEMUMachine.
- detect xen being present.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
Makefile.target | 8 +++
configure | 27 +++++++++
hw/boards.h | 4 +
hw/xen-machine.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++
hw/xen.h | 18 ++++++
target-i386/machine.c | 3 +
vl.c | 19 ++++++-
7 files changed, 223 insertions(+), 2 deletions(-)
create mode 100644 hw/xen-machine.c
create mode 100644 hw/xen.h
diff --git a/Makefile.target b/Makefile.target
index 42162c3..7edef6c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -515,6 +515,14 @@ CPPFLAGS += $(CONFIG_VNC_TLS_CFLAGS)
LIBS += $(CONFIG_VNC_TLS_LIBS)
endif
+# xen backend driver support
+XEN_OBJS := xen-machine.o
+ifeq ($(CONFIG_XEN), yes)
+ OBJS += $(XEN_OBJS)
+ LIBS += $(XEN_LIBS)
+ $(XEN_OBJS) : CFLAGS += -Wall -Wmissing-prototypes -Wstrict-prototypes
+endif
+
# SCSI layer
OBJS+= lsi53c895a.o esp.o
diff --git a/configure b/configure
index 0a3b7c9..5111b73 100755
--- a/configure
+++ b/configure
@@ -108,6 +108,7 @@ uname_release=""
curses="yes"
nptl="yes"
mixemu="no"
+xen="no"
# OS specific
targetos=`uname -s`
@@ -202,6 +203,7 @@ linux="yes"
linux_user="yes"
if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
kqemu="yes"
+ xen="yes"
audio_possible_drivers="$audio_possible_drivers fmod"
fi
;;
@@ -285,6 +287,8 @@ for opt do
;;
--disable-kqemu) kqemu="no"
;;
+ --disable-xen) xen="no"
+ ;;
--disable-brlapi) brlapi="no"
;;
--enable-profiler) profiler="yes"
@@ -421,6 +425,7 @@ echo " Available drivers: $audio_possible_drivers"
echo " --audio-card-list=LIST set list of additional emulated audio cards"
echo " Available cards: ac97 adlib cs4231a gus"
echo " --enable-mixemu enable mixer emulation"
+echo " --disable-xen disable xen backend driver support"
echo " --disable-brlapi disable BrlAPI"
echo " --disable-vnc-tls disable TLS encryption for VNC server"
echo " --disable-curses disable curses output"
@@ -681,6 +686,22 @@ else
fi
##########################################
+# xen probe
+
+if test "$xen" = "yes" ; then
+cat > $TMPC <<EOF
+#include <xs.h>
+#include <xenctrl.h>
+int main(void) { xs_daemon_open; xc_interface_open; }
+EOF
+ if $cc $ARCH_CFLAGS -c -o $TMPO $TMPC -lxenstore -lxenctrl 2> /dev/null ; then
+ :
+ else
+ xen="no"
+ fi
+fi
+
+##########################################
# SDL probe
sdl_too_old=no
@@ -917,6 +938,7 @@ if test -n "$sparc_cpu"; then
echo "Target Sparc Arch $sparc_cpu"
fi
echo "kqemu support $kqemu"
+echo "xen support $xen"
echo "brlapi support $brlapi"
echo "Documentation $build_docs"
[ ! -z "$uname_release" ] && \
@@ -1168,6 +1190,11 @@ if test "$brlapi" = "yes" ; then
echo "#define CONFIG_BRLAPI 1" >> $config_h
echo "BRLAPI_LIBS=-lbrlapi" >> $config_mak
fi
+if test "$xen" = "yes" ; then
+ echo "CONFIG_XEN=yes" >> $config_mak
+ echo "#define CONFIG_XEN 1" >> $config_h
+ echo "XEN_LIBS=-lxenstore -lxenctrl" >> $config_mak
+fi
# XXX: suppress that
if [ "$bsd" = "yes" ] ; then
diff --git a/hw/boards.h b/hw/boards.h
index 22ac332..9af9939 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -16,6 +16,7 @@ typedef struct QEMUMachine {
QEMUMachineInitFunc *init;
#define RAMSIZE_FIXED (1 << 0)
ram_addr_t ram_require;
+ int nodisk_ok;
struct QEMUMachine *next;
} QEMUMachine;
@@ -29,6 +30,9 @@ extern QEMUMachine bareetraxfs_machine;
extern QEMUMachine pc_machine;
extern QEMUMachine isapc_machine;
+/* xen_machine.c */
+extern QEMUMachine xenpv_machine;
+
/* ppc.c */
extern QEMUMachine prep_machine;
extern QEMUMachine core99_machine;
diff --git a/hw/xen-machine.c b/hw/xen-machine.c
new file mode 100644
index 0000000..7757bde
--- /dev/null
+++ b/hw/xen-machine.c
@@ -0,0 +1,146 @@
+/*
+ * QEMU Xen PV Machine
+ *
+ * Copyright (c) 2007,08 Red Hat
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw.h"
+#include "boards.h"
+
+#include <xenctrl.h>
+
+/* -------------------------------------------------------------------- */
+/* variables */
+
+int xen_domid;
+int xen_present = -1;
+int xen_emulate;
+int xen_domainbuild;
+
+/* -------------------------------------------------------------------- */
+/* initialization code */
+
+/*
+ * Figure the environment we are running in.
+ * Returns true when xen is present, false otherwise.
+ * Also checks whenever the domain specified via -domid
+ * exists (so we can attach) or whenever it must be created.
+ */
+int xen_detect(void)
+{
+ struct xc_dominfo info;
+ int xc, rc;
+
+ if (-1 != xen_present)
+ goto out;
+
+ /* running on xen? (priviledged domain) */
+ xen_present = 0;
+ xc = xc_interface_open();
+ if (-1 == xc)
+ goto out;
+ xen_present = 1;
+
+ /* does the domain exist? Or should we create one? */
+ rc = xc_domain_getinfo(xc, xen_domid, 1, &info);
+ if ((1 != rc) || (info.domid != xen_domid))
+ xen_domainbuild = 1;
+ close(xc);
+
+out:
+ return xen_present;
+}
+
+static int xen_init(void)
+{
+ if (!xen_domid) {
+ fprintf(stderr, "%s: no domid specified\n", __FUNCTION__);
+ return -1;
+ }
+
+ if (!xen_detect()) {
+ fprintf(stderr, "%s: emulating Xen\n", __FUNCTION__);
+ xen_emulate = 1;
+ }
+
+ return 0;
+}
+
+static int xen_init_pv(DisplayState *ds)
+{
+ int rc;
+
+ rc = xen_init();
+ if (rc < 0)
+ return rc;
+
+ return 0;
+}
+
+/* -------------------------------------------------------------------- */
+/* paravirtualized xen machine */
+
+static void xenpv_init(ram_addr_t ram_size, int vga_ram_size,
+ const char *boot_device, DisplayState *ds,
+ const char *kernel_filename,
+ const char *kernel_cmdline,
+ const char *initrd_filename,
+ const char *cpu_model)
+{
+ CPUState *env;
+ int rc;
+
+ rc = xen_init_pv(ds);
+ if (-1 == rc)
+ goto err;
+
+ if (xen_emulate) {
+ fprintf(stderr, "xen pv emulation not implemented yet\n");
+ goto err;
+ }
+ if (xen_domainbuild) {
+ fprintf(stderr, "xen pv domain creation not implemented yet\n");
+ goto err;
+ }
+
+ /* create dummy cpu, halted */
+ if (cpu_model == NULL) {
+#ifdef TARGET_X86_64
+ cpu_model = "qemu64";
+#else
+ cpu_model = "qemu32";
+#endif
+ }
+ env = cpu_init(cpu_model);
+ env->halted = 1;
+
+ return;
+
+err:
+ exit(1);
+}
+
+QEMUMachine xenpv_machine = {
+ .name = "xenpv",
+ .desc = "paravirtualized Xen machine",
+ .init = xenpv_init,
+ .nodisk_ok = 1,
+};
diff --git a/hw/xen.h b/hw/xen.h
new file mode 100644
index 0000000..a772aad
--- /dev/null
+++ b/hw/xen.h
@@ -0,0 +1,18 @@
+#ifndef QEMU_XEN_H
+#define QEMU_XEN_H 1
+/*
+ * public xen header
+ * stuff needed outside xen-*.c, i.e. interfaces to qemu.
+ * must not depend on any xen headers being present in
+ * /usr/include/xen, so it can be included unconditionally.
+ */
+
+/* xen-machine.c */
+extern int xen_domid;
+extern int xen_present;
+extern int xen_emulate;
+extern int xen_domainbuild;
+
+int xen_detect(void);
+
+#endif /* QEMU_XEN_H */
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 91dbd55..98ece17 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -9,6 +9,9 @@ void register_machines(void)
{
qemu_register_machine(&pc_machine);
qemu_register_machine(&isapc_machine);
+#ifdef CONFIG_XEN
+ qemu_register_machine(&xenpv_machine);
+#endif
}
static void cpu_put_seg(QEMUFile *f, SegmentCache *dt)
diff --git a/vl.c b/vl.c
index e929370..c3f2185 100644
--- a/vl.c
+++ b/vl.c
@@ -29,6 +29,7 @@
#include "hw/audiodev.h"
#include "hw/isa.h"
#include "hw/baum.h"
+#include "hw/xen.h"
#include "net.h"
#include "console.h"
#include "sysemu.h"
@@ -7744,6 +7745,9 @@ static void help(int exitcode)
"-startdate select initial date of the clock\n"
"-icount [N|auto]\n"
" Enable virtual instruction counter with 2^N clock ticks per instruction\n"
+#ifdef CONFIG_XEN
+ "-domid specify xen guest domain id\n"
+#endif
"\n"
"During emulation, the following keys are useful:\n"
"ctrl-alt-f toggle full screen\n"
@@ -7849,6 +7853,9 @@ enum {
QEMU_OPTION_startdate,
QEMU_OPTION_tb_size,
QEMU_OPTION_icount,
+#ifdef CONFIG_XEN
+ QEMU_OPTION_domid,
+#endif
};
typedef struct QEMUOption {
@@ -7937,6 +7944,9 @@ const QEMUOption qemu_options[] = {
#ifdef CONFIG_CURSES
{ "curses", 0, QEMU_OPTION_curses },
#endif
+#ifdef CONFIG_XEN
+ { "domid", HAS_ARG, QEMU_OPTION_domid },
+#endif
/* temporary options */
{ "usb", 0, QEMU_OPTION_usb },
@@ -8784,6 +8794,11 @@ int main(int argc, char **argv)
icount_time_shift = strtol(optarg, NULL, 0);
}
break;
+#ifdef CONFIG_XEN
+ case QEMU_OPTION_domid:
+ xen_domid = atoi(optarg);
+ break;
+#endif
}
}
}
@@ -8858,9 +8873,9 @@ int main(int argc, char **argv)
linux_boot = (kernel_filename != NULL);
net_boot = (boot_devices_bitmap >> ('n' - 'a')) & 0xF;
- /* XXX: this should not be: some embedded targets just have flash */
+ /* need a disk for this machine to boot ? */
if (!linux_boot && net_boot == 0 &&
- nb_drives_opt == 0)
+ !machine->nodisk_ok && nb_drives_opt == 0)
help(1);
if (!linux_boot && *kernel_cmdline != '\0') {
--
1.5.5.1
^ permalink raw reply related [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 1/7] xen: groundwork for xen support
2008-08-04 15:50 ` [Qemu-devel] [PATCH 1/7] xen: groundwork for xen support Gerd Hoffmann
@ 2008-08-04 16:34 ` Blue Swirl
2008-08-04 18:01 ` Gerd Hoffmann
2008-08-04 17:35 ` Anthony Liguori
1 sibling, 1 reply; 92+ messages in thread
From: Blue Swirl @ 2008-08-04 16:34 UTC (permalink / raw)
To: qemu-devel; +Cc: xen-devel, Gerd Hoffmann
On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> - allow xenpv machines run without disk and kernel specified
> by adding a nodisk_ok field to QEMUMachine.
Nice idea, this would be a useful feature by itself.
> + $(XEN_OBJS) : CFLAGS += -Wall -Wmissing-prototypes -Wstrict-prototypes
These aren't needed for Xen. Though I wonder why at least -Wall
-Wstrict-prototypes could not be added globally.
> + if (-1 != xen_present)
This style looks alien to me. Do you really find it readable?
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 1/7] xen: groundwork for xen support
2008-08-04 16:34 ` Blue Swirl
@ 2008-08-04 18:01 ` Gerd Hoffmann
0 siblings, 0 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 18:01 UTC (permalink / raw)
To: Blue Swirl; +Cc: xen-devel, qemu-devel
Blue Swirl wrote:
> On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> - allow xenpv machines run without disk and kernel specified
>> by adding a nodisk_ok field to QEMUMachine.
>
> Nice idea, this would be a useful feature by itself.
Yes, some embedded devices probably can use that too according to the
source code comment. Guess thats why anthony suggested to do it that
way, and I liked the idea too, so I just did ;)
>> + $(XEN_OBJS) : CFLAGS += -Wall -Wmissing-prototypes -Wstrict-prototypes
>
> These aren't needed for Xen. Though I wonder why at least -Wall
> -Wstrict-prototypes could not be added globally.
IIRC I tried to turn them on globally and found myself swamped with
warnings. So I took the easy way out and enabled them only for the xen
bits I'm working on. I like those warnings, they catch sloppyness and
help writing clean code. They are in no way required though. We can
add them globally nevertheless, I can keep them as local patch, I don't
mind much.
>> + if (-1 != xen_present)
>
> This style looks alien to me. Do you really find it readable?
Bad habit. gcc warns these days on "if (foo = 42)", so the reason to
write "if (42 = foo)" is gone. I'm still doing that for -- say --
historical reasons?
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 1/7] xen: groundwork for xen support
2008-08-04 15:50 ` [Qemu-devel] [PATCH 1/7] xen: groundwork for xen support Gerd Hoffmann
2008-08-04 16:34 ` Blue Swirl
@ 2008-08-04 17:35 ` Anthony Liguori
2008-08-04 18:04 ` Gerd Hoffmann
1 sibling, 1 reply; 92+ messages in thread
From: Anthony Liguori @ 2008-08-04 17:35 UTC (permalink / raw)
To: qemu-devel; +Cc: xen-devel, Gerd Hoffmann
Gerd Hoffmann wrote:
> # OS specific
> targetos=`uname -s`
> @@ -202,6 +203,7 @@ linux="yes"
> linux_user="yes"
> if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
> kqemu="yes"
> + xen="yes"
> audio_possible_drivers="$audio_possible_drivers fmod"
> fi
>
Is this really necessary? The compile check will take care of the non
i386/x86_64 cases. It works a lot better when using a cross-compiler too.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 1/7] xen: groundwork for xen support
2008-08-04 17:35 ` Anthony Liguori
@ 2008-08-04 18:04 ` Gerd Hoffmann
0 siblings, 0 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 18:04 UTC (permalink / raw)
To: Anthony Liguori; +Cc: xen-devel, qemu-devel
Anthony Liguori wrote:
> Gerd Hoffmann wrote:
>> # OS specific
>> targetos=`uname -s`
>> @@ -202,6 +203,7 @@ linux="yes"
>> linux_user="yes"
>> if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
>> kqemu="yes"
>> + xen="yes"
>> audio_possible_drivers="$audio_possible_drivers fmod"
>> fi
>>
>
> Is this really necessary? The compile check will take care of the non
> i386/x86_64 cases. It works a lot better when using a cross-compiler too.
This is just me being conservative and enable the cases I know it works
for. There is ia64 xen ...
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* [Qemu-devel] [PATCH 2/7] xen: backend driver core
2008-08-04 15:50 [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Gerd Hoffmann
2008-08-04 15:50 ` [Qemu-devel] [PATCH 1/7] xen: groundwork for xen support Gerd Hoffmann
@ 2008-08-04 15:50 ` Gerd Hoffmann
2008-08-04 15:50 ` [Qemu-devel] [PATCH 3/7] xen: add console backend driver Gerd Hoffmann
` (5 subsequent siblings)
7 siblings, 0 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 15:50 UTC (permalink / raw)
To: qemu-devel, xen-devel; +Cc: Gerd Hoffmann
This patch adds infrastructure for xen backend drivers living in qemu,
so drivers don't need to implement common stuff on their own. It's
mostly xenbus management stuff: some functions to access xentore,
setting up xenstore watches, callbacks on device discovery and state
changes, handle event channel, ...
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
Makefile.target | 2 +-
hw/xen-backend.c | 665 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/xen-backend.h | 87 +++++++
hw/xen-common.h | 38 +++
hw/xen-machine.c | 7 +-
5 files changed, 797 insertions(+), 2 deletions(-)
create mode 100644 hw/xen-backend.c
create mode 100644 hw/xen-backend.h
create mode 100644 hw/xen-common.h
diff --git a/Makefile.target b/Makefile.target
index 7edef6c..65f4910 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -516,7 +516,7 @@ LIBS += $(CONFIG_VNC_TLS_LIBS)
endif
# xen backend driver support
-XEN_OBJS := xen-machine.o
+XEN_OBJS := xen-machine.o xen-backend.o
ifeq ($(CONFIG_XEN), yes)
OBJS += $(XEN_OBJS)
LIBS += $(XEN_LIBS)
diff --git a/hw/xen-backend.c b/hw/xen-backend.c
new file mode 100644
index 0000000..8f5244a
--- /dev/null
+++ b/hw/xen-backend.c
@@ -0,0 +1,665 @@
+/*
+ * xen backend driver infrastructure
+ * (c) 2008 Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+/*
+ * TODO: add some xenbus / xenstore concepts overview here.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <sys/signal.h>
+
+#include <xs.h>
+#include <xenctrl.h>
+#include <xen/grant_table.h>
+
+#include "hw.h"
+#include "qemu-char.h"
+#include "xen-backend.h"
+
+/* ------------------------------------------------------------- */
+
+/* public */
+int xen_xc;
+struct xs_handle *xenstore = NULL;
+
+/* private */
+static TAILQ_HEAD(XenDeviceHead, XenDevice) xendevs = TAILQ_HEAD_INITIALIZER(xendevs);
+static const char *root = "/local/domain/0/backend";
+static int debug = 0;
+
+/* ------------------------------------------------------------- */
+
+int xenstore_write_str(const char *base, const char *node, const char *val)
+{
+ char abspath[BUFSIZE];
+
+ snprintf(abspath, sizeof(abspath), "%s/%s", base, node);
+ if (!xs_write(xenstore, 0, abspath, val, strlen(val)))
+ return -1;
+ return 0;
+}
+
+char *xenstore_read_str(const char *base, const char *node)
+{
+ char abspath[BUFSIZE];
+ unsigned int len;
+
+ snprintf(abspath, sizeof(abspath), "%s/%s", base, node);
+ return xs_read(xenstore, 0, abspath, &len);
+}
+
+int xenstore_write_int(const char *base, const char *node, int ival)
+{
+ char val[32];
+
+ snprintf(val, sizeof(val), "%d", ival);
+ return xenstore_write_str(base, node, val);
+}
+
+int xenstore_read_int(const char *base, const char *node, int *ival)
+{
+ char *val;
+ int rc = -1;
+
+ val = xenstore_read_str(base, node);
+ if (val && 1 == sscanf(val, "%d", ival))
+ rc = 0;
+ qemu_free(val);
+ return rc;
+}
+
+int xenstore_write_be_str(struct XenDevice *xendev, const char *node, const char *val)
+{
+ return xenstore_write_str(xendev->be, node, val);
+}
+
+int xenstore_write_be_int(struct XenDevice *xendev, const char *node, int ival)
+{
+ return xenstore_write_int(xendev->be, node, ival);
+}
+
+char *xenstore_read_be_str(struct XenDevice *xendev, const char *node)
+{
+ return xenstore_read_str(xendev->be, node);
+}
+
+int xenstore_read_be_int(struct XenDevice *xendev, const char *node, int *ival)
+{
+ return xenstore_read_int(xendev->be, node, ival);
+}
+
+char *xenstore_read_fe_str(struct XenDevice *xendev, const char *node)
+{
+ return xenstore_read_str(xendev->fe, node);
+}
+
+int xenstore_read_fe_int(struct XenDevice *xendev, const char *node, int *ival)
+{
+ return xenstore_read_int(xendev->fe, node, ival);
+}
+
+/* ------------------------------------------------------------- */
+
+const char *xenbus_strstate(enum xenbus_state state)
+{
+ static const char *const name[] = {
+ [ XenbusStateUnknown ] = "Unknown",
+ [ XenbusStateInitialising ] = "Initialising",
+ [ XenbusStateInitWait ] = "InitWait",
+ [ XenbusStateInitialised ] = "Initialised",
+ [ XenbusStateConnected ] = "Connected",
+ [ XenbusStateClosing ] = "Closing",
+ [ XenbusStateClosed ] = "Closed",
+ };
+ return (state < ARRAY_SIZE(name)) ? name[state] : "INVALID";
+}
+
+int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state)
+{
+ int rc;
+
+ rc = xenstore_write_be_int(xendev, "state", state);
+ if (rc < 0)
+ return rc;
+ xen_be_printf(xendev, 1, "backend state: %s -> %s\n",
+ xenbus_strstate(xendev->be_state), xenbus_strstate(state));
+ xendev->be_state = state;
+ return 0;
+}
+
+/* ------------------------------------------------------------- */
+
+struct XenDevice *xen_be_find_xendev(char *type, int dom, int dev)
+{
+ struct XenDevice *xendev;
+
+ TAILQ_FOREACH(xendev, &xendevs, next) {
+ if (xendev->dom != dom)
+ continue;
+ if (xendev->dev != dev)
+ continue;
+ if (0 != strcmp(xendev->type, type))
+ continue;
+ return xendev;
+ }
+ return NULL;
+}
+
+/*
+ * get xen backend device, allocate a new one if it doesn't exist.
+ */
+static struct XenDevice *xen_be_get_xendev(char *type, int dom, int dev,
+ struct XenDevOps *ops)
+{
+ struct XenDevice *xendev;
+
+ xendev = xen_be_find_xendev(type, dom, dev);
+ if (xendev)
+ return xendev;
+
+ /* init new xendev */
+ xendev = qemu_mallocz(ops->size);
+ if (!xendev)
+ return NULL;
+ xendev->type = type;
+ xendev->dom = dom;
+ xendev->dev = dev;
+ xendev->ops = ops;
+ snprintf(xendev->be, sizeof(xendev->be), "%s/%s/%d/%d",
+ root, xendev->type, xendev->dom, xendev->dev);
+ snprintf(xendev->name, sizeof(xendev->name), "%s-%d",
+ xendev->type, xendev->dev);
+
+ xendev->debug = debug;
+ xendev->local_port = -1;
+
+ 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);
+
+ 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);
+ qemu_free(xendev);
+ return NULL;
+ }
+ } else {
+ xendev->gnttabdev = -1;
+ }
+
+ TAILQ_INSERT_TAIL(&xendevs, xendev, next);
+
+ if (xendev->ops->alloc)
+ xendev->ops->alloc(xendev);
+
+ return xendev;
+}
+
+/*
+ * release xen backend device.
+ */
+static struct XenDevice *xen_be_del_xendev(int dom, int dev)
+{
+ struct XenDevice *xendev, *xnext;
+
+ /*
+ * This is pretty much like TAILQ_FOREACH(xendev, &xendevs, next) but
+ * we save the next pointer in xnext because we might free xendev.
+ */
+ xnext = xendevs.tqh_first;
+ while (xnext) {
+ xendev = xnext;
+ xnext = xendev->next.tqe_next;
+
+ if (xendev->dom != dom)
+ continue;
+ if (xendev->dev != dev && dev != -1)
+ continue;
+
+ if (xendev->ops->free)
+ xendev->ops->free(xendev);
+
+ if (xendev->fe) {
+ char token[BUFSIZE];
+ snprintf(token, sizeof(token), "fe:%p", xendev);
+ xs_unwatch(xenstore, xendev->fe, token);
+ qemu_free(xendev->fe);
+ }
+
+ if (xendev->evtchndev >= 0)
+ xc_evtchn_close(xendev->evtchndev);
+ if (xendev->gnttabdev >= 0)
+ xc_gnttab_close(xendev->gnttabdev);
+
+ TAILQ_REMOVE(&xendevs, xendev, next);
+ qemu_free(xendev);
+ }
+ return NULL;
+}
+
+/*
+ * Sync internal data structures on xenstore updates.
+ * Node specifies the changed field. node = NULL means
+ * update all fields (used for initialization).
+ */
+static void xen_be_backend_changed(struct XenDevice *xendev, const char *node)
+{
+ if (NULL == node || 0 == strcmp(node, "online")) {
+ if (-1 == xenstore_read_be_int(xendev, "online", &xendev->online))
+ xendev->online = 0;
+ }
+
+ if (node) {
+ xen_be_printf(xendev, 2, "backend update: %s\n", node);
+ if (xendev->ops->backend_changed)
+ xendev->ops->backend_changed(xendev, node);
+ }
+}
+
+static void xen_be_frontend_changed(struct XenDevice *xendev, const char *node)
+{
+ int fe_state;
+
+ if (NULL == node || 0 == strcmp(node, "state")) {
+ if (-1 == xenstore_read_fe_int(xendev, "state", &fe_state))
+ fe_state = XenbusStateUnknown;
+ if (xendev->fe_state != fe_state)
+ xen_be_printf(xendev, 1, "frontend state: %s -> %s\n",
+ xenbus_strstate(xendev->fe_state),
+ xenbus_strstate(fe_state));
+ xendev->fe_state = fe_state;
+ }
+ if (NULL == node || 0 == strcmp(node, "protocol")) {
+ qemu_free(xendev->protocol);
+ xendev->protocol = xenstore_read_fe_str(xendev, "protocol");
+ if (xendev->protocol)
+ xen_be_printf(xendev, 1, "frontend protocol: %s\n", xendev->protocol);
+ }
+
+ if (node) {
+ xen_be_printf(xendev, 2, "frontend update: %s\n", node);
+ if (xendev->ops->frontend_changed)
+ xendev->ops->frontend_changed(xendev, node);
+ }
+}
+
+/* ------------------------------------------------------------- */
+/* Check for possible state transitions and perform them. */
+
+/*
+ * Initial xendev setup. Read frontend path, register watch for it.
+ * Should succeed once xend finished setting up the backend device.
+ *
+ * Also sets initial state (-> Initializing) when done. Which
+ * only affects the xendev->be_state variable as xenbus should
+ * already be put into that state by xend.
+ */
+static int xen_be_try_setup(struct XenDevice *xendev)
+{
+ char token[BUFSIZE];
+ int be_state;
+
+ if (-1 == xenstore_read_be_int(xendev, "state", &be_state)) {
+ xen_be_printf(xendev, 0, "reading backend state failed\n");
+ return -1;
+ }
+
+ if (be_state != XenbusStateInitialising) {
+ xen_be_printf(xendev, 0, "initial backend state is wrong (%s)\n",
+ xenbus_strstate(be_state));
+ return -1;
+ }
+
+ xendev->fe = xenstore_read_be_str(xendev, "frontend");
+ if (NULL == xendev->fe) {
+ xen_be_printf(xendev, 0, "reading frontend path failed\n");
+ return -1;
+ }
+
+ /* setup frontend watch */
+ snprintf(token, sizeof(token), "fe:%p", xendev);
+ if (!xs_watch(xenstore, xendev->fe, token)) {
+ xen_be_printf(xendev, 0, "watching frontend path (%s) failed\n",
+ xendev->fe);
+ return -1;
+ }
+ xen_be_set_state(xendev, XenbusStateInitialising);
+
+ xen_be_backend_changed(xendev, NULL);
+ xen_be_frontend_changed(xendev, NULL);
+ return 0;
+}
+
+/*
+ * Try initialize xendev. Prepare everything the backend can do
+ * without synchronizing with the frontend. Fakes hotplug-status. No
+ * hotplug involved here because this is about userspace drivers, thus
+ * there are kernel backend devices which could invoke hotplug.
+ *
+ * Goes to InitWait on success.
+ */
+static int xen_be_try_init(struct XenDevice *xendev)
+{
+ int rc = 0;
+
+ if (!xendev->online) {
+ xen_be_printf(xendev, 1, "not online\n");
+ return -1;
+ }
+
+ if (xendev->ops->init)
+ rc = xendev->ops->init(xendev);
+ if (0 != rc) {
+ xen_be_printf(xendev, 1, "init() failed\n");
+ return rc;
+ }
+
+ xenstore_write_be_str(xendev, "hotplug-status", "connected");
+ xen_be_set_state(xendev, XenbusStateInitWait);
+ return 0;
+}
+
+/*
+ * Try to connect xendev. Depends on the frontend being ready
+ * for it (shared ring and evtchn info in xenstore, state being
+ * Initialised or Connected).
+ *
+ * Goes to Connected on success.
+ */
+static int xen_be_try_connect(struct XenDevice *xendev)
+{
+ int rc = 0;
+
+ if (xendev->fe_state != XenbusStateInitialised &&
+ xendev->fe_state != XenbusStateConnected) {
+ if (xendev->ops->flags & DEVOPS_FLAG_IGNORE_STATE) {
+ xen_be_printf(xendev, 2, "frontend not ready, ignoring\n");
+ } else {
+ xen_be_printf(xendev, 2, "frontend not ready (yet)\n");
+ return -1;
+ }
+ }
+
+ if (xendev->ops->connect)
+ rc = xendev->ops->connect(xendev);
+ if (0 != rc) {
+ xen_be_printf(xendev, 0, "connect() failed\n");
+ return rc;
+ }
+
+ xen_be_set_state(xendev, XenbusStateConnected);
+ return 0;
+}
+
+/*
+ * Teardown connection.
+ *
+ * Goes to Closed when done.
+ */
+static void xen_be_disconnect(struct XenDevice *xendev)
+{
+ if (xendev->be_state == XenbusStateClosed)
+ return;
+ if (xendev->ops->disconnect)
+ xendev->ops->disconnect(xendev);
+ xen_be_set_state(xendev, XenbusStateClosed);
+}
+
+/*
+ * state change dispatcher function
+ */
+void xen_be_check_state(struct XenDevice *xendev)
+{
+ int rc = 0;
+
+ /* frontend may request shutdown from almost anywhere */
+ if (xendev->fe_state == XenbusStateClosing ||
+ xendev->fe_state == XenbusStateClosed) {
+ xen_be_disconnect(xendev);
+ return;
+ }
+
+ /* check for possible backend state transitions */
+ for (;;) {
+ switch (xendev->be_state) {
+ case XenbusStateUnknown:
+ rc = xen_be_try_setup(xendev);
+ break;
+ case XenbusStateInitialising:
+ rc = xen_be_try_init(xendev);
+ break;
+ case XenbusStateInitWait:
+ rc = xen_be_try_connect(xendev);
+ break;
+ default:
+ rc = -1;
+ }
+ if (0 != rc)
+ break;
+ }
+}
+
+/* ------------------------------------------------------------- */
+
+static int xenstore_scan(char *type, int dom, struct XenDevOps *ops)
+{
+ struct XenDevice *xendev;
+ char path[BUFSIZE], token[BUFSIZE];
+ char **dev = NULL;
+ unsigned int cdev, j;
+
+ /* setup watch */
+ snprintf(token, sizeof(token), "be:%p:%d:%p", type, dom, ops);
+ snprintf(path, sizeof(path), "%s/%s/%d", root, type, dom);
+ if (!xs_watch(xenstore, path, token)) {
+ fprintf(stderr, "xen be: watching backend path (%s) failed\n", path);
+ return -1;
+ }
+
+ /* look for backends */
+ dev = xs_directory(xenstore, 0, path, &cdev);
+ if (!dev)
+ return 0;
+ for (j = 0; j < cdev; j++) {
+ xendev = xen_be_get_xendev(type, dom, atoi(dev[j]), ops);
+ if (NULL == xendev)
+ continue;
+ xen_be_check_state(xendev);
+ }
+ qemu_free(dev);
+ return 0;
+}
+
+static void xenstore_update_be(char *watch, char *type, int dom,
+ struct XenDevOps *ops)
+{
+ struct XenDevice *xendev;
+ char path[BUFSIZE];
+ unsigned int len, dev;
+
+ len = snprintf(path, sizeof(path), "%s/%s/%d", root, type, dom);
+ if (0 != strncmp(path, watch, len))
+ return;
+ if (2 != sscanf(watch+len, "/%u/%255s", &dev, path)) {
+ strcpy(path, "");
+ if (1 != sscanf(watch+len, "/%u", &dev))
+ dev = -1;
+ }
+ if (-1 == dev)
+ return;
+
+ if (0) {
+ /* FIXME: detect devices being deleted from xenstore ... */
+ xen_be_del_xendev(dom, dev);
+ }
+
+ xendev = xen_be_get_xendev(type, dom, dev, ops);
+ if (NULL != xendev) {
+ xen_be_backend_changed(xendev, path);
+ xen_be_check_state(xendev);
+ }
+}
+
+static void xenstore_update_fe(char *watch, struct XenDevice *xendev)
+{
+ char *node;
+ unsigned int len;
+
+ len = strlen(xendev->fe);
+ if (0 != strncmp(xendev->fe, watch, len))
+ return;
+ if (watch[len] != '/')
+ return;
+ node = watch + len + 1;
+
+ xen_be_frontend_changed(xendev, node);
+ xen_be_check_state(xendev);
+}
+
+static void xenstore_update(void *unused)
+{
+ char **vec = NULL;
+ intptr_t type, ops, ptr;
+ unsigned int dom, count;
+
+ vec = xs_read_watch(xenstore, &count);
+ if (NULL == vec)
+ goto cleanup;
+
+ if (3 == sscanf(vec[XS_WATCH_TOKEN], "be:%" PRIxPTR ":%d:%" PRIxPTR,
+ &type, &dom, &ops))
+ xenstore_update_be(vec[XS_WATCH_PATH], (void*)type, dom, (void*)ops);
+ if (1 == sscanf(vec[XS_WATCH_TOKEN], "fe:%" PRIxPTR, &ptr))
+ xenstore_update_fe(vec[XS_WATCH_PATH], (void*)ptr);
+
+cleanup:
+ qemu_free(vec);
+}
+
+static void xen_be_evtchn_event(void *opaque)
+{
+ struct XenDevice *xendev = opaque;
+ evtchn_port_t port;
+
+ port = xc_evtchn_pending(xendev->evtchndev);
+ if (port != xendev->local_port) {
+ xen_be_printf(xendev, 0, "xc_evtchn_pending returned %d (expected %d)\n",
+ port, xendev->local_port);
+ return;
+ }
+ xc_evtchn_unmask(xendev->evtchndev, port);
+
+ if (xendev->ops->event)
+ xendev->ops->event(xendev);
+}
+
+/* -------------------------------------------------------------------- */
+
+int xen_be_init(void)
+{
+ xenstore = xs_daemon_open();
+ if (!xenstore) {
+ fprintf(stderr, "can't connect to xenstored\n");
+ return -1;
+ }
+
+ if (qemu_set_fd_handler(xs_fileno(xenstore), xenstore_update, NULL, NULL) < 0)
+ goto err;
+
+ xen_xc = xc_interface_open();
+ if (-1 == xen_xc) {
+ fprintf(stderr, "can't open xen interface\n");
+ goto err;
+ }
+ return 0;
+
+err:
+ qemu_set_fd_handler(xs_fileno(xenstore), NULL, NULL, NULL);
+ xs_daemon_close(xenstore);
+ xenstore = NULL;
+
+ return -1;
+}
+
+int xen_be_register(char *type, struct XenDevOps *ops)
+{
+ return xenstore_scan(type, xen_domid, ops);
+}
+
+int xen_be_bind_evtchn(struct XenDevice *xendev)
+{
+ if (xendev->local_port != -1)
+ return 0;
+ xendev->local_port = xc_evtchn_bind_interdomain
+ (xendev->evtchndev, xendev->dom, xendev->remote_port);
+ if (-1 == xendev->local_port) {
+ xendev->local_port = 0;
+ return -1;
+ }
+ qemu_set_fd_handler(xc_evtchn_fd(xendev->evtchndev),
+ xen_be_evtchn_event, NULL, xendev);
+ return 0;
+}
+
+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);
+ xendev->local_port = -1;
+}
+
+int xen_be_send_notify(struct XenDevice *xendev)
+{
+ return xc_evtchn_notify(xendev->evtchndev, xendev->local_port);
+}
+
+/*
+ * msg_level:
+ * 0 == errors.
+ * 1 == informative debug messages.
+ * 2 == noisy debug messages.
+ * 3 == will flood your log.
+ */
+void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, ...)
+{
+ va_list args;
+
+ if (msg_level > xendev->debug)
+ return;
+ fprintf(stderr, "xen be: %s: ", xendev->name);
+ va_start(args, fmt);
+ vfprintf(stderr, fmt, args);
+ va_end(args);
+}
diff --git a/hw/xen-backend.h b/hw/xen-backend.h
new file mode 100644
index 0000000..13b3190
--- /dev/null
+++ b/hw/xen-backend.h
@@ -0,0 +1,87 @@
+#ifndef QEMU_XEN_BACKEND_H
+#define QEMU_XEN_BACKEND_H 1
+
+#include "xen-common.h"
+
+/* ------------------------------------------------------------- */
+
+#define BUFSIZE 1024
+
+struct XenDevice;
+
+/* driver uses grant tables -> open gntdev device (xendev->gnttabdev) */
+#define DEVOPS_FLAG_NEED_GNTDEV 1
+/* don't expect frontend doing correct state transitions (aka console quirk) */
+#define DEVOPS_FLAG_IGNORE_STATE 2
+
+struct XenDevOps {
+ size_t size;
+ uint32_t flags;
+ void (*alloc)(struct XenDevice *xendev);
+ int (*init)(struct XenDevice *xendev);
+ int (*connect)(struct XenDevice *xendev);
+ void (*event)(struct XenDevice *xendev);
+ void (*disconnect)(struct XenDevice *xendev);
+ int (*free)(struct XenDevice *xendev);
+ void (*backend_changed)(struct XenDevice *xendev, const char *node);
+ void (*frontend_changed)(struct XenDevice *xendev, const char *node);
+};
+
+struct XenDevice {
+ char *type;
+ int dom;
+ int dev;
+ char name[64];
+ int debug;
+
+ enum xenbus_state be_state;
+ enum xenbus_state fe_state;
+ int online;
+ char be[BUFSIZE];
+ char *fe;
+ char *protocol;
+ int remote_port;
+ int local_port;
+
+ int evtchndev;
+ int gnttabdev;
+
+ struct XenDevOps *ops;
+ TAILQ_ENTRY(XenDevice) next;
+};
+
+/* ------------------------------------------------------------- */
+
+/* variables */
+extern int xen_domid; /* set by cmd line option, in vl.c */
+extern int xen_xc;
+extern struct xs_handle *xenstore;
+
+/* xenstore helper functions */
+int xenstore_write_str(const char *base, const char *node, const char *val);
+int xenstore_write_int(const char *base, const char *node, int ival);
+char *xenstore_read_str(const char *base, const char *node);
+int xenstore_read_int(const char *base, const char *node, int *ival);
+
+int xenstore_write_be_str(struct XenDevice *xendev, const char *node, const char *val);
+int xenstore_write_be_int(struct XenDevice *xendev, const char *node, int ival);
+char *xenstore_read_be_str(struct XenDevice *xendev, const char *node);
+int xenstore_read_be_int(struct XenDevice *xendev, const char *node, int *ival);
+char *xenstore_read_fe_str(struct XenDevice *xendev, const char *node);
+int xenstore_read_fe_int(struct XenDevice *xendev, const char *node, int *ival);
+
+const char *xenbus_strstate(enum xenbus_state state);
+struct XenDevice *xen_be_find_xendev(char *type, int dom, int dev);
+void xen_be_check_state(struct XenDevice *xendev);
+
+/* xen backend driver bits */
+int xen_be_init(void);
+int xen_be_register(char *type, struct XenDevOps *ops);
+int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state);
+int xen_be_bind_evtchn(struct XenDevice *xendev);
+void xen_be_unbind_evtchn(struct XenDevice *xendev);
+int xen_be_send_notify(struct XenDevice *xendev);
+void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, ...)
+ __attribute__ ((format(printf, 3, 4)));
+
+#endif /* QEMU_XEN_BACKEND_H */
diff --git a/hw/xen-common.h b/hw/xen-common.h
new file mode 100644
index 0000000..866049a
--- /dev/null
+++ b/hw/xen-common.h
@@ -0,0 +1,38 @@
+#ifndef QEMU_XEN_COMMON_H
+#define QEMU_XEN_COMMON_H 1
+
+#include <stddef.h>
+#include <inttypes.h>
+
+#include <xenctrl.h>
+#include <xs.h>
+#include <xen/io/xenbus.h>
+
+#include "hw.h"
+#include "xen.h"
+#include "sys-queue.h" /* BSD list implementation */
+
+/*
+ * tweaks needed to build with different xen versions
+ * 0x00030205 -> 3.1.0
+ * 0x00030207 -> 3.2.0
+ * 0x00030208 -> unstable
+ */
+#include <xen/xen-compat.h>
+#if __XEN_LATEST_INTERFACE_VERSION__ < 0x00030207
+# define xc_map_foreign_pages xc_map_foreign_batch
+#endif
+#if __XEN_LATEST_INTERFACE_VERSION__ < 0x00030208
+# define xen_mb() mb()
+# define xen_rmb() rmb()
+# define xen_wmb() wmb()
+#endif
+
+/* ------------------------------------------------------------- */
+/* useful defines */
+
+#define container_of(ptr, type, member) ({ \
+ const typeof( ((type *)0)->member ) *__mptr = (ptr); \
+ (type *)( (char *)__mptr - offsetof(type,member) );})
+
+#endif /* QEMU_XEN_COMMON_H */
diff --git a/hw/xen-machine.c b/hw/xen-machine.c
index 7757bde..f0ebd60 100644
--- a/hw/xen-machine.c
+++ b/hw/xen-machine.c
@@ -25,7 +25,7 @@
#include "hw.h"
#include "boards.h"
-#include <xenctrl.h>
+#include "xen-backend.h"
/* -------------------------------------------------------------------- */
/* variables */
@@ -81,6 +81,11 @@ static int xen_init(void)
xen_emulate = 1;
}
+ if (-1 == xen_be_init()) {
+ fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
+ return -1;
+ }
+
return 0;
}
--
1.5.5.1
^ permalink raw reply related [flat|nested] 92+ messages in thread* [Qemu-devel] [PATCH 3/7] xen: add console backend driver.
2008-08-04 15:50 [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Gerd Hoffmann
2008-08-04 15:50 ` [Qemu-devel] [PATCH 1/7] xen: groundwork for xen support Gerd Hoffmann
2008-08-04 15:50 ` [Qemu-devel] [PATCH 2/7] xen: backend driver core Gerd Hoffmann
@ 2008-08-04 15:50 ` Gerd Hoffmann
2008-08-04 16:52 ` Blue Swirl
2008-08-04 15:50 ` [Qemu-devel] [PATCH 4/7] xen: add framebuffer " Gerd Hoffmann
` (4 subsequent siblings)
7 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 15:50 UTC (permalink / raw)
To: qemu-devel, xen-devel; +Cc: Gerd Hoffmann
This patch adds a xenconsole backend driver. It it based on current
xen-unstable code. It has been changed to make use of the common
backend driver code.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
Makefile.target | 1 +
hw/xen-backend.h | 3 +
hw/xen-console.c | 270 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/xen-machine.c | 3 +
4 files changed, 277 insertions(+), 0 deletions(-)
create mode 100644 hw/xen-console.c
diff --git a/Makefile.target b/Makefile.target
index 65f4910..f61458c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -517,6 +517,7 @@ endif
# xen backend driver support
XEN_OBJS := xen-machine.o xen-backend.o
+XEN_OBJS += xen-console.o
ifeq ($(CONFIG_XEN), yes)
OBJS += $(XEN_OBJS)
LIBS += $(XEN_LIBS)
diff --git a/hw/xen-backend.h b/hw/xen-backend.h
index 13b3190..61c37ac 100644
--- a/hw/xen-backend.h
+++ b/hw/xen-backend.h
@@ -84,4 +84,7 @@ int xen_be_send_notify(struct XenDevice *xendev);
void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, ...)
__attribute__ ((format(printf, 3, 4)));
+/* actual backend drivers */
+struct XenDevOps xen_console_ops; /* xen_console.c */
+
#endif /* QEMU_XEN_BACKEND_H */
diff --git a/hw/xen-console.c b/hw/xen-console.c
new file mode 100644
index 0000000..57bd3a1
--- /dev/null
+++ b/hw/xen-console.c
@@ -0,0 +1,270 @@
+/*
+ * Copyright (C) International Business Machines Corp., 2005
+ * Author(s): Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * Copyright (C) Red Hat 2007
+ *
+ * Xen Console
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/select.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <termios.h>
+#include <stdarg.h>
+#include <sys/mman.h>
+#include <xs.h>
+#include <xen/io/console.h>
+#include <xenctrl.h>
+
+#include "hw.h"
+#include "sysemu.h"
+#include "qemu-char.h"
+#include "xen-backend.h"
+
+#define dolog(val, fmt, ...) fprintf(stderr, fmt "\n", ## __VA_ARGS__)
+
+struct buffer {
+ uint8_t *data;
+ size_t consumed;
+ size_t size;
+ size_t capacity;
+ size_t max_capacity;
+};
+
+struct XenConsole {
+ struct XenDevice xendev; /* must be first */
+ struct buffer buffer;
+ char console[BUFSIZE];
+ int ring_ref;
+ void *sring;
+ CharDriverState *chr;
+ int backlog;
+};
+
+static void buffer_append(struct XenConsole *con)
+{
+ struct buffer *buffer = &con->buffer;
+ XENCONS_RING_IDX cons, prod, size;
+ struct xencons_interface *intf = con->sring;
+
+ cons = intf->out_cons;
+ prod = intf->out_prod;
+ xen_mb();
+
+ size = prod - cons;
+ if ((size == 0) || (size > sizeof(intf->out)))
+ return;
+
+ if ((buffer->capacity - buffer->size) < size) {
+ buffer->capacity += (size + 1024);
+ buffer->data = realloc(buffer->data, buffer->capacity);
+ if (buffer->data == NULL) {
+ dolog(LOG_ERR, "Memory allocation failed");
+ exit(ENOMEM);
+ }
+ }
+
+ while (cons != prod)
+ buffer->data[buffer->size++] = intf->out[
+ MASK_XENCONS_IDX(cons++, intf->out)];
+
+ xen_mb();
+ intf->out_cons = cons;
+ xen_be_send_notify(&con->xendev);
+
+ if (buffer->max_capacity &&
+ buffer->size > buffer->max_capacity) {
+ /* Discard the middle of the data. */
+
+ size_t over = buffer->size - buffer->max_capacity;
+ uint8_t *maxpos = buffer->data + buffer->max_capacity;
+
+ memmove(maxpos - over, maxpos, over);
+ buffer->data = realloc(buffer->data, buffer->max_capacity);
+ buffer->size = buffer->capacity = buffer->max_capacity;
+
+ if (buffer->consumed > buffer->max_capacity - over)
+ buffer->consumed = buffer->max_capacity - over;
+ }
+}
+
+static void buffer_advance(struct buffer *buffer, size_t len)
+{
+ buffer->consumed += len;
+ if (buffer->consumed == buffer->size) {
+ buffer->consumed = 0;
+ buffer->size = 0;
+ }
+}
+
+static int ring_free_bytes(struct XenConsole *con)
+{
+ struct xencons_interface *intf = con->sring;
+ XENCONS_RING_IDX cons, prod, space;
+
+ cons = intf->in_cons;
+ prod = intf->in_prod;
+ xen_mb();
+
+ space = prod - cons;
+ if (space > sizeof(intf->in))
+ return 0; /* ring is screwed: ignore it */
+
+ return (sizeof(intf->in) - space);
+}
+
+static int xencons_can_receive(void *opaque)
+{
+ struct XenConsole *con = opaque;
+ return ring_free_bytes(con);
+}
+
+static void xencons_receive(void *opaque, const uint8_t *buf, int len)
+{
+ struct XenConsole *con = opaque;
+ struct xencons_interface *intf = con->sring;
+ XENCONS_RING_IDX prod;
+ int i, max;
+
+ max = ring_free_bytes(con);
+ /* The can_receive() func limits this, but check again anyway */
+ if (max < len)
+ len = max;
+
+ prod = intf->in_prod;
+ for (i = 0; i < len; i++) {
+ intf->in[MASK_XENCONS_IDX(prod++, intf->in)] =
+ buf[i];
+ }
+ xen_wmb();
+ intf->in_prod = prod;
+ xen_be_send_notify(&con->xendev);
+}
+
+static void xencons_send(struct XenConsole *con)
+{
+ ssize_t len, size;
+
+ size = con->buffer.size - con->buffer.consumed;
+ len = qemu_chr_write(con->chr, con->buffer.data + con->buffer.consumed,
+ size);
+ if (len < 1) {
+ if (!con->backlog) {
+ con->backlog = 1;
+ xen_be_printf(&con->xendev, 1, "backlog piling up, nobody listening?\n");
+ }
+ } else {
+ buffer_advance(&con->buffer, len);
+ if (con->backlog && len == size) {
+ con->backlog = 0;
+ xen_be_printf(&con->xendev, 1, "backlog is gone\n");
+ }
+ }
+}
+
+/* -------------------------------------------------------------------- */
+
+static int con_init(struct XenDevice *xendev)
+{
+ struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
+ char *type;
+
+ if (!serial_hds[con->xendev.dev]) {
+ xen_be_printf(xendev, 1, "serial line %d not configured\n", con->xendev.dev);
+ return -1;
+ }
+
+ /* setup */
+ snprintf(con->console, sizeof(con->console),
+ "/local/domain/%d/console", con->xendev.dom);
+ con->chr = serial_hds[con->xendev.dev];
+
+ type = xenstore_read_str(con->console, "type");
+ if (!type || 0 != strcmp(type, "ioemu")) {
+ xen_be_printf(xendev, 1, "not for me (type=%s)\n", type);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int con_connect(struct XenDevice *xendev)
+{
+ struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
+ int limit;
+
+ if (-1 == xenstore_read_int(con->console, "ring-ref", &con->ring_ref))
+ return -1;
+ if (-1 == xenstore_read_int(con->console, "port", &con->xendev.remote_port))
+ return -1;
+ if (0 == xenstore_read_int(con->console, "limit", &limit))
+ con->buffer.max_capacity = limit;
+
+ con->sring = xc_map_foreign_range(xen_xc, con->xendev.dom,
+ XC_PAGE_SIZE,
+ PROT_READ|PROT_WRITE,
+ con->ring_ref);
+ if (!con->sring)
+ return -1;
+
+ xen_be_bind_evtchn(&con->xendev);
+ qemu_chr_add_handlers(con->chr, xencons_can_receive, xencons_receive,
+ NULL, con);
+
+ xen_be_printf(xendev, 1, "ring mfn %d, remote port %d, local port %d, limit %zd\n",
+ con->ring_ref,
+ con->xendev.remote_port,
+ con->xendev.local_port,
+ con->buffer.max_capacity);
+ return 0;
+}
+
+static void con_disconnect(struct XenDevice *xendev)
+{
+ struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
+
+ qemu_chr_add_handlers(con->chr, NULL, NULL, NULL, NULL);
+ xen_be_unbind_evtchn(&con->xendev);
+
+ if (con->sring) {
+ munmap(con->sring, XC_PAGE_SIZE);
+ con->sring = NULL;
+ }
+}
+
+static void con_event(struct XenDevice *xendev)
+{
+ struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
+
+ buffer_append(con);
+ if (con->buffer.size - con->buffer.consumed)
+ xencons_send(con);
+}
+
+/* -------------------------------------------------------------------- */
+
+struct XenDevOps xen_console_ops = {
+ .size = sizeof(struct XenConsole),
+ .flags = DEVOPS_FLAG_IGNORE_STATE,
+ .init = con_init,
+ .connect = con_connect,
+ .event = con_event,
+ .disconnect = con_disconnect,
+};
diff --git a/hw/xen-machine.c b/hw/xen-machine.c
index f0ebd60..de22cdb 100644
--- a/hw/xen-machine.c
+++ b/hw/xen-machine.c
@@ -97,6 +97,9 @@ static int xen_init_pv(DisplayState *ds)
if (rc < 0)
return rc;
+ /* xenbus backend drivers */
+ xen_be_register("console", &xen_console_ops);
+
return 0;
}
--
1.5.5.1
^ permalink raw reply related [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 3/7] xen: add console backend driver.
2008-08-04 15:50 ` [Qemu-devel] [PATCH 3/7] xen: add console backend driver Gerd Hoffmann
@ 2008-08-04 16:52 ` Blue Swirl
2008-08-04 18:15 ` Gerd Hoffmann
0 siblings, 1 reply; 92+ messages in thread
From: Blue Swirl @ 2008-08-04 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: xen-devel, Gerd Hoffmann
On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> + buffer->data = realloc(buffer->data, buffer->capacity);
On Qemu, instead of malloc, qemu_malloc should be used to allocate
memory. Though there isn't qemu_realloc.
> + if (!type || 0 != strcmp(type, "ioemu")) {
Shouldn't this be "qemu"?
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 3/7] xen: add console backend driver.
2008-08-04 16:52 ` Blue Swirl
@ 2008-08-04 18:15 ` Gerd Hoffmann
2008-08-04 20:47 ` Blue Swirl
0 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 18:15 UTC (permalink / raw)
To: Blue Swirl; +Cc: xen-devel, qemu-devel
Blue Swirl wrote:
> On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> + buffer->data = realloc(buffer->data, buffer->capacity);
>
> On Qemu, instead of malloc, qemu_malloc should be used to allocate
> memory. Though there isn't qemu_realloc.
Should I add qemu_realloc then?
>> + if (!type || 0 != strcmp(type, "ioemu")) {
>
> Shouldn't this be "qemu"?
No, "ioemu" is correct. xend writes that value into xenstore.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 3/7] xen: add console backend driver.
2008-08-04 18:15 ` Gerd Hoffmann
@ 2008-08-04 20:47 ` Blue Swirl
0 siblings, 0 replies; 92+ messages in thread
From: Blue Swirl @ 2008-08-04 20:47 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, qemu-devel
On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Blue Swirl wrote:
> > On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >> + buffer->data = realloc(buffer->data, buffer->capacity);
> >
> > On Qemu, instead of malloc, qemu_malloc should be used to allocate
> > memory. Though there isn't qemu_realloc.
>
>
> Should I add qemu_realloc then?
That would be nice, but changing also all reallocs to qemu_realloc
calls for a new patch.
^ permalink raw reply [flat|nested] 92+ messages in thread
* [Qemu-devel] [PATCH 4/7] xen: add framebuffer backend driver
2008-08-04 15:50 [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Gerd Hoffmann
` (2 preceding siblings ...)
2008-08-04 15:50 ` [Qemu-devel] [PATCH 3/7] xen: add console backend driver Gerd Hoffmann
@ 2008-08-04 15:50 ` Gerd Hoffmann
2008-08-04 17:09 ` Blue Swirl
2008-08-04 15:50 ` [Qemu-devel] [PATCH 5/7] xen: add block device " Gerd Hoffmann
` (3 subsequent siblings)
7 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 15:50 UTC (permalink / raw)
To: qemu-devel, xen-devel; +Cc: Gerd Hoffmann
This patch adds a frsamebuffer (and kbd+mouse) backend driver. It
it based on current xen-unstable code. It has been changed to make
use of the common backend driver code. It also has been changed to
compile with xen headers older than unstable (aka son-to-be 3.3).
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
Makefile.target | 2 +-
hw/xen-backend.h | 4 +
hw/xen-framebuffer.c | 933 ++++++++++++++++++++++++++++++++++++++++++++++++++
hw/xen-machine.c | 5 +
4 files changed, 943 insertions(+), 1 deletions(-)
create mode 100644 hw/xen-framebuffer.c
diff --git a/Makefile.target b/Makefile.target
index f61458c..39db580 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -517,7 +517,7 @@ endif
# xen backend driver support
XEN_OBJS := xen-machine.o xen-backend.o
-XEN_OBJS += xen-console.o
+XEN_OBJS += xen-console.o xen-framebuffer.o
ifeq ($(CONFIG_XEN), yes)
OBJS += $(XEN_OBJS)
LIBS += $(XEN_LIBS)
diff --git a/hw/xen-backend.h b/hw/xen-backend.h
index 61c37ac..f7dd0c8 100644
--- a/hw/xen-backend.h
+++ b/hw/xen-backend.h
@@ -86,5 +86,9 @@ void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, ...
/* actual backend drivers */
struct XenDevOps xen_console_ops; /* xen_console.c */
+struct XenDevOps xen_kbdmouse_ops; /* xen_framebuffer.c */
+struct XenDevOps xen_framebuffer_ops; /* xen_framebuffer.c */
+
+void xen_set_display(int domid, DisplayState *ds);
#endif /* QEMU_XEN_BACKEND_H */
diff --git a/hw/xen-framebuffer.c b/hw/xen-framebuffer.c
new file mode 100644
index 0000000..d406c22
--- /dev/null
+++ b/hw/xen-framebuffer.c
@@ -0,0 +1,933 @@
+/*
+ * xen paravirt framebuffer backend
+ *
+ * Copyright IBM, Corp. 2005-2006
+ * Copyright Red Hat, Inc. 2006-2008
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>,
+ * Markus Armbruster <armbru@redhat.com>,
+ * Daniel P. Berrange <berrange@redhat.com>,
+ * Pat Campbell <plc@novell.com>,
+ * Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <stdarg.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdbool.h>
+#include <sys/mman.h>
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+#include <time.h>
+
+#include <xs.h>
+#include <xenctrl.h>
+#include <xen/event_channel.h>
+#include <xen/io/xenbus.h>
+#include <xen/io/fbif.h>
+#include <xen/io/kbdif.h>
+#include <xen/io/protocols.h>
+
+#include "hw.h"
+#include "sysemu.h"
+#include "console.h"
+#include "qemu-char.h"
+#include "xen-backend.h"
+
+#ifndef BTN_LEFT
+#define BTN_LEFT 0x110 /* from <linux/input.h> */
+#endif
+
+/* -------------------------------------------------------------------- */
+
+struct common {
+ struct XenDevice xendev; /* must be first */
+ void *page;
+ DisplayState *ds;
+};
+
+struct XenInput {
+ struct common c;
+ int abs_pointer_wanted; /* Whether guest supports absolute pointer */
+ int button_state; /* Last seen pointer button state */
+ int extended;
+ QEMUPutMouseEntry *qmouse;
+};
+
+#define UP_QUEUE 8
+
+struct XenFB {
+ struct common c;
+ size_t fb_len;
+ int row_stride;
+ int depth;
+ int width;
+ int height;
+ int offset;
+ void *pixels;
+ int feature_update;
+ int refresh_period;
+ int bug_trigger;
+
+ struct {
+ int x,y,w,h;
+ } up_rects[UP_QUEUE];
+ int up_count;
+ int up_fullscreen;
+};
+
+/* -------------------------------------------------------------------- */
+
+static int common_bind(struct common *c)
+{
+ int mfn;
+
+ if (-1 == xenstore_read_fe_int(&c->xendev, "page-ref", &mfn))
+ return -1;
+ if (-1 == xenstore_read_fe_int(&c->xendev, "event-channel", &c->xendev.remote_port))
+ return -1;
+
+ xen_be_bind_evtchn(&c->xendev);
+ xen_be_printf(&c->xendev, 1, "ring mfn %d, remote-port %d, local-port %d\n",
+ mfn, c->xendev.remote_port, c->xendev.local_port);
+
+ c->page = xc_map_foreign_range(xen_xc, c->xendev.dom,
+ XC_PAGE_SIZE,
+ PROT_READ | PROT_WRITE, mfn);
+ if (NULL == c->page)
+ return -1;
+ return 0;
+}
+
+static void common_unbind(struct common *c)
+{
+ if (c->page) {
+ munmap(c->page, XC_PAGE_SIZE);
+ c->page = NULL;
+ }
+ xen_be_unbind_evtchn(&c->xendev);
+}
+
+/* -------------------------------------------------------------------- */
+
+/*
+ * Tables to map from scancode to Linux input layer keycode.
+ * Scancodes are hardware-specific. These maps assumes a
+ * standard AT or PS/2 keyboard which is what QEMU feeds us.
+ */
+const unsigned char atkbd_set2_keycode[512] = {
+
+ 0, 67, 65, 63, 61, 59, 60, 88, 0, 68, 66, 64, 62, 15, 41,117,
+ 0, 56, 42, 93, 29, 16, 2, 0, 0, 0, 44, 31, 30, 17, 3, 0,
+ 0, 46, 45, 32, 18, 5, 4, 95, 0, 57, 47, 33, 20, 19, 6,183,
+ 0, 49, 48, 35, 34, 21, 7,184, 0, 0, 50, 36, 22, 8, 9,185,
+ 0, 51, 37, 23, 24, 11, 10, 0, 0, 52, 53, 38, 39, 25, 12, 0,
+ 0, 89, 40, 0, 26, 13, 0, 0, 58, 54, 28, 27, 0, 43, 0, 85,
+ 0, 86, 91, 90, 92, 0, 14, 94, 0, 79,124, 75, 71,121, 0, 0,
+ 82, 83, 80, 76, 77, 72, 1, 69, 87, 78, 81, 74, 55, 73, 70, 99,
+
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+ 217,100,255, 0, 97,165, 0, 0,156, 0, 0, 0, 0, 0, 0,125,
+ 173,114, 0,113, 0, 0, 0,126,128, 0, 0,140, 0, 0, 0,127,
+ 159, 0,115, 0,164, 0, 0,116,158, 0,150,166, 0, 0, 0,142,
+ 157, 0, 0, 0, 0, 0, 0, 0,155, 0, 98, 0, 0,163, 0, 0,
+ 226, 0, 0, 0, 0, 0, 0, 0, 0,255, 96, 0, 0, 0,143, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0,107, 0,105,102, 0, 0,112,
+ 110,111,108,112,106,103, 0,119, 0,118,109, 0, 99,104,119, 0,
+
+};
+
+const unsigned char atkbd_unxlate_table[128] = {
+
+ 0,118, 22, 30, 38, 37, 46, 54, 61, 62, 70, 69, 78, 85,102, 13,
+ 21, 29, 36, 45, 44, 53, 60, 67, 68, 77, 84, 91, 90, 20, 28, 27,
+ 35, 43, 52, 51, 59, 66, 75, 76, 82, 14, 18, 93, 26, 34, 33, 42,
+ 50, 49, 58, 65, 73, 74, 89,124, 17, 41, 88, 5, 6, 4, 12, 3,
+ 11, 2, 10, 1, 9,119,126,108,117,125,123,107,115,116,121,105,
+ 114,122,112,113,127, 96, 97,120, 7, 15, 23, 31, 39, 47, 55, 63,
+ 71, 79, 86, 94, 8, 16, 24, 32, 40, 48, 56, 64, 72, 80, 87,111,
+ 19, 25, 57, 81, 83, 92, 95, 98, 99,100,101,103,104,106,109,110
+
+};
+
+static unsigned char scancode2linux[512];
+
+/* Send an event to the keyboard frontend driver */
+static int xenfb_kbd_event(struct XenInput *xenfb,
+ union xenkbd_in_event *event)
+{
+ struct xenkbd_page *page = xenfb->c.page;
+ uint32_t prod;
+
+ if (xenfb->c.xendev.be_state != XenbusStateConnected)
+ return 0;
+
+ prod = page->in_prod;
+ if (prod - page->in_cons == XENKBD_IN_RING_LEN) {
+ errno = EAGAIN;
+ return -1;
+ }
+
+ xen_mb(); /* ensure ring space available */
+ XENKBD_IN_RING_REF(page, prod) = *event;
+ xen_wmb(); /* ensure ring contents visible */
+ page->in_prod = prod + 1;
+ return xen_be_send_notify(&xenfb->c.xendev);
+}
+
+/* Send a keyboard (or mouse button) event */
+static int xenfb_send_key(struct XenInput *xenfb, bool down, int keycode)
+{
+ union xenkbd_in_event event;
+
+ memset(&event, 0, XENKBD_IN_EVENT_SIZE);
+ event.type = XENKBD_TYPE_KEY;
+ event.key.pressed = down ? 1 : 0;
+ event.key.keycode = keycode;
+
+ return xenfb_kbd_event(xenfb, &event);
+}
+
+/* Send a relative mouse movement event */
+static int xenfb_send_motion(struct XenInput *xenfb,
+ int rel_x, int rel_y, int rel_z)
+{
+ union xenkbd_in_event event;
+
+ memset(&event, 0, XENKBD_IN_EVENT_SIZE);
+ event.type = XENKBD_TYPE_MOTION;
+ event.motion.rel_x = rel_x;
+ event.motion.rel_y = rel_y;
+#if __XEN_LATEST_INTERFACE_VERSION__ >= 0x00030207
+ event.motion.rel_z = rel_z;
+#endif
+
+ return xenfb_kbd_event(xenfb, &event);
+}
+
+/* Send an absolute mouse movement event */
+static int xenfb_send_position(struct XenInput *xenfb,
+ int abs_x, int abs_y, int rel_z)
+{
+ union xenkbd_in_event event;
+
+ memset(&event, 0, XENKBD_IN_EVENT_SIZE);
+ event.type = XENKBD_TYPE_POS;
+ event.pos.abs_x = abs_x;
+ event.pos.abs_y = abs_y;
+#if __XEN_LATEST_INTERFACE_VERSION__ >= 0x00030207
+ event.pos.rel_z = rel_z;
+#endif
+
+ return xenfb_kbd_event(xenfb, &event);
+}
+
+/*
+ * Send a key event from the client to the guest OS
+ * QEMU gives us a raw scancode from an AT / PS/2 style keyboard.
+ * We have to turn this into a Linux Input layer keycode.
+ *
+ * Extra complexity from the fact that with extended scancodes
+ * (like those produced by arrow keys) this method gets called
+ * twice, but we only want to send a single event. So we have to
+ * track the '0xe0' scancode state & collapse the extended keys
+ * as needed.
+ *
+ * Wish we could just send scancodes straight to the guest which
+ * already has code for dealing with this...
+ */
+static void xenfb_key_event(void *opaque, int scancode)
+{
+ struct XenInput *xenfb = opaque;
+ int down = 1;
+
+ if (scancode == 0xe0) {
+ xenfb->extended = 1;
+ return;
+ } else if (scancode & 0x80) {
+ scancode &= 0x7f;
+ down = 0;
+ }
+ if (xenfb->extended) {
+ scancode |= 0x80;
+ xenfb->extended = 0;
+ }
+ xenfb_send_key(xenfb, down, scancode2linux[scancode]);
+}
+
+/*
+ * Send a mouse event from the client to the guest OS
+ *
+ * The QEMU mouse can be in either relative, or absolute mode.
+ * Movement is sent separately from button state, which has to
+ * be encoded as virtual key events. We also don't actually get
+ * given any button up/down events, so have to track changes in
+ * the button state.
+ */
+static void xenfb_mouse_event(void *opaque,
+ int dx, int dy, int dz, int button_state)
+{
+ struct XenInput *xenfb = opaque;
+ int i;
+
+ if (xenfb->abs_pointer_wanted)
+ xenfb_send_position(xenfb,
+ dx * (xenfb->c.ds->width - 1) / 0x7fff,
+ dy * (xenfb->c.ds->height - 1) / 0x7fff,
+ dz);
+ else
+ xenfb_send_motion(xenfb, dx, dy, dz);
+
+ for (i = 0 ; i < 8 ; i++) {
+ int lastDown = xenfb->button_state & (1 << i);
+ int down = button_state & (1 << i);
+ if (down == lastDown)
+ continue;
+
+ if (xenfb_send_key(xenfb, down, BTN_LEFT+i) < 0)
+ return;
+ }
+ xenfb->button_state = button_state;
+}
+
+static int input_init(struct XenDevice *xendev)
+{
+ struct XenInput *in = container_of(xendev, struct XenInput, c.xendev);
+ static int first = 1;
+ int i;
+
+ if (first) {
+ /* Prepare scancode mapping table */
+ for (i = 0; i < 128; i++) {
+ scancode2linux[i] = atkbd_set2_keycode[atkbd_unxlate_table[i]];
+ scancode2linux[i | 0x80] =
+ atkbd_set2_keycode[atkbd_unxlate_table[i] | 0x80];
+ }
+ first = 0;
+ }
+
+ if (!in->c.ds) {
+ /* xen_set_display() below will set that and trigger us again */
+ xen_be_printf(xendev, 1, "ds not set (yet)\n");
+ return -1;
+ }
+
+ xenstore_write_be_int(xendev, "feature-abs-pointer", 1);
+ return 0;
+}
+
+static int input_connect(struct XenDevice *xendev)
+{
+ struct XenInput *in = container_of(xendev, struct XenInput, c.xendev);
+ int rc;
+
+ if (-1 == xenstore_read_fe_int(xendev, "request-abs-pointer", &in->abs_pointer_wanted))
+ in->abs_pointer_wanted = 0;
+
+ rc = common_bind(&in->c);
+ if (0 != rc)
+ return rc;
+
+ qemu_add_kbd_event_handler(xenfb_key_event, in);
+ in->qmouse = qemu_add_mouse_event_handler(xenfb_mouse_event, in,
+ in->abs_pointer_wanted,
+ "Xen PVFB Mouse");
+ return 0;
+}
+
+static void input_disconnect(struct XenDevice *xendev)
+{
+ struct XenInput *in = container_of(xendev, struct XenInput, c.xendev);
+
+ if (in->qmouse) {
+ qemu_remove_mouse_event_handler(in->qmouse);
+ in->qmouse = NULL;
+ }
+ qemu_add_kbd_event_handler(NULL, NULL);
+ common_unbind(&in->c);
+}
+
+static void input_event(struct XenDevice *xendev)
+{
+ struct XenInput *xenfb = container_of(xendev, struct XenInput, c.xendev);
+ struct xenkbd_page *page = xenfb->c.page;
+
+ /* We don't understand any keyboard events, so just ignore them. */
+ if (page->out_prod == page->out_cons)
+ return;
+ page->out_cons = page->out_prod;
+ xen_be_send_notify(&xenfb->c.xendev);
+}
+
+/* -------------------------------------------------------------------- */
+
+static void xenfb_copy_mfns(int mode, int count, unsigned long *dst, void *src)
+{
+ uint32_t *src32 = src;
+ uint64_t *src64 = src;
+ int i;
+
+ for (i = 0; i < count; i++)
+ dst[i] = (mode == 32) ? src32[i] : src64[i];
+}
+
+static int xenfb_map_fb(struct XenFB *xenfb)
+{
+ struct xenfb_page *page = xenfb->c.page;
+ char *protocol = xenfb->c.xendev.protocol;
+ int n_fbmfns;
+ int n_fbdirs;
+ unsigned long *pgmfns = NULL;
+ unsigned long *fbmfns = NULL;
+ void *map, *pd;
+ int mode, ret = -1;
+
+ /* default to native */
+ pd = page->pd;
+ mode = sizeof(unsigned long) * 8;
+
+ if (!protocol) {
+ /*
+ * Undefined protocol, some guesswork needed.
+ *
+ * Old frontends which don't set the protocol use
+ * one page directory only, thus pd[1] must be zero.
+ * pd[1] of the 32bit struct layout and the lower
+ * 32 bits of pd[0] of the 64bit struct layout have
+ * the same location, so we can check that ...
+ */
+ uint32_t *ptr32 = NULL;
+ uint32_t *ptr64 = NULL;
+#if defined(__i386__)
+ ptr32 = (void*)page->pd;
+ ptr64 = ((void*)page->pd) + 4;
+#elif defined(__x86_64__)
+ ptr32 = ((void*)page->pd) - 4;
+ ptr64 = (void*)page->pd;
+#endif
+ if (ptr32) {
+ if (0 == ptr32[1]) {
+ mode = 32;
+ pd = ptr32;
+ } else {
+ mode = 64;
+ pd = ptr64;
+ }
+ }
+#if defined(__x86_64__)
+ } else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32)) {
+ /* 64bit dom0, 32bit domU */
+ mode = 32;
+ pd = ((void*)page->pd) - 4;
+#elif defined(__i386__)
+ } else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_64)) {
+ /* 32bit dom0, 64bit domU */
+ mode = 64;
+ pd = ((void*)page->pd) + 4;
+#endif
+ }
+
+ n_fbmfns = (xenfb->fb_len + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE;
+ n_fbdirs = n_fbmfns * mode / 8;
+ n_fbdirs = (n_fbdirs + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE;
+
+ pgmfns = qemu_mallocz(sizeof(unsigned long) * n_fbdirs);
+ fbmfns = qemu_mallocz(sizeof(unsigned long) * n_fbmfns);
+ if (!pgmfns || !fbmfns)
+ goto out;
+
+ xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd);
+ map = xc_map_foreign_pages(xen_xc, xenfb->c.xendev.dom,
+ PROT_READ, pgmfns, n_fbdirs);
+ if (map == NULL)
+ goto out;
+ xenfb_copy_mfns(mode, n_fbmfns, fbmfns, map);
+ munmap(map, n_fbdirs * XC_PAGE_SIZE);
+
+ xenfb->pixels = xc_map_foreign_pages(xen_xc, xenfb->c.xendev.dom,
+ PROT_READ | PROT_WRITE, fbmfns, n_fbmfns);
+ if (xenfb->pixels == NULL)
+ goto out;
+
+ ret = 0; /* all is fine */
+
+out:
+ qemu_free(pgmfns);
+ qemu_free(fbmfns);
+ return ret;
+}
+
+static int xenfb_configure_fb(struct XenFB *xenfb, size_t fb_len_lim,
+ int width, int height, int depth,
+ size_t fb_len, int offset, int row_stride)
+{
+ size_t mfn_sz = sizeof(*((struct xenfb_page *)0)->pd);
+ size_t pd_len = sizeof(((struct xenfb_page *)0)->pd) / mfn_sz;
+ size_t fb_pages = pd_len * XC_PAGE_SIZE / mfn_sz;
+ size_t fb_len_max = fb_pages * XC_PAGE_SIZE;
+ int max_width, max_height;
+
+ if (fb_len_lim > fb_len_max) {
+ xen_be_printf(&xenfb->c.xendev, 0, "fb size limit %zu exceeds %zu, corrected\n",
+ fb_len_lim, fb_len_max);
+ fb_len_lim = fb_len_max;
+ }
+ if (fb_len_lim && fb_len > fb_len_lim) {
+ xen_be_printf(&xenfb->c.xendev, 0, "frontend fb size %zu limited to %zu\n",
+ fb_len, fb_len_lim);
+ fb_len = fb_len_lim;
+ }
+ if (depth != 8 && depth != 16 && depth != 24 && depth != 32) {
+ xen_be_printf(&xenfb->c.xendev, 0, "can't handle frontend fb depth %d\n",
+ depth);
+ return -1;
+ }
+ if (row_stride < 0 || row_stride > fb_len) {
+ xen_be_printf(&xenfb->c.xendev, 0, "invalid frontend stride %d\n", row_stride);
+ return -1;
+ }
+ max_width = row_stride / (depth / 8);
+ if (width < 0 || width > max_width) {
+ xen_be_printf(&xenfb->c.xendev, 0, "invalid frontend width %d limited to %d\n",
+ width, max_width);
+ width = max_width;
+ }
+ if (offset < 0 || offset >= fb_len) {
+ xen_be_printf(&xenfb->c.xendev, 0, "invalid frontend offset %d (max %zu)\n",
+ offset, fb_len - 1);
+ return -1;
+ }
+ max_height = (fb_len - offset) / row_stride;
+ if (height < 0 || height > max_height) {
+ xen_be_printf(&xenfb->c.xendev, 0, "invalid frontend height %d limited to %d\n",
+ height, max_height);
+ height = max_height;
+ }
+ xenfb->fb_len = fb_len;
+ xenfb->row_stride = row_stride;
+ xenfb->depth = depth;
+ xenfb->width = width;
+ xenfb->height = height;
+ xenfb->offset = offset;
+ xen_be_printf(&xenfb->c.xendev, 1, "framebuffer %dx%dx%d offset %d stride %d\n",
+ width, height, depth, offset, row_stride);
+ return 0;
+}
+
+/* A convenient function for munging pixels between different depths */
+#define BLT(SRC_T,DST_T,RSB,GSB,BSB,RDB,GDB,BDB) \
+ for (line = y ; line < (y+h) ; line++) { \
+ SRC_T *src = (SRC_T *)(xenfb->pixels \
+ + xenfb->offset \
+ + (line * xenfb->row_stride) \
+ + (x * xenfb->depth / 8)); \
+ DST_T *dst = (DST_T *)(xenfb->c.ds->data \
+ + (line * xenfb->c.ds->linesize) \
+ + (x * xenfb->c.ds->depth / 8)); \
+ int col; \
+ const int RSS = 32 - (RSB + GSB + BSB); \
+ const int GSS = 32 - (GSB + BSB); \
+ const int BSS = 32 - (BSB); \
+ const uint32_t RSM = (~0U) << (32 - RSB); \
+ const uint32_t GSM = (~0U) << (32 - GSB); \
+ const uint32_t BSM = (~0U) << (32 - BSB); \
+ const int RDS = 32 - (RDB + GDB + BDB); \
+ const int GDS = 32 - (GDB + BDB); \
+ const int BDS = 32 - (BDB); \
+ const uint32_t RDM = (~0U) << (32 - RDB); \
+ const uint32_t GDM = (~0U) << (32 - GDB); \
+ const uint32_t BDM = (~0U) << (32 - BDB); \
+ for (col = x ; col < (x+w) ; col++) { \
+ uint32_t spix = *src; \
+ *dst = (((spix << RSS) & RSM & RDM) >> RDS) | \
+ (((spix << GSS) & GSM & GDM) >> GDS) | \
+ (((spix << BSS) & BSM & BDM) >> BDS); \
+ src = (SRC_T *) ((unsigned long) src + xenfb->depth / 8); \
+ dst = (DST_T *) ((unsigned long) dst + xenfb->c.ds->depth / 8); \
+ } \
+ }
+
+
+/* This copies data from the guest framebuffer region, into QEMU's copy
+ * NB. QEMU's copy is stored in the pixel format of a) the local X
+ * server (SDL case) or b) the current VNC client pixel format.
+ * When shifting between colour depths we preserve the MSB.
+ */
+static void xenfb_guest_copy(struct XenFB *xenfb, int x, int y, int w, int h)
+{
+ int line;
+
+ if (1 /* !xenfb->c.ds->shared_buf */) {
+ if (xenfb->depth == xenfb->c.ds->depth) { /* Perfect match can use fast path */
+ for (line = y ; line < (y+h) ; line++) {
+ memcpy(xenfb->c.ds->data + (line * xenfb->c.ds->linesize) + (x * xenfb->c.ds->depth / 8),
+ xenfb->pixels + xenfb->offset + (line * xenfb->row_stride) + (x * xenfb->depth / 8),
+ w * xenfb->depth / 8);
+ }
+ } else { /* Mismatch requires slow pixel munging */
+ /* 8 bit == r:3 g:3 b:2 */
+ /* 16 bit == r:5 g:6 b:5 */
+ /* 24 bit == r:8 g:8 b:8 */
+ /* 32 bit == r:8 g:8 b:8 (padding:8) */
+ if (xenfb->depth == 8) {
+ if (xenfb->c.ds->depth == 16) {
+ BLT(uint8_t, uint16_t, 3, 3, 2, 5, 6, 5);
+ } else if (xenfb->c.ds->depth == 32) {
+ BLT(uint8_t, uint32_t, 3, 3, 2, 8, 8, 8);
+ }
+ } else if (xenfb->depth == 16) {
+ if (xenfb->c.ds->depth == 8) {
+ BLT(uint16_t, uint8_t, 5, 6, 5, 3, 3, 2);
+ } else if (xenfb->c.ds->depth == 32) {
+ BLT(uint16_t, uint32_t, 5, 6, 5, 8, 8, 8);
+ }
+ } else if (xenfb->depth == 24 || xenfb->depth == 32) {
+ if (xenfb->c.ds->depth == 8) {
+ BLT(uint32_t, uint8_t, 8, 8, 8, 3, 3, 2);
+ } else if (xenfb->c.ds->depth == 16) {
+ BLT(uint32_t, uint16_t, 8, 8, 8, 5, 6, 5);
+ } else if (xenfb->c.ds->depth == 32) {
+ BLT(uint32_t, uint32_t, 8, 8, 8, 8, 8, 8);
+ }
+ }
+ }
+ }
+ dpy_update(xenfb->c.ds, x, y, w, h);
+}
+
+#ifdef XENFB_TYPE_REFRESH_PERIOD
+static int xenfb_queue_full(struct XenFB *xenfb)
+{
+ struct xenfb_page *page = xenfb->c.page;
+ uint32_t cons, prod;
+
+ prod = page->in_prod;
+ cons = page->in_cons;
+ return prod - cons == XENFB_IN_RING_LEN;
+}
+
+static void xenfb_send_event(struct XenFB *xenfb, union xenfb_in_event *event)
+{
+ uint32_t prod;
+ struct xenfb_page *page = xenfb->c.page;
+
+ prod = page->in_prod;
+ /* caller ensures !xenfb_queue_full() */
+ xen_mb(); /* ensure ring space available */
+ XENFB_IN_RING_REF(page, prod) = *event;
+ xen_wmb(); /* ensure ring contents visible */
+ page->in_prod = prod + 1;
+
+ xen_be_send_notify(&xenfb->c.xendev);
+}
+
+static void xenfb_send_refresh_period(struct XenFB *xenfb, int period)
+{
+ union xenfb_in_event event;
+
+ memset(&event, 0, sizeof(event));
+ event.type = XENFB_TYPE_REFRESH_PERIOD;
+ event.refresh_period.period = period;
+ xenfb_send_event(xenfb, &event);
+}
+#endif
+
+/*
+ * Periodic update of display.
+ * Also transmit the refresh interval to the frontend.
+ *
+ * Never ever do any qemu display operations
+ * (resize, screen update) outside this function.
+ * Our screen might be inactive. When asked for
+ * an update we know it is active.
+ */
+static void xenfb_update(void *opaque)
+{
+ struct XenFB *xenfb = opaque;
+ int i;
+
+ if (xenfb->feature_update) {
+#ifdef XENFB_TYPE_REFRESH_PERIOD
+ int period;
+
+ if (xenfb_queue_full(xenfb))
+ return;
+
+ /*
+ * TODO: xen's qemu-dm seems to have some patches to
+ * make the qemu display code avoid unneeded
+ * work.
+ * - Port them over.
+ * - Put ds->idle back into use then.
+ * - Same goes for ds->shared_buf btw.
+ */
+ if (0 /* xenfb->c.ds->idle */)
+ period = XENFB_NO_REFRESH;
+ else {
+ period = xenfb->c.ds->gui_timer_interval;
+ if (!period)
+ period = 30; // GUI_REFRESH_INTERVAL (see vl.c)
+ }
+
+ if (xenfb->refresh_period != period) {
+ dprintf("%s: %d\n", __FUNCTION__, period);
+ xenfb_send_refresh_period(xenfb, period);
+ xenfb->refresh_period = period;
+ }
+#else
+ ; /* nothing */
+#endif
+ } else {
+ /* we don't get update notifications, thus use the
+ * sledge hammer approach ... */
+ xenfb->up_fullscreen = 1;
+ }
+
+ /* resize if needed */
+ if (xenfb->width != xenfb->c.ds->width || xenfb->height != xenfb->c.ds->height) {
+ xen_be_printf(&xenfb->c.xendev, 1, "update: resizing\n");
+ dpy_resize(xenfb->c.ds, xenfb->width, xenfb->height);
+ xenfb->up_fullscreen = 1;
+ }
+
+ /* run queued updates */
+ if (xenfb->up_fullscreen) {
+ xen_be_printf(&xenfb->c.xendev, 3, "update: fullscreen\n");
+ xenfb_guest_copy(xenfb, 0, 0, xenfb->width, xenfb->height);
+ } else if (xenfb->up_count) {
+ xen_be_printf(&xenfb->c.xendev, 3, "update: %d rects\n", xenfb->up_count);
+ for (i = 0; i < xenfb->up_count; i++)
+ xenfb_guest_copy(xenfb,
+ xenfb->up_rects[i].x,
+ xenfb->up_rects[i].y,
+ xenfb->up_rects[i].w,
+ xenfb->up_rects[i].h);
+ } else {
+ xen_be_printf(&xenfb->c.xendev, 3, "update: nothing\n");
+ }
+ xenfb->up_count = 0;
+ xenfb->up_fullscreen = 0;
+}
+
+static void xenfb_handle_events(struct XenFB *xenfb)
+{
+ uint32_t prod, cons;
+ struct xenfb_page *page = xenfb->c.page;
+
+ prod = page->out_prod;
+ if (prod == page->out_cons)
+ return;
+ xen_rmb(); /* ensure we see ring contents up to prod */
+ for (cons = page->out_cons; cons != prod; cons++) {
+ union xenfb_out_event *event = &XENFB_OUT_RING_REF(page, cons);
+ int x, y, w, h;
+
+ switch (event->type) {
+ case XENFB_TYPE_UPDATE:
+ if (xenfb->up_count == UP_QUEUE)
+ xenfb->up_fullscreen = 1;
+ if (xenfb->up_fullscreen)
+ break;
+ x = MAX(event->update.x, 0);
+ y = MAX(event->update.y, 0);
+ w = MIN(event->update.width, xenfb->width - x);
+ h = MIN(event->update.height, xenfb->height - y);
+ if (w < 0 || h < 0) {
+ fprintf(stderr, "xen be: %s: bogus update ignored\n",
+ xenfb->c.xendev.name);
+ break;
+ }
+ if (x != event->update.x || y != event->update.y
+ || w != event->update.width
+ || h != event->update.height) {
+ fprintf(stderr, "xen be: %s: bogus update clipped\n",
+ xenfb->c.xendev.name);
+ }
+ if (w == xenfb->width && h > xenfb->height / 2) {
+ /* scroll detector: updated more than 50% of the lines,
+ * don't bother keeping track of the rectangles then */
+ xenfb->up_fullscreen = 1;
+ } else {
+ xenfb->up_rects[xenfb->up_count].x = x;
+ xenfb->up_rects[xenfb->up_count].y = y;
+ xenfb->up_rects[xenfb->up_count].w = w;
+ xenfb->up_rects[xenfb->up_count].h = h;
+ xenfb->up_count++;
+ }
+ break;
+#ifdef XENFB_TYPE_RESIZE
+ case XENFB_TYPE_RESIZE:
+ if (xenfb_configure_fb(xenfb, xenfb->fb_len,
+ event->resize.width,
+ event->resize.height,
+ event->resize.depth,
+ xenfb->fb_len,
+ event->resize.offset,
+ event->resize.stride) < 0)
+ break;
+ xenfb_invalidate(xenfb);
+ break;
+#endif
+ }
+ }
+ xen_mb(); /* ensure we're done with ring contents */
+ page->out_cons = cons;
+}
+
+/* QEMU display state changed, so refresh the framebuffer copy */
+static void xenfb_invalidate(void *opaque)
+{
+ struct XenFB *xenfb = opaque;
+ xenfb->up_fullscreen = 1;
+}
+
+static int fb_init(struct XenDevice *xendev)
+{
+ struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev);
+
+ if (!fb->c.ds) {
+ /* xen_set_display() below will set that and trigger us again */
+ xen_be_printf(xendev, 1, "ds not set (yet)\n");
+ return -1;
+ }
+
+ fb->refresh_period = -1;
+
+#ifdef XENFB_TYPE_RESIZE
+ xenstore_write_be_int(xendev, "feature-resize", 1);
+#endif
+ return 0;
+}
+
+static int fb_connect(struct XenDevice *xendev)
+{
+ struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev);
+ struct xenfb_page *fb_page;
+ int videoram;
+ int rc;
+
+ if (-1 == xenstore_read_fe_int(xendev, "feature-update", &fb->feature_update))
+ fb->feature_update = 0;
+ if (fb->feature_update)
+ xenstore_write_be_int(xendev, "request-update", 1);
+
+ if (-1 == xenstore_read_fe_int(xendev, "videoram", &videoram))
+ videoram = 0;
+
+ rc = common_bind(&fb->c);
+ if (0 != rc)
+ return rc;
+
+ fb_page = fb->c.page;
+ rc = xenfb_configure_fb(fb, videoram * 1024 * 1024U,
+ fb_page->width, fb_page->height, fb_page->depth,
+ fb_page->mem_length, 0, fb_page->line_length);
+ if (0 != rc)
+ return rc;
+
+ rc = xenfb_map_fb(fb);
+ if (0 != rc)
+ return rc;
+
+ graphic_console_init(fb->c.ds,
+ xenfb_update,
+ xenfb_invalidate,
+ NULL,
+ NULL,
+ fb);
+
+ xen_be_printf(xendev, 1, "feature-update=%d, videoram=%d\n",
+ fb->feature_update, videoram);
+
+ return 0;
+}
+
+static void fb_disconnect(struct XenDevice *xendev)
+{
+ struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev);
+
+ /* FIXME: Hmm, un-init gfx display? can qemu handle that? */
+ common_unbind(&fb->c);
+}
+
+static void fb_frontend_changed(struct XenDevice *xendev, const char *node)
+{
+ struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev);
+
+ /*
+ * Set state to Connected *again* once the frontend switched
+ * to connected. We must trigger the watch a second time to
+ * workaround a frontend bug.
+ */
+ if (0 == fb->bug_trigger && 0 == strcmp(node, "state") &&
+ xendev->fe_state == XenbusStateConnected &&
+ xendev->be_state == XenbusStateConnected) {
+ xen_be_set_state(xendev, XenbusStateConnected);
+ fb->bug_trigger = 1; /* only once */
+ }
+}
+
+static void fb_event(struct XenDevice *xendev)
+{
+ struct XenFB *xenfb = container_of(xendev, struct XenFB, c.xendev);
+
+ xenfb_handle_events(xenfb);
+ xen_be_send_notify(&xenfb->c.xendev);
+}
+
+/* -------------------------------------------------------------------- */
+
+struct XenDevOps xen_kbdmouse_ops = {
+ .size = sizeof(struct XenInput),
+ .init = input_init,
+ .connect = input_connect,
+ .disconnect = input_disconnect,
+ .event = input_event,
+};
+
+struct XenDevOps xen_framebuffer_ops = {
+ .size = sizeof(struct XenFB),
+ .init = fb_init,
+ .connect = fb_connect,
+ .disconnect = fb_disconnect,
+ .event = fb_event,
+ .frontend_changed = fb_frontend_changed,
+};
+
+static void xen_set_display_type(int domid, char *type, DisplayState *ds)
+{
+ struct XenDevice *xendev;
+ struct common *c;
+
+ xendev = xen_be_find_xendev(type, domid, 0);
+ if (!xendev)
+ return;
+ c = container_of(xendev, struct common, xendev);
+ c->ds = ds;
+ xen_be_printf(xendev, 1, "ds is %p\n", ds);
+ /* retry ->init() */
+ xen_be_check_state(xendev);
+}
+
+void xen_set_display(int domid, DisplayState *ds)
+{
+ xen_set_display_type(domid, "vkbd", ds);
+ xen_set_display_type(domid, "vfb", ds);
+}
diff --git a/hw/xen-machine.c b/hw/xen-machine.c
index de22cdb..8d7140a 100644
--- a/hw/xen-machine.c
+++ b/hw/xen-machine.c
@@ -99,6 +99,11 @@ static int xen_init_pv(DisplayState *ds)
/* xenbus backend drivers */
xen_be_register("console", &xen_console_ops);
+ xen_be_register("vkbd", &xen_kbdmouse_ops);
+ xen_be_register("vfb", &xen_framebuffer_ops);
+
+ /* setup framebuffer */
+ xen_set_display(xen_domid, ds);
return 0;
}
--
1.5.5.1
^ permalink raw reply related [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 4/7] xen: add framebuffer backend driver
2008-08-04 15:50 ` [Qemu-devel] [PATCH 4/7] xen: add framebuffer " Gerd Hoffmann
@ 2008-08-04 17:09 ` Blue Swirl
2008-08-04 18:20 ` Gerd Hoffmann
0 siblings, 1 reply; 92+ messages in thread
From: Blue Swirl @ 2008-08-04 17:09 UTC (permalink / raw)
To: qemu-devel; +Cc: xen-devel, Gerd Hoffmann
On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> +const unsigned char atkbd_set2_keycode[512] = {
Missing "static"?
> +const unsigned char atkbd_unxlate_table[128] = {
Ditto.
> + for (i = 0; i < 128; i++) {
> + scancode2linux[i] = atkbd_set2_keycode[atkbd_unxlate_table[i]];
> + scancode2linux[i | 0x80] =
> + atkbd_set2_keycode[atkbd_unxlate_table[i] | 0x80];
> + }
Can't these table be calculated so that this initial run-time lookup
is not needed?
The framebuffer does not use dirty memory detection. Is this
intentional? I understand that in Xen that's not possible.
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 4/7] xen: add framebuffer backend driver
2008-08-04 17:09 ` Blue Swirl
@ 2008-08-04 18:20 ` Gerd Hoffmann
0 siblings, 0 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 18:20 UTC (permalink / raw)
To: Blue Swirl; +Cc: xen-devel, qemu-devel
Blue Swirl wrote:
> On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> +const unsigned char atkbd_set2_keycode[512] = {
>
> Missing "static"?
Yep, I'll fix it up.
>> + for (i = 0; i < 128; i++) {
>> + scancode2linux[i] = atkbd_set2_keycode[atkbd_unxlate_table[i]];
>> + scancode2linux[i | 0x80] =
>> + atkbd_set2_keycode[atkbd_unxlate_table[i] | 0x80];
>> + }
>
> Can't these table be calculated so that this initial run-time lookup
> is not needed?
Should be doable, yes.
> The framebuffer does not use dirty memory detection. Is this
> intentional? I understand that in Xen that's not possible.
The guest frontend driver does that and sends the info to the backend
driver (XENFB_TYPE_UPDATE messages).
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-04 15:50 [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Gerd Hoffmann
` (3 preceding siblings ...)
2008-08-04 15:50 ` [Qemu-devel] [PATCH 4/7] xen: add framebuffer " Gerd Hoffmann
@ 2008-08-04 15:50 ` Gerd Hoffmann
2008-08-04 17:26 ` Blue Swirl
2008-08-04 15:50 ` [Qemu-devel] [PATCH 6/7] xen: add net " Gerd Hoffmann
` (2 subsequent siblings)
7 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 15:50 UTC (permalink / raw)
To: qemu-devel, xen-devel; +Cc: Gerd Hoffmann
This patch adds a block device backend driver to qemu. It is a pure
userspace implemention using the gntdev interface. It uses "qdisk" as
backend name in xenstore so it doesn't interfere with the other existing
backends (blkback aka "vbd" and tapdisk aka "tap").
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
Makefile.target | 2 +-
hw/xen-backend.h | 2 +
hw/xen-blkif.h | 103 ++++++++
hw/xen-disk.c | 677 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/xen-machine.c | 1 +
sysemu.h | 2 +-
vl.c | 4 +
7 files changed, 789 insertions(+), 2 deletions(-)
create mode 100644 hw/xen-blkif.h
create mode 100644 hw/xen-disk.c
diff --git a/Makefile.target b/Makefile.target
index 39db580..e668bd2 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -517,7 +517,7 @@ endif
# xen backend driver support
XEN_OBJS := xen-machine.o xen-backend.o
-XEN_OBJS += xen-console.o xen-framebuffer.o
+XEN_OBJS += xen-console.o xen-framebuffer.o xen-disk.o
ifeq ($(CONFIG_XEN), yes)
OBJS += $(XEN_OBJS)
LIBS += $(XEN_LIBS)
diff --git a/hw/xen-backend.h b/hw/xen-backend.h
index f7dd0c8..7489cbe 100644
--- a/hw/xen-backend.h
+++ b/hw/xen-backend.h
@@ -2,6 +2,7 @@
#define QEMU_XEN_BACKEND_H 1
#include "xen-common.h"
+#include "sysemu.h"
/* ------------------------------------------------------------- */
@@ -88,6 +89,7 @@ void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, ...
struct XenDevOps xen_console_ops; /* xen_console.c */
struct XenDevOps xen_kbdmouse_ops; /* xen_framebuffer.c */
struct XenDevOps xen_framebuffer_ops; /* xen_framebuffer.c */
+struct XenDevOps xen_blkdev_ops; /* xen_disk.c */
void xen_set_display(int domid, DisplayState *ds);
diff --git a/hw/xen-blkif.h b/hw/xen-blkif.h
new file mode 100644
index 0000000..254a5fd
--- /dev/null
+++ b/hw/xen-blkif.h
@@ -0,0 +1,103 @@
+#ifndef __XEN_BLKIF_H__
+#define __XEN_BLKIF_H__
+
+#include <xen/io/ring.h>
+#include <xen/io/blkif.h>
+#include <xen/io/protocols.h>
+
+/* Not a real protocol. Used to generate ring structs which contain
+ * the elements common to all protocols only. This way we get a
+ * compiler-checkable way to use common struct elements, so we can
+ * avoid using switch(protocol) in a number of places. */
+struct blkif_common_request {
+ char dummy;
+};
+struct blkif_common_response {
+ char dummy;
+};
+
+/* i386 protocol version */
+#pragma pack(push, 4)
+struct blkif_x86_32_request {
+ uint8_t operation; /* BLKIF_OP_??? */
+ uint8_t nr_segments; /* number of segments */
+ blkif_vdev_t handle; /* only for read/write requests */
+ uint64_t id; /* private guest value, echoed in resp */
+ blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */
+ struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+};
+struct blkif_x86_32_response {
+ uint64_t id; /* copied from request */
+ uint8_t operation; /* copied from request */
+ int16_t status; /* BLKIF_RSP_??? */
+};
+typedef struct blkif_x86_32_request blkif_x86_32_request_t;
+typedef struct blkif_x86_32_response blkif_x86_32_response_t;
+#pragma pack(pop)
+
+/* x86_64 protocol version */
+struct blkif_x86_64_request {
+ uint8_t operation; /* BLKIF_OP_??? */
+ uint8_t nr_segments; /* number of segments */
+ blkif_vdev_t handle; /* only for read/write requests */
+ uint64_t __attribute__((__aligned__(8))) id;
+ blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */
+ struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+};
+struct blkif_x86_64_response {
+ uint64_t __attribute__((__aligned__(8))) id;
+ uint8_t operation; /* copied from request */
+ int16_t status; /* BLKIF_RSP_??? */
+};
+typedef struct blkif_x86_64_request blkif_x86_64_request_t;
+typedef struct blkif_x86_64_response blkif_x86_64_response_t;
+
+DEFINE_RING_TYPES(blkif_common, struct blkif_common_request, struct blkif_common_response);
+DEFINE_RING_TYPES(blkif_x86_32, struct blkif_x86_32_request, struct blkif_x86_32_response);
+DEFINE_RING_TYPES(blkif_x86_64, struct blkif_x86_64_request, struct blkif_x86_64_response);
+
+union blkif_back_rings {
+ blkif_back_ring_t native;
+ blkif_common_back_ring_t common;
+ blkif_x86_32_back_ring_t x86_32;
+ blkif_x86_64_back_ring_t x86_64;
+};
+typedef union blkif_back_rings blkif_back_rings_t;
+
+enum blkif_protocol {
+ BLKIF_PROTOCOL_NATIVE = 1,
+ BLKIF_PROTOCOL_X86_32 = 2,
+ BLKIF_PROTOCOL_X86_64 = 3,
+};
+
+static void inline blkif_get_x86_32_req(blkif_request_t *dst, blkif_x86_32_request_t *src)
+{
+ int i, n = BLKIF_MAX_SEGMENTS_PER_REQUEST;
+
+ dst->operation = src->operation;
+ dst->nr_segments = src->nr_segments;
+ dst->handle = src->handle;
+ dst->id = src->id;
+ dst->sector_number = src->sector_number;
+ if (n > src->nr_segments)
+ n = src->nr_segments;
+ for (i = 0; i < n; i++)
+ dst->seg[i] = src->seg[i];
+}
+
+static void inline blkif_get_x86_64_req(blkif_request_t *dst, blkif_x86_64_request_t *src)
+{
+ int i, n = BLKIF_MAX_SEGMENTS_PER_REQUEST;
+
+ dst->operation = src->operation;
+ dst->nr_segments = src->nr_segments;
+ dst->handle = src->handle;
+ dst->id = src->id;
+ dst->sector_number = src->sector_number;
+ if (n > src->nr_segments)
+ n = src->nr_segments;
+ for (i = 0; i < n; i++)
+ dst->seg[i] = src->seg[i];
+}
+
+#endif /* __XEN_BLKIF_H__ */
diff --git a/hw/xen-disk.c b/hw/xen-disk.c
new file mode 100644
index 0000000..96fa9dc
--- /dev/null
+++ b/hw/xen-disk.c
@@ -0,0 +1,677 @@
+/*
+ * xen paravirt block device backend
+ *
+ * (c) Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+/*
+ * FIXME: the code is designed to handle multiple outstanding
+ * requests (using aio or using threads), which isn't used right
+ * now due to limitations of the qemu block driver interface.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <string.h>
+#include <unistd.h>
+#include <signal.h>
+#include <inttypes.h>
+#include <time.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <sys/uio.h>
+
+#include <xs.h>
+#include <xenctrl.h>
+#include <xen/io/xenbus.h>
+
+#include "hw.h"
+#include "block_int.h"
+#include "qemu-char.h"
+#include "xen-blkif.h"
+#include "xen-backend.h"
+
+/* ------------------------------------------------------------- */
+
+#define BLOCK_SIZE 512
+#define IOCB_COUNT (BLKIF_MAX_SEGMENTS_PER_REQUEST + 2)
+
+struct ioreq {
+ blkif_request_t req;
+ int16_t status;
+
+ /* parsed request */
+ off_t start, end;
+ struct iovec vec[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+ int vecs;
+ int presync;
+ int postsync;
+
+ /* grant mapping */
+ uint32_t domids[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+ uint32_t refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+ int prot;
+ void *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+ void *pages;
+
+ struct XenBlkDev *blkdev;
+ LIST_ENTRY(ioreq) list;
+};
+
+struct XenBlkDev {
+ struct XenDevice xendev; /* must be first */
+ char *params;
+ char *mode;
+ char *type;
+ char *dev;
+ char *fileproto;
+ char *filename;
+ int ring_ref;
+ void *sring;
+ int file;
+ int64_t file_blk;
+ int64_t file_size;
+ int protocol;
+ blkif_back_rings_t rings;
+ int more_work;
+ int cnt_map;
+
+ /* request lists */
+ LIST_HEAD(inflight_head, ioreq) inflight;
+ LIST_HEAD(finished_head, ioreq) finished;
+ LIST_HEAD(freelist_head, ioreq) freelist;
+ int requests;
+
+ /* qemu block driver */
+ BlockDriverState *bs;
+};
+
+static int syncwrite = 0;
+static int batch_maps = 0;
+static int max_requests = 32;
+
+/* ------------------------------------------------------------- */
+
+static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
+{
+ struct ioreq *ioreq = NULL;
+
+ if (LIST_EMPTY(&blkdev->freelist)) {
+ if (blkdev->requests >= max_requests)
+ goto out;
+ /* allocate new struct */
+ ioreq = qemu_mallocz(sizeof(*ioreq));
+ if (NULL == ioreq)
+ goto out;
+ ioreq->blkdev = blkdev;
+ blkdev->requests++;
+ } else {
+ /* get one from freelist */
+ ioreq = LIST_FIRST(&blkdev->freelist);
+ LIST_REMOVE(ioreq, list);
+ }
+ LIST_INSERT_HEAD(&blkdev->inflight, ioreq, list);
+
+out:
+ return ioreq;
+}
+
+static void ioreq_finish(struct ioreq *ioreq)
+{
+ struct XenBlkDev *blkdev = ioreq->blkdev;
+
+ LIST_REMOVE(ioreq, list);
+ LIST_INSERT_HEAD(&blkdev->finished, ioreq, list);
+}
+
+static void ioreq_release(struct ioreq *ioreq)
+{
+ struct XenBlkDev *blkdev = ioreq->blkdev;
+
+ LIST_REMOVE(ioreq, list);
+ memset(ioreq, 0, sizeof(*ioreq));
+ ioreq->blkdev = blkdev;
+ LIST_INSERT_HEAD(&blkdev->freelist, ioreq, list);
+}
+
+/*
+ * translate request into iovec + start offset + end offset
+ * do sanity checks along the way
+ */
+static int ioreq_parse(struct ioreq *ioreq)
+{
+ struct XenBlkDev *blkdev = ioreq->blkdev;
+ uintptr_t mem;
+ size_t len;
+ int i;
+
+ xen_be_printf(&blkdev->xendev, 3,
+ "op %d, nr %d, handle %d, id %" PRId64 ", sector %" PRId64 "\n",
+ ioreq->req.operation, ioreq->req.nr_segments,
+ ioreq->req.handle, ioreq->req.id, ioreq->req.sector_number);
+ switch (ioreq->req.operation) {
+ case BLKIF_OP_READ:
+ ioreq->prot = PROT_WRITE; /* to memory */
+ if (BLKIF_OP_READ != ioreq->req.operation && blkdev->mode[0] != 'w') {
+ xen_be_printf(&blkdev->xendev, 0, "error: write req for ro device\n");
+ goto err;
+ }
+ break;
+ case BLKIF_OP_WRITE_BARRIER:
+ if (!syncwrite)
+ ioreq->presync = ioreq->postsync = 1;
+ /* fall through */
+ case BLKIF_OP_WRITE:
+ ioreq->prot = PROT_READ; /* from memory */
+ if (syncwrite)
+ ioreq->postsync = 1;
+ break;
+ default:
+ xen_be_printf(&blkdev->xendev, 0, "error: unknown operation (%d)\n",
+ ioreq->req.operation);
+ goto err;
+ };
+
+ ioreq->start = ioreq->end = ioreq->req.sector_number * blkdev->file_blk;
+ for (i = 0; i < ioreq->req.nr_segments; i++) {
+ if (i == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
+ xen_be_printf(&blkdev->xendev, 0, "error: nr_segments too big\n");
+ goto err;
+ }
+ if (ioreq->req.seg[i].first_sect > ioreq->req.seg[i].last_sect) {
+ xen_be_printf(&blkdev->xendev, 0, "error: first > last sector\n");
+ goto err;
+ }
+ if (ioreq->req.seg[i].last_sect * BLOCK_SIZE >= XC_PAGE_SIZE) {
+ xen_be_printf(&blkdev->xendev, 0, "error: page crossing\n");
+ goto err;
+ }
+ len = (ioreq->req.seg[i].last_sect - ioreq->req.seg[i].first_sect + 1) * blkdev->file_blk;
+
+ ioreq->domids[i] = blkdev->xendev.dom;
+ ioreq->refs[i] = ioreq->req.seg[i].gref;
+ mem = ioreq->req.seg[i].first_sect * blkdev->file_blk;
+
+ ioreq->vec[i].iov_base = (void*)mem;
+ ioreq->vec[i].iov_len = len;
+ ioreq->end += len;
+ }
+ if (ioreq->end > blkdev->file_size) {
+ xen_be_printf(&blkdev->xendev, 0, "error: access beyond end of file\n");
+ goto err;
+ }
+ ioreq->vecs = i;
+ return 0;
+
+err:
+ ioreq->status = BLKIF_RSP_ERROR;
+ return -1;
+}
+
+static void ioreq_unmap(struct ioreq *ioreq)
+{
+ int gnt = ioreq->blkdev->xendev.gnttabdev;
+ int i;
+
+ if (batch_maps) {
+ if (!ioreq->pages)
+ return;
+ if (0 != xc_gnttab_munmap(gnt, ioreq->pages, ioreq->vecs))
+ xen_be_printf(&ioreq->blkdev->xendev, 0, "xc_gnttab_munmap failed: %s\n",
+ strerror(errno));
+ ioreq->blkdev->cnt_map -= ioreq->vecs;
+ ioreq->pages = NULL;
+ } else {
+ for (i = 0; i < ioreq->vecs; i++) {
+ if (!ioreq->page[i])
+ continue;
+ if (0 != xc_gnttab_munmap(gnt, ioreq->page[i], 1))
+ xen_be_printf(&ioreq->blkdev->xendev, 0, "xc_gnttab_munmap failed: %s\n",
+ strerror(errno));
+ ioreq->blkdev->cnt_map--;
+ ioreq->page[i] = NULL;
+ }
+ }
+}
+
+static int ioreq_map(struct ioreq *ioreq)
+{
+ int gnt = ioreq->blkdev->xendev.gnttabdev;
+ int i;
+
+ if (batch_maps) {
+ ioreq->pages = xc_gnttab_map_grant_refs
+ (gnt, ioreq->vecs, ioreq->domids, ioreq->refs, ioreq->prot);
+ if (NULL == ioreq->pages) {
+ xen_be_printf(&ioreq->blkdev->xendev, 0,
+ "can't map %d grant refs (%s, %d maps)\n",
+ ioreq->vecs, strerror(errno), ioreq->blkdev->cnt_map);
+ return -1;
+ }
+ for (i = 0; i < ioreq->vecs; i++)
+ ioreq->vec[i].iov_base = ioreq->pages + i * XC_PAGE_SIZE +
+ (uintptr_t)ioreq->vec[i].iov_base;
+ ioreq->blkdev->cnt_map += ioreq->vecs;
+ } else {
+ for (i = 0; i < ioreq->vecs; i++) {
+ ioreq->page[i] = xc_gnttab_map_grant_ref
+ (gnt, ioreq->domids[i], ioreq->refs[i], ioreq->prot);
+ if (NULL == ioreq->page[i]) {
+ xen_be_printf(&ioreq->blkdev->xendev, 0,
+ "can't map grant ref %d (%s, %d maps)\n",
+ ioreq->refs[i], strerror(errno), ioreq->blkdev->cnt_map);
+ ioreq_unmap(ioreq);
+ return -1;
+ }
+ ioreq->vec[i].iov_base = ioreq->page[i] + (uintptr_t)ioreq->vec[i].iov_base;
+ ioreq->blkdev->cnt_map++;
+ }
+ }
+ return 0;
+}
+
+static int ioreq_runio_qemu(struct ioreq *ioreq)
+{
+ struct XenBlkDev *blkdev = ioreq->blkdev;
+ int i, rc, len = 0;
+ off_t pos;
+
+ if (-1 == ioreq_map(ioreq))
+ goto err;
+ if (ioreq->presync)
+ bdrv_flush(blkdev->bs);
+
+ switch (ioreq->req.operation) {
+ case BLKIF_OP_READ:
+ pos = ioreq->start;
+ for (i = 0; i < ioreq->vecs; i++) {
+ rc = bdrv_read(blkdev->bs, pos / BLOCK_SIZE,
+ ioreq->vec[i].iov_base,
+ ioreq->vec[i].iov_len / BLOCK_SIZE);
+ if (rc != 0) {
+ xen_be_printf(&blkdev->xendev, 0, "rd I/O error (%p, len %zd)\n",
+ ioreq->vec[i].iov_base,
+ ioreq->vec[i].iov_len);
+ goto err;
+ }
+ len += ioreq->vec[i].iov_len;
+ pos += ioreq->vec[i].iov_len;
+ }
+ break;
+ case BLKIF_OP_WRITE:
+ case BLKIF_OP_WRITE_BARRIER:
+ pos = ioreq->start;
+ for (i = 0; i < ioreq->vecs; i++) {
+ rc = bdrv_write(blkdev->bs, pos / BLOCK_SIZE,
+ ioreq->vec[i].iov_base,
+ ioreq->vec[i].iov_len / BLOCK_SIZE);
+ if (rc != 0) {
+ xen_be_printf(&blkdev->xendev, 0, "wr I/O error (%p, len %zd)\n",
+ ioreq->vec[i].iov_base,
+ ioreq->vec[i].iov_len);
+ goto err;
+ }
+ len += ioreq->vec[i].iov_len;
+ pos += ioreq->vec[i].iov_len;
+ }
+ break;
+ default:
+ /* unknown operation (shouldn't happen -- parse catches this) */
+ goto err;
+ }
+
+ if (ioreq->postsync)
+ bdrv_flush(blkdev->bs);
+ ioreq->status = BLKIF_RSP_OKAY;
+
+ ioreq_unmap(ioreq);
+ return 0;
+
+err:
+ ioreq->status = BLKIF_RSP_ERROR;
+ return -1;
+}
+
+static int blk_send_response_one(struct ioreq *ioreq)
+{
+ struct XenBlkDev *blkdev = ioreq->blkdev;
+ int send_notify = 0;
+ int have_requests = 0;
+ blkif_response_t resp;
+ void *dst;
+
+ resp.id = ioreq->req.id;
+ resp.operation = ioreq->req.operation;
+ resp.status = ioreq->status;
+
+ /* Place on the response ring for the relevant domain. */
+ switch (blkdev->protocol) {
+ case BLKIF_PROTOCOL_NATIVE:
+ dst = RING_GET_RESPONSE(&blkdev->rings.native, blkdev->rings.native.rsp_prod_pvt);
+ break;
+ case BLKIF_PROTOCOL_X86_32:
+ dst = RING_GET_RESPONSE(&blkdev->rings.x86_32, blkdev->rings.x86_32.rsp_prod_pvt);
+ break;
+ case BLKIF_PROTOCOL_X86_64:
+ dst = RING_GET_RESPONSE(&blkdev->rings.x86_64, blkdev->rings.x86_64.rsp_prod_pvt);
+ break;
+ default:
+ dst = NULL;
+ }
+ memcpy(dst, &resp, sizeof(resp));
+ blkdev->rings.common.rsp_prod_pvt++;
+
+ RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blkdev->rings.common, send_notify);
+ if (blkdev->rings.common.rsp_prod_pvt == blkdev->rings.common.req_cons) {
+ /*
+ * Tail check for pending requests. Allows frontend to avoid
+ * notifications if requests are already in flight (lower
+ * overheads and promotes batching).
+ */
+ RING_FINAL_CHECK_FOR_REQUESTS(&blkdev->rings.common, have_requests);
+ } else if (RING_HAS_UNCONSUMED_REQUESTS(&blkdev->rings.common)) {
+ have_requests = 1;
+ }
+
+ if (have_requests)
+ blkdev->more_work++;
+ return send_notify;
+}
+
+/* walk finished list, send outstanding responses, free requests */
+static void blk_send_response_all(struct XenBlkDev *blkdev)
+{
+ struct ioreq *ioreq;
+ int send_notify = 0;
+
+ while (!LIST_EMPTY(&blkdev->finished)) {
+ ioreq = LIST_FIRST(&blkdev->finished);
+ send_notify += blk_send_response_one(ioreq);
+ ioreq_release(ioreq);
+ }
+ if (send_notify)
+ xen_be_send_notify(&blkdev->xendev);
+}
+
+static int blk_get_request(struct XenBlkDev *blkdev, struct ioreq *ioreq, RING_IDX rc)
+{
+ switch (blkdev->protocol) {
+ case BLKIF_PROTOCOL_NATIVE:
+ memcpy(&ioreq->req, RING_GET_REQUEST(&blkdev->rings.native, rc),
+ sizeof(ioreq->req));
+ break;
+ case BLKIF_PROTOCOL_X86_32:
+ blkif_get_x86_32_req(&ioreq->req, RING_GET_REQUEST(&blkdev->rings.x86_32, rc));
+ break;
+ case BLKIF_PROTOCOL_X86_64:
+ blkif_get_x86_64_req(&ioreq->req, RING_GET_REQUEST(&blkdev->rings.x86_64, rc));
+ break;
+ }
+ return 0;
+}
+
+static void blk_handle_requests(struct XenBlkDev *blkdev)
+{
+ RING_IDX rc, rp;
+ struct ioreq *ioreq;
+
+ do {
+ blkdev->more_work = 0;
+
+ rc = blkdev->rings.common.req_cons;
+ rp = blkdev->rings.common.sring->req_prod;
+ xen_rmb(); /* Ensure we see queued requests up to 'rp'. */
+
+ /* Limit #of requests we queue up for I/O so we ack requests
+ * faster if busy. Improves backend/frontend parallelism and
+ * reduces evchn signaling. */
+ if (rp > rc + (max_requests >> 2)) {
+ rp = rc + (max_requests >> 2);
+ blkdev->more_work++;
+ }
+
+ while ((rc != rp)) {
+ /* pull request from ring */
+ if (RING_REQUEST_CONS_OVERFLOW(&blkdev->rings.common, rc))
+ break;
+ ioreq = ioreq_start(blkdev);
+ if (NULL == ioreq) {
+ blkdev->more_work++;
+ break;
+ }
+ blk_get_request(blkdev, ioreq, rc);
+ blkdev->rings.common.req_cons = ++rc;
+
+ /* parse them */
+ if (0 != ioreq_parse(ioreq)) {
+ if (blk_send_response_one(ioreq))
+ xen_be_send_notify(&blkdev->xendev);
+ ioreq_release(ioreq);
+ continue;
+ }
+
+ /* run i/o in qemu mode */
+ ioreq_runio_qemu(ioreq);
+ ioreq_finish(ioreq);
+ }
+ blk_send_response_all(blkdev);
+
+ } while (blkdev->more_work);
+}
+
+/* ------------------------------------------------------------- */
+
+static void blk_alloc(struct XenDevice *xendev)
+{
+ struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
+
+ LIST_INIT(&blkdev->inflight);
+ LIST_INIT(&blkdev->finished);
+ LIST_INIT(&blkdev->freelist);
+}
+
+static int blk_init(struct XenDevice *xendev)
+{
+ struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
+ int mode, qflags, have_barriers, index, info = 0;
+ char *h;
+
+ /* read xenstore entries */
+ if (NULL == blkdev->params) {
+ blkdev->params = xenstore_read_be_str(&blkdev->xendev, "params");
+ if (NULL != (h = strchr(blkdev->params, ':'))) {
+ blkdev->fileproto = blkdev->params;
+ blkdev->filename = h+1;
+ *h = 0;
+ } else {
+ blkdev->fileproto = "<unset>";
+ blkdev->filename = blkdev->params;
+ }
+ }
+ if (NULL == blkdev->mode)
+ blkdev->mode = xenstore_read_be_str(&blkdev->xendev, "mode");
+ if (NULL == blkdev->type)
+ blkdev->type = xenstore_read_be_str(&blkdev->xendev, "type");
+ if (NULL == blkdev->dev)
+ blkdev->dev = xenstore_read_be_str(&blkdev->xendev, "dev");
+
+ /* do we have all we need? */
+ if (NULL == blkdev->params ||
+ NULL == blkdev->mode ||
+ NULL == blkdev->type ||
+ NULL == blkdev->dev)
+ return -1;
+
+ /* read-only ? */
+ if (0 == strcmp(blkdev->mode, "w")) {
+ mode = O_RDWR;
+ qflags = BDRV_O_RDWR;
+ } else {
+ mode = O_RDONLY;
+ qflags = BDRV_O_RDONLY;
+ info |= VDISK_READONLY | VDISK_REMOVABLE | VDISK_CDROM;
+ }
+
+ /* init qemu block driver */
+ index = (blkdev->xendev.dev - 202 * 256) / 16;
+ index = drive_get_index(IF_XEN, 0, index);
+ if (-1 == index) {
+ blkdev->bs = bdrv_new(blkdev->dev);
+ if (blkdev->bs) {
+ if (0 != bdrv_open2(blkdev->bs, blkdev->filename, qflags,
+ bdrv_find_format(blkdev->fileproto))) {
+ bdrv_delete(blkdev->bs);
+ blkdev->bs = NULL;
+ }
+ }
+ if (!blkdev->bs)
+ return -1;
+ } else {
+ blkdev->bs = drives_table[index].bdrv;
+ }
+ blkdev->file_blk = BLOCK_SIZE;
+ blkdev->file_size = bdrv_getlength(blkdev->bs);
+ if (blkdev->file_size < 0)
+ blkdev->file_size = 0;
+ have_barriers = blkdev->bs->drv && blkdev->bs->drv->bdrv_flush ? 1 : 0;
+
+ xen_be_printf(xendev, 1, "type \"%s\", fileproto \"%s\", filename \"%s\","
+ " size %" PRId64 " (%" PRId64 " MB)\n",
+ blkdev->type, blkdev->fileproto, blkdev->filename,
+ blkdev->file_size, blkdev->file_size >> 20);
+
+ /* fill info */
+ xenstore_write_be_int(&blkdev->xendev, "feature-barrier", have_barriers);
+ xenstore_write_be_int(&blkdev->xendev, "info", info);
+ xenstore_write_be_int(&blkdev->xendev, "sector-size", blkdev->file_blk);
+ xenstore_write_be_int(&blkdev->xendev, "sectors",
+ blkdev->file_size / blkdev->file_blk);
+ return 0;
+}
+
+static int blk_connect(struct XenDevice *xendev)
+{
+ struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
+
+ if (-1 == xenstore_read_fe_int(&blkdev->xendev, "ring-ref", &blkdev->ring_ref))
+ return -1;
+ if (-1 == xenstore_read_fe_int(&blkdev->xendev, "event-channel",
+ &blkdev->xendev.remote_port))
+ return -1;
+
+ blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
+ if (0 == strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_32))
+ blkdev->protocol = BLKIF_PROTOCOL_X86_32;
+ if (0 == strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_64))
+ blkdev->protocol = BLKIF_PROTOCOL_X86_64;
+
+ blkdev->sring = xc_gnttab_map_grant_ref(blkdev->xendev.gnttabdev,
+ blkdev->xendev.dom,
+ blkdev->ring_ref,
+ PROT_READ | PROT_WRITE);
+ if (!blkdev->sring)
+ return -1;
+ blkdev->cnt_map++;
+
+ switch (blkdev->protocol) {
+ case BLKIF_PROTOCOL_NATIVE:
+ {
+ blkif_sring_t *sring_native = blkdev->sring;
+ BACK_RING_INIT(&blkdev->rings.native, sring_native, XC_PAGE_SIZE);
+ break;
+ }
+ case BLKIF_PROTOCOL_X86_32:
+ {
+ blkif_x86_32_sring_t *sring_x86_32 = blkdev->sring;
+ BACK_RING_INIT(&blkdev->rings.x86_32, sring_x86_32, XC_PAGE_SIZE);
+ break;
+ }
+ case BLKIF_PROTOCOL_X86_64:
+ {
+ blkif_x86_64_sring_t *sring_x86_64 = blkdev->sring;
+ BACK_RING_INIT(&blkdev->rings.x86_64, sring_x86_64, XC_PAGE_SIZE);
+ break;
+ }
+ }
+
+ xen_be_bind_evtchn(&blkdev->xendev);
+
+ xen_be_printf(&blkdev->xendev, 1, "ok: proto %s, ring-ref %d, "
+ "remote port %d, local port %d\n",
+ blkdev->xendev.protocol, blkdev->ring_ref,
+ blkdev->xendev.remote_port, blkdev->xendev.local_port);
+ return 0;
+}
+
+static void blk_disconnect(struct XenDevice *xendev)
+{
+ struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
+
+ if (-1 != blkdev->file) {
+ close(blkdev->file);
+ blkdev->file = -1;
+ }
+ if (blkdev->bs) {
+ bdrv_close(blkdev->bs);
+ bdrv_delete(blkdev->bs);
+ blkdev->bs = NULL;
+ }
+ xen_be_unbind_evtchn(&blkdev->xendev);
+
+ if (blkdev->sring) {
+ xc_gnttab_munmap(blkdev->xendev.gnttabdev, blkdev->sring, 1);
+ blkdev->cnt_map--;
+ blkdev->sring = NULL;
+ }
+}
+
+static int blk_free(struct XenDevice *xendev)
+{
+ struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
+ struct ioreq *ioreq;
+
+ while (!LIST_EMPTY(&blkdev->freelist)) {
+ ioreq = LIST_FIRST(&blkdev->freelist);
+ LIST_REMOVE(ioreq, list);
+ qemu_free(ioreq);
+ }
+
+ qemu_free(blkdev->params);
+ qemu_free(blkdev->mode);
+ return 0;
+}
+
+static void blk_event(struct XenDevice *xendev)
+{
+ struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
+ blk_handle_requests(blkdev);
+}
+
+struct XenDevOps xen_blkdev_ops = {
+ .size = sizeof(struct XenBlkDev),
+ .flags = DEVOPS_FLAG_NEED_GNTDEV,
+ .alloc = blk_alloc,
+ .init = blk_init,
+ .connect = blk_connect,
+ .disconnect = blk_disconnect,
+ .event = blk_event,
+ .free = blk_free,
+};
diff --git a/hw/xen-machine.c b/hw/xen-machine.c
index 8d7140a..a1289c1 100644
--- a/hw/xen-machine.c
+++ b/hw/xen-machine.c
@@ -101,6 +101,7 @@ static int xen_init_pv(DisplayState *ds)
xen_be_register("console", &xen_console_ops);
xen_be_register("vkbd", &xen_kbdmouse_ops);
xen_be_register("vfb", &xen_framebuffer_ops);
+ xen_be_register("qdisk", &xen_blkdev_ops);
/* setup framebuffer */
xen_set_display(xen_domid, ds);
diff --git a/sysemu.h b/sysemu.h
index b12fae0..49e75b1 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -113,7 +113,7 @@ extern unsigned int nb_prom_envs;
#endif
typedef enum {
- IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD
+ IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_XEN
} BlockInterfaceType;
typedef struct DriveInfo {
diff --git a/vl.c b/vl.c
index c3f2185..a80a46e 100644
--- a/vl.c
+++ b/vl.c
@@ -5484,6 +5484,9 @@ static int drive_init(struct drive_opt *arg, int snapshot,
} else if (!strcmp(buf, "sd")) {
type = IF_SD;
max_devs = 0;
+ } else if (!strcmp(buf, "xen")) {
+ type = IF_XEN;
+ max_devs = 0;
} else {
fprintf(stderr, "qemu: '%s' unsupported bus type '%s'\n", str, buf);
return -1;
@@ -5669,6 +5672,7 @@ static int drive_init(struct drive_opt *arg, int snapshot,
switch(type) {
case IF_IDE:
case IF_SCSI:
+ case IF_XEN:
switch(media) {
case MEDIA_DISK:
if (cyls != 0) {
--
1.5.5.1
^ permalink raw reply related [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-04 15:50 ` [Qemu-devel] [PATCH 5/7] xen: add block device " Gerd Hoffmann
@ 2008-08-04 17:26 ` Blue Swirl
2008-08-04 17:37 ` Samuel Thibault
` (2 more replies)
0 siblings, 3 replies; 92+ messages in thread
From: Blue Swirl @ 2008-08-04 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: xen-devel, Gerd Hoffmann
On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> +/* i386 protocol version */
> +#pragma pack(push, 4)
What's wrong with __attribute__(__aligned__)?
> + * FIXME: the code is designed to handle multiple outstanding
> + * requests (using aio or using threads), which isn't used right
> + * now due to limitations of the qemu block driver interface.
Yes, what happened to vectored DMA patches?
> + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blkdev->rings.common, send_notify);
EUGLY_LONG_NAME_WITH_ALL_CAPS
> + mode = O_RDONLY;
> + qflags = BDRV_O_RDONLY;
> + info |= VDISK_READONLY | VDISK_REMOVABLE | VDISK_CDROM;
Does this mean that all read-only disks are assumed to be CDROMs or
that all removable disks are read-only?
Why IF_XEN?
Why different protocols for i386 and x86_64? Would I need to add
Sparc32 and Sparc64 versions some day?
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-04 17:26 ` Blue Swirl
@ 2008-08-04 17:37 ` Samuel Thibault
2008-08-04 17:46 ` Anthony Liguori
2008-08-04 19:50 ` Gerd Hoffmann
2 siblings, 0 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-04 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: xen-devel, Gerd Hoffmann
Blue Swirl, le Mon 04 Aug 2008 20:26:42 +0300, a écrit :
> > + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blkdev->rings.common, send_notify);
>
> EUGLY_LONG_NAME_WITH_ALL_CAPS
That's unfortunately the xen API.
> Why different protocols for i386 and x86_64?
Because the xen block shared structures have been defined at API level,
not ABI, so they differ.
> Would I need to add Sparc32 and Sparc64 versions some day?
Should there be a Xen port to these archs, yes.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-04 17:26 ` Blue Swirl
2008-08-04 17:37 ` Samuel Thibault
@ 2008-08-04 17:46 ` Anthony Liguori
2008-08-04 19:50 ` Gerd Hoffmann
2 siblings, 0 replies; 92+ messages in thread
From: Anthony Liguori @ 2008-08-04 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: xen-devel, Gerd Hoffmann
Blue Swirl wrote:
> On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>> +/* i386 protocol version */
>> +#pragma pack(push, 4)
>>
>
> What's wrong with __attribute__(__aligned__)?
>
>
>> + * FIXME: the code is designed to handle multiple outstanding
>> + * requests (using aio or using threads), which isn't used right
>> + * now due to limitations of the qemu block driver interface.
>>
>
> Yes, what happened to vectored DMA patches?
>
Believe it or not, they were hurting performance in KVM. Passing a
vector to the block layer will force the IO operation to be split up
into multiple requests that are all executed in serial (since QEMU only
supports 1 outstanding request at a time). Copying the vectored IO to a
linear buffer increased performance significantly because it avoided
this serialization.
The block layer needs some refactoring to support proper vectored AIO
operations before a zero-copy API will make sense from a performance
perspective.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-04 17:26 ` Blue Swirl
2008-08-04 17:37 ` Samuel Thibault
2008-08-04 17:46 ` Anthony Liguori
@ 2008-08-04 19:50 ` Gerd Hoffmann
2008-08-04 20:04 ` Paul Brook
` (2 more replies)
2 siblings, 3 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 19:50 UTC (permalink / raw)
To: Blue Swirl; +Cc: xen-devel, qemu-devel
Blue Swirl wrote:
> On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> +/* i386 protocol version */
>> +#pragma pack(push, 4)
>
> What's wrong with __attribute__(__aligned__)?
That one is tricky because we must be binary compatible with xen. And
the block driver protocol has a small design flaw: It has a 64bit value
which is not aligned on a 64bit border. This leads to different struct
layouts on i386 and x86_64 due to different alignment rules. Unless you
force them into something else, like we do in that header file to deal
with it. Which is needed to run 32bit guests on 64bit hosts.
>> + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blkdev->rings.common, send_notify);
>
> EUGLY_LONG_NAME_WITH_ALL_CAPS
Xen API, no way around that, sorry.
>> + mode = O_RDONLY;
>> + qflags = BDRV_O_RDONLY;
>> + info |= VDISK_READONLY | VDISK_REMOVABLE | VDISK_CDROM;
>
> Does this mean that all read-only disks are assumed to be CDROMs or
> that all removable disks are read-only?
I'll double-check, but I think there is no media=[disk|cdrom] indicator
in xenstore. So that was the best I could come up with ...
> Why IF_XEN?
I've seen you've noticed meanwhile in patch #7 ;)
> Why different protocols for i386 and x86_64?
See above.
> Would I need to add
> Sparc32 and Sparc64 versions some day?
Not sure whenever the same 32/64bit ABI issue exists on sparc. If so,
the code handling this on x86 should work for sparc too.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-04 19:50 ` Gerd Hoffmann
@ 2008-08-04 20:04 ` Paul Brook
2008-08-05 7:18 ` Gerd Hoffmann
2008-08-04 20:58 ` Blue Swirl
2008-08-04 21:34 ` [Xen-devel] " Samuel Thibault
2 siblings, 1 reply; 92+ messages in thread
From: Paul Brook @ 2008-08-04 20:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Blue Swirl, xen-devel, Gerd Hoffmann
On Monday 04 August 2008, Gerd Hoffmann wrote:
> Blue Swirl wrote:
> > On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >> +/* i386 protocol version */
> >> +#pragma pack(push, 4)
> >
> > What's wrong with __attribute__(__aligned__)?
>
> That one is tricky because we must be binary compatible with xen. And
> the block driver protocol has a small design flaw: It has a 64bit value
> which is not aligned on a 64bit border. This leads to different struct
> layouts on i386 and x86_64 due to different alignment rules. Unless you
> force them into something else, like we do in that header file to deal
> with it. Which is needed to run 32bit guests on 64bit hosts.
qemu should be host independent. You need to use packed structures with
appropriate padding.
Paul
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-04 20:04 ` Paul Brook
@ 2008-08-05 7:18 ` Gerd Hoffmann
0 siblings, 0 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-05 7:18 UTC (permalink / raw)
To: Paul Brook; +Cc: Blue Swirl, xen-devel, qemu-devel
Paul Brook wrote:
> On Monday 04 August 2008, Gerd Hoffmann wrote:
>> Blue Swirl wrote:
>>> On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>>> +/* i386 protocol version */
>>>> +#pragma pack(push, 4)
>>> What's wrong with __attribute__(__aligned__)?
>> That one is tricky because we must be binary compatible with xen. And
>> the block driver protocol has a small design flaw: It has a 64bit value
>> which is not aligned on a 64bit border. This leads to different struct
>> layouts on i386 and x86_64 due to different alignment rules. Unless you
>> force them into something else, like we do in that header file to deal
>> with it. Which is needed to run 32bit guests on 64bit hosts.
>
> qemu should be host independent. You need to use packed structures with
> appropriate padding.
I think the current code is correct. Ok, the verbose version. The
struct in question looks like this:
struct blkif_request {
uint8_t operation;
uint8_t nr_segments;
blkif_vdev_t handle;
uint64_t id;
[ ... ]
};
The critical element is id. blkif_vdev_t is uint16_t, which means id is
32bit aligned but not 64bit aligned.
On i386 64bit values get 32bit alignments by default.
On x86_64 64bit values get 64bit alignments by default.
Thus x86_64 has a 32bit padding hole just before id. i386 hasn't.
So we define a i386 and a x86_64 version of the struct. The i386
version is wrapped into a pragma like this ...
#pragma pack(push, 4)
struct blkif_x86_32_request {
[ ... ]
}
#pragma pack(pop)
... to enforce i386 alignment rules everythere.
The x86_64 version looks like this ...
struct blkif_x86_64_request {
[ ... ]
uint64_t __attribute__((__aligned__(8))) id;
[ ... ]
};
... to make sure the id element is 64bit aligned everythere.
That gives us the correct struct layouts for both i386 and x86_64 builds.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-04 19:50 ` Gerd Hoffmann
2008-08-04 20:04 ` Paul Brook
@ 2008-08-04 20:58 ` Blue Swirl
2008-08-05 7:01 ` Gerd Hoffmann
2008-08-04 21:34 ` [Xen-devel] " Samuel Thibault
2 siblings, 1 reply; 92+ messages in thread
From: Blue Swirl @ 2008-08-04 20:58 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, qemu-devel
On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Blue Swirl wrote:
> > On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >> +/* i386 protocol version */
> >> +#pragma pack(push, 4)
> >
> > What's wrong with __attribute__(__aligned__)?
>
>
> That one is tricky because we must be binary compatible with xen. And
> the block driver protocol has a small design flaw: It has a 64bit value
> which is not aligned on a 64bit border. This leads to different struct
> layouts on i386 and x86_64 due to different alignment rules. Unless you
> force them into something else, like we do in that header file to deal
> with it. Which is needed to run 32bit guests on 64bit hosts.
Isn't this internal Xen protocol, so hopefully next version of Xen
could use more efficient structures?
> > Would I need to add
> > Sparc32 and Sparc64 versions some day?
>
>
> Not sure whenever the same 32/64bit ABI issue exists on sparc. If so,
> the code handling this on x86 should work for sparc too.
Well, I'd rather define a new, more architecturally neutral structure.
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-04 20:58 ` Blue Swirl
@ 2008-08-05 7:01 ` Gerd Hoffmann
0 siblings, 0 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-05 7:01 UTC (permalink / raw)
To: Blue Swirl; +Cc: xen-devel, qemu-devel
Blue Swirl wrote:
> On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> Blue Swirl wrote:
>> > On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> >> +/* i386 protocol version */
>> >> +#pragma pack(push, 4)
>> >
>> > What's wrong with __attribute__(__aligned__)?
>>
>>
>> That one is tricky because we must be binary compatible with xen. And
>> the block driver protocol has a small design flaw: It has a 64bit value
>> which is not aligned on a 64bit border. This leads to different struct
>> layouts on i386 and x86_64 due to different alignment rules. Unless you
>> force them into something else, like we do in that header file to deal
>> with it. Which is needed to run 32bit guests on 64bit hosts.
>
> Isn't this internal Xen protocol, so hopefully next version of Xen
> could use more efficient structures?
No. It is part of the guest <=> host ABI. So changing that would break
existing guest kernels. Otherwise we wouldn't hop through these loops
in the first place.
In case the ABI is changed anyway at some point in the future (like it
was done from xen 2.x -> 3.0) this can be cleaned up of course. I doubt
that will happen though.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-04 19:50 ` Gerd Hoffmann
2008-08-04 20:04 ` Paul Brook
2008-08-04 20:58 ` Blue Swirl
@ 2008-08-04 21:34 ` Samuel Thibault
2008-08-05 6:52 ` Gerd Hoffmann
2 siblings, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-04 21:34 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Blue Swirl, xen-devel, qemu-devel
Gerd Hoffmann, le Mon 04 Aug 2008 21:50:50 +0200, a écrit :
> >> + mode = O_RDONLY;
> >> + qflags = BDRV_O_RDONLY;
> >> + info |= VDISK_READONLY | VDISK_REMOVABLE | VDISK_CDROM;
> >
> > Does this mean that all read-only disks are assumed to be CDROMs or
> > that all removable disks are read-only?
>
> I'll double-check, but I think there is no media=[disk|cdrom] indicator
> in xenstore.
There is: the info node, which is an OR of the
VDISK_CDROM/REMOVABLE/READONLY flags.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-04 21:34 ` [Xen-devel] " Samuel Thibault
@ 2008-08-05 6:52 ` Gerd Hoffmann
2008-08-05 10:47 ` Samuel Thibault
0 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-05 6:52 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Blue Swirl, xen-devel, qemu-devel
Samuel Thibault wrote:
> Gerd Hoffmann, le Mon 04 Aug 2008 21:50:50 +0200, a écrit :
>>>> + mode = O_RDONLY;
>>>> + qflags = BDRV_O_RDONLY;
>>>> + info |= VDISK_READONLY | VDISK_REMOVABLE | VDISK_CDROM;
>>> Does this mean that all read-only disks are assumed to be CDROMs or
>>> that all removable disks are read-only?
>> I'll double-check, but I think there is no media=[disk|cdrom] indicator
>> in xenstore.
>
> There is: the info node, which is an OR of the
> VDISK_CDROM/REMOVABLE/READONLY flags.
Who sets that node? Backend driver or xend?
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-05 6:52 ` Gerd Hoffmann
@ 2008-08-05 10:47 ` Samuel Thibault
2008-08-05 11:07 ` Gerd Hoffmann
0 siblings, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-05 10:47 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Blue Swirl, xen-devel, qemu-devel
Gerd Hoffmann, le Tue 05 Aug 2008 08:52:26 +0200, a écrit :
> > There is: the info node, which is an OR of the
> > VDISK_CDROM/REMOVABLE/READONLY flags.
>
> Who sets that node? Backend driver or xend?
backend, see linux/driver/xen/blkback/xenbus.c
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-05 10:47 ` Samuel Thibault
@ 2008-08-05 11:07 ` Gerd Hoffmann
2008-08-05 11:36 ` Samuel Thibault
0 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-05 11:07 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Blue Swirl, xen-devel, qemu-devel
Samuel Thibault wrote:
> Gerd Hoffmann, le Tue 05 Aug 2008 08:52:26 +0200, a écrit :
>>> There is: the info node, which is an OR of the
>>> VDISK_CDROM/REMOVABLE/READONLY flags.
>> Who sets that node? Backend driver or xend?
>
> backend, see linux/driver/xen/blkback/xenbus.c
Hmm, ok.
mode="r" (in backend directory) means readonly, ok.
device-type="cdrom" (backend dir too) means cdrom, ok.
fixed up these.
removable can't be indicated to the backend via xenstore?
ignoring that for now.
btw: Looks like blktap doesn't set any VDISK_* flags?
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 5/7] xen: add block device backend driver.
2008-08-05 11:07 ` Gerd Hoffmann
@ 2008-08-05 11:36 ` Samuel Thibault
0 siblings, 0 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-05 11:36 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Blue Swirl, xen-devel, qemu-devel
Gerd Hoffmann, le Tue 05 Aug 2008 13:07:52 +0200, a écrit :
> Samuel Thibault wrote:
> > Gerd Hoffmann, le Tue 05 Aug 2008 08:52:26 +0200, a écrit :
> >>> There is: the info node, which is an OR of the
> >>> VDISK_CDROM/REMOVABLE/READONLY flags.
> >> Who sets that node? Backend driver or xend?
> >
> > backend, see linux/driver/xen/blkback/xenbus.c
>
> Hmm, ok.
>
> mode="r" (in backend directory) means readonly, ok.
> device-type="cdrom" (backend dir too) means cdrom, ok.
These are set by xend, i.e. when they come from the config, indeed.
> removable can't be indicated to the backend via xenstore?
Doesn't look like it is possible. Why would you want to do that?
> btw: Looks like blktap doesn't set any VDISK_* flags?
I guess that's because blktap doesn't have the information. The "info"
node is information about the underlying source of data, detected by
the block backend, while "mode" and "device-type" are configured by the
user (and should override the "info" information I guess).
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* [Qemu-devel] [PATCH 6/7] xen: add net backend driver.
2008-08-04 15:50 [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Gerd Hoffmann
` (4 preceding siblings ...)
2008-08-04 15:50 ` [Qemu-devel] [PATCH 5/7] xen: add block device " Gerd Hoffmann
@ 2008-08-04 15:50 ` Gerd Hoffmann
2008-08-04 15:50 ` [Qemu-devel] [PATCH 7/7] xen: blk & nic configuration via cmd line Gerd Hoffmann
2008-08-04 17:42 ` [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Anthony Liguori
7 siblings, 0 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 15:50 UTC (permalink / raw)
To: qemu-devel, xen-devel; +Cc: Gerd Hoffmann
This patch adds a network interface backend driver to qemu. It is a pure
userspace implemention using the gntdev interface. It uses "qnet" as
backend name in xenstore so it doesn't interfere with the netback
backend (aka "vnif").
The network backend is hooked into the corrosponding qemu vlan, i.e.
vif 0 is hooked into vlan 0. To make the packages actually arrive
somewhere you additionally have to link the vlan to the outside world
using the usual qemu command line options such as "-net tap,...".
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
Makefile.target | 2 +-
hw/xen-backend.h | 1 +
hw/xen-machine.c | 1 +
hw/xen-nic.c | 380 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 383 insertions(+), 1 deletions(-)
create mode 100644 hw/xen-nic.c
diff --git a/Makefile.target b/Makefile.target
index e668bd2..85f938c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -517,7 +517,7 @@ endif
# xen backend driver support
XEN_OBJS := xen-machine.o xen-backend.o
-XEN_OBJS += xen-console.o xen-framebuffer.o xen-disk.o
+XEN_OBJS += xen-console.o xen-framebuffer.o xen-disk.o xen-nic.o
ifeq ($(CONFIG_XEN), yes)
OBJS += $(XEN_OBJS)
LIBS += $(XEN_LIBS)
diff --git a/hw/xen-backend.h b/hw/xen-backend.h
index 7489cbe..b484752 100644
--- a/hw/xen-backend.h
+++ b/hw/xen-backend.h
@@ -90,6 +90,7 @@ struct XenDevOps xen_console_ops; /* xen_console.c */
struct XenDevOps xen_kbdmouse_ops; /* xen_framebuffer.c */
struct XenDevOps xen_framebuffer_ops; /* xen_framebuffer.c */
struct XenDevOps xen_blkdev_ops; /* xen_disk.c */
+struct XenDevOps xen_netdev_ops; /* xen_nic.c */
void xen_set_display(int domid, DisplayState *ds);
diff --git a/hw/xen-machine.c b/hw/xen-machine.c
index a1289c1..c8fb256 100644
--- a/hw/xen-machine.c
+++ b/hw/xen-machine.c
@@ -102,6 +102,7 @@ static int xen_init_pv(DisplayState *ds)
xen_be_register("vkbd", &xen_kbdmouse_ops);
xen_be_register("vfb", &xen_framebuffer_ops);
xen_be_register("qdisk", &xen_blkdev_ops);
+ xen_be_register("qnic", &xen_netdev_ops);
/* setup framebuffer */
xen_set_display(xen_domid, ds);
diff --git a/hw/xen-nic.c b/hw/xen-nic.c
new file mode 100644
index 0000000..b533a60
--- /dev/null
+++ b/hw/xen-nic.c
@@ -0,0 +1,380 @@
+/*
+ * xen paravirt network card backend
+ *
+ * (c) Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <string.h>
+#include <unistd.h>
+#include <signal.h>
+#include <inttypes.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <pthread.h>
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <sys/wait.h>
+#include <linux/if.h>
+#include <linux/if_tun.h>
+
+#include <xs.h>
+#include <xenctrl.h>
+#include <xen/io/xenbus.h>
+#include <xen/io/netif.h>
+
+#include "hw.h"
+#include "net.h"
+#include "qemu-char.h"
+#include "xen-backend.h"
+
+/* ------------------------------------------------------------- */
+
+struct XenNetDev {
+ struct XenDevice xendev; /* must be first */
+ char *mac;
+ int tx_work;
+ int tx_ring_ref;
+ int rx_ring_ref;
+ struct netif_tx_sring *txs;
+ struct netif_rx_sring *rxs;
+ netif_tx_back_ring_t tx_ring;
+ netif_rx_back_ring_t rx_ring;
+ VLANClientState *vs;
+};
+
+/* ------------------------------------------------------------- */
+
+static void net_tx_response(struct XenNetDev *netdev, netif_tx_request_t *txp, int8_t st)
+{
+ RING_IDX i = netdev->tx_ring.rsp_prod_pvt;
+ netif_tx_response_t *resp;
+ int notify;
+
+ resp = RING_GET_RESPONSE(&netdev->tx_ring, i);
+ resp->id = txp->id;
+ resp->status = st;
+
+#if 0
+ if (txp->flags & NETTXF_extra_info)
+ RING_GET_RESPONSE(&netdev->tx_ring, ++i)->status = NETIF_RSP_NULL;
+#endif
+
+ netdev->tx_ring.rsp_prod_pvt = ++i;
+ RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&netdev->tx_ring, notify);
+ if (notify)
+ xen_be_send_notify(&netdev->xendev);
+
+ if (i == netdev->tx_ring.req_cons) {
+ int more_to_do;
+ RING_FINAL_CHECK_FOR_REQUESTS(&netdev->tx_ring, more_to_do);
+ if (more_to_do)
+ netdev->tx_work++;
+ }
+}
+
+static void net_tx_error(struct XenNetDev *netdev, netif_tx_request_t *txp, RING_IDX end)
+{
+#if 0
+ /*
+ * Hmm, why netback fails everything in the ring?
+ * Should we do that even when not supporting SG and TSO?
+ */
+ RING_IDX cons = netdev->tx_ring.req_cons;
+
+ do {
+ make_tx_response(netif, txp, NETIF_RSP_ERROR);
+ if (cons >= end)
+ break;
+ txp = RING_GET_REQUEST(&netdev->tx_ring, cons++);
+ } while (1);
+ netdev->tx_ring.req_cons = cons;
+ netif_schedule_work(netif);
+ netif_put(netif);
+#else
+ net_tx_response(netdev, txp, NETIF_RSP_ERROR);
+#endif
+}
+
+static void net_tx_packets(struct XenNetDev *netdev)
+{
+ netif_tx_request_t txreq;
+ RING_IDX rc, rp;
+ void *page;
+
+ for (;;) {
+ rc = netdev->tx_ring.req_cons;
+ rp = netdev->tx_ring.sring->req_prod;
+ xen_rmb(); /* Ensure we see queued requests up to 'rp'. */
+
+ while ((rc != rp)) {
+ if (RING_REQUEST_CONS_OVERFLOW(&netdev->tx_ring, rc))
+ break;
+ memcpy(&txreq, RING_GET_REQUEST(&netdev->tx_ring, rc), sizeof(txreq));
+ netdev->tx_ring.req_cons = ++rc;
+
+#if 1
+ /* should not happen in theory, we don't announce the *
+ * feature-{sg,gso,whatelse} flags in xenstore (yet?) */
+ if (txreq.flags & NETTXF_extra_info) {
+ xen_be_printf(&netdev->xendev, 0, "FIXME: extra info flag\n");
+ net_tx_error(netdev, &txreq, rc);
+ continue;
+ }
+ if (txreq.flags & NETTXF_more_data) {
+ xen_be_printf(&netdev->xendev, 0, "FIXME: more data flag\n");
+ net_tx_error(netdev, &txreq, rc);
+ continue;
+ }
+#endif
+
+ if (txreq.size < 14) {
+ xen_be_printf(&netdev->xendev, 0, "bad packet size: %d\n", txreq.size);
+ net_tx_error(netdev, &txreq, rc);
+ continue;
+ }
+
+ if ((txreq.offset + txreq.size) > XC_PAGE_SIZE) {
+ xen_be_printf(&netdev->xendev, 0, "error: page crossing\n");
+ net_tx_error(netdev, &txreq, rc);
+ continue;
+ }
+
+ xen_be_printf(&netdev->xendev, 3, "tx packet ref %d, off %d, len %d, flags 0x%x%s%s%s%s\n",
+ txreq.gref, txreq.offset, txreq.size, txreq.flags,
+ (txreq.flags & NETTXF_csum_blank) ? " csum_blank" : "",
+ (txreq.flags & NETTXF_data_validated) ? " data_validated" : "",
+ (txreq.flags & NETTXF_more_data) ? " more_data" : "",
+ (txreq.flags & NETTXF_extra_info) ? " extra_info" : "");
+
+ page = xc_gnttab_map_grant_ref(netdev->xendev.gnttabdev,
+ netdev->xendev.dom,
+ txreq.gref, PROT_READ);
+ if (NULL == page) {
+ xen_be_printf(&netdev->xendev, 0, "error: gref dereference failed\n");
+ net_tx_error(netdev, &txreq, rc);
+ continue;
+ }
+ if (txreq.flags & NETTXF_csum_blank)
+ net_checksum_calculate(page + txreq.offset, txreq.size);
+ qemu_send_packet(netdev->vs, page + txreq.offset, txreq.size);
+ xc_gnttab_munmap(netdev->xendev.gnttabdev, page, 1);
+ net_tx_response(netdev, &txreq, NETIF_RSP_OKAY);
+ }
+ if (!netdev->tx_work)
+ break;
+ netdev->tx_work = 0;
+ }
+}
+
+/* ------------------------------------------------------------- */
+
+static void net_rx_response(struct XenNetDev *netdev,
+ netif_rx_request_t *req, int8_t st,
+ uint16_t offset, uint16_t size,
+ uint16_t flags)
+{
+ RING_IDX i = netdev->rx_ring.rsp_prod_pvt;
+ netif_rx_response_t *resp;
+ int notify;
+
+ resp = RING_GET_RESPONSE(&netdev->rx_ring, i);
+ resp->offset = offset;
+ resp->flags = flags;
+ resp->id = req->id;
+ resp->status = (int16_t)size;
+ if (st < 0)
+ resp->status = (int16_t)st;
+
+ xen_be_printf(&netdev->xendev, 3, "rx response: idx %d, status %d, flags 0x%x\n",
+ i, resp->status, resp->flags);
+
+ netdev->rx_ring.rsp_prod_pvt = ++i;
+ RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&netdev->rx_ring, notify);
+ if (notify)
+ xen_be_send_notify(&netdev->xendev);
+}
+
+#define NET_IP_ALIGN 2
+
+static int net_rx_ok(void *opaque)
+{
+ struct XenNetDev *netdev = opaque;
+ RING_IDX rc, rp;
+
+ if (netdev->xendev.be_state != XenbusStateConnected)
+ return 0;
+
+ rc = netdev->rx_ring.req_cons;
+ rp = netdev->rx_ring.sring->req_prod;
+ xen_rmb();
+
+ if (rc == rp || RING_REQUEST_CONS_OVERFLOW(&netdev->rx_ring, rc)) {
+ xen_be_printf(&netdev->xendev, 2, "%s: no rx buffers (%d/%d)\n",
+ __FUNCTION__, rc, rp);
+ return 0;
+ }
+ return 1;
+}
+
+static void net_rx_packet(void *opaque, const uint8_t *buf, int size)
+{
+ struct XenNetDev *netdev = opaque;
+ netif_rx_request_t rxreq;
+ RING_IDX rc, rp;
+ void *page;
+
+ if (netdev->xendev.be_state != XenbusStateConnected)
+ return;
+
+ rc = netdev->rx_ring.req_cons;
+ rp = netdev->rx_ring.sring->req_prod;
+ xen_rmb(); /* Ensure we see queued requests up to 'rp'. */
+
+ if (rc == rp || RING_REQUEST_CONS_OVERFLOW(&netdev->rx_ring, rc)) {
+ xen_be_printf(&netdev->xendev, 2, "no buffer, drop packet\n");
+ return;
+ }
+ if (size > XC_PAGE_SIZE - NET_IP_ALIGN) {
+ xen_be_printf(&netdev->xendev, 0, "packet too big (%d > %ld)",
+ size, XC_PAGE_SIZE - NET_IP_ALIGN);
+ return;
+ }
+
+ 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,
+ netdev->xendev.dom,
+ rxreq.gref, PROT_WRITE);
+ if (NULL == page) {
+ xen_be_printf(&netdev->xendev, 0, "error: gref dereference failed\n");
+ net_rx_response(netdev, &rxreq, NETIF_RSP_ERROR, 0, 0, 0);
+ return;
+ }
+ memcpy(page + NET_IP_ALIGN, buf, size);
+ xc_gnttab_munmap(netdev->xendev.gnttabdev, page, 1);
+ net_rx_response(netdev, &rxreq, NETIF_RSP_OKAY, NET_IP_ALIGN, size, 0);
+}
+
+/* ------------------------------------------------------------- */
+
+static int net_init(struct XenDevice *xendev)
+{
+ struct XenNetDev *netdev = container_of(xendev, struct XenNetDev, xendev);
+ VLANState *vlan;
+
+ /* read xenstore entries */
+ if (NULL == netdev->mac)
+ netdev->mac = xenstore_read_be_str(&netdev->xendev, "mac");
+
+ /* do we have all we need? */
+ if (NULL == netdev->mac)
+ return -1;
+
+ vlan = qemu_find_vlan(netdev->xendev.dev);
+ netdev->vs = qemu_new_vlan_client(vlan, net_rx_packet, net_rx_ok, netdev);
+ snprintf(netdev->vs->info_str, sizeof(netdev->vs->info_str),
+ "nic: xenbus vif macaddr=%s", netdev->mac);
+
+ /* fill info */
+ xenstore_write_be_int(&netdev->xendev, "feature-rx-copy", 1);
+ xenstore_write_be_int(&netdev->xendev, "feature-rx-flip", 0);
+
+ return 0;
+}
+
+static int net_connect(struct XenDevice *xendev)
+{
+ struct XenNetDev *netdev = container_of(xendev, struct XenNetDev, xendev);
+ int rx_copy;
+
+ if (-1 == xenstore_read_fe_int(&netdev->xendev, "tx-ring-ref",
+ &netdev->tx_ring_ref))
+ return -1;
+ if (-1 == xenstore_read_fe_int(&netdev->xendev, "rx-ring-ref",
+ &netdev->rx_ring_ref))
+ return 1;
+ if (-1 == xenstore_read_fe_int(&netdev->xendev, "event-channel",
+ &netdev->xendev.remote_port))
+ return -1;
+
+ if (-1 == xenstore_read_fe_int(&netdev->xendev, "request-rx-copy", &rx_copy))
+ rx_copy = 0;
+ if (0 == rx_copy) {
+ xen_be_printf(&netdev->xendev, 0, "frontend doesn't support rx-copy.\n");
+ return -1;
+ }
+
+ netdev->txs = xc_gnttab_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->xendev.dom,
+ netdev->rx_ring_ref,
+ PROT_READ | PROT_WRITE);
+ if (!netdev->txs || !netdev->rxs)
+ return -1;
+ BACK_RING_INIT(&netdev->tx_ring, netdev->txs, XC_PAGE_SIZE);
+ BACK_RING_INIT(&netdev->rx_ring, netdev->rxs, XC_PAGE_SIZE);
+
+ xen_be_bind_evtchn(&netdev->xendev);
+
+ xen_be_printf(&netdev->xendev, 1, "ok: tx-ring-ref %d, rx-ring-ref %d, "
+ "remote port %d, local port %d\n",
+ netdev->tx_ring_ref, netdev->rx_ring_ref,
+ netdev->xendev.remote_port, netdev->xendev.local_port);
+ return 0;
+}
+
+static void net_disconnect(struct XenDevice *xendev)
+{
+ struct XenNetDev *netdev = container_of(xendev, struct XenNetDev, xendev);
+
+ xen_be_unbind_evtchn(&netdev->xendev);
+
+ if (netdev->txs) {
+ xc_gnttab_munmap(netdev->xendev.gnttabdev, netdev->txs, 1);
+ netdev->txs = NULL;
+ }
+ if (netdev->rxs) {
+ xc_gnttab_munmap(netdev->xendev.gnttabdev, netdev->rxs, 1);
+ netdev->rxs = NULL;
+ }
+}
+
+static void net_event(struct XenDevice *xendev)
+{
+ struct XenNetDev *netdev = container_of(xendev, struct XenNetDev, xendev);
+ net_tx_packets(netdev);
+}
+
+/* ------------------------------------------------------------- */
+
+struct XenDevOps xen_netdev_ops = {
+ .size = sizeof(struct XenNetDev),
+ .flags = DEVOPS_FLAG_NEED_GNTDEV,
+ .init = net_init,
+ .connect = net_connect,
+ .event = net_event,
+ .disconnect = net_disconnect,
+};
--
1.5.5.1
^ permalink raw reply related [flat|nested] 92+ messages in thread* [Qemu-devel] [PATCH 7/7] xen: blk & nic configuration via cmd line.
2008-08-04 15:50 [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Gerd Hoffmann
` (5 preceding siblings ...)
2008-08-04 15:50 ` [Qemu-devel] [PATCH 6/7] xen: add net " Gerd Hoffmann
@ 2008-08-04 15:50 ` Gerd Hoffmann
2008-08-04 17:35 ` Blue Swirl
2008-08-04 17:42 ` [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Anthony Liguori
7 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 15:50 UTC (permalink / raw)
To: qemu-devel, xen-devel; +Cc: Gerd Hoffmann
This patch makes qemu create backend and frontend device entries in
xenstore for devices configured on the command line. It will use
qdisk and qnic backend names, so the qemu internal backends will
be used.
Disks can be created using -drive if=xen,file=...
Nics can be created using -net nic,macaddr=...
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
Makefile.target | 2 +-
hw/xen-backend.h | 7 +++
hw/xen-config.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/xen-machine.c | 19 +++++++-
4 files changed, 165 insertions(+), 2 deletions(-)
create mode 100644 hw/xen-config.c
diff --git a/Makefile.target b/Makefile.target
index 85f938c..a3f8d24 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -516,7 +516,7 @@ LIBS += $(CONFIG_VNC_TLS_LIBS)
endif
# xen backend driver support
-XEN_OBJS := xen-machine.o xen-backend.o
+XEN_OBJS := xen-machine.o xen-backend.o xen-config.o
XEN_OBJS += xen-console.o xen-framebuffer.o xen-disk.o xen-nic.o
ifeq ($(CONFIG_XEN), yes)
OBJS += $(XEN_OBJS)
diff --git a/hw/xen-backend.h b/hw/xen-backend.h
index b484752..c1da2ca 100644
--- a/hw/xen-backend.h
+++ b/hw/xen-backend.h
@@ -3,6 +3,8 @@
#include "xen-common.h"
#include "sysemu.h"
+#include "net.h"
+#include "block_int.h"
/* ------------------------------------------------------------- */
@@ -94,4 +96,9 @@ struct XenDevOps xen_netdev_ops; /* xen_nic.c */
void xen_set_display(int domid, DisplayState *ds);
+/* configuration (aka xenbus setup) */
+void xen_config_cleanup(void);
+int xen_config_dev_blk(DriveInfo *disk);
+int xen_config_dev_nic(NICInfo *nic);
+
#endif /* QEMU_XEN_BACKEND_H */
diff --git a/hw/xen-config.c b/hw/xen-config.c
new file mode 100644
index 0000000..4156f82
--- /dev/null
+++ b/hw/xen-config.c
@@ -0,0 +1,139 @@
+#include "xen-backend.h"
+
+/* ------------------------------------------------------------- */
+
+struct xs_dirs {
+ char *xs_dir;
+ TAILQ_ENTRY(xs_dirs) list;
+};
+static TAILQ_HEAD(xs_dirs_head, xs_dirs) xs_cleanup = TAILQ_HEAD_INITIALIZER(xs_cleanup);
+
+static void xen_config_cleanup_dir(char *dir)
+{
+ struct xs_dirs *d;
+
+ d = qemu_malloc(sizeof(*d));
+ if (!d)
+ return;
+ d->xs_dir = dir;
+ TAILQ_INSERT_TAIL(&xs_cleanup, d, list);
+}
+
+void xen_config_cleanup(void)
+{
+ struct xs_dirs *d;
+
+ fprintf(stderr, "xen be: %s\n", __FUNCTION__);
+ TAILQ_FOREACH(d, &xs_cleanup, list) {
+ xs_rm(xenstore, 0, d->xs_dir);
+ }
+}
+
+/* ------------------------------------------------------------- */
+
+static int xen_config_dev_mkdir(char *dev, int p)
+{
+ struct xs_permissions perms = {
+ .id = xen_domid,
+ .perms = p,
+ };
+
+ if (!xs_mkdir(xenstore, 0, dev)) {
+ fprintf(stderr, "xs_mkdir %s: failed\n", dev);
+ return -1;
+ }
+ xen_config_cleanup_dir(qemu_strdup(dev));
+
+ if (!xs_set_permissions(xenstore, 0, dev, &perms, 1)) {
+ fprintf(stderr, "%s: xs_set_permissions failed\n", __FUNCTION__);
+ return -1;
+ }
+ return 0;
+}
+
+static int xen_config_dev_dirs(char *ftype, char *btype, int vdev,
+ char *fe, char *be, int len)
+{
+ snprintf(fe, len, "/local/domain/%d/device/%s/%d",
+ xen_domid, ftype, vdev);
+ snprintf(be, len, "/local/domain/0/backend/%s/%d/%d",
+ btype, xen_domid, vdev);
+
+ xen_config_dev_mkdir(fe, XS_PERM_WRITE);
+ xen_config_dev_mkdir(be, XS_PERM_READ);
+ return 0;
+}
+
+static int xen_config_dev_all(char *fe, char *be)
+{
+ /* frontend */
+#if 0
+ xenstore_write_str(fe, "protocol",
+ xen_config_dev_protocol(xen));
+#endif
+ xenstore_write_int(fe, "state", XenbusStateInitialising);
+ xenstore_write_int(fe, "backend-id", 0);
+ xenstore_write_str(fe, "backend", be);
+
+ /* backend */
+ xenstore_write_str(be, "domain", qemu_name ? qemu_name : "no-name");
+ xenstore_write_int(be, "online", 1);
+ xenstore_write_int(be, "state", XenbusStateInitialising);
+ xenstore_write_int(be, "frontend-id", xen_domid);
+ xenstore_write_str(be, "frontend", fe);
+
+ return 0;
+}
+
+/* ------------------------------------------------------------- */
+
+int xen_config_dev_blk(DriveInfo *disk)
+{
+ char fe[256], be[256];
+ int vdev = 202 * 256 + 16 * disk->unit;
+ int cdrom = disk->bdrv->type == BDRV_TYPE_CDROM;
+ char *devtype = cdrom ? "cdrom" : "disk";
+ char *mode = cdrom ? "r" : "w";
+
+ snprintf(disk->bdrv->device_name, sizeof(disk->bdrv->device_name),
+ "xvd%c", 'a' + disk->unit);
+ fprintf(stderr, "xen be: config disk %d [%s]: %s\n",
+ disk->unit, disk->bdrv->device_name, disk->bdrv->filename);
+ xen_config_dev_dirs("vbd", "qdisk", vdev, fe, be, sizeof(fe));
+
+ /* frontend */
+ xenstore_write_int(fe, "virtual-device", vdev);
+ xenstore_write_str(fe, "device-type", devtype);
+
+ /* backend */
+ xenstore_write_str(be, "dev", disk->bdrv->device_name);
+ xenstore_write_str(be, "type", "file");
+ xenstore_write_str(be, "params", disk->bdrv->filename);
+ xenstore_write_str(be, "mode", mode);
+
+ /* common stuff */
+ return xen_config_dev_all(fe, be);
+}
+
+int xen_config_dev_nic(NICInfo *nic)
+{
+ char fe[256], be[256];
+ char mac[20];
+
+ snprintf(mac, sizeof(mac), "%02x:%02x:%02x:%02x:%02x:%02x",
+ nic->macaddr[0], nic->macaddr[1], nic->macaddr[2],
+ nic->macaddr[3], nic->macaddr[4], nic->macaddr[5]);
+ fprintf(stderr, "xen be: config nic %d: mac=\"%s\"\n", nic->vlan->id, mac);
+ xen_config_dev_dirs("vif", "qnic", nic->vlan->id, fe, be, sizeof(fe));
+
+ /* frontend */
+ xenstore_write_int(fe, "handle", nic->vlan->id);
+ xenstore_write_str(fe, "mac", mac);
+
+ /* backend */
+ xenstore_write_int(be, "handle", nic->vlan->id);
+ xenstore_write_str(be, "mac", mac);
+
+ /* common stuff */
+ return xen_config_dev_all(fe, be);
+}
diff --git a/hw/xen-machine.c b/hw/xen-machine.c
index c8fb256..4e4d893 100644
--- a/hw/xen-machine.c
+++ b/hw/xen-machine.c
@@ -121,7 +121,7 @@ static void xenpv_init(ram_addr_t ram_size, int vga_ram_size,
const char *cpu_model)
{
CPUState *env;
- int rc;
+ int index,i,rc;
rc = xen_init_pv(ds);
if (-1 == rc)
@@ -147,6 +147,23 @@ static void xenpv_init(ram_addr_t ram_size, int vga_ram_size,
env = cpu_init(cpu_model);
env->halted = 1;
+ /* configure disks */
+ for (i = 0; i < 16; i++) {
+ index = drive_get_index(IF_XEN, 0, i);
+ if (index == -1)
+ continue;
+ xen_config_dev_blk(drives_table + index);
+ }
+
+ /* configure nics */
+ for (i = 0; i < nb_nics; i++) {
+ if (nd_table[i].model && 0 != strcmp(nd_table[i].model, "xen"))
+ continue;
+ xen_config_dev_nic(nd_table + i);
+ }
+
+ /* config cleanup hook */
+ atexit(xen_config_cleanup);
return;
err:
--
1.5.5.1
^ permalink raw reply related [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 7/7] xen: blk & nic configuration via cmd line.
2008-08-04 15:50 ` [Qemu-devel] [PATCH 7/7] xen: blk & nic configuration via cmd line Gerd Hoffmann
@ 2008-08-04 17:35 ` Blue Swirl
2008-08-04 18:12 ` Gerd Hoffmann
0 siblings, 1 reply; 92+ messages in thread
From: Blue Swirl @ 2008-08-04 17:35 UTC (permalink / raw)
To: qemu-devel; +Cc: xen-devel, Gerd Hoffmann
On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> + index = drive_get_index(IF_XEN, 0, i);
Oh, that's why IF_XEN.
> + /* config cleanup hook */
> + atexit(xen_config_cleanup);
I'd put this in vl.c, it's not machine specific.
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] xen: blk & nic configuration via cmd line.
2008-08-04 17:35 ` Blue Swirl
@ 2008-08-04 18:12 ` Gerd Hoffmann
2008-08-04 20:45 ` Blue Swirl
0 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-04 18:12 UTC (permalink / raw)
To: Blue Swirl; +Cc: xen-devel, qemu-devel
Blue Swirl wrote:
>> + /* config cleanup hook */
>> + atexit(xen_config_cleanup);
>
> I'd put this in vl.c, it's not machine specific.
It actually is because it cleans up stuff only created for the xenpv
machine type. Moving it to vl.c doesn't hurt, I would have to wrap it
into "#ifdef CONFIG_XEN" though ...
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] xen: blk & nic configuration via cmd line.
2008-08-04 18:12 ` Gerd Hoffmann
@ 2008-08-04 20:45 ` Blue Swirl
0 siblings, 0 replies; 92+ messages in thread
From: Blue Swirl @ 2008-08-04 20:45 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, qemu-devel
On 8/4/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Blue Swirl wrote:
> >> + /* config cleanup hook */
> >> + atexit(xen_config_cleanup);
> >
> > I'd put this in vl.c, it's not machine specific.
>
>
> It actually is because it cleans up stuff only created for the xenpv
> machine type. Moving it to vl.c doesn't hurt, I would have to wrap it
> into "#ifdef CONFIG_XEN" though ...
I was thinking that for example Sparc32/64 would need another machine,
but it looks like the changes could be small and all CPUs could use
the same xenpv machine type. So it's fine as it is now.
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-04 15:50 [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Gerd Hoffmann
` (6 preceding siblings ...)
2008-08-04 15:50 ` [Qemu-devel] [PATCH 7/7] xen: blk & nic configuration via cmd line Gerd Hoffmann
@ 2008-08-04 17:42 ` Anthony Liguori
2008-08-05 9:58 ` Gerd Hoffmann
` (2 more replies)
7 siblings, 3 replies; 92+ messages in thread
From: Anthony Liguori @ 2008-08-04 17:42 UTC (permalink / raw)
To: qemu-devel; +Cc: xen-devel, Ian Jackson, Gerd Hoffmann, Samuel Thibault
Gerd Hoffmann wrote:
> Hi folks,
>
> xen support is implemented using another machine type. xen's qemu-dm
> already uses the machine type to switch between paravirtualized and
> fully virtualized machines, so this was the natural choice. qemu has
> gets a new "xenpv" machine type additionally to the "pc" and "isapc"
> ones.
>
> Comments?
>
Modulo some of the stylistic feedback that needs to be addressed, I
think these series looks pretty good. It fits pretty well into QEMU.
Unless there are major objections, I'll apply an updated series once
people have had some time to look through it.
However, I'd like to see agreement within the Xen community that this is
the right approach first. In particular, I would like to see these
patches either merged with upstream Xen or for upstream Xen to plan to
rebase against QEMU to pick them up and use them for PV support. This
doesn't mean they have to use the block and net backends of course. I
really don't want to support two implementations of Xen support in QEMU.
Regards,
Anthony Liguori
> cheers,
> Gerd
>
>
>
>
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-04 17:42 ` [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Anthony Liguori
@ 2008-08-05 9:58 ` Gerd Hoffmann
2008-08-05 10:15 ` Samuel Thibault
2008-08-05 10:46 ` Samuel Thibault
2008-08-11 16:36 ` [Qemu-devel] Xen's qemu branches, etc Ian Jackson
2 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-05 9:58 UTC (permalink / raw)
To: Anthony Liguori; +Cc: xen-devel, Ian Jackson, qemu-devel, Samuel Thibault
Anthony Liguori wrote:
> However, I'd like to see agreement within the Xen community that this
> is the right approach first. In particular, I would like to see
> these patches either merged with upstream Xen or for upstream Xen to
> plan to rebase against QEMU to pick them up and use them for PV
> support.
Ian Jackson has a qemu git tree at
http://www.chiark.greenend.org.uk/~ijackson/qemu-xen.git
As far I know it is rebased now and then and the long-term plan is to
get the delta to upstream qemu at least smaller. Dunno whenever it ever
will be zero due to stuff like mapcache.
There are a few compatibility issues to be solved somehow before
upstream qemu is able to completely replace xen's qemu-dm:
(1) my patches use -domid to specify the domain id. xen uses (used?) -d
instead. Which isn't going to work because upstream qemu uses that
switch for something else. Ian's tree supports both -d and -domid, with
a comment saying -d is deprecated. So I think we have an agreement to
use -domid here ;)
(2) xen added a -domain-name switch. upstream qemu has a -name switch.
my patches do not implement -domain-name, I use the name set via -name
instead. IMHO xen should pick up the upstream way to do things here,
i.e. switch over to -name.
(3) vnc configuration is different. This one is not so easy due to
xenstore being involved. So the way things work are pretty xen specific
and don't make that much sense for upstream qemu. The differences are:
Setting the vnc password: qemu uses a monitor command. xen reads it
from xenstore (set by xend). I think we can just support both ways here.
xen has a -vncunused switch which makes qemu scan for a free vnc
display. The tcp port actually used is written to xenstore. Dunno how
to handle that one best. First, I think should be a -vnc switch option
instead of a separate command line switch. Second, propagating the used
port via xenstore doesn't make sense at all for upstream qemu ...
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-05 9:58 ` Gerd Hoffmann
@ 2008-08-05 10:15 ` Samuel Thibault
0 siblings, 0 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-05 10:15 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, qemu-devel
Gerd Hoffmann, le Tue 05 Aug 2008 11:58:55 +0200, a écrit :
> Anthony Liguori wrote:
> > However, I'd like to see agreement within the Xen community that this
> > is the right approach first. In particular, I would like to see
> > these patches either merged with upstream Xen or for upstream Xen to
> > plan to rebase against QEMU to pick them up and use them for PV
> > support.
>
> Ian Jackson has a qemu git tree at
> http://www.chiark.greenend.org.uk/~ijackson/qemu-xen.git
>
> As far I know it is rebased now and then and the long-term plan is to
> get the delta to upstream qemu at least smaller. Dunno whenever it ever
> will be zero due to stuff like mapcache.
There are two branches there: the xen branch is stuff that Ian indeed
doesn't intend to push. The qemu branch is stuff that he wishes to
push.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-04 17:42 ` [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Anthony Liguori
2008-08-05 9:58 ` Gerd Hoffmann
@ 2008-08-05 10:46 ` Samuel Thibault
2008-08-05 11:12 ` Gerd Hoffmann
2008-08-11 16:36 ` [Qemu-devel] Xen's qemu branches, etc Ian Jackson
2 siblings, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-05 10:46 UTC (permalink / raw)
To: Anthony Liguori; +Cc: xen-devel, Ian Jackson, qemu-devel, Gerd Hoffmann
Anthony Liguori, le Mon 04 Aug 2008 12:42:38 -0500, a écrit :
> However, I'd like to see agreement within the Xen community that this is
> the right approach first.
Unfortunately, Ian Jackson (who will continue to maintain his qemu
tree) is away this week.
> In particular, I would like to see these
> patches either merged with upstream Xen or for upstream Xen to plan to
> rebase against QEMU to pick them up and use them for PV support.
I have not had a precise look at e.g. the xenfb.c code, but if what Gerd
has submitted is exactly the same as xen-unstable's, then that part is
already merged :)
As for the rest, as Gerd said Ian is maintaining a rebased tree, which
is currently used by default in the upcoming xen 4.0 (aka 3.3). He
won't rebase it until that release, but after that he will probably
regularly, with much less pain than previously anyway since we now have
a fine git tree.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-05 10:46 ` Samuel Thibault
@ 2008-08-05 11:12 ` Gerd Hoffmann
2008-08-05 11:29 ` Samuel Thibault
0 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-05 11:12 UTC (permalink / raw)
To: Samuel Thibault, Anthony Liguori, qemu-devel, xen-devel,
Gerd Hoffmann, Ian Jackson
> I have not had a precise look at e.g. the xenfb.c code, but if what Gerd
> has submitted is exactly the same as xen-unstable's, then that part is
> already merged :)
It isn't exactly the same code. For example the xenbus state engine has
been factored out into generic code, shared by all backend drivers. The
behaviour should be identical though.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-05 11:12 ` Gerd Hoffmann
@ 2008-08-05 11:29 ` Samuel Thibault
2008-08-05 13:18 ` Gerd Hoffmann
0 siblings, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-05 11:29 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, qemu-devel
Gerd Hoffmann, le Tue 05 Aug 2008 13:12:35 +0200, a écrit :
> > I have not had a precise look at e.g. the xenfb.c code, but if what Gerd
> > has submitted is exactly the same as xen-unstable's, then that part is
> > already merged :)
>
> It isn't exactly the same code. For example the xenbus state engine has
> been factored out into generic code, shared by all backend drivers.
Then it'd probably be good to push that upstream Xen.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-05 11:29 ` Samuel Thibault
@ 2008-08-05 13:18 ` Gerd Hoffmann
2008-08-05 15:03 ` Samuel Thibault
0 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-05 13:18 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Anthony Liguori, qemu-devel,
xen-devel, Ian Jackson
Samuel Thibault wrote:
> Gerd Hoffmann, le Tue 05 Aug 2008 13:12:35 +0200, a écrit :
>>> I have not had a precise look at e.g. the xenfb.c code, but if what Gerd
>>> has submitted is exactly the same as xen-unstable's, then that part is
>>> already merged :)
>> It isn't exactly the same code. For example the xenbus state engine has
>> been factored out into generic code, shared by all backend drivers.
>
> Then it'd probably be good to push that upstream Xen.
I'm cross-posting the patches to xen-devel for a reason.
Whenever you want to pick them up now or just wait for them appear in
upstream qemu and handle it with the next rebase is your call. But at
least reviewing and commenting now would be very helpful, so I don't
break things for xen by mistake and we can proceed with merging those bits.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-05 13:18 ` Gerd Hoffmann
@ 2008-08-05 15:03 ` Samuel Thibault
2008-08-05 15:41 ` Samuel Thibault
0 siblings, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-05 15:03 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, qemu-devel
Gerd Hoffmann, le Tue 05 Aug 2008 15:18:46 +0200, a écrit :
> Samuel Thibault wrote:
> > Then it'd probably be good to push that upstream Xen.
>
> I'm cross-posting the patches to xen-devel for a reason.
Well, cross-posting qemu patches to xen-devel is not the same as posting
xen patches to xen-devel...
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-05 15:03 ` Samuel Thibault
@ 2008-08-05 15:41 ` Samuel Thibault
2008-08-05 15:46 ` Anthony Liguori
` (2 more replies)
0 siblings, 3 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-05 15:41 UTC (permalink / raw)
To: Gerd Hoffmann, Anthony Liguori, qemu-devel, xen-devel,
Ian Jackson
Samuel Thibault, le Tue 05 Aug 2008 16:03:28 +0100, a écrit :
> Gerd Hoffmann, le Tue 05 Aug 2008 15:18:46 +0200, a écrit :
> > Samuel Thibault wrote:
> > > Then it'd probably be good to push that upstream Xen.
> >
> > I'm cross-posting the patches to xen-devel for a reason.
>
> Well, cross-posting qemu patches to xen-devel is not the same as posting
> xen patches to xen-devel...
Just to explain a bit more, now that I have read the patches a bit: what
is there is somehow far from the xen unstable tree, at least because
of the backend driver core, and because of other 'details' (renaming,
moving). I doubt Ian will be happy to have to make the effort to merge
that in his tree and I guess he will probably just drop everything and
keep the xen-unstable version.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-05 15:41 ` Samuel Thibault
@ 2008-08-05 15:46 ` Anthony Liguori
2008-08-05 16:07 ` Blue Swirl
2008-08-05 15:47 ` Samuel Thibault
2008-08-06 10:14 ` Gerd Hoffmann
2 siblings, 1 reply; 92+ messages in thread
From: Anthony Liguori @ 2008-08-05 15:46 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Anthony Liguori, qemu-devel,
xen-devel, Ian Jackson
Samuel Thibault wrote:
> Samuel Thibault, le Tue 05 Aug 2008 16:03:28 +0100, a écrit :
>
>> Gerd Hoffmann, le Tue 05 Aug 2008 15:18:46 +0200, a écrit :
>>
>> Well, cross-posting qemu patches to xen-devel is not the same as posting
>> xen patches to xen-devel...
>>
>
> Just to explain a bit more, now that I have read the patches a bit: what
> is there is somehow far from the xen unstable tree, at least because
> of the backend driver core, and because of other 'details' (renaming,
> moving). I doubt Ian will be happy to have to make the effort to merge
> that in his tree and I guess he will probably just drop everything and
> keep the xen-unstable version.
>
Which is why ya'll need to work something out before we apply these
patches to QEMU :-)
Regards,
Anthony Liguori
> Samuel
>
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-05 15:46 ` Anthony Liguori
@ 2008-08-05 16:07 ` Blue Swirl
0 siblings, 0 replies; 92+ messages in thread
From: Blue Swirl @ 2008-08-05 16:07 UTC (permalink / raw)
To: qemu-devel; +Cc: xen-devel, Ian Jackson, Gerd Hoffmann, Samuel Thibault
On 8/5/08, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Samuel Thibault wrote:
>
> > Samuel Thibault, le Tue 05 Aug 2008 16:03:28 +0100, a écrit :
> >
> >
> > > Gerd Hoffmann, le Tue 05 Aug 2008 15:18:46 +0200, a écrit :
> > > Well, cross-posting qemu patches to xen-devel is not the same as
> posting
> > > xen patches to xen-devel...
> > >
> > >
> >
> > Just to explain a bit more, now that I have read the patches a bit: what
> > is there is somehow far from the xen unstable tree, at least because
> > of the backend driver core, and because of other 'details' (renaming,
> > moving). I doubt Ian will be happy to have to make the effort to merge
> > that in his tree and I guess he will probably just drop everything and
> > keep the xen-unstable version.
> >
> >
>
> Which is why ya'll need to work something out before we apply these patches
> to QEMU :-)
I'd salvage nodisk feature (useful for Sparc and embedded systems) and
if ever produced, patch that introduces qemu_realloc and does
s/realloc/qemu_realloc/g.
For global -Wall -Wstrict-prototypes flags probably the project
leaders should agree that it will be useful.
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-05 15:41 ` Samuel Thibault
2008-08-05 15:46 ` Anthony Liguori
@ 2008-08-05 15:47 ` Samuel Thibault
2008-08-06 10:14 ` Gerd Hoffmann
2 siblings, 0 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-05 15:47 UTC (permalink / raw)
To: Gerd Hoffmann, Anthony Liguori, qemu-devel, xen-devel,
Ian Jackson
Samuel Thibault, le Tue 05 Aug 2008 16:41:40 +0100, a écrit :
> Samuel Thibault, le Tue 05 Aug 2008 16:03:28 +0100, a écrit :
> > Gerd Hoffmann, le Tue 05 Aug 2008 15:18:46 +0200, a écrit :
> > > Samuel Thibault wrote:
> > > > Then it'd probably be good to push that upstream Xen.
> > >
> > > I'm cross-posting the patches to xen-devel for a reason.
> >
> > Well, cross-posting qemu patches to xen-devel is not the same as posting
> > xen patches to xen-devel...
>
> Just to explain a bit more, now that I have read the patches a bit:
And to be even more explicit, by "I have read the patches", I mean: I
have applied them to upstream qemu, them vimdiffed the files there with
the ones from Ian's tree (sometimes with different names), in order
to have an idea of what the merge would be (i.e. the "xen patches" I
mentioned above, which is what we care about from the point of view of
xen-unstable).
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-05 15:41 ` Samuel Thibault
2008-08-05 15:46 ` Anthony Liguori
2008-08-05 15:47 ` Samuel Thibault
@ 2008-08-06 10:14 ` Gerd Hoffmann
2008-08-06 10:23 ` Samuel Thibault
2 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-06 10:14 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Anthony Liguori, qemu-devel,
xen-devel, Ian Jackson
Samuel Thibault wrote:
> Samuel Thibault, le Tue 05 Aug 2008 16:03:28 +0100, a écrit :
>> Gerd Hoffmann, le Tue 05 Aug 2008 15:18:46 +0200, a écrit :
>>> Samuel Thibault wrote:
>>>> Then it'd probably be good to push that upstream Xen.
>>> I'm cross-posting the patches to xen-devel for a reason.
>> Well, cross-posting qemu patches to xen-devel is not the same as posting
>> xen patches to xen-devel...
>
> Just to explain a bit more, now that I have read the patches a bit: what
> is there is somehow far from the xen unstable tree, at least because
> of the backend driver core, and because of other 'details' (renaming,
> moving). I doubt Ian will be happy to have to make the effort to merge
> that in his tree and I guess he will probably just drop everything and
> keep the xen-unstable version.
Sure, merging stuff upstream involves alot of work short-term, you'll
get the benefits long-term.
pv domain support is easy, as this is largely self-contained. hvm will
be even more work, because hvm support is much more invasive and on top
of that the qemu interfaces will change to nicely support the various
ways to run code natively instead of emulated (kqemu, kvm, xen).
The rough way to merge would look like this:
- drop xen_console.[ch]
- drop xenfb.[ch]
- drop xen_machine_pv.c
- add xen.h
- add xen-machine.c
- add xen-backend.[ch]
- add xen-console.c
- add xen-framebuffer.c
- wind up stuff in the Makefiles.
- some global renames (domid -> xen_domid for example) as I took care
to prefix global xen variables & functions with xen_.
- probably some small fixups are needed ...
You probably wouldn't do that before the 3.3 (4.0?) release. If you
keep the xenish vl.c version the differences between upstream qemu and
qemu-dm command line options and vnc handling go away. The you can
gradually move qemu-dm to be more upstream-ish while also updating xend
accordingly.
Note that qemu-dm has a few optimizations in the qemu display code to
avoid unneeded work. I'd suggest to submit them upstream. The we can
enable the bits in xen-framebuffer.c which make use of them.
DisplayState->idle for example.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 10:14 ` Gerd Hoffmann
@ 2008-08-06 10:23 ` Samuel Thibault
2008-08-06 12:43 ` [Xen-devel] " Markus Armbruster
2008-08-06 13:24 ` Gerd Hoffmann
0 siblings, 2 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-06 10:23 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, qemu-devel
Gerd Hoffmann, le Wed 06 Aug 2008 12:14:25 +0200, a écrit :
> The rough way to merge would look like this:
>
> - drop xen_console.[ch]
> - drop xenfb.[ch]
> - drop xen_machine_pv.c
>
> - add xen.h
> - add xen-machine.c
> - add xen-backend.[ch]
> - add xen-console.c
> - add xen-framebuffer.c
>
> - wind up stuff in the Makefiles.
> - some global renames (domid -> xen_domid for example) as I took care
> to prefix global xen variables & functions with xen_.
> - probably some small fixups are needed ...
You forgot the _test_ stage. You are basically asking us to replace our
well-tested implementation with your implementation, that's quite a
move. You are not even providing a patch for us to check that nothing
has been left behind...
> Note that qemu-dm has a few optimizations in the qemu display code to
> avoid unneeded work. I'd suggest to submit them upstream.
That's planned, yes. But you too could do the same for the device
backend core for instance...
> The you can gradually move qemu-dm to be more upstream-ish while also
> updating xend accordingly.
That's what we are already doing in Ian's tree.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 10:23 ` Samuel Thibault
@ 2008-08-06 12:43 ` Markus Armbruster
2008-08-06 12:50 ` Samuel Thibault
2008-08-06 13:24 ` Gerd Hoffmann
1 sibling, 1 reply; 92+ messages in thread
From: Markus Armbruster @ 2008-08-06 12:43 UTC (permalink / raw)
To: Samuel Thibault; +Cc: xen-devel, Ian Jackson, Gerd Hoffmann, qemu-devel
Samuel Thibault <samuel.thibault@eu.citrix.com> writes:
> Gerd Hoffmann, le Wed 06 Aug 2008 12:14:25 +0200, a écrit :
>> The rough way to merge would look like this:
>>
>> - drop xen_console.[ch]
>> - drop xenfb.[ch]
>> - drop xen_machine_pv.c
>>
>> - add xen.h
>> - add xen-machine.c
>> - add xen-backend.[ch]
>> - add xen-console.c
>> - add xen-framebuffer.c
>>
>> - wind up stuff in the Makefiles.
>> - some global renames (domid -> xen_domid for example) as I took care
>> to prefix global xen variables & functions with xen_.
>> - probably some small fixups are needed ...
>
> You forgot the _test_ stage. You are basically asking us to replace our
> well-tested implementation with your implementation, that's quite a
> move. You are not even providing a patch for us to check that nothing
> has been left behind...
It *is* quite move, becauses it accomplishes a lot: it goes from a
heavily modified fork of an oldish version all the way to merge with
upstream, as far as PV is concerned.
If that's where we want to go, we can of course still argue whether we
should go in leaps or baby steps, and whether Gerd's leap lands in
quite the right spot. But the distance to conquer remains the same,
and so does the testing challenge.
For what it's worth, I went over significant parts of Gerd's patch
(all the generic stuff + pvfb) with a fine comb, comparing it to what
we have now. I consider it sound.
[...]
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 12:43 ` [Xen-devel] " Markus Armbruster
@ 2008-08-06 12:50 ` Samuel Thibault
2008-08-06 13:54 ` Gerd Hoffmann
0 siblings, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-06 12:50 UTC (permalink / raw)
To: Markus Armbruster; +Cc: xen-devel, Ian Jackson, Gerd Hoffmann, qemu-devel
Markus Armbruster, le Wed 06 Aug 2008 08:43:49 -0400, a écrit :
> It *is* quite move, becauses it accomplishes a lot: it goes from a
> heavily modified fork of an oldish version all the way to merge with
> upstream, as far as PV is concerned.
Then why doing it in qemu before having it tested in the xen unstable
tree? That's not the way I usually see merging happen.
> For what it's worth, I went over significant parts of Gerd's patch
> (all the generic stuff + pvfb) with a fine comb, comparing it to what
> we have now. I consider it sound.
I'm not saying it's not fine. I had a look and the code looked fine
indeed. But what I'm afraid of is the delta with Ian would have to
bear when merging: is it save/restore safe, does it work with PCI
pass-through, VT-D, etc.?
> If that's where we want to go, we can of course still argue whether we
> should go in leaps or baby steps, and whether Gerd's leap lands in
> quite the right spot.
Baby steps are much easier to review. That's how things are usually
done, and here it looks to me like it is feasible to achieve in Xen (and
have it tested) before event thinking about importing a pile of code in
qemu where it won't receive as much testing as the xen-unstable tree
receives.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 12:50 ` Samuel Thibault
@ 2008-08-06 13:54 ` Gerd Hoffmann
2008-08-06 14:01 ` Samuel Thibault
` (2 more replies)
0 siblings, 3 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-06 13:54 UTC (permalink / raw)
To: Samuel Thibault, Markus Armbruster, Gerd Hoffmann,
Anthony Liguori, qemu-devel, xen-devel, Ian Jackson
Hi,
> Baby steps are much easier to review. That's how things are usually
> done, and here it looks to me like it is feasible to achieve in Xen (and
> have it tested) before event thinking about importing a pile of code in
> qemu where it won't receive as much testing as the xen-unstable tree
> receives.
IMHO the best way to address this issue is to rebase frequently. The
xen-unstable testing will also covers qemu then. It also keeps the
delta smaller and makes it easier to test qemu patches in qemu-dm.
I'm trying to fiddle my bits into qemu-dm right now. One problem is
that I get build failures simply because of the fact that the qemu-dm
base is old compared to upstream qemu.
It would also be great if you can clean up the header mess. qemu-dm
doesn't build without xen-unstable.hg. And just a checked out tree
isn't good enougth, it must be compiled because qemu-dm depends on some
generated header files. Aiee. IMHO qemu-dm should only need
/usr/include/xen. If headers are missing there, they should be installed.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 13:54 ` Gerd Hoffmann
@ 2008-08-06 14:01 ` Samuel Thibault
2008-08-06 14:08 ` Gerd Hoffmann
2008-08-07 17:40 ` Stefano Stabellini
2008-08-11 9:18 ` Ian Jackson
2 siblings, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-06 14:01 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel
Gerd Hoffmann, le Wed 06 Aug 2008 15:54:33 +0200, a écrit :
> IMHO the best way to address this issue is to rebase frequently.
Then why not pushing the changes to xen repositories in the first place?
> I'm trying to fiddle my bits into qemu-dm right now. One problem is
> that I get build failures simply because of the fact that the qemu-dm
> base is old compared to upstream qemu.
Use Ian Jackson's tree!
> It would also be great if you can clean up the header mess. qemu-dm
> doesn't build without xen-unstable.hg. And just a checked out tree
> isn't good enougth, it must be compiled because qemu-dm depends on some
> generated header files. Aiee. IMHO qemu-dm should only need
> /usr/include/xen. If headers are missing there, they should be installed.
You could submit patches...
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 14:01 ` Samuel Thibault
@ 2008-08-06 14:08 ` Gerd Hoffmann
2008-08-06 14:25 ` Samuel Thibault
0 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-06 14:08 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Markus Armbruster,
Anthony Liguori, qemu-devel, xen-devel, Ian Jackson
Samuel Thibault wrote:
> Gerd Hoffmann, le Wed 06 Aug 2008 15:54:33 +0200, a écrit :
>> IMHO the best way to address this issue is to rebase frequently.
>
> Then why not pushing the changes to xen repositories in the first place?
Because I want have xen support in *upstream* qemu.
>> I'm trying to fiddle my bits into qemu-dm right now. One problem is
>> that I get build failures simply because of the fact that the qemu-dm
>> base is old compared to upstream qemu.
>
> Use Ian Jackson's tree!
Adapt my patches to an oldish qemu fork, so they must be adapted again
for submitting upstream? No, thank you.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 14:08 ` Gerd Hoffmann
@ 2008-08-06 14:25 ` Samuel Thibault
2008-08-06 15:35 ` Gerd Hoffmann
2008-08-06 22:10 ` Samuel Thibault
0 siblings, 2 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-06 14:25 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel
Gerd Hoffmann, le Wed 06 Aug 2008 16:08:47 +0200, a écrit :
> Samuel Thibault wrote:
> > Gerd Hoffmann, le Wed 06 Aug 2008 15:54:33 +0200, a écrit :
> >> IMHO the best way to address this issue is to rebase frequently.
> >
> > Then why not pushing the changes to xen repositories in the first place?
>
> Because I want have xen support in *upstream* qemu.
I know, but you are basically saying "ok, guys, you'll just manage the
breakage of the big merge, thanks", that's not very convenient, to say
the least. For instance, where should fix patches be sent to? Both
qemu-devel and xen-devel? Just xen-devel and thus make Ian have to push
them to qemu-devel?
Pushing the cleaning changes to Xen first can be done and would entail
much easier to tackle breakage, and the merge back from qemu would then
be trivial, why not doing so?
> >> I'm trying to fiddle my bits into qemu-dm right now. One problem is
> >> that I get build failures simply because of the fact that the qemu-dm
> >> base is old compared to upstream qemu.
> >
> > Use Ian Jackson's tree!
>
> Adapt my patches to an oldish qemu fork, so they must be adapted again
> for submitting upstream? No, thank you.
AGAIN, THERE IS THE IAN JACKSON TREE.
Thanks for reading. Ian has _already_ done that hard work.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 14:25 ` Samuel Thibault
@ 2008-08-06 15:35 ` Gerd Hoffmann
2008-08-06 15:47 ` Samuel Thibault
2008-08-06 16:01 ` Laurent Vivier
2008-08-06 22:10 ` Samuel Thibault
1 sibling, 2 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-06 15:35 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Markus Armbruster,
Anthony Liguori, qemu-devel, xen-devel, Ian Jackson
Samuel Thibault wrote:
> Gerd Hoffmann, le Wed 06 Aug 2008 16:08:47 +0200, a écrit :
>> Samuel Thibault wrote:
>>> Gerd Hoffmann, le Wed 06 Aug 2008 15:54:33 +0200, a écrit :
>>>> IMHO the best way to address this issue is to rebase frequently.
>>> Then why not pushing the changes to xen repositories in the first place?
>> Because I want have xen support in *upstream* qemu.
>
> I know, but you are basically saying "ok, guys, you'll just manage the
> breakage of the big merge, thanks", that's not very convenient, to say
> the least. For instance, where should fix patches be sent to? Both
> qemu-devel and xen-devel? Just xen-devel and thus make Ian have to push
> them to qemu-devel?
Most of the patches probably to both qemu-devel and xen-devel. Stuff
unrelated to xen (say e1000 emulation bugs) qemu-devel only.
> Pushing the cleaning changes to Xen first can be done and would entail
> much easier to tackle breakage, and the merge back from qemu would then
> be trivial, why not doing so?
>
>>>> I'm trying to fiddle my bits into qemu-dm right now. One problem is
>>>> that I get build failures simply because of the fact that the qemu-dm
>>>> base is old compared to upstream qemu.
>>> Use Ian Jackson's tree!
>> Adapt my patches to an oldish qemu fork, so they must be adapted again
>> for submitting upstream? No, thank you.
>
> AGAIN, THERE IS THE IAN JACKSON TREE.
>
> Thanks for reading.
I get build failures when patching THE IAN JACKSON TREE due to THE IAN
JACKSON TREE lagging behind upstream.
I can read, thank you. Please stop crying.
> Ian has _already_ done that hard work.
Oh, rebasing isn't a one-shot thing. You have to do it over and over
again. Unless you get your code upstream of course.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 15:35 ` Gerd Hoffmann
@ 2008-08-06 15:47 ` Samuel Thibault
2008-08-06 22:10 ` Gerd Hoffmann
2008-08-06 16:01 ` Laurent Vivier
1 sibling, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-06 15:47 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel
Gerd Hoffmann, le Wed 06 Aug 2008 17:35:39 +0200, a écrit :
> > AGAIN, THERE IS THE IAN JACKSON TREE.
> >
> > Thanks for reading.
>
> I get build failures when patching THE IAN JACKSON TREE due to THE IAN
> JACKSON TREE lagging behind upstream.
You didn't mention that. "lagging" a few months indeed, while merging
stuff. And because xen 4.0 is about to be released, but it will
probably catch up soon. Anyway that's not a reason for not using it, I
guess the build failures can be fixed more easily than redoing Ian's
work.
> > Ian has _already_ done that hard work.
>
> Oh, rebasing isn't a one-shot thing. You have to do it over and over
> again.
Sure, that's why Ian was quite harsh on the modifications to the qemu
code, and these are quite small now (I'm talking about the Xen-specific
things, non-Xen-specific will be pushed upstream), so that shouldn't be
a problem any more.
SAmuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 15:47 ` Samuel Thibault
@ 2008-08-06 22:10 ` Gerd Hoffmann
2008-08-06 22:16 ` Samuel Thibault
0 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-06 22:10 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Markus Armbruster,
Anthony Liguori, qemu-devel, xen-devel, Ian Jackson
Samuel Thibault wrote:
> Gerd Hoffmann, le Wed 06 Aug 2008 17:35:39 +0200, a écrit :
>>> AGAIN, THERE IS THE IAN JACKSON TREE.
>>>
>>> Thanks for reading.
>> I get build failures when patching THE IAN JACKSON TREE due to THE IAN
>> JACKSON TREE lagging behind upstream.
>
> You didn't mention that. "lagging" a few months indeed, while merging
> stuff. And because xen 4.0 is about to be released, but it will
> probably catch up soon.
Well, for 4.0.x you probably have to branch off anyway, so the release
shouldn't stop catching up with upstream.
> Anyway that's not a reason for not using it, I
> guess the build failures can be fixed more easily than redoing Ian's
> work.
I'm using upstream qemu for development, and I'm not going to change that.
I can adapt the patches for the xen tree. Having the xen tree *not*
lagging behind would be *very* helpful for keeping the trees and patches
in sync.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 22:10 ` Gerd Hoffmann
@ 2008-08-06 22:16 ` Samuel Thibault
0 siblings, 0 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-06 22:16 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel
Gerd Hoffmann, le Thu 07 Aug 2008 00:10:12 +0200, a écrit :
> Samuel Thibault wrote:
> > Gerd Hoffmann, le Wed 06 Aug 2008 17:35:39 +0200, a écrit :
> >>> AGAIN, THERE IS THE IAN JACKSON TREE.
> >>>
> >>> Thanks for reading.
> >> I get build failures when patching THE IAN JACKSON TREE due to THE IAN
> >> JACKSON TREE lagging behind upstream.
> >
> > You didn't mention that. "lagging" a few months indeed, while merging
> > stuff. And because xen 4.0 is about to be released, but it will
> > probably catch up soon.
>
> Well, for 4.0.x you probably have to branch off anyway,
When 4.0 gets released, yes.
> so the release shouldn't stop catching up with upstream.
The way Xen has been working up to now is to branch the trunk just after
the release, not before.
> > Anyway that's not a reason for not using it, I
> > guess the build failures can be fixed more easily than redoing Ian's
> > work.
>
> I'm using upstream qemu for development, and I'm not going to change that.
No problem.
> I can adapt the patches for the xen tree.
That's what I'm asking you from the start of the thread, I guess my
english is not so clear.
> Having the xen tree *not* lagging behind would be *very* helpful for
> keeping the trees and patches in sync.
Sure, it just happens that it couldn't be done before we get Ian's tree
working. Once the 4.0 release is done we can tighten the gap. And
things that shouldn't be only in Ian's tree (e.g. the enhanced serial
support) are being pushed already.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 15:35 ` Gerd Hoffmann
2008-08-06 15:47 ` Samuel Thibault
@ 2008-08-06 16:01 ` Laurent Vivier
1 sibling, 0 replies; 92+ messages in thread
From: Laurent Vivier @ 2008-08-06 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: xen-devel, Ian Jackson, Markus Armbruster, Gerd Hoffmann,
Samuel Thibault
Le mercredi 06 août 2008 à 17:35 +0200, Gerd Hoffmann a écrit :
> Samuel Thibault wrote:
[...]
> > AGAIN, THERE IS THE IAN JACKSON TREE.
> >
> > Thanks for reading.
>
> I get build failures when patching THE IAN JACKSON TREE due to THE IAN
> JACKSON TREE lagging behind upstream.
>
> I can read, thank you. Please stop crying.
Hi guys,
if you want to merge your "xen-bits" into qemu, I think you should work
together not against each other...
(whereas personally I prefer KVM ;-) )
Gerd, IMHO, starting from scratch is never the best solution.
Samuel, perhaps you can help Gerd...
Regards,
Laurent
--
----------------- Laurent.Vivier@bull.net ------------------
"La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 14:25 ` Samuel Thibault
2008-08-06 15:35 ` Gerd Hoffmann
@ 2008-08-06 22:10 ` Samuel Thibault
2008-08-07 7:33 ` Gerd Hoffmann
1 sibling, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-06 22:10 UTC (permalink / raw)
To: Gerd Hoffmann, Markus Armbruster, Anthony Liguori, qemu-devel,
xen-devel, Ian Jackson
Samuel Thibault, le Wed 06 Aug 2008 15:25:26 +0100, a écrit :
> Pushing the cleaning changes to Xen first can be done and would entail
> much easier to tackle breakage, and the merge back from qemu would then
> be trivial, why not doing so?
You didn't answer that part. Really, my only concern is about having
things tested. Isn't it possible for instance to just merge the backend
core (and console/xenfb updates) in Ian's tree first for a start? As
Markus and I said, it looked sane. Then you can push your code to qemu,
I guess that could be fine, as you said xen will not need to use e.g.
the block and net backends.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 22:10 ` Samuel Thibault
@ 2008-08-07 7:33 ` Gerd Hoffmann
2008-08-07 10:53 ` Samuel Thibault
2008-08-07 15:06 ` Blue Swirl
0 siblings, 2 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-07 7:33 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Markus Armbruster,
Anthony Liguori, qemu-devel, xen-devel, Ian Jackson
Samuel Thibault wrote:
> Samuel Thibault, le Wed 06 Aug 2008 15:25:26 +0100, a écrit :
>> Pushing the cleaning changes to Xen first can be done and would entail
>> much easier to tackle breakage, and the merge back from qemu would then
>> be trivial, why not doing so?
>
> You didn't answer that part. Really, my only concern is about having
> things tested. Isn't it possible for instance to just merge the backend
> core (and console/xenfb updates) in Ian's tree first for a start?
http://kraxel.fedorapeople.org/patches/qemu-xen/
I didn't touch the build system, it is even more scary than the qemu one
alone, I just set CONFIG_XEN unconditionally.
I also largely left vl.c as-is, so xend shouldn't need any changes. The
-domid switch sets an additional (redundant) variable, to keep the
amount of changes as small as possible for now. Also -name and
-domain-name are aliased, both set qemu_name and domain_name.
In upstream qemu xenpv support is a runtime-switch for the normal qemu,
the xen patches leave the qemu-dm target in place.
The framebuffer driver probably has some performance regressions.
Fixing those depends on the display patches being pushed upstream.
> Then you can push your code to qemu,
> I guess that could be fine, as you said xen will not need to use e.g.
> the block and net backends.
blk and net backends are not there (yet). But they should be a nop for
xen anyway as long as you don't wind up stuff in xend to put them in
use. For the net backend it probably wouldn't be that useful. The
block backend should be a good replacement for blktap though and maybe
can save you the effort of porting the blktap kernel driver to the
pv_ops kernel.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 7:33 ` Gerd Hoffmann
@ 2008-08-07 10:53 ` Samuel Thibault
2008-08-07 12:13 ` Gerd Hoffmann
2008-08-07 15:06 ` Blue Swirl
1 sibling, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-07 10:53 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel
Gerd Hoffmann, le Thu 07 Aug 2008 09:33:06 +0200, a écrit :
> Samuel Thibault wrote:
> > Samuel Thibault, le Wed 06 Aug 2008 15:25:26 +0100, a écrit :
> >> Pushing the cleaning changes to Xen first can be done and would entail
> >> much easier to tackle breakage, and the merge back from qemu would then
> >> be trivial, why not doing so?
> >
> > You didn't answer that part. Really, my only concern is about having
> > things tested. Isn't it possible for instance to just merge the backend
> > core (and console/xenfb updates) in Ian's tree first for a start?
>
> http://kraxel.fedorapeople.org/patches/qemu-xen/
Ok, thanks, that's already better.
I'd still say that "delete a file then add another one" is far from
convenient both from a point of view of proof-reading the patches and
repository history, but I guess we can manage that with a renaming step
first.
Any reason for the renames, though? (they tend to bother developpers who
have to change their habits, so we can not do that without a reason)
- Why dashes instead of underscores?
- In Xen, we call xen-machine.c xen_machine_pv.c because there is also
a xen_machine_fv.c. Can't the xen_machine_pv.c name be fine for qemu
upstream too? Or xen can keep its own xen_machine_{pv,fv}.c and your
xen-machine.c goes upstream, I don't think that would be a problem.
- I guess a xenfb -> xen[-_]{fb,framebuffer} rename is indeed more
coherent, Markus, do you prefer just "fb" or the longer "framebuffer"?
Misc stuff:
- I guess the sys-queue.h file should be merged and used in qemu first?
- xen_domid is re-declared in xen-backend.h
- The BUFSIZE macro in xen-backend.h is a bit unfortunate, but still
better than what we have currently. Maybe it should be renamed with a
- XEN_ prefix to avoid any clash?
- container_of would probably be useful in other parts of qemu, I guess
it could be merged in qemu first?
- nice work on moving stuff from console and fb to the backend, console
is easier to read now :)
> I also largely left vl.c as-is, so xend shouldn't need any changes. The
> -domid switch sets an additional (redundant) variable, to keep the
> amount of changes as small as possible for now. Also -name and
> -domain-name are aliased, both set qemu_name and domain_name.
That's a good thing :)
> The framebuffer driver probably has some performance regressions.
> Fixing those depends on the display patches being pushed upstream.
It would have been easier to review if you had provided a delta patch,
not a delete/add patch </grin>.
That's actually the kind of things I was afraid of and that would have
been harder to spot when just pulling your work from qemu upstream.
In your xen patch, since idle and GUI_REFRESH_INTERVAL are there, you
can just use them. We'll push them to qemu. We'll manage the _shared
stuff (and push it eventually).
I'll let Markus comment on the rest (again, thanks for moving the xenbus
stuff to the backend part). There is one change that is not backend
changes or just moving code around: you are queuing rectangle updates,
why? (I'm not arguing, just wondering the kind of optimization that can
be).
> > Then you can push your code to qemu,
> > I guess that could be fine, as you said xen will not need to use e.g.
> > the block and net backends.
>
> blk and net backends are not there (yet). But they should be a nop for
> xen anyway
Indeed, I'd say don't bother to port those.
> The block backend should be a good replacement for blktap though and
> maybe can save you the effort of porting the blktap kernel driver to
> the pv_ops kernel.
Well, for better performance an in-kernel blktap would still be useful.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 10:53 ` Samuel Thibault
@ 2008-08-07 12:13 ` Gerd Hoffmann
2008-08-07 12:54 ` Samuel Thibault
` (3 more replies)
0 siblings, 4 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-07 12:13 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Markus Armbruster,
Anthony Liguori, qemu-devel, xen-devel, Ian Jackson
> Ok, thanks, that's already better.
>
> I'd still say that "delete a file then add another one" is far from
> convenient both from a point of view of proof-reading the patches and
> repository history, but I guess we can manage that with a renaming step
> first.
Fine with me, I can't send patches with renames though ...
> Any reason for the renames, though? (they tend to bother developpers who
> have to change their habits, so we can not do that without a reason)
Get consistent naming (all xen stuff in hw/ is prefixed with xen-).
> - Why dashes instead of underscores?
(1) It looks more pleasent to my eyes.
(2) It is easier to type (no shift needed) on both us and german
keyboards.
(3) The files in the qemu source tree don't have a consistent style
in respect to '-' vs. '_', so I had no reason to not use my
personal preference ;)
> - In Xen, we call xen-machine.c xen_machine_pv.c because there is also
> a xen_machine_fv.c. Can't the xen_machine_pv.c name be fine for qemu
> upstream too?
Dunno whenever there ever will be a xenfv machine type upstream as most
likely the normal 'pc' machine type will get a fancy interface to switch
between emulation, kqemu and kvm. And I think the best option for xen
hvm would be to hook in there too. That is a long-term thing though,
xen_machine_fv.c will probably stay for quite a while at in the xen
tree, so maybe 'pv' in the name helps avoiding confusion. I'd prefer to
stay with the xen-machine.c though.
> Or xen can keep its own xen_machine_{pv,fv}.c and your
> xen-machine.c goes upstream, I don't think that would be a problem.
We have two files defining xenpv_machine then. I don't think it is a
good idea.
> Misc stuff:
> - I guess the sys-queue.h file should be merged and used in qemu first?
Rebase. Then it will be there, and you can skip the patch ;)
> - xen_domid is re-declared in xen-backend.h
Oops. Fixed.
> - The BUFSIZE macro in xen-backend.h is a bit unfortunate, but still
> better than what we have currently. Maybe it should be renamed with a
> - XEN_ prefix to avoid any clash?
xen-backend.h is supposed to be included by xen-related code only (it
barfs when /usr/include/xen isn't there), so it shouldn't clash in
theory. Well, I can prefix it nevertheless, better safe than sorry.
> - container_of would probably be useful in other parts of qemu, I guess
> it could be merged in qemu first?
Has qemu a nice place for that kind of helper macros?
> In your xen patch, since idle and GUI_REFRESH_INTERVAL are there, you
> can just use them. We'll push them to qemu. We'll manage the _shared
> stuff (and push it eventually).
I'd prefer to do it the other way around (push depending changes
upstream, then adapt xen-framebuffer.c). I want xen-framebuffer.c look
the same in xen and upstream.
> I'll let Markus comment on the rest (again, thanks for moving the xenbus
> stuff to the backend part). There is one change that is not backend
> changes or just moving code around: you are queuing rectangle updates,
> why? (I'm not arguing, just wondering the kind of optimization that can
> be).
Thats actually a bugfix. Doing screen updates outside the ->update
callback isn't safe.
And as we must queuing up update rectangles anyway to fix that it also
tries to optimize stuff a bit and avoid unneeded bitblits. Right now
that catches only frequent fullscreen updates (such as a scrolling
textconsole) and doesn't copy stuff multiple times in case the update
events from the guest are more frequent than update callback calls from
qemu.
Could be improved to analyze the rectangles in more detail (for example:
figure a update is a superset of another one, so one can be dropped).
>> The block backend should be a good replacement for blktap though and
>> maybe can save you the effort of porting the blktap kernel driver to
>> the pv_ops kernel.
>
> Well, for better performance an in-kernel blktap would still be useful.
Don't confuse blkback and blktap. I don't want to replace the in-kernel
blkback driver. blktap isn't a in-kernel driver though. It is just an
interface between the hypervisor and a userspace application,
specialized for block devices. My backend driver does pretty much the
same, but uses the generic gntdev interface instead of blktap. I can't
see any reason why it shouldn't match blktap performance-wise. I have
no benchmarks numbers though.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 12:13 ` Gerd Hoffmann
@ 2008-08-07 12:54 ` Samuel Thibault
2008-08-07 16:17 ` Gerd Hoffmann
2008-08-07 16:09 ` Samuel Thibault
` (2 subsequent siblings)
3 siblings, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-07 12:54 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel
Gerd Hoffmann, le Thu 07 Aug 2008 14:13:22 +0200, a écrit :
> > Any reason for the renames, though? (they tend to bother developpers who
> > have to change their habits, so we can not do that without a reason)
>
> Get consistent naming (all xen stuff in hw/ is prefixed with xen-).
Err, no, in xen they are all prefixed with xen_ (except xenfb).
> > - Why dashes instead of underscores?
>
> (1) It looks more pleasent to my eyes.
> (2) It is easier to type (no shift needed) on both us and german
> keyboards.
Not on fr keyboard :p
> (3) The files in the qemu source tree don't have a consistent style
> in respect to '-' vs. '_',
There are far more _ than - in qemu. - seems to be only used for
things that just share a very generic idea (i.e. usb- and scsi-), while
_ seems to be used for things that are more closely related, like arm_*,
mips_*, ppc_*, ... xen_* would make sense to my mind.
> so I had no reason to not use my personal preference ;)
Yes, there is a reason: as I said, that puts a little burden on
developpers that have already been working on it in Xen for some time.
That also asks Ian to do the move, that makes history digging more
tricky, etc.
> I'd prefer to stay with the xen-machine.c though.
Ok for your taste, but as I said the burden involved by renamings looks
more important to me.
> > Misc stuff:
> > - I guess the sys-queue.h file should be merged and used in qemu first?
>
> Rebase. Then it will be there, and you can skip the patch ;)
Ah ok, sorry, then fine.
> > - The BUFSIZE macro in xen-backend.h is a bit unfortunate, but still
> > better than what we have currently. Maybe it should be renamed with a
> > - XEN_ prefix to avoid any clash?
>
> xen-backend.h is supposed to be included by xen-related code only (it
> barfs when /usr/include/xen isn't there), so it shouldn't clash in
> theory.
In theory yes, but even people working on the xen part could want to
define a very local BUFSIZE, and then they could get a clash. Of course
they can easily fix their code, but as you say
> Well, I can prefix it nevertheless, better safe than sorry.
> > - container_of would probably be useful in other parts of qemu, I guess
> > it could be merged in qemu first?
>
> Has qemu a nice place for that kind of helper macros?
I'll leave that one for more experienced qemu people.
> > In your xen patch, since idle and GUI_REFRESH_INTERVAL are there, you
> > can just use them. We'll push them to qemu. We'll manage the _shared
> > stuff (and push it eventually).
>
> I'd prefer to do it the other way around (push depending changes
> upstream, then adapt xen-framebuffer.c). I want xen-framebuffer.c look
> the same in xen and upstream.
Sure, it's just that your code could be merged earlier with these
modifications. But well we can handle that later it's not a problem.
> > I'll let Markus comment on the rest (again, thanks for moving the xenbus
> > stuff to the backend part). There is one change that is not backend
> > changes or just moving code around: you are queuing rectangle updates,
> > why? (I'm not arguing, just wondering the kind of optimization that can
> > be).
>
> Thats actually a bugfix. Doing screen updates outside the ->update
> callback isn't safe.
Oh. Well, that's the kind of comments which you could have put
somewhere along your patches, for us to not frown on it while
reviewing...
For more performance, maybe it'd be better to only move the dpy_update()
part. It's better to do the xenfb_guest_copy() immediately since the
source data is probably already hot in the cache.
> >> The block backend should be a good replacement for blktap though and
> >> maybe can save you the effort of porting the blktap kernel driver to
> >> the pv_ops kernel.
> >
> > Well, for better performance an in-kernel blktap would still be useful.
>
> Don't confuse blkback and blktap. I don't want to replace the in-kernel
> blkback driver. blktap isn't a in-kernel driver though.
Well, actually _you_ confused me about this above :)
> My backend driver does pretty much the same, but uses the generic
> gntdev interface instead of blktap. I can't see any reason why it
> shouldn't match blktap performance-wise.
That's possible indeed.
> I have no benchmarks numbers though.
That'd be good :)
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 12:54 ` Samuel Thibault
@ 2008-08-07 16:17 ` Gerd Hoffmann
2008-08-07 16:48 ` Samuel Thibault
0 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-07 16:17 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Markus Armbruster,
Anthony Liguori, qemu-devel, xen-devel, Ian Jackson
Samuel Thibault wrote:
> Gerd Hoffmann, le Thu 07 Aug 2008 14:13:22 +0200, a écrit :
>>> Any reason for the renames, though? (they tend to bother developpers who
>>> have to change their habits, so we can not do that without a reason)
>> Get consistent naming (all xen stuff in hw/ is prefixed with xen-).
>
> Err, no, in xen they are all prefixed with xen_ (except xenfb).
Uhm, No.
~/xen/qemu-dm# grep ^OBJS xen-hooks.mak
OBJS += piix4acpi.o
OBJS += xenstore.o
OBJS += xen_platform.o
OBJS += xen_machine_fv.o
OBJS += xen_machine_fv.o
OBJS += xen_blktap.o
OBJS += exec-dm.o
OBJS += pci_emulation.o
OBJS += tpm_tis.o
OBJS+= pass-through.o pt-msi.o
OBJS := $(filter-out $(BAD_OBJS), $(OBJS))
That is neither consistent wrt using _ everythere nor all files are
prefixed consistently. At least all prefixed ones use underscores.
>> (3) The files in the qemu source tree don't have a consistent style
>> in respect to '-' vs. '_',
>
> There are far more _ than - in qemu. - seems to be only used for
> things that just share a very generic idea (i.e. usb- and scsi-), while
> _ seems to be used for things that are more closely related, like arm_*,
> mips_*, ppc_*, ... xen_* would make sense to my mind.
To me it looks pretty random, probably depending on the person creating
that file. And when you count them, then there is no clear winner:
~/projects/qemu# find -name "*.[ch]" -print | grep "-" | wc -l
293
~/projects/qemu# find -name "*.[ch]" -print | grep "_" | wc -l
231
>> so I had no reason to not use my personal preference ;)
>
> Yes, there is a reason: as I said, that puts a little burden on
> developpers that have already been working on it in Xen for some time.
> That also asks Ian to do the move, that makes history digging more
> tricky, etc.
git handles renames just fine.
> For more performance, maybe it'd be better to only move the dpy_update()
> part. It's better to do the xenfb_guest_copy() immediately since the
> source data is probably already hot in the cache.
No. The copy is unsafe.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 16:17 ` Gerd Hoffmann
@ 2008-08-07 16:48 ` Samuel Thibault
2008-08-07 16:54 ` Samuel Thibault
0 siblings, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-07 16:48 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel
Gerd Hoffmann, le Thu 07 Aug 2008 18:17:39 +0200, a écrit :
> Samuel Thibault wrote:
> > Gerd Hoffmann, le Thu 07 Aug 2008 14:13:22 +0200, a écrit :
> >>> Any reason for the renames, though? (they tend to bother developpers who
> >>> have to change their habits, so we can not do that without a reason)
> >> Get consistent naming (all xen stuff in hw/ is prefixed with xen-).
> >
> > Err, no, in xen they are all prefixed with xen_ (except xenfb).
>
> Uhm, No.
Right, there is xenstore as well.
> ~/xen/qemu-dm# grep ^OBJS xen-hooks.mak
> OBJS += piix4acpi.o
> [snip xen*]
> OBJS += exec-dm.o
> OBJS += pci_emulation.o
> OBJS += tpm_tis.o
> OBJS+= pass-through.o pt-msi.o
> OBJS := $(filter-out $(BAD_OBJS), $(OBJS))
These aren't really xen-specific, that's why they don't have a xen or
xen_ prefix.
> That is neither consistent wrt using _ everythere nor all files are
> prefixed consistently. At least all prefixed ones use underscores.
And that's my point. I don't see why we should take the burden of
renaming them with dashes.
> >> (3) The files in the qemu source tree don't have a consistent style
> >> in respect to '-' vs. '_',
> >
> > There are far more _ than - in qemu.
Just to comment on that. I actually meant in hw/ . There are a lot of -
in the root, because there are block-*, qemu-*, cpu-*, config-*, etc.
> - seems to be only used for
> > things that just share a very generic idea (i.e. usb- and scsi-), while
> > _ seems to be used for things that are more closely related, like arm_*,
> > mips_*, ppc_*, ... xen_* would make sense to my mind.
>
> To me it looks pretty random,
I doesn't look so much random to me. There are oddities, but the rule
above seems mostly respected.
> And when you count them, then there is no clear winner:
>
> ~/projects/qemu# find -name "*.[ch]" -print | grep "-" | wc -l
> 293
> ~/projects/qemu# find -name "*.[ch]" -print | grep "_" | wc -l
> 231
Sure, they have different purpose. As I said, _ for closely related
(like must be compiled together), - for not closely related (i.e.
independant matter that just have some generic link, like the block
interface, scsi or usb bus).
> >> so I had no reason to not use my personal preference ;)
> >
> > Yes, there is a reason: as I said, that puts a little burden on
> > developpers that have already been working on it in Xen for some time.
> > That also asks Ian to do the move, that makes history digging more
> > tricky, etc.
>
> git handles renames just fine.
Yes, sure, that's what I meant before ("having a renaming step first").
But that's still work to actually do it, change the Makefiles, and then
when you want to git annotate an old version it becomes tricky: you have
to remember the old name. So renaming really needs a reason.
> > For more performance, maybe it'd be better to only move the dpy_update()
> > part. It's better to do the xenfb_guest_copy() immediately since the
> > source data is probably already hot in the cache.
>
> No. The copy is unsafe.
Ah, because we're writing to ds->data which is handled by the display
backend, right.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 12:13 ` Gerd Hoffmann
2008-08-07 12:54 ` Samuel Thibault
@ 2008-08-07 16:09 ` Samuel Thibault
2008-08-07 16:34 ` Samuel Thibault
2008-08-11 10:12 ` Ian Jackson
3 siblings, 0 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-07 16:09 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel
Gerd Hoffmann, le Thu 07 Aug 2008 14:13:22 +0200, a écrit :
> > Or xen can keep its own xen_machine_{pv,fv}.c and your
> > xen-machine.c goes upstream, I don't think that would be a problem.
>
> We have two files defining xenpv_machine then. I don't think it is a
> good idea.
I guess Ian would just drop the xen_machine.c file in his tree.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 12:13 ` Gerd Hoffmann
2008-08-07 12:54 ` Samuel Thibault
2008-08-07 16:09 ` Samuel Thibault
@ 2008-08-07 16:34 ` Samuel Thibault
2008-08-11 8:16 ` Gerd Hoffmann
2008-08-11 10:12 ` Ian Jackson
3 siblings, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-07 16:34 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel
Gerd Hoffmann, le Thu 07 Aug 2008 14:13:22 +0200, a écrit :
> > In your xen patch, since idle and GUI_REFRESH_INTERVAL are there, you
> > can just use them. We'll push them to qemu.
>
> I'd prefer to do it the other way around (push depending changes
> upstream, then adapt xen-framebuffer.c). I want xen-framebuffer.c look
> the same in xen and upstream.
Mmm, looking at that again, most of my idleness patch is actually
already upstream. What wasn't pushed is what is specific to xenfb:
transmitting the idleness to the backend. For this xenfb needs to get
idleness information. Below is the patch that does that, you can just
fold it in your qemu-upstream/0008-xen-add-framebuffer-backend-driver.patch
About GUI_REFRESH_INTERVAL, you can just move it to a header.
Samuel
Index: console.h
===================================================================
--- console.h (révision 4992)
+++ console.h (copie de travail)
@@ -80,6 +80,7 @@
void *opaque;
struct QEMUTimer *gui_timer;
uint64_t gui_timer_interval;
+ int idle;
void (*dpy_update)(struct DisplayState *s, int x, int y, int w, int h);
void (*dpy_resize)(struct DisplayState *s, int w, int h);
Index: vl.c
===================================================================
--- vl.c (révision 4992)
+++ vl.c (copie de travail)
@@ -5930,6 +5930,8 @@
ds->dpy_update = dumb_update;
ds->dpy_resize = dumb_resize;
ds->dpy_refresh = dumb_refresh;
+ ds->gui_timer_interval = 500;
+ ds->idle = 1;
}
/***********************************************************/
Index: sdl.c
===================================================================
--- sdl.c (révision 4992)
+++ sdl.c (copie de travail)
@@ -526,9 +526,11 @@
if (ev->active.gain) {
/* Back to default interval */
ds->gui_timer_interval = 0;
+ ds->idle = 0;
} else {
/* Sleeping interval */
ds->gui_timer_interval = 500;
+ ds->idle = 1;
}
}
break;
Index: vnc.c
===================================================================
--- vnc.c (révision 4992)
+++ vnc.c (copie de travail)
@@ -660,6 +660,7 @@ client_io_error
qemu_set_fd_handler2(vs->csock, NULL, NULL, NULL, NULL);
closesocket(vs->csock);
vs->csock = -1;
+ vs->ds->idle = 1;
buffer_reset(&vs->input);
buffer_reset(&vs->output);
vs->need_update = 0;
@@ -1920,6 +1921,7 @@
static void vnc_connect(VncState *vs)
{
VNC_DEBUG("New client on socket %d\n", vs->csock);
+ vs->ds->idle = 0;
socket_set_nonblock(vs->csock);
qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
vnc_write(vs, "RFB 003.008\n", 12);
@@ -1959,6 +1961,7 @@
exit(1);
ds->opaque = vs;
+ ds->idle = 1;
vnc_state = vs;
vs->display = NULL;
vs->password = NULL;
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 16:34 ` Samuel Thibault
@ 2008-08-11 8:16 ` Gerd Hoffmann
2008-08-11 9:23 ` Stefano Stabellini
0 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-11 8:16 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Markus Armbruster,
Anthony Liguori, qemu-devel, xen-devel, Ian Jackson
Hi,
> Mmm, looking at that again, most of my idleness patch is actually
> already upstream. What wasn't pushed is what is specific to xenfb:
> transmitting the idleness to the backend. For this xenfb needs to get
> idleness information. Below is the patch that does that, you can just
> fold it in your qemu-upstream/0008-xen-add-framebuffer-backend-driver.patch
> About GUI_REFRESH_INTERVAL, you can just move it to a header.
Thanks, I'll stick it into my patch queue.
That leaves the DisplayState->shared_buf bits which should be upstreamed
too ...
cheers,
Gerd
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-11 8:16 ` Gerd Hoffmann
@ 2008-08-11 9:23 ` Stefano Stabellini
0 siblings, 0 replies; 92+ messages in thread
From: Stefano Stabellini @ 2008-08-11 9:23 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: xen-devel, Ian Jackson, qemu-devel, Markus Armbruster,
Samuel Thibault
Gerd Hoffmann wrote:
> Hi,
>
>> Mmm, looking at that again, most of my idleness patch is actually
>> already upstream. What wasn't pushed is what is specific to xenfb:
>> transmitting the idleness to the backend. For this xenfb needs to get
>> idleness information. Below is the patch that does that, you can just
>> fold it in your qemu-upstream/0008-xen-add-framebuffer-backend-driver.patch
>> About GUI_REFRESH_INTERVAL, you can just move it to a header.
>
> Thanks, I'll stick it into my patch queue.
>
> That leaves the DisplayState->shared_buf bits which should be upstreamed
> too ...
>
It is the next item on my TODO :)
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 12:13 ` Gerd Hoffmann
` (2 preceding siblings ...)
2008-08-07 16:34 ` Samuel Thibault
@ 2008-08-11 10:12 ` Ian Jackson
3 siblings, 0 replies; 92+ messages in thread
From: Ian Jackson @ 2008-08-11 10:12 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, xen-devel, Markus Armbruster, Samuel Thibault
Gerd Hoffmann writes ("Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu"):
> [Samuel asks:]
> > - Why dashes instead of underscores?
>
> (1) It looks more pleasent to my eyes.
> (2) It is easier to type (no shift needed) on both us and german
> keyboards.
> (3) The files in the qemu source tree don't have a consistent style
> in respect to '-' vs. '_', so I had no reason to not use my
> personal preference ;)
I basically agree with this. However:
> > Any reason for the renames, though? (they tend to bother developpers who
> > have to change their habits, so we can not do that without a reason)
>
> Get consistent naming (all xen stuff in hw/ is prefixed with xen-).
I absolutely and vehemently disagree that files should be renamed (or
indeed code style changed) to make things prettier or more consistent.
There are currently at least three substantial branches and numerous
smaller branches of this xen code in at least two different revision
control systems - and there are probably many more.
The very last thing we want is to make porting patches between the
various branches difficult.
Ian.
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 7:33 ` Gerd Hoffmann
2008-08-07 10:53 ` Samuel Thibault
@ 2008-08-07 15:06 ` Blue Swirl
2008-08-07 15:13 ` Samuel Thibault
` (3 more replies)
1 sibling, 4 replies; 92+ messages in thread
From: Blue Swirl @ 2008-08-07 15:06 UTC (permalink / raw)
To: qemu-devel
Cc: xen-devel, Ian Jackson, Markus Armbruster, Gerd Hoffmann,
Samuel Thibault
On 8/7/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Samuel Thibault wrote:
> > Samuel Thibault, le Wed 06 Aug 2008 15:25:26 +0100, a écrit :
> >> Pushing the cleaning changes to Xen first can be done and would entail
> >> much easier to tackle breakage, and the merge back from qemu would then
> >> be trivial, why not doing so?
> >
> > You didn't answer that part. Really, my only concern is about having
> > things tested. Isn't it possible for instance to just merge the backend
> > core (and console/xenfb updates) in Ian's tree first for a start?
>
>
> http://kraxel.fedorapeople.org/patches/qemu-xen/
>
> I didn't touch the build system, it is even more scary than the qemu one
> alone, I just set CONFIG_XEN unconditionally.
>
> I also largely left vl.c as-is, so xend shouldn't need any changes. The
> -domid switch sets an additional (redundant) variable, to keep the
> amount of changes as small as possible for now. Also -name and
> -domain-name are aliased, both set qemu_name and domain_name.
0004-xenpv-groundwork.patch
You dropped nodisk_ok support from vl.c.
0005-xen-backend-core.patch
container_of could go to osdep.h.
0006-xen-console-backend.patch
Not your fault, but a lot of places (at least ps2.c and
slavio_serial.c) define some kind of ring buffers. Maybe these could
be consolidated some time.
0007-xen-framebuffer-backend.patch
After you optimized scancode2linux[], it looks like
atkbd_set2_keycode[] and atkbd_unxlate_table[] are not needed.
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 15:06 ` Blue Swirl
@ 2008-08-07 15:13 ` Samuel Thibault
2008-08-07 15:13 ` Samuel Thibault
` (2 subsequent siblings)
3 siblings, 0 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-07 15:13 UTC (permalink / raw)
To: Blue Swirl
Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel,
Gerd Hoffmann
Blue Swirl, le Thu 07 Aug 2008 18:06:07 +0300, a écrit :
> After you optimized scancode2linux[], it looks like
> atkbd_set2_keycode[] and atkbd_unxlate_table[] are not needed.
They should however probably be kept as comments just to know where
scancode2linux comes from.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 15:06 ` Blue Swirl
2008-08-07 15:13 ` Samuel Thibault
@ 2008-08-07 15:13 ` Samuel Thibault
2008-08-07 15:21 ` Gerd Hoffmann
2008-08-08 11:03 ` Samuel Thibault
3 siblings, 0 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-07 15:13 UTC (permalink / raw)
To: Blue Swirl
Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel,
Gerd Hoffmann
Blue Swirl, le Thu 07 Aug 2008 18:06:07 +0300, a écrit :
> 0004-xenpv-groundwork.patch
>
> You dropped nodisk_ok support from vl.c.
I guess that's ok for a merge in Ian's tree, which probably has it
already in another form.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 15:06 ` Blue Swirl
2008-08-07 15:13 ` Samuel Thibault
2008-08-07 15:13 ` Samuel Thibault
@ 2008-08-07 15:21 ` Gerd Hoffmann
2008-08-08 15:24 ` Blue Swirl
2008-08-08 11:03 ` Samuel Thibault
3 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-07 15:21 UTC (permalink / raw)
To: Blue Swirl
Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel,
Samuel Thibault
Blue Swirl wrote:
>> http://kraxel.fedorapeople.org/patches/qemu-xen/
> 0004-xenpv-groundwork.patch
>
> You dropped nodisk_ok support from vl.c.
Oh, there are *two* patchsets:
http://kraxel.fedorapeople.org/patches/qemu-upstream/
http://kraxel.fedorapeople.org/patches/qemu-xen/
The first also includes some not-yet submitted work-in-progress bits.
It is against upstream qemu svn. The second is against xen's qemu fork,
so the xen-related changes can get a test-drive there.
Especially the vl.c changes are very different in the two patch sets
because xen's vl.c looks very different (lot of CONFIG_DM conditional
code) and because I don't want to break the command line interface for
now to not break xend's expectations.
That doesn't mean that the nodisk_ok got lost. It is only in the first
patchset though.
> 0005-xen-backend-core.patch
>
> container_of could go to osdep.h.
Will do.
> 0006-xen-console-backend.patch
>
> Not your fault, but a lot of places (at least ps2.c and
> slavio_serial.c) define some kind of ring buffers. Maybe these could
> be consolidated some time.
Will have a look if I find some time.
> 0007-xen-framebuffer-backend.patch
>
> After you optimized scancode2linux[], it looks like
> atkbd_set2_keycode[] and atkbd_unxlate_table[] are not needed.
Indeed. Think I just ifdef them out and leave them in there as
documentation.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 15:21 ` Gerd Hoffmann
@ 2008-08-08 15:24 ` Blue Swirl
2008-08-11 12:46 ` Gerd Hoffmann
0 siblings, 1 reply; 92+ messages in thread
From: Blue Swirl @ 2008-08-08 15:24 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel,
Samuel Thibault
[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]
On 8/7/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Blue Swirl wrote:
> >> http://kraxel.fedorapeople.org/patches/qemu-xen/
>
>
> > 0004-xenpv-groundwork.patch
> >
> > You dropped nodisk_ok support from vl.c.
>
>
> Oh, there are *two* patchsets:
> http://kraxel.fedorapeople.org/patches/qemu-upstream/
>
> http://kraxel.fedorapeople.org/patches/qemu-xen/
>
>
> The first also includes some not-yet submitted work-in-progress bits.
> It is against upstream qemu svn. The second is against xen's qemu fork,
> so the xen-related changes can get a test-drive there.
>
> Especially the vl.c changes are very different in the two patch sets
> because xen's vl.c looks very different (lot of CONFIG_DM conditional
> code) and because I don't want to break the command line interface for
> now to not break xend's expectations.
>
> That doesn't mean that the nodisk_ok got lost. It is only in the first
> patchset though.
I extracted a small patch that only introduces the nodisk_ok part.
Anybody unhappy if I commit it?
[-- Attachment #2: nodisk.diff --]
[-- Type: plain/text, Size: 3351 bytes --]
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-08 15:24 ` Blue Swirl
@ 2008-08-11 12:46 ` Gerd Hoffmann
2008-08-11 18:53 ` Blue Swirl
0 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-11 12:46 UTC (permalink / raw)
To: Blue Swirl
Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel,
Samuel Thibault
Blue Swirl wrote:
> I extracted a small patch that only introduces the nodisk_ok part.
> Anybody unhappy if I commit it?
How about converting all struct fields to C99 initializers in
hw/sun4*.c? Mixing the two styles looks quite odd.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-11 12:46 ` Gerd Hoffmann
@ 2008-08-11 18:53 ` Blue Swirl
0 siblings, 0 replies; 92+ messages in thread
From: Blue Swirl @ 2008-08-11 18:53 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel,
Samuel Thibault
On 8/11/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Blue Swirl wrote:
> > I extracted a small patch that only introduces the nodisk_ok part.
> > Anybody unhappy if I commit it?
>
>
> How about converting all struct fields to C99 initializers in
> hw/sun4*.c? Mixing the two styles looks quite odd.
Good idea, I'll do the switch.
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-07 15:06 ` Blue Swirl
` (2 preceding siblings ...)
2008-08-07 15:21 ` Gerd Hoffmann
@ 2008-08-08 11:03 ` Samuel Thibault
3 siblings, 0 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-08 11:03 UTC (permalink / raw)
To: Blue Swirl
Cc: xen-devel, Ian Jackson, Markus Armbruster, qemu-devel,
Gerd Hoffmann
Blue Swirl, le Thu 07 Aug 2008 18:06:07 +0300, a écrit :
> 0006-xen-console-backend.patch
>
> Not your fault, but a lot of places (at least ps2.c and
> slavio_serial.c) define some kind of ring buffers. Maybe these could
> be consolidated some time.
There is also one in console.c
It'd be probably be good to have an implementation of a ring of
arbitrary structures. We have a pending patch of usb-hid to fix double
clicks by queueing click events.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 13:54 ` Gerd Hoffmann
2008-08-06 14:01 ` Samuel Thibault
@ 2008-08-07 17:40 ` Stefano Stabellini
2008-08-11 9:18 ` Ian Jackson
2 siblings, 0 replies; 92+ messages in thread
From: Stefano Stabellini @ 2008-08-07 17:40 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: xen-devel, Ian Jackson, qemu-devel, Markus Armbruster,
Samuel Thibault
Gerd Hoffmann wrote:
> It would also be great if you can clean up the header mess. qemu-dm
> doesn't build without xen-unstable.hg. And just a checked out tree
> isn't good enougth, it must be compiled because qemu-dm depends on some
> generated header files. Aiee. IMHO qemu-dm should only need
> /usr/include/xen. If headers are missing there, they should be installed.
>
It is not difficult to make Ian's qemu tree compile against
/usr/include/xen.
Expect a patch to fix this on Monday.
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 13:54 ` Gerd Hoffmann
2008-08-06 14:01 ` Samuel Thibault
2008-08-07 17:40 ` Stefano Stabellini
@ 2008-08-11 9:18 ` Ian Jackson
2008-08-11 11:08 ` Gerd Hoffmann
2 siblings, 1 reply; 92+ messages in thread
From: Ian Jackson @ 2008-08-11 9:18 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, xen-devel, Markus Armbruster, Samuel Thibault
Gerd Hoffmann writes ("Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu"):
> IMHO the best way to address this issue is to rebase frequently. The
> xen-unstable testing will also covers qemu then. It also keeps the
> delta smaller and makes it easier to test qemu patches in qemu-dm.
I'm not going to be rebasing, but I will merge instead.
> I'm trying to fiddle my bits into qemu-dm right now. One problem is
> that I get build failures simply because of the fact that the qemu-dm
> base is old compared to upstream qemu.
Yes, that's currently the case because qemu-xen-unstable is frozen for
the forthcoming Xen release.
> It would also be great if you can clean up the header mess. qemu-dm
> doesn't build without xen-unstable.hg.
This is true and is likely to continue to be the case indefinitely.
> And just a checked out tree
> isn't good enougth, it must be compiled because qemu-dm depends on some
> generated header files. Aiee. IMHO qemu-dm should only need
> /usr/include/xen. If headers are missing there, they should be installed.
qemu-dm also needs libxc, libthis, libthat from xen-unstable.
Ian.
^ permalink raw reply [flat|nested] 92+ messages in thread* Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-11 9:18 ` Ian Jackson
@ 2008-08-11 11:08 ` Gerd Hoffmann
0 siblings, 0 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-11 11:08 UTC (permalink / raw)
To: Ian Jackson; +Cc: qemu-devel, xen-devel, Markus Armbruster, Samuel Thibault
Ian Jackson wrote:
>> And just a checked out tree
>> isn't good enougth, it must be compiled because qemu-dm depends on some
>> generated header files. Aiee. IMHO qemu-dm should only need
>> /usr/include/xen. If headers are missing there, they should be installed.
>
> qemu-dm also needs libxc, libthis, libthat from xen-unstable.
That kind of stuff is usually packed up as xen-devel in linux
distributions. I want being able to just install xen-devel on my
machine, then compile qemu with xen support.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 10:23 ` Samuel Thibault
2008-08-06 12:43 ` [Xen-devel] " Markus Armbruster
@ 2008-08-06 13:24 ` Gerd Hoffmann
2008-08-06 13:39 ` Samuel Thibault
1 sibling, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-06 13:24 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Anthony Liguori, qemu-devel,
xen-devel, Ian Jackson
Hi,
> You forgot the _test_ stage. You are basically asking us to replace our
> well-tested implementation with your implementation, that's quite a
> move.
Sure, and more will follow. I'd expect the merge involves rewriting
large parts of the code anyway, especially the hvm bits due to the
nessesary interface cleanups in qemu, but also because the code must
pass the review process before it can be merged. Pretty much like
upstreaming the xen support into the linux kernel. All that also needs
testing, no doubt.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 13:24 ` Gerd Hoffmann
@ 2008-08-06 13:39 ` Samuel Thibault
2008-08-06 14:18 ` Gerd Hoffmann
0 siblings, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-06 13:39 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, qemu-devel
Gerd Hoffmann, le Wed 06 Aug 2008 15:24:39 +0200, a écrit :
> > You forgot the _test_ stage. You are basically asking us to replace our
> > well-tested implementation with your implementation, that's quite a
> > move.
>
> Sure, and more will follow.
But by having it merged into just upstream qemu you won't magically get
Xen people actually test it, so it would be mostly dead code.
> Pretty much like upstreaming the xen support into the linux kernel.
There's a big difference here. The upstream linux kernel merge won't be
merged back to the linux-2.6.18 tree. And people do test upstream xen
support.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 13:39 ` Samuel Thibault
@ 2008-08-06 14:18 ` Gerd Hoffmann
2008-08-06 14:51 ` Samuel Thibault
0 siblings, 1 reply; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-06 14:18 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Anthony Liguori, qemu-devel,
xen-devel, Ian Jackson
Samuel Thibault wrote:
> Gerd Hoffmann, le Wed 06 Aug 2008 15:24:39 +0200, a écrit :
>>> You forgot the _test_ stage. You are basically asking us to replace our
>>> well-tested implementation with your implementation, that's quite a
>>> move.
>> Sure, and more will follow.
>
> But by having it merged into just upstream qemu you won't magically get
> Xen people actually test it, so it would be mostly dead code.
>
>> Pretty much like upstreaming the xen support into the linux kernel.
>
> There's a big difference here. The upstream linux kernel merge won't be
> merged back to the linux-2.6.18 tree.
Oh, we can also arrange things without merging back. Stop rebasing
qemu-dm (pretty much like 2.6.18), additionally hook upstream qemu as-is
into testing (like pv_ops kernels). Then start submitting patches to
qemu. When done, zap old qemu-dm.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 14:18 ` Gerd Hoffmann
@ 2008-08-06 14:51 ` Samuel Thibault
2008-08-06 15:25 ` Gerd Hoffmann
0 siblings, 1 reply; 92+ messages in thread
From: Samuel Thibault @ 2008-08-06 14:51 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: xen-devel, Ian Jackson, qemu-devel
Gerd Hoffmann, le Wed 06 Aug 2008 16:18:51 +0200, a écrit :
> Samuel Thibault wrote:
> > But by having it merged into just upstream qemu you won't magically get
> > Xen people actually test it, so it would be mostly dead code.
> >
> >> Pretty much like upstreaming the xen support into the linux kernel.
> >
> > There's a big difference here. The upstream linux kernel merge won't be
> > merged back to the linux-2.6.18 tree.
>
> Oh, we can also arrange things without merging back. Stop rebasing
> qemu-dm (pretty much like 2.6.18), additionally hook upstream qemu as-is
> into testing (like pv_ops kernels). Then start submitting patches to
> qemu. When done, zap old qemu-dm.
Again, there's a difference. The interface between Linux and Xen has
always been meant to be a "public" thing. The interface between ioemu
and the other xen components has not, and changing it is considered fine
(we still do it). Sure that could be cleaned up, but that's probably
not something that should happen on the upstream qemu side.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
2008-08-06 14:51 ` Samuel Thibault
@ 2008-08-06 15:25 ` Gerd Hoffmann
0 siblings, 0 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-06 15:25 UTC (permalink / raw)
To: Samuel Thibault, Gerd Hoffmann, Anthony Liguori, qemu-devel,
xen-devel, Ian Jackson
Samuel Thibault wrote:
>> Oh, we can also arrange things without merging back. Stop rebasing
>> qemu-dm (pretty much like 2.6.18), additionally hook upstream qemu as-is
>> into testing (like pv_ops kernels). Then start submitting patches to
>> qemu. When done, zap old qemu-dm.
>
> Again, there's a difference. The interface between Linux and Xen has
> always been meant to be a "public" thing. The interface between ioemu
> and the other xen components has not, and changing it is considered fine
> (we still do it). Sure that could be cleaned up, but that's probably
> not something that should happen on the upstream qemu side.
Hmm. That makes me think that starting over from scratch really is the
best option. Drop the old cruft, design a clean interface, code it up,
discuss it in public and submit to upstream qemu.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread
* [Qemu-devel] Xen's qemu branches, etc.
2008-08-04 17:42 ` [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu Anthony Liguori
2008-08-05 9:58 ` Gerd Hoffmann
2008-08-05 10:46 ` Samuel Thibault
@ 2008-08-11 16:36 ` Ian Jackson
2008-08-11 16:48 ` [Qemu-devel] Re: [Xen-devel] " Samuel Thibault
` (2 more replies)
2 siblings, 3 replies; 92+ messages in thread
From: Ian Jackson @ 2008-08-11 16:36 UTC (permalink / raw)
To: Anthony Liguori; +Cc: xen-devel, qemu-devel
I thought that following that lovely discussion it might be helpful to
summarise a few things about the way I see things:
As Samuel says, I'm currently maintaining two branches: one contains
code which I would like to get to upstream qemu in the nearish future,
and one which contains changes which I don't think have a practical
chance of going upstream any time soon. (I also have a gaggle of
topic branches of course.)
Because Xen 3.3 is nearing release and currently firmly frozen, both
of these branches are currently based on a few months' old version of
qemu upstream. I intend to fork off new development branches of both
very soon, and then bring both of them up to parity with a modern qemu
upstream. One natural output of such a merge process is a number of
patches for upstream.
Once that merge is done I intend, while Xen development is fully open,
to frequently update both my new trees from upstream - say, weekly.
As before, shortly before the Xen freeze I will stop pulling from qemu
upstream.
It would be very nice to get the differences between qemu in Xen and
qemu upstream down to zero. There are three kinds of change in our
tree which will prevent this in the short to medium term:
Firstly, there are some things in our tree which are simply too ugly.
We would like to improve them and feed them upstream in due course,
but that's more of a medium to long term project. A top priority in
this area is display handling.
Secondly, there are changes which are only useful in a Xen context.
An example would be the machinery to monitor `disk change' events
which are injected via the Xen management tools. This is done in our
qemu by xen-specific code which talks to the xenstore IPC/IDC daemon.
This code is completely Xen specific and depends on the presence of
the Xen management stack, so it must only be compiled in to the
specific Xen device model qemu executable. We would be happy to have
this code in qemu upstream, but it is quite closely linked to much of
the Xen management toolstack and quite loosely coupled to the rest of
qemu so in practice this is not currently a priority for us.
Thirdly, there are some changes which we have been told are not
suitable for qemu upstream, but which we (Xen) are pretty convinced we
need to retain. An example is the mapcache. (Please don't explain
again why the mapcache is not necessary. That conversation has been
had and we've come to different conclusions, which is fine - this is
Free Software after all, which means that we can work together well,
and hopefully without rancour, even though we sometimes disagree.)
There are also another set of concievable things: _emulations_ in qemu
of the Xen environment, so that a Xen PV guest can be run without the
Xen hypervisor and host infrastructure, or so that Xen PV drivers can
be used to improve the performance of a plain qemu guest.
That includes, for example, xenstore/xenbus emulation, Xen PV backend
drivers, and the like. Gerd's patches contained several such things.
I think a qemu emulator which can support Xen guests without needing
Xen is in principle a fine and good thing to have in qemu upstream.
We don't have it in our Xen tree because we have the real hypervisor
and dom0-provided backends. But I'm sure we'd find them useful for
debugging.
BUT we have to not get confused so the names must be different.
It is necessary to clearly distinguish these two use cases:
* qemu-dm run under the Xen hypervisor, xend, with xenstore, to
support Xen guests as currently done in the Xen upstream project
* qemu emulating the Xen environment
I think these should be given different names. We should not use
`xen' for both.
The plumbing involved is quite different. Users will get confused and
the results will be mysterious failures. Developers will get confused
and the results will be people talking and coding past each other.
I would like to suggest that the first of these two options should be
called `xen' or `xen-hvm' or `xen-pv' or `xen-fv' or some such.
Code in qemu intended to support running under the Xen hypervisor and
upstream Xen toolstack should use the name `xen'. This includes (for
example) portability to the Xen minios embedded system, frontend
drivers for Xen PV block and network devices, interaction with
xenstore, interaction with the hypervisor to collect IO requests from
the running guest and return the responses, and so on.
Code in qemu intended to support Xen guests without the use of the Xen
hypervisor and tools should be called `xenemu' or some such. This
includes backend drivers for block and network devices,
xenstore/xenbus emulation, and so on.
Ian.
^ permalink raw reply [flat|nested] 92+ messages in thread* [Qemu-devel] Re: [Xen-devel] Xen's qemu branches, etc.
2008-08-11 16:36 ` [Qemu-devel] Xen's qemu branches, etc Ian Jackson
@ 2008-08-11 16:48 ` Samuel Thibault
2008-08-11 19:17 ` [Qemu-devel] " Anthony Liguori
2008-08-11 19:34 ` [Qemu-devel] " Gerd Hoffmann
2 siblings, 0 replies; 92+ messages in thread
From: Samuel Thibault @ 2008-08-11 16:48 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, qemu-devel
Ian Jackson, le Mon 11 Aug 2008 17:36:22 +0100, a écrit :
> Code in qemu intended to support Xen guests without the use of the Xen
> hypervisor and tools should be called `xenemu' or some such. This
> includes backend drivers for block and network devices,
> xenstore/xenbus emulation, and so on.
Mmm, for backends it would be a bit odd, since in xen upstream we do
have backends too (console and framebuffer), and as Gerd said we could
in theory use the block and network backends too.
Samuel
^ permalink raw reply [flat|nested] 92+ messages in thread
* [Qemu-devel] Re: Xen's qemu branches, etc.
2008-08-11 16:36 ` [Qemu-devel] Xen's qemu branches, etc Ian Jackson
2008-08-11 16:48 ` [Qemu-devel] Re: [Xen-devel] " Samuel Thibault
@ 2008-08-11 19:17 ` Anthony Liguori
2008-08-11 19:34 ` [Qemu-devel] " Gerd Hoffmann
2 siblings, 0 replies; 92+ messages in thread
From: Anthony Liguori @ 2008-08-11 19:17 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, qemu-devel
Hi Ian,
Ian Jackson wrote:
> Because Xen 3.3 is nearing release and currently firmly frozen, both
> of these branches are currently based on a few months' old version of
> qemu upstream. I intend to fork off new development branches of both
> very soon, and then bring both of them up to parity with a modern qemu
> upstream. One natural output of such a merge process is a number of
> patches for upstream.
>
I'm looking forward to this and appreciate your work here.
> It would be very nice to get the differences between qemu in Xen and
> qemu upstream down to zero. There are three kinds of change in our
> tree which will prevent this in the short to medium term:
>
> Firstly, there are some things in our tree which are simply too ugly.
> We would like to improve them and feed them upstream in due course,
> but that's more of a medium to long term project. A top priority in
> this area is display handling.
>
> Secondly, there are changes which are only useful in a Xen context.
> An example would be the machinery to monitor `disk change' events
> which are injected via the Xen management tools. This is done in our
> qemu by xen-specific code which talks to the xenstore IPC/IDC daemon.
> This code is completely Xen specific and depends on the presence of
> the Xen management stack, so it must only be compiled in to the
> specific Xen device model qemu executable. We would be happy to have
> this code in qemu upstream, but it is quite closely linked to much of
> the Xen management toolstack and quite loosely coupled to the rest of
> qemu so in practice this is not currently a priority for us.
>
I think that there is room to discuss these sort of things. I think
most of these things will actually fall into the first category where we
could add the same mechanism to QEMU (perhaps via the monitor), and then
Xen could have a small shim that interacted with both XenStore and the
QEMU monitor. But really, this isn't the low hanging fruit so we can
cross this bridge when we come to it :-)
> There are also another set of concievable things: _emulations_ in qemu
> of the Xen environment, so that a Xen PV guest can be run without the
> Xen hypervisor and host infrastructure, or so that Xen PV drivers can
> be used to improve the performance of a plain qemu guest.
>
> That includes, for example, xenstore/xenbus emulation, Xen PV backend
> drivers, and the like. Gerd's patches contained several such things.
>
The patch series that Gerd posted has nothing to do with a Xen shim or
emulation layer. The PV backends are directly usable by Xen. Now if
ya'll want nothing to do with these backends, then I'm inclined to wait
to add those backends until they are being used by something before
pulling it into QEMU. There's no point in adding code to QEMU that
noone is actually going to use.
So that may mean waiting to pull in the userspace net/block backends
until Xenner can be merged into QEMU. That seems fine to me.
> I think a qemu emulator which can support Xen guests without needing
> Xen is in principle a fine and good thing to have in qemu upstream.
> We don't have it in our Xen tree because we have the real hypervisor
> and dom0-provided backends. But I'm sure we'd find them useful for
> debugging.
>
> BUT we have to not get confused so the names must be different.
>
So far, I haven't seen patches on qemu-devel that are for a Xen
shim/emulation layer so that doesn't seem like a problem to me. I
assume anything related to the shim/emulation layer will be clearly
identified as 'Xenner' anyway.
> It is necessary to clearly distinguish these two use cases:
>
> * qemu-dm run under the Xen hypervisor, xend, with xenstore, to
> support Xen guests as currently done in the Xen upstream project
>
> * qemu emulating the Xen environment
>
> I think these should be given different names. We should not use
> `xen' for both.
>
To me, this isn't a big issue. Whether we have one or two machine types
will be determined by the implementations. What I want to see is as
much code sharing as possible for both cases.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 92+ messages in thread
* Re: [Qemu-devel] Xen's qemu branches, etc.
2008-08-11 16:36 ` [Qemu-devel] Xen's qemu branches, etc Ian Jackson
2008-08-11 16:48 ` [Qemu-devel] Re: [Xen-devel] " Samuel Thibault
2008-08-11 19:17 ` [Qemu-devel] " Anthony Liguori
@ 2008-08-11 19:34 ` Gerd Hoffmann
2 siblings, 0 replies; 92+ messages in thread
From: Gerd Hoffmann @ 2008-08-11 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: xen-devel
Hi,
> Code in qemu intended to support Xen guests without the use of the Xen
> hypervisor and tools should be called `xenemu' or some such.
Sure, right now I have the files and functions prefixed with "xenner".
> This
> includes backend drivers for block and network devices,
> xenstore/xenbus emulation, and so on.
Yes, for xenstore/xenbus/... emulation.
No for the backends. They run on xen just fine too.
Additionally you can run xen domains *with* the xen hypervisor but
*without* the management stack.
cheers,
Gerd
--
http://kraxel.fedorapeople.org/xenner/
^ permalink raw reply [flat|nested] 92+ messages in thread