[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-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.

> 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);
> 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.