* [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
@ 2012-06-05 11:22 ZhouPeng
2012-06-06 13:05 ` Ian Campbell
0 siblings, 1 reply; 18+ messages in thread
From: ZhouPeng @ 2012-06-05 11:22 UTC (permalink / raw)
To: Ian Campbell; +Cc: Xen-Devel (E-mail), ian.jackson, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 5740 bytes --]
changeset: 25454:1804a873a64d
tag: tip
user: Zhou Peng <ailvpeng25@gmail.com>
date: Tue Jun 05 17:56:46 2012 +0800
files: tools/libxl/libxl_create.c tools/libxl/libxl_dm.c
tools/libxl/libxl_types.idl tools/libxl/xl_cmdimpl.c
description:
tools:libxl: Add qxl vga interface support.
Usage:
qxl=1
qxlvram=64
qxlram=64
Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Tue Jun 05 17:39:37 2012 +0800
+++ b/tools/libxl/libxl_create.c Tue Jun 05 17:56:46 2012 +0800
@@ -23,6 +23,32 @@
#include <xc_dom.h>
#include <xenguest.h>
+/*
+ * For qxl vga interface, the total video mem is determined by
+ * RAM bar and VRAM bar. But it is not simply linearly determined,
+ * get_qxl_ram_size below gives the details.
+ */
+static inline uint32_t msb_mask(uint32_t val)
+{
+ uint32_t mask;
+
+ do {
+ mask = ~(val - 1) & val;
+ val &= ~mask;
+ } while (mask < val);
+
+ return mask;
+}
+
+static inline uint32_t get_qxl_ram_size(uint32_t vram_sizekb,
+ uint32_t ram_sizekb)
+{
+ uint32_t vram = msb_mask(2 * vram_sizekb * 1024 - 1);
+ uint32_t ram = msb_mask(2 * ram_sizekb * 1024 - 1);
+
+ return (vram + ram + 1023) / 1024;
+}
+
void libxl_domain_config_init(libxl_domain_config *d_config)
{
memset(d_config, 0, sizeof(*d_config));
@@ -195,6 +221,25 @@ int libxl__domain_build_info_setdefault(
if (b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_DEFAULT)
b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
+ && b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_QXL) {
+ if (b_info->u.hvm.vga.vramkb == LIBXL_MEMKB_DEFAULT)
+ b_info->u.hvm.vga.vramkb = 64 * 1024;
+ if (b_info->u.hvm.vga.ramkb == LIBXL_MEMKB_DEFAULT)
+ b_info->u.hvm.vga.ramkb = 64 * 1024;
+ uint32_t qxl_ram = get_qxl_ram_size(b_info->u.hvm.vga.vramkb,
+ b_info->u.hvm.vga.ramkb);
+ /*
+ * video_memkb is the real size of video memory to assign.
+ * If video_memkb can't meet the need of qxl, adjust it
+ * accordingly.
+ */
+ if ((b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
+ || (b_info->video_memkb < qxl_ram)) {
+ b_info->video_memkb = qxl_ram;
+ }
+ }
+
libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true);
if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) {
libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true);
diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Tue Jun 05 17:39:37 2012 +0800
+++ b/tools/libxl/libxl_dm.c Tue Jun 05 17:56:46 2012 +0800
@@ -181,6 +181,8 @@ static char ** libxl__build_device_model
flexarray_append(dm_args, "-std-vga");
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
break;
}
@@ -429,6 +431,17 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ flexarray_vappend(dm_args, "-vga", "qxl", NULL);
+ flexarray_vappend(dm_args, "-global",
+ GCSPRINTF("qxl-vga.vram_size=%lu",
+ b_info->u.hvm.vga.vramkb * 1024),
+ NULL);
+ flexarray_vappend(dm_args, "-global",
+ GCSPRINTF("qxl-vga.ram_size=%lu",
+ b_info->u.hvm.vga.ramkb * 1024),
+ NULL);
break;
}
diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Tue Jun 05 17:39:37 2012 +0800
+++ b/tools/libxl/libxl_types.idl Tue Jun 05 17:56:46 2012 +0800
@@ -128,6 +128,7 @@ libxl_vga_interface_type = Enumeration("
libxl_vga_interface_type = Enumeration("vga_interface_type", [
(0, "CIRRUS"),
(1, "STD"),
+ (2, "QXL"),
], init_val = "LIBXL_VGA_INTERFACE_TYPE_DEFAULT")
#
@@ -136,6 +137,10 @@ libxl_vga_interface_type = Enumeration("
libxl_vga_interface_info = Struct("vga_interface_info", [
("type", libxl_vga_interface_type),
+ # VRAM bar for qxl
+ ("vramkb", MemKB),
+ # RAM bar for qxl
+ ("ramkb", MemKB),
])
libxl_vnc_info = Struct("vnc_info", [
diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:39:37 2012 +0800
+++ b/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:56:46 2012 +0800
@@ -1260,6 +1260,16 @@ skip_vfb:
if (l)
b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_STD;
+ if (!xlu_cfg_get_long(config, "qxl", &l, 0)) {
+ if (l) {
+ b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_QXL;
+ if (!xlu_cfg_get_long (config, "qxlvram", &l, 0))
+ b_info->u.hvm.vga.vramkb = l * 1024;
+ if (!xlu_cfg_get_long (config, "qxlram", &l, 0))
+ b_info->u.hvm.vga.ramkb = l * 1024;
+ }
+ }
+
xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
xlu_cfg_replace_string (config, "vnclisten",
&b_info->u.hvm.vnc.listen, 0);
--
Zhou Peng
[-- Attachment #2: spice.tools.libxl.qxl.support.v2.diff --]
[-- Type: application/octet-stream, Size: 5733 bytes --]
changeset: 25454:1804a873a64d
tag: tip
user: Zhou Peng <ailvpeng25@gmail.com>
date: Tue Jun 05 17:56:46 2012 +0800
files: tools/libxl/libxl_create.c tools/libxl/libxl_dm.c tools/libxl/libxl_types.idl tools/libxl/xl_cmdimpl.c
description:
tools:libxl: Add qxl vga interface support.
Usage:
qxl=1
qxlvram=64
qxlram=64
Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Tue Jun 05 17:39:37 2012 +0800
+++ b/tools/libxl/libxl_create.c Tue Jun 05 17:56:46 2012 +0800
@@ -23,6 +23,32 @@
#include <xc_dom.h>
#include <xenguest.h>
+/*
+ * For qxl vga interface, the total video mem is determined by
+ * RAM bar and VRAM bar. But it is not simply linearly determined,
+ * get_qxl_ram_size below gives the details.
+ */
+static inline uint32_t msb_mask(uint32_t val)
+{
+ uint32_t mask;
+
+ do {
+ mask = ~(val - 1) & val;
+ val &= ~mask;
+ } while (mask < val);
+
+ return mask;
+}
+
+static inline uint32_t get_qxl_ram_size(uint32_t vram_sizekb,
+ uint32_t ram_sizekb)
+{
+ uint32_t vram = msb_mask(2 * vram_sizekb * 1024 - 1);
+ uint32_t ram = msb_mask(2 * ram_sizekb * 1024 - 1);
+
+ return (vram + ram + 1023) / 1024;
+}
+
void libxl_domain_config_init(libxl_domain_config *d_config)
{
memset(d_config, 0, sizeof(*d_config));
@@ -195,6 +221,25 @@ int libxl__domain_build_info_setdefault(
if (b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_DEFAULT)
b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
+ && b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_QXL) {
+ if (b_info->u.hvm.vga.vramkb == LIBXL_MEMKB_DEFAULT)
+ b_info->u.hvm.vga.vramkb = 64 * 1024;
+ if (b_info->u.hvm.vga.ramkb == LIBXL_MEMKB_DEFAULT)
+ b_info->u.hvm.vga.ramkb = 64 * 1024;
+ uint32_t qxl_ram = get_qxl_ram_size(b_info->u.hvm.vga.vramkb,
+ b_info->u.hvm.vga.ramkb);
+ /*
+ * video_memkb is the real size of video memory to assign.
+ * If video_memkb can't meet the need of qxl, adjust it
+ * accordingly.
+ */
+ if ((b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
+ || (b_info->video_memkb < qxl_ram)) {
+ b_info->video_memkb = qxl_ram;
+ }
+ }
+
libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true);
if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) {
libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true);
diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Tue Jun 05 17:39:37 2012 +0800
+++ b/tools/libxl/libxl_dm.c Tue Jun 05 17:56:46 2012 +0800
@@ -181,6 +181,8 @@ static char ** libxl__build_device_model
flexarray_append(dm_args, "-std-vga");
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
break;
}
@@ -429,6 +431,17 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ flexarray_vappend(dm_args, "-vga", "qxl", NULL);
+ flexarray_vappend(dm_args, "-global",
+ GCSPRINTF("qxl-vga.vram_size=%lu",
+ b_info->u.hvm.vga.vramkb * 1024),
+ NULL);
+ flexarray_vappend(dm_args, "-global",
+ GCSPRINTF("qxl-vga.ram_size=%lu",
+ b_info->u.hvm.vga.ramkb * 1024),
+ NULL);
break;
}
diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Tue Jun 05 17:39:37 2012 +0800
+++ b/tools/libxl/libxl_types.idl Tue Jun 05 17:56:46 2012 +0800
@@ -128,6 +128,7 @@ libxl_vga_interface_type = Enumeration("
libxl_vga_interface_type = Enumeration("vga_interface_type", [
(0, "CIRRUS"),
(1, "STD"),
+ (2, "QXL"),
], init_val = "LIBXL_VGA_INTERFACE_TYPE_DEFAULT")
#
@@ -136,6 +137,10 @@ libxl_vga_interface_type = Enumeration("
libxl_vga_interface_info = Struct("vga_interface_info", [
("type", libxl_vga_interface_type),
+ # VRAM bar for qxl
+ ("vramkb", MemKB),
+ # RAM bar for qxl
+ ("ramkb", MemKB),
])
libxl_vnc_info = Struct("vnc_info", [
diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:39:37 2012 +0800
+++ b/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:56:46 2012 +0800
@@ -1260,6 +1260,16 @@ skip_vfb:
if (l)
b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_STD;
+ if (!xlu_cfg_get_long(config, "qxl", &l, 0)) {
+ if (l) {
+ b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_QXL;
+ if (!xlu_cfg_get_long (config, "qxlvram", &l, 0))
+ b_info->u.hvm.vga.vramkb = l * 1024;
+ if (!xlu_cfg_get_long (config, "qxlram", &l, 0))
+ b_info->u.hvm.vga.ramkb = l * 1024;
+ }
+ }
+
xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
xlu_cfg_replace_string (config, "vnclisten",
&b_info->u.hvm.vnc.listen, 0);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-06-05 11:22 [PATCH 2/3] tools:libxl: Add qxl vga interface support v2 ZhouPeng
@ 2012-06-06 13:05 ` Ian Campbell
2012-06-07 3:19 ` ZhouPeng
0 siblings, 1 reply; 18+ messages in thread
From: Ian Campbell @ 2012-06-06 13:05 UTC (permalink / raw)
To: ZhouPeng; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini
On Tue, 2012-06-05 at 12:22 +0100, ZhouPeng wrote:
> changeset: 25454:1804a873a64d
> tag: tip
> user: Zhou Peng <ailvpeng25@gmail.com>
> date: Tue Jun 05 17:56:46 2012 +0800
> files: tools/libxl/libxl_create.c tools/libxl/libxl_dm.c
> tools/libxl/libxl_types.idl tools/libxl/xl_cmdimpl.c
> description:
> tools:libxl: Add qxl vga interface support.
>
> Usage:
> qxl=1
> qxlvram=64
> qxlram=64
>
> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
Thanks.
As previously mentioned this is 4.3 material. Please can you resubmit
once the 4.3 dev cycle opens.
> diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_create.c
> --- a/tools/libxl/libxl_create.c Tue Jun 05 17:39:37 2012 +0800
> +++ b/tools/libxl/libxl_create.c Tue Jun 05 17:56:46 2012 +0800
> @@ -23,6 +23,32 @@
> #include <xc_dom.h>
> #include <xenguest.h>
>
> +/*
> + * For qxl vga interface, the total video mem is determined by
> + * RAM bar and VRAM bar. But it is not simply linearly determined,
> + * get_qxl_ram_size below gives the details.
>From this statement I expected get_qxl_ram_size to have a nice comment
explaining what is going on, but it doesn't have this.
Can you please explain somewhere how this value is determined (I can see
it is not simple ;-)). Perhaps a link to some QXL/qemu document
discussing these parameters would be helpful too?
> + */
> +static inline uint32_t msb_mask(uint32_t val)
> +{
> + uint32_t mask;
> +
> + do {
> + mask = ~(val - 1) & val;
> + val &= ~mask;
> + } while (mask < val);
> +
> + return mask;
> +}
> +
> +static inline uint32_t get_qxl_ram_size(uint32_t vram_sizekb,
> + uint32_t ram_sizekb)
> +{
> + uint32_t vram = msb_mask(2 * vram_sizekb * 1024 - 1);
> + uint32_t ram = msb_mask(2 * ram_sizekb * 1024 - 1);
Why 2*?
> +
> + return (vram + ram + 1023) / 1024;
> +}
> +
> void libxl_domain_config_init(libxl_domain_config *d_config)
> {
> memset(d_config, 0, sizeof(*d_config));
> @@ -195,6 +221,25 @@ int libxl__domain_build_info_setdefault(
>
> if (b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_DEFAULT)
> b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> + if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
> + && b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_QXL) {
> + if (b_info->u.hvm.vga.vramkb == LIBXL_MEMKB_DEFAULT)
> + b_info->u.hvm.vga.vramkb = 64 * 1024;
> + if (b_info->u.hvm.vga.ramkb == LIBXL_MEMKB_DEFAULT)
> + b_info->u.hvm.vga.ramkb = 64 * 1024;
> + uint32_t qxl_ram = get_qxl_ram_size(b_info->u.hvm.vga.vramkb,
> + b_info->u.hvm.vga.ramkb);
> + /*
> + * video_memkb is the real size of video memory to assign.
> + * If video_memkb can't meet the need of qxl, adjust it
> + * accordingly.
> + */
> + if ((b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> + || (b_info->video_memkb < qxl_ram)) {
> + b_info->video_memkb = qxl_ram;
If video_memkb is != LIBXL_MEMKB_DEFAULT and it is not sufficiently
large then I think the correct thing to do is to error out and return
ERROR_INVAL.
If it is == LIBXL_MEMKB_DEFAULT then of course you can feel free to
override as necessary.
e.g.
if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
b_info->video_memkb = qxl_ram;
if (b_info->video_memkb < qxl_ram) {
LOG(...)
return ERROR_INVAL;
}
> + }
> + }
> +
> libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true);
> if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) {
> libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true);
> diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.c Tue Jun 05 17:39:37 2012 +0800
> +++ b/tools/libxl/libxl_dm.c Tue Jun 05 17:56:46 2012 +0800
> @@ -181,6 +181,8 @@ static char ** libxl__build_device_model
> flexarray_append(dm_args, "-std-vga");
> break;
> case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> + break;
> + case LIBXL_VGA_INTERFACE_TYPE_QXL:
> break;
> }
>
> @@ -429,6 +431,17 @@ static char ** libxl__build_device_model
> break;
> case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
> + break;
> + case LIBXL_VGA_INTERFACE_TYPE_QXL:
> + flexarray_vappend(dm_args, "-vga", "qxl", NULL);
> + flexarray_vappend(dm_args, "-global",
> + GCSPRINTF("qxl-vga.vram_size=%lu",
> + b_info->u.hvm.vga.vramkb * 1024),
> + NULL);
> + flexarray_vappend(dm_args, "-global",
> + GCSPRINTF("qxl-vga.ram_size=%lu",
> + b_info->u.hvm.vga.ramkb * 1024),
> + NULL);
> break;
> }
>
> diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl Tue Jun 05 17:39:37 2012 +0800
> +++ b/tools/libxl/libxl_types.idl Tue Jun 05 17:56:46 2012 +0800
> @@ -128,6 +128,7 @@ libxl_vga_interface_type = Enumeration("
> libxl_vga_interface_type = Enumeration("vga_interface_type", [
> (0, "CIRRUS"),
> (1, "STD"),
> + (2, "QXL"),
> ], init_val = "LIBXL_VGA_INTERFACE_TYPE_DEFAULT")
>
> #
> @@ -136,6 +137,10 @@ libxl_vga_interface_type = Enumeration("
>
> libxl_vga_interface_info = Struct("vga_interface_info", [
> ("type", libxl_vga_interface_type),
> + # VRAM bar for qxl
> + ("vramkb", MemKB),
> + # RAM bar for qxl
> + ("ramkb", MemKB),
> ])
>
> libxl_vnc_info = Struct("vnc_info", [
> diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:39:37 2012 +0800
> +++ b/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:56:46 2012 +0800
> @@ -1260,6 +1260,16 @@ skip_vfb:
> if (l)
> b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_STD;
>
> + if (!xlu_cfg_get_long(config, "qxl", &l, 0)) {
> + if (l) {
> + b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_QXL;
> + if (!xlu_cfg_get_long (config, "qxlvram", &l, 0))
> + b_info->u.hvm.vga.vramkb = l * 1024;
> + if (!xlu_cfg_get_long (config, "qxlram", &l, 0))
> + b_info->u.hvm.vga.ramkb = l * 1024;
> + }
> + }
> +
> xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
> xlu_cfg_replace_string (config, "vnclisten",
> &b_info->u.hvm.vnc.listen, 0);
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-06-06 13:05 ` Ian Campbell
@ 2012-06-07 3:19 ` ZhouPeng
2012-06-07 6:08 ` Ian Campbell
0 siblings, 1 reply; 18+ messages in thread
From: ZhouPeng @ 2012-06-07 3:19 UTC (permalink / raw)
To: Ian Campbell; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini
On Wed, Jun 6, 2012 at 9:05 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Tue, 2012-06-05 at 12:22 +0100, ZhouPeng wrote:
>> changeset: 25454:1804a873a64d
>> tag: tip
>> user: Zhou Peng <ailvpeng25@gmail.com>
>> date: Tue Jun 05 17:56:46 2012 +0800
>> files: tools/libxl/libxl_create.c tools/libxl/libxl_dm.c
>> tools/libxl/libxl_types.idl tools/libxl/xl_cmdimpl.c
>> description:
>> tools:libxl: Add qxl vga interface support.
>>
>> Usage:
>> qxl=1
>> qxlvram=64
>> qxlram=64
>>
>> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
>
> Thanks.
>
> As previously mentioned this is 4.3 material. Please can you resubmit
> once the 4.3 dev cycle opens.
ok, thanks.
>
>> diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_create.c
>> --- a/tools/libxl/libxl_create.c Tue Jun 05 17:39:37 2012 +0800
>> +++ b/tools/libxl/libxl_create.c Tue Jun 05 17:56:46 2012 +0800
>> @@ -23,6 +23,32 @@
>> #include <xc_dom.h>
>> #include <xenguest.h>
>>
>> +/*
>> + * For qxl vga interface, the total video mem is determined by
>> + * RAM bar and VRAM bar. But it is not simply linearly determined,
>> + * get_qxl_ram_size below gives the details.
>
> From this statement I expected get_qxl_ram_size to have a nice comment
> explaining what is going on, but it doesn't have this.
>
> Can you please explain somewhere how this value is determined (I can see
> it is not simple ;-)). Perhaps a link to some QXL/qemu document
> discussing these parameters would be helpful too?
Sorry, there is not a piece of docs on ram bar and vram bar, and how
the total size
of qxl video memory is calculated from ram bar and vram bar parameters.
But from my digging into qxl's source code and debugging, it works this way.
I asked similar question in spice-list and get response from:
http://comments.gmane.org/gmane.comp.emulators.spice.devel/9501
Any way, I will rich the document if get more info.
>> + */
>> +static inline uint32_t msb_mask(uint32_t val)
>> +{
>> + uint32_t mask;
>> +
>> + do {
>> + mask = ~(val - 1) & val;
>> + val &= ~mask;
>> + } while (mask < val);
>> +
>> + return mask;
>> +}
>> +
>> +static inline uint32_t get_qxl_ram_size(uint32_t vram_sizekb,
>> + uint32_t ram_sizekb)
>> +{
>> + uint32_t vram = msb_mask(2 * vram_sizekb * 1024 - 1);
>> + uint32_t ram = msb_mask(2 * ram_sizekb * 1024 - 1);
>
> Why 2*?
I don't know why yet, but it works this way in qxl.
>
>> +
>> + return (vram + ram + 1023) / 1024;
>> +}
>> +
>> void libxl_domain_config_init(libxl_domain_config *d_config)
>> {
>> memset(d_config, 0, sizeof(*d_config));
>> @@ -195,6 +221,25 @@ int libxl__domain_build_info_setdefault(
>>
>> if (b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_DEFAULT)
>> b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
>> + if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
>> + && b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_QXL) {
>> + if (b_info->u.hvm.vga.vramkb == LIBXL_MEMKB_DEFAULT)
>> + b_info->u.hvm.vga.vramkb = 64 * 1024;
>> + if (b_info->u.hvm.vga.ramkb == LIBXL_MEMKB_DEFAULT)
>> + b_info->u.hvm.vga.ramkb = 64 * 1024;
>> + uint32_t qxl_ram = get_qxl_ram_size(b_info->u.hvm.vga.vramkb,
>> + b_info->u.hvm.vga.ramkb);
>> + /*
>> + * video_memkb is the real size of video memory to assign.
>> + * If video_memkb can't meet the need of qxl, adjust it
>> + * accordingly.
>> + */
>> + if ((b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
>> + || (b_info->video_memkb < qxl_ram)) {
>> + b_info->video_memkb = qxl_ram;
>
> If video_memkb is != LIBXL_MEMKB_DEFAULT and it is not sufficiently
> large then I think the correct thing to do is to error out and return
> ERROR_INVAL.
Not agreed.
This will let user must to set a proper value to meet qxl, but from
discussing above,
it's difficult for user to give this decision.
qxlram and qxlvram parameters are enough for user to set qxl's video
ram (libvirt also use
these two parameters).
>
> If it is == LIBXL_MEMKB_DEFAULT then of course you can feel free to
> override as necessary.
>
> e.g.
> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> b_info->video_memkb = qxl_ram;
> if (b_info->video_memkb < qxl_ram) {
> LOG(...)
> return ERROR_INVAL;
> }
>
>> + }
>> + }
>> +
>> libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true);
>> if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) {
>> libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true);
>> diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_dm.c
>> --- a/tools/libxl/libxl_dm.c Tue Jun 05 17:39:37 2012 +0800
>> +++ b/tools/libxl/libxl_dm.c Tue Jun 05 17:56:46 2012 +0800
>> @@ -181,6 +181,8 @@ static char ** libxl__build_device_model
>> flexarray_append(dm_args, "-std-vga");
>> break;
>> case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>> + break;
>> + case LIBXL_VGA_INTERFACE_TYPE_QXL:
>> break;
>> }
>>
>> @@ -429,6 +431,17 @@ static char ** libxl__build_device_model
>> break;
>> case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>> flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
>> + break;
>> + case LIBXL_VGA_INTERFACE_TYPE_QXL:
>> + flexarray_vappend(dm_args, "-vga", "qxl", NULL);
>> + flexarray_vappend(dm_args, "-global",
>> + GCSPRINTF("qxl-vga.vram_size=%lu",
>> + b_info->u.hvm.vga.vramkb * 1024),
>> + NULL);
>> + flexarray_vappend(dm_args, "-global",
>> + GCSPRINTF("qxl-vga.ram_size=%lu",
>> + b_info->u.hvm.vga.ramkb * 1024),
>> + NULL);
>> break;
>> }
>>
>> diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_types.idl
>> --- a/tools/libxl/libxl_types.idl Tue Jun 05 17:39:37 2012 +0800
>> +++ b/tools/libxl/libxl_types.idl Tue Jun 05 17:56:46 2012 +0800
>> @@ -128,6 +128,7 @@ libxl_vga_interface_type = Enumeration("
>> libxl_vga_interface_type = Enumeration("vga_interface_type", [
>> (0, "CIRRUS"),
>> (1, "STD"),
>> + (2, "QXL"),
>> ], init_val = "LIBXL_VGA_INTERFACE_TYPE_DEFAULT")
>>
>> #
>> @@ -136,6 +137,10 @@ libxl_vga_interface_type = Enumeration("
>>
>> libxl_vga_interface_info = Struct("vga_interface_info", [
>> ("type", libxl_vga_interface_type),
>> + # VRAM bar for qxl
>> + ("vramkb", MemKB),
>> + # RAM bar for qxl
>> + ("ramkb", MemKB),
>> ])
>>
>> libxl_vnc_info = Struct("vnc_info", [
>> diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/xl_cmdimpl.c
>> --- a/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:39:37 2012 +0800
>> +++ b/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:56:46 2012 +0800
>> @@ -1260,6 +1260,16 @@ skip_vfb:
>> if (l)
>> b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_STD;
>>
>> + if (!xlu_cfg_get_long(config, "qxl", &l, 0)) {
>> + if (l) {
>> + b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_QXL;
>> + if (!xlu_cfg_get_long (config, "qxlvram", &l, 0))
>> + b_info->u.hvm.vga.vramkb = l * 1024;
>> + if (!xlu_cfg_get_long (config, "qxlram", &l, 0))
>> + b_info->u.hvm.vga.ramkb = l * 1024;
>> + }
>> + }
>> +
>> xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
>> xlu_cfg_replace_string (config, "vnclisten",
>> &b_info->u.hvm.vnc.listen, 0);
>>
>>
>
>
--
Zhou Peng
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-06-07 3:19 ` ZhouPeng
@ 2012-06-07 6:08 ` Ian Campbell
2012-06-07 7:49 ` ZhouPeng
2012-07-03 11:30 ` ZhouPeng
0 siblings, 2 replies; 18+ messages in thread
From: Ian Campbell @ 2012-06-07 6:08 UTC (permalink / raw)
To: ZhouPeng; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini
On Thu, 2012-06-07 at 04:19 +0100, ZhouPeng wrote:
> On Wed, Jun 6, 2012 at 9:05 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Tue, 2012-06-05 at 12:22 +0100, ZhouPeng wrote:
> >> changeset: 25454:1804a873a64d
> >> tag: tip
> >> user: Zhou Peng <ailvpeng25@gmail.com>
> >> date: Tue Jun 05 17:56:46 2012 +0800
> >> files: tools/libxl/libxl_create.c tools/libxl/libxl_dm.c
> >> tools/libxl/libxl_types.idl tools/libxl/xl_cmdimpl.c
> >> description:
> >> tools:libxl: Add qxl vga interface support.
> >>
> >> Usage:
> >> qxl=1
> >> qxlvram=64
> >> qxlram=64
> >>
> >> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
> >
> > Thanks.
> >
> > As previously mentioned this is 4.3 material. Please can you resubmit
> > once the 4.3 dev cycle opens.
> ok, thanks.
> >
> >> diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_create.c
> >> --- a/tools/libxl/libxl_create.c Tue Jun 05 17:39:37 2012 +0800
> >> +++ b/tools/libxl/libxl_create.c Tue Jun 05 17:56:46 2012 +0800
> >> @@ -23,6 +23,32 @@
> >> #include <xc_dom.h>
> >> #include <xenguest.h>
> >>
> >> +/*
> >> + * For qxl vga interface, the total video mem is determined by
> >> + * RAM bar and VRAM bar. But it is not simply linearly determined,
> >> + * get_qxl_ram_size below gives the details.
> >
> > From this statement I expected get_qxl_ram_size to have a nice comment
> > explaining what is going on, but it doesn't have this.
> >
> > Can you please explain somewhere how this value is determined (I can see
> > it is not simple ;-)). Perhaps a link to some QXL/qemu document
> > discussing these parameters would be helpful too?
>
> Sorry, there is not a piece of docs on ram bar and vram bar, and how
> the total size
> of qxl video memory is calculated from ram bar and vram bar parameters.
>
> But from my digging into qxl's source code and debugging, it works this way.
>
> I asked similar question in spice-list and get response from:
> http://comments.gmane.org/gmane.comp.emulators.spice.devel/9501
>
> Any way, I will rich the document if get more info.
OK, thanks.
> >> +
> >> + return (vram + ram + 1023) / 1024;
> >> +}
> >> +
> >> void libxl_domain_config_init(libxl_domain_config *d_config)
> >> {
> >> memset(d_config, 0, sizeof(*d_config));
> >> @@ -195,6 +221,25 @@ int libxl__domain_build_info_setdefault(
> >>
> >> if (b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_DEFAULT)
> >> b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> >> + if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
> >> + && b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_QXL) {
> >> + if (b_info->u.hvm.vga.vramkb == LIBXL_MEMKB_DEFAULT)
> >> + b_info->u.hvm.vga.vramkb = 64 * 1024;
> >> + if (b_info->u.hvm.vga.ramkb == LIBXL_MEMKB_DEFAULT)
> >> + b_info->u.hvm.vga.ramkb = 64 * 1024;
> >> + uint32_t qxl_ram = get_qxl_ram_size(b_info->u.hvm.vga.vramkb,
> >> + b_info->u.hvm.vga.ramkb);
> >> + /*
> >> + * video_memkb is the real size of video memory to assign.
> >> + * If video_memkb can't meet the need of qxl, adjust it
> >> + * accordingly.
> >> + */
> >> + if ((b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> >> + || (b_info->video_memkb < qxl_ram)) {
> >> + b_info->video_memkb = qxl_ram;
> >
> > If video_memkb is != LIBXL_MEMKB_DEFAULT and it is not sufficiently
> > large then I think the correct thing to do is to error out and return
> > ERROR_INVAL.
>
> Not agreed.
> This will let user must to set a proper value to meet qxl, but from
> discussing above, it's difficult for user to give this decision.
> qxlram and qxlvram parameters are enough for user to set qxl's video
> ram (libvirt also use these two parameters).
If the user has asked for a specific amount of video RAM which is not
sufficient then the correct answer is to log an error (including the
required minimum) and exit.
You are correct that it is hard to figure out what "enough" RAM is. I
expect that for that reason almost all users won't pass any of these
arguments and will just accept the default, which will work just fine.
Ian.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-06-07 6:08 ` Ian Campbell
@ 2012-06-07 7:49 ` ZhouPeng
2012-07-03 11:30 ` ZhouPeng
1 sibling, 0 replies; 18+ messages in thread
From: ZhouPeng @ 2012-06-07 7:49 UTC (permalink / raw)
To: Ian Campbell; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini
On Thu, Jun 7, 2012 at 2:08 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2012-06-07 at 04:19 +0100, ZhouPeng wrote:
>> On Wed, Jun 6, 2012 at 9:05 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Tue, 2012-06-05 at 12:22 +0100, ZhouPeng wrote:
>> >> changeset: 25454:1804a873a64d
>> >> tag: tip
>> >> user: Zhou Peng <ailvpeng25@gmail.com>
>> >> date: Tue Jun 05 17:56:46 2012 +0800
>> >> files: tools/libxl/libxl_create.c tools/libxl/libxl_dm.c
>> >> tools/libxl/libxl_types.idl tools/libxl/xl_cmdimpl.c
>> >> description:
>> >> tools:libxl: Add qxl vga interface support.
>> >>
>> >> Usage:
>> >> qxl=1
>> >> qxlvram=64
>> >> qxlram=64
>> >>
>> >> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
>> >
>> > Thanks.
>> >
>> > As previously mentioned this is 4.3 material. Please can you resubmit
>> > once the 4.3 dev cycle opens.
>> ok, thanks.
>> >
>> >> diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_create.c
>> >> --- a/tools/libxl/libxl_create.c Tue Jun 05 17:39:37 2012 +0800
>> >> +++ b/tools/libxl/libxl_create.c Tue Jun 05 17:56:46 2012 +0800
>> >> @@ -23,6 +23,32 @@
>> >> #include <xc_dom.h>
>> >> #include <xenguest.h>
>> >>
>> >> +/*
>> >> + * For qxl vga interface, the total video mem is determined by
>> >> + * RAM bar and VRAM bar. But it is not simply linearly determined,
>> >> + * get_qxl_ram_size below gives the details.
>> >
>> > From this statement I expected get_qxl_ram_size to have a nice comment
>> > explaining what is going on, but it doesn't have this.
>> >
>> > Can you please explain somewhere how this value is determined (I can see
>> > it is not simple ;-)). Perhaps a link to some QXL/qemu document
>> > discussing these parameters would be helpful too?
>>
>> Sorry, there is not a piece of docs on ram bar and vram bar, and how
>> the total size
>> of qxl video memory is calculated from ram bar and vram bar parameters.
>>
>> But from my digging into qxl's source code and debugging, it works this way.
>>
>> I asked similar question in spice-list and get response from:
>> http://comments.gmane.org/gmane.comp.emulators.spice.devel/9501
>>
>> Any way, I will rich the document if get more info.
>
> OK, thanks.
>
>> >> +
>> >> + return (vram + ram + 1023) / 1024;
>> >> +}
>> >> +
>> >> void libxl_domain_config_init(libxl_domain_config *d_config)
>> >> {
>> >> memset(d_config, 0, sizeof(*d_config));
>> >> @@ -195,6 +221,25 @@ int libxl__domain_build_info_setdefault(
>> >>
>> >> if (b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_DEFAULT)
>> >> b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
>> >> + if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
>> >> + && b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_QXL) {
>> >> + if (b_info->u.hvm.vga.vramkb == LIBXL_MEMKB_DEFAULT)
>> >> + b_info->u.hvm.vga.vramkb = 64 * 1024;
>> >> + if (b_info->u.hvm.vga.ramkb == LIBXL_MEMKB_DEFAULT)
>> >> + b_info->u.hvm.vga.ramkb = 64 * 1024;
>> >> + uint32_t qxl_ram = get_qxl_ram_size(b_info->u.hvm.vga.vramkb,
>> >> + b_info->u.hvm.vga.ramkb);
>> >> + /*
>> >> + * video_memkb is the real size of video memory to assign.
>> >> + * If video_memkb can't meet the need of qxl, adjust it
>> >> + * accordingly.
>> >> + */
>> >> + if ((b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
>> >> + || (b_info->video_memkb < qxl_ram)) {
>> >> + b_info->video_memkb = qxl_ram;
>> >
>> > If video_memkb is != LIBXL_MEMKB_DEFAULT and it is not sufficiently
>> > large then I think the correct thing to do is to error out and return
>> > ERROR_INVAL.
>>
>> Not agreed.
>> This will let user must to set a proper value to meet qxl, but from
>> discussing above, it's difficult for user to give this decision.
>> qxlram and qxlvram parameters are enough for user to set qxl's video
>> ram (libvirt also use these two parameters).
>
> If the user has asked for a specific amount of video RAM which is not
> sufficient then the correct answer is to log an error (including the
> required minimum) and exit.
>
> You are correct that it is hard to figure out what "enough" RAM is. I
> expect that for that reason almost all users won't pass any of these
> arguments and will just accept the default, which will work just fine.
I think it's reasonable.
Just use the default will make things much more simpler.
> Ian.
>
--
Zhou Peng
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-06-07 6:08 ` Ian Campbell
2012-06-07 7:49 ` ZhouPeng
@ 2012-07-03 11:30 ` ZhouPeng
2012-07-03 14:24 ` Ian Campbell
1 sibling, 1 reply; 18+ messages in thread
From: ZhouPeng @ 2012-07-03 11:30 UTC (permalink / raw)
To: Ian Campbell; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 9867 bytes --]
qxl support and docs accordingly
v3
--
tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
Usage:
qxl=1|0
Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
diff -r f54cdc27e904 docs/man/xl.cfg.pod.5
--- a/docs/man/xl.cfg.pod.5 Mon Jul 02 09:08:35 2012 +0800
+++ b/docs/man/xl.cfg.pod.5 Tue Jul 03 19:11:47 2012 +0800
@@ -709,11 +709,12 @@ in the B<VFB_SPEC_STRING> for configurin
Sets the amount of RAM which the emulated video card will contain,
which in turn limits the resolutions and bit depths which will be
-available. This option is only available when using the B<stdvga>
-option (see below). The default is 8MB which is sufficient for
-e.g. 1600x1200 at 32bpp. When not using the B<stdvga> option the
-amount of video ram is fixed at 4MB which is sufficient for 1024x768
-at 32 bpp.
+available. This option is available when using the B<stdvga> and
+B<qxl> options (see below).
+For B<stdvga> option, the default is 8MB which is sufficient for
+e.g. 1600x1200 at 32bpp. For B<qxl> option, the default is 128MB.
+When not using the B<stdvga> and B<qxl> options, the amount of video
+ram is fixed at 4MB which is sufficient for 1024x768 at 32 bpp.
=item B<stdvga=BOOLEAN>
@@ -721,6 +722,14 @@ emulated graphics device. The default is
emulated graphics device. The default is false which means to emulate
a Cirrus Logic GD5446 VGA card. If your guest supports VBE 2.0 or
later (e.g. Windows XP onwards) then you should enable this.
+
+=item B<qxl=BOOLEAN>
+
+Select a QXL VGA card as the emulated graphics device.
+In general, QXL should work with the Spice remote display protocol
+for acceleration, and QXL driver is necessary in guest in this case.
+QXL can also work with the VNC protocol, but it will be like a standard
+VGA without acceleration in this case.
=item B<vnc=BOOLEAN>
diff -r f54cdc27e904 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Mon Jul 02 09:08:35 2012 +0800
+++ b/tools/libxl/libxl_create.c Tue Jul 03 19:11:47 2012 +0800
@@ -223,8 +223,29 @@ int libxl__domain_build_info_setdefault(
case LIBXL_DOMAIN_TYPE_HVM:
if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
b_info->shadow_memkb = 0;
+
+ if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
+ && b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_QXL) {
+ /*
+ * QXL needs 128 Mib video ram by default.
+ */
+ if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) {
+ b_info->video_memkb = 128 * 1024;
+ }
+ if (b_info->video_memkb < 128 * 1024) {
+ LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+ "128 Mib videoram is necessary for qxl default");
+ return ERROR_INVAL;
+ }
+ if (b_info->video_memkb > 128 * 1024) {
+ b_info->video_memkb = 128 * 1024;
+ LIBXL__LOG(CTX, LIBXL__LOG_WARNING,
+ "trim videoram to qxl default: 128 Mib");
+ }
+ }
if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
b_info->video_memkb = 8 * 1024;
+
if (b_info->u.hvm.timer_mode == LIBXL_TIMER_MODE_DEFAULT)
b_info->u.hvm.timer_mode =
LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
diff -r f54cdc27e904 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Mon Jul 02 09:08:35 2012 +0800
+++ b/tools/libxl/libxl_dm.c Tue Jul 03 19:11:47 2012 +0800
@@ -182,6 +182,8 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ break;
}
if (b_info->u.hvm.boot) {
@@ -431,6 +433,9 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ flexarray_vappend(dm_args, "-vga", "qxl", NULL);
break;
}
diff -r f54cdc27e904 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Mon Jul 02 09:08:35 2012 +0800
+++ b/tools/libxl/libxl_types.idl Tue Jul 03 19:11:47 2012 +0800
@@ -129,6 +129,7 @@ libxl_vga_interface_type = Enumeration("
libxl_vga_interface_type = Enumeration("vga_interface_type", [
(1, "CIRRUS"),
(2, "STD"),
+ (3, "QXL"),
], init_val = 0)
#
diff -r f54cdc27e904 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Jul 02 09:08:35 2012 +0800
+++ b/tools/libxl/xl_cmdimpl.c Tue Jul 03 19:11:47 2012 +0800
@@ -1260,6 +1260,14 @@ skip_vfb:
b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD :
LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ if (!xlu_cfg_get_long(config, "qxl", &l, 0)) {
+ if (l) {
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_QXL;
+ } else if (!b_info->u.hvm.vga.kind) {
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ }
+ }
+
xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
xlu_cfg_replace_string (config, "vnclisten",
&b_info->u.hvm.vnc.listen, 0);
On Thu, Jun 7, 2012 at 2:08 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2012-06-07 at 04:19 +0100, ZhouPeng wrote:
>> On Wed, Jun 6, 2012 at 9:05 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Tue, 2012-06-05 at 12:22 +0100, ZhouPeng wrote:
>> >> changeset: 25454:1804a873a64d
>> >> tag: tip
>> >> user: Zhou Peng <ailvpeng25@gmail.com>
>> >> date: Tue Jun 05 17:56:46 2012 +0800
>> >> files: tools/libxl/libxl_create.c tools/libxl/libxl_dm.c
>> >> tools/libxl/libxl_types.idl tools/libxl/xl_cmdimpl.c
>> >> description:
>> >> tools:libxl: Add qxl vga interface support.
>> >>
>> >> Usage:
>> >> qxl=1
>> >> qxlvram=64
>> >> qxlram=64
>> >>
>> >> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
>> >
>> > Thanks.
>> >
>> > As previously mentioned this is 4.3 material. Please can you resubmit
>> > once the 4.3 dev cycle opens.
>> ok, thanks.
>> >
>> >> diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_create.c
>> >> --- a/tools/libxl/libxl_create.c Tue Jun 05 17:39:37 2012 +0800
>> >> +++ b/tools/libxl/libxl_create.c Tue Jun 05 17:56:46 2012 +0800
>> >> @@ -23,6 +23,32 @@
>> >> #include <xc_dom.h>
>> >> #include <xenguest.h>
>> >>
>> >> +/*
>> >> + * For qxl vga interface, the total video mem is determined by
>> >> + * RAM bar and VRAM bar. But it is not simply linearly determined,
>> >> + * get_qxl_ram_size below gives the details.
>> >
>> > From this statement I expected get_qxl_ram_size to have a nice comment
>> > explaining what is going on, but it doesn't have this.
>> >
>> > Can you please explain somewhere how this value is determined (I can see
>> > it is not simple ;-)). Perhaps a link to some QXL/qemu document
>> > discussing these parameters would be helpful too?
>>
>> Sorry, there is not a piece of docs on ram bar and vram bar, and how
>> the total size
>> of qxl video memory is calculated from ram bar and vram bar parameters.
>>
>> But from my digging into qxl's source code and debugging, it works this way.
>>
>> I asked similar question in spice-list and get response from:
>> http://comments.gmane.org/gmane.comp.emulators.spice.devel/9501
>>
>> Any way, I will rich the document if get more info.
>
> OK, thanks.
>
>> >> +
>> >> + return (vram + ram + 1023) / 1024;
>> >> +}
>> >> +
>> >> void libxl_domain_config_init(libxl_domain_config *d_config)
>> >> {
>> >> memset(d_config, 0, sizeof(*d_config));
>> >> @@ -195,6 +221,25 @@ int libxl__domain_build_info_setdefault(
>> >>
>> >> if (b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_DEFAULT)
>> >> b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
>> >> + if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
>> >> + && b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_QXL) {
>> >> + if (b_info->u.hvm.vga.vramkb == LIBXL_MEMKB_DEFAULT)
>> >> + b_info->u.hvm.vga.vramkb = 64 * 1024;
>> >> + if (b_info->u.hvm.vga.ramkb == LIBXL_MEMKB_DEFAULT)
>> >> + b_info->u.hvm.vga.ramkb = 64 * 1024;
>> >> + uint32_t qxl_ram = get_qxl_ram_size(b_info->u.hvm.vga.vramkb,
>> >> + b_info->u.hvm.vga.ramkb);
>> >> + /*
>> >> + * video_memkb is the real size of video memory to assign.
>> >> + * If video_memkb can't meet the need of qxl, adjust it
>> >> + * accordingly.
>> >> + */
>> >> + if ((b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
>> >> + || (b_info->video_memkb < qxl_ram)) {
>> >> + b_info->video_memkb = qxl_ram;
>> >
>> > If video_memkb is != LIBXL_MEMKB_DEFAULT and it is not sufficiently
>> > large then I think the correct thing to do is to error out and return
>> > ERROR_INVAL.
>>
>> Not agreed.
>> This will let user must to set a proper value to meet qxl, but from
>> discussing above, it's difficult for user to give this decision.
>> qxlram and qxlvram parameters are enough for user to set qxl's video
>> ram (libvirt also use these two parameters).
>
> If the user has asked for a specific amount of video RAM which is not
> sufficient then the correct answer is to log an error (including the
> required minimum) and exit.
>
> You are correct that it is hard to figure out what "enough" RAM is. I
> expect that for that reason almost all users won't pass any of these
> arguments and will just accept the default, which will work just fine.
>
> Ian.
>
--
Zhou Peng
[-- Attachment #2: spice.tools.libxl.qxl.support+docs.v3.diff --]
[-- Type: application/octet-stream, Size: 5296 bytes --]
tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
Usage:
qxl=1|0
Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
diff -r f54cdc27e904 docs/man/xl.cfg.pod.5
--- a/docs/man/xl.cfg.pod.5 Mon Jul 02 09:08:35 2012 +0800
+++ b/docs/man/xl.cfg.pod.5 Tue Jul 03 19:11:47 2012 +0800
@@ -709,11 +709,12 @@ in the B<VFB_SPEC_STRING> for configurin
Sets the amount of RAM which the emulated video card will contain,
which in turn limits the resolutions and bit depths which will be
-available. This option is only available when using the B<stdvga>
-option (see below). The default is 8MB which is sufficient for
-e.g. 1600x1200 at 32bpp. When not using the B<stdvga> option the
-amount of video ram is fixed at 4MB which is sufficient for 1024x768
-at 32 bpp.
+available. This option is available when using the B<stdvga> and
+B<qxl> options (see below).
+For B<stdvga> option, the default is 8MB which is sufficient for
+e.g. 1600x1200 at 32bpp. For B<qxl> option, the default is 128MB.
+When not using the B<stdvga> and B<qxl> options, the amount of video
+ram is fixed at 4MB which is sufficient for 1024x768 at 32 bpp.
=item B<stdvga=BOOLEAN>
@@ -721,6 +722,14 @@ emulated graphics device. The default is
emulated graphics device. The default is false which means to emulate
a Cirrus Logic GD5446 VGA card. If your guest supports VBE 2.0 or
later (e.g. Windows XP onwards) then you should enable this.
+
+=item B<qxl=BOOLEAN>
+
+Select a QXL VGA card as the emulated graphics device.
+In general, QXL should work with the Spice remote display protocol
+for acceleration, and QXL driver is necessary in guest in this case.
+QXL can also work with the VNC protocol, but it will be like a standard
+VGA without acceleration in this case.
=item B<vnc=BOOLEAN>
diff -r f54cdc27e904 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Mon Jul 02 09:08:35 2012 +0800
+++ b/tools/libxl/libxl_create.c Tue Jul 03 19:11:47 2012 +0800
@@ -223,8 +223,29 @@ int libxl__domain_build_info_setdefault(
case LIBXL_DOMAIN_TYPE_HVM:
if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
b_info->shadow_memkb = 0;
+
+ if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
+ && b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_QXL) {
+ /*
+ * QXL needs 128 Mib video ram by default.
+ */
+ if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) {
+ b_info->video_memkb = 128 * 1024;
+ }
+ if (b_info->video_memkb < 128 * 1024) {
+ LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+ "128 Mib videoram is necessary for qxl default");
+ return ERROR_INVAL;
+ }
+ if (b_info->video_memkb > 128 * 1024) {
+ b_info->video_memkb = 128 * 1024;
+ LIBXL__LOG(CTX, LIBXL__LOG_WARNING,
+ "trim videoram to qxl default: 128 Mib");
+ }
+ }
if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
b_info->video_memkb = 8 * 1024;
+
if (b_info->u.hvm.timer_mode == LIBXL_TIMER_MODE_DEFAULT)
b_info->u.hvm.timer_mode =
LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
diff -r f54cdc27e904 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Mon Jul 02 09:08:35 2012 +0800
+++ b/tools/libxl/libxl_dm.c Tue Jul 03 19:11:47 2012 +0800
@@ -182,6 +182,8 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ break;
}
if (b_info->u.hvm.boot) {
@@ -431,6 +433,9 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ flexarray_vappend(dm_args, "-vga", "qxl", NULL);
break;
}
diff -r f54cdc27e904 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Mon Jul 02 09:08:35 2012 +0800
+++ b/tools/libxl/libxl_types.idl Tue Jul 03 19:11:47 2012 +0800
@@ -129,6 +129,7 @@ libxl_vga_interface_type = Enumeration("
libxl_vga_interface_type = Enumeration("vga_interface_type", [
(1, "CIRRUS"),
(2, "STD"),
+ (3, "QXL"),
], init_val = 0)
#
diff -r f54cdc27e904 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Jul 02 09:08:35 2012 +0800
+++ b/tools/libxl/xl_cmdimpl.c Tue Jul 03 19:11:47 2012 +0800
@@ -1260,6 +1260,14 @@ skip_vfb:
b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD :
LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ if (!xlu_cfg_get_long(config, "qxl", &l, 0)) {
+ if (l) {
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_QXL;
+ } else if (!b_info->u.hvm.vga.kind) {
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ }
+ }
+
xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
xlu_cfg_replace_string (config, "vnclisten",
&b_info->u.hvm.vnc.listen, 0);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-07-03 11:30 ` ZhouPeng
@ 2012-07-03 14:24 ` Ian Campbell
2012-10-10 16:50 ` George Dunlap
0 siblings, 1 reply; 18+ messages in thread
From: Ian Campbell @ 2012-07-03 14:24 UTC (permalink / raw)
To: ZhouPeng; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini
On Tue, 2012-07-03 at 12:30 +0100, ZhouPeng wrote:
> qxl support and docs accordingly
> v3
> --
>
> tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
>
> Usage:
> qxl=1|0
>
> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
Thanks. As previously mentioned we think this is 4.3 material. Please
could you resubmit (or otherwise remind us) once the 4.3 development
branch has opened.
Thanks,
Ian.
>
> diff -r f54cdc27e904 docs/man/xl.cfg.pod.5
> --- a/docs/man/xl.cfg.pod.5 Mon Jul 02 09:08:35 2012 +0800
> +++ b/docs/man/xl.cfg.pod.5 Tue Jul 03 19:11:47 2012 +0800
> @@ -709,11 +709,12 @@ in the B<VFB_SPEC_STRING> for configurin
>
> Sets the amount of RAM which the emulated video card will contain,
> which in turn limits the resolutions and bit depths which will be
> -available. This option is only available when using the B<stdvga>
> -option (see below). The default is 8MB which is sufficient for
> -e.g. 1600x1200 at 32bpp. When not using the B<stdvga> option the
> -amount of video ram is fixed at 4MB which is sufficient for 1024x768
> -at 32 bpp.
> +available. This option is available when using the B<stdvga> and
> +B<qxl> options (see below).
> +For B<stdvga> option, the default is 8MB which is sufficient for
> +e.g. 1600x1200 at 32bpp. For B<qxl> option, the default is 128MB.
> +When not using the B<stdvga> and B<qxl> options, the amount of video
> +ram is fixed at 4MB which is sufficient for 1024x768 at 32 bpp.
>
> =item B<stdvga=BOOLEAN>
>
> @@ -721,6 +722,14 @@ emulated graphics device. The default is
> emulated graphics device. The default is false which means to emulate
> a Cirrus Logic GD5446 VGA card. If your guest supports VBE 2.0 or
> later (e.g. Windows XP onwards) then you should enable this.
> +
> +=item B<qxl=BOOLEAN>
> +
> +Select a QXL VGA card as the emulated graphics device.
> +In general, QXL should work with the Spice remote display protocol
> +for acceleration, and QXL driver is necessary in guest in this case.
> +QXL can also work with the VNC protocol, but it will be like a standard
> +VGA without acceleration in this case.
>
> =item B<vnc=BOOLEAN>
>
> diff -r f54cdc27e904 tools/libxl/libxl_create.c
> --- a/tools/libxl/libxl_create.c Mon Jul 02 09:08:35 2012 +0800
> +++ b/tools/libxl/libxl_create.c Tue Jul 03 19:11:47 2012 +0800
> @@ -223,8 +223,29 @@ int libxl__domain_build_info_setdefault(
> case LIBXL_DOMAIN_TYPE_HVM:
> if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
> b_info->shadow_memkb = 0;
> +
> + if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
> + && b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_QXL) {
> + /*
> + * QXL needs 128 Mib video ram by default.
> + */
> + if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) {
> + b_info->video_memkb = 128 * 1024;
> + }
> + if (b_info->video_memkb < 128 * 1024) {
> + LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
> + "128 Mib videoram is necessary for qxl default");
> + return ERROR_INVAL;
> + }
> + if (b_info->video_memkb > 128 * 1024) {
> + b_info->video_memkb = 128 * 1024;
> + LIBXL__LOG(CTX, LIBXL__LOG_WARNING,
> + "trim videoram to qxl default: 128 Mib");
> + }
> + }
> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> b_info->video_memkb = 8 * 1024;
> +
> if (b_info->u.hvm.timer_mode == LIBXL_TIMER_MODE_DEFAULT)
> b_info->u.hvm.timer_mode =
> LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
> diff -r f54cdc27e904 tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.c Mon Jul 02 09:08:35 2012 +0800
> +++ b/tools/libxl/libxl_dm.c Tue Jul 03 19:11:47 2012 +0800
> @@ -182,6 +182,8 @@ static char ** libxl__build_device_model
> break;
> case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> break;
> + case LIBXL_VGA_INTERFACE_TYPE_QXL:
> + break;
> }
>
> if (b_info->u.hvm.boot) {
> @@ -431,6 +433,9 @@ static char ** libxl__build_device_model
> break;
> case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
> + break;
> + case LIBXL_VGA_INTERFACE_TYPE_QXL:
> + flexarray_vappend(dm_args, "-vga", "qxl", NULL);
> break;
> }
>
> diff -r f54cdc27e904 tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl Mon Jul 02 09:08:35 2012 +0800
> +++ b/tools/libxl/libxl_types.idl Tue Jul 03 19:11:47 2012 +0800
> @@ -129,6 +129,7 @@ libxl_vga_interface_type = Enumeration("
> libxl_vga_interface_type = Enumeration("vga_interface_type", [
> (1, "CIRRUS"),
> (2, "STD"),
> + (3, "QXL"),
> ], init_val = 0)
>
> #
> diff -r f54cdc27e904 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c Mon Jul 02 09:08:35 2012 +0800
> +++ b/tools/libxl/xl_cmdimpl.c Tue Jul 03 19:11:47 2012 +0800
> @@ -1260,6 +1260,14 @@ skip_vfb:
> b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD :
> LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
>
> + if (!xlu_cfg_get_long(config, "qxl", &l, 0)) {
> + if (l) {
> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_QXL;
> + } else if (!b_info->u.hvm.vga.kind) {
> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> + }
> + }
> +
> xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
> xlu_cfg_replace_string (config, "vnclisten",
> &b_info->u.hvm.vnc.listen, 0);
>
> On Thu, Jun 7, 2012 at 2:08 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2012-06-07 at 04:19 +0100, ZhouPeng wrote:
> >> On Wed, Jun 6, 2012 at 9:05 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> > On Tue, 2012-06-05 at 12:22 +0100, ZhouPeng wrote:
> >> >> changeset: 25454:1804a873a64d
> >> >> tag: tip
> >> >> user: Zhou Peng <ailvpeng25@gmail.com>
> >> >> date: Tue Jun 05 17:56:46 2012 +0800
> >> >> files: tools/libxl/libxl_create.c tools/libxl/libxl_dm.c
> >> >> tools/libxl/libxl_types.idl tools/libxl/xl_cmdimpl.c
> >> >> description:
> >> >> tools:libxl: Add qxl vga interface support.
> >> >>
> >> >> Usage:
> >> >> qxl=1
> >> >> qxlvram=64
> >> >> qxlram=64
> >> >>
> >> >> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
> >> >
> >> > Thanks.
> >> >
> >> > As previously mentioned this is 4.3 material. Please can you resubmit
> >> > once the 4.3 dev cycle opens.
> >> ok, thanks.
> >> >
> >> >> diff -r 7bd08f83a2ce -r 1804a873a64d tools/libxl/libxl_create.c
> >> >> --- a/tools/libxl/libxl_create.c Tue Jun 05 17:39:37 2012 +0800
> >> >> +++ b/tools/libxl/libxl_create.c Tue Jun 05 17:56:46 2012 +0800
> >> >> @@ -23,6 +23,32 @@
> >> >> #include <xc_dom.h>
> >> >> #include <xenguest.h>
> >> >>
> >> >> +/*
> >> >> + * For qxl vga interface, the total video mem is determined by
> >> >> + * RAM bar and VRAM bar. But it is not simply linearly determined,
> >> >> + * get_qxl_ram_size below gives the details.
> >> >
> >> > From this statement I expected get_qxl_ram_size to have a nice comment
> >> > explaining what is going on, but it doesn't have this.
> >> >
> >> > Can you please explain somewhere how this value is determined (I can see
> >> > it is not simple ;-)). Perhaps a link to some QXL/qemu document
> >> > discussing these parameters would be helpful too?
> >>
> >> Sorry, there is not a piece of docs on ram bar and vram bar, and how
> >> the total size
> >> of qxl video memory is calculated from ram bar and vram bar parameters.
> >>
> >> But from my digging into qxl's source code and debugging, it works this way.
> >>
> >> I asked similar question in spice-list and get response from:
> >> http://comments.gmane.org/gmane.comp.emulators.spice.devel/9501
> >>
> >> Any way, I will rich the document if get more info.
> >
> > OK, thanks.
> >
> >> >> +
> >> >> + return (vram + ram + 1023) / 1024;
> >> >> +}
> >> >> +
> >> >> void libxl_domain_config_init(libxl_domain_config *d_config)
> >> >> {
> >> >> memset(d_config, 0, sizeof(*d_config));
> >> >> @@ -195,6 +221,25 @@ int libxl__domain_build_info_setdefault(
> >> >>
> >> >> if (b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_DEFAULT)
> >> >> b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> >> >> + if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
> >> >> + && b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_QXL) {
> >> >> + if (b_info->u.hvm.vga.vramkb == LIBXL_MEMKB_DEFAULT)
> >> >> + b_info->u.hvm.vga.vramkb = 64 * 1024;
> >> >> + if (b_info->u.hvm.vga.ramkb == LIBXL_MEMKB_DEFAULT)
> >> >> + b_info->u.hvm.vga.ramkb = 64 * 1024;
> >> >> + uint32_t qxl_ram = get_qxl_ram_size(b_info->u.hvm.vga.vramkb,
> >> >> + b_info->u.hvm.vga.ramkb);
> >> >> + /*
> >> >> + * video_memkb is the real size of video memory to assign.
> >> >> + * If video_memkb can't meet the need of qxl, adjust it
> >> >> + * accordingly.
> >> >> + */
> >> >> + if ((b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> >> >> + || (b_info->video_memkb < qxl_ram)) {
> >> >> + b_info->video_memkb = qxl_ram;
> >> >
> >> > If video_memkb is != LIBXL_MEMKB_DEFAULT and it is not sufficiently
> >> > large then I think the correct thing to do is to error out and return
> >> > ERROR_INVAL.
> >>
> >> Not agreed.
> >> This will let user must to set a proper value to meet qxl, but from
> >> discussing above, it's difficult for user to give this decision.
> >> qxlram and qxlvram parameters are enough for user to set qxl's video
> >> ram (libvirt also use these two parameters).
> >
> > If the user has asked for a specific amount of video RAM which is not
> > sufficient then the correct answer is to log an error (including the
> > required minimum) and exit.
> >
> > You are correct that it is hard to figure out what "enough" RAM is. I
> > expect that for that reason almost all users won't pass any of these
> > arguments and will just accept the default, which will work just fine.
> >
> > Ian.
> >
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-07-03 14:24 ` Ian Campbell
@ 2012-10-10 16:50 ` George Dunlap
2012-10-14 5:16 ` ZhouPeng
0 siblings, 1 reply; 18+ messages in thread
From: George Dunlap @ 2012-10-10 16:50 UTC (permalink / raw)
To: Ian Campbell
Cc: ZhouPeng, Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini
On Tue, Jul 3, 2012 at 3:24 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Tue, 2012-07-03 at 12:30 +0100, ZhouPeng wrote:
>> qxl support and docs accordingly
>> v3
>> --
>>
>> tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
>>
>> Usage:
>> qxl=1|0
>>
>> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
>
> Thanks. As previously mentioned we think this is 4.3 material. Please
> could you resubmit (or otherwise remind us) once the 4.3 development
> branch has opened.
Now that 4.3 development has started, what's the status of this patch?
Does it need a refresh? Zhou Peng, if you're planning to get this
working for 4.3, can I add this to my 4.3 feature tracking list?
Thanks,
-George
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-10-10 16:50 ` George Dunlap
@ 2012-10-14 5:16 ` ZhouPeng
2012-10-18 10:19 ` ZhouPeng
0 siblings, 1 reply; 18+ messages in thread
From: ZhouPeng @ 2012-10-14 5:16 UTC (permalink / raw)
To: George Dunlap
Cc: Xen-Devel (E-mail), Ian Jackson, Ian Campbell, Stefano Stabellini
On Thu, Oct 11, 2012 at 12:50 AM, George Dunlap
<George.Dunlap@eu.citrix.com> wrote:
> On Tue, Jul 3, 2012 at 3:24 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> On Tue, 2012-07-03 at 12:30 +0100, ZhouPeng wrote:
>>> qxl support and docs accordingly
>>> v3
>>> --
>>>
>>> tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
>>>
>>> Usage:
>>> qxl=1|0
>>>
>>> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
>>
>> Thanks. As previously mentioned we think this is 4.3 material. Please
>> could you resubmit (or otherwise remind us) once the 4.3 development
>> branch has opened.
>
> Now that 4.3 development has started, what's the status of this patch?
> Does it need a refresh?
I will have a test with the latest Xen to response to you.
> Zhou Peng, if you're planning to get this
> working for 4.3, can I add this to my 4.3 feature tracking list?
I think it's ok to add to 4.3 feature tracking list.
Thanks,
- Zhou Peng
>
> Thanks,
> -George
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-10-14 5:16 ` ZhouPeng
@ 2012-10-18 10:19 ` ZhouPeng
2012-10-22 14:36 ` George Dunlap
0 siblings, 1 reply; 18+ messages in thread
From: ZhouPeng @ 2012-10-18 10:19 UTC (permalink / raw)
To: Ian Campbell, George Dunlap
Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 6328 bytes --]
v4
test and fix conflict with the latest Xen
-----
tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
Usage:
qxl=1|0
Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
diff -r c1c549c4fe9e docs/man/xl.cfg.pod.5
--- a/docs/man/xl.cfg.pod.5 Mon Oct 15 16:51:44 2012 +0100
+++ b/docs/man/xl.cfg.pod.5 Thu Oct 18 17:53:25 2012 +0800
@@ -930,10 +930,13 @@ in the B<VFB_SPEC_STRING> for configurin
Sets the amount of RAM which the emulated video card will contain,
which in turn limits the resolutions and bit depths which will be
-available. This option is only available when using the B<stdvga>
-option (see below).
+available. This option is only available when using the B<stdvga> and
+B<qxl> option (see below).
The default amount of video ram for stdvga is 8MB which is sufficient
for e.g. 1600x1200 at 32bpp.
+For B<qxl> option, the default is 128MB. If B<videoram> is set greater
+than 128MB, it will be trimmed to 128MB; if set less than 128MB, an
+error will be triggered.
When using the emulated Cirrus graphics card (B<stdvga=0>)
the amount of video ram is fixed at 4MB which is sufficient
@@ -951,6 +954,14 @@ a Cirrus Logic GD5446 VGA card. If your
a Cirrus Logic GD5446 VGA card. If your guest supports VBE 2.0 or
later (e.g. Windows XP onwards) then you should enable this.
stdvga supports more video ram and bigger resolutions than Cirrus.
+
+=item B<qxl=BOOLEAN>
+
+Select a QXL VGA card as the emulated graphics device.
+In general, QXL should work with the Spice remote display protocol
+for acceleration, and QXL driver is necessary in guest in this case.
+QXL can also work with the VNC protocol, but it will be like a standard
+VGA without acceleration.
=item B<vnc=BOOLEAN>
diff -r c1c549c4fe9e tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_create.c Thu Oct 18 17:53:25 2012 +0800
@@ -232,8 +232,29 @@ int libxl__domain_build_info_setdefault(
case LIBXL_DOMAIN_TYPE_HVM:
if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
b_info->shadow_memkb = 0;
+
+ if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
+ && b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_QXL) {
+ /*
+ * QXL needs 128 Mib video ram by default.
+ */
+ if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) {
+ b_info->video_memkb = 128 * 1024;
+ }
+ if (b_info->video_memkb < 128 * 1024) {
+ LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+ "128 Mib videoram is necessary for qxl default");
+ return ERROR_INVAL;
+ }
+ if (b_info->video_memkb > 128 * 1024) {
+ b_info->video_memkb = 128 * 1024;
+ LIBXL__LOG(CTX, LIBXL__LOG_WARNING,
+ "trim videoram to qxl default: 128 Mib");
+ }
+ }
if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
b_info->video_memkb = 8 * 1024;
+
if (b_info->u.hvm.timer_mode == LIBXL_TIMER_MODE_DEFAULT)
b_info->u.hvm.timer_mode =
LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
diff -r c1c549c4fe9e tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_dm.c Thu Oct 18 17:53:25 2012 +0800
@@ -181,6 +181,8 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ break;
}
if (b_info->u.hvm.boot) {
@@ -430,6 +432,9 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ flexarray_vappend(dm_args, "-vga", "qxl", NULL);
break;
}
diff -r c1c549c4fe9e tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_types.idl Thu Oct 18 17:53:25 2012 +0800
@@ -130,6 +130,7 @@ libxl_vga_interface_type = Enumeration("
libxl_vga_interface_type = Enumeration("vga_interface_type", [
(1, "CIRRUS"),
(2, "STD"),
+ (3, "QXL"),
], init_val = 0)
#
diff -r c1c549c4fe9e tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Thu Oct 18 17:53:25 2012 +0800
@@ -1402,6 +1402,14 @@ skip_vfb:
b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD :
LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ if (!xlu_cfg_get_long(config, "qxl", &l, 0)) {
+ if (l) {
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_QXL;
+ } else if (!b_info->u.hvm.vga.kind) {
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ }
+ }
+
xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
xlu_cfg_replace_string (config, "vnclisten",
&b_info->u.hvm.vnc.listen, 0);
On Sun, Oct 14, 2012 at 1:16 PM, ZhouPeng <zpengxen@gmail.com> wrote:
> On Thu, Oct 11, 2012 at 12:50 AM, George Dunlap
> <George.Dunlap@eu.citrix.com> wrote:
>> On Tue, Jul 3, 2012 at 3:24 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>>> On Tue, 2012-07-03 at 12:30 +0100, ZhouPeng wrote:
>>>> qxl support and docs accordingly
>>>> v3
>>>> --
>>>>
>>>> tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
>>>>
>>>> Usage:
>>>> qxl=1|0
>>>>
>>>> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
>>>
>>> Thanks. As previously mentioned we think this is 4.3 material. Please
>>> could you resubmit (or otherwise remind us) once the 4.3 development
>>> branch has opened.
>>
>> Now that 4.3 development has started, what's the status of this patch?
>> Does it need a refresh?
>
> I will have a test with the latest Xen to response to you.
>> Zhou Peng, if you're planning to get this
>> working for 4.3, can I add this to my 4.3 feature tracking list?
>
> I think it's ok to add to 4.3 feature tracking list.
>
> Thanks,
> - Zhou Peng
>>
>> Thanks,
>> -George
--
Zhou Peng
[-- Attachment #2: spice.tools.libxl.qxl.support+docs.v4.diff --]
[-- Type: application/octet-stream, Size: 5186 bytes --]
tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
Usage:
qxl=1|0
Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
diff -r c1c549c4fe9e docs/man/xl.cfg.pod.5
--- a/docs/man/xl.cfg.pod.5 Mon Oct 15 16:51:44 2012 +0100
+++ b/docs/man/xl.cfg.pod.5 Thu Oct 18 17:53:25 2012 +0800
@@ -930,10 +930,13 @@ in the B<VFB_SPEC_STRING> for configurin
Sets the amount of RAM which the emulated video card will contain,
which in turn limits the resolutions and bit depths which will be
-available. This option is only available when using the B<stdvga>
-option (see below).
+available. This option is only available when using the B<stdvga> and
+B<qxl> option (see below).
The default amount of video ram for stdvga is 8MB which is sufficient
for e.g. 1600x1200 at 32bpp.
+For B<qxl> option, the default is 128MB. If B<videoram> is set greater
+than 128MB, it will be trimmed to 128MB; if set less than 128MB, an
+error will be triggered.
When using the emulated Cirrus graphics card (B<stdvga=0>)
the amount of video ram is fixed at 4MB which is sufficient
@@ -951,6 +954,14 @@ a Cirrus Logic GD5446 VGA card. If your
a Cirrus Logic GD5446 VGA card. If your guest supports VBE 2.0 or
later (e.g. Windows XP onwards) then you should enable this.
stdvga supports more video ram and bigger resolutions than Cirrus.
+
+=item B<qxl=BOOLEAN>
+
+Select a QXL VGA card as the emulated graphics device.
+In general, QXL should work with the Spice remote display protocol
+for acceleration, and QXL driver is necessary in guest in this case.
+QXL can also work with the VNC protocol, but it will be like a standard
+VGA without acceleration.
=item B<vnc=BOOLEAN>
diff -r c1c549c4fe9e tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_create.c Thu Oct 18 17:53:25 2012 +0800
@@ -232,8 +232,29 @@ int libxl__domain_build_info_setdefault(
case LIBXL_DOMAIN_TYPE_HVM:
if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
b_info->shadow_memkb = 0;
+
+ if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
+ && b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_QXL) {
+ /*
+ * QXL needs 128 Mib video ram by default.
+ */
+ if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) {
+ b_info->video_memkb = 128 * 1024;
+ }
+ if (b_info->video_memkb < 128 * 1024) {
+ LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+ "128 Mib videoram is necessary for qxl default");
+ return ERROR_INVAL;
+ }
+ if (b_info->video_memkb > 128 * 1024) {
+ b_info->video_memkb = 128 * 1024;
+ LIBXL__LOG(CTX, LIBXL__LOG_WARNING,
+ "trim videoram to qxl default: 128 Mib");
+ }
+ }
if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
b_info->video_memkb = 8 * 1024;
+
if (b_info->u.hvm.timer_mode == LIBXL_TIMER_MODE_DEFAULT)
b_info->u.hvm.timer_mode =
LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
diff -r c1c549c4fe9e tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_dm.c Thu Oct 18 17:53:25 2012 +0800
@@ -181,6 +181,8 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ break;
}
if (b_info->u.hvm.boot) {
@@ -430,6 +432,9 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ flexarray_vappend(dm_args, "-vga", "qxl", NULL);
break;
}
diff -r c1c549c4fe9e tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_types.idl Thu Oct 18 17:53:25 2012 +0800
@@ -130,6 +130,7 @@ libxl_vga_interface_type = Enumeration("
libxl_vga_interface_type = Enumeration("vga_interface_type", [
(1, "CIRRUS"),
(2, "STD"),
+ (3, "QXL"),
], init_val = 0)
#
diff -r c1c549c4fe9e tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Thu Oct 18 17:53:25 2012 +0800
@@ -1402,6 +1402,14 @@ skip_vfb:
b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD :
LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ if (!xlu_cfg_get_long(config, "qxl", &l, 0)) {
+ if (l) {
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_QXL;
+ } else if (!b_info->u.hvm.vga.kind) {
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ }
+ }
+
xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
xlu_cfg_replace_string (config, "vnclisten",
&b_info->u.hvm.vnc.listen, 0);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-10-18 10:19 ` ZhouPeng
@ 2012-10-22 14:36 ` George Dunlap
2012-10-24 8:06 ` ZhouPeng
0 siblings, 1 reply; 18+ messages in thread
From: George Dunlap @ 2012-10-22 14:36 UTC (permalink / raw)
To: ZhouPeng
Cc: Xen-Devel (E-mail), Ian Jackson, Ian Campbell, Stefano Stabellini
On 18/10/12 11:19, ZhouPeng wrote:
> v4
>
> test and fix conflict with the latest Xen
> -----
>
>
> tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
>
> Usage:
> qxl=1|0
>
> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
Thanks, Zhou Peng. Just a couple of comments below:
>
> diff -r c1c549c4fe9e docs/man/xl.cfg.pod.5
> --- a/docs/man/xl.cfg.pod.5 Mon Oct 15 16:51:44 2012 +0100
> +++ b/docs/man/xl.cfg.pod.5 Thu Oct 18 17:53:25 2012 +0800
> @@ -930,10 +930,13 @@ in the B<VFB_SPEC_STRING> for configurin
>
> Sets the amount of RAM which the emulated video card will contain,
> which in turn limits the resolutions and bit depths which will be
> -available. This option is only available when using the B<stdvga>
> -option (see below).
> +available. This option is only available when using the B<stdvga> and
> +B<qxl> option (see below).
> The default amount of video ram for stdvga is 8MB which is sufficient
> for e.g. 1600x1200 at 32bpp.
> +For B<qxl> option, the default is 128MB. If B<videoram> is set greater
> +than 128MB, it will be trimmed to 128MB; if set less than 128MB, an
> +error will be triggered.
Is this a fixed value in qemu, or is this something that can be changed
via the command-line?
> +
> + if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
> + && b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_QXL) {
> + /*
> + * QXL needs 128 Mib video ram by default.
> + */
> + if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) {
> + b_info->video_memkb = 128 * 1024;
> + }
> + if (b_info->video_memkb < 128 * 1024) {
> + LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
> + "128 Mib videoram is necessary for qxl default");
> + return ERROR_INVAL;
> + }
> + if (b_info->video_memkb > 128 * 1024) {
> + b_info->video_memkb = 128 * 1024;
> + LIBXL__LOG(CTX, LIBXL__LOG_WARNING,
> + "trim videoram to qxl default: 128 Mib");
> + }
> + }
It would be better to have a single #define for the required QXL video
mem size, rather than duplicating "128*1024" several times.
Other than that, looks pretty good to me.
Thanks,
-George
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-10-22 14:36 ` George Dunlap
@ 2012-10-24 8:06 ` ZhouPeng
2012-10-24 13:25 ` George Dunlap
2012-11-19 11:37 ` Ian Campbell
0 siblings, 2 replies; 18+ messages in thread
From: ZhouPeng @ 2012-10-24 8:06 UTC (permalink / raw)
To: George Dunlap
Cc: Xen-Devel (E-mail), Ian Jackson, Ian Campbell, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 8244 bytes --]
On Mon, Oct 22, 2012 at 10:36 PM, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
> On 18/10/12 11:19, ZhouPeng wrote:
>>
>> v4
>>
>> test and fix conflict with the latest Xen
>> -----
>>
>>
>> tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
>>
>> Usage:
>> qxl=1|0
>>
>> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
>
>
> Thanks, Zhou Peng. Just a couple of comments below:
>
>
>>
>> diff -r c1c549c4fe9e docs/man/xl.cfg.pod.5
>> --- a/docs/man/xl.cfg.pod.5 Mon Oct 15 16:51:44 2012 +0100
>> +++ b/docs/man/xl.cfg.pod.5 Thu Oct 18 17:53:25 2012 +0800
>> @@ -930,10 +930,13 @@ in the B<VFB_SPEC_STRING> for configurin
>>
>> Sets the amount of RAM which the emulated video card will contain,
>> which in turn limits the resolutions and bit depths which will be
>> -available. This option is only available when using the B<stdvga>
>> -option (see below).
>> +available. This option is only available when using the B<stdvga> and
>> +B<qxl> option (see below).
>> The default amount of video ram for stdvga is 8MB which is sufficient
>> for e.g. 1600x1200 at 32bpp.
>> +For B<qxl> option, the default is 128MB. If B<videoram> is set greater
>> +than 128MB, it will be trimmed to 128MB; if set less than 128MB, an
>> +error will be triggered.
>
>
> Is this a fixed value in qemu, or is this something that can be changed via
> the command-line?
Can be changed.
But Ian Campbell and me have talked on sth related with this
closely before. And we both agree to just support the default is
enough in most time. You can get it from the url below quickly, pls
read from bottom up, which can save time.
http://lists.xen.org/archives/html/xen-devel/2012-07/msg00098.html
Thanks,
-Zhou Peng
>
>
>> +
>> + if (b_info->device_model_version ==
>> LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
>> + && b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_QXL) {
>> + /*
>> + * QXL needs 128 Mib video ram by default.
>> + */
>> + if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) {
>> + b_info->video_memkb = 128 * 1024;
>> + }
>> + if (b_info->video_memkb < 128 * 1024) {
>> + LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
>> + "128 Mib videoram is necessary for qxl default");
>> + return ERROR_INVAL;
>> + }
>> + if (b_info->video_memkb > 128 * 1024) {
>> + b_info->video_memkb = 128 * 1024;
>> + LIBXL__LOG(CTX, LIBXL__LOG_WARNING,
>> + "trim videoram to qxl default: 128 Mib");
>> + }
>> + }
>
>
> It would be better to have a single #define for the required QXL video mem
> size, rather than duplicating "128*1024" several times.
>
> Other than that, looks pretty good to me.
Fixed in v5
> Thanks,
> -George
>
Patch v5 is appended and attached below .
------------------
tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
Usage:
qxl=1|0
Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
diff -r c1c549c4fe9e docs/man/xl.cfg.pod.5
--- a/docs/man/xl.cfg.pod.5 Mon Oct 15 16:51:44 2012 +0100
+++ b/docs/man/xl.cfg.pod.5 Wed Oct 24 15:39:43 2012 +0800
@@ -930,10 +930,13 @@ in the B<VFB_SPEC_STRING> for configurin
Sets the amount of RAM which the emulated video card will contain,
which in turn limits the resolutions and bit depths which will be
-available. This option is only available when using the B<stdvga>
-option (see below).
+available. This option is only available when using the B<stdvga> and
+B<qxl> option (see below).
The default amount of video ram for stdvga is 8MB which is sufficient
for e.g. 1600x1200 at 32bpp.
+For B<qxl> option, the default is 128MB. If B<videoram> is set greater
+than 128MB, it will be trimmed to 128MB; if set less than 128MB, an
+error will be triggered.
When using the emulated Cirrus graphics card (B<stdvga=0>)
the amount of video ram is fixed at 4MB which is sufficient
@@ -951,6 +954,14 @@ a Cirrus Logic GD5446 VGA card. If your
a Cirrus Logic GD5446 VGA card. If your guest supports VBE 2.0 or
later (e.g. Windows XP onwards) then you should enable this.
stdvga supports more video ram and bigger resolutions than Cirrus.
+
+=item B<qxl=BOOLEAN>
+
+Select a QXL VGA card as the emulated graphics device.
+In general, QXL should work with the Spice remote display protocol
+for acceleration, and QXL driver is necessary in guest in this case.
+QXL can also work with the VNC protocol, but it will be like a standard
+VGA without acceleration.
=item B<vnc=BOOLEAN>
diff -r c1c549c4fe9e tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_create.c Wed Oct 24 15:39:43 2012 +0800
@@ -232,8 +232,31 @@ int libxl__domain_build_info_setdefault(
case LIBXL_DOMAIN_TYPE_HVM:
if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
b_info->shadow_memkb = 0;
+
+ if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
+ && b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_QXL) {
+ /*
+ * QXL needs 128 Mib video ram by default.
+ */
+#define QXL_VIDEO_RAM_DEFAULT (128 * 1024)
+ if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) {
+ b_info->video_memkb = QXL_VIDEO_RAM_DEFAULT;
+ }
+ if (b_info->video_memkb < QXL_VIDEO_RAM_DEFAULT) {
+ LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+ "128 Mib videoram is necessary for qxl default");
+ return ERROR_INVAL;
+ }
+ if (b_info->video_memkb > QXL_VIDEO_RAM_DEFAULT) {
+ b_info->video_memkb = QXL_VIDEO_RAM_DEFAULT;
+ LIBXL__LOG(CTX, LIBXL__LOG_WARNING,
+ "trim videoram to qxl default: 128 Mib");
+ }
+#undef QXL_VIDEO_RAM_DEFAULT
+ }
if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
b_info->video_memkb = 8 * 1024;
+
if (b_info->u.hvm.timer_mode == LIBXL_TIMER_MODE_DEFAULT)
b_info->u.hvm.timer_mode =
LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
diff -r c1c549c4fe9e tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_dm.c Wed Oct 24 15:39:43 2012 +0800
@@ -181,6 +181,8 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ break;
}
if (b_info->u.hvm.boot) {
@@ -430,6 +432,9 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ flexarray_vappend(dm_args, "-vga", "qxl", NULL);
break;
}
diff -r c1c549c4fe9e tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_types.idl Wed Oct 24 15:39:43 2012 +0800
@@ -130,6 +130,7 @@ libxl_vga_interface_type = Enumeration("
libxl_vga_interface_type = Enumeration("vga_interface_type", [
(1, "CIRRUS"),
(2, "STD"),
+ (3, "QXL"),
], init_val = 0)
#
diff -r c1c549c4fe9e tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Wed Oct 24 15:39:43 2012 +0800
@@ -1402,6 +1402,14 @@ skip_vfb:
b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD :
LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ if (!xlu_cfg_get_long(config, "qxl", &l, 0)) {
+ if (l) {
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_QXL;
+ } else if (!b_info->u.hvm.vga.kind) {
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ }
+ }
+
xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
xlu_cfg_replace_string (config, "vnclisten",
&b_info->u.hvm.vnc.listen, 0);
--
Zhou Peng
[-- Attachment #2: spice.tools.libxl.qxl.support+docs.v5.diff --]
[-- Type: application/octet-stream, Size: 5304 bytes --]
tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
Usage:
qxl=1|0
Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
diff -r c1c549c4fe9e docs/man/xl.cfg.pod.5
--- a/docs/man/xl.cfg.pod.5 Mon Oct 15 16:51:44 2012 +0100
+++ b/docs/man/xl.cfg.pod.5 Wed Oct 24 15:39:43 2012 +0800
@@ -930,10 +930,13 @@ in the B<VFB_SPEC_STRING> for configurin
Sets the amount of RAM which the emulated video card will contain,
which in turn limits the resolutions and bit depths which will be
-available. This option is only available when using the B<stdvga>
-option (see below).
+available. This option is only available when using the B<stdvga> and
+B<qxl> option (see below).
The default amount of video ram for stdvga is 8MB which is sufficient
for e.g. 1600x1200 at 32bpp.
+For B<qxl> option, the default is 128MB. If B<videoram> is set greater
+than 128MB, it will be trimmed to 128MB; if set less than 128MB, an
+error will be triggered.
When using the emulated Cirrus graphics card (B<stdvga=0>)
the amount of video ram is fixed at 4MB which is sufficient
@@ -951,6 +954,14 @@ a Cirrus Logic GD5446 VGA card. If your
a Cirrus Logic GD5446 VGA card. If your guest supports VBE 2.0 or
later (e.g. Windows XP onwards) then you should enable this.
stdvga supports more video ram and bigger resolutions than Cirrus.
+
+=item B<qxl=BOOLEAN>
+
+Select a QXL VGA card as the emulated graphics device.
+In general, QXL should work with the Spice remote display protocol
+for acceleration, and QXL driver is necessary in guest in this case.
+QXL can also work with the VNC protocol, but it will be like a standard
+VGA without acceleration.
=item B<vnc=BOOLEAN>
diff -r c1c549c4fe9e tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_create.c Wed Oct 24 15:39:43 2012 +0800
@@ -232,8 +232,31 @@ int libxl__domain_build_info_setdefault(
case LIBXL_DOMAIN_TYPE_HVM:
if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
b_info->shadow_memkb = 0;
+
+ if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
+ && b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_QXL) {
+ /*
+ * QXL needs 128 Mib video ram by default.
+ */
+#define QXL_VIDEO_RAM_DEFAULT (128 * 1024)
+ if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) {
+ b_info->video_memkb = QXL_VIDEO_RAM_DEFAULT;
+ }
+ if (b_info->video_memkb < QXL_VIDEO_RAM_DEFAULT) {
+ LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+ "128 Mib videoram is necessary for qxl default");
+ return ERROR_INVAL;
+ }
+ if (b_info->video_memkb > QXL_VIDEO_RAM_DEFAULT) {
+ b_info->video_memkb = QXL_VIDEO_RAM_DEFAULT;
+ LIBXL__LOG(CTX, LIBXL__LOG_WARNING,
+ "trim videoram to qxl default: 128 Mib");
+ }
+#undef QXL_VIDEO_RAM_DEFAULT
+ }
if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
b_info->video_memkb = 8 * 1024;
+
if (b_info->u.hvm.timer_mode == LIBXL_TIMER_MODE_DEFAULT)
b_info->u.hvm.timer_mode =
LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
diff -r c1c549c4fe9e tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_dm.c Wed Oct 24 15:39:43 2012 +0800
@@ -181,6 +181,8 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ break;
}
if (b_info->u.hvm.boot) {
@@ -430,6 +432,9 @@ static char ** libxl__build_device_model
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_QXL:
+ flexarray_vappend(dm_args, "-vga", "qxl", NULL);
break;
}
diff -r c1c549c4fe9e tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_types.idl Wed Oct 24 15:39:43 2012 +0800
@@ -130,6 +130,7 @@ libxl_vga_interface_type = Enumeration("
libxl_vga_interface_type = Enumeration("vga_interface_type", [
(1, "CIRRUS"),
(2, "STD"),
+ (3, "QXL"),
], init_val = 0)
#
diff -r c1c549c4fe9e tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Wed Oct 24 15:39:43 2012 +0800
@@ -1402,6 +1402,14 @@ skip_vfb:
b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD :
LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ if (!xlu_cfg_get_long(config, "qxl", &l, 0)) {
+ if (l) {
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_QXL;
+ } else if (!b_info->u.hvm.vga.kind) {
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+ }
+ }
+
xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
xlu_cfg_replace_string (config, "vnclisten",
&b_info->u.hvm.vnc.listen, 0);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-10-24 8:06 ` ZhouPeng
@ 2012-10-24 13:25 ` George Dunlap
2012-10-30 7:45 ` ZhouPeng
2012-11-19 11:37 ` Ian Campbell
1 sibling, 1 reply; 18+ messages in thread
From: George Dunlap @ 2012-10-24 13:25 UTC (permalink / raw)
To: ZhouPeng
Cc: Xen-Devel (E-mail), Ian Jackson, Ian Campbell, Stefano Stabellini
On 24/10/12 09:06, ZhouPeng wrote:
> Can be changed.But Ian Campbell and me have talked on sth related with this
> closely before. And we both agree to just support the default is
> enough in most time. You can get it from the url below quickly, pls
> read from bottom up, which can save time.
OK -- in that case, v5 looks OK to me:
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
谢谢，
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-10-24 13:25 ` George Dunlap
@ 2012-10-30 7:45 ` ZhouPeng
0 siblings, 0 replies; 18+ messages in thread
From: ZhouPeng @ 2012-10-30 7:45 UTC (permalink / raw)
To: Ian Campbell
Cc: George Dunlap, Xen-Devel (E-mail), Ian Jackson,
Stefano Stabellini
ping Ian C
On Wed, Oct 24, 2012 at 9:25 PM, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
> On 24/10/12 09:06, ZhouPeng wrote:
>>
>> Can be changed.But Ian Campbell and me have talked on sth related with
>> this
>>
>> closely before. And we both agree to just support the default is
>> enough in most time. You can get it from the url below quickly, pls
>> read from bottom up, which can save time.
>
>
> OK -- in that case, v5 looks OK to me:
>
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
>
> 谢谢，
> -George
--
Zhou Peng
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-10-24 8:06 ` ZhouPeng
2012-10-24 13:25 ` George Dunlap
@ 2012-11-19 11:37 ` Ian Campbell
2012-11-21 3:27 ` ZhouPeng
1 sibling, 1 reply; 18+ messages in thread
From: Ian Campbell @ 2012-11-19 11:37 UTC (permalink / raw)
To: ZhouPeng; +Cc: George Dunlap, Xen-Devel (E-mail), Ian Jackson,
Stefano Stabellini
On Wed, 2012-10-24 at 09:06 +0100, ZhouPeng wrote:
> >> +For B<qxl> option, the default is 128MB. If B<videoram> is set greater
> >> +than 128MB, it will be trimmed to 128MB; if set less than 128MB, an
> >> +error will be triggered.
> >
> >
> > Is this a fixed value in qemu, or is this something that can be changed via
> > the command-line?
> Can be changed.
>
> But Ian Campbell and me have talked on sth related with this
> closely before. And we both agree to just support the default is
> enough in most time. You can get it from the url below quickly, pls
> read from bottom up, which can save time.
>
> http://lists.xen.org/archives/html/xen-devel/2012-07/msg00098.html
We discussed what to do if the supplied value was too small but I don't
recall this behaviour of clamping a value which is larger to a specific
value.
If we reject values which are <128M, and clamp anything which is >128M
to 128M then what is the point of the option?
Ian.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-11-19 11:37 ` Ian Campbell
@ 2012-11-21 3:27 ` ZhouPeng
2012-11-21 11:02 ` Ian Campbell
0 siblings, 1 reply; 18+ messages in thread
From: ZhouPeng @ 2012-11-21 3:27 UTC (permalink / raw)
To: Ian Campbell
Cc: George Dunlap, Xen-Devel (E-mail), Ian Jackson,
Stefano Stabellini
On Mon, Nov 19, 2012 at 7:37 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Wed, 2012-10-24 at 09:06 +0100, ZhouPeng wrote:
>
>> >> +For B<qxl> option, the default is 128MB. If B<videoram> is set greater
>> >> +than 128MB, it will be trimmed to 128MB; if set less than 128MB, an
>> >> +error will be triggered.
>> >
>> >
>> > Is this a fixed value in qemu, or is this something that can be changed via
>> > the command-line?
>> Can be changed.
>>
>> But Ian Campbell and me have talked on sth related with this
>> closely before. And we both agree to just support the default is
>> enough in most time. You can get it from the url below quickly, pls
>> read from bottom up, which can save time.
>>
>> http://lists.xen.org/archives/html/xen-devel/2012-07/msg00098.html
>
> We discussed what to do if the supplied value was too small but I don't
> recall this behaviour of clamping a value which is larger to a specific
> value.
I think, the key idea is just to support the default for qxl (128Mib)
in the discussion.
videoram is not designed for qxl video ram.
> If we reject values which are <128M, and clamp anything which is >128M
> to 128M then what is the point of the option?
This piece of code doesn't mean to use videoram option to assign memory for qxl.
Because videoram option comes in Xen before qxl, it need to be consided here.
v2 gives user the chance to assign memory for qxl.
>
> Ian.
--
Zhou Peng
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-11-21 3:27 ` ZhouPeng
@ 2012-11-21 11:02 ` Ian Campbell
2013-01-16 17:00 ` George Dunlap
0 siblings, 1 reply; 18+ messages in thread
From: Ian Campbell @ 2012-11-21 11:02 UTC (permalink / raw)
To: ZhouPeng; +Cc: George Dunlap, Xen-Devel (E-mail), Ian Jackson,
Stefano Stabellini
On Wed, 2012-11-21 at 03:27 +0000, ZhouPeng wrote:
> On Mon, Nov 19, 2012 at 7:37 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Wed, 2012-10-24 at 09:06 +0100, ZhouPeng wrote:
> >
> >> >> +For B<qxl> option, the default is 128MB. If B<videoram> is set greater
> >> >> +than 128MB, it will be trimmed to 128MB; if set less than 128MB, an
> >> >> +error will be triggered.
> >> >
> >> >
> >> > Is this a fixed value in qemu, or is this something that can be changed via
> >> > the command-line?
> >> Can be changed.
> >>
> >> But Ian Campbell and me have talked on sth related with this
> >> closely before. And we both agree to just support the default is
> >> enough in most time. You can get it from the url below quickly, pls
> >> read from bottom up, which can save time.
> >>
> >> http://lists.xen.org/archives/html/xen-devel/2012-07/msg00098.html
> >
> > We discussed what to do if the supplied value was too small but I don't
> > recall this behaviour of clamping a value which is larger to a specific
> > value.
>
> I think, the key idea is just to support the default for qxl (128Mib)
> in the discussion.
> videoram is not designed for qxl video ram.
> > If we reject values which are <128M, and clamp anything which is >128M
> > to 128M then what is the point of the option?
>
> This piece of code doesn't mean to use videoram option to assign memory for qxl.
> Because videoram option comes in Xen before qxl, it need to be consided here.
You mean because video_memkb is used for VGA when QXL is disabled and
QXL when it is? So the confusion here arises because this field is doing
double duty.
Perhaps it would be better to add a separate notion of qxl_memkb? The
code in libxl__build_device_model_args_new and perhaps a handful of
other places would need to account for either videoram or qxlram
depending on which was enabled (you'd probably want a helper function
for this).
Is it possible to enable both VGA and QXL for a domain?
>
> v2 gives user the chance to assign memory for qxl.
> >
> > Ian.
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
2012-11-21 11:02 ` Ian Campbell
@ 2013-01-16 17:00 ` George Dunlap
0 siblings, 0 replies; 18+ messages in thread
From: George Dunlap @ 2013-01-16 17:00 UTC (permalink / raw)
To: Ian Campbell
Cc: ZhouPeng, Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini
[-- Attachment #1.1: Type: text/plain, Size: 1147 bytes --]
On Wed, Nov 21, 2012 at 11:02 AM, Ian Campbell <Ian.Campbell@citrix.com>wrote:
> On Wed, 2012-11-21 at 03:27 +0000, ZhouPeng wrote:
> > I think, the key idea is just to support the default for qxl (128Mib)
> > in the discussion.
> > videoram is not designed for qxl video ram.
> > > If we reject values which are <128M, and clamp anything which is >128M
> > > to 128M then what is the point of the option?
> >
> > This piece of code doesn't mean to use videoram option to assign memory
> for qxl.
> > Because videoram option comes in Xen before qxl, it need to be consided
> here.
>
> You mean because video_memkb is used for VGA when QXL is disabled and
> QXL when it is? So the confusion here arises because this field is doing
> double duty.
>
> Perhaps it would be better to add a separate notion of qxl_memkb? The
> code in libxl__build_device_model_args_new and perhaps a handful of
> other places would need to account for either videoram or qxlram
> depending on which was enabled (you'd probably want a helper function
> for this).
>
> Is it possible to enable both VGA and QXL for a domain?
>
What's the status of this patch?
-George
[-- Attachment #1.2: Type: text/html, Size: 1625 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2013-01-16 17:00 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-05 11:22 [PATCH 2/3] tools:libxl: Add qxl vga interface support v2 ZhouPeng
2012-06-06 13:05 ` Ian Campbell
2012-06-07 3:19 ` ZhouPeng
2012-06-07 6:08 ` Ian Campbell
2012-06-07 7:49 ` ZhouPeng
2012-07-03 11:30 ` ZhouPeng
2012-07-03 14:24 ` Ian Campbell
2012-10-10 16:50 ` George Dunlap
2012-10-14 5:16 ` ZhouPeng
2012-10-18 10:19 ` ZhouPeng
2012-10-22 14:36 ` George Dunlap
2012-10-24 8:06 ` ZhouPeng
2012-10-24 13:25 ` George Dunlap
2012-10-30 7:45 ` ZhouPeng
2012-11-19 11:37 ` Ian Campbell
2012-11-21 3:27 ` ZhouPeng
2012-11-21 11:02 ` Ian Campbell
2013-01-16 17:00 ` George Dunlap
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).