From mboxrd@z Thu Jan 1 00:00:00 1970 From: Fabio Fantoni Subject: Re: [PATCH v3] tools/libxl: Improve videoram setting Date: Tue, 15 Jan 2013 16:03:28 +0100 Message-ID: <50F56FC0.6000902@tiscali.it> References: <50F036FA.9090500@tiscali.it> <50F41548.1010508@tiscali.it> Reply-To: fantonifabio@tiscali.it Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4894879214654454373==" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: xen-devel , Ian Campbell List-Id: xen-devel@lists.xenproject.org Questo è un messaggio firmato digitalmente in formato MIME. --===============4894879214654454373== Content-Type: multipart/signed; protocol="application/pkcs7-signature"; micalg=sha1; boundary="------------ms080109050106030207080901" Questo è un messaggio firmato digitalmente in formato MIME. --------------ms080109050106030207080901 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: quoted-printable Il 14/01/2013 19:21, Stefano Stabellini ha scritto: > On Mon, 14 Jan 2013, Fabio Fantoni wrote: >> Il 14/01/2013 13:49, Stefano Stabellini ha scritto: >>> On Fri, 11 Jan 2013, Fabio Fantoni wrote: >>>> tools/libxl: Improve videoram setting >>>> >>>> - If videoram setting is less than 8 mb shows error. >>>> - Added videoram setting for qemu upstream with cirrus (added in qem= u >>>> 1.3). >>>> - Updated xl.cfg man. >>>> >>>> Signed-off-by: Fabio Fantoni >>>> --- >>>> docs/man/xl.cfg.pod.5 | 14 +++++--------- >>>> tools/libxl/libxl_dm.c | 6 ++++++ >>>> tools/libxl/xl_cmdimpl.c | 10 ++++++++-- >>>> 3 files changed, 19 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 >>>> index caba162..9c5cdcd 100644 >>>> --- a/docs/man/xl.cfg.pod.5 >>>> +++ b/docs/man/xl.cfg.pod.5 >>>> @@ -974,19 +974,15 @@ in the B for configuring virt= ual >>>> frame >>>> buffer devices >>>> >>>> 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 >>>> -option (see below). >>>> +available. >>>> The default amount of video ram for stdvga is 8MB which is suffic= ient >>>> -for e.g. 1600x1200 at 32bpp. >>>> +for e.g. 1600x1200 at 32bpp and videoram option is currently workin= g >>>> +only when using the qemu-xen-traditional device-model. >>>> >>>> When using the emulated Cirrus graphics card (B) >>>> the amount of video ram is fixed at 4MB which is sufficient >>>> -for 1024x768 at 32 bpp. >>>> - >>>> -videoram option is currently only available when using the >>>> -qemu-xen-traditional device-model. Upstream qemu-xen device-model >>>> -currently does not support changing the amount of video memory for = the >>>> -emulated graphics device. >>>> +for 1024x768 at 32 bpp and videoram option is currently working >>>> +only when using the upstream qemu-xen device-model. >>>> >>>> =3Ditem B >>>> >>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c >>>> index c036dc1..d719130 100644 >>>> --- a/tools/libxl/libxl_dm.c >>>> +++ b/tools/libxl/libxl_dm.c >>>> @@ -430,6 +430,12 @@ static char ** >>>> libxl__build_device_model_args_new(libxl__gc *gc, >>>> break; >>>> case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: >>>> flexarray_vappend(dm_args, "-vga", "cirrus", NULL); >>>> + if (b_info->video_memkb) { >>>> + flexarray_vappend(dm_args, "-global", >>>> + libxl__sprintf(gc, "vga.vram_size_mb=3D%d", >>>> + libxl__sizekb_to_mb(b_info->video_memkb)), >>>> + NULL); >>>> + } >>>> break; >>>> } >>> Looking at the QEMU code, it seems to be that there another variable >>> with the same name to define the videoram size in vga-pci.c: >>> >>> static Property vga_pci_properties[] =3D { >>> DEFINE_PROP_UINT32("vgamem_mb", PCIVGAState, vga.vram_size_mb, = 16), >>> >>> Is it possible that this code works even for std-vga out of the box? >>> >>> >>>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c >>>> index e964bf1..6e11c8d 100644 >>>> --- a/tools/libxl/xl_cmdimpl.c >>>> +++ b/tools/libxl/xl_cmdimpl.c >>>> @@ -831,8 +831,14 @@ static void parse_config_data(const char >>>> *config_source, >>>> >>>> xlu_cfg_get_defbool(config, "localtime", &b_info->localtime, = 0); >>>> >>>> - if (!xlu_cfg_get_long (config, "videoram", &l, 0)) >>>> - b_info->video_memkb =3D l * 1024; >>>> + if (!xlu_cfg_get_long (config, "videoram", &l, 0)){ >>>> + if ( l < 8 ){ >>>> + fprintf(stderr, "ERROR: videoram must be at least 8 mb\= n"); >>>> + exit (1); >>>> + } else { >>>> + b_info->video_memkb =3D l * 1024; >>>> + } >>>> + } >>> As Ian pointed out, it would be better if you moved the error check i= n >>> libxl, probably somewhere towards the beginning of >>> initiate_domain_create. >>> >>> >>> ----- >>> Nessun virus nel messaggio. >>> Controllato da AVG - www.avg.com >>> Versione: 2013.0.2890 / Database dei virus: 2638/6030 - Data di rila= scio: >>> 13/01/2013 >>> >>> >> About std-vga videoram it seems defined at fixed value and I found onl= y the >> cirrus and qxl being custimisable. >> For example this is the commit that make cirrus custimisable: >> http://xenbits.xen.org/gitweb/?p=3Dstaging/qemu-upstream-unstable.git;= a=3Dcommit;h=3D19403a68fb8eaefb2e1245b6a8384d3a3ffa7ca0 > I did a quick test and it seems that it should be possible to change th= e > amount of videoram for stdvga too using the same command line option, > however at the moment it just errors out. > Therefore I am OK with this patch only taking care of Cirrus for the mo= ment. > > >> About error check I added it only in this last version of patch. >> Why wouldn't check it near other domU configuration parameters? >> This seems more a sanity check to do before initiate starting domU. > =20 > I know that it might seem counterintuitive but we try to keep all the > significant informations about the platform within libxl. So if 8MB is > an important limit of the platform that libxl should know about it, > while xl should stay as dumb as possible. > I tried to move the check within libxl_create.c, at the point where it=20 sets video_memkb default value, but I couldn't make it exit in case of=20 failure. Also I'm not sure if the check should go there. Can you help me = to do it please? --------------ms080109050106030207080901 Content-Type: application/pkcs7-signature; name="smime.p7s" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="smime.p7s" Content-Description: Firma crittografica S/MIME MIAGCSqGSIb3DQEHAqCAMIACAQExCzAJBgUrDgMCGgUAMIAGCSqGSIb3DQEHAQAAoIINhjCC BjQwggQcoAMCAQICASAwDQYJKoZIhvcNAQEFBQAwfTELMAkGA1UEBhMCSUwxFjAUBgNVBAoT DVN0YXJ0Q29tIEx0ZC4xKzApBgNVBAsTIlNlY3VyZSBEaWdpdGFsIENlcnRpZmljYXRlIFNp Z25pbmcxKTAnBgNVBAMTIFN0YXJ0Q29tIENlcnRpZmljYXRpb24gQXV0aG9yaXR5MB4XDTA3 MTAyNDIxMDI1NVoXDTE3MTAyNDIxMDI1NVowgYwxCzAJBgNVBAYTAklMMRYwFAYDVQQKEw1T dGFydENvbSBMdGQuMSswKQYDVQQLEyJTZWN1cmUgRGlnaXRhbCBDZXJ0aWZpY2F0ZSBTaWdu aW5nMTgwNgYDVQQDEy9TdGFydENvbSBDbGFzcyAyIFByaW1hcnkgSW50ZXJtZWRpYXRlIENs aWVudCBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMsohUWcASz7GfKrpTOM KqANy9BV7V0igWdGxA8IU77L3aTxErQ+fcxtDYZ36Z6GH0YFn7fq5RADteP0AYzrCA+EQTfi 8q1+kA3m0nwtwXG94M5sIqsvs7lRP1aycBke/s5g9hJHryZ2acScnzczjBCAo7X1v5G3yw8M DP2m2RCye0KfgZ4nODerZJVzhAlOD9YejvAXZqHksw56HzElVIoYSZ3q4+RJuPXXfIoyby+Y 2m1E+YzX5iCZXBx05gk6MKAW1vaw4/v2OOLy6FZH3XHHtOkzUreG//CsFnB9+uaYSlR65cdG zTsmoIK8WH1ygoXhRBm98SD7Hf/r3FELNvUCAwEAAaOCAa0wggGpMA8GA1UdEwEB/wQFMAMB Af8wDgYDVR0PAQH/BAQDAgEGMB0GA1UdDgQWBBSuVYNv7DHKufcd+q9rMfPIHeOsuzAfBgNV HSMEGDAWgBROC+8apEBbpRdphzDKNGhD0EGu8jBmBggrBgEFBQcBAQRaMFgwJwYIKwYBBQUH MAGGG2h0dHA6Ly9vY3NwLnN0YXJ0c3NsLmNvbS9jYTAtBggrBgEFBQcwAoYhaHR0cDovL3d3 dy5zdGFydHNzbC5jb20vc2ZzY2EuY3J0MFsGA1UdHwRUMFIwJ6AloCOGIWh0dHA6Ly93d3cu c3RhcnRzc2wuY29tL3Nmc2NhLmNybDAnoCWgI4YhaHR0cDovL2NybC5zdGFydHNzbC5jb20v c2ZzY2EuY3JsMIGABgNVHSAEeTB3MHUGCysGAQQBgbU3AQIBMGYwLgYIKwYBBQUHAgEWImh0 dHA6Ly93d3cuc3RhcnRzc2wuY29tL3BvbGljeS5wZGYwNAYIKwYBBQUHAgEWKGh0dHA6Ly93 d3cuc3RhcnRzc2wuY29tL2ludGVybWVkaWF0ZS5wZGYwDQYJKoZIhvcNAQEFBQADggIBADqp Jw3I07QWke9plNBpxUxcffc7nUrIQpJHDci91DFG7fVhHRkMZ1J+BKg5UNUxIFJ2Z9B90Mic c/NXcs7kPBRdn6XGO/vPc87Y6R+cWS9Nc9+fp3Enmsm94OxOwI9wn8qnr/6o3mD4noP9Jphw UPTXwHovjavRnhUQHLfo/i2NG0XXgTHXS2Xm0kVUozXqpYpAdumMiB/vezj1QHQJDmUdPYMc p+reg9901zkyT3fDW/ivJVv6pWtkh6Pw2ytZT7mvg7YhX3V50Nv860cV11mocUVcqBLv0gcT +HBDYtbuvexNftwNQKD5193A7zN4vG7CTYkXxytSjKuXrpEatEiFPxWgb84nVj25SU5q/r1X hwby6mLhkbaXslkVtwEWT3Van49rKjlK4XrUKYYWtnfzq6aSak5u0Vpxd1rY79tWhD3EdCvO hNz/QplNa+VkIsrcp7+8ZhP1l1b2U6MaxIVteuVMD3X0vziIwr7jxYae9FZjbxlpUemqXjcC 0QaFfN7qI0JsQMALL7iGRBg7K0CoOBzECdD3fuZil5kU/LP9cr1BK31U0Uy651bFnAMMMkqh AChIbn0ei72VnbpSsrrSdF0BAGYQ8vyHae5aCg+H75dVCV33K6FuxZrf09yTz+Vx/PkdRUYk XmZz/OTfyJXsUOUXrym6KvI2rYpccSk5MIIHSjCCBjKgAwIBAgICHmMwDQYJKoZIhvcNAQEF BQAwgYwxCzAJBgNVBAYTAklMMRYwFAYDVQQKEw1TdGFydENvbSBMdGQuMSswKQYDVQQLEyJT ZWN1cmUgRGlnaXRhbCBDZXJ0aWZpY2F0ZSBTaWduaW5nMTgwNgYDVQQDEy9TdGFydENvbSBD bGFzcyAyIFByaW1hcnkgSW50ZXJtZWRpYXRlIENsaWVudCBDQTAeFw0xMjAzMTgyMjE0MzBa Fw0xNDAzMjAwODU3MDlaMIGMMRkwFwYDVQQNExBlQjZPRTM3UlJOUHlsNW0yMQswCQYDVQQG EwJJVDEQMA4GA1UECBMHQmVyZ2FtbzEQMA4GA1UEBxMHUm92ZXR0YTEWMBQGA1UEAxMNRmFi aW8gRmFudG9uaTEmMCQGCSqGSIb3DQEJARYXZmFudG9uaWZhYmlvQHRpc2NhbGkuaXQwggEi MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC1XhckXsX23vgJq76s2f0KT8U8Msov5QgV 10eQBb2wL/TzcmqtZotI7ztKVhio3ehHg+mfu+3EqOkX9Umgut8rP0bPi7AGjkPXbOTT/cSU Xz2Kw31VGOmiOVoUFGvpQitp3weCkhUJLBipI8EpNyBXpjtQ9yCpnIAqfuc77ybfSnCy7tTR MBq1BUkfjH1+GL45riosuS4+F+MSUvlYzLiT4rAduAX1Y2IuORDsf9Bce8GBxa6syP9rCyzl Vk7DIX5k8j2vlnyRATIypn5CQLQxGT6e0f6ac4gvWOHwO2QEBsmZKKs1ZidE4q/9OoNXYX6A jnHtp1H1vcrek/vVcs19AgMBAAGjggOyMIIDrjAJBgNVHRMEAjAAMAsGA1UdDwQEAwIEsDAd BgNVHSUEFjAUBggrBgEFBQcDAgYIKwYBBQUHAwQwHQYDVR0OBBYEFFan8cbEWWBmSTWFtLk2 YNdAcGUbMB8GA1UdIwQYMBaAFK5Vg2/sMcq59x36r2sx88gd46y7MCIGA1UdEQQbMBmBF2Zh bnRvbmlmYWJpb0B0aXNjYWxpLml0MIICIQYDVR0gBIICGDCCAhQwggIQBgsrBgEEAYG1NwEC AjCCAf8wLgYIKwYBBQUHAgEWImh0dHA6Ly93d3cuc3RhcnRzc2wuY29tL3BvbGljeS5wZGYw NAYIKwYBBQUHAgEWKGh0dHA6Ly93d3cuc3RhcnRzc2wuY29tL2ludGVybWVkaWF0ZS5wZGYw gfcGCCsGAQUFBwICMIHqMCcWIFN0YXJ0Q29tIENlcnRpZmljYXRpb24gQXV0aG9yaXR5MAMC AQEagb5UaGlzIGNlcnRpZmljYXRlIHdhcyBpc3N1ZWQgYWNjb3JkaW5nIHRvIHRoZSBDbGFz cyAyIFZhbGlkYXRpb24gcmVxdWlyZW1lbnRzIG9mIHRoZSBTdGFydENvbSBDQSBwb2xpY3ks IHJlbGlhbmNlIG9ubHkgZm9yIHRoZSBpbnRlbmRlZCBwdXJwb3NlIGluIGNvbXBsaWFuY2Ug b2YgdGhlIHJlbHlpbmcgcGFydHkgb2JsaWdhdGlvbnMuMIGcBggrBgEFBQcCAjCBjzAnFiBT dGFydENvbSBDZXJ0aWZpY2F0aW9uIEF1dGhvcml0eTADAgECGmRMaWFiaWxpdHkgYW5kIHdh cnJhbnRpZXMgYXJlIGxpbWl0ZWQhIFNlZSBzZWN0aW9uICJMZWdhbCBhbmQgTGltaXRhdGlv bnMiIG9mIHRoZSBTdGFydENvbSBDQSBwb2xpY3kuMDYGA1UdHwQvMC0wK6ApoCeGJWh0dHA6 Ly9jcmwuc3RhcnRzc2wuY29tL2NydHUyLWNybC5jcmwwgY4GCCsGAQUFBwEBBIGBMH8wOQYI KwYBBQUHMAGGLWh0dHA6Ly9vY3NwLnN0YXJ0c3NsLmNvbS9zdWIvY2xhc3MyL2NsaWVudC9j YTBCBggrBgEFBQcwAoY2aHR0cDovL2FpYS5zdGFydHNzbC5jb20vY2VydHMvc3ViLmNsYXNz Mi5jbGllbnQuY2EuY3J0MCMGA1UdEgQcMBqGGGh0dHA6Ly93d3cuc3RhcnRzc2wuY29tLzAN BgkqhkiG9w0BAQUFAAOCAQEAjzHNqifpDVMkH1TSPFZVIiQ4fh49/V5JMpstgqEZPDaDe5r8 h+fMBZtUa6LLMco03Z9BNEXlqlXKiFk8feVYB8obEjz7YYq1XhO9q7JUmkSs0WGIH4xU0XB1 kPC8T8H+5E//84poYSFHE4pA+Ff68UANP2/EuFJWMjegiefnOr8aM42OAcUkjEWSlautIIX8 oD2GizwQYjWdDDjEonbuMKFP6rY2xGI3PSLI3IVU2opb0/itNhQui3WRxafloJqTlriY8m8+ qSLr2HGftbBlbyzVWB8o//aW0H0LMabjkIvrm7Zmh2vcCxiSxGBwYASuSYXGuQiKAgGptUs1 XJLZuzGCA9owggPWAgEBMIGTMIGMMQswCQYDVQQGEwJJTDEWMBQGA1UEChMNU3RhcnRDb20g THRkLjErMCkGA1UECxMiU2VjdXJlIERpZ2l0YWwgQ2VydGlmaWNhdGUgU2lnbmluZzE4MDYG A1UEAxMvU3RhcnRDb20gQ2xhc3MgMiBQcmltYXJ5IEludGVybWVkaWF0ZSBDbGllbnQgQ0EC Ah5jMAkGBSsOAwIaBQCgggIbMBgGCSqGSIb3DQEJAzELBgkqhkiG9w0BBwEwHAYJKoZIhvcN AQkFMQ8XDTEzMDExNTE1MDMyOFowIwYJKoZIhvcNAQkEMRYEFIdGAG7nfpKiFzrU6welIK9V lHhuMGwGCSqGSIb3DQEJDzFfMF0wCwYJYIZIAWUDBAEqMAsGCWCGSAFlAwQBAjAKBggqhkiG 9w0DBzAOBggqhkiG9w0DAgICAIAwDQYIKoZIhvcNAwICAUAwBwYFKw4DAgcwDQYIKoZIhvcN AwICASgwgaQGCSsGAQQBgjcQBDGBljCBkzCBjDELMAkGA1UEBhMCSUwxFjAUBgNVBAoTDVN0 YXJ0Q29tIEx0ZC4xKzApBgNVBAsTIlNlY3VyZSBEaWdpdGFsIENlcnRpZmljYXRlIFNpZ25p bmcxODA2BgNVBAMTL1N0YXJ0Q29tIENsYXNzIDIgUHJpbWFyeSBJbnRlcm1lZGlhdGUgQ2xp ZW50IENBAgIeYzCBpgYLKoZIhvcNAQkQAgsxgZaggZMwgYwxCzAJBgNVBAYTAklMMRYwFAYD VQQKEw1TdGFydENvbSBMdGQuMSswKQYDVQQLEyJTZWN1cmUgRGlnaXRhbCBDZXJ0aWZpY2F0 ZSBTaWduaW5nMTgwNgYDVQQDEy9TdGFydENvbSBDbGFzcyAyIFByaW1hcnkgSW50ZXJtZWRp YXRlIENsaWVudCBDQQICHmMwDQYJKoZIhvcNAQEBBQAEggEAhDk90zaom3dDF203QFo3M+M3 LaPp4K82Zy3o8GI1IEUk1q77oENwZoplEK9OX2cLp79NPY5OoMMoycrnFLJq9Hk8hIgbIn39 3yxPUuY//kscgyIe1JoXWutnLTYH2smYYTW357IyEy5bxtpauHlxAuOn7YIt0DIcWWuxyPbE 5v222qJliBrmVYhJn5m74Dmm/O1V1zfwBR/UAwavhGmlaWWkinOGk7vQ+NRU8b9wFMzxoYJK DYxwQi3gzSpIuN6egLaFX9p2wHzTvDRr8DJG22TOfZ8Z9wX206IyN1a/eCbOfO1lrmLuO7yP WMGNLvWqpxVdRDy+CXN1zuknG4a0XwAAAAAAAA== --------------ms080109050106030207080901-- --===============4894879214654454373== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============4894879214654454373==--