From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [PATCH v3 02/16] tools part of add vmware_hw to xl.cfg Date: Thu, 11 Sep 2014 13:48:32 -0400 Message-ID: <5411E070.3030606@terremark.com> References: <1410182158-8542-1-git-send-email-dslutz@verizon.com> <1410182158-8542-3-git-send-email-dslutz@verizon.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit 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: George Dunlap Cc: Tim Deegan , Kevin Tian , Keir Fraser , Ian Campbell , Stefano Stabellini , Ian Jackson , Eddie Dong , Don Slutz , "xen-devel@lists.xen.org" , Jan Beulich , Aravind Gopalakrishnan , Jun Nakajima , Andrew Cooper , Boris Ostrovsky , Suravee Suthikulpanit List-Id: xen-devel@lists.xenproject.org On 09/11/14 07:23, George Dunlap wrote: > On Mon, Sep 8, 2014 at 2:15 PM, Don Slutz wrote: >> If non-zero then >> Force use of VMware's VGA in QEMU. >> >> Signed-off-by: Don Slutz > I think the title would be better "tools: Add vmware_hw support" Ok. >> --- >> docs/man/xl.cfg.pod.5 | 6 ++++++ >> tools/libxc/xc_domain_restore.c | 14 ++++++++++++++ >> tools/libxc/xc_domain_save.c | 11 +++++++++++ >> tools/libxc/xg_save_restore.h | 2 ++ >> tools/libxl/libxl_create.c | 4 +++- >> tools/libxl/libxl_dm.c | 33 +++++++++++++++++++++------------ >> tools/libxl/libxl_dom.c | 2 ++ >> tools/libxl/libxl_types.idl | 1 + >> tools/libxl/xl_cmdimpl.c | 2 ++ >> 9 files changed, 62 insertions(+), 13 deletions(-) >> >> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 >> index 517ae2f..7f7319a 100644 >> --- a/docs/man/xl.cfg.pod.5 >> +++ b/docs/man/xl.cfg.pod.5 >> @@ -1147,6 +1147,12 @@ some other Operating Systems and in some circumstance can prevent >> Xen's own paravirtualisation interfaces for HVM guests from being >> used. >> >> +=item B >> + >> +Turns on or off the exposure of VMware cpuid. The number is the >> +VMware's hardware version number, where 0 is off. If on it also >> +forces the use of VMware's VGA in QEMU. > You should also say what values are supported. Here is what I have in v4: =item B Turns on or off the exposure of VMware cpuid. The number is the VMware's hardware version number, where 0 is off. If not zero it changes the default VGA to VMware's VGA. The hardware version number (vmware_hw) come from VMware config files. =over 4 In a .vmx it is virtualHW.version In a .ovf it is part of the value of vssd:VirtualSystemType. For vssd:VirtualSystemType == vmx-07, vmware_hw = 7. =back There is a lot in the different thread: Message-ID: <54108DFB.8030804@terremark.com> Date: Wed, 10 Sep 2014 13:44:27 -0400 User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 To: Ian Campbell , Don Slutz CC: Andrew Cooper , Tim Deegan , Kevin Tian , Keir Fraser , Jun Nakajima , Stefano Stabellini , Ian Jackson , Eddie Dong , , "Aravind Gopalakrishnan" , Jan Beulich , Boris Ostrovsky , "Suravee Suthikulpanit" Subject: Re: [Xen-devel] [PATCH v2 1/3] Add vmware_hw to xl.cfg References: <1409585629-25840-1-git-send-email-dslutz@verizon.com> <1409585629-25840-2-git-send-email-dslutz@verizon.com> <1410182256.3680.16.camel@kazak.uk.xensource.com> <540E00AB.1000501@terremark.com> <1410255568.8217.65.camel@kazak.uk.xensource.com> <540F32A0.2070609@terremark.com> <1410341443.8217.260.camel@kazak.uk.xensource.com> In-Reply-To: <1410341443.8217.260.camel@kazak.uk.xensource.com> >> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c >> index 103cbca..c79274b 100644 >> --- a/tools/libxl/libxl_dm.c >> +++ b/tools/libxl/libxl_dm.c >> @@ -542,19 +542,28 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, >> } >> } >> >> - switch (b_info->u.hvm.vga.kind) { >> - case LIBXL_VGA_INTERFACE_TYPE_STD: >> - flexarray_append_pair(dm_args, "-device", >> - GCSPRINTF("VGA,vgamem_mb=%d", >> - libxl__sizekb_to_mb(b_info->video_memkb))); >> - break; >> - case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: >> + if (b_info->u.hvm.vmware_hw) { >> flexarray_append_pair(dm_args, "-device", >> - GCSPRINTF("cirrus-vga,vgamem_mb=%d", >> - libxl__sizekb_to_mb(b_info->video_memkb))); >> - break; >> - case LIBXL_VGA_INTERFACE_TYPE_NONE: >> - break; >> + GCSPRINTF("vmware-svga,vgamem_mb=%d", >> + libxl__sizekb_to_mb( >> + b_info->video_memkb))); >> + } else { >> + switch (b_info->u.hvm.vga.kind) { >> + case LIBXL_VGA_INTERFACE_TYPE_STD: >> + flexarray_append_pair(dm_args, "-device", >> + GCSPRINTF("VGA,vgamem_mb=%d", >> + libxl__sizekb_to_mb( >> + b_info->video_memkb))); >> + break; >> + case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: >> + flexarray_append_pair(dm_args, "-device", >> + GCSPRINTF("cirrus-vga,vgamem_mb=%d", >> + libxl__sizekb_to_mb( >> + b_info->video_memkb))); >> + break; >> + case LIBXL_VGA_INTERFACE_TYPE_NONE: >> + break; >> + } >> } > So if we set vmware_hw, then we: > 1. always add a vmware-svga device, regargless of whether vga has been requested > 2. Ignore the vga parameter and only add vmware-svga, even if someone > may want something else? > > I think at the libxl level, we should add a VMWARE interface type. > Then in xl_cmdimpl.c, we should: > * Add a vga="vmware" type in xl.cfg > * honor the vga= setting (perhaps with a warning if vmware_hw is true?) > * if vga is not set, and vmware_hw is true, default to vga=VMWARE In v2 and v3 yes. In v4 no. What you are suggesting I have already coded. >> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c >> index 8a38077..c30341a 100644 >> --- a/tools/libxl/xl_cmdimpl.c >> +++ b/tools/libxl/xl_cmdimpl.c >> @@ -1033,6 +1033,8 @@ static void parse_config_data(const char *config_source, >> xlu_cfg_get_defbool(config, "acpi_s4", &b_info->u.hvm.acpi_s4, 0); >> xlu_cfg_get_defbool(config, "nx", &b_info->u.hvm.nx, 0); >> xlu_cfg_get_defbool(config, "viridian", &b_info->u.hvm.viridian, 0); >> + if (!xlu_cfg_get_long(config, "vmware_hw", &l, 1)) >> + b_info->u.hvm.vmware_hw = l; >> xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0); >> xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0); > This is a really minor thing, but is there a reason you put this in > the middle of a bunch of other get_defbool()s, instead of putting it > just after? It makes the code just look a bit more ugly. :-) Not a good one, I was just following where viridian was done. Will move it down a few lines. -Don Slutz > -George