[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu

Il 02/05/2014 22:04, Fabio Fantoni ha scritto:
2014-05-02 21:44 GMT+02:00 Don Slutz <dslutz@xxxxxxxxxxx <mailto:dslutz@xxxxxxxxxxx>>:

    On 05/02/14 07:41, Ian Campbell wrote:

        On Sat, 2014-04-19 at 14:16 +0200, Fabio Fantoni wrote:

            Reading one qemu-devel post seems that setting video memory of
            cirrus vga with upstream qemu is wrong even if not show

      You can add my:

    Reviewed-by: Don Slutz <dslutz@xxxxxxxxxxx

    I have a similar code change locally (part of my pending
    list of to dos) (I just changed the global arg...).

        You later provided links but I think the conversation should be
        referenced here.

    I was part of the conversation.  When I was looking into upstreaming
    a change I have (pci_min_hole) to xen & qemu, I was asked by QEMU
    to report if it was not used (i.e. non x86 cpu's).  While I was
    testing my
    change to QEMU under xen I noticed:

    Warning: "-global vga.vram_size_mb=16" not used

    In /var/log/xen/qemu-dm-<guest>.log

    Here is the cross post to xen-devel:

     [Xen-devel] [PATCH v3 2/4] GlobalProperty: Display warning about
    unused -global


        Is this change correct for all versions of mainline qemu which
        might be using with Xen?

    What I know is that "-global cirrus-vga.vgamem_mb=32" does work
    with upstream QEMU 1.5.0, 1.6.0, 1.7.0 and 2.0.0.  I had added a debug
    output into the QEMU version above that show the amount of video ram
    that gets allocated and so the testing was quick and easy.  My
    understanding of QEMU is that when specified this way:

     "-device cirrus-vga,vgamem_mb=32"

    the error checking in QEMU will report when it does not like it:

    qemu-system-x86_64: Property '.vram_size_mb' not found

    (unlike -global).  So while I have not tested it, I would guess that
    any QEMU that accepts "-device cirrus-vga" will also either
    accept the change or report and error and not start. (Note: the
    change from "-vga cirrus" to "-device cirrus-vga" was done since
    4.3.0 and has yet to generate a bug).

Thanks for your reply, probably with my bad english I not understand good this part of your reply.
I changed to -device following this official qemu doc:
I did several tests of additional vga's parameters without using the-global in recent days with cirrus, stdvga and qxl, I've never seen errors and the amount of videoram by domUs seem ok.


        Please can you also explain what "wrong" means? Does it crash?
        Does it
        silently ignore the setting? Does it do something else
        "wrong"? How bad
        is it?

    For me it just silently ignores the setting.  Which means that the
    cirrus vga has the default memory and so will not support any
    display setting that needs more memory (the most likely reason
    videoram=32 is specified in the config file).

    I feel it is bad enough that a backport to 4.4 makes sense to

    Hope this helps.
        -Don Slutz

            Signed-off-by: Fabio Fantoni <fabio.fantoni@xxxxxxx


            - when this patch will be accepted upstream should be
               backported also on xen 4.4

        I think this very much depends on what you mean by "wrong" above.

            - with this qemu parameters seems correct but for further
               confirmation I posted a question about it:

        Any reply to this question?

        Anthony, Stefano: Opinions on this change?

              tools/libxl/libxl_dm.c |    5 ++---
              1 file changed, 2 insertions(+), 3 deletions(-)

            diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
            index 8abed7b..90f19b7 100644
            --- a/tools/libxl/libxl_dm.c
            +++ b/tools/libxl/libxl_dm.c
            @@ -508,9 +508,8 @@ static char **
            libxl__build_device_model_args_new(libxl__gc *gc,
                          flexarray_append_pair(dm_args, "-device",
                      case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
            -            flexarray_append_pair(dm_args, "-device",
            -            flexarray_append_pair(dm_args, "-global",
            -                GCSPRINTF("vga.vram_size_mb=%d",
            +            flexarray_append_pair(dm_args, "-device",
            +                GCSPRINTF("cirrus-vga,vgamem_mb=%d",
                      case LIBXL_VGA_INTERFACE_TYPE_NONE:

        Xen-devel mailing list
        Xen-devel@xxxxxxxxxxxxx <mailto:Xen-devel@xxxxxxxxxxxxx>

Xen-devel mailing list



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