* libxl videoram for cirrus graphics
@ 2013-09-17 12:54 Rob Hoes
2013-09-17 13:17 ` Ian Campbell
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Rob Hoes @ 2013-09-17 12:54 UTC (permalink / raw
To: xen-devel@lists.xen.org
Hi,
The videoram setting in xl config files is documented as follows:
"
videoram=MBYTES
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. The default amount of video ram for stdvga is 8MB which is sufficient for e.g. 1600x1200 at 32bpp and videoram option is currently working only when using the qemu-xen-traditional device-model.
When using the emulated Cirrus graphics card (vga="cirrus") the amount of video ram is fixed at 4MB which is sufficient for 1024x768 at 32 bpp and videoram option is currently working only when using the upstream qemu-xen device-model.
"
XenServer also uses a default of 4MB video ram for cirrus graphics.
However, when I use xl to create a VM with cirrus graphics, I get 8MB of video RAM by default. Moreover, setting it to videoram=4 in the config file leads to complaints from libxl:
libxl_create.c:228:libxl__domain_build_info_setdefault: videoram must be at least 8 mb
Is there a bug in the logic in libxl_create.c?
Cheers,
Rob
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 12:54 libxl videoram for cirrus graphics Rob Hoes
@ 2013-09-17 13:17 ` Ian Campbell
2013-09-17 13:29 ` Rob Hoes
2013-09-20 10:49 ` Stefano Stabellini
2013-09-17 13:24 ` Wei Liu
2013-09-17 14:03 ` Frediano Ziglio
2 siblings, 2 replies; 16+ messages in thread
From: Ian Campbell @ 2013-09-17 13:17 UTC (permalink / raw
To: Rob Hoes; +Cc: Ian Jackson, Stefano Stabellini, xen-devel@lists.xen.org
On Tue, 2013-09-17 at 12:54 +0000, Rob Hoes wrote:
> Hi,
>
> The videoram setting in xl config files is documented as follows:
>
> "
> videoram=MBYTES
>
> 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. The default amount of video ram for stdvga is 8MB which is
> sufficient for e.g. 1600x1200 at 32bpp and videoram option is
> currently working only when using the qemu-xen-traditional
> device-model.
>
> When using the emulated Cirrus graphics card (vga="cirrus") the
> amount of video ram is fixed at 4MB which is sufficient for 1024x768
> at 32 bpp and videoram option is currently working only when using the
> upstream qemu-xen device-model.
> "
>
> XenServer also uses a default of 4MB video ram for cirrus graphics.
>
> However, when I use xl to create a VM with cirrus graphics, I get 8MB
> of video RAM by default. Moreover, setting it to videoram=4 in the
> config file leads to complaints from libxl:
>
> libxl_create.c:228:libxl__domain_build_info_setdefault: videoram must be at least 8 mb
>
> Is there a bug in the logic in libxl_create.c?
Either the docs of the code or both would seem to be wrong. Since you
say you happily use 4MB in XenServer I guess it is both?
I noticed that the docs also say the amount is fixed, but that's not
what the code implements either..
I'm not really sure who, if anyone, might know definitively what is
going on here. Stefano has some involvement in this video mem stuff once
upon a time and Ian is the qemu-trad maintainer, so I've CCdthem
both ;-)
>
> Cheers,
> Rob
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 12:54 libxl videoram for cirrus graphics Rob Hoes
2013-09-17 13:17 ` Ian Campbell
@ 2013-09-17 13:24 ` Wei Liu
2013-09-17 14:00 ` Ian Campbell
2013-09-17 14:03 ` Frediano Ziglio
2 siblings, 1 reply; 16+ messages in thread
From: Wei Liu @ 2013-09-17 13:24 UTC (permalink / raw
To: Rob Hoes; +Cc: wei.liu2, xen-devel@lists.xen.org
On Tue, Sep 17, 2013 at 12:54:57PM +0000, Rob Hoes wrote:
> Hi,
>
> The videoram setting in xl config files is documented as follows:
>
> "
> videoram=MBYTES
>
> 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. The default amount of video ram for stdvga is 8MB which is sufficient for e.g. 1600x1200 at 32bpp and videoram option is currently working only when using the qemu-xen-traditional device-model.
>
> When using the emulated Cirrus graphics card (vga="cirrus") the amount of video ram is fixed at 4MB which is sufficient for 1024x768 at 32 bpp and videoram option is currently working only when using the upstream qemu-xen device-model.
> "
>
> XenServer also uses a default of 4MB video ram for cirrus graphics.
>
> However, when I use xl to create a VM with cirrus graphics, I get 8MB of video RAM by default. Moreover, setting it to videoram=4 in the config file leads to complaints from libxl:
>
> libxl_create.c:228:libxl__domain_build_info_setdefault: videoram must be at least 8 mb
>
> Is there a bug in the logic in libxl_create.c?
>
I think the document needs to be updated.
commit 2e814a017155b885e4d4b5a88dc05e7367a9722a
Author: Fabio Fantoni <fabio.fantoni@heliman.it>
Date: Fri Feb 15 13:32:27 2013 +0000
tools/libxl: Improve videoram setting
- If videoram setting is less than 8 mb shows error and exit.
- Added videoram setting for qemu upstream with cirrus (added in qemu 1.3).
- Updated xl.cfg man.
- Default and minimal videoram changed to 16 mb if stdvga is set and upstream
qemu is being used. This is required by qemu 1.4 to avoid a xen memory error
(qemu 1.3 doesn't complain about it, probably buggy).
Signed-off-by: Fabio Fantoni <fabio.fantoni@heliman.it>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
Wei.
> Cheers,
> Rob
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 13:17 ` Ian Campbell
@ 2013-09-17 13:29 ` Rob Hoes
2013-09-17 14:04 ` Ian Campbell
2013-09-20 10:49 ` Stefano Stabellini
1 sibling, 1 reply; 16+ messages in thread
From: Rob Hoes @ 2013-09-17 13:29 UTC (permalink / raw
To: Ian Campbell; +Cc: Stefano Stabellini, Ian Jackson, xen-devel@lists.xen.org
> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: 17 September 2013 2:18 PM
> To: Rob Hoes
> Cc: xen-devel@lists.xen.org; Stefano Stabellini; Ian Jackson
> Subject: Re: [Xen-devel] libxl videoram for cirrus graphics
>
> On Tue, 2013-09-17 at 12:54 +0000, Rob Hoes wrote:
> > Hi,
> >
> > The videoram setting in xl config files is documented as follows:
> >
> > "
> > videoram=MBYTES
> >
> > 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. The default amount of video ram for stdvga is 8MB which is
> > sufficient for e.g. 1600x1200 at 32bpp and videoram option is
> > currently working only when using the qemu-xen-traditional
> > device-model.
> >
> > When using the emulated Cirrus graphics card (vga="cirrus") the
> > amount of video ram is fixed at 4MB which is sufficient for 1024x768
> > at 32 bpp and videoram option is currently working only when using the
> > upstream qemu-xen device-model.
> > "
> >
> > XenServer also uses a default of 4MB video ram for cirrus graphics.
> >
> > However, when I use xl to create a VM with cirrus graphics, I get 8MB
> > of video RAM by default. Moreover, setting it to videoram=4 in the
> > config file leads to complaints from libxl:
> >
> > libxl_create.c:228:libxl__domain_build_info_setdefault: videoram
> > must be at least 8 mb
> >
> > Is there a bug in the logic in libxl_create.c?
>
> Either the docs of the code or both would seem to be wrong. Since you say
> you happily use 4MB in XenServer I guess it is both?
>
> I noticed that the docs also say the amount is fixed, but that's not what the
> code implements either..
Yes, as far as I can see, XenServer has always used 4MB by default, and allows one to override it as well (so the amount is not fixed). Not allowing 4MB anymore, as well as not allowing the override, may therefore lead to upgrade problems for XenServer users when we switch to using libxl, so I am hoping that we can make the checks in libxl a little less strict.
I think that something like this would work:
diff -r ba248e555458 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -213,8 +213,13 @@
if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
b_info->shadow_memkb = 0;
- if (b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_STD &&
- b_info->device_model_version ==
+ if (!b_info->u.hvm.vga.kind)
+ b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+
+ if (b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_CIRRUS) {
+ if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
+ b_info->video_memkb = 4 * 1024;
+ } else if (b_info->device_model_version ==
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
b_info->video_memkb = 16 * 1024;
@@ -251,8 +256,6 @@
if (!b_info->u.hvm.boot) return ERROR_NOMEM;
}
- if (!b_info->u.hvm.vga.kind)
- b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
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);
> I'm not really sure who, if anyone, might know definitively what is going on
> here. Stefano has some involvement in this video mem stuff once upon a
> time and Ian is the qemu-trad maintainer, so I've CCdthem both ;-)
>
> >
> > Cheers,
> > Rob
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 13:24 ` Wei Liu
@ 2013-09-17 14:00 ` Ian Campbell
2013-09-17 17:51 ` Fabio Fantoni
0 siblings, 1 reply; 16+ messages in thread
From: Ian Campbell @ 2013-09-17 14:00 UTC (permalink / raw
To: Wei Liu; +Cc: Fabio Fantoni, Rob Hoes, xen-devel@lists.xen.org
On Tue, 2013-09-17 at 14:24 +0100, Wei Liu wrote:
> On Tue, Sep 17, 2013 at 12:54:57PM +0000, Rob Hoes wrote:
> > Hi,
> >
> > The videoram setting in xl config files is documented as follows:
> >
> > "
> > videoram=MBYTES
> >
> > 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. The default amount of video ram for stdvga is 8MB which is sufficient for e.g. 1600x1200 at 32bpp and videoram option is currently working only when using the qemu-xen-traditional device-model.
> >
> > When using the emulated Cirrus graphics card (vga="cirrus") the amount of video ram is fixed at 4MB which is sufficient for 1024x768 at 32 bpp and videoram option is currently working only when using the upstream qemu-xen device-model.
> > "
> >
> > XenServer also uses a default of 4MB video ram for cirrus graphics.
> >
> > However, when I use xl to create a VM with cirrus graphics, I get 8MB of video RAM by default. Moreover, setting it to videoram=4 in the config file leads to complaints from libxl:
> >
> > libxl_create.c:228:libxl__domain_build_info_setdefault: videoram must be at least 8 mb
> >
> > Is there a bug in the logic in libxl_create.c?
> >
>
> I think the document needs to be updated.
>
> commit 2e814a017155b885e4d4b5a88dc05e7367a9722a
> Author: Fabio Fantoni <fabio.fantoni@heliman.it>
> Date: Fri Feb 15 13:32:27 2013 +0000
>
> tools/libxl: Improve videoram setting
>
> - If videoram setting is less than 8 mb shows error and exit.
I wonder why this is the case. Fabvio do you remmeberwhere this number
8M came from for Cirrus?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 12:54 libxl videoram for cirrus graphics Rob Hoes
2013-09-17 13:17 ` Ian Campbell
2013-09-17 13:24 ` Wei Liu
@ 2013-09-17 14:03 ` Frediano Ziglio
2013-09-17 17:00 ` Pasi Kärkkäinen
2013-09-18 9:13 ` Rob Hoes
2 siblings, 2 replies; 16+ messages in thread
From: Frediano Ziglio @ 2013-09-17 14:03 UTC (permalink / raw
To: Rob Hoes; +Cc: xen-devel@lists.xen.org
On Tue, 2013-09-17 at 12:54 +0000, Rob Hoes wrote:
> Hi,
>
> The videoram setting in xl config files is documented as follows:
>
> "
> videoram=MBYTES
>
> 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. The default amount of video ram for stdvga is 8MB which is sufficient for e.g. 1600x1200 at 32bpp and videoram option is currently working only when using the qemu-xen-traditional device-model.
>
> When using the emulated Cirrus graphics card (vga="cirrus") the amount of video ram is fixed at 4MB which is sufficient for 1024x768 at 32 bpp and videoram option is currently working only when using the upstream qemu-xen device-model.
> "
>
> XenServer also uses a default of 4MB video ram for cirrus graphics.
>
> However, when I use xl to create a VM with cirrus graphics, I get 8MB of video RAM by default. Moreover, setting it to videoram=4 in the config file leads to complaints from libxl:
>
> libxl_create.c:228:libxl__domain_build_info_setdefault: videoram must be at least 8 mb
>
> Is there a bug in the logic in libxl_create.c?
>
> Cheers,
> Rob
>
cirrus card support maximum of 4mb (which is also the default) so there
is IMHO no point in reporting an error, just a warning would be enough
Frediano
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 13:29 ` Rob Hoes
@ 2013-09-17 14:04 ` Ian Campbell
2013-09-17 14:52 ` Rob Hoes
0 siblings, 1 reply; 16+ messages in thread
From: Ian Campbell @ 2013-09-17 14:04 UTC (permalink / raw
To: Rob Hoes; +Cc: Stefano Stabellini, Ian Jackson, xen-devel@lists.xen.org
On Tue, 2013-09-17 at 14:29 +0100, Rob Hoes wrote:
>
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: 17 September 2013 2:18 PM
> > To: Rob Hoes
> > Cc: xen-devel@lists.xen.org; Stefano Stabellini; Ian Jackson
> > Subject: Re: [Xen-devel] libxl videoram for cirrus graphics
> >
> > On Tue, 2013-09-17 at 12:54 +0000, Rob Hoes wrote:
> > > Hi,
> > >
> > > The videoram setting in xl config files is documented as follows:
> > >
> > > "
> > > videoram=MBYTES
> > >
> > > 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. The default amount of video ram for stdvga is 8MB which is
> > > sufficient for e.g. 1600x1200 at 32bpp and videoram option is
> > > currently working only when using the qemu-xen-traditional
> > > device-model.
> > >
> > > When using the emulated Cirrus graphics card (vga="cirrus") the
> > > amount of video ram is fixed at 4MB which is sufficient for 1024x768
> > > at 32 bpp and videoram option is currently working only when using the
> > > upstream qemu-xen device-model.
> > > "
> > >
> > > XenServer also uses a default of 4MB video ram for cirrus graphics.
> > >
> > > However, when I use xl to create a VM with cirrus graphics, I get 8MB
> > > of video RAM by default. Moreover, setting it to videoram=4 in the
> > > config file leads to complaints from libxl:
> > >
> > > libxl_create.c:228:libxl__domain_build_info_setdefault: videoram
> > > must be at least 8 mb
> > >
> > > Is there a bug in the logic in libxl_create.c?
> >
> > Either the docs of the code or both would seem to be wrong. Since you say
> > you happily use 4MB in XenServer I guess it is both?
> >
> > I noticed that the docs also say the amount is fixed, but that's not what the
> > code implements either..
>
> Yes, as far as I can see, XenServer has always used 4MB by default, and allows one to override it as well (so the amount is not fixed). Not allowing 4MB anymore, as well as not allowing the override, may therefore lead to upgrade problems for XenServer users when we switch to using libxl, so I am hoping that we can make the checks in libxl a little less strict.
>
> I think that something like this would work:
>
> diff -r ba248e555458 tools/libxl/libxl_create.c
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -213,8 +213,13 @@
> if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
> b_info->shadow_memkb = 0;
>
> - if (b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_STD &&
> - b_info->device_model_version ==
> + if (!b_info->u.hvm.vga.kind)
> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> +
> + if (b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_CIRRUS) {
> + if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> + b_info->video_memkb = 4 * 1024;
> + } else if (b_info->device_model_version ==
> LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> b_info->video_memkb = 16 * 1024;
Do the following checks for b_info->video_memkb == LIBXL_MEMKB_DEFAULT
and b_info->video_memkb < (8 * 1024) not need modifying then?
Is the behaviour here any different to just changing the 8s into 4s?
Can you also update the docs, it's at least wrong about the 4MB being a
fixed value.
> @@ -251,8 +256,6 @@
> if (!b_info->u.hvm.boot) return ERROR_NOMEM;
> }
>
> - if (!b_info->u.hvm.vga.kind)
> - b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> 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);
>
> > I'm not really sure who, if anyone, might know definitively what is going on
> > here. Stefano has some involvement in this video mem stuff once upon a
> > time and Ian is the qemu-trad maintainer, so I've CCdthem both ;-)
> >
> > >
> > > Cheers,
> > > Rob
> > >
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@lists.xen.org
> > > http://lists.xen.org/xen-devel
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 14:04 ` Ian Campbell
@ 2013-09-17 14:52 ` Rob Hoes
2013-09-17 15:01 ` Ian Campbell
0 siblings, 1 reply; 16+ messages in thread
From: Rob Hoes @ 2013-09-17 14:52 UTC (permalink / raw
To: Ian Campbell; +Cc: Stefano Stabellini, Ian Jackson, xen-devel@lists.xen.org
> > I think that something like this would work:
> >
> > diff -r ba248e555458 tools/libxl/libxl_create.c
> > --- a/tools/libxl/libxl_create.c
> > +++ b/tools/libxl/libxl_create.c
> > @@ -213,8 +213,13 @@
> > if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
> > b_info->shadow_memkb = 0;
> >
> > - if (b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_STD &&
> > - b_info->device_model_version ==
> > + if (!b_info->u.hvm.vga.kind)
> > + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> > +
> > + if (b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_CIRRUS) {
> > + if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> > + b_info->video_memkb = 4 * 1024;
> > + } else if (b_info->device_model_version ==
> > LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> > if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> > b_info->video_memkb = 16 * 1024;
>
> Do the following checks for b_info->video_memkb ==
> LIBXL_MEMKB_DEFAULT and b_info->video_memkb < (8 * 1024) not need
> modifying then?
>
> Is the behaviour here any different to just changing the 8s into 4s?
I was assuming that we need to keep the default of 8MB, as well as the restriction to have at least 8MB, for standard vga (when kind != CIRRUS)
> Can you also update the docs, it's at least wrong about the 4MB being a fixed
> value.
Yes, I'll send a new patch.
Rob
> > @@ -251,8 +256,6 @@
> > if (!b_info->u.hvm.boot) return ERROR_NOMEM;
> > }
> >
> > - if (!b_info->u.hvm.vga.kind)
> > - b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> > 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);
> >
> > > I'm not really sure who, if anyone, might know definitively what is
> > > going on here. Stefano has some involvement in this video mem stuff
> > > once upon a time and Ian is the qemu-trad maintainer, so I've
> > > CCdthem both ;-)
> > >
> > > >
> > > > Cheers,
> > > > Rob
> > > >
> > > > _______________________________________________
> > > > Xen-devel mailing list
> > > > Xen-devel@lists.xen.org
> > > > http://lists.xen.org/xen-devel
> > >
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 14:52 ` Rob Hoes
@ 2013-09-17 15:01 ` Ian Campbell
0 siblings, 0 replies; 16+ messages in thread
From: Ian Campbell @ 2013-09-17 15:01 UTC (permalink / raw
To: Rob Hoes; +Cc: Stefano Stabellini, Ian Jackson, xen-devel@lists.xen.org
On Tue, 2013-09-17 at 15:52 +0100, Rob Hoes wrote:
> > > I think that something like this would work:
> > >
> > > diff -r ba248e555458 tools/libxl/libxl_create.c
> > > --- a/tools/libxl/libxl_create.c
> > > +++ b/tools/libxl/libxl_create.c
> > > @@ -213,8 +213,13 @@
> > > if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
> > > b_info->shadow_memkb = 0;
> > >
> > > - if (b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_STD &&
> > > - b_info->device_model_version ==
> > > + if (!b_info->u.hvm.vga.kind)
> > > + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> > > +
> > > + if (b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_CIRRUS) {
> > > + if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> > > + b_info->video_memkb = 4 * 1024;
> > > + } else if (b_info->device_model_version ==
> > > LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> > > if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> > > b_info->video_memkb = 16 * 1024;
> >
> > Do the following checks for b_info->video_memkb ==
> > LIBXL_MEMKB_DEFAULT and b_info->video_memkb < (8 * 1024) not need
> > modifying then?
> >
> > Is the behaviour here any different to just changing the 8s into 4s?
>
> I was assuming that we need to keep the default of 8MB, as well as the
> restriction to have at least 8MB, for standard vga (when kind !=
> CIRRUS)
Oh right, this nest of twisted conditionals had me confused.I thought at
first it could be made a switch but then realised it was testing
different properties down the if/else chain...
I wonder if it might be better to duplication some code for clarities
sake:
switch(dm_versioN)
case LIBXL...QEMU_TRAD:
switch (vga.kind):
case CIRRUS:
if videomembk == default
videomemkb= 4
if videomemkbv < 4
error
break
case STDVGA:
some stuff
break
case LIBXL..QEMU_XEN
switch(vga.kind)
etc duplicating some of the above with default of 8
etc
or maybe it would be even clearer to refactor into
libxl__video_{cirrus,stdvga}_set defaults?
> > Can you also update the docs, it's at least wrong about the 4MB being a fixed
> > value.
>
> Yes, I'll send a new patch.
>
> Rob
>
> > > @@ -251,8 +256,6 @@
> > > if (!b_info->u.hvm.boot) return ERROR_NOMEM;
> > > }
> > >
> > > - if (!b_info->u.hvm.vga.kind)
> > > - b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> > > 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);
> > >
> > > > I'm not really sure who, if anyone, might know definitively what is
> > > > going on here. Stefano has some involvement in this video mem stuff
> > > > once upon a time and Ian is the qemu-trad maintainer, so I've
> > > > CCdthem both ;-)
> > > >
> > > > >
> > > > > Cheers,
> > > > > Rob
> > > > >
> > > > > _______________________________________________
> > > > > Xen-devel mailing list
> > > > > Xen-devel@lists.xen.org
> > > > > http://lists.xen.org/xen-devel
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 14:03 ` Frediano Ziglio
@ 2013-09-17 17:00 ` Pasi Kärkkäinen
2013-09-18 13:51 ` Rob Hoes
2013-09-18 9:13 ` Rob Hoes
1 sibling, 1 reply; 16+ messages in thread
From: Pasi Kärkkäinen @ 2013-09-17 17:00 UTC (permalink / raw
To: Frediano Ziglio; +Cc: Rob Hoes, xen-devel@lists.xen.org
On Tue, Sep 17, 2013 at 03:03:20PM +0100, Frediano Ziglio wrote:
> On Tue, 2013-09-17 at 12:54 +0000, Rob Hoes wrote:
> > Hi,
> >
> > The videoram setting in xl config files is documented as follows:
> >
> > "
> > videoram=MBYTES
> >
> > 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. The default amount of video ram for stdvga is 8MB which is sufficient for e.g. 1600x1200 at 32bpp and videoram option is currently working only when using the qemu-xen-traditional device-model.
> >
> > When using the emulated Cirrus graphics card (vga="cirrus") the amount of video ram is fixed at 4MB which is sufficient for 1024x768 at 32 bpp and videoram option is currently working only when using the upstream qemu-xen device-model.
> > "
> >
> > XenServer also uses a default of 4MB video ram for cirrus graphics.
> >
> > However, when I use xl to create a VM with cirrus graphics, I get 8MB of video RAM by default. Moreover, setting it to videoram=4 in the config file leads to complaints from libxl:
> >
> > libxl_create.c:228:libxl__domain_build_info_setdefault: videoram must be at least 8 mb
> >
> > Is there a bug in the logic in libxl_create.c?
> >
> > Cheers,
> > Rob
> >
>
> cirrus card support maximum of 4mb (which is also the default) so there
> is IMHO no point in reporting an error, just a warning would be enough
>
Uhm, I think Rob says above he's able to use 8MB with Cirrus.
Maybe there's a difference between qemu-traditional and qemu-upstream?
Maybe the old qemu-traditional only supports fixed 4MB with Cirrus?
And new qemu-upstream allows more videomem, with the possibility of configuring the amount of videomem?
Just guessing here..
> Frediano
>
-- Pasi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 14:00 ` Ian Campbell
@ 2013-09-17 17:51 ` Fabio Fantoni
2013-09-17 19:13 ` Ian Campbell
0 siblings, 1 reply; 16+ messages in thread
From: Fabio Fantoni @ 2013-09-17 17:51 UTC (permalink / raw
To: Ian Campbell; +Cc: Fabio Fantoni, Wei Liu, Rob Hoes, xen-devel@lists.xen.org
[-- Attachment #1.1: Type: text/plain, Size: 2153 bytes --]
With upstream qemu >1.4 videoram must be atleast 8 mb with cirrus and 16 mb
with stdvga or domUs will be unable to start (critical xen memory error).
All details should be on topic about my patch of some months ago.
2013/9/17 Ian Campbell <ian.campbell@citrix.com>
> On Tue, 2013-09-17 at 14:24 +0100, Wei Liu wrote:
> > On Tue, Sep 17, 2013 at 12:54:57PM +0000, Rob Hoes wrote:
> > > Hi,
> > >
> > > The videoram setting in xl config files is documented as follows:
> > >
> > > "
> > > videoram=MBYTES
> > >
> > > 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. The default amount of video ram for stdvga is 8MB which is
> sufficient for e.g. 1600x1200 at 32bpp and videoram option is currently
> working only when using the qemu-xen-traditional device-model.
> > >
> > > When using the emulated Cirrus graphics card (vga="cirrus") the
> amount of video ram is fixed at 4MB which is sufficient for 1024x768 at 32
> bpp and videoram option is currently working only when using the upstream
> qemu-xen device-model.
> > > "
> > >
> > > XenServer also uses a default of 4MB video ram for cirrus graphics.
> > >
> > > However, when I use xl to create a VM with cirrus graphics, I get 8MB
> of video RAM by default. Moreover, setting it to videoram=4 in the config
> file leads to complaints from libxl:
> > >
> > > libxl_create.c:228:libxl__domain_build_info_setdefault: videoram
> must be at least 8 mb
> > >
> > > Is there a bug in the logic in libxl_create.c?
> > >
> >
> > I think the document needs to be updated.
> >
> > commit 2e814a017155b885e4d4b5a88dc05e7367a9722a
> > Author: Fabio Fantoni <fabio.fantoni@heliman.it>
> > Date: Fri Feb 15 13:32:27 2013 +0000
> >
> > tools/libxl: Improve videoram setting
> >
> > - If videoram setting is less than 8 mb shows error and exit.
>
> I wonder why this is the case. Fabvio do you remmeberwhere this number
> 8M came from for Cirrus?
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
[-- Attachment #1.2: Type: text/html, Size: 2996 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 17:51 ` Fabio Fantoni
@ 2013-09-17 19:13 ` Ian Campbell
0 siblings, 0 replies; 16+ messages in thread
From: Ian Campbell @ 2013-09-17 19:13 UTC (permalink / raw
To: Fabio Fantoni; +Cc: Fabio Fantoni, Wei Liu, Rob Hoes, xen-devel@lists.xen.org
On Tue, 2013-09-17 at 19:51 +0200, Fabio Fantoni wrote:
> With upstream qemu >1.4 videoram must be atleast 8 mb with cirrus and
> 16 mb with stdvga or domUs will be unable to start (critical xen
> memory error).
So we should make it 4mb for qemu-trad and 8 for qemu-xen?
>
> All details should be on topic about my patch of some months ago.
Do you happen to have a link?
>
>
>
> 2013/9/17 Ian Campbell <ian.campbell@citrix.com>
> On Tue, 2013-09-17 at 14:24 +0100, Wei Liu wrote:
> > On Tue, Sep 17, 2013 at 12:54:57PM +0000, Rob Hoes wrote:
> > > Hi,
> > >
> > > The videoram setting in xl config files is documented as
> follows:
> > >
> > > "
> > > videoram=MBYTES
> > >
> > > 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. The default amount of video
> ram for stdvga is 8MB which is sufficient for e.g. 1600x1200
> at 32bpp and videoram option is currently working only when
> using the qemu-xen-traditional device-model.
> > >
> > > When using the emulated Cirrus graphics card
> (vga="cirrus") the amount of video ram is fixed at 4MB which
> is sufficient for 1024x768 at 32 bpp and videoram option is
> currently working only when using the upstream qemu-xen
> device-model.
> > > "
> > >
> > > XenServer also uses a default of 4MB video ram for cirrus
> graphics.
> > >
> > > However, when I use xl to create a VM with cirrus
> graphics, I get 8MB of video RAM by default. Moreover, setting
> it to videoram=4 in the config file leads to complaints from
> libxl:
> > >
> > >
> libxl_create.c:228:libxl__domain_build_info_setdefault:
> videoram must be at least 8 mb
> > >
> > > Is there a bug in the logic in libxl_create.c?
> > >
> >
> > I think the document needs to be updated.
> >
> > commit 2e814a017155b885e4d4b5a88dc05e7367a9722a
> > Author: Fabio Fantoni <fabio.fantoni@heliman.it>
> > Date: Fri Feb 15 13:32:27 2013 +0000
> >
> > tools/libxl: Improve videoram setting
> >
> > - If videoram setting is less than 8 mb shows error and
> exit.
>
>
> I wonder why this is the case. Fabvio do you remmeberwhere
> this number
> 8M came from for Cirrus?
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 14:03 ` Frediano Ziglio
2013-09-17 17:00 ` Pasi Kärkkäinen
@ 2013-09-18 9:13 ` Rob Hoes
2013-09-18 9:40 ` Fabio Fantoni
1 sibling, 1 reply; 16+ messages in thread
From: Rob Hoes @ 2013-09-18 9:13 UTC (permalink / raw
To: Frediano Ziglio; +Cc: xen-devel@lists.xen.org
> cirrus card support maximum of 4mb (which is also the default) so there is
> IMHO no point in reporting an error, just a warning would be enough
Frediano, does what you said apply to traditional QEMU only? Fabio seems to suggest 8MB is needed on upstream QEMU.
And did you really mean "maximum"? 8MB seems to work for cirrus on traditional-QEMU.
Rob
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-18 9:13 ` Rob Hoes
@ 2013-09-18 9:40 ` Fabio Fantoni
0 siblings, 0 replies; 16+ messages in thread
From: Fabio Fantoni @ 2013-09-18 9:40 UTC (permalink / raw
To: Rob Hoes; +Cc: Frediano Ziglio, xen-devel@lists.xen.org
Il 18/09/2013 11:13, Rob Hoes ha scritto:
>> cirrus card support maximum of 4mb (which is also the default) so there is
>> IMHO no point in reporting an error, just a warning would be enough
> Frediano, does what you said apply to traditional QEMU only? Fabio seems to suggest 8MB is needed on upstream QEMU.
After my patch the hvm domUs start correctly with both cirrus and stdvga
with all upstream qemu versions (I tested up to 1.6). Without it the hvm
domUs was unable to start in some case with qemu 1.3 and all case of
qemu >=1.4.
>
> And did you really mean "maximum"? 8MB seems to work for cirrus on traditional-QEMU.
If I remember good I did fast test with qemu traditional with my patch
and was working with both vgas, the only case (but I not completely
sure) is with qemu-trad and cirrus vga where 4 mb of videoram could be
unused IF cirrus support only up 4 mb.
>
> Rob
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 17:00 ` Pasi Kärkkäinen
@ 2013-09-18 13:51 ` Rob Hoes
0 siblings, 0 replies; 16+ messages in thread
From: Rob Hoes @ 2013-09-18 13:51 UTC (permalink / raw
To: 'Pasi Kärkkäinen', Frediano Ziglio
Cc: Ian Campbell, xen-devel@lists.xen.org
> > cirrus card support maximum of 4mb (which is also the default) so
> > there is IMHO no point in reporting an error, just a warning would be
> > enough
> >
>
> Uhm, I think Rob says above he's able to use 8MB with Cirrus.
> Maybe there's a difference between qemu-traditional and qemu-upstream?
>
> Maybe the old qemu-traditional only supports fixed 4MB with Cirrus?
> And new qemu-upstream allows more videomem, with the possibility of
> configuring the amount of videomem?
I just talked to Frediano. He says that it is likely that, for Cirrus, the videoram parameter on the QEMU command line is silently ignored. When I said that I was able to use 8MB on cirrus, I only looked at what was put on the command line, so perhaps I wasn't actually getting 8MB.
I think that for traditional, we should make the default 4MB, and also allow other values (but log a warning if a value other than 4MB is used). For upstream qemu, I think we should just leave things as they are, unless someone has a better idea (I am no expert in this field).
I'll shortly submit a patch along the lines of what Ian suggested.
Rob
> Just guessing here..
>
> > Frediano
> >
>
> -- Pasi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libxl videoram for cirrus graphics
2013-09-17 13:17 ` Ian Campbell
2013-09-17 13:29 ` Rob Hoes
@ 2013-09-20 10:49 ` Stefano Stabellini
1 sibling, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2013-09-20 10:49 UTC (permalink / raw
To: Ian Campbell
Cc: Ian Jackson, Stefano Stabellini, Rob Hoes,
xen-devel@lists.xen.org
On Tue, 17 Sep 2013, Ian Campbell wrote:
> On Tue, 2013-09-17 at 12:54 +0000, Rob Hoes wrote:
> > Hi,
> >
> > The videoram setting in xl config files is documented as follows:
> >
> > "
> > videoram=MBYTES
> >
> > 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. The default amount of video ram for stdvga is 8MB which is
> > sufficient for e.g. 1600x1200 at 32bpp and videoram option is
> > currently working only when using the qemu-xen-traditional
> > device-model.
> >
> > When using the emulated Cirrus graphics card (vga="cirrus") the
> > amount of video ram is fixed at 4MB which is sufficient for 1024x768
> > at 32 bpp and videoram option is currently working only when using the
> > upstream qemu-xen device-model.
> > "
> >
> > XenServer also uses a default of 4MB video ram for cirrus graphics.
> >
> > However, when I use xl to create a VM with cirrus graphics, I get 8MB
> > of video RAM by default. Moreover, setting it to videoram=4 in the
> > config file leads to complaints from libxl:
> >
> > libxl_create.c:228:libxl__domain_build_info_setdefault: videoram must be at least 8 mb
> >
> > Is there a bug in the logic in libxl_create.c?
>
> Either the docs of the code or both would seem to be wrong. Since you
> say you happily use 4MB in XenServer I guess it is both?
>
> I noticed that the docs also say the amount is fixed, but that's not
> what the code implements either..
>
> I'm not really sure who, if anyone, might know definitively what is
> going on here. Stefano has some involvement in this video mem stuff once
> upon a time and Ian is the qemu-trad maintainer, so I've CCdthem
> both ;-)
AFAICT we always had 8 mb for Cirrus in libxl. I am not really sure
where this discrepancy between the docs and the code comes from.
I think that if 4mb actually works fine with different guests OSes then
we should allow it.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-09-20 10:49 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-17 12:54 libxl videoram for cirrus graphics Rob Hoes
2013-09-17 13:17 ` Ian Campbell
2013-09-17 13:29 ` Rob Hoes
2013-09-17 14:04 ` Ian Campbell
2013-09-17 14:52 ` Rob Hoes
2013-09-17 15:01 ` Ian Campbell
2013-09-20 10:49 ` Stefano Stabellini
2013-09-17 13:24 ` Wei Liu
2013-09-17 14:00 ` Ian Campbell
2013-09-17 17:51 ` Fabio Fantoni
2013-09-17 19:13 ` Ian Campbell
2013-09-17 14:03 ` Frediano Ziglio
2013-09-17 17:00 ` Pasi Kärkkäinen
2013-09-18 13:51 ` Rob Hoes
2013-09-18 9:13 ` Rob Hoes
2013-09-18 9:40 ` Fabio Fantoni
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.