Xen project Mailing List

Re: [Xen-devel] [PATCH 2/3] tools:libxl: Add qxl vga interface support v2

To: Ian Campbell <Ian.Campbell@xxxxxxxxxx>

Date: Tue, 3 Jul 2012 19:30:02 +0800

Cc: "Xen-Devel \(E-mail\)" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>

Delivery-date: Tue, 03 Jul 2012 11:30:27 +0000

List-id: Xen developer discussion <xen-devel.lists.xen.org>

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@xxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx> wrote: >> > On Tue, 2012-06-05 at 12:22 +0100, ZhouPeng wrote: >> >> changeset: 25454:1804a873a64d >> >> tag: tip >> >> user: Zhou Peng <ailvpeng25@xxxxxxxxx> >> >> 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@xxxxxxxxx> >> > >> > 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@xxxxxxxxxxxxx http://lists.xen.org/xen-devel

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.