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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Ian Campbell
2012-Jun-06 13:05 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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); > >
ZhouPeng
2012-Jun-07 03:19 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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
Ian Campbell
2012-Jun-07 06:08 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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.
ZhouPeng
2012-Jun-07 07:49 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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
ZhouPeng
2012-Jul-03 11:30 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Ian Campbell
2012-Jul-03 14:24 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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. > > > > >
George Dunlap
2012-Oct-10 16:50 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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
ZhouPeng
2012-Oct-14 05:16 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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
ZhouPeng
2012-Oct-18 10:19 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
George Dunlap
2012-Oct-22 14:36 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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
ZhouPeng
2012-Oct-24 08:06 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
George Dunlap
2012-Oct-24 13:25 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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
ZhouPeng
2012-Oct-30 07:45 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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
Ian Campbell
2012-Nov-19 11:37 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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.htmlWe 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.
ZhouPeng
2012-Nov-21 03:27 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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
Ian Campbell
2012-Nov-21 11:02 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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. >
George Dunlap
2013-Jan-16 17:00 UTC
Re: [PATCH 2/3] tools:libxl: Add qxl vga interface support v2
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel