|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [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@xxxxxxxxx>
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@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.
> >
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |