qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
@ 2014-01-23 22:16 Wei Liu
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 1/5] xen: move Xen PV machine files to hw/xenpv Wei Liu
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Wei Liu @ 2014-01-23 22:16 UTC (permalink / raw)
  To: xen-devel, qemu-devel
  Cc: anthony.perard, peter.maydell, Wei Liu, stefano.stabellini

As promised I hacked a prototype based on Paolo's disable TCG series.
However I coded some stubs for TCG anyway. So this series in principle
should work with / without Paolo's series.

The first 3 patches refactor some code to disentangle Xen PV and HVM
guest. The 4th patch has the real meat. It introduces Xen PV target,
which contains basically a dummy CPU, then hooks up this Xen PV CPU to
QEMU internal structures.

The last patch introduces xenpv-softmmu, which contains *no* emulation
code. I know that in previous discussion people said that every device
emulation should be included if the target architecture is called null.
But since this target CPU is now called xenpv I don't feel obliged to
include any device emulation in this prototype anymore. :-)

Please note that the existing Xen QEMU build is not affected at all. You
can still use "--disable-tcg --enable-xen --target-list=i386-softmmu"
(or x86_64-softmmu") to build qemu-system-{i386,x86_64} and use it for
both HVM and PV guest. This series adds another option to build QEMU with
"--disable-tcg --enable-xen --target-list=xenpv-softmmu" and get a QEMU
binary tailored for Xen PV guest. The effect is that we reduce the
binary size from 14MB to 7.3MB.

What do you think of this idea? I'm all ears.

Wei.

Wei Liu (5):
  xen: move Xen PV machine files to hw/xenpv
  xen: factor out common functions
  exec: guard Xen HVM hooks with CONFIG_XEN_I386
  xen: implement Xen PV target
  xen: introduce xenpv-softmmu.mak

 Makefile.target                      |    3 +-
 arch_init.c                          |    2 +
 configure                            |   12 ++-
 cpu-exec.c                           |    2 +
 default-configs/i386-softmmu.mak     |    1 -
 default-configs/x86_64-softmmu.mak   |    1 -
 default-configs/xenpv-softmmu.mak    |    2 +
 exec.c                               |   16 ++++
 hw/i386/Makefile.objs                |    2 +-
 hw/xenpv/Makefile.objs               |    2 +
 hw/{i386 => xenpv}/xen_domainbuild.c |    0
 hw/{i386 => xenpv}/xen_domainbuild.h |    0
 hw/{i386 => xenpv}/xen_machine_pv.c  |    0
 include/exec/memory-internal.h       |    2 +
 include/sysemu/arch_init.h           |    1 +
 target-xenpv/Makefile.objs           |    1 +
 target-xenpv/cpu-qom.h               |   64 ++++++++++++++++
 target-xenpv/cpu.h                   |   66 ++++++++++++++++
 target-xenpv/helper.c                |   32 ++++++++
 target-xenpv/translate.c             |   27 +++++++
 xen-common.c                         |  137 ++++++++++++++++++++++++++++++++++
 xen-all.c => xen-hvm.c               |  112 +--------------------------
 xen-stub.c                           |    4 -
 23 files changed, 368 insertions(+), 121 deletions(-)
 create mode 100644 default-configs/xenpv-softmmu.mak
 create mode 100644 hw/xenpv/Makefile.objs
 rename hw/{i386 => xenpv}/xen_domainbuild.c (100%)
 rename hw/{i386 => xenpv}/xen_domainbuild.h (100%)
 rename hw/{i386 => xenpv}/xen_machine_pv.c (100%)
 create mode 100644 target-xenpv/Makefile.objs
 create mode 100644 target-xenpv/cpu-qom.h
 create mode 100644 target-xenpv/cpu.h
 create mode 100644 target-xenpv/helper.c
 create mode 100644 target-xenpv/helper.h
 create mode 100644 target-xenpv/translate.c
 create mode 100644 xen-common.c
 rename xen-all.c => xen-hvm.c (92%)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH RFC 1/5] xen: move Xen PV machine files to hw/xenpv
  2014-01-23 22:16 [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Wei Liu
@ 2014-01-23 22:16 ` Wei Liu
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 2/5] xen: factor out common functions Wei Liu
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Wei Liu @ 2014-01-23 22:16 UTC (permalink / raw)
  To: xen-devel, qemu-devel
  Cc: anthony.perard, peter.maydell, Wei Liu, stefano.stabellini

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 hw/i386/Makefile.objs                |    2 +-
 hw/xenpv/Makefile.objs               |    2 ++
 hw/{i386 => xenpv}/xen_domainbuild.c |    0
 hw/{i386 => xenpv}/xen_domainbuild.h |    0
 hw/{i386 => xenpv}/xen_machine_pv.c  |    0
 5 files changed, 3 insertions(+), 1 deletion(-)
 create mode 100644 hw/xenpv/Makefile.objs
 rename hw/{i386 => xenpv}/xen_domainbuild.c (100%)
 rename hw/{i386 => xenpv}/xen_domainbuild.h (100%)
 rename hw/{i386 => xenpv}/xen_machine_pv.c (100%)

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 09ac433..0faccd7 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -2,7 +2,7 @@ obj-$(CONFIG_KVM) += kvm/
 obj-y += multiboot.o smbios.o
 obj-y += pc.o pc_piix.o pc_q35.o
 obj-y += pc_sysfw.o
-obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
+obj-$(CONFIG_XEN) += ../xenpv/
 
 obj-y += kvmvapic.o
 obj-y += acpi-build.o
diff --git a/hw/xenpv/Makefile.objs b/hw/xenpv/Makefile.objs
new file mode 100644
index 0000000..49f6e9e
--- /dev/null
+++ b/hw/xenpv/Makefile.objs
@@ -0,0 +1,2 @@
+# Xen PV machine support
+obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
diff --git a/hw/i386/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
similarity index 100%
rename from hw/i386/xen_domainbuild.c
rename to hw/xenpv/xen_domainbuild.c
diff --git a/hw/i386/xen_domainbuild.h b/hw/xenpv/xen_domainbuild.h
similarity index 100%
rename from hw/i386/xen_domainbuild.h
rename to hw/xenpv/xen_domainbuild.h
diff --git a/hw/i386/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c
similarity index 100%
rename from hw/i386/xen_machine_pv.c
rename to hw/xenpv/xen_machine_pv.c
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH RFC 2/5] xen: factor out common functions
  2014-01-23 22:16 [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Wei Liu
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 1/5] xen: move Xen PV machine files to hw/xenpv Wei Liu
@ 2014-01-23 22:16 ` Wei Liu
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 3/5] exec: guard Xen HVM hooks with CONFIG_XEN_I386 Wei Liu
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Wei Liu @ 2014-01-23 22:16 UTC (permalink / raw)
  To: xen-devel, qemu-devel
  Cc: anthony.perard, peter.maydell, Wei Liu, stefano.stabellini

So common functions used by both HVM and PV are factored out from
xen-all.c to xen-common.c.

Also extract a QMP function from xen-all.c and xen-stub.c to
xen-common.c

Finally rename xen-all.c to xen-hvm.c, as those functions are only
useful to HVM guest.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 Makefile.target        |    3 +-
 xen-common.c           |  137 ++++++++++++++++++++++++++++++++++++++++++++++++
 xen-all.c => xen-hvm.c |  112 +--------------------------------------
 xen-stub.c             |    4 --
 4 files changed, 141 insertions(+), 115 deletions(-)
 create mode 100644 xen-common.c
 rename xen-all.c => xen-hvm.c (92%)

diff --git a/Makefile.target b/Makefile.target
index d07cd95..12c6448 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -123,7 +123,8 @@ obj-y += dump.o
 LIBS+=$(libs_softmmu)
 
 # xen support
-obj-$(CONFIG_XEN) += xen-all.o xen-mapcache.o
+obj-$(CONFIG_XEN) += xen-common.o
+obj-$(CONFIG_XEN_I386) += xen-hvm.o xen-mapcache.o
 obj-$(call lnot,$(CONFIG_XEN)) += xen-stub.o
 
 # Hardware support
diff --git a/xen-common.c b/xen-common.c
new file mode 100644
index 0000000..05a244a
--- /dev/null
+++ b/xen-common.c
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2014       Citrix Systems UK Ltd.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#include "hw/xen/xen_backend.h"
+#include "qmp-commands.h"
+#include "sysemu/char.h"
+
+//#define DEBUG_XEN
+
+#ifdef DEBUG_XEN
+#define DPRINTF(fmt, ...) \
+    do { fprintf(stderr, "xen: " fmt, ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+    do { } while (0)
+#endif
+
+static int store_dev_info(int domid, CharDriverState *cs, const char *string)
+{
+    struct xs_handle *xs = NULL;
+    char *path = NULL;
+    char *newpath = NULL;
+    char *pts = NULL;
+    int ret = -1;
+
+    /* Only continue if we're talking to a pty. */
+    if (strncmp(cs->filename, "pty:", 4)) {
+        return 0;
+    }
+    pts = cs->filename + 4;
+
+    /* We now have everything we need to set the xenstore entry. */
+    xs = xs_open(0);
+    if (xs == NULL) {
+        fprintf(stderr, "Could not contact XenStore\n");
+        goto out;
+    }
+
+    path = xs_get_domain_path(xs, domid);
+    if (path == NULL) {
+        fprintf(stderr, "xs_get_domain_path() error\n");
+        goto out;
+    }
+    newpath = realloc(path, (strlen(path) + strlen(string) +
+                strlen("/tty") + 1));
+    if (newpath == NULL) {
+        fprintf(stderr, "realloc error\n");
+        goto out;
+    }
+    path = newpath;
+
+    strcat(path, string);
+    strcat(path, "/tty");
+    if (!xs_write(xs, XBT_NULL, path, pts, strlen(pts))) {
+        fprintf(stderr, "xs_write for '%s' fail", string);
+        goto out;
+    }
+    ret = 0;
+
+out:
+    free(path);
+    xs_close(xs);
+
+    return ret;
+}
+
+void xenstore_store_pv_console_info(int i, CharDriverState *chr)
+{
+    if (i == 0) {
+        store_dev_info(xen_domid, chr, "/console");
+    } else {
+        char buf[32];
+        snprintf(buf, sizeof(buf), "/device/console/%d", i);
+        store_dev_info(xen_domid, chr, buf);
+    }
+}
+
+
+static void xenstore_record_dm_state(struct xs_handle *xs, const char *state)
+{
+    char path[50];
+
+    if (xs == NULL) {
+        fprintf(stderr, "xenstore connection not initialized\n");
+        exit(1);
+    }
+
+    snprintf(path, sizeof (path), "device-model/%u/state", xen_domid);
+    if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) {
+        fprintf(stderr, "error recording dm state\n");
+        exit(1);
+    }
+}
+
+
+static void xen_change_state_handler(void *opaque, int running,
+                                     RunState state)
+{
+    if (running) {
+        /* record state running */
+        xenstore_record_dm_state(xenstore, "running");
+    }
+}
+
+int xen_init(void)
+{
+    xen_xc = xen_xc_interface_open(0, 0, 0);
+    if (xen_xc == XC_HANDLER_INITIAL_VALUE) {
+        xen_be_printf(NULL, 0, "can't open xen interface\n");
+        return -1;
+    }
+    qemu_add_vm_change_state_handler(xen_change_state_handler, NULL);
+
+    return 0;
+}
+
+#ifdef CONFIG_XEN_I386
+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
+{
+    if (enable) {
+        memory_global_dirty_log_start();
+    } else {
+        memory_global_dirty_log_stop();
+    }
+}
+#else
+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
+{
+}
+#endif
diff --git a/xen-all.c b/xen-hvm.c
similarity index 92%
rename from xen-all.c
rename to xen-hvm.c
index 4a594bd..5b9bc5f 100644
--- a/xen-all.c
+++ b/xen-hvm.c
@@ -26,9 +26,9 @@
 #include <xen/hvm/params.h>
 #include <xen/hvm/e820.h>
 
-//#define DEBUG_XEN
+//#define DEBUG_XEN_HVM
 
-#ifdef DEBUG_XEN
+#ifdef DEBUG_XEN_HVM
 #define DPRINTF(fmt, ...) \
     do { fprintf(stderr, "xen: " fmt, ## __VA_ARGS__); } while (0)
 #else
@@ -569,15 +569,6 @@ static MemoryListener xen_memory_listener = {
     .priority = 10,
 };
 
-void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
-{
-    if (enable) {
-        memory_global_dirty_log_start();
-    } else {
-        memory_global_dirty_log_stop();
-    }
-}
-
 /* get the ioreq packets from share mem */
 static ioreq_t *cpu_get_ioreq_from_shared_memory(XenIOState *state, int vcpu)
 {
@@ -880,82 +871,6 @@ static void cpu_handle_ioreq(void *opaque)
     }
 }
 
-static int store_dev_info(int domid, CharDriverState *cs, const char *string)
-{
-    struct xs_handle *xs = NULL;
-    char *path = NULL;
-    char *newpath = NULL;
-    char *pts = NULL;
-    int ret = -1;
-
-    /* Only continue if we're talking to a pty. */
-    if (strncmp(cs->filename, "pty:", 4)) {
-        return 0;
-    }
-    pts = cs->filename + 4;
-
-    /* We now have everything we need to set the xenstore entry. */
-    xs = xs_open(0);
-    if (xs == NULL) {
-        fprintf(stderr, "Could not contact XenStore\n");
-        goto out;
-    }
-
-    path = xs_get_domain_path(xs, domid);
-    if (path == NULL) {
-        fprintf(stderr, "xs_get_domain_path() error\n");
-        goto out;
-    }
-    newpath = realloc(path, (strlen(path) + strlen(string) +
-                strlen("/tty") + 1));
-    if (newpath == NULL) {
-        fprintf(stderr, "realloc error\n");
-        goto out;
-    }
-    path = newpath;
-
-    strcat(path, string);
-    strcat(path, "/tty");
-    if (!xs_write(xs, XBT_NULL, path, pts, strlen(pts))) {
-        fprintf(stderr, "xs_write for '%s' fail", string);
-        goto out;
-    }
-    ret = 0;
-
-out:
-    free(path);
-    xs_close(xs);
-
-    return ret;
-}
-
-void xenstore_store_pv_console_info(int i, CharDriverState *chr)
-{
-    if (i == 0) {
-        store_dev_info(xen_domid, chr, "/console");
-    } else {
-        char buf[32];
-        snprintf(buf, sizeof(buf), "/device/console/%d", i);
-        store_dev_info(xen_domid, chr, buf);
-    }
-}
-
-static void xenstore_record_dm_state(struct xs_handle *xs, const char *state)
-{
-    char path[50];
-
-    if (xs == NULL) {
-        fprintf(stderr, "xenstore connection not initialized\n");
-        exit(1);
-    }
-
-    snprintf(path, sizeof (path), "device-model/%u/state", xen_domid);
-    if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) {
-        fprintf(stderr, "error recording dm state\n");
-        exit(1);
-    }
-}
-
 static void xen_main_loop_prepare(XenIOState *state)
 {
     int evtchn_fd = -1;
@@ -973,17 +888,6 @@ static void xen_main_loop_prepare(XenIOState *state)
 }
 
 
-/* Initialise Xen */
-
-static void xen_change_state_handler(void *opaque, int running,
-                                     RunState state)
-{
-    if (running) {
-        /* record state running */
-        xenstore_record_dm_state(xenstore, "running");
-    }
-}
-
 static void xen_hvm_change_state_handler(void *opaque, int running,
                                          RunState rstate)
 {
@@ -1001,18 +905,6 @@ static void xen_exit_notifier(Notifier *n, void *data)
     xs_daemon_close(state->xenstore);
 }
 
-int xen_init(void)
-{
-    xen_xc = xen_xc_interface_open(0, 0, 0);
-    if (xen_xc == XC_HANDLER_INITIAL_VALUE) {
-        xen_be_printf(NULL, 0, "can't open xen interface\n");
-        return -1;
-    }
-    qemu_add_vm_change_state_handler(xen_change_state_handler, NULL);
-
-    return 0;
-}
-
 static void xen_read_physmap(XenIOState *state)
 {
     XenPhysmap *physmap = NULL;
diff --git a/xen-stub.c b/xen-stub.c
index ad189a6..4e2e1e8 100644
--- a/xen-stub.c
+++ b/xen-stub.c
@@ -56,10 +56,6 @@ void xen_register_framebuffer(MemoryRegion *mr)
 {
 }
 
-void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
-{
-}
-
 void xen_modified_memory(ram_addr_t start, ram_addr_t length)
 {
 }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH RFC 3/5] exec: guard Xen HVM hooks with CONFIG_XEN_I386
  2014-01-23 22:16 [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Wei Liu
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 1/5] xen: move Xen PV machine files to hw/xenpv Wei Liu
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 2/5] xen: factor out common functions Wei Liu
@ 2014-01-23 22:16 ` Wei Liu
  2014-01-24  7:35   ` Paolo Bonzini
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 4/5] xen: implement Xen PV target Wei Liu
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Wei Liu @ 2014-01-23 22:16 UTC (permalink / raw)
  To: xen-devel, qemu-devel
  Cc: anthony.perard, peter.maydell, Wei Liu, stefano.stabellini

Those are only useful when building QEMU with HVM support.

We need to expose CONFIG_XEN_I386 to source code so we modify configure
and i386/x86_64-softmmu.mak.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 configure                          |    1 +
 default-configs/i386-softmmu.mak   |    1 -
 default-configs/x86_64-softmmu.mak |    1 -
 exec.c                             |   16 ++++++++++++++++
 include/exec/memory-internal.h     |    2 ++
 5 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 10a6562..1e515be 100755
--- a/configure
+++ b/configure
@@ -4472,6 +4472,7 @@ echo "TARGET_ABI_DIR=$TARGET_ABI_DIR" >> $config_target_mak
 
 if supported_xen_target; then
     echo "CONFIG_XEN=y" >> $config_target_mak
+    echo "CONFIG_XEN_I386=y" >> $config_target_mak
     if test "$xen_pci_passthrough" = yes; then
         echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak"
     fi
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 37ef90f..3c89aaa 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -33,7 +33,6 @@ CONFIG_MC146818RTC=y
 CONFIG_PAM=y
 CONFIG_PCI_PIIX=y
 CONFIG_WDT_IB700=y
-CONFIG_XEN_I386=$(CONFIG_XEN)
 CONFIG_ISA_DEBUG=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_VMPORT=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 31bddce..1dc1f85 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -33,7 +33,6 @@ CONFIG_MC146818RTC=y
 CONFIG_PAM=y
 CONFIG_PCI_PIIX=y
 CONFIG_WDT_IB700=y
-CONFIG_XEN_I386=$(CONFIG_XEN)
 CONFIG_ISA_DEBUG=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_VMPORT=y
diff --git a/exec.c b/exec.c
index defe38f..a72efe2 100644
--- a/exec.c
+++ b/exec.c
@@ -1228,7 +1228,9 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
             fprintf(stderr, "-mem-path not supported with Xen\n");
             exit(1);
         }
+#ifdef CONFIG_XEN_I386
         xen_ram_alloc(new_block->offset, size, mr);
+#endif
     } else {
         if (mem_path) {
             if (phys_mem_alloc != qemu_anon_ram_alloc) {
@@ -1324,7 +1326,9 @@ void qemu_ram_free(ram_addr_t addr)
             if (block->flags & RAM_PREALLOC_MASK) {
                 ;
             } else if (xen_enabled()) {
+#ifdef CONFIG_XEN_I386
                 xen_invalidate_map_cache_entry(block->host);
+#endif
 #ifndef _WIN32
             } else if (block->fd >= 0) {
                 munmap(block->host, block->length);
@@ -1409,6 +1413,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
     RAMBlock *block = qemu_get_ram_block(addr);
 
     if (xen_enabled()) {
+#ifdef CONFIG_XEN_I386
         /* We need to check if the requested address is in the RAM
          * because we don't want to map the entire memory in QEMU.
          * In that case just map until the end of the page.
@@ -1419,6 +1424,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
             block->host =
                 xen_map_cache(block->offset, block->length, 1);
         }
+#endif
     }
     return block->host + (addr - block->offset);
 }
@@ -1431,7 +1437,11 @@ static void *qemu_ram_ptr_length(ram_addr_t addr, hwaddr *size)
         return NULL;
     }
     if (xen_enabled()) {
+#ifdef CONFIG_XEN_I386
         return xen_map_cache(addr, *size, 1);
+#else
+        return NULL;
+#endif
     } else {
         RAMBlock *block;
 
@@ -1456,8 +1466,10 @@ MemoryRegion *qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
     uint8_t *host = ptr;
 
     if (xen_enabled()) {
+#ifdef CONFIG_XEN_I386
         *ram_addr = xen_ram_addr_from_mapcache(ptr);
         return qemu_get_ram_block(*ram_addr)->mr;
+#endif
     }
 
     block = ram_list.mru_block;
@@ -1921,7 +1933,9 @@ static void invalidate_and_set_dirty(hwaddr addr,
         /* set dirty bit */
         cpu_physical_memory_set_dirty_flags(addr, (0xff & ~CODE_DIRTY_FLAG));
     }
+#ifdef CONFIG_XEN_I386
     xen_modified_memory(addr, length);
+#endif
 }
 
 static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
@@ -2298,7 +2312,9 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
             }
         }
         if (xen_enabled()) {
+#ifdef CONFIG_XEN_I386
             xen_invalidate_map_cache_entry(buffer);
+#endif
         }
         memory_region_unref(mr);
         return;
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index d0e0633..b4e76e2 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -100,7 +100,9 @@ static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
     for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
         cpu_physical_memory_set_dirty_flags(addr, dirty_flags);
     }
+#ifdef CONFIG_XEN_I386
     xen_modified_memory(addr, length);
+#endif
 }
 
 static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH RFC 4/5] xen: implement Xen PV target
  2014-01-23 22:16 [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Wei Liu
                   ` (2 preceding siblings ...)
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 3/5] exec: guard Xen HVM hooks with CONFIG_XEN_I386 Wei Liu
@ 2014-01-23 22:16 ` Wei Liu
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 5/5] xen: introduce xenpv-softmmu.mak Wei Liu
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Wei Liu @ 2014-01-23 22:16 UTC (permalink / raw)
  To: xen-devel, qemu-devel
  Cc: anthony.perard, peter.maydell, Wei Liu, stefano.stabellini

Basically it's a dummy CPU that doens't do anything. This patch contains
necessary hooks to make QEMU compile.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 arch_init.c                |    2 ++
 cpu-exec.c                 |    2 ++
 include/sysemu/arch_init.h |    1 +
 target-xenpv/Makefile.objs |    1 +
 target-xenpv/cpu-qom.h     |   64 ++++++++++++++++++++++++++++++++++++++++++
 target-xenpv/cpu.h         |   66 ++++++++++++++++++++++++++++++++++++++++++++
 target-xenpv/helper.c      |   32 +++++++++++++++++++++
 target-xenpv/translate.c   |   27 ++++++++++++++++++
 8 files changed, 195 insertions(+)
 create mode 100644 target-xenpv/Makefile.objs
 create mode 100644 target-xenpv/cpu-qom.h
 create mode 100644 target-xenpv/cpu.h
 create mode 100644 target-xenpv/helper.c
 create mode 100644 target-xenpv/helper.h
 create mode 100644 target-xenpv/translate.c

diff --git a/arch_init.c b/arch_init.c
index 2935426..41041d5 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -101,6 +101,8 @@ int graphic_depth = 32;
 #define QEMU_ARCH QEMU_ARCH_XTENSA
 #elif defined(TARGET_UNICORE32)
 #define QEMU_ARCH QEMU_ARCH_UNICORE32
+#elif defined(TARGET_XENPV)
+#define QEMU_ARCH QEMU_ARCH_XENPV
 #endif
 
 const uint32_t arch_type = QEMU_ARCH;
diff --git a/cpu-exec.c b/cpu-exec.c
index b744a1f..c86c383 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -255,6 +255,7 @@ int cpu_exec(CPUArchState *env)
 #elif defined(TARGET_CRIS)
 #elif defined(TARGET_S390X)
 #elif defined(TARGET_XTENSA)
+#elif defined(TARGET_XENPV)
     /* XXXXX */
 #else
 #error unsupported target CPU
@@ -710,6 +711,7 @@ int cpu_exec(CPUArchState *env)
 #elif defined(TARGET_CRIS)
 #elif defined(TARGET_S390X)
 #elif defined(TARGET_XTENSA)
+#elif defined(TARGET_XENPV)
     /* XXXXX */
 #else
 #error unsupported target CPU
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index be71bca..66ea63f 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -22,6 +22,7 @@ enum {
     QEMU_ARCH_OPENRISC = 8192,
     QEMU_ARCH_UNICORE32 = 0x4000,
     QEMU_ARCH_MOXIE = 0x8000,
+    QEMU_ARCH_XENPV = 0x10000,
 };
 
 extern const uint32_t arch_type;
diff --git a/target-xenpv/Makefile.objs b/target-xenpv/Makefile.objs
new file mode 100644
index 0000000..ae3cad5
--- /dev/null
+++ b/target-xenpv/Makefile.objs
@@ -0,0 +1 @@
+obj-$(CONFIG_TCG) += helper.o translate.o
diff --git a/target-xenpv/cpu-qom.h b/target-xenpv/cpu-qom.h
new file mode 100644
index 0000000..61135a6
--- /dev/null
+++ b/target-xenpv/cpu-qom.h
@@ -0,0 +1,64 @@
+/*
+ * QEMU XenPV CPU
+ *
+ * Copyright (c) 2014 Citrix Systems UK Ltd
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+#ifndef QEMU_XENPV_CPU_QOM_H
+#define QEMU_XENPV_CPU_QOM_H
+
+#include "qom/cpu.h"
+#include "cpu.h"
+#include "qapi/error.h"
+
+#define TYPE_XENPV_CPU "xenpv-cpu"
+
+/**
+ * XenPVCPUClass:
+ * @parent_realize: The parent class' realize handler.
+ * @parent_reset: The parent class' reset handler.
+ *
+ */
+typedef struct XenPVCPUClass {
+    /*< private >*/
+    CPUClass parent_class;
+    /*< public >*/
+
+    DeviceRealize parent_realize;
+    void (*parent_reset)(CPUState *cpu);
+} XenPVCPUClass;
+
+/**
+ * XenPVCPU:
+ * @env: #CPUXenPVState
+ *
+ */
+typedef struct XenPVCPU {
+    /*< private >*/
+    CPUState parent_obj;
+    /*< public >*/
+    CPUXenPVState env;
+} XenPVCPU;
+
+static inline XenPVCPU *noarch_env_get_cpu(CPUXenPVState *env)
+{
+    return container_of(env, XenPVCPU, env);
+}
+
+#define ENV_GET_CPU(e) CPU(noarch_env_get_cpu(e))
+
+#endif
+
diff --git a/target-xenpv/cpu.h b/target-xenpv/cpu.h
new file mode 100644
index 0000000..0e65707
--- /dev/null
+++ b/target-xenpv/cpu.h
@@ -0,0 +1,66 @@
+/*
+ * XenPV virtual CPU header
+ *
+ *  Copyright (c) 2014 Citrix Systems UK Ltd
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef CPU_XENPV_H
+#define CPU_XENPV_H
+
+#include "config.h"
+#include "qemu-common.h"
+
+#define TARGET_LONG_BITS 64
+#define TARGET_PAGE_BITS 12
+#define TARGET_PHYS_ADDR_SPACE_BITS 52
+#define TARGET_VIRT_ADDR_SPACE_BITS 47
+#define NB_MMU_MODES 1
+
+#define CPUArchState struct CPUXenPVState
+
+#include "exec/cpu-defs.h"
+
+typedef struct CPUXenPVState {
+    CPU_COMMON
+} CPUXenPVState;
+
+#include "cpu-qom.h"
+
+static inline int cpu_mmu_index (CPUXenPVState *env)
+{
+    abort();
+}
+
+static inline void cpu_get_tb_cpu_state(CPUXenPVState *env, target_ulong *pc,
+                                        target_ulong *cs_base, int *flags)
+{
+    abort();
+}
+
+static inline bool cpu_has_work(CPUState *cs)
+{
+    abort();
+    return false;
+}
+
+int cpu_xenpv_exec(CPUXenPVState *s);
+#define cpu_exec cpu_xenpv_exec
+
+#include "exec/cpu-all.h"
+
+#include "exec/exec-all.h"
+
+#endif /* CPU_XENPV_H */
+
diff --git a/target-xenpv/helper.c b/target-xenpv/helper.c
new file mode 100644
index 0000000..225a063
--- /dev/null
+++ b/target-xenpv/helper.c
@@ -0,0 +1,32 @@
+#include "cpu.h"
+#include "helper.h"
+#if !defined(CONFIG_USER_ONLY)
+#include "exec/softmmu_exec.h"
+#endif
+
+#if !defined(CONFIG_USER_ONLY)
+
+#define MMUSUFFIX _mmu
+
+#define SHIFT 0
+#include "exec/softmmu_template.h"
+
+#define SHIFT 1
+#include "exec/softmmu_template.h"
+
+#define SHIFT 2
+#include "exec/softmmu_template.h"
+
+#define SHIFT 3
+#include "exec/softmmu_template.h"
+
+#endif
+
+#if !defined(CONFIG_USER_ONLY)
+void tlb_fill(CPUXenPVState *env, target_ulong addr, int is_write,
+              int mmu_idx, uintptr_t retaddr)
+{
+    abort();
+}
+#endif
+
diff --git a/target-xenpv/helper.h b/target-xenpv/helper.h
new file mode 100644
index 0000000..e69de29
diff --git a/target-xenpv/translate.c b/target-xenpv/translate.c
new file mode 100644
index 0000000..4bc84e5
--- /dev/null
+++ b/target-xenpv/translate.c
@@ -0,0 +1,27 @@
+#include <inttypes.h>
+#include "qemu/host-utils.h"
+#include "cpu.h"
+#include "disas/disas.h"
+#include "tcg-op.h"
+
+#include "helper.h"
+#define GEN_HELPER 1
+#include "helper.h"
+
+void gen_intermediate_code(CPUXenPVState *env, TranslationBlock *tb)
+{
+    abort();
+}
+
+void gen_intermediate_code_pc(CPUXenPVState *env, TranslationBlock *tb)
+{
+    abort();
+}
+
+void restore_state_to_opc(CPUXenPVState *env, TranslationBlock *tb,
+                          int pc_pos)
+{
+    abort();
+}
+
+
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH RFC 5/5] xen: introduce xenpv-softmmu.mak
  2014-01-23 22:16 [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Wei Liu
                   ` (3 preceding siblings ...)
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 4/5] xen: implement Xen PV target Wei Liu
@ 2014-01-23 22:16 ` Wei Liu
  2014-01-24  7:38   ` Paolo Bonzini
  2014-01-23 22:30 ` [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Peter Maydell
  2014-01-24  7:36 ` [Qemu-devel] " Paolo Bonzini
  6 siblings, 1 reply; 19+ messages in thread
From: Wei Liu @ 2014-01-23 22:16 UTC (permalink / raw)
  To: xen-devel, qemu-devel
  Cc: anthony.perard, peter.maydell, Wei Liu, stefano.stabellini

The modification to configure is rebased on Paolo's change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 configure                         |   13 +++++++++----
 default-configs/xenpv-softmmu.mak |    2 ++
 2 files changed, 11 insertions(+), 4 deletions(-)
 create mode 100644 default-configs/xenpv-softmmu.mak

diff --git a/configure b/configure
index 1e515be..324270c 100755
--- a/configure
+++ b/configure
@@ -4283,7 +4283,7 @@ supported_xen_target() {
     test "$xen" = "yes" || return 1
     test "$target_softmmu" = "yes" || return 1
     case "$target_name:$cpu" in
-        i386:i386 | i386:x86_64 | x86_64:i386 | x86_64:x86_64)
+        i386:i386 | i386:x86_64 | x86_64:i386 | x86_64:x86_64 | xenpv:*)
             return 0
         ;;
     esac
@@ -4443,6 +4443,9 @@ case "$target_name" in
   ;;
   unicore32)
   ;;
+  xenpv)
+    TARGET_ARCH=xenpv
+  ;;
   xtensa|xtensaeb)
     TARGET_ARCH=xtensa
   ;;
@@ -4472,9 +4475,11 @@ echo "TARGET_ABI_DIR=$TARGET_ABI_DIR" >> $config_target_mak
 
 if supported_xen_target; then
     echo "CONFIG_XEN=y" >> $config_target_mak
-    echo "CONFIG_XEN_I386=y" >> $config_target_mak
-    if test "$xen_pci_passthrough" = yes; then
-        echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak"
+    if test "$target_name" != "xenpv"; then
+        echo "CONFIG_XEN_I386=y" >> $config_target_mak
+        if test "$xen_pci_passthrough" = yes; then
+            echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak"
+        fi
     fi
 fi
 if supported_kvm_target; then
diff --git a/default-configs/xenpv-softmmu.mak b/default-configs/xenpv-softmmu.mak
new file mode 100644
index 0000000..773f128
--- /dev/null
+++ b/default-configs/xenpv-softmmu.mak
@@ -0,0 +1,2 @@
+# Default configuration for xenpv-softmmu
+# Yes it is empty as we don't need to include any device emulation code
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
  2014-01-23 22:16 [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Wei Liu
                   ` (4 preceding siblings ...)
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 5/5] xen: introduce xenpv-softmmu.mak Wei Liu
@ 2014-01-23 22:30 ` Peter Maydell
  2014-01-24 14:23   ` Wei Liu
  2014-01-24 14:30   ` Paolo Bonzini
  2014-01-24  7:36 ` [Qemu-devel] " Paolo Bonzini
  6 siblings, 2 replies; 19+ messages in thread
From: Peter Maydell @ 2014-01-23 22:30 UTC (permalink / raw)
  To: Wei Liu
  Cc: Anthony PERARD, Stefano Stabellini, QEMU Developers,
	xen-devel@lists.xen.org

On 23 January 2014 22:16, Wei Liu <wei.liu2@citrix.com> wrote:
> As promised I hacked a prototype based on Paolo's disable TCG series.
> However I coded some stubs for TCG anyway. So this series in principle
> should work with / without Paolo's series.

I'm afraid I still think this is a terrible idea. "Xen" isn't a CPU, and
"the binary is smaller" isn't IMHO sufficient justification for breaking
QEMU's basic structure of "target-* define target CPUs and we have
a lot of compile time constants which are specific to a CPU which
get defined there". How would you support a bigendian Xen CPU,
just to pick one example of where this falls down?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 3/5] exec: guard Xen HVM hooks with CONFIG_XEN_I386
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 3/5] exec: guard Xen HVM hooks with CONFIG_XEN_I386 Wei Liu
@ 2014-01-24  7:35   ` Paolo Bonzini
  0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2014-01-24  7:35 UTC (permalink / raw)
  To: Wei Liu, xen-devel, qemu-devel
  Cc: anthony.perard, peter.maydell, stefano.stabellini

Il 23/01/2014 23:16, Wei Liu ha scritto:
> Those are only useful when building QEMU with HVM support.
>
> We need to expose CONFIG_XEN_I386 to source code so we modify configure
> and i386/x86_64-softmmu.mak.

I think the right way is to add a xen-hvm-stub.c file and include it in 
xenpv-softmmu.

Since you are at it, xen_platform.o xen_apic.o xen_pvdevice.o can be 
moved to hw/i386/xen and you can drop CONFIG_XEN_I386 completely.

Paolo

> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  configure                          |    1 +
>  default-configs/i386-softmmu.mak   |    1 -
>  default-configs/x86_64-softmmu.mak |    1 -
>  exec.c                             |   16 ++++++++++++++++
>  include/exec/memory-internal.h     |    2 ++
>  5 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index 10a6562..1e515be 100755
> --- a/configure
> +++ b/configure
> @@ -4472,6 +4472,7 @@ echo "TARGET_ABI_DIR=$TARGET_ABI_DIR" >> $config_target_mak
>
>  if supported_xen_target; then
>      echo "CONFIG_XEN=y" >> $config_target_mak
> +    echo "CONFIG_XEN_I386=y" >> $config_target_mak
>      if test "$xen_pci_passthrough" = yes; then
>          echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak"
>      fi
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 37ef90f..3c89aaa 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -33,7 +33,6 @@ CONFIG_MC146818RTC=y
>  CONFIG_PAM=y
>  CONFIG_PCI_PIIX=y
>  CONFIG_WDT_IB700=y
> -CONFIG_XEN_I386=$(CONFIG_XEN)
>  CONFIG_ISA_DEBUG=y
>  CONFIG_ISA_TESTDEV=y
>  CONFIG_VMPORT=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index 31bddce..1dc1f85 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -33,7 +33,6 @@ CONFIG_MC146818RTC=y
>  CONFIG_PAM=y
>  CONFIG_PCI_PIIX=y
>  CONFIG_WDT_IB700=y
> -CONFIG_XEN_I386=$(CONFIG_XEN)
>  CONFIG_ISA_DEBUG=y
>  CONFIG_ISA_TESTDEV=y
>  CONFIG_VMPORT=y
> diff --git a/exec.c b/exec.c
> index defe38f..a72efe2 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1228,7 +1228,9 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
>              fprintf(stderr, "-mem-path not supported with Xen\n");
>              exit(1);
>          }
> +#ifdef CONFIG_XEN_I386
>          xen_ram_alloc(new_block->offset, size, mr);
> +#endif
>      } else {
>          if (mem_path) {
>              if (phys_mem_alloc != qemu_anon_ram_alloc) {
> @@ -1324,7 +1326,9 @@ void qemu_ram_free(ram_addr_t addr)
>              if (block->flags & RAM_PREALLOC_MASK) {
>                  ;
>              } else if (xen_enabled()) {
> +#ifdef CONFIG_XEN_I386
>                  xen_invalidate_map_cache_entry(block->host);
> +#endif
>  #ifndef _WIN32
>              } else if (block->fd >= 0) {
>                  munmap(block->host, block->length);
> @@ -1409,6 +1413,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
>      RAMBlock *block = qemu_get_ram_block(addr);
>
>      if (xen_enabled()) {
> +#ifdef CONFIG_XEN_I386
>          /* We need to check if the requested address is in the RAM
>           * because we don't want to map the entire memory in QEMU.
>           * In that case just map until the end of the page.
> @@ -1419,6 +1424,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
>              block->host =
>                  xen_map_cache(block->offset, block->length, 1);
>          }
> +#endif
>      }
>      return block->host + (addr - block->offset);
>  }
> @@ -1431,7 +1437,11 @@ static void *qemu_ram_ptr_length(ram_addr_t addr, hwaddr *size)
>          return NULL;
>      }
>      if (xen_enabled()) {
> +#ifdef CONFIG_XEN_I386
>          return xen_map_cache(addr, *size, 1);
> +#else
> +        return NULL;
> +#endif
>      } else {
>          RAMBlock *block;
>
> @@ -1456,8 +1466,10 @@ MemoryRegion *qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
>      uint8_t *host = ptr;
>
>      if (xen_enabled()) {
> +#ifdef CONFIG_XEN_I386
>          *ram_addr = xen_ram_addr_from_mapcache(ptr);
>          return qemu_get_ram_block(*ram_addr)->mr;
> +#endif
>      }
>
>      block = ram_list.mru_block;
> @@ -1921,7 +1933,9 @@ static void invalidate_and_set_dirty(hwaddr addr,
>          /* set dirty bit */
>          cpu_physical_memory_set_dirty_flags(addr, (0xff & ~CODE_DIRTY_FLAG));
>      }
> +#ifdef CONFIG_XEN_I386
>      xen_modified_memory(addr, length);
> +#endif
>  }
>
>  static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
> @@ -2298,7 +2312,9 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
>              }
>          }
>          if (xen_enabled()) {
> +#ifdef CONFIG_XEN_I386
>              xen_invalidate_map_cache_entry(buffer);
> +#endif
>          }
>          memory_region_unref(mr);
>          return;
> diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
> index d0e0633..b4e76e2 100644
> --- a/include/exec/memory-internal.h
> +++ b/include/exec/memory-internal.h
> @@ -100,7 +100,9 @@ static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
>      for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
>          cpu_physical_memory_set_dirty_flags(addr, dirty_flags);
>      }
> +#ifdef CONFIG_XEN_I386
>      xen_modified_memory(addr, length);
> +#endif
>  }
>
>  static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
  2014-01-23 22:16 [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Wei Liu
                   ` (5 preceding siblings ...)
  2014-01-23 22:30 ` [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Peter Maydell
@ 2014-01-24  7:36 ` Paolo Bonzini
  6 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2014-01-24  7:36 UTC (permalink / raw)
  To: Wei Liu, xen-devel, qemu-devel
  Cc: anthony.perard, peter.maydell, stefano.stabellini

Il 23/01/2014 23:16, Wei Liu ha scritto:
> As promised I hacked a prototype based on Paolo's disable TCG series.
> However I coded some stubs for TCG anyway. So this series in principle
> should work with / without Paolo's series.
>
> The first 3 patches refactor some code to disentangle Xen PV and HVM
> guest. The 4th patch has the real meat. It introduces Xen PV target,
> which contains basically a dummy CPU, then hooks up this Xen PV CPU to
> QEMU internal structures.
>
> The last patch introduces xenpv-softmmu, which contains *no* emulation
> code. I know that in previous discussion people said that every device
> emulation should be included if the target architecture is called null.
> But since this target CPU is now called xenpv I don't feel obliged to
> include any device emulation in this prototype anymore. :-)
>
> Please note that the existing Xen QEMU build is not affected at all. You
> can still use "--disable-tcg --enable-xen --target-list=i386-softmmu"
> (or x86_64-softmmu") to build qemu-system-{i386,x86_64} and use it for
> both HVM and PV guest. This series adds another option to build QEMU with
> "--disable-tcg --enable-xen --target-list=xenpv-softmmu" and get a QEMU
> binary tailored for Xen PV guest. The effect is that we reduce the
> binary size from 14MB to 7.3MB.
>
> What do you think of this idea? I'm all ears.
>
> Wei.
>
> Wei Liu (5):
>   xen: move Xen PV machine files to hw/xenpv
>   xen: factor out common functions
>   exec: guard Xen HVM hooks with CONFIG_XEN_I386
>   xen: implement Xen PV target
>   xen: introduce xenpv-softmmu.mak
>
>  Makefile.target                      |    3 +-
>  arch_init.c                          |    2 +
>  configure                            |   12 ++-
>  cpu-exec.c                           |    2 +
>  default-configs/i386-softmmu.mak     |    1 -
>  default-configs/x86_64-softmmu.mak   |    1 -
>  default-configs/xenpv-softmmu.mak    |    2 +
>  exec.c                               |   16 ++++
>  hw/i386/Makefile.objs                |    2 +-
>  hw/xenpv/Makefile.objs               |    2 +
>  hw/{i386 => xenpv}/xen_domainbuild.c |    0
>  hw/{i386 => xenpv}/xen_domainbuild.h |    0
>  hw/{i386 => xenpv}/xen_machine_pv.c  |    0
>  include/exec/memory-internal.h       |    2 +
>  include/sysemu/arch_init.h           |    1 +
>  target-xenpv/Makefile.objs           |    1 +
>  target-xenpv/cpu-qom.h               |   64 ++++++++++++++++
>  target-xenpv/cpu.h                   |   66 ++++++++++++++++
>  target-xenpv/helper.c                |   32 ++++++++
>  target-xenpv/translate.c             |   27 +++++++
>  xen-common.c                         |  137 ++++++++++++++++++++++++++++++++++
>  xen-all.c => xen-hvm.c               |  112 +--------------------------
>  xen-stub.c                           |    4 -
>  23 files changed, 368 insertions(+), 121 deletions(-)
>  create mode 100644 default-configs/xenpv-softmmu.mak
>  create mode 100644 hw/xenpv/Makefile.objs
>  rename hw/{i386 => xenpv}/xen_domainbuild.c (100%)
>  rename hw/{i386 => xenpv}/xen_domainbuild.h (100%)
>  rename hw/{i386 => xenpv}/xen_machine_pv.c (100%)
>  create mode 100644 target-xenpv/Makefile.objs
>  create mode 100644 target-xenpv/cpu-qom.h
>  create mode 100644 target-xenpv/cpu.h
>  create mode 100644 target-xenpv/helper.c
>  create mode 100644 target-xenpv/helper.h
>  create mode 100644 target-xenpv/translate.c
>  create mode 100644 xen-common.c
>  rename xen-all.c => xen-hvm.c (92%)
>

Mostly looks good!

Feel free to take any initial patches you need from my branch, and post 
them for inclusion together with this series!

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 5/5] xen: introduce xenpv-softmmu.mak
  2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 5/5] xen: introduce xenpv-softmmu.mak Wei Liu
@ 2014-01-24  7:38   ` Paolo Bonzini
  2014-01-24 17:00     ` Wei Liu
  0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2014-01-24  7:38 UTC (permalink / raw)
  To: Wei Liu, xen-devel, qemu-devel
  Cc: anthony.perard, peter.maydell, stefano.stabellini

Il 23/01/2014 23:16, Wei Liu ha scritto:
> -        echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak"
> +    if test "$target_name" != "xenpv"; then
> +        echo "CONFIG_XEN_I386=y" >> $config_target_mak
> +        if test "$xen_pci_passthrough" = yes; then
> +            echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak"
> +        fi

You can add

CONFIG_XEN_PCI_PASSTHROUGH=$(CONFIG_XEN)

to i386-softmmu.mak and x86_64-softmmu.mak, and drop this setting from 
config-target.mak too.

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
  2014-01-23 22:30 ` [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Peter Maydell
@ 2014-01-24 14:23   ` Wei Liu
  2014-01-24 14:38     ` Paolo Bonzini
  2014-01-24 14:30   ` Paolo Bonzini
  1 sibling, 1 reply; 19+ messages in thread
From: Wei Liu @ 2014-01-24 14:23 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Anthony PERARD, Stefano Stabellini, Wei Liu, QEMU Developers,
	xen-devel@lists.xen.org

On Thu, Jan 23, 2014 at 10:30:16PM +0000, Peter Maydell wrote:
> On 23 January 2014 22:16, Wei Liu <wei.liu2@citrix.com> wrote:
> > As promised I hacked a prototype based on Paolo's disable TCG series.
> > However I coded some stubs for TCG anyway. So this series in principle
> > should work with / without Paolo's series.
> 
> I'm afraid I still think this is a terrible idea. "Xen" isn't a CPU, and

Thanks for being blunt. ;-)

> "the binary is smaller" isn't IMHO sufficient justification for breaking
> QEMU's basic structure of "target-* define target CPUs and we have
> a lot of compile time constants which are specific to a CPU which
> get defined there". How would you support a bigendian Xen CPU,
> just to pick one example of where this falls down?
> 

I think about this deeper. From Xen's (and I speculate this applies to
other hardware assisted virtulization solution as well) PoV only the
native endianess is supported, does it make sense to have a
target-native thing?

Wei.

> thanks -- PMM

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
  2014-01-23 22:30 ` [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Peter Maydell
  2014-01-24 14:23   ` Wei Liu
@ 2014-01-24 14:30   ` Paolo Bonzini
  2014-01-24 14:35     ` Peter Maydell
  1 sibling, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2014-01-24 14:30 UTC (permalink / raw)
  To: Peter Maydell, Wei Liu
  Cc: Anthony PERARD, xen-devel@lists.xen.org, QEMU Developers,
	Stefano Stabellini

Il 23/01/2014 23:30, Peter Maydell ha scritto:
>> > As promised I hacked a prototype based on Paolo's disable TCG series.
>> > However I coded some stubs for TCG anyway. So this series in principle
>> > should work with / without Paolo's series.
> I'm afraid I still think this is a terrible idea. "Xen" isn't a CPU, and
> "the binary is smaller" isn't IMHO sufficient justification for breaking
> QEMU's basic structure of "target-* define target CPUs and we have
> a lot of compile time constants which are specific to a CPU which
> get defined there". How would you support a bigendian Xen CPU,
> just to pick one example of where this falls down?

(1) decide that the Xen ring buffers are little-endian even on 
big-endian CPUs

(2) communicate the endianness of the Xen ring buffers via Xenstore, 
just like we do for sizeof(long), and let the guest use either 
endianness on any architecture.

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
  2014-01-24 14:30   ` Paolo Bonzini
@ 2014-01-24 14:35     ` Peter Maydell
  2014-01-24 14:42       ` Paolo Bonzini
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2014-01-24 14:35 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Anthony PERARD, xen-devel@lists.xen.org, Wei Liu, QEMU Developers,
	Stefano Stabellini

On 24 January 2014 14:30, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 23/01/2014 23:30, Peter Maydell ha scritto:
>> I'm afraid I still think this is a terrible idea. "Xen" isn't a CPU, and
>> "the binary is smaller" isn't IMHO sufficient justification for breaking
>> QEMU's basic structure of "target-* define target CPUs and we have
>> a lot of compile time constants which are specific to a CPU which
>> get defined there". How would you support a bigendian Xen CPU,
>> just to pick one example of where this falls down?
>
>
> (1) decide that the Xen ring buffers are little-endian even on big-endian
> CPUs
>
> (2) communicate the endianness of the Xen ring buffers via Xenstore, just
> like we do for sizeof(long), and let the guest use either endianness on any
> architecture.

You still have to make a choice about what you think
TARGET_WORDS_BIGENDIAN should be, and it's still going
to be wrong half the time and horribly confusing.
I just think this is completely the wrong solution to
the problem.

If Xen really wants a totally standalone binary of the
smallest possible size with just paravirtualized hardware
and minimal to no dependency on guest CPU architecture then
they should write one, along the lines of kvmtool :-)

thanks
-- PMM

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
  2014-01-24 14:23   ` Wei Liu
@ 2014-01-24 14:38     ` Paolo Bonzini
  2014-01-24 14:50       ` Wei Liu
  0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2014-01-24 14:38 UTC (permalink / raw)
  To: Wei Liu, Peter Maydell
  Cc: Anthony PERARD, xen-devel@lists.xen.org, QEMU Developers,
	Stefano Stabellini

Il 24/01/2014 15:23, Wei Liu ha scritto:
> On Thu, Jan 23, 2014 at 10:30:16PM +0000, Peter Maydell wrote:
>> On 23 January 2014 22:16, Wei Liu <wei.liu2@citrix.com> wrote:
>>> As promised I hacked a prototype based on Paolo's disable TCG series.
>>> However I coded some stubs for TCG anyway. So this series in principle
>>> should work with / without Paolo's series.
>>
>> I'm afraid I still think this is a terrible idea. "Xen" isn't a CPU, and
>
> Thanks for being blunt. ;-)
>
>> "the binary is smaller" isn't IMHO sufficient justification for breaking
>> QEMU's basic structure of "target-* define target CPUs and we have
>> a lot of compile time constants which are specific to a CPU which
>> get defined there". How would you support a bigendian Xen CPU,
>> just to pick one example of where this falls down?
>>
>
> I think about this deeper. From Xen's (and I speculate this applies to
> other hardware assisted virtulization solution as well) PoV only the
> native endianess is supported, does it make sense to have a
> target-native thing?

I think this is wrong, for a few reasons:

(1) xenpv is not hardware assisted virtualization

(2) supporting only native endianness leads to complications when 
systems are bi-endian, as is the case for PPC.  For example, virtio 1.0 
will always be little endian.

(3) there's a precedent for supporting different guests between the 
guest and the host in blkback, you can do the same for endianness.

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
  2014-01-24 14:35     ` Peter Maydell
@ 2014-01-24 14:42       ` Paolo Bonzini
  2014-01-24 14:56         ` Stefano Stabellini
  0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2014-01-24 14:42 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Anthony PERARD, xen-devel@lists.xen.org, Wei Liu, QEMU Developers,
	Stefano Stabellini

Il 24/01/2014 15:35, Peter Maydell ha scritto:
>> > (1) decide that the Xen ring buffers are little-endian even on big-endian
>> > CPUs
>> >
>> > (2) communicate the endianness of the Xen ring buffers via Xenstore, just
>> > like we do for sizeof(long), and let the guest use either endianness on any
>> > architecture.
> You still have to make a choice about what you think
> TARGET_WORDS_BIGENDIAN should be, and it's still going
> to be wrong half the time and horribly confusing.
> I just think this is completely the wrong solution to
> the problem.

Theoretically the xenpv-softmmu machine shouldn't need any code that 
depends on TARGET_WORDS_BIGENDIAN.

If we changed every #ifdef TARGET_WORDS_BIGENDIAN to if(), we could 
compile it with "#define TARGET_WORDS_BIGENDIAN abort()".

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
  2014-01-24 14:38     ` Paolo Bonzini
@ 2014-01-24 14:50       ` Wei Liu
  0 siblings, 0 replies; 19+ messages in thread
From: Wei Liu @ 2014-01-24 14:50 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Peter Maydell, Wei Liu, Stefano Stabellini, QEMU Developers,
	xen-devel@lists.xen.org, Anthony PERARD

On Fri, Jan 24, 2014 at 03:38:01PM +0100, Paolo Bonzini wrote:
> Il 24/01/2014 15:23, Wei Liu ha scritto:
> >On Thu, Jan 23, 2014 at 10:30:16PM +0000, Peter Maydell wrote:
> >>On 23 January 2014 22:16, Wei Liu <wei.liu2@citrix.com> wrote:
> >>>As promised I hacked a prototype based on Paolo's disable TCG series.
> >>>However I coded some stubs for TCG anyway. So this series in principle
> >>>should work with / without Paolo's series.
> >>
> >>I'm afraid I still think this is a terrible idea. "Xen" isn't a CPU, and
> >
> >Thanks for being blunt. ;-)
> >
> >>"the binary is smaller" isn't IMHO sufficient justification for breaking
> >>QEMU's basic structure of "target-* define target CPUs and we have
> >>a lot of compile time constants which are specific to a CPU which
> >>get defined there". How would you support a bigendian Xen CPU,
> >>just to pick one example of where this falls down?
> >>
> >
> >I think about this deeper. From Xen's (and I speculate this applies to
> >other hardware assisted virtulization solution as well) PoV only the
> >native endianess is supported, does it make sense to have a
> >target-native thing?
> 
> I think this is wrong, for a few reasons:
> 
> (1) xenpv is not hardware assisted virtualization
> 

Correct... What I really meant was "those virtualization solutions don't
care much about CPU emulation" (if there's any other than Xen).

> (2) supporting only native endianness leads to complications when
> systems are bi-endian, as is the case for PPC.  For example, virtio
> 1.0 will always be little endian.
> 

OK, good to know.

> (3) there's a precedent for supporting different guests between the
> guest and the host in blkback, you can do the same for endianness.
> 

Yes. But this is still open question at the moment, as there's no
canonical protocol for this (it hasn't even been discussed).

Your second point is the thing I missed though. Thanks for reminding.

Wei.

> Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
  2014-01-24 14:42       ` Paolo Bonzini
@ 2014-01-24 14:56         ` Stefano Stabellini
  2014-01-24 15:22           ` [Qemu-devel] [Xen-devel] " Ian Campbell
  0 siblings, 1 reply; 19+ messages in thread
From: Stefano Stabellini @ 2014-01-24 14:56 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Peter Maydell, Wei Liu, Stefano Stabellini, QEMU Developers,
	xen-devel@lists.xen.org, Anthony PERARD

On Fri, 24 Jan 2014, Paolo Bonzini wrote:
> Il 24/01/2014 15:35, Peter Maydell ha scritto:
> > > > (1) decide that the Xen ring buffers are little-endian even on
> > > big-endian
> > > > CPUs
> > > >
> > > > (2) communicate the endianness of the Xen ring buffers via Xenstore,
> > > just
> > > > like we do for sizeof(long), and let the guest use either endianness on
> > > any
> > > > architecture.
> > You still have to make a choice about what you think
> > TARGET_WORDS_BIGENDIAN should be, and it's still going
> > to be wrong half the time and horribly confusing.
> > I just think this is completely the wrong solution to
> > the problem.
> 
> Theoretically the xenpv-softmmu machine shouldn't need any code that depends
> on TARGET_WORDS_BIGENDIAN.
> 
> If we changed every #ifdef TARGET_WORDS_BIGENDIAN to if(), we could compile it
> with "#define TARGET_WORDS_BIGENDIAN abort()".

Right. All our PV protocols are little endian by definition.

Besides I don't know how to state this more clearly but there are no big
endian Xen guests. There have never been any big endian Xen guests.
There are no big endian Xen guests on the roadmap.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [Xen-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
  2014-01-24 14:56         ` Stefano Stabellini
@ 2014-01-24 15:22           ` Ian Campbell
  0 siblings, 0 replies; 19+ messages in thread
From: Ian Campbell @ 2014-01-24 15:22 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Peter Maydell, Wei Liu, QEMU Developers, xen-devel@lists.xen.org,
	Anthony PERARD, Paolo Bonzini

On Fri, 2014-01-24 at 14:56 +0000, Stefano Stabellini wrote:
> Besides I don't know how to state this more clearly but there are no big
> endian Xen guests. There have never been any big endian Xen guests.
> There are no big endian Xen guests on the roadmap.

That is perhaps a bit strong, but what is 100% true is that none of the
defined Xen PV protocols (listed at [0]) is big endian.

If someone were to build e.g. an armbe guest then they would either have
to do the swapping in the frontend (and continue to claim to be the l.e.
XEN_IO_PROTO_ABI_ARM) or they would have to define
XEN_IO_PROTO_ABI_ARMBE and arrange for the front and backend to
negotiate as appropriate, and that would be the appropriate point in
time for the backends to be taught about non-native endianess IMHO.

[0] http://xenbits.xen.org/docs/unstable-staging/hypercall/x86_64/include,public,io,protocols.h.html

Ian.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 5/5] xen: introduce xenpv-softmmu.mak
  2014-01-24  7:38   ` Paolo Bonzini
@ 2014-01-24 17:00     ` Wei Liu
  0 siblings, 0 replies; 19+ messages in thread
From: Wei Liu @ 2014-01-24 17:00 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: peter.maydell, Wei Liu, stefano.stabellini, qemu-devel, xen-devel,
	anthony.perard

On Fri, Jan 24, 2014 at 08:38:19AM +0100, Paolo Bonzini wrote:
> Il 23/01/2014 23:16, Wei Liu ha scritto:
> >-        echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak"
> >+    if test "$target_name" != "xenpv"; then
> >+        echo "CONFIG_XEN_I386=y" >> $config_target_mak
> >+        if test "$xen_pci_passthrough" = yes; then
> >+            echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak"
> >+        fi
> 
> You can add
> 
> CONFIG_XEN_PCI_PASSTHROUGH=$(CONFIG_XEN)
> 
> to i386-softmmu.mak and x86_64-softmmu.mak, and drop this setting
> from config-target.mak too.
> 

I'm afraid not. CONFIG_XEN is prerequiste for CONFIG_XEN_PCI_PASSTHROUGH
but doens't imply the feature is enabled by the user.

Wei.

> Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2014-01-24 17:00 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-23 22:16 [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Wei Liu
2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 1/5] xen: move Xen PV machine files to hw/xenpv Wei Liu
2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 2/5] xen: factor out common functions Wei Liu
2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 3/5] exec: guard Xen HVM hooks with CONFIG_XEN_I386 Wei Liu
2014-01-24  7:35   ` Paolo Bonzini
2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 4/5] xen: implement Xen PV target Wei Liu
2014-01-23 22:16 ` [Qemu-devel] [PATCH RFC 5/5] xen: introduce xenpv-softmmu.mak Wei Liu
2014-01-24  7:38   ` Paolo Bonzini
2014-01-24 17:00     ` Wei Liu
2014-01-23 22:30 ` [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target Peter Maydell
2014-01-24 14:23   ` Wei Liu
2014-01-24 14:38     ` Paolo Bonzini
2014-01-24 14:50       ` Wei Liu
2014-01-24 14:30   ` Paolo Bonzini
2014-01-24 14:35     ` Peter Maydell
2014-01-24 14:42       ` Paolo Bonzini
2014-01-24 14:56         ` Stefano Stabellini
2014-01-24 15:22           ` [Qemu-devel] [Xen-devel] " Ian Campbell
2014-01-24  7:36 ` [Qemu-devel] " Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).